From 999bffa05d2f99cb33e14f50ed17b2da331b887b Mon Sep 17 00:00:00 2001 From: Linus Nordberg Date: Tue, 24 Mar 2015 12:38:14 +0100 Subject: Clarify that 0.test.pem is not a valid #'OTPCertificate'{}. Also some cosmetic changes. --- src/x509.erl | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) (limited to 'src') diff --git a/src/x509.erl b/src/x509.erl index eae1468..c3a829f 100644 --- a/src/x509.erl +++ b/src/x509.erl @@ -400,36 +400,36 @@ valid_cert_test_() -> fun({KnownRoots, Chains}) -> [ %% Self-signed but verified against itself so pass. - %% Not a valid OTPCertificate: - %% {error,{asn1,{invalid_choice_tag,{22,<<"US">>}}}} - %% 'OTP-PUB-KEY':Func('OTP-X520countryname', Value0) - %% FIXME: This error doesn't make much sense -- is my - %% environment borked? - ?_assertMatch({true, _}, normalise_chain(lists:nth(1, Chains), - lists:nth(1, Chains), 10)), + %% Note that this certificate is rejected by the + %% stricter OTP-PKIX.asn1 specification generating + %% #'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)), %% Self-signed so fail. ?_assertMatch({false, root_unknown}, normalise_chain(KnownRoots, - lists:nth(2, Chains), 10)), + nth(2, Chains), 10)), %% Leaf signed by known CA, pass. ?_assertMatch({true, _}, normalise_chain(KnownRoots, - lists:nth(3, Chains), 10)), + 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, - lists:nth(4, Chains), 3)), + nth(4, Chains), 3)), %% Verify against self, pass. %% Bug CATLFISH-??, can't handle issuer keytype ECPoint. %% Issuer sha1: 6969562e4080f424a1e7199f14baf3ee58ab6abb - ?_assertMatch(true, signed_by_p(hd(lists:nth(5, Chains)), - hd(lists:nth(5, Chains)))), + ?_assertMatch(true, signed_by_p(hd(nth(5, Chains)), + hd(nth(5, Chains)))), %% Unsupported signature algorithm MD2-RSA, fail. %% Signature Algorithm: md2WithRSAEncryption %% CA cert with sha1 96974cd6b663a7184526b1d648ad815cf51e801a - ?_assertMatch(false, signed_by_p(hd(lists:nth(6, Chains)), - hd(lists:nth(6, Chains)))) + ?_assertMatch(false, signed_by_p(hd(nth(6, Chains)), + hd(nth(6, Chains)))) ] end}. chain_test_() -> -- cgit v1.1 From cbf3f563074e4f9a9024db0c0393aa4e6ba97087 Mon Sep 17 00:00:00 2001 From: Linus Nordberg Date: Wed, 25 Mar 2015 09:53:34 +0100 Subject: Store rejected certificates. Not storing the full chain, which would be even more useful. No rate limiting, which would be good. Also, reorganise some in x509.erl and add tests. --- src/x509.erl | 146 ++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 94 insertions(+), 52 deletions(-) (limited to 'src') diff --git a/src/x509.erl b/src/x509.erl index c3a829f..1b0db5e 100644 --- a/src/x509.erl +++ b/src/x509.erl @@ -127,37 +127,55 @@ encoded_tbs_cert(DerCert) -> PKIXCert, EncodedTBSCert. --spec extract_verify_data(#'Certificate'{}, binary()) -> {ok, tuple()} | error. -%% @doc Return DER encoded TBScertificate, digest type and signature. -%% Code from pubkey_cert:extract_verify_data/2. -extract_verify_data(Cert, DerCert) -> - PlainText = encoded_tbs_cert(DerCert), - {_, Sig} = Cert#'Certificate'.signature, - SigAlgRecord = Cert#'Certificate'.signatureAlgorithm, - SigAlg = SigAlgRecord#'AlgorithmIdentifier'.algorithm, - try - {DigestType, _} = public_key:pkix_sign_types(SigAlg), - {ok, {PlainText, DigestType, Sig}} - catch - error:function_clause -> - lager:debug("signature algorithm not supported: ~p", [SigAlg]), +-spec decode_cert(binary()) -> #'Certificate'{} | error. +decode_cert(Der) -> + case (catch public_key:pkix_decode_cert(Der, plain)) of + #'Certificate'{} = Cert -> + Cert; + {'EXIT', Reason} -> + lager:info("invalid certificate: ~p: ~p", [cert_string(Der), Reason]), + dump_unparsable_cert(Der), + error; + Unknown -> + lager:info("unknown error decoding cert: ~p: ~p", + [cert_string(Der), Unknown]), error end. -%% @doc Verify that Cert/DerCert is signed by Issuer. --spec verify_sig(#'Certificate'{}, binary(), #'Certificate'{}) -> boolean(). -verify_sig(Cert, DerCert, % Certificate to verify. - #'Certificate'{ % Issuer. - tbsCertificate = #'TBSCertificate'{ - subjectPublicKeyInfo = IssuerSPKI}}) -> - %% Dig out digest, digest type and signature from Cert/DerCert. - case extract_verify_data(Cert, DerCert) of - error -> false; - {ok, Tuple} -> verify_sig2(IssuerSPKI, Tuple) +parsable_cert_p(Der) -> + case decode_cert(Der) of + error -> + false; + _ -> + true end. -verify_sig2(IssuerSPKI, {DigestOrPlainText, DigestType, Signature}) -> - %% Dig out issuer key from issuer cert. +%% @doc Is Cert signed by Issuer? Only verify that the signature +%% matches and don't check things like Cert.issuer == Issuer.subject. +-spec signed_by_p(binary(), binary()) -> boolean(). +signed_by_p(SubjectDer, IssuerDer) -> + SubjectCert = decode_cert(SubjectDer), + IssuerCert = decode_cert(IssuerDer), + + case {SubjectCert, IssuerCert} of + {#'Certificate'{}, + #'Certificate'{tbsCertificate = + #'TBSCertificate'{subjectPublicKeyInfo = + IssuerSPKI}}} -> + %% Dig out digest, digest type and signature from subject cert and + %% verify signature. + case extract_verify_data(decode_cert(SubjectDer), SubjectDer) of + error -> + false; + {ok, SubjectData} -> + verify_sig(IssuerSPKI, SubjectData) + end; + _ -> + false + end. + +verify_sig(IssuerSPKI, {DigestOrPlainText, DigestType, Signature}) -> + %% Dig out alg, params and key from issuer. #'SubjectPublicKeyInfo'{ algorithm = #'AlgorithmIdentifier'{algorithm = Alg, parameters = Params}, subjectPublicKey = {0, Key0}} = IssuerSPKI, @@ -170,33 +188,29 @@ verify_sig2(IssuerSPKI, {DigestOrPlainText, DigestType, Signature}) -> Point = #'ECPoint'{point = Key0}, ECParams = public_key:der_decode('EcpkParameters', Params), {Point, ECParams}; - _ -> % FIXME: 'DSAPublicKey' + _ -> % FIXME: 'DSAPublicKey' lager:error("NIY: Issuer key type ~p", [KeyType]), false end, %% Verify the signature. public_key:verify(DigestOrPlainText, DigestType, Signature, IssuerKey). -%% @doc Is Cert signed by Issuer? Only verify that the signature -%% matches and don't check things like Cert.issuer == Issuer.subject. --spec signed_by_p(binary(), binary()) -> boolean(). -signed_by_p(DerCert, IssuerDerCert) when is_binary(DerCert), - is_binary(IssuerDerCert) -> - verify_sig(public_key:pkix_decode_cert(DerCert, plain), - DerCert, - public_key:pkix_decode_cert(IssuerDerCert, plain)). - -parsable_cert_p(Der) -> - case (catch public_key:pkix_decode_cert(Der, plain)) of - #'Certificate'{} -> - true; - {'EXIT', Reason} -> - lager:info("invalid certificate: ~p: ~p", [cert_string(Der), Reason]), - false; - Unknown -> - lager:info("unknown error decoding cert: ~p: ~p", - [cert_string(Der), Unknown]), - false +-spec extract_verify_data(#'Certificate'{}, binary()) -> {ok, tuple()} | error. +%% @doc Return DER encoded TBScertificate, digest type and signature. +%% Code from pubkey_cert:extract_verify_data/2. +extract_verify_data(Cert, DerCert) -> + PlainText = encoded_tbs_cert(DerCert), + {_, Sig} = Cert#'Certificate'.signature, + SigAlgRecord = Cert#'Certificate'.signatureAlgorithm, + SigAlg = SigAlgRecord#'AlgorithmIdentifier'.algorithm, + try + {DigestType, _} = public_key:pkix_sign_types(SigAlg), + {ok, {PlainText, DigestType, Sig}} + catch + error:function_clause -> + lager:debug("~p: signature algorithm not supported: ~p", + [cert_string(DerCert), SigAlg]), + error end. %% Precerts according to RFC6962. @@ -367,18 +381,18 @@ pems_from_file(Filename) -> Pems. -spec dump_unparsable_cert(binary()) -> ok | {error, atom()} | not_logged. -dump_unparsable_cert(CertDer) -> +dump_unparsable_cert(Der) -> 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), + [cert_string(Der), NowMegaSec * 1000 * 1000 + NowSec, NowMicroSec])), - lager:debug("dumping cert to ~p~n", [Filename]), - file:write_file(Filename, CertDer); + lager:info("dumping cert to ~p~n", [Filename]), + file:write_file(Filename, Der); _ -> not_logged end. @@ -429,7 +443,35 @@ valid_cert_test_() -> %% Signature Algorithm: md2WithRSAEncryption %% CA cert with sha1 96974cd6b663a7184526b1d648ad815cf51e801a ?_assertMatch(false, signed_by_p(hd(nth(6, Chains)), - hd(nth(6, Chains)))) + hd(nth(6, Chains)))), + + %% Supposedly problematic chains from Google Aviator, fatal. + %% 00459972: asn1: syntax error: sequence truncated + ?_assertMatch({true, _}, normalise_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)), + %% 1345105: asn1: syntax error: IA5String contains invalid character + ?_assertMatch({true, _}, normalise_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)), + + %% 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)), + %% 22487: x509: unhandled critical extension ([2 5 29 32]) + %% b.pem + ?_assertMatch({true, _}, normalise_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)) ] end}. chain_test_() -> -- cgit v1.1