Bug 1029364: Centralize version parsing in BackCert::Init, r=cviecco

--HG--
extra : rebase_source : 79d5f29c2af1ec77d6bb8a7936bb0a17f28e8d52

security/manager/ssl/tests/unit/test_cert_version.js

@@ -85,7 +85,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v1_int-v1_ca.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v1_int-v1_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v1_int-v1_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v1_int-v1_ca.der'), SEC_ERROR_BAD_DER);
 
   // v1 intermediate with v3 extensions. CA is invalid.
   ca_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
@@ -97,7 +97,7 @@ function run_test() {
   check_cert_err(cert_from_file('v2_bc_ee-v1_int_bc-v1_ca.der'), ee_error);
   check_cert_err(cert_from_file('v3_missing_bc_ee-v1_int_bc-v1_ca.der'), ee_error);
   check_cert_err(cert_from_file('v3_bc_ee-v1_int_bc-v1_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v1_int_bc-v1_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v1_int_bc-v1_ca.der'), SEC_ERROR_BAD_DER);
 
   // A v2 intermediate with a v1 CA
   ca_error = SEC_ERROR_CA_CERT_INVALID;
@@ -110,7 +110,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v2_int-v1_ca.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v2_int-v1_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v2_int-v1_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v2_int-v1_ca.der'), SEC_ERROR_BAD_DER);
 
   // A v2 intermediate with basic constraints (not allowed in insanity)
   ca_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
@@ -122,7 +122,7 @@ function run_test() {
   check_cert_err(cert_from_file('v2_bc_ee-v2_int_bc-v1_ca.der'), ee_error);
   check_cert_err(cert_from_file('v3_missing_bc_ee-v2_int_bc-v1_ca.der'), ee_error);
   check_cert_err(cert_from_file('v3_bc_ee-v2_int_bc-v1_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v2_int_bc-v1_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v2_int_bc-v1_ca.der'), SEC_ERROR_BAD_DER);
 
   // Section is OK. A x509 v3 CA MUST have bc
   // http://tools.ietf.org/html/rfc5280#section-4.2.1.9
@@ -136,7 +136,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v3_int_missing_bc-v1_ca.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v3_int_missing_bc-v1_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v3_int_missing_bc-v1_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v3_int_missing_bc-v1_ca.der'), SEC_ERROR_BAD_DER);
 
   // It is valid for a v1 ca to sign a v3 intemediate.
   check_ok_ca(cert_from_file('v3_int-v1_ca.der'));
@@ -148,7 +148,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v3_int-v1_ca.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v3_int-v1_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v3_int-v1_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v3_int-v1_ca.der'), SEC_ERROR_BAD_DER);
 
   // The next groups change the v1 ca for a v1 ca with base constraints
   // (invalid trust anchor). The error pattern is the same as the groups
@@ -165,7 +165,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v1_int-v1_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v1_int-v1_ca_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v1_int-v1_ca_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v1_int-v1_ca_bc.der'), SEC_ERROR_BAD_DER);
 
   // Using a v1 intermediate with v3 extenstions (invalid).
   ca_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
@@ -177,7 +177,7 @@ function run_test() {
   check_cert_err(cert_from_file('v2_bc_ee-v1_int_bc-v1_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_missing_bc_ee-v1_int_bc-v1_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_bc_ee-v1_int_bc-v1_ca_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v1_int_bc-v1_ca_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v1_int_bc-v1_ca_bc.der'), SEC_ERROR_BAD_DER);
 
   // Using v2 intermediate
   ca_error = SEC_ERROR_CA_CERT_INVALID;
@@ -190,7 +190,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v2_int-v1_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v2_int-v1_ca_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v2_int-v1_ca_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v2_int-v1_ca_bc.der'), SEC_ERROR_BAD_DER);
 
   // Using a v2 intermediate with basic constraints (invalid)
   ca_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
@@ -202,7 +202,7 @@ function run_test() {
   check_cert_err(cert_from_file('v2_bc_ee-v2_int_bc-v1_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_missing_bc_ee-v2_int_bc-v1_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_bc_ee-v2_int_bc-v1_ca_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v2_int_bc-v1_ca_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v2_int_bc-v1_ca_bc.der'), SEC_ERROR_BAD_DER);
 
   // Using a v3 intermediate that is missing basic constraints (invalid)
   ca_error = SEC_ERROR_CA_CERT_INVALID;
@@ -215,7 +215,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v3_int_missing_bc-v1_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v3_int_missing_bc-v1_ca_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v3_int_missing_bc-v1_ca_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v3_int_missing_bc-v1_ca_bc.der'), SEC_ERROR_BAD_DER);
 
   // these should pass assuming we are OK with v1 ca signing v3 intermediates
   ca_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
@@ -227,7 +227,7 @@ function run_test() {
   check_cert_err(cert_from_file('v2_bc_ee-v3_int-v1_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_missing_bc_ee-v3_int-v1_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_bc_ee-v3_int-v1_ca_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v3_int-v1_ca_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v3_int-v1_ca_bc.der'), SEC_ERROR_BAD_DER);
 
 
   //////////////
@@ -245,7 +245,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v1_int-v2_ca.der'), ee_error)
   check_cert_err(cert_from_file('v2_bc_ee-v1_int-v2_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v1_int-v2_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v1_int-v2_ca.der'), SEC_ERROR_BAD_DER);
 
   // v2 ca, v1 intermediate with basic constraints (invalid)
   ca_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
@@ -257,7 +257,7 @@ function run_test() {
   check_cert_err(cert_from_file('v2_bc_ee-v1_int_bc-v2_ca.der'), ee_error);
   check_cert_err(cert_from_file('v3_missing_bc_ee-v1_int_bc-v2_ca.der'), ee_error);
   check_cert_err(cert_from_file('v3_bc_ee-v1_int_bc-v2_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v1_int_bc-v2_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v1_int_bc-v2_ca.der'), SEC_ERROR_BAD_DER);
 
   // v2 ca, v2 intermediate
   ca_error = SEC_ERROR_CA_CERT_INVALID;
@@ -270,7 +270,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v2_int-v2_ca.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v2_int-v2_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v2_int-v2_ca.der'), ee_error)
+  check_cert_err(cert_from_file('v4_bc_ee-v2_int-v2_ca.der'), SEC_ERROR_BAD_DER);
 
   // v2 ca, v2 intermediate with basic constraints (invalid)
   ca_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
@@ -282,7 +282,7 @@ function run_test() {
   check_cert_err(cert_from_file('v2_bc_ee-v2_int_bc-v2_ca.der'), ee_error);
   check_cert_err(cert_from_file('v3_missing_bc_ee-v2_int_bc-v2_ca.der'), ee_error);
   check_cert_err(cert_from_file('v3_bc_ee-v2_int_bc-v2_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v2_int_bc-v2_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v2_int_bc-v2_ca.der'), SEC_ERROR_BAD_DER);
 
   // v2 ca, v3 intermediate missing basic constraints
   ca_error = SEC_ERROR_CA_CERT_INVALID;
@@ -295,7 +295,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v3_int_missing_bc-v2_ca.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v3_int_missing_bc-v2_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v3_int_missing_bc-v2_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v3_int_missing_bc-v2_ca.der'), SEC_ERROR_BAD_DER);
 
   // v2 ca, v3 intermediate
   ca_error = SEC_ERROR_CA_CERT_INVALID;
@@ -309,7 +309,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v3_int-v2_ca.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v3_int-v2_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v3_int-v2_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v3_int-v2_ca.der'), SEC_ERROR_BAD_DER);
 
   // v2 ca, v1 intermediate
   ca_error = SEC_ERROR_CA_CERT_INVALID;
@@ -322,7 +322,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v1_int-v2_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v1_int-v2_ca_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v1_int-v2_ca_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v1_int-v2_ca_bc.der'), SEC_ERROR_BAD_DER);
 
   // v2 ca, v1 intermediate with bc (invalid)
   ca_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
@@ -334,7 +334,7 @@ function run_test() {
   check_cert_err(cert_from_file('v2_bc_ee-v1_int_bc-v2_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_missing_bc_ee-v1_int_bc-v2_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_bc_ee-v1_int_bc-v2_ca_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v1_int_bc-v2_ca_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v1_int_bc-v2_ca_bc.der'), SEC_ERROR_BAD_DER);
 
   // v2 ca, v2 intermediate
   ca_error = SEC_ERROR_CA_CERT_INVALID;
@@ -347,7 +347,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v2_int-v2_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v2_int-v2_ca_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v2_int-v2_ca_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v2_int-v2_ca_bc.der'), SEC_ERROR_BAD_DER);
 
   // v2 ca, v2 intermediate with bc (invalid)
   ca_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
@@ -359,7 +359,7 @@ function run_test() {
   check_cert_err(cert_from_file('v2_bc_ee-v2_int_bc-v2_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_missing_bc_ee-v2_int_bc-v2_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_bc_ee-v2_int_bc-v2_ca_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v2_int_bc-v2_ca_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v2_int_bc-v2_ca_bc.der'), SEC_ERROR_BAD_DER);
 
   // v2 ca, invalid v3 intermediate
   ca_error = SEC_ERROR_CA_CERT_INVALID;
@@ -372,7 +372,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v3_int_missing_bc-v2_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v3_int_missing_bc-v2_ca_bc.der'), ee_error)
-  check_cert_err(cert_from_file('v4_bc_ee-v3_int_missing_bc-v2_ca_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v3_int_missing_bc-v2_ca_bc.der'), SEC_ERROR_BAD_DER);
 
   // v2 ca, valid v3 intermediate (is OK if we use 'classic' semantics)
   ca_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
@@ -384,7 +384,7 @@ function run_test() {
   check_cert_err(cert_from_file('v2_bc_ee-v3_int-v2_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_missing_bc_ee-v3_int-v2_ca_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_bc_ee-v3_int-v2_ca_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v3_int-v2_ca_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v3_int-v2_ca_bc.der'), SEC_ERROR_BAD_DER);
 
   //////////////
   // v3 CA supersection
@@ -401,7 +401,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v1_int-v3_ca.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v1_int-v3_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v1_int-v3_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v1_int-v3_ca.der'), SEC_ERROR_BAD_DER);
 
   // A v1 intermediate with v3 extensions
   ca_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
@@ -413,7 +413,7 @@ function run_test() {
   check_cert_err(cert_from_file('v2_bc_ee-v1_int_bc-v3_ca.der'), ee_error);
   check_cert_err(cert_from_file('v3_missing_bc_ee-v1_int_bc-v3_ca.der'), ee_error);
   check_cert_err(cert_from_file('v3_bc_ee-v1_int_bc-v3_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v1_int_bc-v3_ca.der'), ee_error)
+  check_cert_err(cert_from_file('v4_bc_ee-v1_int_bc-v3_ca.der'), SEC_ERROR_BAD_DER);
 
   // reject a v2 cert as intermediate
   ca_error = SEC_ERROR_CA_CERT_INVALID;
@@ -426,7 +426,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v2_int-v3_ca.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v2_int-v3_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v2_int-v3_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v2_int-v3_ca.der'), SEC_ERROR_BAD_DER);
 
   // v2 intermediate with bc (invalid)
   ca_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
@@ -438,7 +438,7 @@ function run_test() {
   check_cert_err(cert_from_file('v2_bc_ee-v2_int_bc-v3_ca.der'), ee_error);
   check_cert_err(cert_from_file('v3_missing_bc_ee-v2_int_bc-v3_ca.der'), ee_error);
   check_cert_err(cert_from_file('v3_bc_ee-v2_int_bc-v3_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v2_int_bc-v3_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v2_int_bc-v3_ca.der'), SEC_ERROR_BAD_DER);
 
   // invalid v3 intermediate
   ca_error = SEC_ERROR_CA_CERT_INVALID;
@@ -451,7 +451,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v3_int_missing_bc-v3_ca.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v3_int_missing_bc-v3_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v3_int_missing_bc-v3_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v3_int_missing_bc-v3_ca.der'), SEC_ERROR_BAD_DER);
 
   // I dont think that v3 intermediates should be allowed to sign v1 or v2
   // certs, but other thanthat this  is what we usually get in the wild.
@@ -464,7 +464,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v3_int-v3_ca.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v3_int-v3_ca.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v3_int-v3_ca.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v3_int-v3_ca.der'), SEC_ERROR_BAD_DER);
 
   // v3 CA, invalid v3 intermediate
   ca_error = SEC_ERROR_CA_CERT_INVALID;
@@ -477,7 +477,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v1_int-v3_ca_missing_bc.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v1_int-v3_ca_missing_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v1_int-v3_ca_missing_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v1_int-v3_ca_missing_bc.der'), SEC_ERROR_BAD_DER);
 
   // Int v1 with BC that is just invalid (classic fail insanity OK)
   ca_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
@@ -489,7 +489,7 @@ function run_test() {
   check_cert_err(cert_from_file('v2_bc_ee-v1_int_bc-v3_ca_missing_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_missing_bc_ee-v1_int_bc-v3_ca_missing_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_bc_ee-v1_int_bc-v3_ca_missing_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v1_int_bc-v3_ca_missing_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v1_int_bc-v3_ca_missing_bc.der'), SEC_ERROR_BAD_DER);
 
   // Good section (all fail)
   ca_error = SEC_ERROR_CA_CERT_INVALID;
@@ -502,7 +502,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v2_int-v3_ca_missing_bc.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v2_int-v3_ca_missing_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v2_int-v3_ca_missing_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v2_int-v3_ca_missing_bc.der'), SEC_ERROR_BAD_DER);
 
   // v2 intermediate (even with basic constraints) is invalid
   ca_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
@@ -514,7 +514,7 @@ function run_test() {
   check_cert_err(cert_from_file('v2_bc_ee-v2_int_bc-v3_ca_missing_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_missing_bc_ee-v2_int_bc-v3_ca_missing_bc.der'), ee_error);
   check_cert_err(cert_from_file('v3_bc_ee-v2_int_bc-v3_ca_missing_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v2_int_bc-v3_ca_missing_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v2_int_bc-v3_ca_missing_bc.der'), SEC_ERROR_BAD_DER);
 
   // v3 intermediate missing basic constraints is invalid
   ca_error = SEC_ERROR_CA_CERT_INVALID;
@@ -527,7 +527,7 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v3_int_missing_bc-v3_ca_missing_bc.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v3_int_missing_bc-v3_ca_missing_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v3_int_missing_bc-v3_ca_missing_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v3_int_missing_bc-v3_ca_missing_bc.der'), SEC_ERROR_BAD_DER);
 
   // With a v3 root missing bc and valid v3 intermediate
   ca_error = SEC_ERROR_CA_CERT_INVALID;
@@ -541,5 +541,5 @@ function run_test() {
   ee_error = SEC_ERROR_EXTENSION_VALUE_INVALID;
   check_cert_err(cert_from_file('v1_bc_ee-v3_int-v3_ca_missing_bc.der'), ee_error);
   check_cert_err(cert_from_file('v2_bc_ee-v3_int-v3_ca_missing_bc.der'), ee_error);
-  check_cert_err(cert_from_file('v4_bc_ee-v3_int-v3_ca_missing_bc.der'), ee_error);
+  check_cert_err(cert_from_file('v4_bc_ee-v3_int-v3_ca_missing_bc.der'), SEC_ERROR_BAD_DER);
 }

security/pkix/lib/pkixbuild.cpp

@@ -52,18 +52,31 @@ BackCert::Init(const SECItem& certDER)
     return MapSECStatus(SECFailure);
   }
 
+  if (nssCert->version.len == 1 &&
+      nssCert->version.data[0] == static_cast<uint8_t>(der::Version::v3)) {
+    version = der::Version::v3;
+  } else if (nssCert->version.len == 1 &&
+             nssCert->version.data[0] == static_cast<uint8_t>(der::Version::v2)) {
+    version = der::Version::v2;
+  } else if (nssCert->version.len == 0) {
+    version = der::Version::v1;
+  } else {
+    // Explicit encoding of v1 is not allowed. We do not support any other
+    // version except v3.
+    return Fail(RecoverableError, SEC_ERROR_BAD_DER);
+  }
+
   const CERTCertExtension* const* exts = nssCert->extensions;
   if (!exts) {
     return Success;
   }
-  // We only decode v3 extensions for v3 certificates for two reasons.
-  // 1. They make no sense in non-v3 certs
-  // 2. An invalid cert can embed a basic constraints extension and the
-  //    check basic constrains will asume that this is valid. Making it
-  //    posible to create chains with v1 and v2 intermediates with is
-  //    not desirable.
-  if (! (nssCert->version.len == 1 &&
-      nssCert->version.data[0] == mozilla::pkix::der::Version::v3)) {
+
+  // Extensions are only allowed in v3 certificates, not v1 or v2. Also, we
+  // use presence of the basic constraints extension with isCA==true to decide
+  // whether to treat a certificate as a CA certificate, and we don't want to
+  // allow v1 or v2 intermediate CA certificates; this check is part of that
+  // enforcement as well.
+  if (version < der::Version::v3) {
     return Fail(RecoverableError, SEC_ERROR_EXTENSION_VALUE_INVALID);
   }
 
@@ -128,7 +141,6 @@ BackCert::Init(const SECItem& certDER)
   return Success;
 }
 
-
 Result
 BackCert::VerifyOwnSignatureWithKey(TrustDomain& trustDomain,
                                     const SECItem& subjectPublicKeyInfo) const

security/pkix/lib/pkixcheck.cpp

@@ -312,7 +312,7 @@ DecodeBasicConstraints(der::Input& input, /*out*/ bool& isCA,
 Result
 CheckBasicConstraints(EndEntityOrCA endEntityOrCA,
                       const SECItem* encodedBasicConstraints,
-                      const der::Version version, TrustLevel trustLevel,
+                      der::Version version, TrustLevel trustLevel,
                       unsigned int subCACount)
 {
   bool isCA = false;
@@ -635,14 +635,6 @@ CheckIssuerIndependentProperties(TrustDomain& trustDomain,
     *trustLevelOut = trustLevel;
   }
 
-  // XXX: Good enough for now. There could be an illegal explicit version
-  // number or one we don't support, but we can safely treat those all as v3
-  // for now since processing of v3 certificates is strictly more strict than
-  // processing of v1 certificates.
-  der::Version version = (!cert.GetNSSCert()->version.data &&
-                          !cert.GetNSSCert()->version.len) ? der::Version::v1
-                                                           : der::Version::v3;
-
   // 4.2.1.1. Authority Key Identifier is ignored (see bug 965136).
 
   // 4.2.1.2. Subject Key Identifier is ignored (see bug 965136).
@@ -675,7 +667,7 @@ CheckIssuerIndependentProperties(TrustDomain& trustDomain,
 
   // 4.2.1.9. Basic Constraints.
   rv = CheckBasicConstraints(endEntityOrCA, cert.encodedBasicConstraints,
-                             version, trustLevel, subCACount);
+                             cert.version, trustLevel, subCACount);
   if (rv != Success) {
     return rv;
   }

security/pkix/lib/pkixder.h

@@ -622,30 +622,35 @@ CertificateSerialNumber(Input& input, /*out*/ SECItem& value)
 
 // x.509 and OCSP both use this same version numbering scheme, though OCSP
 // only supports v1.
-enum Version { v1 = 0, v2 = 1, v3 = 2 };
+MOZILLA_PKIX_ENUM_CLASS Version { v1 = 0, v2 = 1, v3 = 2 };
 
 // X.509 Certificate and OCSP ResponseData both use this
 // "[0] EXPLICIT Version DEFAULT <defaultVersion>" construct, but with
 // different default versions.
 inline Result
-OptionalVersion(Input& input, /*out*/ uint8_t& version)
+OptionalVersion(Input& input, /*out*/ Version& version)
 {
-  const uint8_t tag = CONTEXT_SPECIFIC | CONSTRUCTED | 0;
-  if (!input.Peek(tag)) {
-    version = v1;
+  static const uint8_t TAG = CONTEXT_SPECIFIC | CONSTRUCTED | 0;
+  if (!input.Peek(TAG)) {
+    version = Version::v1;
     return Success;
   }
-  if (ExpectTagAndLength(input, tag, 3) != Success) {
+  Input value;
+  if (ExpectTagAndGetValue(input, TAG, value) != Success) {
     return Failure;
   }
-  if (ExpectTagAndLength(input, INTEGER, 1) != Success) {
+  uint8_t integerValue;
+  if (Integer(value, integerValue) != Success) {
     return Failure;
   }
-  if (input.Read(version) != Success) {
+  if (End(value) != Success) {
     return Failure;
   }
-  if (version & 0x80) { // negative
-    return Fail(SEC_ERROR_BAD_DER);
+  switch (integerValue) {
+    case static_cast<uint8_t>(Version::v3): version = Version::v3; break;
+    case static_cast<uint8_t>(Version::v2): version = Version::v2; break;
+    default:
+      return Fail(SEC_ERROR_BAD_DER);
   }
   return Success;
 }

security/pkix/lib/pkixocsp.cpp

@@ -486,11 +486,11 @@ ResponseData(der::Input& input, Context& context,
              const CERTSignedData& signedResponseData,
              /*const*/ SECItem* certs, size_t numCerts)
 {
-  uint8_t version;
+  der::Version version;
   if (der::OptionalVersion(input, version) != der::Success) {
     return der::Failure;
   }
-  if (version != der::v1) {
+  if (version != der::Version::v1) {
     // TODO: more specific error code for bad version?
     return der::Fail(SEC_ERROR_BAD_DER);
   }

security/pkix/lib/pkixutil.h

@@ -27,6 +27,7 @@
 
 #include "pkix/enumclass.h"
 #include "pkix/pkixtypes.h"
+#include "pkixder.h"
 #include "prerror.h"
 #include "seccomon.h"
 #include "secerr.h"
@@ -123,6 +124,8 @@ public:
   Result VerifyOwnSignatureWithKey(TrustDomain& trustDomain,
                                    const SECItem& subjectPublicKeyInfo) const;
 
+  der::Version version;
+
   const SECItem* encodedAuthorityInfoAccess;
   const SECItem* encodedBasicConstraints;
   const SECItem* encodedCertificatePolicies;

security/pkix/test/gtest/pkixder_pki_types_tests.cpp

@@ -169,7 +169,7 @@ TEST_F(pkixder_pki_types_tests, CertificateSerialNumberZeroLength)
   ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
 }
 
-TEST_F(pkixder_pki_types_tests, OptionalVersionV1)
+TEST_F(pkixder_pki_types_tests, OptionalVersionV1ExplicitEncodingNotAllowed)
 {
   const uint8_t DER_OPTIONAL_VERSION_V1[] = {
     0xa0, 0x03,                   // context specific 0
@@ -180,11 +180,9 @@ TEST_F(pkixder_pki_types_tests, OptionalVersionV1)
   ASSERT_EQ(Success, input.Init(DER_OPTIONAL_VERSION_V1,
                                 sizeof DER_OPTIONAL_VERSION_V1));
 
-  uint8_t version = 99;
-  // TODO(bug 982783): An explicit value of 1 is not allowed, because it is not
-  // the shortest possible encoding!
-  ASSERT_EQ(Success, OptionalVersion(input, version));
-  ASSERT_EQ(v1, version);
+  Version version;
+  ASSERT_EQ(Failure, OptionalVersion(input, version));
+  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
 }
 
 TEST_F(pkixder_pki_types_tests, OptionalVersionV2)
@@ -198,9 +196,9 @@ TEST_F(pkixder_pki_types_tests, OptionalVersionV2)
   ASSERT_EQ(Success, input.Init(DER_OPTIONAL_VERSION_V2,
                                 sizeof DER_OPTIONAL_VERSION_V2));
 
-  uint8_t version = 99;
+  Version version = Version::v1;
   ASSERT_EQ(Success, OptionalVersion(input, version));
-  ASSERT_EQ(v2, version);
+  ASSERT_EQ(Version::v2, version);
 }
 
 TEST_F(pkixder_pki_types_tests, OptionalVersionV3)
@@ -214,9 +212,9 @@ TEST_F(pkixder_pki_types_tests, OptionalVersionV3)
   ASSERT_EQ(Success, input.Init(DER_OPTIONAL_VERSION_V3,
                                 sizeof DER_OPTIONAL_VERSION_V3));
 
-  uint8_t version = 99;
+  Version version = Version::v1;
   ASSERT_EQ(Success, OptionalVersion(input, version));
-  ASSERT_EQ(v3, version);
+  ASSERT_EQ(Version::v3, version);
 }
 
 TEST_F(pkixder_pki_types_tests, OptionalVersionUnknown)
@@ -230,9 +228,9 @@ TEST_F(pkixder_pki_types_tests, OptionalVersionUnknown)
   ASSERT_EQ(Success, input.Init(DER_OPTIONAL_VERSION_INVALID,
                                 sizeof DER_OPTIONAL_VERSION_INVALID));
 
-  uint8_t version = 99;
-  ASSERT_EQ(Success, OptionalVersion(input, version));
-  ASSERT_EQ(0x42, version);
+  Version version = Version::v1;
+  ASSERT_EQ(Failure, OptionalVersion(input, version));
+  ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
 }
 
 TEST_F(pkixder_pki_types_tests, OptionalVersionInvalidTooLong)
@@ -246,7 +244,7 @@ TEST_F(pkixder_pki_types_tests, OptionalVersionInvalidTooLong)
   ASSERT_EQ(Success, input.Init(DER_OPTIONAL_VERSION_INVALID_TOO_LONG,
                                 sizeof DER_OPTIONAL_VERSION_INVALID_TOO_LONG));
 
-  uint8_t version = 99;
+  Version version;
   ASSERT_EQ(Failure, OptionalVersion(input, version));
   ASSERT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
 }
@@ -261,8 +259,8 @@ TEST_F(pkixder_pki_types_tests, OptionalVersionMissing)
   ASSERT_EQ(Success, input.Init(DER_OPTIONAL_VERSION_MISSING,
                                 sizeof DER_OPTIONAL_VERSION_MISSING));
 
-  uint8_t version = 99;
+  Version version = Version::v3;
   ASSERT_EQ(Success, OptionalVersion(input, version));
-  ASSERT_EQ(v1, version);
+  ASSERT_EQ(Version::v1, version);
 }
 } // unnamed namespace

security/pkix/test/lib/pkixtestutil.cpp

@@ -722,7 +722,7 @@ TBSCertificate(PLArenaPool* arena, long versionValue,
 
   Output output;
 
-  if (versionValue != der::v1) {
+  if (versionValue != static_cast<long>(der::Version::v1)) {
     SECItem* versionInteger(Integer(arena, versionValue));
     if (!versionInteger) {
       return nullptr;