bug 1058812 - (1/3) mozilla::pkix: add SignatureAlgorithm::unsupported_algorithm to better handle e.g. roots signed with RSA/MD5 r=briansmith
--- a/security/pkix/include/pkix/pkixtypes.h
+++ b/security/pkix/include/pkix/pkixtypes.h
@@ -69,16 +69,19 @@ MOZILLA_PKIX_ENUM_CLASS SignatureAlgorit
// sha-1WithRSAEncryption (OID 1.2.840.113549.1.1.5, RFC 3279 Section 2.2.1)
rsa_pkcs1_with_sha1 = 16,
// id-dsa-with-sha256 (OID 2.16.840.1.101.3.4.3.2, RFC 5758 Section 3.1)
dsa_with_sha256 = 17,
// id-dsa-with-sha1 (OID 1.2.840.10040.4.3, RFC 3279 Section 2.2.2)
dsa_with_sha1 = 18,
+
+ // Used to indicate any unsupported algorithm.
+ unsupported_algorithm = 19,
};
struct SignedDataWithSignature
{
public:
Input data;
SignatureAlgorithm algorithm;
Input signature;
--- a/security/pkix/lib/pkixbuild.cpp
+++ b/security/pkix/lib/pkixbuild.cpp
@@ -176,18 +176,18 @@ PathBuildingStep::Check(Input potentialI
// subject public key MUST NOT be used to verify signatures on certificates
// or CRLs unless the corresponding keyCertSign or cRLSign bit is set."
rv = BuildForward(trustDomain, potentialIssuer, time, KeyUsage::keyCertSign,
requiredEKUIfPresent, requiredPolicy, nullptr, subCACount);
if (rv != Success) {
return RecordResult(rv, keepGoing);
}
- rv = trustDomain.VerifySignedData(subject.GetSignedData(),
- potentialIssuer.GetSubjectPublicKeyInfo());
+ rv = WrappedVerifySignedData(trustDomain, subject.GetSignedData(),
+ potentialIssuer.GetSubjectPublicKeyInfo());
if (rv != Success) {
return RecordResult(rv, keepGoing);
}
// We avoid doing revocation checking for expired certificates because OCSP
// responders are allowed to forget about expired certificates, and many OCSP
// responders return an error when asked for the status of an expired
// certificate.
--- a/security/pkix/lib/pkixder.cpp
+++ b/security/pkix/lib/pkixder.cpp
@@ -229,18 +229,17 @@ SignatureAlgorithmOIDValue(Reader& algor
} else if (algorithmID.MatchRest(id_dsa_with_sha1)) {
algorithm = SignatureAlgorithm::dsa_with_sha1;
} else if (algorithmID.MatchRest(id_dsa_with_sha256)) {
algorithm = SignatureAlgorithm::dsa_with_sha256;
} else if (algorithmID.MatchRest(sha1WithRSASignature)) {
// XXX(bug 1042479): recognize this old OID for compatibility.
algorithm = SignatureAlgorithm::rsa_pkcs1_with_sha1;
} else {
- // Any MD5-based signature algorithm, or any unknown signature algorithm.
- return Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED;
+ algorithm = SignatureAlgorithm::unsupported_algorithm;
}
return Success;
}
template <typename OidValueParser, typename Algorithm>
Result
AlgorithmIdentifier(OidValueParser oidValueParser, Reader& input,
--- a/security/pkix/lib/pkixnss.cpp
+++ b/security/pkix/lib/pkixnss.cpp
@@ -129,16 +129,17 @@ VerifySignedData(const SignedDataWithSig
case SignatureAlgorithm::dsa_with_sha256:
pubKeyAlg = SEC_OID_ANSIX9_DSA_SIGNATURE;
digestAlg = SEC_OID_SHA256;
break;
case SignatureAlgorithm::dsa_with_sha1:
pubKeyAlg = SEC_OID_ANSIX9_DSA_SIGNATURE;
digestAlg = SEC_OID_SHA1;
break;
+ case SignatureAlgorithm::unsupported_algorithm:
default:
PR_NOT_REACHED("unknown signature algorithm");
return Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED;
}
Result rv;
ScopedSECKeyPublicKey pubKey;
rv = CheckPublicKeySize(subjectPublicKeyInfo, pubKey);
--- a/security/pkix/lib/pkixocsp.cpp
+++ b/security/pkix/lib/pkixocsp.cpp
@@ -125,18 +125,18 @@ CheckOCSPResponseSignerCert(TrustDomain&
// XXX(bug 926270) XXX(bug 1008133) XXX(bug 980163): Improve name
// comparison.
// TODO: needs test
if (!InputsAreEqual(potentialSigner.GetIssuer(), issuerSubject)) {
return Result::ERROR_OCSP_RESPONDER_CERT_INVALID;
}
// TODO(bug 926260): check name constraints
- rv = trustDomain.VerifySignedData(potentialSigner.GetSignedData(),
- issuerSubjectPublicKeyInfo);
+ rv = WrappedVerifySignedData(trustDomain, potentialSigner.GetSignedData(),
+ issuerSubjectPublicKeyInfo);
// TODO: check for revocation of the OCSP responder certificate unless no-check
// or the caller forcing no-check. To properly support the no-check policy, we'd
// need to enforce policy constraints from the issuerChain.
return rv;
}
@@ -202,17 +202,17 @@ MatchResponderID(TrustDomain& trustDomai
}
}
static Result
VerifyOCSPSignedData(TrustDomain& trustDomain,
const SignedDataWithSignature& signedResponseData,
Input spki)
{
- Result rv = trustDomain.VerifySignedData(signedResponseData, spki);
+ Result rv = WrappedVerifySignedData(trustDomain, signedResponseData, spki);
if (rv == Result::ERROR_BAD_SIGNATURE) {
rv = Result::ERROR_OCSP_BAD_SIGNATURE;
}
return rv;
}
// RFC 6960 section 4.2.2.2: The OCSP responder must either be the issuer of
// the cert or it must be a delegated OCSP response signing cert directly
--- a/security/pkix/lib/pkixutil.h
+++ b/security/pkix/lib/pkixutil.h
@@ -188,11 +188,25 @@ DaysBeforeYear(unsigned int year)
{
assert(year <= 9999);
return ((year - 1u) * 365u)
+ ((year - 1u) / 4u) // leap years are every 4 years,
- ((year - 1u) / 100u) // except years divisible by 100,
+ ((year - 1u) / 400u); // except years divisible by 400.
}
+// Ensures that we do not call the TrustDomain's VerifySignedData function if
+// the algorithm is unsupported.
+inline Result
+WrappedVerifySignedData(TrustDomain& trustDomain,
+ const SignedDataWithSignature& signedData,
+ Input subjectPublicKeyInfo)
+{
+ if (signedData.algorithm == SignatureAlgorithm::unsupported_algorithm) {
+ return Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED;
+ }
+
+ return trustDomain.VerifySignedData(signedData, subjectPublicKeyInfo);
+}
+
} } // namespace mozilla::pkix
#endif // mozilla_pkix__pkixutil_h