1649487 - secvfy - bad assert in VFY_EndWithSignature

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Alexander Scheel]

Attachment: secvfy.patch

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

Steps to reproduce:

JSS upstream test suite is failing with sandboxed build of NSS. This fails within our SSLEngine code due to suboptimal verification of certificates.

This manifests as the following:

2020-06-25T22:50:11.0913044Z FINE: - CN=localhost,OU=JSS Testing20,O=Mozilla,C=US
2020-06-25T22:50:11.0913171Z FINE: parent: CN=CACert,OU=JSS Testing20,O=Mozilla,C=US
2020-06-25T22:50:11.0913404Z FINE: - CN=CACert,OU=JSS Testing20,O=Mozilla,C=US
2020-06-25T22:50:11.0913530Z FINE: child: CN=localhost,OU=JSS Testing20,O=Mozilla,C=US
2020-06-25T22:50:11.0913801Z FINE: JSSTrustManager: - CN=CACert,OU=JSS Testing20,O=Mozilla,C=US
2020-06-25T22:50:11.0914077Z FINE: JSSTrustManager: - CN=localhost,OU=JSS Testing20,O=Mozilla,C=US
2020-06-25T22:50:11.0914212Z FINE: JSSTrustManager: getAcceptedIssuers():
2020-06-25T22:50:11.0914478Z FINE: JSSTrustManager: - CN=CACert,OU=JSS Testing50,O=Mozilla,C=US
2020-06-25T22:50:11.0914749Z FINE: JSSTrustManager: - CN=CACert,OU=JSS Testing40,O=Mozilla,C=US
2020-06-25T22:50:11.0915018Z FINE: JSSTrustManager: - CN=CACert,OU=JSS Testing30,O=Mozilla,C=US
2020-06-25T22:50:11.0915286Z FINE: JSSTrustManager: - CN=CACert,OU=JSS Testing20,O=Mozilla,C=US
2020-06-25T22:50:11.0915399Z FINE: JSSTrustManager: checkCert(CN=CACert,OU=JSS Testing20,O=Mozilla,C=US):
2020-06-25T22:50:11.0915529Z FINE: JSSTrustManager: cert AKI: null
2020-06-25T22:50:11.0915656Z FINE: JSSTrustManager: SKI of CN=CACert,OU=JSS Testing50,O=Mozilla,C=US: null
2020-06-25T22:50:11.0915940Z Assertion failure: cx->hashAlg == hashid, at ../../lib/cryptohi/secvfy.c:691
2020-06-25T22:50:11.0916026Z

In particular, because recoverPKCS1DigestInfo() can fail, the assert immediately following it is invalid. This should only be checked when rv == SECSuccess.

See attached patch. Perhaps the assert should be removed?

Actual results:

secvfy caused NSS + JSS to crash.

Expected results:

secvfy should succeed and NSS + JSS shouldn't crash.

Comment 1

[Benjamin Beurdouche [:beurdouche]]

Attachment: Bug 1649487 - Fix bad assert in VFY_EndWithSignature. r=kjacobs

Comment 2

[J.C. Jones [:jcj] (he/they)]

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