Bug 1002933: Use Strongly-typed enums more often in mozilla::pkix, r=mmc

--HG--
extra : rebase_source : 3f67f48d1f4150df0830f89e6c07bbbf3a8fc7e8

lib/mozpkix/include/pkix/enumclass.h

@@ -0,0 +1,35 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* Copyright 2013 Mozilla Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Work around missing std::bind, std::ref, std::cref in older compilers. This
+// implementation isn't intended to be complete; rather, it is the minimal
+// implementation needed to make our use of std::bind work.
+
+#ifndef mozilla_pkix__enumclass_h
+#define mozilla_pkix__enumclass_h
+
+#if defined(_MSC_VER) && (_MSC_VER < 1700)
+// Microsoft added support for "enum class" in Visual C++ 2012. Before that,
+// Visual C++ has supported typed enums for longer than that, but using typed
+// enums results in C4480: nonstandard extension used: specifying underlying
+// type for enum.
+#define MOZILLA_PKIX_ENUM_CLASS  __pragma(warning(disable: 4480)) enum
+#else
+#define MOZILLA_PKIX_ENUM_CLASS enum class
+#endif
+
+#endif // mozilla_pkix__enumclass_h

lib/mozpkix/include/pkix/pkixtypes.h

@@ -18,6 +18,7 @@
 #ifndef mozilla_pkix__pkixtypes_h
 #define mozilla_pkix__pkixtypes_h
 
+#include "pkix/enumclass.h"
 #include "pkix/ScopedPtr.h"
 #include "plarena.h"
 #include "cert.h"
@@ -35,7 +36,14 @@ typedef ScopedPtr<SECKEYPublicKey, SECKEY_DestroyPublicKey>
 
 typedef unsigned int KeyUsages;
 
-enum EndEntityOrCA { MustBeEndEntity, MustBeCA };
+MOZILLA_PKIX_ENUM_CLASS EndEntityOrCA { MustBeEndEntity = 0, MustBeCA = 1 };
+
+MOZILLA_PKIX_ENUM_CLASS TrustLevel {
+  TrustAnchor = 1,        // certificate is a trusted root CA certificate or
+                          // equivalent *for the given policy*.
+  ActivelyDistrusted = 2, // certificate is known to be bad
+  InheritsTrust = 3       // certificate must chain to a trust anchor
+};
 
 // Applications control the behavior of path building and verification by
 // implementing the TrustDomain interface. The TrustDomain is used for all
@@ -46,13 +54,6 @@ class TrustDomain
 public:
   virtual ~TrustDomain() { }
 
-  enum TrustLevel {
-    TrustAnchor = 1,        // certificate is a trusted root CA certificate or
-                            // equivalent *for the given policy*.
-    ActivelyDistrusted = 2, // certificate is known to be bad
-    InheritsTrust = 3       // certificate must chain to a trust anchor
-  };
-
   // Determine the level of trust in the given certificate for the given role.
   // This will be called for every certificate encountered during path
   // building.

lib/mozpkix/lib/pkixbuild.cpp

@@ -128,7 +128,7 @@ BuildForwardInner(TrustDomain& trustDomain,
   PORT_Assert(potentialIssuerCertToDup);
 
   BackCert potentialIssuer(potentialIssuerCertToDup, &subject,
-                           BackCert::ExcludeCN);
+                           BackCert::IncludeCN::No);
   Result rv = potentialIssuer.Init();
   if (rv != Success) {
     return rv;
@@ -157,12 +157,12 @@ BuildForwardInner(TrustDomain& trustDomain,
   }
 
   unsigned int newSubCACount = subCACount;
-  if (endEntityOrCA == MustBeCA) {
+  if (endEntityOrCA == EndEntityOrCA::MustBeCA) {
     newSubCACount = subCACount + 1;
   } else {
     PR_ASSERT(newSubCACount == 0);
   }
-  rv = BuildForward(trustDomain, potentialIssuer, time, MustBeCA,
+  rv = BuildForward(trustDomain, potentialIssuer, time, EndEntityOrCA::MustBeCA,
                     KU_KEY_CERT_SIGN, requiredEKUIfPresent, requiredPolicy,
                     nullptr, newSubCACount, results);
   if (rv != Success) {
@@ -204,7 +204,7 @@ BuildForward(TrustDomain& trustDomain,
 
   Result rv;
 
-  TrustDomain::TrustLevel trustLevel;
+  TrustLevel trustLevel;
   // If this is an end-entity and not a trust anchor, we defer reporting
   // any error found here until after attempting to find a valid chain.
   // See the explanation of error prioritization in pkix.h.
@@ -215,15 +215,15 @@ BuildForward(TrustDomain& trustDomain,
                                         subCACount, &trustLevel);
   PRErrorCode deferredEndEntityError = 0;
   if (rv != Success) {
-    if (endEntityOrCA == MustBeEndEntity &&
-        trustLevel != TrustDomain::TrustAnchor) {
+    if (endEntityOrCA == EndEntityOrCA::MustBeEndEntity &&
+        trustLevel != TrustLevel::TrustAnchor) {
       deferredEndEntityError = PR_GetError();
     } else {
       return rv;
     }
   }
 
-  if (trustLevel == TrustDomain::TrustAnchor) {
+  if (trustLevel == TrustLevel::TrustAnchor) {
     ScopedCERTCertList certChain(CERT_NewCertList());
     if (!certChain) {
       PR_SetError(SEC_ERROR_NO_MEMORY, 0);
@@ -346,13 +346,13 @@ BuildCertChain(TrustDomain& trustDomain,
 
   // XXX: Support the legacy use of the subject CN field for indicating the
   // domain name the certificate is valid for.
-  BackCert::ConstrainedNameOptions cnOptions
-    = endEntityOrCA == MustBeEndEntity &&
+  BackCert::IncludeCN includeCN
+    = endEntityOrCA == EndEntityOrCA::MustBeEndEntity &&
       requiredEKUIfPresent == SEC_OID_EXT_KEY_USAGE_SERVER_AUTH
-    ? BackCert::IncludeCN
-    : BackCert::ExcludeCN;
+    ? BackCert::IncludeCN::Yes
+    : BackCert::IncludeCN::No;
 
-  BackCert cert(certToDup, nullptr, cnOptions);
+  BackCert cert(certToDup, nullptr, includeCN);
   Result rv = cert.Init();
   if (rv != Success) {
     return SECFailure;

lib/mozpkix/lib/pkixcheck.cpp

@@ -51,7 +51,7 @@ CheckKeyUsage(EndEntityOrCA endEntityOrCA,
     // certificate signatures unless the certificate is a trust anchor, to
     // reduce the chances of an end-entity certificate being abused as a CA
     // certificate.
-    // if (endEntityOrCA == MustBeCA && !isTrustAnchor) {
+    // if (endEntityOrCA == EndEntityOrCA::MustBeCA && !isTrustAnchor) {
     //   return Fail(RecoverableError, SEC_ERROR_INADEQUATE_KEY_USAGE);
     // }
     //
@@ -77,7 +77,7 @@ CheckKeyUsage(EndEntityOrCA endEntityOrCA,
     return Fail(RecoverableError, SEC_ERROR_INADEQUATE_KEY_USAGE);
   }
 
-  if (endEntityOrCA == MustBeCA) {
+  if (endEntityOrCA == EndEntityOrCA::MustBeCA) {
    // "If the keyUsage extension is present, then the subject public key
    //  MUST NOT be used to verify signatures on certificates or CRLs unless
    //  the corresponding keyCertSign or cRLSign bit is set."
@@ -129,7 +129,7 @@ CheckCertificatePolicies(BackCert& cert, EndEntityOrCA endEntityOrCA,
   // trusted for, so we cannot require the policies to be present in those
   // certificates. Instead, the determination of which roots are trusted for
   // which policies is made by the TrustDomain's GetCertTrust method.
-  if (isTrustAnchor && endEntityOrCA == MustBeCA) {
+  if (isTrustAnchor && endEntityOrCA == EndEntityOrCA::MustBeCA) {
     return Success;
   }
 
@@ -150,7 +150,7 @@ CheckCertificatePolicies(BackCert& cert, EndEntityOrCA endEntityOrCA,
       return Success;
     }
     // Intermediate certs are allowed to have the anyPolicy OID
-    if (endEntityOrCA == MustBeCA &&
+    if (endEntityOrCA == EndEntityOrCA::MustBeCA &&
         (*policyInfos)->oid == SEC_OID_X509_ANY_POLICY) {
       return Success;
     }
@@ -248,7 +248,7 @@ CheckBasicConstraints(const BackCert& cert,
     // constraints as CAs.
     //
     // TODO: add check for self-signedness?
-    if (endEntityOrCA == MustBeCA && isTrustAnchor) {
+    if (endEntityOrCA == EndEntityOrCA::MustBeCA && isTrustAnchor) {
       const CERTCertificate* nssCert = cert.GetNSSCert();
       // We only allow trust anchor CA certs to omit the
       // basicConstraints extension if they are v1. v1 is encoded
@@ -260,7 +260,7 @@ CheckBasicConstraints(const BackCert& cert,
     }
   }
 
-  if (endEntityOrCA == MustBeEndEntity) {
+  if (endEntityOrCA == EndEntityOrCA::MustBeEndEntity) {
     // CA certificates are not trusted as EE certs.
 
     if (basicConstraints.isCA) {
@@ -280,7 +280,7 @@ CheckBasicConstraints(const BackCert& cert,
     return Success;
   }
 
-  PORT_Assert(endEntityOrCA == MustBeCA);
+  PORT_Assert(endEntityOrCA == EndEntityOrCA::MustBeCA);
 
   // End-entity certificates are not allowed to act as CA certs.
   if (!basicConstraints.isCA) {
@@ -307,7 +307,7 @@ BackCert::GetConstrainedNames(/*out*/ const CERTGeneralName** result)
 
     constrainedNames =
       CERT_GetConstrainedCertificateNames(nssCert, arena.get(),
-                                          cnOptions == IncludeCN);
+                                          includeCN == IncludeCN::Yes);
     if (!constrainedNames) {
       return MapSECStatus(SECFailure);
     }
@@ -397,7 +397,7 @@ CheckExtendedKeyUsage(EndEntityOrCA endEntityOrCA, const SECItem* encodedEKUs,
         // COMODO has issued certificates that require this behavior
         // that don't expire until June 2020!
         // TODO 982932: Limit this expection to old certificates
-        if (endEntityOrCA == MustBeCA &&
+        if (endEntityOrCA == EndEntityOrCA::MustBeCA &&
             requiredEKU == SEC_OID_EXT_KEY_USAGE_SERVER_AUTH &&
             oidTag == SEC_OID_NS_KEY_USAGE_GOVT_APPROVED) {
           found = true;
@@ -417,7 +417,7 @@ CheckExtendedKeyUsage(EndEntityOrCA endEntityOrCA, const SECItem* encodedEKUs,
 
   // pkixocsp.cpp depends on the following additional checks.
 
-  if (endEntityOrCA == MustBeEndEntity) {
+  if (endEntityOrCA == EndEntityOrCA::MustBeEndEntity) {
     // When validating anything other than an delegated OCSP signing cert,
     // reject any cert that also claims to be an OCSP responder, because such
     // a cert does not make sense. For example, if an SSL certificate were to
@@ -461,23 +461,23 @@ CheckIssuerIndependentProperties(TrustDomain& trustDomain,
                                  SECOidTag requiredEKUIfPresent,
                                  SECOidTag requiredPolicy,
                                  unsigned int subCACount,
-                /*optional out*/ TrustDomain::TrustLevel* trustLevelOut)
+                /*optional out*/ TrustLevel* trustLevelOut)
 {
   Result rv;
 
-  TrustDomain::TrustLevel trustLevel;
+  TrustLevel trustLevel;
   rv = MapSECStatus(trustDomain.GetCertTrust(endEntityOrCA,
                                              requiredPolicy,
                                              cert.GetNSSCert(),
                                              &trustLevel));
   if (rv != Success) {
     return rv;
   }
-  if (trustLevel == TrustDomain::ActivelyDistrusted) {
+  if (trustLevel == TrustLevel::ActivelyDistrusted) {
     return Fail(RecoverableError, SEC_ERROR_UNTRUSTED_CERT);
   }
-  if (trustLevel != TrustDomain::TrustAnchor &&
-      trustLevel != TrustDomain::InheritsTrust) {
+  if (trustLevel != TrustLevel::TrustAnchor &&
+      trustLevel != TrustLevel::InheritsTrust) {
     // The TrustDomain returned a trust level that we weren't expecting.
     PORT_SetError(PR_INVALID_STATE_ERROR);
     return FatalError;
@@ -486,8 +486,8 @@ CheckIssuerIndependentProperties(TrustDomain& trustDomain,
     *trustLevelOut = trustLevel;
   }
 
-  bool isTrustAnchor = endEntityOrCA == MustBeCA &&
-                       trustLevel == TrustDomain::TrustAnchor;
+  bool isTrustAnchor = endEntityOrCA == EndEntityOrCA::MustBeCA &&
+                       trustLevel == TrustLevel::TrustAnchor;
 
   PLArenaPool* arena = cert.GetArena();
   if (!arena) {

lib/mozpkix/lib/pkixcheck.h

@@ -32,7 +32,7 @@ Result CheckIssuerIndependentProperties(
           SECOidTag requiredEKUIfPresent,
           SECOidTag requiredPolicy,
           unsigned int subCACount,
-          /*optional out*/ TrustDomain::TrustLevel* trustLevel = nullptr);
+          /*optional out*/ TrustLevel* trustLevel = nullptr);
 
 Result CheckNameConstraints(BackCert& cert);
 

lib/mozpkix/lib/pkixder.h

@@ -18,6 +18,7 @@
 #ifndef mozilla_pkix__pkixder_h
 #define mozilla_pkix__pkixder_h
 
+#include "pkix/enumclass.h"
 #include "pkix/nullptr.h"
 
 #include "prerror.h"
@@ -61,7 +62,7 @@ enum Result
   Success = 0
 };
 
-enum EmptyAllowed { MayBeEmpty = 0, MustNotBeEmpty = 1 };
+MOZILLA_PKIX_ENUM_CLASS EmptyAllowed { No = 0, Yes = 1 };
 
 Result Fail(PRErrorCode errorCode);
 
@@ -324,7 +325,7 @@ NestedOf(Input& input, uint8_t outerTag, uint8_t innerTag,
   }
 
   if (inner.AtEnd()) {
-    if (mayBeEmpty != MayBeEmpty) {
+    if (mayBeEmpty != EmptyAllowed::Yes) {
       return Fail(SEC_ERROR_BAD_DER);
     }
     return Success;

lib/mozpkix/lib/pkixocsp.cpp

@@ -26,13 +26,6 @@
 #include "pk11pub.h"
 #include "secder.h"
 
-#ifdef _MSC_VER
-// C4480: nonstandard extension used: specifying underlying type for enum
-#define ENUM_CLASS  __pragma(warning(disable: 4480)) enum
-#else
-#define ENUM_CLASS enum class
-#endif
-
 // TODO: use typed/qualified typedefs everywhere?
 // TODO: When should we return SEC_ERROR_OCSP_UNAUTHORIZED_RESPONSE?
 
@@ -43,7 +36,7 @@ static const PRTime ONE_DAY
 static const PRTime SLOP = ONE_DAY;
 
 // These values correspond to the tag values in the ASN.1 CertStatus
-ENUM_CLASS CertStatus : uint8_t {
+MOZILLA_PKIX_ENUM_CLASS CertStatus : uint8_t {
   Good = der::CONTEXT_SPECIFIC | 0,
   Revoked = der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 1,
   Unknown = der::CONTEXT_SPECIFIC | 2
@@ -96,7 +89,7 @@ CheckOCSPResponseSignerCert(TrustDomain& trustDomain,
 {
   Result rv;
 
-  BackCert cert(&potentialSigner, nullptr, BackCert::ExcludeCN);
+  BackCert cert(&potentialSigner, nullptr, BackCert::IncludeCN::No);
   rv = cert.Init();
   if (rv != Success) {
     return rv;
@@ -126,7 +119,7 @@ CheckOCSPResponseSignerCert(TrustDomain& trustDomain,
   // TODO(bug 926261): If we're validating for a policy then the policy OID we
   // are validating for should be passed to CheckIssuerIndependentProperties.
   rv = CheckIssuerIndependentProperties(trustDomain, cert, time,
-                                        MustBeEndEntity, 0,
+                                        EndEntityOrCA::MustBeEndEntity, 0,
                                         SEC_OID_OCSP_RESPONDER,
                                         SEC_OID_X509_ANY_POLICY, 0);
   if (rv != Success) {
@@ -157,12 +150,7 @@ CheckOCSPResponseSignerCert(TrustDomain& trustDomain,
   return Success;
 }
 
-//typedef enum {
-//    ocspResponderID_byName = 1,
-//    ocspResponderID_byKey = 2
-//} ResponderIDType;
-
-ENUM_CLASS ResponderIDType : uint8_t
+MOZILLA_PKIX_ENUM_CLASS ResponderIDType : uint8_t
 {
   byName = der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 1,
   byKey = der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 2
@@ -580,7 +568,7 @@ ResponseData(der::Input& input, Context& context,
   // responder will never return an empty response, and handling the case of an
   // empty response makes things unnecessarily complicated.
   if (der::NestedOf(input, der::SEQUENCE, der::SEQUENCE,
-                    der::MustNotBeEmpty,
+                    der::EmptyAllowed::No,
                     bind(SingleResponse, _1, ref(context))) != der::Success) {
     return der::Failure;
   }
@@ -866,7 +854,7 @@ CheckExtensionsForCriticality(der::Input& input)
   // Extension, which is invalid (der::MayBeEmpty should really be
   // der::MustNotBeEmpty).
   return der::NestedOf(input, der::SEQUENCE, der::SEQUENCE,
-                       der::MayBeEmpty, CheckExtensionForCriticality);
+                       der::EmptyAllowed::Yes, CheckExtensionForCriticality);
 }
 
 //   1. The certificate identified in a received response corresponds to