File 4542-ssl-Refactor-to-avoid-re-encoding-of-decoded-certs.patch of Package erlang
From 58b7b1b37a349bf854ac1346201b49edacd2fc65 Mon Sep 17 00:00:00 2001
From: Ingela Anderton Andin <ingela@erlang.org>
Date: Tue, 21 Sep 2021 17:16:01 +0200
Subject: [PATCH 2/5] ssl: Refactor to avoid re-encoding of decoded certs
Let public key define the record #cert{} that can be used to provide
the original DER certs received from the peer and the decoded version
together as input to certificate handling functions such as
public:pkix_path_validation/3
will be more efficient as we can avoid conversions and also a way for
applications such as ssl to tolerate certs with some ASN1-encoding errors
that can be worked around.
---
lib/public_key/doc/src/public_key.xml | 62 +++++++++++++++++----------
lib/public_key/include/public_key.hrl | 5 +++
lib/public_key/src/public_key.erl | 43 +++++++++++++------
lib/ssl/src/ssl_certificate.erl | 22 ++++------
lib/ssl/src/ssl_crl.erl | 2 +-
lib/ssl/src/ssl_handshake.erl | 13 ++++--
lib/ssl/src/ssl_internal.hrl | 1 -
7 files changed, 94 insertions(+), 54 deletions(-)
diff --git a/lib/public_key/doc/src/public_key.xml b/lib/public_key/doc/src/public_key.xml
index 4f569f2c6d..f4eb4bdefd 100644
--- a/lib/public_key/doc/src/public_key.xml
+++ b/lib/public_key/doc/src/public_key.xml
@@ -64,6 +64,14 @@
</desc>
</datatype>
+ <datatype>
+ <name name="key_oid_name"/>
+ <desc>
+ <p>Macro names for key object identifiers used by prefixing with ?</p>
+ </desc>
+ </datatype>
+
+
<datatype>
<name name="der_encoded"/>
<desc>
@@ -100,11 +108,14 @@
<datatype>
<name name="public_key"/>
<name name="rsa_public_key"/>
+ <name name="dss_public_key"/>
<name name="rsa_pss_public_key"/>
<name name="dsa_public_key"/>
<name name="ec_public_key"/>
+ <name name="public_key_params"/>
<name name="ecpk_parameters"/>
<name name="ecpk_parameters_api"/>
+ <name name="public_key_info"/>
<desc>
</desc>
</datatype>
@@ -139,6 +150,12 @@
</desc>
</datatype>
+ <datatype>
+ <name name="ed_oid_name"/>
+ <desc>
+ Macro names for object identifiers for EDDSA curves used by prefixing with ?
+ </desc>
+ </datatype>
<datatype>
<name name="key_params"/>
@@ -146,6 +163,7 @@
</desc>
</datatype>
+
<datatype>
<name name="digest_type"/>
<desc>
@@ -153,17 +171,32 @@
</datatype>
<datatype>
- <name name="crl_reason"/>
+ <name name="cert_id"/>
<desc>
</desc>
</datatype>
+ <datatype>
+ <name name="cert"/>
+ <desc>
+ </desc>
+ </datatype>
+
+
<datatype>
- <name name="cert_id"/>
+ <name name="bad_cert_reason"/>
<desc>
</desc>
</datatype>
+
+ <datatype>
+ <name name="crl_reason"/>
+ <desc>
+ </desc>
+ </datatype>
+
+
<datatype>
<name name="issuer_name"/>
<desc>
@@ -401,33 +434,16 @@
</func>
<func>
- <name since="OTP R16B">pkix_path_validation(TrustedCert, CertChain, Options) -> {ok, {PublicKeyInfo, PolicyTree}} | {error, {bad_cert, Reason}} </name>
+ <name since="OTP R16B" name="pkix_path_validation" arity="3"/>
<fsummary>Performs a basic path validation according to RFC 5280.</fsummary>
- <type>
- <v>TrustedCert = #'OTPCertificate'{} | der_encoded() | atom()</v>
- <d>Normally a trusted certificate, but it can also be a path-validation
- error that can be discovered while
- constructing the input to this function and that is to be run through the <c>verify_fun</c>.
- Examples are <c>unknown_ca</c> and <c>selfsigned_peer.</c>
- </d>
- <v>CertChain = [der_encoded()]</v>
- <d>A list of DER-encoded certificates in trust order ending with the peer certificate.</d>
- <v>Options = proplists:proplist()</v>
- <v>PublicKeyInfo = {?'rsaEncryption' | ?'id-RSASSA-PSS'| ?'id-dsa',
- rsa_public_key() | integer(), 'NULL' | 'RSASSA-PSS-params'{} | 'Dss-Parms'{}}</v>
- <v>PolicyTree = term()</v>
- <d>At the moment this is always an empty list as policies are not currently supported.</d>
- <v>Reason = cert_expired | invalid_issuer | invalid_signature | name_not_permitted |
- missing_basic_constraint | invalid_key_usage | {revoked, crl_reason()} | atom()
- </v>
- </type>
<desc>
<p>
Performs a basic path validation according to
<url href="http://www.ietf.org/rfc/rfc5280.txt">RFC 5280.</url>
However, CRL validation is done separately by <seemfa
marker="#pkix_crls_validate/3">pkix_crls_validate/3 </seemfa> and is to be called
- from the supplied <c>verify_fun</c>.
+ from the supplied <c>verify_fun</c>. The optional policy tree check is currently not implemented
+ but an empty place holder list is returned instead.
</p>
<p>Available options:</p>
@@ -473,7 +489,7 @@ fun(OtpCert :: #'OTPCertificate'{},
</item>
</taglist>
- <p>Possible reasons for a bad certificate: </p>
+ <p>Explanations of reasons for a bad certificate: </p>
<taglist>
<tag>cert_expired</tag>
<item><p>Certificate is no longer valid as its expiration date has passed.</p></item>
diff --git a/lib/public_key/include/public_key.hrl b/lib/public_key/include/public_key.hrl
index 98d032bfdd..bea4b5bc06 100644
--- a/lib/public_key/include/public_key.hrl
+++ b/lib/public_key/include/public_key.hrl
@@ -78,6 +78,11 @@
point
}).
+-record(cert, {
+ der :: public_key:der_encoded(),
+ otp :: #'OTPCertificate'{}
+ }).
+
-define(unspecified, 0).
-define(keyCompromise, 1).
-define(cACompromise, 2).
diff --git a/lib/public_key/src/public_key.erl b/lib/public_key/src/public_key.erl
index 6ecb902db5..0371917009 100644
--- a/lib/public_key/src/public_key.erl
+++ b/lib/public_key/src/public_key.erl
@@ -103,10 +103,12 @@
-type private_key() :: rsa_private_key() | rsa_pss_private_key() | dsa_private_key() | ec_private_key() | ed_private_key() .
-type rsa_public_key() :: #'RSAPublicKey'{}.
-type rsa_private_key() :: #'RSAPrivateKey'{}.
--type rsa_pss_public_key() :: {#'RSAPublicKey'{}, #'RSASSA-PSS-params'{}}.
+-type dss_public_key() :: integer().
+-type rsa_pss_public_key() :: {rsa_pss_public_key(), #'RSASSA-PSS-params'{}}.
-type rsa_pss_private_key() :: { #'RSAPrivateKey'{}, #'RSASSA-PSS-params'{}}.
-type dsa_private_key() :: #'DSAPrivateKey'{}.
--type dsa_public_key() :: {integer(), #'Dss-Parms'{}}.
+-type dsa_public_key() :: {dss_public_key(), #'Dss-Parms'{}}.
+-type public_key_params() :: 'NULL' | #'RSASSA-PSS-params'{} | {namedCurve, oid()} | #'ECParameters'{} | #'Dss-Parms'{}.
-type ecpk_parameters() :: {ecParameters, #'ECParameters'{}} | {namedCurve, Oid::tuple()}.
-type ecpk_parameters_api() :: ecpk_parameters() | #'ECParameters'{} | {namedCurve, Name::crypto:ec_named_curve()}.
-type ec_public_key() :: {#'ECPoint'{}, ecpk_parameters_api()}.
@@ -114,7 +116,8 @@
-type ed_public_key() :: {#'ECPoint'{}, ed_params()} | {ed_pub, ed25519|ed448, Key::binary()}.
-type ed_private_key() :: #'ECPrivateKey'{parameters :: ed_params()} |
{ed_pri, ed25519|ed448, Pub::binary(), Priv::binary()}.
--type ed_params() :: {namedCurve, ?'id-Ed25519' | ?'id-Ed448'}.
+-type ed_oid_name() :: 'id-Ed25519' | 'id-Ed448'.
+-type ed_params() :: {namedCurve, ed_oid_name()}.
-type key_params() :: #'DHParameter'{} | {namedCurve, oid()} | #'ECParameters'{} |
{rsa, Size::integer(), PubExp::integer()}.
-type der_encoded() :: binary().
@@ -151,7 +154,11 @@
-type cert_id() :: {SerialNr::integer(), issuer_name()} .
-type issuer_name() :: {rdnSequence,[[#'AttributeTypeAndValue'{}]]} .
-
+-type bad_cert_reason() :: cert_expired | invalid_issuer | invalid_signature | name_not_permitted | missing_basic_constraint | invalid_key_usage |
+ {revoked, crl_reason()} | atom().
+-type cert() :: #cert{} | der_encoded() | #'OTPCertificate'{}.
+-type public_key_info() :: {key_oid_name(), rsa_public_key() | #'ECPoint'{} | dss_public_key(), public_key_params()}.
+-type key_oid_name() :: 'rsaEncryption' | 'id-RSASSA-PSS' | 'id-ecPublicKey' | 'id-Ed25519' | 'id-Ed448' | 'id-dsa'.
-define(UINT32(X), X:32/unsigned-big-integer).
@@ -1095,16 +1102,22 @@ pkix_normalize_name(Issuer) ->
pubkey_cert:normalize_general_name(Issuer).
%%--------------------------------------------------------------------
--spec pkix_path_validation(Cert::binary()| #'OTPCertificate'{} | atom(),
- CertChain :: [binary() | #'OTPCertificate'{}] ,
- Options :: [{atom(),term()}]) ->
- {ok, {PublicKeyInfo :: term(),
- PolicyTree :: term()}} |
- {error, {bad_cert, Reason :: term()}}.
+-spec pkix_path_validation(Cert, CertChain, Options) ->
+ {ok, {PublicKeyInfo, PolicyTree}} |
+ {error, {bad_cert, Reason :: bad_cert_reason()}}
+ when
+ Cert::binary()| #'OTPCertificate'{} | atom(),
+ CertChain :: [cert()],
+ Options :: [Option],
+ Option :: {verify_fun, {fun(), term()}} | {max_path_lengh, integer()},
+ PublicKeyInfo :: public_key_info(),
+ PolicyTree :: list().
+
%% Description: Performs a basic path validation according to RFC 5280.
%%--------------------------------------------------------------------
pkix_path_validation(TrustedCert, CertChain, Options)
when is_binary(TrustedCert) ->
+
OtpCert = pkix_decode_cert(TrustedCert, otp),
pkix_path_validation(OtpCert, CertChain, Options);
pkix_path_validation(#'OTPCertificate'{} = TrustedCert, CertChain, Options)
@@ -1641,13 +1654,17 @@ validate(Cert, #path_validation_state{working_issuer_name = Issuer,
otp_cert(Der) when is_binary(Der) ->
pkix_decode_cert(Der, otp);
-otp_cert(#'OTPCertificate'{} =Cert) ->
- Cert.
+otp_cert(#'OTPCertificate'{} = Cert) ->
+ Cert;
+otp_cert(#cert{otp = OtpCert}) ->
+ OtpCert.
der_cert(#'OTPCertificate'{} = Cert) ->
pkix_encode('OTPCertificate', Cert, otp);
der_cert(Der) when is_binary(Der) ->
- Der.
+ Der;
+der_cert(#cert{der = DerCert}) ->
+ DerCert.
pkix_crls_validate(_, [],_, Options, #revoke_state{details = Details}) ->
case proplists:get_value(undetermined_details, Options, false) of
diff --git a/lib/ssl/src/ssl_certificate.erl b/lib/ssl/src/ssl_certificate.erl
index cbeb4e4521..acdc85255f 100644
--- a/lib/ssl/src/ssl_certificate.erl
+++ b/lib/ssl/src/ssl_certificate.erl
@@ -88,7 +88,7 @@
%%--------------------------------------------------------------------
-spec trusted_cert_and_paths([der_cert()], db_handle(), certdb_ref(), fun()) ->
- [{der_cert() | unknown_ca | invalid_issuer | selfsigned_peer, [der_cert()]}].
+ [{#cert{} | unknown_ca | invalid_issuer | selfsigned_peer, [#cert{}]}].
%%
%% Description: Construct input to public_key:pkix_path_validation/3,
%% If the ROOT cert is not found {bad_cert, unknown_ca} will be returned
@@ -103,7 +103,7 @@ trusted_cert_and_paths([Peer], CertDbHandle, CertDbRef, PartialChainHandler) ->
Chain = [#cert{der=Peer, otp=OtpCert}],
case public_key:pkix_is_self_signed(OtpCert) of
true ->
- [{selfsigned_peer, [Peer]}];
+ [{selfsigned_peer, Chain}];
false ->
[handle_incomplete_chain(Chain, PartialChainHandler, {unknown_ca, Chain},
CertDbHandle, CertDbRef)]
@@ -122,11 +122,10 @@ trusted_cert_and_paths(Chain0, CertDbHandle, CertDbRef, PartialChainHandler) ->
PartialChainHandler,
Result,
CertDbHandle, CertDbRef);
- {Root, NewChain}->
- decoded_chain(Root, NewChain)
+ {_Root, _NewChain} = Result ->
+ Result
end
end, Paths).
-
%%--------------------------------------------------------------------
-spec certificate_chain(undefined | binary() | #'OTPCertificate'{} , db_handle(),
certdb_ref() | {extracted, list()}) ->
@@ -362,9 +361,8 @@ do_certificate_chain(CertDbHandle, CertsDbRef, Chain, SerialNr, Issuer, _, ListD
{ok, undefined, lists:reverse(Chain)}
end.
-find_alternative_root([OtpCert | _], CertDbHandle, CertDbRef, InvalidatedList) ->
- Cert = public_key:pkix_encode('OTPCertificate', OtpCert, otp),
- case find_issuer(#cert{der=Cert, otp=OtpCert}, CertDbHandle, CertDbRef, [], InvalidatedList) of
+find_alternative_root([Cert | _], CertDbHandle, CertDbRef, InvalidatedList) ->
+ case find_issuer(Cert, CertDbHandle, CertDbRef, [], InvalidatedList) of
{error, issuer_not_found} ->
unknown_ca;
{ok, {SerialNr, IssuerId}} ->
@@ -663,14 +661,12 @@ handle_incomplete_chain([#cert{}=Peer| _] = Chain0, PartialChainHandler, Default
true ->
{Root, Chain} = handle_partial_chain(lists:reverse(ChainCandidate), PartialChainHandler,
CertDbHandle, CertDbRef),
- decoded_chain(Root, Chain);
+ {Root, Chain};
false ->
- {Root, Chain} = Default,
- decoded_chain(Root, Chain)
+ Default
end;
_ ->
- {Root, Chain} = Default,
- decoded_chain(Root, Chain)
+ Default
end.
extraneous_chains(Certs) ->
diff --git a/lib/ssl/src/ssl_crl.erl b/lib/ssl/src/ssl_crl.erl
index 5e35e89b78..d3b66df870 100644
--- a/lib/ssl/src/ssl_crl.erl
+++ b/lib/ssl/src/ssl_crl.erl
@@ -105,7 +105,7 @@ find_issuer(IsIssuerFun, Db, _) ->
Result
end.
-verify_crl_issuer(CRL, ErlCertCandidate, Issuer, NotIssuer) ->
+verify_crl_issuer(CRL, #cert{otp = ErlCertCandidate}, Issuer, NotIssuer) ->
TBSCert = ErlCertCandidate#'OTPCertificate'.tbsCertificate,
case public_key:pkix_normalize_name(TBSCert#'OTPTBSCertificate'.subject) of
Issuer ->
diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl
index f7dc641fb3..63fbf5ffff 100644
--- a/lib/ssl/src/ssl_handshake.erl
+++ b/lib/ssl/src/ssl_handshake.erl
@@ -3726,9 +3726,9 @@ path_validate([], _, _, _, _, _, _, _, _, _, Error) ->
path_validate([{TrustedCert, Path} | Rest], ServerName, Role, CertDbHandle, CertDbRef, CRLDbHandle,
Version, SslOptions, CertExt, InvalidatedList, Error) ->
CB = path_validation_cb(Version),
- case CB:path_validation(TrustedCert, Path, ServerName,
- Role, CertDbHandle, CertDbRef, CRLDbHandle,
- Version, SslOptions, CertExt) of
+ case CB:path_validation(trusted_unwrap(TrustedCert), Path, ServerName,
+ Role, CertDbHandle, CertDbRef, CRLDbHandle,
+ Version, SslOptions, CertExt) of
{error, {bad_cert, root_cert_expired}} = NewError ->
NewInvalidatedList = [TrustedCert | InvalidatedList],
Alt = ssl_certificate:find_cross_sign_root_paths(Path, CertDbHandle, CertDbRef, NewInvalidatedList),
@@ -3745,6 +3745,13 @@ path_validate([{TrustedCert, Path} | Rest], ServerName, Role, CertDbHandle, Cert
Result
end.
+trusted_unwrap(#cert{otp = TrustedCert}) ->
+ TrustedCert;
+trusted_unwrap(#'OTPCertificate'{} = TrustedCert) ->
+ TrustedCert;
+trusted_unwrap(ErrAtom) when is_atom(ErrAtom) ->
+ ErrAtom.
+
%% Call basic path validation algorithm in public_key pre TLS-1.3
path_validation(TrustedCert, Path, ServerName, Role, CertDbHandle, CertDbRef, CRLDbHandle, Version,
#{verify_fun := VerifyFun,
diff --git a/lib/ssl/src/ssl_internal.hrl b/lib/ssl/src/ssl_internal.hrl
index 3a8568d1e3..b56ba24d78 100644
--- a/lib/ssl/src/ssl_internal.hrl
+++ b/lib/ssl/src/ssl_internal.hrl
@@ -252,7 +252,6 @@
max_size %% max early data size allowed by this ticket
}).
--record(cert, {der, otp}).
-endif. % -ifdef(ssl_internal).
--
2.31.1
