Skip to content

Commit

Permalink
Bug 1366464 - compare signature and signatureAlgorithm fields in lega…
Browse files Browse the repository at this point in the history
…cy certificate verifier. r=rrelyea

Differential Revision: https://phabricator.services.mozilla.com/D148414

--HG--
extra : moz-landing-system : lando
  • Loading branch information
jschanck committed Jun 7, 2022
1 parent 71407db commit 3fe6a46
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 0 deletions.
10 changes: 10 additions & 0 deletions lib/certhigh/certvfy.c
Expand Up @@ -737,6 +737,16 @@ cert_VerifyCertChainOld(CERTCertDBHandle *handle, CERTCertificate *cert,
LOG_ERROR_OR_EXIT(log, subjectCert, count, 0);
}

/* check that the signatureAlgorithm field of the certificate
* matches the signature field of the tbsCertificate */
if (SECOID_CompareAlgorithmID(
&subjectCert->signatureWrap.signatureAlgorithm,
&subjectCert->signature)) {
PORT_SetError(SEC_ERROR_ALGORITHM_MISMATCH);
LOG_ERROR(log, subjectCert, count, 0);
goto loser;
}

/* find the certificate of the issuer */
issuerCert = CERT_FindCertIssuer(subjectCert, t, certUsage);
if (!issuerCert) {
Expand Down
3 changes: 3 additions & 0 deletions lib/util/SECerrs.h
Expand Up @@ -558,3 +558,6 @@ ER3(SEC_ERROR_POLICY_LOCKED, (SEC_ERROR_BASE + 180),

ER3(SEC_ERROR_SIGNATURE_ALGORITHM_DISABLED, (SEC_ERROR_BASE + 181),
"Could not create or verify a signature using a signature algorithm that is disabled because it is not secure.")

ER3(SEC_ERROR_SIGNATURE_ALGORITHM_DISABLED, (SEC_ERROR_BASE + 181),

This comment has been minimized.

Copy link
@roytam1

roytam1 Jun 17, 2022

should this one be "SEC_ERROR_ALGORITHM_MISMATCH" instead of "SEC_ERROR_SIGNATURE_ALGORITHM_DISABLED"?

This comment has been minimized.

Copy link
@jschanck

jschanck Jun 17, 2022

Author Contributor

Yes, thanks for pointing this out. I've opened Bug 1774720 for this.

This comment has been minimized.

Copy link
@roytam1

roytam1 Jun 17, 2022

I've opened Bug 1774720 for this.

Thanks for opening a new issue about this.
As original bugzilla entry and phabricator URL is inaccessible to me, I don't know what should I do at the moment.

This comment has been minimized.

Copy link
@jschanck

jschanck Jun 17, 2022

Author Contributor

I was just letting you know that I filed a patch to fix the issue. You don't need to do anything. Thanks again.

"The signature algorithm in the signature field of the certificate does not match the algorithm in its signatureAlgorithm field.")
1 change: 1 addition & 0 deletions lib/util/secerr.h
Expand Up @@ -214,6 +214,7 @@ typedef enum {

SEC_ERROR_POLICY_LOCKED = (SEC_ERROR_BASE + 180),
SEC_ERROR_SIGNATURE_ALGORITHM_DISABLED = (SEC_ERROR_BASE + 181),
SEC_ERROR_ALGORITHM_MISMATCH = (SEC_ERROR_BASE + 182),

/* Add new error codes above here. */
SEC_ERROR_END_OF_LIST
Expand Down

0 comments on commit 3fe6a46

Please sign in to comment.