Repository: randombit/botan
Pull request #2359 (2020-05-25 11:32:53)

Check that a v1 certificate does not include the v2 identifier fields
As with other such conformance issues we allow the certificate to parse but signal the problem during verification.

Bug reported by Mario Korth of Ruhr-Universität Bochum


Patch:
diff --git a/doc/dev_ref/todo.rst b/doc/dev_ref/todo.rst
index 87445344e..891475caa 100644
--- a/doc/dev_ref/todo.rst
+++ b/doc/dev_ref/todo.rst
@@ -126,6 +126,7 @@ Cleanups
 
 * Split test_ffi.cpp into multiple files
 * Unicode path support on Windows (GH #1615)
+* The X.509 path validation tests have much duplicated logic
 
 Compat Headers
 ----------------
diff --git a/src/lib/x509/cert_status.cpp b/src/lib/x509/cert_status.cpp
index bd27a6d5b..eab196c26 100644
--- a/src/lib/x509/cert_status.cpp
+++ b/src/lib/x509/cert_status.cpp
@@ -91,7 +91,9 @@ const char* to_string(Certificate_Status_Code code)
       case Certificate_Status_Code::DUPLICATE_CERT_EXTENSION:
          return "Duplicate certificate extension encountered";
       case Certificate_Status_Code::EXT_IN_V1_V2_CERT:
-         return "Encountered extension in certificate with version < 3";
+         return "Encountered extension in certificate with version that does not allow it";
+      case Certificate_Status_Code::V2_IDENTIFIERS_IN_V1_CERT:
+         return "Encountered v2 identifiers in v1 certificate";
       case Certificate_Status_Code::OCSP_SIGNATURE_ERROR:
          return "OCSP signature error";
       case Certificate_Status_Code::OCSP_ISSUER_NOT_FOUND:
diff --git a/src/lib/x509/cert_status.h b/src/lib/x509/cert_status.h
index 33d6b6b65..2f869c1ec 100644
--- a/src/lib/x509/cert_status.h
+++ b/src/lib/x509/cert_status.h
@@ -14,7 +14,6 @@ namespace Botan {
 
 /**
 * Certificate validation status code
-* Warning: reflect any changes to this in botan_cert_status_code in ffi.h
 */
 enum class Certificate_Status_Code {
    OK = 0,
@@ -83,6 +82,7 @@ enum class Certificate_Status_Code {
    OCSP_RESPONSE_INVALID = 4504,
    EXT_IN_V1_V2_CERT = 4505,
    DUPLICATE_CERT_POLICY = 4506,
+   V2_IDENTIFIERS_IN_V1_CERT = 4507,
 
    // Hard failures
    CERT_IS_REVOKED = 5000,
diff --git a/src/lib/x509/x509path.cpp b/src/lib/x509/x509path.cpp
index a2cfbbb1c..b5cdc27c2 100644
--- a/src/lib/x509/x509path.cpp
+++ b/src/lib/x509/x509path.cpp
@@ -148,6 +148,16 @@ PKIX::check_chain(const std::vector<std::shared_ptr<const X509_Certificate>>& ce
          }
 
       // Check cert extensions
+
+      if(subject->x509_version() == 1)
+         {
+         if(subject->v2_issuer_key_id().empty() == false ||
+            subject->v2_subject_key_id().empty() == false)
+            {
+            status.insert(Certificate_Status_Code::V2_IDENTIFIERS_IN_V1_CERT);
+            }
+         }
+
       Extensions extensions = subject->v3_extensions();
       const auto& extensions_vec = extensions.extensions();
       if(subject->x509_version() < 3 && !extensions_vec.empty())


Test patch:
diff --git a/src/tests/data/x509/bsi/expected.txt b/src/tests/data/x509/bsi/expected.txt
index 6ae2df005..9099ab58a 100644
--- a/src/tests/data/x509/bsi/expected.txt
+++ b/src/tests/data/x509/bsi/expected.txt
@@ -33,8 +33,8 @@ cert_path_CRL_15$No CRL with matching distribution point for certificate
 cert_path_CRL_16$Certificate is revoked
 cert_path_crypt_01$Signature error
 cert_path_crypt_02$Signature error
-cert_path_ext_01$Encountered extension in certificate with version < 3
-cert_path_ext_02$Encountered extension in certificate with version < 3
+cert_path_ext_01$Encountered extension in certificate with version that does not allow it
+cert_path_ext_02$Encountered extension in certificate with version that does not allow it
 cert_path_ext_03$Verified
 cert_path_ext_04$Unknown critical extension encountered
 cert_path_ext_05$Duplicate certificate extension encountered
diff --git a/src/tests/data/x509/v2-in-v1/int.pem b/src/tests/data/x509/v2-in-v1/int.pem
new file mode 100644
index 000000000..0c8a2d4b5
--- /dev/null
+++ b/src/tests/data/x509/v2-in-v1/int.pem
@@ -0,0 +1,21 @@
+-----BEGIN CERTIFICATE-----
+MIIDazCCAlOgAwIBAgIBMjANBgkqhkiG9w0BAQsFADBEMQswCQYDVQQGEwJERTEM
+MAoGA1UECgwDUlVCMScwJQYDVQQLDB5UTFMtU2Nhbm5lciBDQ0EgUlNBIFJPT1Qt
+Q0EgdjMwHhcNMTkxMjEyMDAwMDAwWhcNMjAxMjE5MDAwMDAwWjBoMQswCQYDVQQG
+EwJERTEMMAoGA1UECgwDUlVCMUswSQYDVQQLDEJUTFMtU2Nhbm5lciBDQ0EgQ0Eg
+MSB2MyAoUk9PVHYzX0NBdjNfTEVBRl9SU0F2MV9VbmlxdWVJZGVudGlmaWVycykw
+ggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDSkJ3o2EAq9jlHVEXnll10
+BkmJI5EkmE6tGVLvngdL/1IUz+jAWr8Y2U4gDRVHHVNhlp9LXsSBjcJLF5+jkLDM
+mMQP8bH1YRgpQZ0s3QuRZiJ0cv/1DCgfbye+6Wl9m6Xfrv4n676gSY0ck5+/KoMB
+8jqJu1vPE/PF7PqRsEYbiDjIO4dPfUgg40X4/+n2lsg6it/pbllmC3JGRPifip0N
+OeKrYKEipO5Ov+f4d3oXQmiBj+WZMVJBpgHes7IcBwx6m9JorV98man/N/K/RINX
+lU4slPErtnA9uUstamoL5OFq/kf7+R0N2/OT1oYpPUO2cCI9Hpudy1vn8Jok0yar
+AgMBAAGjRDBCMAoGA1UdDgQDBAECMA8GA1UdIwEBAAQFMAOAAQEwDwYDVR0PAQH/
+BAUDAwcEADASBgNVHRMBAf8ECDAGAQH/AgEBMA0GCSqGSIb3DQEBCwUAA4IBAQAM
+nfOfhRgilWZvraBuj0GtmLW072Zy8snWwV2AUxLW8VoRMjvaNgCgxZJ8SNxFjZBB
+tpkDfPO0CsrgF81L9KfLBAie39YfG+nQuTR9supuycpkaDRaKY11QfAhGG0E84kE
+lTuG7izbY2ftmOXGUTopHyJxXmJaKbJl8AQBSj9pgR33OI/p5hSxv/ep4S3ZChpT
+2E7vVHGZtfTnjgz23L9CJprKT/88tZC/ZpD1H/PD7fzwhXbfBoBeelpZur/preV1
+V36XYp2e8G2EpY2X6RZOkQ0Anx4gDYkudObnzLmyoninC3XOOpSaBpC53doZWHd7
+DWBXvHbIePk4KfvkCVm5
+-----END CERTIFICATE-----
diff --git a/src/tests/data/x509/v2-in-v1/leaf.pem b/src/tests/data/x509/v2-in-v1/leaf.pem
new file mode 100644
index 000000000..0ee72a8d4
--- /dev/null
+++ b/src/tests/data/x509/v2-in-v1/leaf.pem
@@ -0,0 +1,23 @@
+-----BEGIN CERTIFICATE-----
+MIID0DCCArigAwIBAAIBMTANBgkqhkiG9w0BAQsFADBoMQswCQYDVQQGEwJERTEM
+MAoGA1UECgwDUlVCMUswSQYDVQQLDEJUTFMtU2Nhbm5lciBDQ0EgQ0EgMSB2MyAo
+Uk9PVHYzX0NBdjNfTEVBRl9SU0F2MV9VbmlxdWVJZGVudGlmaWVycykwHhcNMTkx
+MjEyMDAwMDAwWhcNMjAxMjE5MDAwMDAwWjBxMQswCQYDVQQGEwJERTEMMAoGA1UE
+CgwDUlVCMVQwUgYDVQQLDEtUTFMtU2Nhbm5lciBDQ0EgTGVhZiBDZXJ0aWZpY2F0
+ZSAoUk9PVHYzX0NBdjNfTEVBRl9SU0F2MV9VbmlxdWVJZGVudGlmaWVycykwggEi
+MA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDO5T3lUJsMCK/7d7Ae/pWScF+k
+gxSRjfdL23zWGHp/b7QiVnMh8uNp7d8CTAaaXDLtzzleIa13W44t3cJIWNaohNg8
+Oec540dFt8+jG0gMTi+MeF+gLonSB8ppN+Xh15nssVRutmA7cFtrAoTsSCG2HzJH
+ZhQIG2BVMR+z/CVtQ2tg7nGK9lUHw00L3Gu4wVTktssH25h5HMjksZgUhWX2RCuo
+JyGec/eT+tRwG3wqWfjuFyh1SLJoQ2lvF/GThz3PvLSTkhd6i5DvO+Je8b6c0T0C
+dWnvGM4JZS4ZIDMUyCjMdlz6hL5ERXrF5Ji/83hnUKWkVtupZDejmzk+qlF9AgMB
+AAGBPQDerb7v3q2+796tvu/erb7v3q2+796tvu/erb7v3q2+796tvu/erb7v3q2+
+796tvu/erb7v3q2+796tvu+CPQDK/rq+yv66vsr+ur7K/rq+yv66vsr+ur7K/rq+
+yv66vsr+ur7K/rq+yv66vsr+ur7K/rq+yv66vsr+ur4wDQYJKoZIhvcNAQELBQAD
+ggEBAKCUm2sD3b+q3bfKiFwlcIYzaXilkGHGht0cwHcuQOJqzWq6hPV0vzF7/X21
+jPOVMKaxKWFnGCvFtQOGKMzc4iB88rHoprnRGFvqluCGGyo7FUNclJEXQobwpJyQ
+nNtOe2/p+DD+2VgaFYlRNYBuNFhSTJC1zyl3smzNBo+hImvcJvtGdCHyOmObSnGP
+GAj3ykd+aSxAXJthGhhZ8drxnw7XhSmN37HmPV0Y9WRcVZgcc6P40MJCLVVvaRdv
+TIKzUMXWCnt7V6dfaVHks1SW5rw9oU8YWxka0px+aTXPB9+hqTLmvxqxmEqY2F10
+GSGIdI6A1jXk4a1Ug4OWO+F4B0o=
+-----END CERTIFICATE-----
diff --git a/src/tests/data/x509/v2-in-v1/root.pem b/src/tests/data/x509/v2-in-v1/root.pem
new file mode 100644
index 000000000..5b1a9986c
--- /dev/null
+++ b/src/tests/data/x509/v2-in-v1/root.pem
@@ -0,0 +1,20 @@
+-----BEGIN CERTIFICATE-----
+MIIDRzCCAi+gAwIBAgIBATANBgkqhkiG9w0BAQsFADBEMQswCQYDVQQGEwJERTEM
+MAoGA1UECgwDUlVCMScwJQYDVQQLDB5UTFMtU2Nhbm5lciBDQ0EgUlNBIFJPT1Qt
+Q0EgdjMwHhcNMTkxMjEyMDAwMDAwWhcNMjAxMjE5MDAwMDAwWjBEMQswCQYDVQQG
+EwJERTEMMAoGA1UECgwDUlVCMScwJQYDVQQLDB5UTFMtU2Nhbm5lciBDQ0EgUlNB
+IFJPT1QtQ0EgdjMwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDmWaMk
+zTTP966yakxiXR8MvePmYgJyz/dS1m2pZxbkgBQJy7VFE7qwJSF5EkVHnAAFohK9
+B7XtSuMRRXTrRwuBHfQrBun8pm9+4bdA62YrbG1+5+uryO3EdavhIJElvlZAI3UA
+yE+s+Vi7AG9V3oJFSR6NO3yItKkagmiX0yetXmc3efJUY7DOCnhYge9dUU12ab5Z
+wytbzTJmMOx0inlche8wm2DNXA33vDTlLoBEryqcg6JS6GFkdG7LC+LhSJOP/vbK
+P06qx93zDbjqU8eNWJABME9Vjv+MW6JRtDR35yjOyWOn4E5Sba1BiJ5wTRx0pFVw
+4mXgsKTHwHHNTasRAgMBAAGjRDBCMAoGA1UdDgQDBAEBMA8GA1UdIwEBAAQFMAOA
+AQEwDwYDVR0PAQH/BAUDAwcEADASBgNVHRMBAf8ECDAGAQH/AgEFMA0GCSqGSIb3
+DQEBCwUAA4IBAQAOF1UZQG2B0hrEahv9KlCx/MTuKBk0S/slyurj+a6KQ9TQ5ChZ
+RDD89Ypfta3tBKFWuIYvRXDCR00Cwe14hThj6eefjd2kpC/P0p8GXXT2+Q2MPEcR
+qYE+C6Obi19z7QlyviI0hx2oFAtVus6QATvavhr2j4SuhzBWhtCH3x/oFuebY7Me
+pp3XuUo57RWl947k7Up2rixrCx5fwmE5A4fbaalC4UjPG/tGUdrFoYpC1tD53q1k
+BEZRljRRRK4vu0P2ki3dI14zbwYFKc/TeZVs7vjiquXoTInTpdzJ4EIzfcCLrsIj
+aNxDdCLl1Erilh1LC1evZLt1+rs91INkEnMM
+-----END CERTIFICATE-----
diff --git a/src/tests/test_x509_path.cpp b/src/tests/test_x509_path.cpp
index 72e759385..a20f417dd 100644
--- a/src/tests/test_x509_path.cpp
+++ b/src/tests/test_x509_path.cpp
@@ -541,6 +541,55 @@ std::vector<Test::Result> Validate_V1Cert_Test::run()
 
 BOTAN_REGISTER_TEST("x509_v1_ca", Validate_V1Cert_Test);
 
+class Validate_V2Uid_in_V1_Test final : public Test
+   {
+   public:
+      std::vector<Test::Result> run() override;
+   };
+
+std::vector<Test::Result> Validate_V2Uid_in_V1_Test::run()
+   {
+   if(Botan::has_filesystem_impl() == false)
+      {
+      return {Test::Result::Note("Path validation",
+                                 "Skipping due to missing filesystem access")};
+      }
+
+   std::vector<Test::Result> results;
+
+   const std::string root_crt = Test::data_file("/x509/v2-in-v1/root.pem");
+   const std::string int_crt  = Test::data_file("/x509/v2-in-v1/int.pem");
+   const std::string ee_crt   = Test::data_file("/x509/v2-in-v1/leaf.pem");
+
+   auto validation_time =
+      Botan::calendar_point(2020, 1, 1, 1, 0, 0).to_std_timepoint();
+
+   Botan::X509_Certificate root(root_crt);
+   Botan::X509_Certificate intermediate(int_crt);
+   Botan::X509_Certificate ee_cert(ee_crt);
+
+   Botan::Certificate_Store_In_Memory trusted;
+   trusted.add_certificate(root);
+
+   std::vector<Botan::X509_Certificate> chain = { ee_cert, intermediate };
+
+   Botan::Path_Validation_Restrictions restrictions;
+   Botan::Path_Validation_Result validation_result =
+      Botan::x509_path_validate(chain, restrictions, trusted, "",
+                                Botan::Usage_Type::UNSPECIFIED, validation_time);
+
+   Test::Result result("Verifying v1 certificate using v2 uid fields");
+   result.test_eq("Path validation failed",
+                  validation_result.successful_validation(), false);
+   result.test_eq("Path validation result",
+                  validation_result.result_string(),
+                  "Encountered v2 identifiers in v1 certificate");
+
+   return {result};
+   }
+
+BOTAN_REGISTER_TEST("x509_v2uid_in_v1", Validate_V2Uid_in_V1_Test);
+
 class BSI_Path_Validation_Tests final : public Test
 
    {