1735925 - NULL parameter tags in signatureAlgorithm are normalized before validation

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[taviso]

I've noticed that there is no limitation on the size of the mandatory NULL after the signatureAlgorithm OID. This is a minor annoyance for me, because I can't use the signature thumbprint as a guarantee of uniqueness, maybe other people do too.

i.e. Given any valid certificate, you can create an infinite number of valid variations with different thumbprints.

The CA/B BRs actually specify the exact encodings necessary for the algorithmIdentifier (e.g. 7.1.3.2), but I think there is no way to test that a certificate complies with NSS (AFAIK).

I made a demo, I took the bugzilla.mozilla.org certificate and added some junk strings.

FWIW, OpenSSL does not accept this certificate.

-----BEGIN CERTIFICATE-----
MIIHADCCBcOgAwIBAgIQAiIkEH7zmGpfCIkTSs3VbTANBgkqhkiG9w0BAQsFADBPMQswCQYDVQQG
EwJVUzEVMBMGA1UEChMMRGlnaUNlcnQgSW5jMSkwJwYDVQQDEyBEaWdpQ2VydCBUTFMgUlNBIFNI
QTI1NiAyMDIwIENBMTAeFw0yMTA1MjAwMDAwMDBaFw0yMjA2MDkyMzU5NTlaMHgxCzAJBgNVBAYT
AlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MRswGQYDVQQK
ExJNb3ppbGxhIEZvdW5kYXRpb24xHzAdBgNVBAMMFiouYnVnemlsbGEubW96aWxsYS5vcmcwggEi
MA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCujDuA9KUZUsVGQe5WVNKFo965ZCqNDT97JgM3
lilaE6vP5kknYA1swuiRQBaThHNumMzpDPYzKFz/a14xnUDQLi/bNOkDHjjpqs7lf27npwJf05S8
VXK4kmYZg9lSspjGwNKi1vU2wzXpTBcmqkyrdfqarDajZydebHM90Bimbn2FqbqCPcnt8PrUyosW
OUFgXCxmKhePW8DGmAqpKMGyTb9VHo4ugTo9JWh9kLw1xJTNbRPBAHJ2m5rKuuzNJTQuXDD8kZfN
TxZ0e02ix5Uj6iadxgECDPsquPNwvqEdMhTddzGIpWum6kkq9v+qzfWqqTQ9jgqG2DIiBy4kR31R
AgMBAAGjggOIMIIDhDAfBgNVHSMEGDAWgBS3a6LqqKqEjHnqtNoPmLLFlXa59DAdBgNVHQ4EFgQU
yAjj9BuVudAR/cZjX64/9ViP0+0wNwYDVR0RBDAwLoIWKi5idWd6aWxsYS5tb3ppbGxhLm9yZ4IU
YnVnemlsbGEubW96aWxsYS5vcmcwDgYDVR0PAQH/BAQDAgWgMB0GA1UdJQQWMBQGCCsGAQUFBwMB
BggrBgEFBQcDAjCBiwYDVR0fBIGDMIGAMD6gPKA6hjhodHRwOi8vY3JsMy5kaWdpY2VydC5jb20v
RGlnaUNlcnRUTFNSU0FTSEEyNTYyMDIwQ0ExLmNybDA+oDygOoY4aHR0cDovL2NybDQuZGlnaWNl
cnQuY29tL0RpZ2lDZXJ0VExTUlNBU0hBMjU2MjAyMENBMS5jcmwwPgYDVR0gBDcwNTAzBgZngQwB
AgIwKTAnBggrBgEFBQcCARYbaHR0cDovL3d3dy5kaWdpY2VydC5jb20vQ1BTMH0GCCsGAQUFBwEB
BHEwbzAkBggrBgEFBQcwAYYYaHR0cDovL29jc3AuZGlnaWNlcnQuY29tMEcGCCsGAQUFBzAChjto
dHRwOi8vY2FjZXJ0cy5kaWdpY2VydC5jb20vRGlnaUNlcnRUTFNSU0FTSEEyNTYyMDIwQ0ExLmNy
dDAMBgNVHRMBAf8EAjAAMIIBfQYKKwYBBAHWeQIEAgSCAW0EggFpAWcAdgApeb7wnjk5IfBWc59j
pXflvld9nGAK+PlNXSZcJV3HhAAAAXmKWF+NAAAEAwBHMEUCIDx4BYRjAGEfCdIljSV38z7iV02/
tiGDYHZwnUC2b/UaAiEAqepa5zcq8qbsSwAO8gqaJmJiRsZPOEaXG4qKaERCkHwAdgAiRUUHWVUk
VpY/oS/x922G4CMmY63AS39dxoNcbuIPAgAAAXmKWF/cAAAEAwBHMEUCIFZ1z1/FG0bxOuWWv4fL
Q63FEoSrA8KdpQeo9DZmVArUAiEA+oqhBLKLoicHbI4ZC6OrSQv+UO5VVS4f801vs67O+44AdQBB
yMqx3yJGShDGoToJQodeTjGLGwPr60vHaPCQYpYG9gAAAXmKWF96AAAEAwBGMEQCIEk2dw83cYDP
Sqjcwl3lMLl5gO75pSlfNOHvc9H/g6JJAiBhBc/N4YbwLpZ2/vSm4jik7DKtsQcVAb1rdZIyOfhW
ijAyBgkqhkiG9w0BAQsFJRMjWW91IGNhbiBwdXQgYW55dGhpbmcgeW91IHdhbnQgaGVyZSEDggEB
AIUVhNEg6VfGe6VX2o4X2h8eYSoD463I4hcSbKyUgVK+M4jnTEw66wPEjHR2b/YB1cD+N1g/s1VG
eAMWs1+52aimZaA3+1jVMHDXSwewux/q9C2gH6zLV1iQ69t4RfdWytfEz3xb7UwR1bCOtcoJ6LQU
HTwy8FCPlJvxYs5GY8F6xQxYQdIq7Xu5LLFWs1ukfJ295Lw34OzQ9qCzxo2IvpMsbkrGc73+tM1p
nWgwrMQL+4/rbW709Xnpwy+qlgYm+X63dzWHA2N6UMchbQAGQpf0DDlx0JValba21gEgEwEit38V
JUdxXH6tGWON6knYxD+Rx+0/3mwhQBvWV5pvIog=
-----END CERTIFICATE-----

Comment 1

[Dana Keeler (she/her) [:keeler]]

What are you using to verify the certificate? certutil or vfychain, or something else?

Comment 2

[taviso]

I was just using CERT_VerifyCertificate(), but I just checked and vfychain works too (I just used vfychain -u 11 -a digicert.pem -a test.pem

Comment 3

[Ryan Sleevi]

Dana: mozpkix appears to strictly check, c.f. https://searchfox.org/mozilla-central/rev/0ec81de2037cb0a0701d5d316f42763230b3a142/security/nss/lib/mozpkix/lib/pkixder.cpp#89-90,104,122 and https://searchfox.org/mozilla-central/rev/0ec81de2037cb0a0701d5d316f42763230b3a142/security/nss/lib/mozpkix/include/pkix/pkixder.h#395-397

The issue for legacy appears to be because sec_DecodeSigAlg ignores param except for PSS keys. This is the basis for CERT_VerifySignedDataWithPublicKey (I'm ignoring checkKeyParams because that's the policy side of enforcement, and because CERT_VerifySignedDataWithPublicKey is the basis for libpkix as well).

Interestingly, legacy appears to use the outer signatureAlgorithm (signatureWrap.signatureAlgorithm) rather than the inner algorithm. Presumably, a similar check could be implemented for legacy.

mozpkix allows the inner to omit the null while the outer includes it, or vice versa. Mozilla Root Store Policy does not permit this.

For comparison, Golang ensures a byte-for-byte equality, while both BoringSSL and OpenSSL use X509_ALGOR_cmp, which tries to do a type-aware comparison, which memcmps for most types. NULL tags are generically enforced to have zero content octets.

So you have degrees of flexibility here:

• Enforce NULL tags are actually zero length
• Enforce that any parameters, if present, are also (zero-length) NULLs, like mozpkix
• Enforce that any parameters, if present, are byte-for-byte equal (Like OpenSSL, BoringSSL, Golang)
• Enforce that any parameters, if present, are byte-for-byte equal with Mozilla Policy

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:beurdouche, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 5

[John Schanck [:jschanck]]

Attachment: Bug 1735925 - QuickDER: Forbid NULL tags with non-zero length. r=#nss-reviewers

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:jschanck, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 7

[John Schanck [:jschanck]]

https://hg.mozilla.org/projects/nss/rev/3bfc72bf0ba67aa4c0f1bfe5d437511d78b22973