Skip to content

Commit 35cc603

Browse files
committed
Merge branch 'ingela/ssl/OTP-19352' into maint
* ingela/ssl/OTP-19352: ssl: Peer ext-keyusage extension should alwyas be verified if present public_key: Also apply to end entity cert public_key: CA extended key usage check
2 parents 3b92c38 + e1fdb78 commit 35cc603

File tree

9 files changed

+361
-112
lines changed

9 files changed

+361
-112
lines changed

lib/public_key/src/pubkey_cert.erl

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -700,21 +700,10 @@ validate_extensions(Cert, asn1_NOVALUE, ValidationState, ExistBasicCon,
700700
SelfSigned, UserState, VerifyFun) ->
701701
validate_extensions(Cert, [], ValidationState, ExistBasicCon,
702702
SelfSigned, UserState, VerifyFun);
703-
704-
validate_extensions(#cert{otp = OtpCert} = Cert,[], ValidationState, basic_constraint, _SelfSigned,
703+
validate_extensions(Cert, [], ValidationState, basic_constraint, _SelfSigned,
705704
UserState0, VerifyFun) ->
706-
TBSCert = OtpCert#'OTPCertificate'.tbsCertificate,
707-
Extensions = extensions_list(TBSCert#'OTPTBSCertificate'.extensions),
708-
KeyUseExt = pubkey_cert:select_extension(?'id-ce-keyUsage', Extensions),
709-
ExtKeyUseExt = pubkey_cert:select_extension(?'id-ce-extKeyUsage', Extensions),
710-
case compatible_ext_key_usage(KeyUseExt, ExtKeyUseExt) of
711-
true ->
712-
{ValidationState, UserState0};
713-
false ->
714-
UserState = verify_fun(Cert, {bad_cert, {key_usage_mismatch, {KeyUseExt, ExtKeyUseExt}}},
715-
UserState0, VerifyFun),
716-
{ValidationState, UserState}
717-
end;
705+
UserState = validate_ext_key_usage(Cert, UserState0, VerifyFun, ca),
706+
{ValidationState, UserState};
718707
validate_extensions(Cert, [], ValidationState =
719708
#path_validation_state{max_path_length = Len,
720709
last_cert = Last},
@@ -723,8 +712,9 @@ validate_extensions(Cert, [], ValidationState =
723712
true when SelfSigned ->
724713
{ValidationState, UserState0};
725714
true ->
715+
UserState = validate_ext_key_usage(Cert, UserState0, VerifyFun, endentity),
726716
{ValidationState#path_validation_state{max_path_length = Len - 1},
727-
UserState0};
717+
UserState};
728718
false ->
729719
%% basic_constraint must appear in certs used for digital sign
730720
%% see 4.2.1.10 in rfc 3280
@@ -737,7 +727,6 @@ validate_extensions(Cert, [], ValidationState =
737727
{ValidationState, UserState0}
738728
end
739729
end;
740-
741730
validate_extensions(Cert,
742731
[#'Extension'{extnID = ?'id-ce-basicConstraints',
743732
extnValue =
@@ -890,6 +879,18 @@ handle_last_cert(Cert, #path_validation_state{last_cert = true,
890879
handle_last_cert(_, ValidationState) ->
891880
ValidationState.
892881

882+
validate_ext_key_usage(#cert{otp = OtpCert} = Cert, UserState, VerifyFun, Type) ->
883+
TBSCert = OtpCert#'OTPCertificate'.tbsCertificate,
884+
Extensions = extensions_list(TBSCert#'OTPTBSCertificate'.extensions),
885+
KeyUseExt = pubkey_cert:select_extension(?'id-ce-keyUsage', Extensions),
886+
ExtKeyUseExt = pubkey_cert:select_extension(?'id-ce-extKeyUsage', Extensions),
887+
case compatible_ext_key_usage(KeyUseExt, ExtKeyUseExt, Type) of
888+
true ->
889+
UserState;
890+
false ->
891+
verify_fun(Cert, {bad_cert, {key_usage_mismatch, {KeyUseExt, ExtKeyUseExt}}},
892+
UserState, VerifyFun)
893+
end.
893894

894895
%%====================================================================
895896
%% Policy handling
@@ -1799,9 +1800,11 @@ is_digitally_sign_cert(Cert) ->
17991800
lists:member(keyCertSign, KeyUse)
18001801
end.
18011802

1802-
compatible_ext_key_usage(_, undefined) ->
1803+
compatible_ext_key_usage(undefined, _, endentity) -> %% keyusage (first arg )is mandantory in CAs
18031804
true;
1804-
compatible_ext_key_usage(#'Extension'{extnValue = KeyUse}, #'Extension'{extnValue = Purposes}) ->
1805+
compatible_ext_key_usage(_, undefined, _) ->
1806+
true;
1807+
compatible_ext_key_usage(#'Extension'{extnValue = KeyUse}, #'Extension'{extnValue = Purposes}, _) ->
18051808
case ext_keyusage_includes_any(Purposes) of
18061809
true ->
18071810
true;
@@ -2104,7 +2107,7 @@ extensions(Role, Type, Opts) ->
21042107

21052108
add_default_extensions(_, ca, Exts) ->
21062109
Default = [#'Extension'{extnID = ?'id-ce-keyUsage',
2107-
extnValue = [keyCertSign, cRLSign],
2110+
extnValue = [keyCertSign, digitalSignature, cRLSign],
21082111
critical = false},
21092112
#'Extension'{extnID = ?'id-ce-basicConstraints',
21102113
extnValue = #'BasicConstraints'{cA = true},
@@ -2121,9 +2124,12 @@ add_default_extensions(server, peer, Exts) ->
21212124
critical = false}
21222125
],
21232126
add_default_extensions(Default, Exts);
2124-
21252127
add_default_extensions(client, peer, Exts) ->
2126-
Exts.
2128+
Default = [#'Extension'{extnID = ?'id-ce-keyUsage',
2129+
extnValue = [digitalSignature],
2130+
critical = false}
2131+
],
2132+
add_default_extensions(Default, Exts).
21272133

21282134
add_default_extensions(Defaults0, Exts) ->
21292135
Defaults = lists:filtermap(fun(#'Extension'{extnID = ID} = Ext) ->

lib/public_key/src/public_key.erl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,7 +1567,7 @@ Available options:
15671567

15681568
```erlang
15691569
fun(OtpCert :: #'OTPCertificate'{},
1570-
Event :: {bad_cert, Reason :: atom() | {revoked, atom()}} |
1570+
Event :: {bad_cert, Reason :: bad_cert_reason() | {revoked, atom()}} |
15711571
{extension, #'Extension'{}},
15721572
UserState :: term()) ->
15731573
{valid, UserState :: term()} |
@@ -1581,7 +1581,7 @@ Available options:
15811581
```erlang
15821582
fun(OtpCert :: #'OTPCertificate'{},
15831583
DerCert :: der_encoded(),
1584-
Event :: {bad_cert, Reason :: atom() | {revoked, atom()}} |
1584+
Event :: {bad_cert, Reason :: bad_cert_reason() | {revoked, atom()}} |
15851585
{extension, #'Extension'{}},
15861586
UserState :: term()) ->
15871587
{valid, UserState :: term()} |

lib/ssl/src/ssl.app.src

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,6 @@
9292
{applications, [crypto, public_key, kernel, stdlib]},
9393
{env, []},
9494
{mod, {ssl_app, []}},
95-
{runtime_dependencies, ["stdlib-6.0","public_key-1.16.2","kernel-9.0",
95+
{runtime_dependencies, ["stdlib-6.0","public_key-1.16.4","kernel-9.0",
9696
"erts-15.0","crypto-5.0", "inets-5.10.7",
9797
"runtime_tools-1.15.1"]}]}.

lib/ssl/src/ssl.erl

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -847,6 +847,20 @@ Common certificate related options to both client and server.
847847

848848
The chain consisted only of one self-signed certificate.
849849

850+
- **{invalid_ext_keyusage, [public_key:oid()]} **
851+
852+
If the peer certificate specifies the extended keyusage extension and does
853+
not include the purpose for either being a TLS server (id-kp-ServerAuth) or
854+
TLS client (id-kp-ClientAuth) depending on the peers role.
855+
856+
- **{ca_invalid_ext_keyusage, [public_key:oid()]} **
857+
858+
If a CA certificate specifies the extended keyusage extension and does
859+
not include the purpose for either being a TLS server
860+
(id-kp-ServerAuth) or TLS client (id-kp-ClientAuth) depending
861+
on the role of the peer chained with this CA, or the option allow_any_ca_purpose is set to `true`
862+
but the special any-value (anyExtendedKeyUsage) is not included in the CA cert purposes.
863+
850864
- **`PKIX X-509-path validation error`**
851865

852866
For possible reasons, see `public_key:pkix_path_validation/3`.
@@ -857,6 +871,13 @@ Common certificate related options to both client and server.
857871
see [public_key:pkix_path_validation/3](`public_key:pkix_path_validation/3`) for
858872
more details.
859873

874+
- **`{allow_any_ca_purpose, boolean()}`** - Handle certificate extended key usages extension
875+
876+
If a CA certificate has an extended key usage extension but it does not want to
877+
restrict the usages of the key it can include a special `anyExtendedKeyUsage` purpose.
878+
If this is option is set to `true` all key usage purposes is automatically
879+
accepted for a CA that include that purpose, the options default to false.
880+
860881
- **`{cerl_check, Check}`** - Handle certificate revocation lists.
861882

862883
Perform CRL (Certificate Revocation List) verification
@@ -892,6 +913,7 @@ Common certificate related options to both client and server.
892913
{explicit_policy, boolean()} |
893914
{inhibit_policy_mapping, boolean()} |
894915
{inhibit_any_policy, boolean()}]} |
916+
{allow_any_ca_purpose, Allow::boolean()} |
895917
{crl_check, Check::boolean() | peer | best_effort} |
896918
{crl_cache, crl_cache_opts()} |
897919
{partial_chain, anchor_fun()}.
@@ -3706,6 +3728,7 @@ ssl_options() ->
37063728
use_srtp,
37073729
user_lookup_fun,
37083730
verify, verify_fun, cert_policy_opts,
3731+
allow_any_ca_purpose,
37093732
versions
37103733
].
37113734

@@ -3874,7 +3897,7 @@ opt_verification(UserOpts, Opts0, #{role := Role} = Env) ->
38743897

38753898
Opts3 = set_opt_int(depth, 0, 255, ?DEFAULT_DEPTH, UserOpts, Opts2),
38763899

3877-
Opts = case Role of
3900+
Opts4 = case Role of
38783901
client ->
38793902
opt_verify_fun(UserOpts, Opts3#{partial_chain => PartialChain},
38803903
Env);
@@ -3883,7 +3906,8 @@ opt_verification(UserOpts, Opts0, #{role := Role} = Env) ->
38833906
fail_if_no_peer_cert => FailNoPeerCert},
38843907
Env)
38853908
end,
3886-
opt_policies(UserOpts, Opts).
3909+
Opts = opt_policies(UserOpts, Opts4),
3910+
opt_extend_keyusage(UserOpts, Opts).
38873911

38883912
default_verify(client) ->
38893913
%% Server authenication is by default requiered
@@ -3948,6 +3972,16 @@ opt_policies(UserOpts, Opts) ->
39483972
Opts#{cert_policy_opts => POpts}
39493973
end.
39503974

3975+
opt_extend_keyusage(UserOpts, Opts) ->
3976+
case get_opt_bool(allow_any_ca_purpose, false, UserOpts, Opts) of
3977+
{default, Value} ->
3978+
Opts#{allow_any_ca_purpose => Value};
3979+
{old, _OldValue} ->
3980+
Opts;
3981+
{new, NewValue} ->
3982+
Opts#{allow_any_ca_purpose => NewValue}
3983+
end.
3984+
39513985
validate_policy_opts([]) ->
39523986
true;
39533987
validate_policy_opts([{policy_set, OidList} | Rest]) when is_list(OidList) ->

lib/ssl/src/ssl_certificate.erl

Lines changed: 56 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -202,25 +202,42 @@ file_to_crls(File, DbHandle) ->
202202
%%
203203
%% Description: Validates ssl/tls specific extensions
204204
%%--------------------------------------------------------------------
205-
validate(_,{extension, #'Extension'{extnID = ?'id-ce-extKeyUsage',
206-
critical = Critical,
207-
extnValue = KeyUse}}, #{path_len := 1} = UserState,
208-
_LogLevel) ->
209-
%% If extension in peer, check for TLS server/client usage
210-
case is_valid_extkey_usage(KeyUse, Critical, UserState) of
211-
true ->
212-
{valid, UserState};
213-
false ->
214-
{unknown, UserState}
205+
validate(_, {extension, #'Extension'{extnID = ?'id-ce-extKeyUsage'} = Ext}, #{path_len := 1} = UserState, _) ->
206+
case verify_extkeyusage(Ext, UserState) of
207+
valid ->
208+
{valid, UserState};
209+
KeyUses ->
210+
{fail, {bad_cert, {invalid_ext_keyusage, KeyUses}}}
215211
end;
216-
validate(_, {extension, _}, UserState, _LogLevel) ->
212+
validate(_, {extension, #'Extension'{extnID = ?'id-ce-extKeyUsage'} = Ext}, UserState, _) ->
213+
case verify_extkeyusage(Ext, UserState) of
214+
valid ->
215+
{valid, UserState};
216+
KeyUses ->
217+
{fail, {bad_cert, {ca_invalid_ext_keyusage, KeyUses}}}
218+
end;
219+
validate(_, {extension, _}, UserState, _) ->
217220
{unknown, UserState};
218-
validate(Issuer, {bad_cert, cert_expired}, #{issuer := Issuer}, _LogLevel) ->
221+
validate(Issuer, {bad_cert, cert_expired}, #{issuer := Issuer}, _) ->
219222
{fail, {bad_cert, root_cert_expired}};
220-
validate(_, {bad_cert, _} = Reason, _, _LogLevel) ->
223+
validate(_, {bad_cert, _} = Reason, _, _) ->
221224
{fail, Reason};
222-
validate(Cert, valid, #{path_len := N} = UserState, LogLevel) ->
223-
case verify_sign_support(Cert, UserState) of
225+
validate(Cert, valid, UserState, LogLevel) ->
226+
common_cert_validation(Cert, UserState, LogLevel);
227+
validate(Cert, valid_peer, UserState0 = #{role := client, server_name := Hostname,
228+
customize_hostname_check := Customize},
229+
LogLevel) when Hostname =/= disable ->
230+
case verify_hostname(Hostname, Customize, Cert, UserState0) of
231+
{valid, UserState} ->
232+
common_cert_validation(Cert, UserState, LogLevel);
233+
Error ->
234+
Error
235+
end;
236+
validate(Cert, valid_peer, UserState, LogLevel) ->
237+
common_cert_validation(Cert, UserState, LogLevel).
238+
239+
common_cert_validation(Cert, #{path_len := N} = UserState, LogLevel) ->
240+
case verify_sign_support(Cert, UserState) of
224241
true ->
225242
case maps:get(cert_ext, UserState, undefined) of
226243
undefined ->
@@ -231,19 +248,7 @@ validate(Cert, valid, #{path_len := N} = UserState, LogLevel) ->
231248
end;
232249
false ->
233250
{fail, {bad_cert, unsupported_signature}}
234-
end;
235-
validate(Cert, valid_peer, UserState = #{role := client, server_name := Hostname,
236-
customize_hostname_check := Customize},
237-
LogLevel) when Hostname =/= disable ->
238-
case verify_hostname(Hostname, Customize, Cert, UserState) of
239-
{valid, UserState} ->
240-
validate(Cert, valid, UserState, LogLevel);
241-
Error ->
242-
Error
243-
end;
244-
validate(Cert, valid_peer, UserState, LogLevel) ->
245-
validate(Cert, valid, UserState, LogLevel).
246-
251+
end.
247252
%%--------------------------------------------------------------------
248253
-spec is_valid_key_usage(list(), term()) -> boolean().
249254
%%
@@ -500,20 +505,35 @@ do_find_issuer(IssuerFun, CertDbHandle, CertDb) ->
500505
Return
501506
end.
502507

503-
is_valid_extkey_usage(KeyUse, true, #{role := Role}) when is_list(KeyUse) ->
504-
is_valid_key_usage(KeyUse, ext_keysage(Role));
505-
is_valid_extkey_usage(KeyUse, true, #{role := Role}) ->
506-
is_valid_key_usage([KeyUse], ext_keysage(Role));
507-
is_valid_extkey_usage(_, false, _) ->
508-
false.
508+
verify_extkeyusage( #'Extension'{extnValue = KeyUses}, UserState)->
509+
case is_valid_extkey_usage(KeyUses, UserState) of
510+
true ->
511+
valid;
512+
false ->
513+
KeyUses
514+
end.
515+
516+
is_valid_extkey_usage(KeyUses, #{path_len := PathNum,
517+
role := Role,
518+
allow_any_ca_purpose := Allow}) ->
519+
case PathNum of
520+
1 -> %% Peer Cert
521+
is_valid_key_usage(KeyUses, ext_keyusage(Role));
522+
_ -> %% CA cert
523+
is_valid_key_usage(KeyUses, ext_keyusage(Role))
524+
orelse (Allow andalso ext_keyusage_includes_any(KeyUses))
525+
end.
509526

510-
ext_keysage(client) ->
527+
ext_keyusage(client) ->
511528
%% Client wants to verify server
512529
?'id-kp-serverAuth';
513-
ext_keysage(server) ->
530+
ext_keyusage(server) ->
514531
%% Server wants to verify client
515532
?'id-kp-clientAuth'.
516533

534+
ext_keyusage_includes_any(KeyUse) ->
535+
lists:member(?anyExtendedKeyUsage, KeyUse).
536+
517537
verify_cert_signer(BinCert, SignerTBSCert) ->
518538
PublicKey = public_key(SignerTBSCert#'OTPTBSCertificate'.subjectPublicKeyInfo),
519539
public_key:pkix_verify(BinCert, PublicKey).

0 commit comments

Comments
 (0)