DsaTest.testModifiedSignatures doesn't report errors about wycheproof HOT 5 CLOSED

Thanks for your bug report.

Below is a reply from bleichen who can't access GitHub for now.

The signature that is accepted by the SUN provider is a legacy signature.
The provider is deliberately accepting the signature to be compatible with
some legacy implementation that formatted DSA signatures incorrectly.
I don't know what this legacy implementation is or whether it is still in use.
Since it is possible to generate a valid signature from the invalid one
by fixing the ASN format this does not introduce a vulnerability.

We are currently working on switching the tests to a new version that
is using a JSON file for the test vectors. Each test vector has a status
that is either "valid", "invalid" or "acceptable". The expectation is that
a library accepts all signatures that are marked as "valid", rejects
all signatures marked as "invalid" and is free to accept or reject
signatures marked as "acceptable". The signature discussed above
is marked as "acceptable".

BTW, it still makes sense to test vectors marked as "acceptable", because
they should of course not crash an implementation. For Java implementations
we additionally expect for example that verify either returns a boolean or
throws a SignatureException. E.g., so that other exceptions are a clear sign
that there is a bug in the code or configuration.

from wycheproof.

Sorry for the late reply.

It is probably possible to do some stricter checking now. When I wrote the first tests there
were too many libraries that did not care about ASN parsing, and so BER vs DER issues
had to be ignored just so that the bugs did not pass unnoticed.
I'm also working on documenting the test vectors better, so that it should be at least
possible to ignore test vectors for those cases where an implementer disagrees.

from wycheproof.

Thanks for the explanation. That makes sense so I think the issue is solved for me. It would still be useful if, in the future, those accepted modified signatures could optionally be reported (like for avoiding malleability issues) but the tests are already very useful as they are.

from wycheproof.

from wycheproof.

@bbc2 check out the flags in https://github.com/google/wycheproof/blob/master/testvectors/ecdsa_secp256r1_sha256_test.json and other JSON files.

There might be future changes, but I guess this should enough if you want to skip certain malleability tests.

Please reopen if you have any further comments or questions.

from wycheproof.