From 4d2993890fed46c5611735e84d4f737e8c342718 Mon Sep 17 00:00:00 2001
From: Linus Nordberg <linus@nordberg.se>
Date: Fri, 20 Feb 2015 12:20:09 +0100
Subject: Stop validating that cert.issuer matches issuer.subject.

Even canoncalized versions of this data mismatch in otherwise proper
chains. Since we're not here to validate chains for any other reasons
than attribution and spam control, let's stop validate
cert.issuer==candidate.subject. We still verify the cryptographic
chain with signatures of tbsCertificates of course.

Resolves CATLFISH-19.
---
 src/x509.erl | 73 ++++++++++++++++++++++--------------------------------------
 1 file changed, 27 insertions(+), 46 deletions(-)

(limited to 'src')

diff --git a/src/x509.erl b/src/x509.erl
index 32ade83..c815ca4 100644
--- a/src/x509.erl
+++ b/src/x509.erl
@@ -9,7 +9,6 @@
 
 -type reason() :: {chain_too_long |
                    root_unknown |
-                   chain_broken |
                    signature_mismatch |
                    encoding_invalid}.
 
@@ -58,7 +57,10 @@ valid_chain_p(AcceptableRootCerts, [BottomCert|Rest], MaxChainLength) ->
         Err -> Err
     end.
 
-%% @doc Return first cert in list signing Cert, or notfound.
+%% @doc Return first cert in list signing Cert, or notfound. NOTE:
+%% This is potentially expensive. It'd be more efficient to search for
+%% Cert.issuer in a list of Issuer.subject's. If so, maybe make the
+%% matching somewhat fuzzy unless that too is expensive.
 -spec signer(binary(), [binary()]) -> notfound | binary().
 signer(_Cert, []) ->
     notfound;
@@ -68,15 +70,15 @@ signer(Cert, [H|T]) ->
         {false, _} -> signer(Cert, T)
     end.
 
-%% encoded_tbs_cert: verbatim from pubkey_cert.erl
-encoded_tbs_cert(Cert) ->
+%% Code from pubkey_cert:encoded_tbs_cert/1.
+encoded_tbs_cert(DerCert) ->
     {ok, PKIXCert} =
-	'OTP-PUB-KEY':decode_TBSCert_exclusive(Cert),
-    {'Certificate',
-     {'Certificate_tbsCertificate', EncodedTBSCert}, _, _} = PKIXCert,
+	'OTP-PUB-KEY':decode_TBSCert_exclusive(DerCert),
+    {'Certificate', {'Certificate_tbsCertificate', EncodedTBSCert}, _, _} =
+        PKIXCert,
     EncodedTBSCert.
 
-%% extract_verify_data: close to pubkey_cert:extract_verify_data/2
+%% Code from pubkey_cert:extract_verify_data/2.
 verifydata_from_cert(Cert, DerCert) ->
     PlainText = encoded_tbs_cert(DerCert),
     {_, Sig} = Cert#'Certificate'.signature,
@@ -85,18 +87,20 @@ verifydata_from_cert(Cert, DerCert) ->
     {DigestType,_} = public_key:pkix_sign_types(SigAlg),
     {PlainText, DigestType, Sig}.
 
-verify(Cert, DerCert,
-       #'Certificate'{
+%% @doc Verify that Cert/DerCert is signed by Issuer.
+verify(Cert, DerCert,                         % Certificate to verify.
+       #'Certificate'{                        % Issuer.
           tbsCertificate = #'TBSCertificate'{
                               subjectPublicKeyInfo = IssuerSPKI}}) ->
-    {DigestOrPlainText, DigestType, Signature} =
-        verifydata_from_cert(Cert, DerCert),
+
+    %% Dig out digest, digest type and signature from Cert/DerCert.
+    {DigestOrPlainText, DigestType, Signature} = verifydata_from_cert(Cert,
+                                                                      DerCert),
+    %% Dig out issuer key from issuer cert.
     #'SubjectPublicKeyInfo'{
        algorithm = #'AlgorithmIdentifier'{algorithm = Alg, parameters = Params},
        subjectPublicKey = {0, Key0}} = IssuerSPKI,
     KeyType = pubkey_cert_records:supportedPublicKeyAlgorithms(Alg),
-
-    %% public_key:pem_entry_decode()
     IssuerKey =
         case KeyType of
             'RSAPublicKey' ->
@@ -107,43 +111,20 @@ verify(Cert, DerCert,
             'ECPoint' ->
                 public_key:der_decode(KeyType, Key0)
         end,
+
+    %% Verify the signature.
     public_key:verify(DigestOrPlainText, DigestType, Signature, IssuerKey).
 
+%% @doc Is Cert signed by Issuer? Only verify that the signature
+%% matches and don't check things like Cert.issuer == Issuer.subject.
 -spec signed_by_p(binary(), binary()) -> true | {false, reason()}.
 signed_by_p(DerCert, IssuerDerCert) when is_binary(DerCert),
                                          is_binary(IssuerDerCert) ->
-    Cert = public_key:pkix_decode_cert(DerCert, plain),
-    TBSCert = Cert#'Certificate'.tbsCertificate,
-    IssuerCert = public_key:pkix_decode_cert(IssuerDerCert, plain),
-    IssuerTBSCert = IssuerCert#'Certificate'.tbsCertificate,
-    case pubkey_cert:is_issuer(TBSCert#'TBSCertificate'.issuer,
-                               IssuerTBSCert#'TBSCertificate'.subject) of
-        false ->
-            {false, chain_broken};
-        true ->                                 % Verify signature.
-            case verify(Cert, DerCert, IssuerCert) of
-                false -> {false, signature_mismatch};
-                true -> true
-            end
-    end;
-signed_by_p(#'OTPCertificate'{} = Cert,
-            #'OTPCertificate'{} = IssuerCert) ->
-    %% FIXME: Validate presence and contents (against constraints) of
-    %% names (subject, subjectAltName, emailAddress) too?
-    case (catch public_key:pkix_is_issuer(Cert, IssuerCert)) of
-        {'EXIT', Reason} ->
-            lager:info("invalid certificate: ~p: ~p",
-                       [cert_string(Cert), Reason]),
-            {false, encoding_invalid};
-        true ->
-            %% Cert.issuer does match IssuerCert.subject. Now verify
-            %% the signature.
-            case public_key:pkix_verify(Cert, public_key(IssuerCert)) of
-                true -> true;
-                false -> {false, signature_mismatch}
-            end;
-        false ->
-            {false, chain_broken}
+    case verify(public_key:pkix_decode_cert(DerCert, plain),
+                DerCert,
+                public_key:pkix_decode_cert(IssuerDerCert, plain)) of
+        false -> {false, signature_mismatch};
+        true -> true
     end.
 
 -spec public_key(binary() | #'OTPCertificate'{}) -> public_key:public_key().
-- 
cgit v1.1