Account for RSA-PSS salt length special cases #19
Conversation
| case -1: // rsa.PSSSaltLengthEqualsHash | ||
| info.Salt = uint32(h.Size()) | ||
| case 0: // rsa.PSSSaltLengthAuto | ||
| err = errors.New("crypto/rsa: rsa.PSSSaltLengthAuto not supported") |
There was a problem hiding this comment.
I would rather error out and fallback to Go crypto when
rsa.PSSSaltLengthAutois used.
How will this work--upon any error? Is it possible go-crypto-winnative (and go-crypto-openssl) should each export an ErrUnsupported, wrap that error here, and then the backend checks ErrUnsupported and only falls back in those cases?
Or do I have a wrong idea about how the fallback would work? 😄
There was a problem hiding this comment.
ErrUnsupported would be ideal, as it wouldn't require polluting the code with backend-specific checks. Unfortunately it would be an important performance penalty for the fallback cases, as sometimes we don't know a set of parameters are unsupported until we have already done several backend calls.
At the moment I'm planning to just do good-old backend-specific checks:
if boring.Enabled &&
(!goexperiment.CNGCrypto || (opts.saltLength() != PSSSaltLengthAuto && hash != crypto.MD5SHA1)) {
bkey, err := boringPublicKey(pub)
if err != nil {
return err
}
if err := boring.VerifyRSAPSS(bkey, hash, digest, sig, opts.saltLength()); err != nil {
return ErrVerification
}
return nil
}
cng/rsa_test.go
Outdated
| @@ -157,11 +157,26 @@ func TestSignVerifyRSAPSS(t *testing.T) { | |||
| priv, pub := newRSAKey(t, 2048) | |||
| sha256.Write([]byte("testing")) | |||
| hashed := sha256.Sum(nil) | |||
| signed, err := cng.SignRSAPSS(priv, crypto.SHA256, hashed, 0) | |||
| signed, err := cng.SignRSAPSS(priv, crypto.SHA256, hashed, 8) | |||
There was a problem hiding this comment.
8 is an arbitrary number > 0, right? It just can't be zero because it's now a special value?
There was a problem hiding this comment.
Yep. But let me change it to 42 so it is clearer that it does not have any meaning.
This PR teaches
SignRSAPSSandVerifyRSAPSSto understand Go salt length special cases:rsa.PSSSaltLengthAutoandrsa.PSSSaltLengthEqualsHash.Unfortunately, CNG does not support them out-of-the-box.
rsa.PSSSaltLengthEqualsHashis straightforward to implement, it just means the salt length should be equal to the hash length.rsa.PSSSaltLengthAutorequires decoding and understanding the bits of the signature, thus out of our scope. I would rather error out and fallback to Go crypto whenrsa.PSSSaltLengthAutois used.Luckly it will only affect
rsa.VerifyPSS, asrsa.SignPSSis converting the salt length to a concret value before calling us.Added to Missing algorithms #4 so we don't miss this limitation in the documentation.