Verify certificates by decoding them as 'plain' certs rather than 'otp.

OTP cert validation is too strict. Let's see if this is forgiving
enough for our needs.

Also, move all cert reading from disk to x509.erl.

2 files changed, 201 insertions, 67 deletions

diff --git a/src/catlfish.erl b/src/catlfish.erl
index 98ec4dd..83ca3db 100644
--- a/src/catlfish.erl
+++ b/src/catlfish.erl
@@ -177,66 +177,24 @@ known_roots() ->
         undefined -> []
     end.
 
--spec known_roots(file:filename(), use_cache|update_tab) -> list().
+-spec known_roots(file:filename(), use_cache|update_tab) -> [binary()].
 known_roots(Directory, CacheUsage) ->
     case CacheUsage of
         use_cache ->
             case ets:lookup(?CACHE_TABLE, ?ROOTS_CACHE_KEY) of
                 [] ->
-                    read_pemfiles_from_dir(Directory);
+                    read_files_and_udpate_table(Directory);
                 [{roots, DerList}] ->
                     DerList
             end;
         update_tab ->
-            read_pemfiles_from_dir(Directory)
+            read_files_and_udpate_table(Directory)
     end.
 
--spec read_pemfiles_from_dir(file:filename()) -> list().
-read_pemfiles_from_dir(Dir) ->
-    DerList =
-        case file:list_dir(Dir) of
-            {error, enoent} ->
-                [];                             % FIXME: log enoent
-            {error, _Reason} ->
-                [];                             % FIXME: log Reason
-            {ok, Filenames} ->
-                Files = lists:filter(
-                          fun(F) ->
-                                  string:equal(".pem", filename:extension(F))
-                          end,
-                          Filenames),
-                ders_from_pemfiles(Dir, Files)
-        end,
-    true = ets:insert(?CACHE_TABLE, {?ROOTS_CACHE_KEY, DerList}),
-    DerList.
-
-ders_from_pemfiles(Dir, Filenames) ->
-    L = [ders_from_pemfile(filename:join(Dir, X)) || X <- Filenames],
-    lists:flatten(L).
-
-ders_from_pemfile(Filename) ->
-    Pems = case (catch public_key:pem_decode(pems_from_file(Filename))) of
-               {'EXIT', Reason} ->
-                   lager:info("badly encoded cert in ~p: ~p", [Filename, Reason]),
-                   [];
-               P -> P
-           end,
-    [der_from_pem(X) || X <- Pems].
-
--include_lib("public_key/include/public_key.hrl").
-der_from_pem(Pem) ->
-    case Pem of
-        {_Type, Der, not_encrypted} ->
-            case x509:valid_cert_p(Der) of
-                true -> Der;
-                false -> []
-            end;
-        _ -> []
-    end.
-
-pems_from_file(Filename) ->
-    {ok, Pems} = file:read_file(Filename),
-    Pems.
+read_files_and_udpate_table(Directory) ->
+    L = x509:read_pemfiles_from_dir(Directory),
+    true = ets:insert(?CACHE_TABLE, {?ROOTS_CACHE_KEY, L}),
+    L.
 
 %%%%%%%%%%%%%%%%%%%%
 %% Testing internal functions.
diff --git a/src/x509.erl b/src/x509.erl
index 5a96a29..b0363cd 100644
--- a/src/x509.erl
+++ b/src/x509.erl
@@ -2,9 +2,10 @@
 %%% See LICENSE for licensing information.
 
 -module(x509).
--export([normalise_chain/2, cert_string/1, valid_cert_p/1]).
+-export([normalise_chain/2, cert_string/1, read_pemfiles_from_dir/1]).
 
 -include_lib("public_key/include/public_key.hrl").
+-include_lib("eunit/include/eunit.hrl").
 
 -type reason() :: {chain_too_long |
                    root_unknown |
@@ -68,14 +69,72 @@ signer(Cert, [H|T]) ->
         {false, _} -> signer(Cert, T)
     end.
 
+%% encoded_tbs_cert: verbatim from pubkey_cert.erl
+encoded_tbs_cert(Cert) ->
+    {ok, PKIXCert} =
+	'OTP-PUB-KEY':decode_TBSCert_exclusive(Cert),
+    {'Certificate',
+     {'Certificate_tbsCertificate', EncodedTBSCert}, _, _} = PKIXCert,
+    EncodedTBSCert.
+
+%% extract_verify_data: close to pubkey_cert:extract_verify_data/2
+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}.
+
+verify(Cert, DerCert,
+       #'Certificate'{
+          tbsCertificate = #'TBSCertificate'{
+                              subjectPublicKeyInfo = IssuerSPKI}}) ->
+    {DigestOrPlainText, DigestType, Signature} =
+        verifydata_from_cert(Cert, DerCert),
+    #'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' ->
+                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)
+        end,
+    public_key:verify(DigestOrPlainText, DigestType, Signature, IssuerKey).
+
 -spec signed_by_p(binary(), binary()) -> true | {false, reason()}.
-signed_by_p(Cert, IssuerCert) ->
+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",
-                       [mochihex:to_hex(crypto:hash(sha, Cert)), Reason]),
+                       [cert_string(Cert), Reason]),
             {false, encoding_invalid};
         true ->
             %% Cert.issuer does match IssuerCert.subject. Now verify
@@ -99,23 +158,18 @@ public_key(#'OTPCertificate'{
     Key.
 
 cert_string(Der) ->
-    lists:flatten([io_lib:format("~2.16.0B", [X]) ||
-		      X <- binary_to_list(crypto:hash(sha, Der))]).
-
-valid_cert_p(Der) ->
-    %% Use the customized ASN.1 specification "OTP-PKIX.asn1" since
-    %% that's what's required for public_key functions we're using
-    %% (pkix_verify, public_key:pkix_is_issuer).
-    case (catch public_key:pkix_decode_cert(Der, otp)) of
-        #'OTPCertificate'{} ->
+    mochihex:to_hex(crypto:hash(sha, Der)).
+
+parsable_cert_p(Der) ->
+    case (catch public_key:pkix_decode_cert(Der, plain)) of
+        #'Certificate'{} ->
             true;
         {'EXIT', Reason} ->
-            lager:info("invalid certificate: ~p: ~p",
-                       [mochihex:to_hex(crypto:hash(sha, Der)), Reason]),
+            lager:info("invalid certificate: ~p: ~p", [cert_string(Der), Reason]),
             false;
         Unknown ->
             lager:info("unknown error decoding cert: ~p: ~p",
-                       [mochihex:to_hex(crypto:hash(sha, Der)), Unknown]),
+                       [cert_string(Der), Unknown]),
             false
     end.
 
@@ -142,13 +196,118 @@ valid_cert_p(Der) ->
 detox_precert(CertChain) ->
     CertChain.                                  % NYI
 
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+-spec read_pemfiles_from_dir(file:filename()) -> [binary()].
+%% @doc Reading certificates from files. Flattening the result -- all
+%% certs in all files are returned in a single list.
+read_pemfiles_from_dir(Dir) ->
+    case file:list_dir(Dir) of
+        {error, enoent} ->
+            lager:error("directory does not exist: ~p", [Dir]),
+            [];
+        {error, Reason} ->
+            lager:error("unable to read directory ~p: ~p", [Dir, Reason]),
+            [];
+        {ok, Filenames} ->
+            Files = lists:filter(
+                      fun(F) ->
+                              string:equal(".pem", filename:extension(F))
+                      end,
+                      Filenames),
+            ders_from_pemfiles(Dir, Files)
+    end.
+
+ders_from_pemfiles(Dir, Filenames) ->
+    lists:flatten(
+      [ders_from_pemfile(filename:join(Dir, X)) || X <- Filenames]).
+
+ders_from_pemfile(Filename) ->
+    PemBins = pems_from_file(Filename),
+    Pems = case (catch public_key:pem_decode(PemBins)) of
+               {'EXIT', Reason} ->
+                   lager:info("~p: invalid PEM-encoding: ~p", [Filename, Reason]),
+                   [];
+               P -> P
+           end,
+    [der_from_pem(X) || X <- Pems].
+
+der_from_pem(Pem) ->
+    case Pem of
+        {_Type, Der, not_encrypted} ->
+            case parsable_cert_p(Der) of
+                true ->
+                    Der;
+                false ->
+                    dump_unparsable_cert(Der),
+                    []
+            end;
+        Fail ->
+            lager:info("ignoring PEM-encoded data: ~p~n", [Fail]),
+            []
+    end.
+
+-spec pems_from_file(file:filename()) -> binary().
+pems_from_file(Filename) ->
+    {ok, Pems} = file:read_file(Filename),
+    Pems.
+
+-spec dump_unparsable_cert(binary()) -> ok | {error, atom()} | not_logged.
+dump_unparsable_cert(CertDer) ->
+    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),
+                                             NowMegaSec * 1000 * 1000 + NowSec,
+                                             NowMicroSec])),
+            lager:debug("dumping cert to ~p~n", [Filename]),
+            file:write_file(Filename, CertDer);
+        _ ->
+            not_logged
+    end.
+
 %%%%%%%%%%%%%%%%%%%%
 %% Testing private functions.
--include_lib("eunit/include/eunit.hrl").
 -include("x509_test.hrl").
+sign_test_() ->
+    {setup,
+     fun() -> ok end,
+     fun(_) -> ok end,
+     fun(_) -> [?_assertMatch(true, signed_by_p(?C0, ?C1))] end}.
+
 valid_cert_test_() ->
-    C0 = ?C0,
-    C1 = ?C1,
+    {setup,
+     fun() -> {read_pemfiles_from_dir("../test/testdata/known_roots"),
+               read_certs("../test/testdata/chains")} end,
+     fun(_) -> ok end,
+     fun({KnownRoots, Chains}) ->
+             [
+              %% self-signed, not a valid OTPCertificate:
+              %% {error,{asn1,{invalid_choice_tag,{22,<<"US">>}}}}
+              %% 'OTP-PUB-KEY':Func('OTP-X520countryname', Value0)
+              %% FIXME: this doesn't make much sense -- is my environment borked?
+              ?_assertMatch({true, _},
+                            valid_chain_p(lists:nth(1, Chains),
+                                          lists:nth(1, Chains), 10)),
+              %% self-signed
+              ?_assertMatch({false, root_unknown},
+                            valid_chain_p(KnownRoots,
+                                          lists:nth(2, Chains), 10)),
+              %% leaf signed by known CA
+              ?_assertMatch({true, _},
+                            valid_chain_p(KnownRoots,
+                                          lists:nth(3, Chains), 10))
+              ] end}.
+
+chain_test_() ->
+    {setup,
+     fun() -> {?C0, ?C1} end,
+     fun(_) -> ok end,
+     fun({C0, C1}) -> chain_test(C0, C1) end}.
+
+chain_test(C0, C1) ->
     [
      %% Root not in chain but in trust store.
      ?_assertEqual({true, [C1]}, valid_chain_p([C1], [C0], 10)),
@@ -169,3 +328,20 @@ valid_cert_test_() ->
      %% Max chain length 0 is not OK.
      ?_assertMatch({false, chain_too_long}, valid_chain_p([C0], [C0], 0))
     ].
+
+%%-spec read_certs(file:filename()) -> [string:string()].
+-spec read_certs(file:filename()) -> [[binary()]].
+read_certs(Dir) ->
+    {ok, Fnames} = file:list_dir(Dir),
+    PemBins =
+        [Pems || {ok, Pems} <-
+                     [file:read_file(filename:join(Dir, F)) ||
+                         F <- lists:sort(
+                                lists:filter(
+                                  fun(FN) -> string:equal(
+                                               ".pem", filename:extension(FN))
+                                  end,
+                                  Fnames))]],
+    PemEntries = [public_key:pem_decode(P) || P <- PemBins],
+    lists:map(fun(L) -> [Der || {'Certificate', Der, not_encrypted} <- L] end,
+              PemEntries).