Store rejected certificates.

Not storing the full chain, which would be even more useful.
No rate limiting, which would be good.

Also, reorganise some in x509.erl and add tests.

1 files changed, 94 insertions, 52 deletions

diff --git a/src/x509.erl b/src/x509.erl
index c3a829f..1b0db5e 100644
--- a/src/x509.erl
+++ b/src/x509.erl
@@ -127,37 +127,55 @@ encoded_tbs_cert(DerCert) ->
         PKIXCert,
     EncodedTBSCert.
 
--spec extract_verify_data(#'Certificate'{}, binary()) -> {ok, tuple()} | error.
-%% @doc Return DER encoded TBScertificate, digest type and signature.
-%% Code from pubkey_cert:extract_verify_data/2.
-extract_verify_data(Cert, DerCert) ->
-    PlainText = encoded_tbs_cert(DerCert),
-    {_, Sig} = Cert#'Certificate'.signature,
-    SigAlgRecord = Cert#'Certificate'.signatureAlgorithm,
-    SigAlg = SigAlgRecord#'AlgorithmIdentifier'.algorithm,
-    try
-        {DigestType, _} = public_key:pkix_sign_types(SigAlg),
-        {ok, {PlainText, DigestType, Sig}}
-    catch
-        error:function_clause ->
-            lager:debug("signature algorithm not supported: ~p", [SigAlg]),
+-spec decode_cert(binary()) -> #'Certificate'{} | error.
+decode_cert(Der) ->
+    case (catch public_key:pkix_decode_cert(Der, plain)) of
+        #'Certificate'{} = Cert ->
+            Cert;
+        {'EXIT', Reason} ->
+            lager:info("invalid certificate: ~p: ~p", [cert_string(Der), Reason]),
+            dump_unparsable_cert(Der),
+            error;
+        Unknown ->
+            lager:info("unknown error decoding cert: ~p: ~p",
+                       [cert_string(Der), Unknown]),
             error
     end.
 
-%% @doc Verify that Cert/DerCert is signed by Issuer.
--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.
-    case extract_verify_data(Cert, DerCert) of
-        error -> false;
-        {ok, Tuple} -> verify_sig2(IssuerSPKI, Tuple)
+parsable_cert_p(Der) ->
+    case decode_cert(Der) of
+        error ->
+            false;
+        _ ->
+            true
     end.
 
-verify_sig2(IssuerSPKI, {DigestOrPlainText, DigestType, Signature}) ->
-    %% Dig out issuer key from issuer cert.
+%% @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()) -> boolean().
+signed_by_p(SubjectDer, IssuerDer) ->
+    SubjectCert = decode_cert(SubjectDer),
+    IssuerCert = decode_cert(IssuerDer),
+
+    case {SubjectCert, IssuerCert} of
+        {#'Certificate'{},
+         #'Certificate'{tbsCertificate =
+                            #'TBSCertificate'{subjectPublicKeyInfo =
+                                                  IssuerSPKI}}} ->
+            %% Dig out digest, digest type and signature from subject cert and
+            %% verify signature.
+            case extract_verify_data(decode_cert(SubjectDer), SubjectDer) of
+                error ->
+                    false;
+                {ok, SubjectData} ->
+                    verify_sig(IssuerSPKI, SubjectData)
+            end;
+        _ ->
+            false
+    end.
+
+verify_sig(IssuerSPKI, {DigestOrPlainText, DigestType, Signature}) ->
+    %% Dig out alg, params and key from issuer.
     #'SubjectPublicKeyInfo'{
        algorithm = #'AlgorithmIdentifier'{algorithm = Alg, parameters = Params},
        subjectPublicKey = {0, Key0}} = IssuerSPKI,
@@ -170,33 +188,29 @@ verify_sig2(IssuerSPKI, {DigestOrPlainText, DigestType, Signature}) ->
                 Point = #'ECPoint'{point = Key0},
                 ECParams = public_key:der_decode('EcpkParameters', Params),
                 {Point, ECParams};
-            _ ->                              % FIXME: 'DSAPublicKey'
+            _ ->                               % FIXME: 'DSAPublicKey'
                 lager:error("NIY: Issuer key type ~p", [KeyType]),
                 false
         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()) -> boolean().
-signed_by_p(DerCert, IssuerDerCert) when is_binary(DerCert),
-                                         is_binary(IssuerDerCert) ->
-    verify_sig(public_key:pkix_decode_cert(DerCert, plain),
-               DerCert,
-               public_key:pkix_decode_cert(IssuerDerCert, plain)).
-
-parsable_cert_p(Der) ->
-    case (catch public_key:pkix_decode_cert(Der, plain)) of
-        #'Certificate'{} ->
-            true;
-        {'EXIT', Reason} ->
-            lager:info("invalid certificate: ~p: ~p", [cert_string(Der), Reason]),
-            false;
-        Unknown ->
-            lager:info("unknown error decoding cert: ~p: ~p",
-                       [cert_string(Der), Unknown]),
-            false
+-spec extract_verify_data(#'Certificate'{}, binary()) -> {ok, tuple()} | error.
+%% @doc Return DER encoded TBScertificate, digest type and signature.
+%% Code from pubkey_cert:extract_verify_data/2.
+extract_verify_data(Cert, DerCert) ->
+    PlainText = encoded_tbs_cert(DerCert),
+    {_, Sig} = Cert#'Certificate'.signature,
+    SigAlgRecord = Cert#'Certificate'.signatureAlgorithm,
+    SigAlg = SigAlgRecord#'AlgorithmIdentifier'.algorithm,
+    try
+        {DigestType, _} = public_key:pkix_sign_types(SigAlg),
+        {ok, {PlainText, DigestType, Sig}}
+    catch
+        error:function_clause ->
+            lager:debug("~p: signature algorithm not supported: ~p",
+                        [cert_string(DerCert), SigAlg]),
+            error
     end.
 
 %% Precerts according to RFC6962.
@@ -367,18 +381,18 @@ pems_from_file(Filename) ->
     Pems.
 
 -spec dump_unparsable_cert(binary()) -> ok | {error, atom()} | not_logged.
-dump_unparsable_cert(CertDer) ->
+dump_unparsable_cert(Der) ->
     case application:get_env(catlfish, rejected_certs_path) of
         {ok, Directory} ->
             {NowMegaSec, NowSec, NowMicroSec} = now(),
             Filename =
                 filename:join(Directory,
                               io_lib:format("~p:~p.~p",
-                                            [cert_string(CertDer),
+                                            [cert_string(Der),
                                              NowMegaSec * 1000 * 1000 + NowSec,
                                              NowMicroSec])),
-            lager:debug("dumping cert to ~p~n", [Filename]),
-            file:write_file(Filename, CertDer);
+            lager:info("dumping cert to ~p~n", [Filename]),
+            file:write_file(Filename, Der);
         _ ->
             not_logged
     end.
@@ -429,7 +443,35 @@ valid_cert_test_() ->
               %% Signature Algorithm: md2WithRSAEncryption
               %% CA cert with sha1 96974cd6b663a7184526b1d648ad815cf51e801a
               ?_assertMatch(false, signed_by_p(hd(nth(6, Chains)),
-                                               hd(nth(6, Chains))))
+                                               hd(nth(6, Chains)))),
+
+              %% Supposedly problematic chains from Google Aviator, fatal.
+              %% 00459972: asn1: syntax error: sequence truncated
+              ?_assertMatch({true, _}, normalise_chain(nth(7, Chains),
+                                                       nth(7, Chains), 10)),
+              %% 1402673: x509: RSA modulus is not a positive number
+              ?_assertMatch({true, _}, normalise_chain(nth(8, Chains),
+                                                       nth(8, Chains), 10)),
+              %% 1345105: asn1: syntax error: IA5String contains invalid character
+              ?_assertMatch({true, _}, normalise_chain(nth(9, Chains),
+                                                       nth(9, Chains), 10)),
+              %% 1557693: asn1: structure error: integer too large
+              ?_assertMatch({true, _}, normalise_chain(nth(10, Chains),
+                                                       nth(10, Chains), 10)),
+
+              %% Supposedly problematic chains from Google Aviator, non-fatal.
+              %% 16800: x509: negative serial number
+              %% a.pem
+              ?_assertMatch({true, _}, normalise_chain(nth(11, Chains),
+                                                       nth(11, Chains), 10)),
+              %% 22487: x509: unhandled critical extension ([2 5 29 32])
+              %% b.pem
+              ?_assertMatch({true, _}, normalise_chain(nth(12, Chains),
+                                                       nth(12, Chains), 10)),
+              %% 5198: x509: certificate contained IP address of length 8
+              %% c.pem
+              ?_assertMatch({true, _}, normalise_chain(nth(13, Chains),
+                                                       nth(13, Chains), 10))
               ] end}.
 
 chain_test_() ->