File 4542-ssl-Refactor-to-avoid-re-encoding-of-decod... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 4542-ssl-Refactor-to-avoid-re-encoding-of-decoded-certs.patch of Package erlang (Revision fab5cf9be99874816e9cecc0e0f8d85e)

Currently displaying revision fab5cf9be99874816e9cecc0e0f8d85e , Show latest

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>
+	Macro names for key object identifiers used by prefixing with ?
+ </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>
 
 	 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.
 
 
 Available options:
@@ -473,7 +489,7 @@ fun(OtpCert :: #'OTPCertificate'{},
 	</item>
 </taglist>
 
- Possible reasons for a bad certificate: 
+ Explanations of reasons for a bad certificate: 
 <taglist>
 	<tag>cert_expired</tag>
 	<item>Certificate is no longer valid as its expiration date has passed.</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

Places

File 4542-ssl-Refactor-to-avoid-re-encoding-of-decoded-certs.patch of Package erlang (Revision fab5cf9be99874816e9cecc0e0f8d85e)

Places