diff options
| -rw-r--r-- | lib/public_key/doc/src/public_key.xml | 38 | ||||
| -rw-r--r-- | lib/public_key/src/public_key.erl | 118 | ||||
| -rw-r--r-- | lib/public_key/test/public_key_SUITE.erl | 39 | ||||
| -rw-r--r-- | lib/ssl/doc/src/ssl.xml | 81 | ||||
| -rw-r--r-- | lib/ssl/doc/src/ssl_crl_cache_api.xml | 15 | ||||
| -rw-r--r-- | lib/ssl/src/Makefile | 1 | ||||
| -rw-r--r-- | lib/ssl/src/ssl.app.src | 1 | ||||
| -rw-r--r-- | lib/ssl/src/ssl_crl_cache.erl | 5 | ||||
| -rw-r--r-- | lib/ssl/src/ssl_crl_cache_api.erl | 7 | ||||
| -rw-r--r-- | lib/ssl/src/ssl_crl_hash_dir.erl | 106 | ||||
| -rw-r--r-- | lib/ssl/src/ssl_handshake.erl | 31 | ||||
| -rw-r--r-- | lib/ssl/test/make_certs.erl | 15 | ||||
| -rw-r--r-- | lib/ssl/test/ssl_crl_SUITE.erl | 203 |
13 files changed, 613 insertions, 47 deletions
diff --git a/lib/public_key/doc/src/public_key.xml b/lib/public_key/doc/src/public_key.xml index 1aa601dc55..74bb06b9b2 100644 --- a/lib/public_key/doc/src/public_key.xml +++ b/lib/public_key/doc/src/public_key.xml @@ -701,6 +701,23 @@ fun(#'DistributionPoint'{}, #'CertificateList'{}, </func> <func> + <name>pkix_match_dist_point(CRL, DistPoint) -> boolean()</name> + <fsummary>Checks whether the given distribution point matches the + Issuing Distribution Point of the CRL.</fsummary> + + <type> + <v>CRL = der_encoded() | #'CertificateList'{} </v> + <v>DistPoint = #'DistributionPoint'{}</v> + </type> + <desc> + <p>Checks whether the given distribution point matches the + Issuing Distribution Point of the CRL, as described in RFC 5280. + If the CRL doesn't have an Issuing Distribution Point extension, + the distribution point always matches.</p> + </desc> + </func> + + <func> <name>pkix_sign(#'OTPTBSCertificate'{}, Key) -> der_encoded()</name> <fsummary>Signs certificate.</fsummary> <type> @@ -819,7 +836,26 @@ fun(#'DistributionPoint'{}, #'CertificateList'{}, <p>Verifies a digital signature.</p> </desc> </func> - + + <func> + <name>short_name_hash(Name) -> string()</name> + <type> + <v>Name = issuer_name()</v> + </type> + <desc> + <p>Generates a short hash of an issuer name. The hash is + returned as a string containing eight hexadecimal digits.</p> + + <p>The return value of this function is the same as the result + of the commands <c>openssl crl -hash</c> and + <c>openssl x509 -issuer_hash</c>, when passed the issuer name of + a CRL or a certificate, respectively. This hash is used by the + <c>c_rehash</c> tool to maintain a directory of symlinks to CRL + files, in order to facilitate looking up a CRL by its issuer + name.</p> + </desc> + </func> + </funcs> </erlref> diff --git a/lib/public_key/src/public_key.erl b/lib/public_key/src/public_key.erl index a5944bd604..d23abfe256 100644 --- a/lib/public_key/src/public_key.erl +++ b/lib/public_key/src/public_key.erl @@ -53,8 +53,10 @@ pkix_crls_validate/3, pkix_dist_point/1, pkix_dist_points/1, + pkix_match_dist_point/2, pkix_crl_verify/2, - pkix_crl_issuer/1 + pkix_crl_issuer/1, + short_name_hash/1 ]). -export_type([public_key/0, private_key/0, pem_entry/0, @@ -524,6 +526,38 @@ pkix_dist_points(OtpCert) -> [], Value). %%-------------------------------------------------------------------- +-spec pkix_match_dist_point(der_encoded() | #'CertificateList'{}, + #'DistributionPoint'{}) -> boolean(). +%% Description: Check whether the given distribution point matches +%% the "issuing distribution point" of the CRL. +%%-------------------------------------------------------------------- +pkix_match_dist_point(CRL, DistPoint) when is_binary(CRL) -> + pkix_match_dist_point(der_decode('CertificateList', CRL), DistPoint); +pkix_match_dist_point(#'CertificateList'{}, + #'DistributionPoint'{distributionPoint = asn1_NOVALUE}) -> + %% No distribution point name specified - that's considered a match. + true; +pkix_match_dist_point(#'CertificateList'{ + tbsCertList = + #'TBSCertList'{ + crlExtensions = Extensions}}, + #'DistributionPoint'{ + distributionPoint = {fullName, DPs}}) -> + case pubkey_cert:select_extension(?'id-ce-issuingDistributionPoint', Extensions) of + undefined -> + %% If the CRL doesn't have an IDP extension, it + %% automatically qualifies. + true; + #'Extension'{extnValue = IDPValue} -> + %% If the CRL does have an IDP extension, it must match + %% the given DistributionPoint to be considered a match. + IDPEncoded = der_decode('IssuingDistributionPoint', IDPValue), + #'IssuingDistributionPoint'{distributionPoint = {fullName, IDPs}} = + pubkey_cert_records:transform(IDPEncoded, decode), + pubkey_crl:match_one(IDPs, DPs) + end. + +%%-------------------------------------------------------------------- -spec pkix_sign(#'OTPTBSCertificate'{}, rsa_private_key() | dsa_private_key()) -> Der::binary(). %% @@ -785,6 +819,17 @@ oid2ssh_curvename(?'secp384r1') -> <<"nistp384">>; oid2ssh_curvename(?'secp521r1') -> <<"nistp521">>. %%-------------------------------------------------------------------- +-spec short_name_hash({rdnSequence, [#'AttributeTypeAndValue'{}]}) -> + string(). + +%% Description: Generates OpenSSL-style hash of a name. +%%-------------------------------------------------------------------- +short_name_hash({rdnSequence, _Attributes} = Name) -> + HashThis = encode_name_for_short_hash(Name), + <<HashValue:32/little, _/binary>> = crypto:hash(sha, HashThis), + string:to_lower(string:right(integer_to_list(HashValue, 16), 8, $0)). + +%%-------------------------------------------------------------------- %%% Internal functions %%-------------------------------------------------------------------- do_verify(DigestOrPlainText, DigestType, Signature, @@ -1047,3 +1092,74 @@ ec_key({PubKey, PrivateKey}, Params) -> parameters = Params, publicKey = PubKey}. +encode_name_for_short_hash({rdnSequence, Attributes0}) -> + Attributes = lists:map(fun normalise_attribute/1, Attributes0), + {Encoded, _} = 'OTP-PUB-KEY':'enc_RDNSequence'(Attributes, []), + Encoded. + +%% Normalise attribute for "short hash". If the attribute value +%% hasn't been decoded yet, decode it so we can normalise it. +normalise_attribute([#'AttributeTypeAndValue'{ + type = _Type, + value = Binary} = ATV]) when is_binary(Binary) -> + case pubkey_cert_records:transform(ATV, decode) of + #'AttributeTypeAndValue'{value = Binary} -> + %% Cannot decode attribute; return original. + [ATV]; + DecodedATV = #'AttributeTypeAndValue'{} -> + %% The new value will either be String or {Encoding,String}. + normalise_attribute([DecodedATV]) + end; +normalise_attribute([#'AttributeTypeAndValue'{ + type = _Type, + value = {Encoding, String}} = ATV]) + when + Encoding =:= utf8String; + Encoding =:= printableString; + Encoding =:= teletexString; + Encoding =:= ia5String -> + %% These string types all give us something that the unicode + %% module understands. + NewValue = normalise_attribute_value(String), + [ATV#'AttributeTypeAndValue'{value = NewValue}]; +normalise_attribute([#'AttributeTypeAndValue'{ + type = _Type, + value = String} = ATV]) when is_list(String) -> + %% A string returned by pubkey_cert_records:transform/2, for + %% certain attributes that commonly have incorrect value types. + NewValue = normalise_attribute_value(String), + [ATV#'AttributeTypeAndValue'{value = NewValue}]. + +normalise_attribute_value(String) -> + Converted = unicode:characters_to_binary(String), + NormalisedString = normalise_string(Converted), + %% We can't use the encoding function for the actual type of the + %% attribute, since some of them don't allow utf8Strings, which is + %% the required encoding when creating the hash. + {NewBinary, _} = 'OTP-PUB-KEY':'enc_X520CommonName'({utf8String, NormalisedString}, []), + NewBinary. + +normalise_string(String) -> + %% Normalise attribute values as required for "short hashes", as + %% implemented by OpenSSL. + + %% Remove ASCII whitespace from beginning and end. + TrimmedLeft = re:replace(String, "^[\s\f\n\r\t\v]+", "", [unicode, global]), + TrimmedRight = re:replace(TrimmedLeft, "[\s\f\n\r\t\v]+$", "", [unicode, global]), + %% Convert multiple whitespace characters to a single space. + Collapsed = re:replace(TrimmedRight, "[\s\f\n\r\t\v]+", "\s", [unicode, global]), + %% Convert ASCII characters to lowercase + Lower = ascii_to_lower(Collapsed), + %% And we're done! + Lower. + +ascii_to_lower(String) -> + %% Can't use string:to_lower/1, because that changes Latin-1 + %% characters as well. + << <<(if $A =< C, C =< $Z -> + C + ($a - $A); + true -> + C + end)>> + || + <<C>> <= iolist_to_binary(String) >>. diff --git a/lib/public_key/test/public_key_SUITE.erl b/lib/public_key/test/public_key_SUITE.erl index 71a77efa2e..2a84bb8121 100644 --- a/lib/public_key/test/public_key_SUITE.erl +++ b/lib/public_key/test/public_key_SUITE.erl @@ -44,7 +44,8 @@ all() -> encrypt_decrypt, {group, sign_verify}, pkix, pkix_countryname, pkix_emailaddress, pkix_path_validation, - pkix_iso_rsa_oid, pkix_iso_dsa_oid, pkix_crl, general_name]. + pkix_iso_rsa_oid, pkix_iso_dsa_oid, pkix_crl, general_name + short_cert_issuer_hash, short_crl_issuer_hash]. groups() -> [{pem_decode_encode, [], [dsa_pem, rsa_pem, ec_pem, encrypted_pem, @@ -817,6 +818,42 @@ general_name(Config) when is_list(Config) -> authorityCertSerialNumber = 1}). %%-------------------------------------------------------------------- +short_cert_issuer_hash() -> + [{doc, "Test OpenSSL-style hash for certificate issuer"}]. + +short_cert_issuer_hash(Config) when is_list(Config) -> + Datadir = ?config(data_dir, Config), + [{'Certificate', CertDER, _}] = + erl_make_certs:pem_to_der(filename:join(Datadir, "client_cert.pem")), + + %% This hash value was obtained by running: + %% openssl x509 -in client_cert.pem -issuer_hash -noout + CertIssuerHash = "d4c8d7e5", + + #'OTPCertificate'{tbsCertificate = #'OTPTBSCertificate'{issuer = Issuer}} = + public_key:pkix_decode_cert(CertDER, otp), + + CertIssuerHash = public_key:short_name_hash(Issuer). + +%%-------------------------------------------------------------------- +short_crl_issuer_hash() -> + [{doc, "Test OpenSSL-style hash for CRL issuer"}]. + +short_crl_issuer_hash(Config) when is_list(Config) -> + Datadir = ?config(data_dir, Config), + [{'CertificateList', CrlDER, _}] = + erl_make_certs:pem_to_der(filename:join(Datadir, "idp_crl.pem")), + + %% This hash value was obtained by running: + %% openssl crl -in idp_crl.pem -hash -noout + CrlIssuerHash = "d6134ed3", + + Issuer = public_key:pkix_crl_issuer(CrlDER), + + CrlIssuerHash = public_key:short_name_hash(Issuer). + + +%%-------------------------------------------------------------------- %% Internal functions ------------------------------------------------ %%-------------------------------------------------------------------- asn1_encode_decode({Asn1Type, Der, not_encrypted} = Entry) -> diff --git a/lib/ssl/doc/src/ssl.xml b/lib/ssl/doc/src/ssl.xml index bed82cdb91..4ea000802f 100644 --- a/lib/ssl/doc/src/ssl.xml +++ b/lib/ssl/doc/src/ssl.xml @@ -331,39 +331,88 @@ marker="public_key:public_key#pkix_path_validation-3">public_key:pkix_path_valid <tag><c>{crl_check, boolean() | peer | best_effort }</c></tag> <item> - Perform CRL (Certificate Revocation List) verification + <p>Perform CRL (Certificate Revocation List) verification <seealso marker="public_key:public_key#pkix_crls_validate-3"> (public_key:pkix_crls_validate/3)</seealso> on all the certificates during the path validation <seealso marker="public_key:public_key#pkix_path_validation-3">(public_key:pkix_path_validation/3) </seealso> - of the certificate chain. Defaults to false. + of the certificate chain. Defaults to <c>false</c>.</p> - <p><c>peer</c> - check is only performed on - the peer certificate.</p> + <taglist> + <tag><c>peer</c></tag> + <item>check is only performed on the peer certificate.</item> - <p><c>best_effort</c> - if certificate revocation status can not be determined - it will be accepted as valid.</p> + <tag><c>best_effort</c></tag> + <item>if certificate revocation status can not be determined + it will be accepted as valid.</item> + </taglist> <p>The CA certificates specified for the connection will be used to construct the certificate chain validating the CRLs.</p> - <p>The CRLs will be fetched from a local or external cache see + <p>The CRLs will be fetched from a local or external cache. See <seealso marker="ssl:ssl_crl_cache_api">ssl_crl_cache_api(3)</seealso>.</p> </item> <tag><c>{crl_cache, {Module :: atom(), {DbHandle :: internal | term(), Args :: list()}}}</c></tag> <item> - <p>Module defaults to ssl_crl_cache with <c> DbHandle </c> internal and an - empty argument list. The following arguments may be specified for the internal cache.</p> + <p>Specify how to perform lookup and caching of certificate revocation lists. + <c>Module</c> defaults to <seealso marker="ssl:ssl_crl_cache">ssl_crl_cache</seealso> + with <c> DbHandle </c> being <c>internal</c> and an + empty argument list.</p> + + <p>There are two implementations available:</p> + <taglist> - <tag><c>{http, timeout()}</c></tag> - <item><p> - Enables fetching of CRLs specified as http URIs in<seealso - marker="public_key:public_key_records"> X509 certificate extensions.</seealso> - Requires the OTP inets application.</p> - </item> - </taglist> + <tag><c>ssl_crl_cache</c></tag> + <item> + <p>This module maintains a cache of CRLs. CRLs can be + added to the cache using the function <seealso + marker="ssl:ssl_crl_cache#insert-1">ssl_crl_cache:insert/1</seealso>, + and optionally automatically fetched through HTTP if the + following argument is specified:</p> + + <taglist> + <tag><c>{http, timeout()}</c></tag> + <item><p> + Enables fetching of CRLs specified as http URIs in<seealso + marker="public_key:public_key_records">X509 certificate extensions</seealso>. + Requires the OTP inets application.</p> + </item> + </taglist> + </item> + + <tag><c>ssl_crl_hash_dir</c></tag> + <item> + <p>This module makes use of a directory where CRLs are + stored in files named by the hash of the issuer name.</p> + + <p>The file names consist of eight hexadecimal digits + followed by <c>.rN</c>, where <c>N</c> is an integer, + e.g. <c>1a2b3c4d.r0</c>. For the first version of the + CRL, <c>N</c> starts at zero, and for each new version, + <c>N</c> is incremented by one. The OpenSSL utility + <c>c_rehash</c> creates symlinks according to this + pattern.</p> + + <p>For a given hash value, this module finds all + consecutive <c>.r*</c> files starting from zero, and those + files taken together make up the revocation list. CRL + files whose <c>nextUpdate</c> fields are in the past, or + that are issued by a different CA that happens to have the + same name hash, are excluded.</p> + + <p>The following argument is required:</p> + + <taglist> + <tag><c>{dir, string()}</c></tag> + <item><p>Specifies the directory in which the CRLs can be found.</p></item> + </taglist> + + </item> + </taglist> + </item> <tag><c>{partial_chain, fun(Chain::[DerCert]) -> {trusted_ca, DerCert} | diff --git a/lib/ssl/doc/src/ssl_crl_cache_api.xml b/lib/ssl/doc/src/ssl_crl_cache_api.xml index 03ac010bfe..7440b6ef04 100644 --- a/lib/ssl/doc/src/ssl_crl_cache_api.xml +++ b/lib/ssl/doc/src/ssl_crl_cache_api.xml @@ -76,10 +76,13 @@ </func> <func> + <name>lookup(DistributionPoint, Issuer, DbHandle) -> not_available | CRLs </name> <name>lookup(DistributionPoint, DbHandle) -> not_available | CRLs </name> <fsummary> </fsummary> <type> <v> DistributionPoint = dist_point() </v> + <v> Issuer = <seealso + marker="public_key:public_key">public_key:issuer_name()</seealso> </v> <v> DbHandle = cache_ref() </v> <v> CRLs = [<seealso marker="public_key:public_key">public_key:der_encoded()</seealso>] </v> @@ -87,6 +90,18 @@ <desc> <p>Lookup the CRLs belonging to the distribution point <c> Distributionpoint</c>. This function may choose to only look in the cache or to follow distribution point links depending on how the cache is administrated. </p> + + <p>The <c>Issuer</c> argument contains the issuer name of the + certificate to be checked. Normally the returned CRL should + be issued by this issuer, except if the <c>cRLIssuer</c> field + of <c>DistributionPoint</c> has a value, in which case that + value should be used instead.</p> + + <p>In an earlier version of this API, the <c>lookup</c> + function received two arguments, omitting <c>Issuer</c>. For + compatibility, this is still supported: if there is no + <c>lookup/3</c> function in the callback module, + <c>lookup/2</c> is called instead.</p> </desc> </func> diff --git a/lib/ssl/src/Makefile b/lib/ssl/src/Makefile index 7a7a373487..b625db0656 100644 --- a/lib/ssl/src/Makefile +++ b/lib/ssl/src/Makefile @@ -70,6 +70,7 @@ MODULES= \ ssl_session_cache \ ssl_crl\ ssl_crl_cache \ + ssl_crl_hash_dir \ ssl_socket \ ssl_listen_tracker_sup \ tls_record \ diff --git a/lib/ssl/src/ssl.app.src b/lib/ssl/src/ssl.app.src index 937a3b1bd1..6467cedf9d 100644 --- a/lib/ssl/src/ssl.app.src +++ b/lib/ssl/src/ssl.app.src @@ -44,6 +44,7 @@ ssl_crl, ssl_crl_cache, ssl_crl_cache_api, + ssl_crl_hash_dir, %% App structure ssl_app, ssl_sup, diff --git a/lib/ssl/src/ssl_crl_cache.erl b/lib/ssl/src/ssl_crl_cache.erl index 60e7427737..647e0465fe 100644 --- a/lib/ssl/src/ssl_crl_cache.erl +++ b/lib/ssl/src/ssl_crl_cache.erl @@ -28,7 +28,7 @@ -behaviour(ssl_crl_cache_api). --export([lookup/2, select/2, fresh_crl/2]). +-export([lookup/3, select/2, fresh_crl/2]). -export([insert/1, insert/2, delete/1]). %%==================================================================== @@ -36,9 +36,10 @@ %%==================================================================== lookup(#'DistributionPoint'{distributionPoint = {fullName, Names}}, + _Issuer, CRLDbInfo) -> get_crls(Names, CRLDbInfo); -lookup(_,_) -> +lookup(_,_,_) -> not_available. select(Issuer, {{_Cache, Mapping},_}) -> diff --git a/lib/ssl/src/ssl_crl_cache_api.erl b/lib/ssl/src/ssl_crl_cache_api.erl index d7b31f280e..c3a57e2f49 100644 --- a/lib/ssl/src/ssl_crl_cache_api.erl +++ b/lib/ssl/src/ssl_crl_cache_api.erl @@ -24,8 +24,9 @@ -include_lib("public_key/include/public_key.hrl"). --type db_handle() :: term(). +-type db_handle() :: term(). +-type issuer_name() :: {rdnSequence, [#'AttributeTypeAndValue'{}]}. --callback lookup(#'DistributionPoint'{}, db_handle()) -> not_available | [public_key:der_encoded()]. --callback select(term(), db_handle()) -> [public_key:der_encoded()]. +-callback lookup(#'DistributionPoint'{}, issuer_name(), db_handle()) -> not_available | [public_key:der_encoded()]. +-callback select(issuer_name(), db_handle()) -> [public_key:der_encoded()]. -callback fresh_crl(#'DistributionPoint'{}, public_key:der_encoded()) -> public_key:der_encoded(). diff --git a/lib/ssl/src/ssl_crl_hash_dir.erl b/lib/ssl/src/ssl_crl_hash_dir.erl new file mode 100644 index 0000000000..bb62737232 --- /dev/null +++ b/lib/ssl/src/ssl_crl_hash_dir.erl @@ -0,0 +1,106 @@ +%% +%% %CopyrightBegin% +%% +%% Copyright Ericsson AB 2016-2016. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%% +%% %CopyrightEnd% + +-module(ssl_crl_hash_dir). + +-include_lib("public_key/include/public_key.hrl"). + +-behaviour(ssl_crl_cache_api). + +-export([lookup/3, select/2, fresh_crl/2]). + +lookup(#'DistributionPoint'{cRLIssuer = CRLIssuer} = DP, CertIssuer, CRLDbInfo) -> + Issuer = + case CRLIssuer of + asn1_NOVALUE -> + %% If the distribution point extension doesn't + %% indicate a CRL issuer, use the certificate issuer. + CertIssuer; + _ -> + CRLIssuer + end, + %% Find all CRLs for this issuer, and return those that match the + %% given distribution point. + AllCRLs = select(Issuer, CRLDbInfo), + lists:filter(fun(DER) -> + public_key:pkix_match_dist_point(DER, DP) + end, AllCRLs). + +fresh_crl(#'DistributionPoint'{}, CurrentCRL) -> + CurrentCRL. + +select(Issuer, {_DbHandle, [{dir, Dir}]}) -> + case find_crls(Issuer, Dir) of + [_|_] = DERs -> + DERs; + [] -> + %% That's okay, just report that we didn't find any CRL. + %% If the crl_check setting is best_effort, ssl_handshake + %% is happy with that, but if it's true, this is an error. + []; + {error, Error} -> + error_logger:error_report( + [{cannot_find_crl, Error}, + {dir, Dir}, + {module, ?MODULE}, + {line, ?LINE}]), + [] + end. + +find_crls(Issuer, Dir) -> + case filelib:is_dir(Dir) of + true -> + Hash = public_key:short_name_hash(Issuer), + find_crls(Issuer, Hash, Dir, 0, []); + false -> + {error, not_a_directory} + end. + +find_crls(Issuer, Hash, Dir, N, Acc) -> + Filename = filename:join(Dir, Hash ++ ".r" ++ integer_to_list(N)), + case file:read_file(Filename) of + {error, enoent} -> + Acc; + {ok, Bin} -> + try maybe_parse_pem(Bin) of + DER when is_binary(DER) -> + %% Found one file. Let's see if there are more. + find_crls(Issuer, Hash, Dir, N + 1, [DER] ++ Acc) + catch + error:Error -> + %% Something is wrong with the file. Report + %% it, and try the next one. + error_logger:error_report( + [{crl_parse_error, Error}, + {filename, Filename}, + {module, ?MODULE}, + {line, ?LINE}]), + find_crls(Issuer, Hash, Dir, N + 1, Acc) + end + end. + +maybe_parse_pem(<<"-----BEGIN", _/binary>> = PEM) -> + %% It's a PEM encoded file. Need to extract the DER + %% encoded data. + [{'CertificateList', DER, not_encrypted}] = public_key:pem_decode(PEM), + DER; +maybe_parse_pem(DER) when is_binary(DER) -> + %% Let's assume it's DER-encoded. + DER. + diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl index 0787e151c0..acc5ae6bd7 100644 --- a/lib/ssl/src/ssl_handshake.erl +++ b/lib/ssl/src/ssl_handshake.erl @@ -2128,13 +2128,14 @@ crl_check_same_issuer(OtpCert, _, Dps, Options) -> public_key:pkix_crls_validate(OtpCert, Dps, Options). dps_and_crls(OtpCert, Callback, CRLDbHandle, ext) -> - case public_key:pkix_dist_points(OtpCert) of - [] -> - no_dps; - DistPoints -> - distpoints_lookup(DistPoints, Callback, CRLDbHandle) - end; - + case public_key:pkix_dist_points(OtpCert) of + [] -> + no_dps; + DistPoints -> + Issuer = OtpCert#'OTPCertificate'.tbsCertificate#'OTPTBSCertificate'.issuer, + distpoints_lookup(DistPoints, Issuer, Callback, CRLDbHandle) + end; + dps_and_crls(OtpCert, Callback, CRLDbHandle, same_issuer) -> DP = #'DistributionPoint'{distributionPoint = {fullName, GenNames}} = public_key:pkix_dist_point(OtpCert), @@ -2145,12 +2146,20 @@ dps_and_crls(OtpCert, Callback, CRLDbHandle, same_issuer) -> end, GenNames), [{DP, {CRL, public_key:der_decode('CertificateList', CRL)}} || CRL <- CRLs]. -distpoints_lookup([], _, _) -> +distpoints_lookup([], _, _, _) -> []; -distpoints_lookup([DistPoint | Rest], Callback, CRLDbHandle) -> - case Callback:lookup(DistPoint, CRLDbHandle) of +distpoints_lookup([DistPoint | Rest], Issuer, Callback, CRLDbHandle) -> + Result = + try Callback:lookup(DistPoint, Issuer, CRLDbHandle) + catch + error:undef -> + %% The callback module still uses the 2-argument + %% version of the lookup function. + Callback:lookup(DistPoint, CRLDbHandle) + end, + case Result of not_available -> - distpoints_lookup(Rest, Callback, CRLDbHandle); + distpoints_lookup(Rest, Issuer, Callback, CRLDbHandle); CRLs -> [{DistPoint, {CRL, public_key:der_decode('CertificateList', CRL)}} || CRL <- CRLs] end. diff --git a/lib/ssl/test/make_certs.erl b/lib/ssl/test/make_certs.erl index 5eebf773a7..009bcd81ad 100644 --- a/lib/ssl/test/make_certs.erl +++ b/lib/ssl/test/make_certs.erl @@ -172,16 +172,29 @@ revoke(Root, CA, User, C) -> gencrl(Root, CA, C). gencrl(Root, CA, C) -> + %% By default, the CRL is valid for 24 hours from now. + gencrl(Root, CA, C, 24). + +gencrl(Root, CA, C, CrlHours) -> CACnfFile = filename:join([Root, CA, "ca.cnf"]), CACRLFile = filename:join([Root, CA, "crl.pem"]), Cmd = [C#config.openssl_cmd, " ca" " -gencrl ", - " -crlhours 24", + " -crlhours ", integer_to_list(CrlHours), " -out ", CACRLFile, " -config ", CACnfFile], Env = [{"ROOTDIR", filename:absname(Root)}], cmd(Cmd, Env). +can_generate_expired_crls(C) -> + %% OpenSSL can generate CRLs with an expiration date in the past, + %% if we pass a negative number for -crlhours. However, LibreSSL + %% rejects this with the error "invalid argument -24: too small". + %% Let's check which one we have. + Cmd = [C#config.openssl_cmd, " ca -crlhours -24"], + Output = os:cmd(Cmd), + 0 =:= string:str(Output, "too small"). + verify(Root, CA, User, C) -> CAFile = filename:join([Root, User, "cacerts.pem"]), CACRLFile = filename:join([Root, CA, "crl.pem"]), diff --git a/lib/ssl/test/ssl_crl_SUITE.erl b/lib/ssl/test/ssl_crl_SUITE.erl index aa321407b7..00636e5660 100644 --- a/lib/ssl/test/ssl_crl_SUITE.erl +++ b/lib/ssl/test/ssl_crl_SUITE.erl @@ -41,20 +41,26 @@ groups() -> [ {check_true, [], [{group, v2_crl}, {group, v1_crl}, - {group, idp_crl}]}, + {group, idp_crl}, + {group, crl_hash_dir}]}, {check_peer, [], [{group, v2_crl}, {group, v1_crl}, - {group, idp_crl}]}, + {group, idp_crl}, + {group, crl_hash_dir}]}, {check_best_effort, [], [{group, v2_crl}, {group, v1_crl}, - {group, idp_crl}]}, + {group, idp_crl}, + {group, crl_hash_dir}]}, {v2_crl, [], basic_tests()}, {v1_crl, [], basic_tests()}, - {idp_crl, [], basic_tests()}]. + {idp_crl, [], basic_tests()}, + {crl_hash_dir, [], basic_tests() ++ crl_hash_dir_tests()}]. basic_tests() -> [crl_verify_valid, crl_verify_revoked, crl_verify_no_crl]. +crl_hash_dir_tests() -> + [crl_hash_dir_collision, crl_hash_dir_expired]. init_per_suite(Config) -> case os:find_executable("openssl") of @@ -101,7 +107,24 @@ init_per_group(Group, Config0) -> CertDir = filename:join(proplists:get_value(priv_dir, Config0), Group), {CertOpts, Config} = init_certs(CertDir, Group, Config0), {ok, _} = make_certs:all(DataDir, CertDir, CertOpts), - [{cert_dir, CertDir}, {idp_crl, false} | Config] + case Group of + crl_hash_dir -> + CrlDir = filename:join(CertDir, "crls"), + %% Copy CRLs to their hashed filenames. + %% Find the hashes with 'openssl crl -noout -hash -in crl.pem'. + populate_crl_hash_dir(CertDir, CrlDir, + [{"erlangCA", "d6134ed3"}, + {"otpCA", "d4c8d7e5"}], + replace), + CrlCacheOpts = [{crl_cache, + {ssl_crl_hash_dir, + {internal, [{dir, CrlDir}]}}}]; + _ -> + CrlCacheOpts = [] + end, + [{crl_cache_opts, CrlCacheOpts}, + {cert_dir, CertDir}, + {idp_crl, false} | Config] end. end_per_group(_GroupName, Config) -> @@ -164,9 +187,10 @@ crl_verify_valid(Config) when is_list(Config) -> {crl_cache, {ssl_crl_cache, {internal, [{http, 5000}]}}}, {verify, verify_peer}]; false -> - [{cacertfile, filename:join([PrivDir, "server", "cacerts.pem"])}, - {crl_check, Check}, - {verify, verify_peer}] + ?config(crl_cache_opts, Config) ++ + [{cacertfile, filename:join([PrivDir, "server", "cacerts.pem"])}, + {crl_check, Check}, + {verify, verify_peer}] end, {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config), @@ -196,9 +220,10 @@ crl_verify_revoked(Config) when is_list(Config) -> {crl_check, Check}, {verify, verify_peer}]; false -> - [{cacertfile, filename:join([PrivDir, "revoked", "cacerts.pem"])}, - {crl_check, Check}, - {verify, verify_peer}] + ?config(crl_cache_opts, Config) ++ + [{cacertfile, filename:join([PrivDir, "revoked", "cacerts.pem"])}, + {crl_check, Check}, + {verify, verify_peer}] end, crl_verify_error(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts, @@ -251,6 +276,134 @@ crl_verify_no_crl(Config) when is_list(Config) -> crl_verify_valid(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts) end. +crl_hash_dir_collision() -> + [{doc,"Verify ssl_crl_hash_dir behaviour with hash collisions"}]. +crl_hash_dir_collision(Config) when is_list(Config) -> + PrivDir = ?config(cert_dir, Config), + Check = ?config(crl_check, Config), + + %% Create two CAs whose names hash to the same value + CA1 = "hash-collision-0000000000", + CA2 = "hash-collision-0258497583", + CertsConfig = make_certs:make_config([]), + make_certs:intermediateCA(PrivDir, CA1, "erlangCA", CertsConfig), + make_certs:intermediateCA(PrivDir, CA2, "erlangCA", CertsConfig), + + make_certs:enduser(PrivDir, CA1, "collision-client-1", CertsConfig), + make_certs:enduser(PrivDir, CA2, "collision-client-2", CertsConfig), + + [ServerOpts1, ServerOpts2] = + [ + [{keyfile, filename:join([PrivDir, EndUser, "key.pem"])}, + {certfile, filename:join([PrivDir, EndUser, "cert.pem"])}, + {cacertfile, filename:join([PrivDir, EndUser, "cacerts.pem"])}] + || EndUser <- ["collision-client-1", "collision-client-2"]], + + %% Add CRLs for our new CAs into the CRL hash directory. + %% Find the hashes with 'openssl crl -noout -hash -in crl.pem'. + CrlDir = filename:join(PrivDir, "crls"), + populate_crl_hash_dir(PrivDir, CrlDir, + [{CA1, "b68fc624"}, + {CA2, "b68fc624"}], + replace), + + ClientOpts = ?config(crl_cache_opts, Config) ++ + [{cacertfile, filename:join([PrivDir, "erlangCA", "cacerts.pem"])}, + {crl_check, Check}, + {verify, verify_peer}], + + {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config), + + %% Neither certificate revoked; both succeed. + crl_verify_valid(Hostname, ServerNode, ServerOpts1, ClientNode, ClientOpts), + crl_verify_valid(Hostname, ServerNode, ServerOpts2, ClientNode, ClientOpts), + + make_certs:revoke(PrivDir, CA1, "collision-client-1", CertsConfig), + populate_crl_hash_dir(PrivDir, CrlDir, + [{CA1, "b68fc624"}, + {CA2, "b68fc624"}], + replace), + + %% First certificate revoked; first fails, second succeeds. + crl_verify_error(Hostname, ServerNode, ServerOpts1, ClientNode, ClientOpts, + "certificate revoked"), + crl_verify_valid(Hostname, ServerNode, ServerOpts2, ClientNode, ClientOpts), + + make_certs:revoke(PrivDir, CA2, "collision-client-2", CertsConfig), + populate_crl_hash_dir(PrivDir, CrlDir, + [{CA1, "b68fc624"}, + {CA2, "b68fc624"}], + replace), + + %% Second certificate revoked; both fail. + crl_verify_error(Hostname, ServerNode, ServerOpts1, ClientNode, ClientOpts, + "certificate revoked"), + crl_verify_error(Hostname, ServerNode, ServerOpts2, ClientNode, ClientOpts, + "certificate revoked"), + + ok. + +crl_hash_dir_expired() -> + [{doc,"Verify ssl_crl_hash_dir behaviour with expired CRLs"}]. +crl_hash_dir_expired(Config) when is_list(Config) -> + PrivDir = ?config(cert_dir, Config), + Check = ?config(crl_check, Config), + + CA = "CRL-maybe-expired-CA", + %% Add "issuing distribution point", to ensure that verification + %% fails if there is no valid CRL. + CertsConfig = make_certs:make_config([{issuing_distribution_point, true}]), + make_certs:can_generate_expired_crls(CertsConfig) + orelse throw({skip, "cannot generate CRLs with expiry date in the past"}), + make_certs:intermediateCA(PrivDir, CA, "erlangCA", CertsConfig), + EndUser = "CRL-maybe-expired", + make_certs:enduser(PrivDir, CA, EndUser, CertsConfig), + + ServerOpts = [{keyfile, filename:join([PrivDir, EndUser, "key.pem"])}, + {certfile, filename:join([PrivDir, EndUser, "cert.pem"])}, + {cacertfile, filename:join([PrivDir, EndUser, "cacerts.pem"])}], + ClientOpts = ?config(crl_cache_opts, Config) ++ + [{cacertfile, filename:join([PrivDir, CA, "cacerts.pem"])}, + {crl_check, Check}, + {verify, verify_peer}], + {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config), + + %% First make a CRL that expired yesterday. + make_certs:gencrl(PrivDir, CA, CertsConfig, -24), + CrlDir = filename:join(PrivDir, "crls"), + populate_crl_hash_dir(PrivDir, CrlDir, + [{CA, "1627b4b0"}], + replace), + + %% Since the CRL has expired, it's treated as missing, and the + %% outcome depends on the crl_check setting. + case Check of + true -> + %% The error "revocation status undetermined" gets turned + %% into "bad certificate". + crl_verify_error(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts, + "bad certificate"); + peer -> + crl_verify_error(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts, + "bad certificate"); + best_effort -> + %% In "best effort" mode, we consider the certificate not + %% to be revoked if we can't find the appropriate CRL. + crl_verify_valid(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts) + end, + + %% Now make a CRL that expires tomorrow. + make_certs:gencrl(PrivDir, CA, CertsConfig, 24), + CrlDir = filename:join(PrivDir, "crls"), + populate_crl_hash_dir(PrivDir, CrlDir, + [{CA, "1627b4b0"}], + add), + + %% With a valid CRL, verification should always pass. + crl_verify_valid(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts), + + ok. + crl_verify_valid(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts) -> Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0}, {from, self()}, @@ -311,3 +464,31 @@ make_dir_path(PathComponents) -> rename_crl(Filename) -> file:rename(Filename, Filename ++ ".notfound"). + +populate_crl_hash_dir(CertDir, CrlDir, CAsHashes, AddOrReplace) -> + ok = filelib:ensure_dir(filename:join(CrlDir, "crls")), + case AddOrReplace of + replace -> + %% Delete existing files, so we can override them. + [ok = file:delete(FileToDelete) || + {_CA, Hash} <- CAsHashes, + FileToDelete <- filelib:wildcard( + filename:join(CrlDir, Hash ++ ".r*"))]; + add -> + ok + end, + %% Create new files, incrementing suffix if needed to find unique names. + [{ok, _} = + file:copy(filename:join([CertDir, CA, "crl.pem"]), + find_free_name(CrlDir, Hash, 0)) + || {CA, Hash} <- CAsHashes], + ok. + +find_free_name(CrlDir, Hash, N) -> + Name = filename:join(CrlDir, Hash ++ ".r" ++ integer_to_list(N)), + case filelib:is_file(Name) of + true -> + find_free_name(CrlDir, Hash, N + 1); + false -> + Name + end. |