Return correct extra-data for precerts too (closes CATLFISH-56).

Verify precerts in make tests too.

4 files changed, 76 insertions, 59 deletions

diff --git a/Makefile b/Makefile
index 4762424..f7f27a9 100644
--- a/Makefile
+++ b/Makefile
@@ -90,6 +90,8 @@ tests-run:
 	@(cd $(INSTDIR) && python ../tools/merge.py --config ../test/catlfish-test.cfg --localconfig ../test/catlfish-test-local-merge.cfg) || (echo "Merge failed" ; false)
 	@diff -r -x nursery -x verifiedsize catlfish/tests/mergedb catlfish/tests/mergedb-secondary || (echo "Merge databases not matching" ; false)
 	@(cd $(INSTDIR) && python ../tools/check-sth.py --publickey=tests/keys/logkey.pem --cafile tests/httpsca/demoCA/cacert.pem https://localhost:8080/) || (echo "Check failed" ; false)
+	@(cd $(INSTDIR) && mkdir fetchcertstore)
+	@(cd $(INSTDIR) && python ../tools/fetchallcerts.py $(BASEURL) --store fetchcertstore --publickey=tests/keys/logkey.pem --cafile tests/httpsca/demoCA/cacert.pem) || (echo "Verification failed" ; false)
 
 tests-run2:
 	@(cd $(INSTDIR) ; python ../tools/verifysct.py --sct-file=submittedcerts --parallel 1 $(BASEURL) --publickey=tests/keys/logkey.pem --cafile tests/httpsca/demoCA/cacert.pem) || echo "Verification of SCT:s failed"
diff --git a/NEWS.md b/NEWS.md
index c985a2a..84a5a6c 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,5 +1,12 @@
 # Changes in version 0.8.0-dev
 
+## Incompatible changes
+
+- The file format for persistent storage of log entries have
+  changed. catlfish-0.8.0 is unable to read a database created by all
+  previous versions. Previous versions are unable to read a database
+  created by 0.8.0.
+
 ## Features
 
 - Library call for plop verification of entries added.
@@ -13,6 +20,10 @@
 - A bug with merging submitted root certs, i.e. lacking ExtraData has
   been fixed (closes CATLFISH-45).
 - Merge now fsyncs the logorder file (closes CATLFISH-46).
+- A chain returned from the log (get-entries) now always contains a
+  known root cert (closes CATLFISH-55).
+- "Extra data" for precerts returned from the log is now conformant
+  with RFC6962 (closes CATLFISH-56).
 
 ## Code cleanup
 
diff --git a/src/catlfish.erl b/src/catlfish.erl
index e48f788..d483249 100644
--- a/src/catlfish.erl
+++ b/src/catlfish.erl
@@ -124,20 +124,7 @@ add_to_db(Type, LeafCert, CertChain, EntryHash) ->
                              leaf_type = timestamped_entry,
                              entry = TSE}),
     MTLHash = ht:leaf_hash(MTLText),
-    ExtraData =
-        case Type of
-            normal -> CertChain;
-            precert -> [LeafCert | CertChain]
-        end,
-    LogEntry =
-        list_to_binary(
-          [encode_tls_vector(MTLText, 4),
-           encode_tls_vector(
-             encode_tls_vector(
-               list_to_binary(
-                 [encode_tls_vector(C, 3) || C <- ExtraData]),
-               3),
-             4)]),
+    LogEntry = pack_entry(Type, MTLText, LeafCert, CertChain),
     ok = plop:add(LogEntry, MTLHash, EntryHash),
     {TSE, MTLHash}.
 
@@ -157,7 +144,7 @@ add_chain(LeafCert, CertChain, Type) ->
                         exit({internalerror, "Rate limiting"})
                 end;
             {_Index, MTLHash, DBEntry} ->
-                {MTLText, _ExtraData} = unpack_entry(DBEntry),
+                {_Type, MTLText, _Cert, _Chain} = unpack_entry(DBEntry),
                 MTL = deserialise_mtl(MTLText),
                 MTLText = serialise(MTL),       % verify FIXME: remove
                 {MTL#mtl.entry, MTLHash}
@@ -236,8 +223,9 @@ entries(Start, End) ->
 entry_and_proof(Index, TreeSize) ->
     case plop:inclusion_and_entry(Index, TreeSize) of
         {ok, Entry, Path} ->
-            {MTL, ExtraData} = unpack_entry(Entry),
-            {[{leaf_input, base64:encode(MTL)},
+            {Type, MTLText, Cert, Chain} = unpack_entry(Entry),
+            ExtraData = extra_data(Type, Cert, Chain),
+            {[{leaf_input, base64:encode(MTLText)},
               {extra_data, base64:encode(ExtraData)},
               {audit_path, [base64:encode(X) || X <- Path]}]};
         {notfound, Msg} ->
@@ -253,29 +241,6 @@ init_cache_table() ->
     end,
     ets:new(?CACHE_TABLE, [set, public, named_table]).
 
-deserialise_extra_data(<<>>) ->
-    [];
-deserialise_extra_data(ExtraData) ->
-    {E, Rest} = decode_tls_vector(ExtraData, 3),
-    [E | deserialise_extra_data(Rest)].
-
-chain_from_mtl_extradata(MTL, ExtraData) ->
-    TimestampedEntry = MTL#mtl.entry,
-    Chain = deserialise_extra_data(ExtraData),
-    case TimestampedEntry#timestamped_entry.entry_type of
-        x509_entry ->
-            SignedEntry = TimestampedEntry#timestamped_entry.signed_entry,
-            [SignedEntry#signed_x509_entry.asn1_cert | Chain];
-        precert_entry ->
-            Chain
-    end.
-
-mtl_and_extra_from_entry(Entry) ->
-    {MTLText, ExtraDataPacked} = unpack_entry(Entry),
-    {ExtraData, <<>>} = decode_tls_vector(ExtraDataPacked, 3),
-    MTL = deserialise_mtl(MTLText),
-    {MTL, ExtraData}.
-
 verify_mtl(MTL, LeafCert, CertChain) ->
     Timestamp = MTL#mtl.entry#timestamped_entry.timestamp,
     EntryType = MTL#mtl.entry#timestamped_entry.entry_type,
@@ -293,15 +258,14 @@ verify_entry(Entry) ->
     RootCerts = known_roots(),
     verify_entry(Entry, RootCerts).
 
-verify_entry(Entry, RootCerts) ->
-    {MTL, ExtraData} = mtl_and_extra_from_entry(Entry),
-    Chain = chain_from_mtl_extradata(MTL, ExtraData),
-
-    case x509:normalise_chain(RootCerts, Chain) of
-        {ok, [LeafCert|CertChain]} ->
-            case verify_mtl(MTL, LeafCert, CertChain) of
+%% Used from plop.
+verify_entry(PackedEntry, RootCerts) ->
+    {_Type, MTLText, Cert, Chain} = unpack_entry(PackedEntry),
+    case x509:normalise_chain(RootCerts, [Cert | Chain]) of
+        {ok, [Cert | FullChain]} ->
+            case verify_mtl(deserialise_mtl(MTLText), Cert, FullChain) of
                 ok ->
-                    {ok, ht:leaf_hash(serialise(MTL))};
+                    {ok, ht:leaf_hash(MTLText)};
                 error ->
                     {error, "MTL verification failed"}
             end;
@@ -309,24 +273,63 @@ verify_entry(Entry, RootCerts) ->
             {error, Reason}
     end.
 
-entryhash_from_entry(Entry) ->
-    {MTL, ExtraData} = mtl_and_extra_from_entry(Entry),
-    Chain = chain_from_mtl_extradata(MTL, ExtraData),
-    crypto:hash(sha256, Chain).
+%% Used from plop.
+entryhash_from_entry(PackedEntry) ->
+    {_Type, _MTLText, Cert, Chain} = unpack_entry(PackedEntry),
+    crypto:hash(sha256, [Cert | Chain]).
 
 %% Private functions.
--spec unpack_entry(binary()) -> {binary(), binary()}.
+-spec pack_entry(normal|precert, binary(), binary(), [binary()]) -> binary().
+pack_entry(Type, MTLText, EndEntityCert, CertChain) ->
+    list_to_binary(
+      [tlv:encode(<<"MTL1">>, MTLText),
+       tlv:encode(case Type of
+                      normal -> <<"EEC1">>;
+                      precert -> <<"PRC1">>
+                  end, EndEntityCert),
+       tlv:encode(<<"CHN1">>,
+                  list_to_binary(
+                    [tlv:encode(<<"X509">>, E) || E <- CertChain]))]).
+
+-spec unpack_entry(binary()) -> {normal|precert, binary(), binary(), [binary()]}.
 unpack_entry(Entry) ->
-    {MTL, Rest} = decode_tls_vector(Entry, 4),
-    {ExtraData, <<>>} = decode_tls_vector(Rest, 4),
-    {MTL, ExtraData}.
+    {<<"MTL1">>, MTLText, Rest1} = tlv:decode(Entry),
+    {EECType, EndEntityCert, Rest2} = tlv:decode(Rest1),
+    Type = case EECType of
+               <<"EEC1">> ->
+                   normal;
+               <<"PRC1">> ->
+                   precert
+           end,
+    {<<"CHN1">>, PackedChain, _Rest3} = tlv:decode(Rest2), % Ignore rest.
+    Chain = unpack_certchain(PackedChain),
+    {Type, MTLText, EndEntityCert, Chain}.
+
+unpack_certchain(<<>>) ->
+    [];
+unpack_certchain(Data) ->
+    {<<"X509">>, Unpacked, Rest} = tlv:decode(Data),
+    [Unpacked | unpack_certchain(Rest)].
+
+extra_data(Type, Cert, Chain) ->
+    EncodedChain = encode_tls_vector(
+                     list_to_binary(
+                       [encode_tls_vector(C, 3) || C <- Chain]), 3),
+    case Type of
+        normal ->
+            EncodedChain;
+        precert ->
+            list_to_binary(
+              [encode_tls_vector(Cert, 3) | EncodedChain])
+    end.
 
 -spec x_entries([{non_neg_integer(), binary(), binary()}]) -> list().
 x_entries([]) ->
     [];
 x_entries([H|T]) ->
     {_Index, _Hash, Entry} = H,
-    {MTL, ExtraData} = unpack_entry(Entry),
+    {Type, MTL, Cert, Chain} = unpack_entry(Entry),
+    ExtraData = extra_data(Type, Cert, Chain),
     [{[{leaf_input, base64:encode(MTL)},
        {extra_data, base64:encode(ExtraData)}]} | x_entries(T)].
 
diff --git a/tools/mergetools.py b/tools/mergetools.py
index 9e84038..9f5feee 100644
--- a/tools/mergetools.py
+++ b/tools/mergetools.py
@@ -31,8 +31,9 @@ def unpack_entry(entry):
     pieces = []
     while len(entry):
         (length,) = struct.unpack(">I", entry[0:4])
-        data = entry[4:4+length]
-        entry = entry[4+length:]
+        type = entry[4:8]
+        data = entry[8:length]
+        entry = entry[length:]
         pieces.append(data)
     return pieces