Catch badly ASN.1-encoded certificates.

Now not crashing badly encoded certs in the list of known roots, which
is good. They're simply ignored. Next step is to figure out if we
should accept some anomalies, due to reality.

2 files changed, 31 insertions, 18 deletions

diff --git a/src/v1.erl b/src/v1.erl
index 0c13cbc..086fa35 100644
--- a/src/v1.erl
+++ b/src/v1.erl
@@ -21,13 +21,13 @@ request(post, "ct/v1/add-chain", Input) ->
                         Roots = catlfish:known_roots(),
                         case x509:normalise_chain(Roots, [LeafCert|CertChain]) of
                             {ok, [Leaf | Chain]} ->
-				io:format("[info] adding ~p~n",
+                                io:format("[info] adding ~p~n",
                                           [x509:cert_string(LeafCert)]),
                                 success(catlfish:add_chain(Leaf, Chain));
-                            {Err, Msg} ->
-				io:format("[info] rejecting ~p: ~p~n",
-                                          [x509:cert_string(LeafCert), Err]),
-                                html("add-chain: ", [Msg, Err])
+                            {error, Reason} ->
+                                io:format("[info] rejecting ~p: ~p~n",
+                                          [x509:cert_string(LeafCert), Reason]),
+                                html("add-chain: invalid chain", Reason)
                         end;
                     Invalid ->
                         html("add-chain: chain is not a list: ", [Invalid])
diff --git a/src/x509.erl b/src/x509.erl
index 9b6b386..a784354 100644
--- a/src/x509.erl
+++ b/src/x509.erl
@@ -6,15 +6,20 @@
 
 -include_lib("public_key/include/public_key.hrl").
 
--type reason() :: {chain_too_long | root_unknown | chain_broken}.
+-type reason() :: {chain_too_long |
+                   root_unknown |
+                   chain_broken |
+                   signature_mismatch |
+                   encoding_invalid}.
 
 -define(MAX_CHAIN_LENGTH, 10).
 
--spec normalise_chain([binary()], [binary()]) -> [binary()].
+-spec normalise_chain([binary()], [binary()]) -> {ok, [binary()]} |
+                                                 {error, reason()}.
 normalise_chain(AcceptableRootCerts, CertChain) ->
     case valid_chain_p(AcceptableRootCerts, CertChain, ?MAX_CHAIN_LENGTH) of
         {false, Reason} ->
-            {Reason, "invalid chain"};
+            {error, Reason};
         {true, Root} ->
             [Leaf | Chain] = CertChain,
             {ok, [detox_precert(Leaf) | Chain] ++ Root}
@@ -49,29 +54,37 @@ valid_chain_p(AcceptableRootCerts, [TopCert], MaxChainLength) ->
     end;
 valid_chain_p(AcceptableRootCerts, [BottomCert|Rest], MaxChainLength) ->
     case signed_by_p(BottomCert, hd(Rest)) of
-        false -> {false, chain_broken};
-        true -> valid_chain_p(AcceptableRootCerts, Rest, MaxChainLength - 1)
+        true -> valid_chain_p(AcceptableRootCerts, Rest, MaxChainLength - 1);
+        Err -> Err
     end.
 
-%% @doc Return list with first
--spec signer(binary(), [binary()]) -> list().
+%% @doc Return first cert in list signing Cert, or notfound.
+-spec signer(binary(), [binary()]) -> notfound | binary().
 signer(_Cert, []) ->
     notfound;
 signer(Cert, [H|T]) ->
     case signed_by_p(Cert, H) of
         true -> H;
-        false -> signer(Cert, T)
+        {false, _} -> signer(Cert, T)
     end.
 
--spec signed_by_p(binary(), binary()) -> boolean().
+-spec signed_by_p(binary(), binary()) -> true | {false, reason()}.
 signed_by_p(Cert, IssuerCert) ->
     %% FIXME: Validate presence and contents (against constraints) of
     %% names (subject, subjectAltName, emailAddress) too?
-    case public_key:pkix_is_issuer(Cert, IssuerCert) of
-        true ->           % Cert.issuer does match IssuerCert.subject.
-            public_key:pkix_verify(Cert, public_key(IssuerCert));
+    case (catch public_key:pkix_is_issuer(Cert, IssuerCert)) of
+        {'EXIT', _Reason} ->
+            %% Invalid ASN.1.
+            {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
+            {false, chain_broken}
     end.
 
 -spec public_key(binary() | #'OTPCertificate'{}) -> public_key:public_key().