Signature validation with mismatched key size or algorithms

[mttcpr]

Not exactly a bug, more a suggestion.

Using windows, if you try to verify a 2048 bit signature with a 4096 bit key (or vice versa I assume), you get this back:

OSError: NTSTATUS error 0xC000000D: An invalid parameter was passed to a service or function

From a usability standpoint I think this condition should raise [for example] SignatureError('Signature length does not match RSA key size') instead of sending back an OSError or a ValueError for algorithm mismatch.

As a caller, I want to answer "Can I verify this signature with this public key (certificate)?" Assuming I pass in valid objects, I think it makes sense to get a "No" response (e.g. SignatureError) back and save the ValueError and OSError for stuff that can't be handled a gracefully like "I can't verify this because I don't support MD2"

For example, in CertValidator - validate.py, _self_signed(cert), if the algorithm or key size is not matched to the signature in the certificate then this error will pop past your oscrypto.errors.SignatureError catch block:

    try:
        key_object = asymmetric.load_certificate(cert)
        verify_func(key_object, cert['signature_value'].native, cert['tbs_certificate'].dump(), hash_algo)
        return True

    except (oscrypto.errors.SignatureError):
        return False

The _validate_path function has the same issue and could realistically expose this if the caller didn't ensure they were doing the algorithm matching and key size checks during path building.

It seems there are several ways you might address handling these conditions, including saying you want those errors to propagate back up and doing nothing! However, if you do agree that this should be addressed somewhere, I imagine you'd prefer the fix not be per-platform. That said, the particular case that brought this to my attention was mismatched rsa key/sig size on Windows. I noted that if this code were changed

failure = res == BcryptConst.STATUS_INVALID_SIGNATURE
failure = failure or (rsa_pss_padding and res == BcryptConst.STATUS_INVALID_PARAMETER)

to

failure = res == BcryptConst.STATUS_INVALID_SIGNATURE or res == BcryptConst.STATUS_INVALID_PARAMETER`

that it would give the desired SignatureError answer for this particular case. Given you're already abstracting away STATUS_INVALID_PARAMETER in the rsa_pss_padding case perhaps it makes sense to simply always do so as it might pop up for other unimagined conditions? Obviously the better solution for my exact use case would be to never get to that line and raise SignatureError('Signature length does not match RSA key size') farther up the stack.

Thoughts?

[wbond]

Yes, I think it would be great to improve the error messaging surrounding this. Ideally the error messages are as accurate as possible, otherwise we get into situations where we are trying to debug crypto APIs when the real issue is some data mismatch.

I think it probably will have to happen inside of oscrypto in the different backend code, but then all of the consumers can handle it gracefully.

[wbond]

For now I have 0b41acfb4ce7f5a13f8f0731c6b87f4e9723a311 in place to close this. With RSA we can explicitly check the key size, but it didn't seem obvious with DSA how we would check the key size. At least for now certvalidator won't bail on you.

If you are interested in having a more specific message when the RSA key size is incorrect, could you open a new issue?