From 6cced538abd4f8053c009b163efa8c6d568b9580 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Thu, 9 Sep 2010 17:07:22 +0200 Subject: Improved certificate extension handling Added the functionality so that the verification fun will be called when a certificate is considered valid by the path validation to allow access to eachs certificate in the path to the user application. Removed clause that only check that a extension is not critical, it does alter the verification rusult only withholds information from the application. Try to verify subject-AltName, if unable to verify it let application try. --- lib/public_key/include/public_key.hrl | 6 ++- lib/public_key/src/pubkey_cert.erl | 91 +++++++++++++------------------- lib/public_key/src/public_key.erl | 5 +- lib/public_key/test/public_key_SUITE.erl | 8 ++- lib/ssl/doc/src/ssl.xml | 55 +++++++++---------- lib/ssl/src/ssl.erl | 8 ++- lib/ssl/src/ssl_certificate.erl | 10 ++-- lib/ssl/test/ssl_basic_SUITE.erl | 10 ++-- 8 files changed, 97 insertions(+), 96 deletions(-) diff --git a/lib/public_key/include/public_key.hrl b/lib/public_key/include/public_key.hrl index 82681502ab..a16eb10fe6 100644 --- a/lib/public_key/include/public_key.hrl +++ b/lib/public_key/include/public_key.hrl @@ -31,8 +31,10 @@ -define(DEFAULT_VERIFYFUN, {fun(_,{bad_cert, _} = Reason, _) -> {fail, Reason}; - (_,{extension, _}, UserState) -> - {unknown, UserState} + (_,{extension, _}, UserState) -> + {unknown, UserState}; + (_, valid, UserState) -> + {valid, UserState} end, []}). -record(path_validation_state, { diff --git a/lib/public_key/src/pubkey_cert.erl b/lib/public_key/src/pubkey_cert.erl index 7851981a30..e704c168f1 100644 --- a/lib/public_key/src/pubkey_cert.erl +++ b/lib/public_key/src/pubkey_cert.erl @@ -29,7 +29,7 @@ validate_revoked_status/3, validate_extensions/4, normalize_general_name/1, digest_type/1, is_self_signed/1, is_issuer/2, issuer_id/2, is_fixed_dh_cert/1, - verify_data/1]). + verify_data/1, verify_fun/4]). -define(NULL, 0). @@ -288,6 +288,35 @@ is_fixed_dh_cert(#'OTPCertificate'{tbsCertificate = Extensions}}) -> is_fixed_dh_cert(SubjectPublicKeyInfo, extensions_list(Extensions)). + +%%-------------------------------------------------------------------- +-spec verify_fun(#'OTPTBSCertificate'{}, {bad_cert, atom()} | {extension, #'Extension'{}}| + valid, term(), fun()) -> term(). +%% +%% Description: Gives the user application the opportunity handle path +%% validation errors and unknown extensions and optional do other +%% things with a validated certificate. +%% -------------------------------------------------------------------- +verify_fun(Otpcert, Result, UserState0, VerifyFun) -> + case VerifyFun(Otpcert, Result, UserState0) of + {valid,UserState} -> + UserState; + {fail, Reason} -> + case Result of + {bad_cert, _} -> + throw(Result); + _ -> + throw({bad_cert, Reason}) + end; + {unknown, UserState} -> + case Result of + {extension, #'Extension'{critical = true}} -> + throw({bad_cert, unknown_critical_extension}); + _ -> + UserState + end + end. + %%-------------------------------------------------------------------- %%% Internal functions %%-------------------------------------------------------------------- @@ -315,25 +344,6 @@ extensions_list(asn1_NOVALUE) -> extensions_list(Extensions) -> Extensions. -verify_fun(Otpcert, Result, UserState0, VerifyFun) -> - case VerifyFun(Otpcert, Result, UserState0) of - {valid,UserState} -> - UserState; - {fail, Reason} -> - case Result of - {bad_cert, _} -> - throw(Result); - _ -> - throw({bad_cert, Reason}) - end; - {unknown, UserState} -> - case Result of - {extension, #'Extension'{critical = true}} -> - throw({bad_cert, unknown_critical_extension}); - _ -> - UserState - end - end. extract_verify_data(OtpCert, DerCert) -> {0, Signature} = OtpCert#'OTPCertificate'.signature, @@ -538,47 +548,21 @@ validate_extensions(OtpCert, [#'Extension'{extnID = ?'id-ce-keyUsage', end; validate_extensions(OtpCert, [#'Extension'{extnID = ?'id-ce-subjectAltName', - extnValue = Names} = Ext | Rest], + extnValue = Names, + critical = true} = Ext | Rest], ValidationState, ExistBasicCon, SelfSigned, UserState0, VerifyFun) -> case validate_subject_alt_names(Names) of - true when Names =/= [] -> + true -> validate_extensions(OtpCert, Rest, ValidationState, ExistBasicCon, SelfSigned, UserState0, VerifyFun); false -> - UserState = verify_fun(OtpCert, {bad_cert, invalid_subject_altname}, - UserState0, VerifyFun), - validate_extensions(OtpCert, Rest, ValidationState, ExistBasicCon, - SelfSigned, UserState, VerifyFun); - other -> UserState = verify_fun(OtpCert, {extension, Ext}, UserState0, VerifyFun), validate_extensions(OtpCert, Rest, ValidationState, ExistBasicCon, SelfSigned, UserState, VerifyFun) end; -%% This extension SHOULD NOT be marked critical. Its value -%% does not have to be further validated at this point. -validate_extensions(OtpCert, [#'Extension'{extnID = ?'id-ce-issuerAltName', - extnValue = _} | Rest], - ValidationState, ExistBasicCon, - SelfSigned, UserState, VerifyFun) -> - validate_extensions(OtpCert, Rest, ValidationState, ExistBasicCon, - SelfSigned, UserState, VerifyFun); - -%% This extension MUST NOT be marked critical.Its value -%% does not have to be further validated at this point. -validate_extensions(OtpCert, [#'Extension'{extnID = Id, - extnValue = _, - critical = false} | Rest], - ValidationState, - ExistBasicCon, SelfSigned, - UserState, VerifyFun) - when Id == ?'id-ce-subjectKeyIdentifier'; - Id == ?'id-ce-authorityKeyIdentifier'-> - validate_extensions(OtpCert, Rest, ValidationState, ExistBasicCon, - SelfSigned, UserState, VerifyFun); - validate_extensions(OtpCert, [#'Extension'{extnID = ?'id-ce-nameConstraints', extnValue = NameConst} | Rest], ValidationState, @@ -592,7 +576,6 @@ validate_extensions(OtpCert, [#'Extension'{extnID = ?'id-ce-nameConstraints', validate_extensions(OtpCert, Rest, NewValidationState, ExistBasicCon, SelfSigned, UserState, VerifyFun); - validate_extensions(OtpCert, [#'Extension'{extnID = ?'id-ce-certificatePolicies', critical = true} = Ext| Rest], ValidationState, ExistBasicCon, SelfSigned, UserState0, VerifyFun) -> @@ -653,13 +636,13 @@ is_valid_key_usage(KeyUse, Use) -> lists:member(Use, KeyUse). validate_subject_alt_names([]) -> - true; + false; validate_subject_alt_names([AltName | Rest]) -> case is_valid_subject_alt_name(AltName) of true -> - validate_subject_alt_names(Rest); + true; false -> - false + validate_subject_alt_names(Rest) end. is_valid_subject_alt_name({Name, Value}) when Name == rfc822Name; @@ -688,7 +671,7 @@ is_valid_subject_alt_name({directoryName, _}) -> is_valid_subject_alt_name({_, [_|_]}) -> true; is_valid_subject_alt_name({otherName, #'AnotherName'{}}) -> - other; + false; is_valid_subject_alt_name({_, _}) -> false. diff --git a/lib/public_key/src/public_key.erl b/lib/public_key/src/public_key.erl index 68bf04eeff..9c7817fa8e 100644 --- a/lib/public_key/src/public_key.erl +++ b/lib/public_key/src/public_key.erl @@ -539,6 +539,7 @@ validate(DerCert, #path_validation_state{working_issuer_name = Issuer, user_state = UserState0, verify_fun = VerifyFun} = ValidationState0) -> + OtpCert = pkix_decode_cert(DerCert, otp), UserState1 = pubkey_cert:validate_time(OtpCert, UserState0, VerifyFun), @@ -556,10 +557,12 @@ validate(DerCert, #path_validation_state{working_issuer_name = Issuer, %% We want the key_usage extension to be checked before we validate %% the signature. - UserState = pubkey_cert:validate_signature(OtpCert, DerCert, + UserState0 = pubkey_cert:validate_signature(OtpCert, DerCert, Key, KeyParams, UserState5, VerifyFun), + UserState = pubkey_cert:verify_fun(OtpCert, valid, UserState0, VerifyFun), ValidationState = ValidationState1#path_validation_state{user_state = UserState}, + pubkey_cert:prepare_for_next_cert(OtpCert, ValidationState). sized_binary(Binary) when is_binary(Binary) -> diff --git a/lib/public_key/test/public_key_SUITE.erl b/lib/public_key/test/public_key_SUITE.erl index 46b8c3db8b..ea6a925139 100644 --- a/lib/public_key/test/public_key_SUITE.erl +++ b/lib/public_key/test/public_key_SUITE.erl @@ -377,7 +377,9 @@ pkix_path_validation(Config) when is_list(Config) -> (_,{bad_cert, _} = Reason, _) -> {fail, Reason}; (_,{extension, _}, UserState) -> - {unknown, UserState} + {unknown, UserState}; + (_, valid, UserState) -> + {valid, UserState} end, []}, {ok, _} = public_key:pkix_path_validation(Trusted, [Cert1, Cert3,Cert4], @@ -392,7 +394,9 @@ pkix_path_validation(Config) when is_list(Config) -> (_,{bad_cert, _} = Reason, _) -> {fail, Reason}; (_,{extension, _}, UserState) -> - {unknown, UserState} + {unknown, UserState}; + (_, valid, UserState) -> + {valid, UserState} end, []}, {ok, _} = diff --git a/lib/ssl/doc/src/ssl.xml b/lib/ssl/doc/src/ssl.xml index 0f3054aec3..d5b7253ef3 100644 --- a/lib/ssl/doc/src/ssl.xml +++ b/lib/ssl/doc/src/ssl.xml @@ -202,16 +202,19 @@

The verification fun should be defined as:

- fun(OtpCert :: #'OtpCertificate'{}, - Event :: {bad_cert, Reason :: atom()} | - {extension, #'Extension'{}}, InitialUserState :: term()) -> - {valid, UserState :: term()} | {fail, Reason :: term()} | - {unknown, UserState :: term()}. +fun(OtpCert :: #'OtpCertificate'{}, Event :: {bad_cert, Reason :: atom()} | + {extension, #'Extension'{}}, InitialUserState :: term()) -> + {valid, UserState :: term()} | {fail, Reason :: term()} | + {unknown, UserState :: term()}.

The verify fun will be called during the X509-path validation when an error or an extension unknown to the ssl - application is encountered. See + application is encountered. Additionally it will be called + when a certificate is considered valid by the path validation + to allow access to each certificate in the path to the user + application. + See public_key(3) for definition of #'OtpCertificate'{} and #'Extension'{}.

@@ -229,34 +232,32 @@

The default verify_fun option in verify_peer mode:

- {fun(_,{bad_cert, _} = Reason, _) -> - {fail, Reason}; - (_,{extension, _}, UserState) -> - {unknown, UserState} - end, []} +{fun(_,{bad_cert, _} = Reason, _) -> + {fail, Reason}; + (_,{extension, _}, UserState) -> + {unknown, UserState}; + (_, valid, UserState) -> + {valid, UserState} + end, []}

The default verify_fun option in verify_none mode:

- {fun(_,{bad_cert, unknown_ca}, UserState) -> - {valid, UserState}; - (_,{bad_cert, _} = Reason, _) -> - {fail, Reason}; - (_,{extension, _}, UserState) -> - {unknown, UserState} - end, []} +{fun(_,{bad_cert, unknown_ca}, UserState) -> + {valid, UserState}; + (_,{bad_cert, _} = Reason, _) -> + {fail, Reason}; + (_,{extension, _}, UserState) -> + {unknown, UserState}; + (_, valid, UserState) -> + {valid, UserState} + end, []} -

Possible path validation errors: - {bad_cert, cert_expired}, - {bad_cert, invalid_issuer}, - {bad_cert, invalid_signature}, - {bad_cert, unknown_ca}, - {bad_cert, name_not_permitted}, - {bad_cert, missing_basic_constraint}, - {bad_cert, invalid_key_usage}, - {bad_cert, invalid_subject_altname}

+

Possible path validation errors:

+ +

{bad_cert, cert_expired}, {bad_cert, invalid_issuer}, {bad_cert, invalid_signature}, {bad_cert, unknown_ca}, {bad_cert, name_not_permitted}, {bad_cert, missing_basic_constraint}, {bad_cert, invalid_key_usage}

diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl index cc01b35b64..12dffb413c 100644 --- a/lib/ssl/src/ssl.erl +++ b/lib/ssl/src/ssl.erl @@ -535,7 +535,9 @@ handle_options(Opts0, _Role) -> (_,{bad_cert, _} = Reason, _) -> {fail, Reason}; (_,{extension, _}, UserState) -> - {unknown, UserState} + {unknown, UserState}; + (_, valid, UserState) -> + {valid, UserState} end, []}, UserFailIfNoPeerCert = handle_option(fail_if_no_peer_cert, Opts, false), @@ -631,7 +633,9 @@ validate_option(verify_fun, Fun) when is_function(Fun) -> {fail, Reason} end; (_,{extension, _}, UserState) -> - {unknown, UserState} + {unknown, UserState}; + (_, valid, UserState) -> + {valid, UserState} end, Fun}; validate_option(verify_fun, {Fun, _} = Value) when is_function(Fun) -> Value; diff --git a/lib/ssl/src/ssl_certificate.erl b/lib/ssl/src/ssl_certificate.erl index 6cf57ced81..206024315e 100644 --- a/lib/ssl/src/ssl_certificate.erl +++ b/lib/ssl/src/ssl_certificate.erl @@ -34,7 +34,6 @@ -export([trusted_cert_and_path/2, certificate_chain/2, file_to_certificats/1, - %validate_extensions/6, validate_extension/3, is_valid_extkey_usage/2, is_valid_key_usage/2, @@ -118,8 +117,7 @@ file_to_certificats(File) -> %% Description: Validates ssl/tls specific extensions %%-------------------------------------------------------------------- validate_extension(_,{extension, #'Extension'{extnID = ?'id-ce-extKeyUsage', - extnValue = KeyUse, - critical = true}}, Role) -> + extnValue = KeyUse}}, Role) -> case is_valid_extkey_usage(KeyUse, Role) of true -> {valid, Role}; @@ -128,8 +126,10 @@ validate_extension(_,{extension, #'Extension'{extnID = ?'id-ce-extKeyUsage', end; validate_extension(_, {bad_cert, _} = Reason, _) -> {fail, Reason}; -validate_extension(_, _, Role) -> - {unknown, Role}. +validate_extension(_, {extension, _}, Role) -> + {unknown, Role}; +validate_extension(_, valid, Role) -> + {valid, Role}. %%-------------------------------------------------------------------- -spec is_valid_key_usage(list(), term()) -> boolean(). diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl index 1e96880801..3cb9337775 100644 --- a/lib/ssl/test/ssl_basic_SUITE.erl +++ b/lib/ssl/test/ssl_basic_SUITE.erl @@ -2860,7 +2860,9 @@ unknown_server_ca_fail(Config) when is_list(Config) -> FunAndState = {fun(_,{bad_cert, _} = Reason, _) -> {fail, Reason}; (_,{extension, _}, UserState) -> - {unknown, UserState} + {unknown, UserState}; + (_, valid, UserState) -> + {valid, UserState} end, []}, Client = ssl_test_lib:start_client_error([{node, ClientNode}, {port, Port}, @@ -2926,7 +2928,9 @@ unknown_server_ca_accept_verify_peer(Config) when is_list(Config) -> (_,{bad_cert, _} = Reason, _) -> {fail, Reason}; (_,{extension, _}, UserState) -> - {unknown, UserState} + {unknown, UserState}; + (_, valid, UserState) -> + {valid, UserState} end, []}, Client = ssl_test_lib:start_client([{node, ClientNode}, {port, Port}, @@ -3095,7 +3099,7 @@ session_cache_process_mnesia(suite) -> session_cache_process_mnesia(Config) when is_list(Config) -> session_cache_process(mnesia,Config). -session_cache_process(Type,Config) when is_list(Config) -> +session_cache_process(_Type,Config) when is_list(Config) -> reuse_session(Config). init([Type]) -> -- cgit v1.2.3