1893404 - NSS doesn't allow for ecdsa signatures shorter than maximum length when using softoken

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P3)

Description

[fkrenzel]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:124.0) Gecko/20100101 Firefox/124.0

Steps to reproduce:

So this was a RedHat customer report,
when verifying ecdsa signature

Actual results:

Some of the signatures fail.
For ecdsa p521 it was 1/4 signatures.

Expected results:

Signatures should be valid.

Comment 1

[fkrenzel]

Please assign me to the bug, i already have a fix

Comment 2

[fkrenzel]

Attachment: Bug 1893404 - Allow for non-full length ecdsa signature when using softoken, r=rrelyea

Comment 3

[Anna Weine]

Hi!

Thanks for opening this bug.
Could you give me an example of such signature?

Comment 4

[fkrenzel]

They are in the gtest in the patch, I can look for other a pem signature if you want.
Let me elaborate:
AFAIK: Softoken requires full-sized signatures so for p254 this would be 32+32 = 64, in some cases the r and s can be for instance 31 bit long, which will fail on

if (key->publicValue.len != 65 ||
        digest->len == 0 ||
        signature->len != 64) {
        PORT_SetError(SEC_ERROR_INPUT_LEN);
        res = SECFailure;
        return res;
}

specificaly the signature->len != 64
This is correct by the pkcs #11 standard that demands that only the r and s components are padded to the same length but it doesn't require the signature to be 64Bytes
https://docs.oasis-open.org/pkcs11/pkcs11-curr/v3.0/os/pkcs11-curr-v3.0-os.html#_Toc30061178

Comment 5

[Anna Weine]

Same source: For applications, it is recommended to encode the signature as an octet string of length two times nLen if possible. We follow this strategy.

Comment 6

[fkrenzel]

I suppose that is the case for signatures generated by nss, but there are signature generated outside of nss that are shorter then the maximum size, we have reached to java for them to consider doing the padding on their side. Yet I still suggest we don't strictly enforce the signature length to the max, and allow NSS to pad it.

Comment 7

[John Schanck [:jschanck]]

I agree with fkrenzel here. Section 6.3.1 of PKCS#11 3.1 says:

For signatures created by a token, the resulting signature is always of length 2nLen. For signatures passed to a token for verification, the signature may have a shorter length but must be composed as specified before.

where "composed as specified before" means that

The signature octets correspond to the concatenation of the ECDSA values r and s, both represented as an octet string of equal length of at most nLen with the most significant byte first

I'm curious why the report mentions P521 and the patch only touches the P256 code. Is it because RedHat's copy of NSS already uses Cryspen's implementation of P384 and P521? Did the old P384/P521 code accept short signatures?

Comment 8

[John Schanck [:jschanck]]

ni for my question in Comment 7

Comment 9

[fkrenzel]

@jschanck, Yes, to both questions.

Comment 10

[Robert Relyea]

AS John surmised, we discovered this when we picked up the pending patched in bug 1854438 and bug 1854439 to fix or at least mitigate CVE-2023-6135. In 256 and 384 curves, the high byte in the order is 0xf, which means both r and s must have values of 0x0 in the high byte (which happens 1 in 256 each or 1 in 512 signatures), frequent enough to generate mysterious failures, not frequent enough to easily reproduce. The 521 curve has 0x1 in the high byte of the order, so it fails when r and s are 0 in that high byte (1 in 2 each or 1 in 4 signatures), which is easily reproducible. I'll add notes to those bugs so we can mitigate those issues before they land in main tree.

This issue was partially masked because NSS always uses the full encoding and zero pads short lengths. When we are in FIPS mode, Java uses softoken so it gets FIPS rated cryptography (rather then it's own non-FIPS validated provider). It doesn't do the padding, which is how we found this.

Comment 11

[John Schanck [:jschanck]]

https://hg.mozilla.org/projects/nss/rev/fe25eb4363fcc9bc2312274974cbf9e99a60020c