From ec817aad427b01ae60ff2d25df34a7babf1865fa Mon Sep 17 00:00:00 2001
From: Linus Nordberg <linus@nordberg.se>
Date: Fri, 24 Oct 2014 11:40:41 +0200
Subject: 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.
---
 src/v1.erl   | 10 +++++-----
 src/x509.erl | 39 ++++++++++++++++++++++++++-------------
 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().
-- 
cgit v1.1