Skip to content

Commit

Permalink
public_key: CA extended key usage check
Browse files Browse the repository at this point in the history 
Should check CAs extended key usage conforms with key usage
and therby handle possible critical extension.

Closes #8806
  • Loading branch information
@IngelaAndin
IngelaAndin committed Oct 25, 2024
1 parent 44ffe88 commit 31341c2
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 9 deletions.
81 changes: 73 additions & 8 deletions lib/public_key/src/pubkey_cert.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -701,9 +701,20 @@ validate_extensions(Cert, asn1_NOVALUE, ValidationState, ExistBasicCon,
validate_extensions(Cert, [], ValidationState, ExistBasicCon,
SelfSigned, UserState, VerifyFun);

validate_extensions(_,[], ValidationState, basic_constraint, _SelfSigned,
UserState, _) ->
{ValidationState, UserState};
validate_extensions(#cert{otp = OtpCert} = Cert,[], ValidationState, basic_constraint, _SelfSigned,
UserState0, VerifyFun) ->
TBSCert = OtpCert#'OTPCertificate'.tbsCertificate,
Extensions = extensions_list(TBSCert#'OTPTBSCertificate'.extensions),
KeyUseExt = pubkey_cert:select_extension(?'id-ce-keyUsage', Extensions),
ExtKeyUseExt = pubkey_cert:select_extension(?'id-ce-extKeyUsage', Extensions),
case compatible_ext_key_usage(KeyUseExt, ExtKeyUseExt) of
true ->
{ValidationState, UserState0};
false ->
UserState = verify_fun(Cert, {bad_cert, {key_usage_mismatch, {KeyUseExt, ExtKeyUseExt}}},
UserState0, VerifyFun),
{ValidationState, UserState}
end;
validate_extensions(Cert, [], ValidationState =
#path_validation_state{max_path_length = Len,
last_cert = Last},
Expand Down Expand Up @@ -832,15 +843,20 @@ validate_extensions(Cert, [#'Extension'{extnID = ?'id-ce-inhibitAnyPolicy'} = Ex
SelfSigned, UserState, VerifyFun);
validate_extensions(Cert, [#'Extension'{extnID = ?'id-ce-extKeyUsage',
critical = true,
extnValue = KeyUse} = Extension | Rest],
extnValue = ExtKeyUse} = Extension | Rest],
#path_validation_state{last_cert = false} = ValidationState, ExistBasicCon,
SelfSigned, UserState0, VerifyFun) ->
UserState =
case ext_keyusage_includes_any(KeyUse) of
case ext_keyusage_includes_any(ExtKeyUse) of
true -> %% CA cert that specifies ?anyExtendedKeyUsage should not be marked critical
verify_fun(Cert, {bad_cert, invalid_ext_key_usage}, UserState0, VerifyFun);
false ->
verify_fun(Cert, {extension, Extension}, UserState0, VerifyFun)
case ca_known_extend_key_use(ExtKeyUse) of
true ->
UserState0;
false ->
verify_fun(Cert, {extension, Extension}, UserState0, VerifyFun)
end
end,
validate_extensions(Cert, Rest, ValidationState, ExistBasicCon, SelfSigned,
UserState, VerifyFun);
Expand Down Expand Up @@ -1783,8 +1799,57 @@ is_digitally_sign_cert(Cert) ->
lists:member(keyCertSign, KeyUse)
end.

missing_basic_constraints(Cert, SelfSigned, ValidationState, VerifyFun, UserState0,Len) ->
UserState = verify_fun(Cert, {bad_cert, missing_basic_constraint},
compatible_ext_key_usage(_, undefined) ->
true;
compatible_ext_key_usage(#'Extension'{extnValue = KeyUse}, #'Extension'{extnValue = Purposes}) ->
case ext_keyusage_includes_any(Purposes) of
true ->
true;
false ->
is_compatible_purposes(KeyUse, Purposes)
end.

is_compatible_purposes(_, []) ->
true;
is_compatible_purposes(KeyUse, [?'id-kp-serverAuth'| Rest]) ->
(lists:member(digitalSignature, KeyUse) orelse
lists:member(keyAgreement, KeyUse)) andalso
is_compatible_purposes(KeyUse, Rest);
is_compatible_purposes(KeyUse, [?'id-kp-clientAuth'| Rest]) ->
(lists:member(digitalSignature, KeyUse)
orelse
(lists:member(keyAgreement, KeyUse) orelse lists:member(keyEncipherment, KeyUse)))
andalso is_compatible_purposes(KeyUse, Rest);
is_compatible_purposes(KeyUse, [?'id-kp-codeSigning'| Rest]) ->
lists:member(digitalSignature, KeyUse) andalso
is_compatible_purposes(KeyUse, Rest);
is_compatible_purposes(KeyUse, [?'id-kp-emailProtection'| Rest]) ->
((lists:member(digitalSignature, KeyUse) orelse
lists:member(nonRepudiation, KeyUse))
orelse
(lists:member(keyAgreement, KeyUse) orelse lists:member(keyEncipherment, KeyUse)))
andalso is_compatible_purposes(KeyUse, Rest);
is_compatible_purposes(KeyUse, [Id| Rest]) when Id == ?'id-kp-timeStamping';
Id == ?'id-kp-OCSPSigning'->
(lists:member(digitalSignature, KeyUse) orelse
lists:member(nonRepudiation, KeyUse)) andalso
is_compatible_purposes(KeyUse, Rest);
is_compatible_purposes(KeyUse, [_| Rest]) -> %% Unknown purposes are for user verify_fun to care about
is_compatible_purposes(KeyUse, Rest).

ca_known_extend_key_use(ExtKeyUse) ->
CAExtSet = ca_known_ext_key_usage(),
Intersertion = sets:intersection(CAExtSet, sets:from_list(ExtKeyUse)),
not sets:is_empty(Intersertion).

ca_known_ext_key_usage() ->
%% Following extended key usages are known
sets:from_list([?'id-kp-serverAuth', ?'id-kp-clientAuth',
?'id-kp-codeSigning', ?'id-kp-emailProtection',
?'id-kp-timeStamping', ?'id-kp-OCSPSigning']).

missing_basic_constraints(OtpCert, SelfSigned, ValidationState, VerifyFun, UserState0,Len) ->
UserState = verify_fun(OtpCert, {bad_cert, missing_basic_constraint},
UserState0, VerifyFun),
case SelfSigned of
true ->
Expand Down
1 change: 1 addition & 0 deletions lib/public_key/src/public_key.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ The reason that a certifcate gets rejected by the certificate path validation.
""".
-type bad_cert_reason() :: cert_expired | invalid_issuer | invalid_signature | name_not_permitted |
missing_basic_constraint | invalid_key_usage | duplicate_cert_in_path |
{key_usage_mismatch, term()} |
{'policy_requirement_not_met', term()} | {'invalid_policy_mapping', term()} |
{revoked, crl_reason()} | invalid_validity_dates |
{revocation_status_undetermined, term()} | atom().
Expand Down
57 changes: 56 additions & 1 deletion lib/public_key/test/public_key_SUITE.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@
pkix_path_validation_root_expired/1,
pkix_ext_key_usage/0,
pkix_ext_key_usage/1,
pkix_ext_key_usage_any/0,
pkix_ext_key_usage_any/1,
pkix_path_validation_bad_date/0,
pkix_path_validation_bad_date/1,
pkix_verify_hostname_cn/1,
Expand Down Expand Up @@ -170,6 +172,7 @@ all() ->
pkix_path_validation,
pkix_path_validation_root_expired,
pkix_ext_key_usage,
pkix_ext_key_usage_any,
pkix_path_validation_bad_date,
pkix_iso_rsa_oid,
pkix_iso_dsa_oid,
Expand Down Expand Up @@ -1006,11 +1009,63 @@ pkix_path_validation_root_expired(Config) when is_list(Config) ->
{error, {bad_cert, cert_expired}} = public_key:pkix_path_validation(Root, [ICA, Peer], []).

pkix_ext_key_usage() ->
[{doc, "Extended key usage is usually in end entity certs, may be in CA but should not be critical in such case"}].
[{doc, "If extended key usage is a critical extension in a CA (usually not included) make sure it is compatible with keyUsage extension"}].
pkix_ext_key_usage(Config) when is_list(Config) ->
SRootSpec = public_key:pkix_test_root_cert("OTP test server ROOT", []),
CRootSpec = public_key:pkix_test_root_cert("OTP test client ROOT", []),

CAExtServer = [#'Extension'{extnID = ?'id-ce-keyUsage',
extnValue = [digitalSignature, keyCertSign, cRLSign],
critical = false},
#'Extension'{extnID = ?'id-ce-extKeyUsage',
extnValue = [?'id-kp-OCSPSigning'],
critical = true}],
CAExtClient = [#'Extension'{extnID = ?'id-ce-keyUsage',
extnValue = [digitalSignature, keyCertSign, cRLSign],
critical = false},
#'Extension'{extnID = ?'id-ce-extKeyUsage',
extnValue = [?'id-kp-codeSigning'],
critical = true}],

#{server_config := SConf,
client_config := CConf} = public_key:pkix_test_data(#{server_chain => #{root => SRootSpec,
intermediates => [[{extensions, CAExtServer}]],
peer => []},
client_chain => #{root => CRootSpec,
intermediates => [[{extensions, CAExtClient}]],
peer => []}}),
[_STRoot, SICA, SRoot] = proplists:get_value(cacerts, SConf),
[_CTRoot, CICA, CRoot] = proplists:get_value(cacerts, CConf),
SPeer = proplists:get_value(cert, SConf),
CPeer = proplists:get_value(cert, CConf),

{ok, _} = public_key:pkix_path_validation(SRoot, [SICA, SPeer], []),
{ok, _} = public_key:pkix_path_validation(CRoot, [CICA, CPeer], []),

CAExtServerFail = [#'Extension'{extnID = ?'id-ce-keyUsage',
extnValue = [keyCertSign, cRLSign],
critical = false},
#'Extension'{extnID = ?'id-ce-extKeyUsage',
extnValue = [?'id-kp-timeStamping'],
critical = true}],

#{server_config := SConf1} = public_key:pkix_test_data(#{server_chain => #{root => SRootSpec,
intermediates => [[{extensions, CAExtServerFail}]],
peer => []},
client_chain => #{root => CRootSpec,
intermediates => [[{extensions, CAExtClient}]],
peer => []}}),
[_, SICA1, SRoot1] = proplists:get_value(cacerts, SConf1),
SPeer1 = proplists:get_value(cert, SConf1),

{error, {bad_cert,{key_usage_mismatch, _}}} = public_key:pkix_path_validation(SRoot1, [SICA1, SPeer1], []).

pkix_ext_key_usage_any() ->
[{doc, "Extended key usage is usually in end entity certs, may be in CA but should not be critical in such case"}].
pkix_ext_key_usage_any(Config) when is_list(Config) ->
SRootSpec = public_key:pkix_test_root_cert("OTP test server ROOT", []),
CRootSpec = public_key:pkix_test_root_cert("OTP test client ROOT", []),

FailCAExt = [#'Extension'{extnID = ?'id-ce-extKeyUsage',
extnValue = [?'anyExtendedKeyUsage'],
critical = true}],
Expand Down

0 comments on commit 31341c2

Please sign in to comment.