summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--NEWS.md8
-rw-r--r--ebin/catlfish.app2
-rw-r--r--reltool.config4
-rw-r--r--src/x509.erl101
-rwxr-xr-xverifycert.erl4
5 files changed, 65 insertions, 54 deletions
diff --git a/NEWS.md b/NEWS.md
index 28caec3..52d2017 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -9,6 +9,14 @@
never be merged. An effect of this is that the SCT cache is now
mandatory and can not be disabled.
+## Bug fixes
+
+- Don't store, and deliver in get-entries, root cert twice when
+ included in submission (closes CATLFISH-96).
+
+- Don't use nc(1) for talking to control port. It behaves incorrectly
+ under some circumstances.
+
# Changes in catlfish 0.10.0 - 2017-02-11
## Incompatible changes
diff --git a/ebin/catlfish.app b/ebin/catlfish.app
index a1cff39..b21b86d 100644
--- a/ebin/catlfish.app
+++ b/ebin/catlfish.app
@@ -5,7 +5,7 @@
{application, catlfish,
[{description, "catlfish -- Certificate Transparency Log Server"},
- {vsn, "0.10.0"},
+ {vsn, "1.0.1-alpha-dev"},
{modules, [catlfish, catlfish_app, catlfish_sup, catlfish_web, v1, x509]},
{applications, [kernel, stdlib, plop, lager, mochiweb, idna, asn1, crypto]},
{mod, {catlfish_app, []}}]}.
diff --git a/reltool.config b/reltool.config
index 59f257a..759d67b 100644
--- a/reltool.config
+++ b/reltool.config
@@ -2,8 +2,8 @@
{sys, [
{erts, [{mod_cond, derived}, {app_file, strip}]},
{app_file, strip},
- {rel, "catlfish", "0.10.0", [sasl, catlfish]},
- {rel, "merge", "0.10.1", [sasl, merge]},
+ {rel, "catlfish", "1.0.1-alpha-dev", [sasl, catlfish]},
+ {rel, "merge", "1.0.1-alpha-dev", [sasl, merge]},
{boot_rel, "catlfish"},
{profile, standalone},
{incl_sys_filters, ["^bin/", "^erts-.*/", "^lib"]},
diff --git a/src/x509.erl b/src/x509.erl
index c31b699..f44e36f 100644
--- a/src/x509.erl
+++ b/src/x509.erl
@@ -21,11 +21,16 @@
-spec normalise_chain([binary()], [binary()]) -> {ok, [binary()]} |
{error, reason()}.
normalise_chain(AcceptableRootCerts, CertChain) ->
- case normalise_chain(AcceptableRootCerts, CertChain, ?MAX_CHAIN_LENGTH) of
+ case verify_chain(AcceptableRootCerts, CertChain, ?MAX_CHAIN_LENGTH) of
{false, Reason} ->
{error, Reason};
{true, Root} ->
- {ok, CertChain ++ Root}
+ case lists:last(CertChain) of
+ Root ->
+ {ok, CertChain};
+ _ ->
+ {ok, CertChain ++ [Root]}
+ end
end.
-spec cert_string(binary()) -> string().
@@ -63,26 +68,24 @@ detox(LeafDer, ChainDer) ->
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%% Private functions.
--spec normalise_chain([binary()], [binary()], integer()) ->
- {false, reason()} | {true, [binary()]}.
+-spec verify_chain([binary()], [binary()], integer()) ->
+ {false, reason()} | {true, [binary()]}.
%% @doc Verify that the leaf cert or precert has a valid chain back to
%% an acceptable root cert. The order of certificates in the second
-%% argument is: leaf cert in head, chain in tail. Order of first
+%% argument is: leaf cert in head, chain in tail. Order of certs in first
%% argument is irrelevant.
%%
-%% Return {false, Reason} or {true, ListWithRoot}. Note that
-%% ListWithRoot allways contain exactly one element -- a CA cert from
-%% first argument (AcceptableRootCerts) signing the root of the
-%% chain. FIXME: Any point in returning this as a list?
-normalise_chain(_, _, MaxChainLength) when MaxChainLength =< 0 ->
+%% Return {false, Reason} or {true, Root} where Root is a CA cert from
+%% AcceptableRootCerts signing the root of the chain.
+verify_chain(_, _, MaxChainLength) when MaxChainLength =< 0 ->
%% Chain too long.
{false, chain_too_long};
-normalise_chain(AcceptableRootCerts, [TopCert], MaxChainLength) ->
+verify_chain(AcceptableRootCerts, [TopCert], MaxChainLength) ->
%% Check root of chain.
case lists:member(TopCert, AcceptableRootCerts) of
true ->
%% Top cert is part of chain.
- {true, [TopCert]};
+ {true, TopCert};
false when MaxChainLength =< 1 ->
%% Chain too long.
{false, chain_too_long};
@@ -90,12 +93,12 @@ normalise_chain(AcceptableRootCerts, [TopCert], MaxChainLength) ->
%% Top cert _might_ be signed by a cert in truststore.
case signer(TopCert, AcceptableRootCerts) of
notfound -> {false, root_unknown};
- Root -> {true, [Root]}
+ Root -> {true, Root}
end
end;
-normalise_chain(AcceptableRootCerts, [BottomCert|Rest], MaxChainLength) ->
- case signed_by_p(BottomCert, hd(Rest)) of
- true -> normalise_chain(AcceptableRootCerts, Rest, MaxChainLength - 1);
+verify_chain(AcceptableRootCerts, [Cert|Rest], MaxChainLength) ->
+ case signed_by_p(Cert, hd(Rest)) of
+ true -> verify_chain(AcceptableRootCerts, Rest, MaxChainLength - 1);
false -> {false, signature_mismatch}
end.
@@ -411,21 +414,21 @@ valid_cert_test_() ->
%% #'OTPCertificate'{}. The error is
%% {invalid_choice_tag,{22,<<"US">>}}}} in
%% 'OTP-PUB-KEY':Func('OTP-X520countryname', Value0).
- ?_assertMatch({true, _}, normalise_chain(nth(1, Chains),
- nth(1, Chains), 10)),
+ ?_assertMatch({true, _}, verify_chain(nth(1, Chains),
+ nth(1, Chains), 10)),
%% Self-signed so fail.
?_assertMatch({false, root_unknown},
- normalise_chain(KnownRoots,
- nth(2, Chains), 10)),
+ verify_chain(KnownRoots,
+ nth(2, Chains), 10)),
%% Leaf signed by known CA, pass.
- ?_assertMatch({true, _}, normalise_chain(KnownRoots,
- nth(3, Chains), 10)),
+ ?_assertMatch({true, _}, verify_chain(KnownRoots,
+ nth(3, Chains), 10)),
%% Proper 3-depth chain with root in KnownRoots, pass.
%% Bug CATLFISH-19 --> [info] rejecting "3ee62cb678014c14d22ebf96f44cc899adea72f1": chain_broken
%% leaf sha1: 3ee62cb678014c14d22ebf96f44cc899adea72f1
%% leaf Subject: C=KR, O=Government of Korea, OU=Group of Server, OU=\xEA\xB5\x90\xEC\x9C\xA1\xEA\xB3\xBC\xED\x95\x99\xEA\xB8\xB0\xEC\x88\xA0\xEB\xB6\x80, CN=www.berea.ac.kr, CN=haksa.bits.ac.kr
- ?_assertMatch({true, _}, normalise_chain(KnownRoots,
- nth(4, Chains), 3)),
+ ?_assertMatch({true, _}, verify_chain(KnownRoots,
+ nth(4, Chains), 3)),
%% Verify against self, pass.
%% Bug CATLFISH-??, can't handle issuer keytype ECPoint.
%% Issuer sha1: 6969562e4080f424a1e7199f14baf3ee58ab6abb
@@ -439,31 +442,31 @@ valid_cert_test_() ->
%% Supposedly problematic chains from Google Aviator, fatal.
%% 00459972: asn1: syntax error: sequence truncated
- ?_assertMatch({true, _}, normalise_chain(nth(7, Chains),
- nth(7, Chains), 10)),
+ ?_assertMatch({true, _}, verify_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)),
+ ?_assertMatch({true, _}, verify_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)),
+ ?_assertMatch({true, _}, verify_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)),
+ ?_assertMatch({true, _}, verify_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)),
+ ?_assertMatch({true, _}, verify_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)),
+ ?_assertMatch({true, _}, verify_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))
+ ?_assertMatch({true, _}, verify_chain(nth(13, Chains),
+ nth(13, Chains), 10))
] end}.
chain_test_() ->
@@ -475,21 +478,21 @@ chain_test_() ->
chain_test(C0, C1) ->
[
%% Root not in chain but in trust store.
- ?_assertEqual({true, [C1]}, normalise_chain([C1], [C0], 10)),
- ?_assertEqual({true, [C1]}, normalise_chain([C1], [C0], 2)),
- %% Chain too long.
- ?_assertMatch({false, chain_too_long}, normalise_chain([C1], [C0], 1)),
+ ?_assertEqual({ok, [C0, C1]}, normalise_chain([C1], [C0])),
+ %% Same, but chain too long.
+ ?_assertEqual({false, chain_too_long}, verify_chain([C1], [C0], 1)),
%% Root in chain and in trust store.
- ?_assertEqual({true, [C1]}, normalise_chain([C1], [C0, C1], 2)),
- %% Chain too long.
- ?_assertMatch({false, chain_too_long}, normalise_chain([C1], [C0, C1], 1)),
+ ?_assertEqual({ok, [C0, C1]}, normalise_chain([C1], [C0, C1])),
+ %% Same, but chain too long.
+ ?_assertEqual({false, chain_too_long}, verify_chain([C1], [C0, C1], 1)),
%% Root not in trust store.
- ?_assertMatch({false, root_unknown}, normalise_chain([], [C0, C1], 10)),
+ ?_assertEqual({error, root_unknown}, normalise_chain([], [C0, C1])),
%% Selfsigned. Actually OK.
- ?_assertMatch({true, [C0]}, normalise_chain([C0], [C0], 10)),
- ?_assertMatch({true, [C0]}, normalise_chain([C0], [C0], 1)),
+ ?_assertEqual({ok, [C0]}, normalise_chain([C0], [C0])),
+ %% Same, still OK.
+ ?_assertEqual({true, C0}, verify_chain([C0], [C0], 1)),
%% Max chain length 0 is not OK.
- ?_assertMatch({false, chain_too_long}, normalise_chain([C0], [C0], 0))
+ ?_assertEqual({false, chain_too_long}, verify_chain([C0], [C0], 0))
].
%%-spec read_certs(file:filename()) -> [string:string()].
diff --git a/verifycert.erl b/verifycert.erl
index c9ad8dd..876cf15 100755
--- a/verifycert.erl
+++ b/verifycert.erl
@@ -1,8 +1,8 @@
#!/usr/bin/env escript
%% -*- erlang -*-
--define(CATLFISH_VER, "catlfish-0.10.0").
--define(PLOP_VER, "plop-0.10.1").
+-define(CATLFISH_VER, "catlfish-1.0.1-alpha-dev").
+-define(PLOP_VER, "plop-1.0.1-alpha-dev").
-define(LAGER_VER, "lager-3.2.2").
write_reply(Bin) ->