diff options
| -rw-r--r-- | NEWS.md | 8 | ||||
| -rw-r--r-- | ebin/catlfish.app | 2 | ||||
| -rw-r--r-- | reltool.config | 4 | ||||
| -rw-r--r-- | src/x509.erl | 101 | ||||
| -rwxr-xr-x | verifycert.erl | 4 |
5 files changed, 65 insertions, 54 deletions
@@ -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) -> |