From b41acdada125a41c40e94177b8ebdc2bb7d130b6 Mon Sep 17 00:00:00 2001
From: Linus Nordberg <linus@nordberg.se>
Date: Fri, 27 Feb 2015 01:55:15 +0100
Subject: Fix a bug where verification of EC signatures made us crash.

Also, have valid_chain_p return boolean, add some debug logging and
detect invalid signature types instead of crashing.
---
 src/x509.erl | 73 +++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 40 insertions(+), 33 deletions(-)

(limited to 'src')

diff --git a/src/x509.erl b/src/x509.erl
index 9030e04..5a0e871 100644
--- a/src/x509.erl
+++ b/src/x509.erl
@@ -61,7 +61,7 @@ valid_chain_p(AcceptableRootCerts, [TopCert], MaxChainLength) ->
 valid_chain_p(AcceptableRootCerts, [BottomCert|Rest], MaxChainLength) ->
     case signed_by_p(BottomCert, hd(Rest)) of
         true -> valid_chain_p(AcceptableRootCerts, Rest, MaxChainLength - 1);
-        Err -> Err
+        false -> {false, signature_mismatch}
     end.
 
 %% @doc Return first cert in list signing Cert, or notfound. NOTE:
@@ -72,9 +72,14 @@ valid_chain_p(AcceptableRootCerts, [BottomCert|Rest], MaxChainLength) ->
 signer(_Cert, []) ->
     notfound;
 signer(Cert, [H|T]) ->
+    lager:debug("Is ~p signed by ~p?", [cert_string(Cert), cert_string(H)]),
     case signed_by_p(Cert, H) of
-        true -> H;
-        {false, _} -> signer(Cert, T)
+        true ->
+            lager:debug("~p is signed by ~p",
+                        [cert_string(Cert), cert_string(H)]),
+            H;
+        false ->
+            signer(Cert, T)
     end.
 
 %% Code from pubkey_cert:encoded_tbs_cert/1.
@@ -86,40 +91,55 @@ encoded_tbs_cert(DerCert) ->
     EncodedTBSCert.
 
 %% Code from pubkey_cert:extract_verify_data/2.
+-spec verifydata_from_cert(#'Certificate'{}, binary()) -> {ok, tuple()} | error.
 verifydata_from_cert(Cert, DerCert) ->
     PlainText = encoded_tbs_cert(DerCert),
     {_, Sig} = Cert#'Certificate'.signature,
     SigAlgRecord = Cert#'Certificate'.signatureAlgorithm,
     SigAlg = SigAlgRecord#'AlgorithmIdentifier'.algorithm,
-    {DigestType,_} = public_key:pkix_sign_types(SigAlg),
-    {PlainText, DigestType, Sig}.
+    lager:debug("SigAlg: ~p", [SigAlg]),
+    try
+        {DigestType, _} = public_key:pkix_sign_types(SigAlg),
+        {ok, {PlainText, DigestType, Sig}}
+    catch
+        error:function_clause ->
+            lager:debug("signature algorithm not supported: ~p", [SigAlg]),
+            error
+    end.
 
 %% @doc Verify that Cert/DerCert is signed by Issuer.
-verify(Cert, DerCert,                         % Certificate to verify.
-       #'Certificate'{                        % Issuer.
-          tbsCertificate = #'TBSCertificate'{
-                              subjectPublicKeyInfo = IssuerSPKI}}) ->
-
+-spec verify_sig(#'Certificate'{}, binary(), #'Certificate'{}) -> boolean().
+verify_sig(Cert, DerCert,                     % Certificate to verify.
+           #'Certificate'{                    % Issuer.
+              tbsCertificate = #'TBSCertificate'{
+                                  subjectPublicKeyInfo = IssuerSPKI}}) ->
     %% Dig out digest, digest type and signature from Cert/DerCert.
-    {DigestOrPlainText, DigestType, Signature} = verifydata_from_cert(Cert,
-                                                                      DerCert),
+    case verifydata_from_cert(Cert, DerCert) of
+        error -> false;
+        {ok, Tuple} -> verify_sig2(IssuerSPKI, Tuple)
+    end.
+
+verify_sig2(IssuerSPKI, {DigestOrPlainText, DigestType, Signature}) ->
     %% Dig out issuer key from issuer cert.
     #'SubjectPublicKeyInfo'{
        algorithm = #'AlgorithmIdentifier'{algorithm = Alg, parameters = Params},
        subjectPublicKey = {0, Key0}} = IssuerSPKI,
     KeyType = pubkey_cert_records:supportedPublicKeyAlgorithms(Alg),
     lager:debug("Alg: ~p", [Alg]),
+    lager:debug("Params: ~p", [Params]),
     lager:debug("KeyType: ~p", [KeyType]),
     lager:debug("Key0: ~p", [Key0]),
     IssuerKey =
         case KeyType of
             'RSAPublicKey' ->
                 public_key:der_decode(KeyType, Key0);
-            'DSAPublicKey' ->
-                {params, DssParams} = public_key:der_decode('DSAParams', Params),
-                {public_key:der_decode(KeyType, Key0), DssParams};
             'ECPoint' ->
-                public_key:der_decode(KeyType, Key0)
+                Point = #'ECPoint'{point = Key0},
+                ECParams = public_key:der_decode('EcpkParameters', Params),
+                {Point, ECParams};
+            _ ->                              % FIXME: 'DSAPublicKey'
+                lager:error("NIY: Issuer key type ~p", [KeyType]),
+                false
         end,
 
     lager:debug("DigestOrPlainText: ~p", [DigestOrPlainText]),
@@ -132,25 +152,12 @@ verify(Cert, DerCert,                         % Certificate to verify.
 
 %% @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()}.
+-spec signed_by_p(binary(), binary()) -> boolean().
 signed_by_p(DerCert, IssuerDerCert) when is_binary(DerCert),
                                          is_binary(IssuerDerCert) ->
-    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().
-public_key(CertDer) when is_binary(CertDer) ->
-    public_key(public_key:pkix_decode_cert(CertDer, otp));
-public_key(#'OTPCertificate'{
-              tbsCertificate =
-                  #'OTPTBSCertificate'{subjectPublicKeyInfo =
-                                           #'OTPSubjectPublicKeyInfo'{
-                                              subjectPublicKey = Key}}}) ->
-    Key.
+    verify_sig(public_key:pkix_decode_cert(DerCert, plain),
+               DerCert,
+               public_key:pkix_decode_cert(IssuerDerCert, plain)).
 
 cert_string(Der) ->
     mochihex:to_hex(crypto:hash(sha, Der)).
-- 
cgit v1.1