ssl: verify cert signature against original cert binary

When searching for a certificate's issuer in the `CertDB`, verify the signature
against the original DER certificate from the handshake instead of a re-encoding
of the parsed certificate. This avoids false negatives due to differences
between DER encoding implementations of OTP and other platforms.

2 files changed, 75 insertions, 12 deletions

diff --git a/lib/ssl/src/ssl_certificate.erl b/lib/ssl/src/ssl_certificate.erl
index 4658e76ab1..e9dc5764a3 100644
--- a/lib/ssl/src/ssl_certificate.erl
+++ b/lib/ssl/src/ssl_certificate.erl
@@ -56,15 +56,15 @@
 %% errors. Returns {RootCert, Path, VerifyErrors}
 %%--------------------------------------------------------------------
 trusted_cert_and_path(CertChain, CertDbHandle, CertDbRef, PartialChainHandler) ->
-    Path = [Cert | _] = lists:reverse(CertChain),
-    OtpCert = public_key:pkix_decode_cert(Cert, otp),
+    Path = [BinCert | _] = lists:reverse(CertChain),
+    OtpCert = public_key:pkix_decode_cert(BinCert, otp),
     SignedAndIssuerID =
 	case public_key:pkix_is_self_signed(OtpCert) of
 	    true ->
 		{ok, IssuerId} = public_key:pkix_issuer_id(OtpCert, self),
 		{self, IssuerId};
 	    false ->
-		other_issuer(OtpCert, CertDbHandle)
+		other_issuer(OtpCert, BinCert, CertDbHandle)
 	end,
     
     case SignedAndIssuerID of
@@ -187,7 +187,7 @@ public_key_type(?'id-ecPublicKey') ->
 %%--------------------------------------------------------------------
 %%% Internal functions
 %%--------------------------------------------------------------------
-certificate_chain(OtpCert, _Cert, CertDbHandle, CertsDbRef, Chain) ->
+certificate_chain(OtpCert, BinCert, CertDbHandle, CertsDbRef, Chain) ->
     IssuerAndSelfSigned = 
 	case public_key:pkix_is_self_signed(OtpCert) of
 	    true ->
@@ -200,7 +200,7 @@ certificate_chain(OtpCert, _Cert, CertDbHandle, CertsDbRef, Chain) ->
 	{_, true = SelfSigned} ->
 	    certificate_chain(CertDbHandle, CertsDbRef, Chain, ignore, ignore, SelfSigned);
 	{{error, issuer_not_found}, SelfSigned} ->
-	    case find_issuer(OtpCert, CertDbHandle) of
+	    case find_issuer(OtpCert, BinCert, CertDbHandle) of
 		{ok, {SerialNr, Issuer}} ->
 		    certificate_chain(CertDbHandle, CertsDbRef, Chain,
 				      SerialNr, Issuer, SelfSigned);
@@ -232,12 +232,12 @@ certificate_chain(CertDbHandle, CertsDbRef, Chain, SerialNr, Issuer, _SelfSigned
 	    {ok, undefined, lists:reverse(Chain)}		      
     end.
 
-find_issuer(OtpCert, CertDbHandle) ->
+find_issuer(OtpCert, BinCert, CertDbHandle) ->
     IsIssuerFun =
 	fun({_Key, {_Der, #'OTPCertificate'{} = ErlCertCandidate}}, Acc) ->
 		case public_key:pkix_is_issuer(OtpCert, ErlCertCandidate) of
 		    true ->
-			case verify_cert_signer(OtpCert, ErlCertCandidate#'OTPCertificate'.tbsCertificate) of
+			case verify_cert_signer(BinCert, ErlCertCandidate#'OTPCertificate'.tbsCertificate) of
 			    true ->
 				throw(public_key:pkix_issuer_id(ErlCertCandidate, self));
 			    false ->
@@ -265,9 +265,9 @@ is_valid_extkey_usage(KeyUse, server) ->
     %% Server wants to verify client
     is_valid_key_usage(KeyUse, ?'id-kp-clientAuth').
 
-verify_cert_signer(OtpCert, SignerTBSCert) -> 
+verify_cert_signer(BinCert, SignerTBSCert) ->
     PublicKey = public_key(SignerTBSCert#'OTPTBSCertificate'.subjectPublicKeyInfo),
-    public_key:pkix_verify(public_key:pkix_encode('OTPCertificate', OtpCert, otp),  PublicKey).
+    public_key:pkix_verify(BinCert, PublicKey).
 
 public_key(#'OTPSubjectPublicKeyInfo'{algorithm = #'PublicKeyAlgorithm'{algorithm = ?'id-ecPublicKey',
 									parameters = Params},
@@ -281,12 +281,12 @@ public_key(#'OTPSubjectPublicKeyInfo'{algorithm = #'PublicKeyAlgorithm'{algorith
 				      subjectPublicKey = Key}) ->
     {Key, Params}.
 
-other_issuer(OtpCert, CertDbHandle) ->
+other_issuer(OtpCert, BinCert, CertDbHandle) ->
     case public_key:pkix_issuer_id(OtpCert, other) of
 	{ok, IssuerId} ->
 	    {other, IssuerId};
 	{error, issuer_not_found} ->
-	    case find_issuer(OtpCert, CertDbHandle) of
+	    case find_issuer(OtpCert, BinCert, CertDbHandle) of
 		{ok, IssuerId} ->
 		    {other, IssuerId};
 		Other ->
diff --git a/lib/ssl/test/ssl_certificate_verify_SUITE.erl b/lib/ssl/test/ssl_certificate_verify_SUITE.erl
index 5940a86a7f..968ef30791 100644
--- a/lib/ssl/test/ssl_certificate_verify_SUITE.erl
+++ b/lib/ssl/test/ssl_certificate_verify_SUITE.erl
@@ -75,7 +75,8 @@ error_handling_tests()->
      unknown_server_ca_accept_verify_none,
      unknown_server_ca_accept_verify_peer,
      unknown_server_ca_accept_backwardscompatibility,
-     no_authority_key_identifier].
+     no_authority_key_identifier,
+     no_authority_key_identifier_and_nonstandard_encoding].
 
 init_per_suite(Config0) ->
     catch crypto:stop(),
@@ -850,6 +851,68 @@ delete_authority_key_extension([Head | Rest], Acc) ->
 
 %%--------------------------------------------------------------------
 
+no_authority_key_identifier_and_nonstandard_encoding() ->
+    [{doc, "Test cert with nonstandard encoding that does not have"
+      " authorityKeyIdentifier extension but are present in trusted certs db."}].
+
+no_authority_key_identifier_and_nonstandard_encoding(Config) when is_list(Config) ->
+    ClientOpts = ?config(client_verification_opts, Config),
+    ServerOpts = ?config(server_verification_opts, Config),
+    PrivDir = ?config(priv_dir, Config),
+
+    KeyFile = filename:join(PrivDir, "otpCA/private/key.pem"),
+    [KeyEntry] = ssl_test_lib:pem_to_der(KeyFile),
+    Key = ssl_test_lib:public_key(public_key:pem_entry_decode(KeyEntry)),
+
+    CertFile = proplists:get_value(certfile, ServerOpts),
+    NewCertFile = filename:join(PrivDir, "server/new_cert.pem"),
+    [{'Certificate', DerCert, _}] = ssl_test_lib:pem_to_der(CertFile),
+    ServerCert = public_key:pkix_decode_cert(DerCert, plain),
+    ServerTbsCert = ServerCert#'Certificate'.tbsCertificate,
+    Extensions0 =  ServerTbsCert#'TBSCertificate'.extensions,
+    %% need to remove authorityKeyIdentifier extension to cause DB lookup by signature
+    Extensions = delete_authority_key_extension(Extensions0, []),
+    NewExtensions = replace_key_usage_extension(Extensions, []),
+    NewServerTbsCert = ServerTbsCert#'TBSCertificate'{extensions = NewExtensions},
+
+    ct:log("Extensions ~p~n, NewExtensions: ~p~n", [Extensions, NewExtensions]),
+
+    TbsDer = public_key:pkix_encode('TBSCertificate', NewServerTbsCert, plain),
+    Sig = public_key:sign(TbsDer, md5, Key),
+    NewServerCert = ServerCert#'Certificate'{tbsCertificate = NewServerTbsCert, signature = Sig},
+    NewDerCert = public_key:pkix_encode('Certificate', NewServerCert, plain),
+    ssl_test_lib:der_to_pem(NewCertFile, [{'Certificate', NewDerCert, not_encrypted}]),
+    NewServerOpts = [{certfile, NewCertFile} | proplists:delete(certfile, ServerOpts)],
+
+    {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
+
+    Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
+					{from, self()},
+					{mfa, {ssl_test_lib,
+					       send_recv_result_active, []}},
+					{options, [{active, true} | NewServerOpts]}]),
+    Port = ssl_test_lib:inet_port(Server),
+    Client = ssl_test_lib:start_client([{node, ClientNode}, {port, Port},
+					{host, Hostname},
+					{from, self()},
+					{mfa, {ssl_test_lib,
+					       send_recv_result_active, []}},
+					{options, [{verify, verify_peer} | ClientOpts]}]),
+    ssl_test_lib:check_result(Server, ok, Client, ok),
+    ssl_test_lib:close(Server),
+    ssl_test_lib:close(Client).
+
+replace_key_usage_extension([], Acc) ->
+    lists:reverse(Acc);
+replace_key_usage_extension([#'Extension'{extnID = ?'id-ce-keyUsage'} = E | Rest], Acc) ->
+    %% A nonstandard DER encoding of [digitalSignature, keyEncipherment]
+    Val = <<3, 2, 0, 16#A0>>,
+    replace_key_usage_extension(Rest, [E#'Extension'{extnValue = Val} | Acc]);
+replace_key_usage_extension([Head | Rest], Acc) ->
+    replace_key_usage_extension(Rest, [Head | Acc]).
+
+%%--------------------------------------------------------------------
+
 invalid_signature_server() ->
     [{doc,"Test client with invalid signature"}].