Merge branch 'ingela/public_key/ssl/CRL-error-propagation/OTP-14236' into maint

* ingela/public_key/ssl/CRL-error-propagation/OTP-14236:
  ssl: Try to make asn1 decode errors of certificates as specific as possible
  ssl,public_key: Provide details for CRL check failiures when revokation state can not be determined
  ssl: Enhance error logging

4 files changed, 57 insertions, 26 deletions

diff --git a/lib/public_key/doc/src/public_key.xml b/lib/public_key/doc/src/public_key.xml
index 35f9e86cad..942203bd12 100644
--- a/lib/public_key/doc/src/public_key.xml
+++ b/lib/public_key/doc/src/public_key.xml
@@ -634,8 +634,8 @@ fun(OtpCert :: #'OTPCertificate'{},
        <v>OTPCertificate =  #'OTPCertificate'{}</v>
        <v>DPAndCRLs  = [{DP::#'DistributionPoint'{}, {DerCRL::der_encoded(), CRL::#'CertificateList'{}}}] </v>
        <v>Options = proplists:proplist()</v>
-       <v>CRLStatus() =  valid | {bad_cert, revocation_status_undetermined} |
-       {bad_cert, {revoked, crl_reason()}}</v>
+       <v>CRLStatus() =  valid | {bad_cert, revocation_status_undetermined} |  {bad_cert, {revocation_status_undetermined,
+       {bad_crls, Details::term()}}} | {bad_cert, {revoked, crl_reason()}}</v>
      </type>
      <desc>
       <p>Performs CRL validation. It is intended to be called from
@@ -663,7 +663,7 @@ fun(OtpCert :: #'OTPCertificate'{},
 	<tag>{issuer_fun, fun()}</tag>
 	<item>
 	  <p>The fun has the following type specification:</p>
-
+	  
 	  <code>
 fun(#'DistributionPoint'{}, #'CertificateList'{},
     {rdnSequence,[#'AttributeTypeAndValue'{}]}, term()) ->
@@ -673,7 +673,15 @@ fun(#'DistributionPoint'{}, #'CertificateList'{},
 	  that has signed the CRL. 
 	  </p>
 	  <code> fun(DP, CRL, Issuer, UserState) -> {ok, RootCert, CertChain}</code>
-	</item>	
+	</item>
+
+	<tag>{undetermined_details, boolean()}</tag>
+	<item>
+	  <p>Defaults to false. When revocation status can not be
+	  determined, and this option is set to true, details of why no
+	  CRLs where accepted are included in the return value.</p>
+	</item>
+
       </taglist>
     </desc>
    </func>
diff --git a/lib/public_key/include/public_key.hrl b/lib/public_key/include/public_key.hrl
index a1e7dd31bc..663e1856ac 100644
--- a/lib/public_key/include/public_key.hrl
+++ b/lib/public_key/include/public_key.hrl
@@ -70,7 +70,8 @@
 	  reasons_mask,
 	  cert_status,
 	  interim_reasons_mask,
-	  valid_ext
+	  valid_ext,
+          details
 	 }).
 
 -record('ECPoint', {
diff --git a/lib/public_key/src/pubkey_crl.erl b/lib/public_key/src/pubkey_crl.erl
index 33bef91827..3621e9c0da 100644
--- a/lib/public_key/src/pubkey_crl.erl
+++ b/lib/public_key/src/pubkey_crl.erl
@@ -58,7 +58,8 @@ validate(OtpCert, OtherDPCRLs, DP, {DerCRL, CRL}, {DerDeltaCRL, DeltaCRL},
 init_revokation_state() ->
     #revoke_state{reasons_mask = sets:new(),
 		  interim_reasons_mask = sets:new(),
-		  cert_status = unrevoked}.
+		  cert_status = unrevoked,
+                  details = []}.
 
 fresh_crl(_, {undefined, undefined}, _) ->
     %% Typically happens when there is no delta CRL that covers a CRL
@@ -152,9 +153,10 @@ verify_crl(OtpCert, DP, CRL, DerCRL, DeltaCRL, DerDeltaCRL, OtherDPCRLs,
 					       RevokedState,
 					       CRL, DerCRL, DeltaCRL, DerDeltaCRL,
 					       IssuerFun, TrustedOtpCert, Path, OtherDPCRLs, IDP);
-		_ ->
-		    {invalid, State0#revoke_state{valid_ext = ValidExt}}
-	    end;
+	        _ ->
+                    Details = RevokedState#revoke_state.details,
+		    {invalid, RevokedState#revoke_state{valid_ext = ValidExt, details = [{{bad_crl, no_issuer_cert_chain}, CRL} | Details]}}
+            end;
 	{error, issuer_not_found} ->
 	    case Fun(DP, CRL, issuer_not_found, AdditionalArgs) of
 		{ok, TrustedOtpCert, Path} ->
@@ -163,13 +165,16 @@ verify_crl(OtpCert, DP, CRL, DerCRL, DeltaCRL, DerDeltaCRL, OtherDPCRLs,
 					       DerDeltaCRL, IssuerFun,
 					       TrustedOtpCert, Path, OtherDPCRLs, IDP);
 		_ ->
-		    {invalid, {skip, State0}}
-	    end
+                    Details = State0#revoke_state.details,
+		    {invalid, {skip, State0#revoke_state{details = [{{bad_crl, no_issuer_cert_chain}, CRL} | Details] }}}
+            end
     catch
-	throw:{bad_crl, invalid_issuer} ->
-	    {invalid, {skip, State0}};
-	throw:_ ->
-	    {invalid, State0#revoke_state{valid_ext = ValidExt}}
+	throw:{bad_crl, invalid_issuer} = Reason ->
+            Details = RevokedState#revoke_state.details,
+	    {invalid, {skip, RevokedState#revoke_state{details = [{Reason, CRL} | Details]}}};
+	throw:Reason ->
+            Details = RevokedState#revoke_state.details,
+	    {invalid, RevokedState#revoke_state{details =  [{Reason, CRL} | Details]}}
     end.
 
 verify_mask_and_signatures(Revoked, DeltaRevoked, RevokedState, CRL, DerCRL, DeltaCRL, DerDeltaCRL,
@@ -183,10 +188,12 @@ verify_mask_and_signatures(Revoked, DeltaRevoked, RevokedState, CRL, DerCRL, Del
 				     TrustedOtpCert, Path, IssuerFun, OtherDPCRLs, IDP),
 	{valid, Revoked, DeltaRevoked, RevokedState#revoke_state{reasons_mask = ReasonsMask}, IDP}
     catch
-	throw:_ ->
-	    {invalid, RevokedState};
+	throw:Reason ->
+            Details = RevokedState#revoke_state.details,
+	    {invalid, RevokedState#revoke_state{details =  [{Reason, CRL} | Details]}};
 	error:{badmatch, _} ->
-	    {invalid, RevokedState}
+            Details = RevokedState#revoke_state.details,
+	    {invalid, RevokedState#revoke_state{details = [{{bad_crl, invalid_signature}, CRL} | Details]}}
     end.
 
 
@@ -356,7 +363,7 @@ verify_scope(#'OTPCertificate'{tbsCertificate = TBSCert}, #'DistributionPoint'{c
     verify_scope(DPName, IDPName, Names, TBSCert, IDP).
 
 verify_scope(asn1_NOVALUE, _, asn1_NOVALUE, _, _) ->
-    throw({bad_crl, scope_error1});
+    throw({bad_crl, scope_error});
 verify_scope(asn1_NOVALUE, IDPName, DPIssuerNames, TBSCert, IDP) ->
     verify_dp_name(IDPName, DPIssuerNames),
     verify_dp_bools(TBSCert, IDP);
diff --git a/lib/public_key/src/public_key.erl b/lib/public_key/src/public_key.erl
index 834a75983e..1776baf830 100644
--- a/lib/public_key/src/public_key.erl
+++ b/lib/public_key/src/public_key.erl
@@ -823,8 +823,9 @@ pkix_path_validation(#'OTPCertificate'{} = TrustedCert, CertChain, Options)
 %--------------------------------------------------------------------
 -spec pkix_crls_validate(#'OTPCertificate'{},
 			 [{DP::#'DistributionPoint'{}, {DerCRL::binary(), CRL::#'CertificateList'{}}}],
-			 Options :: proplists:proplist()) -> valid | {bad_cert, revocation_status_undetermined}
-								| {bad_cert, {revoked, crl_reason()}}.
+			 Options :: proplists:proplist()) -> valid | {bad_cert, revocation_status_undetermined} |
+                                                             {bad_cert, {revocation_status_undetermined, Reason::term()}} |
+                                                             {bad_cert, {revoked, crl_reason()}}.
 
 %% Description: Performs a CRL validation according to RFC 5280.
 %%--------------------------------------------------------------------
@@ -1165,8 +1166,13 @@ der_cert(#'OTPCertificate'{} = Cert) ->
 der_cert(Der) when is_binary(Der) ->
     Der.
 
-pkix_crls_validate(_, [],_, _, _) ->
-    {bad_cert, revocation_status_undetermined};
+pkix_crls_validate(_, [],_, Options, #revoke_state{details = Details}) ->
+     case proplists:get_value(undetermined_details, Options, false) of
+         false ->
+             {bad_cert, revocation_status_undetermined};
+         true ->
+             {bad_cert, {revocation_status_undetermined, {bad_crls, format_details(Details)}}}
+     end;
 pkix_crls_validate(OtpCert, [{DP, CRL, DeltaCRL} | Rest],  All, Options, RevokedState0) ->
     CallBack = proplists:get_value(update_crl, Options, fun(_, CurrCRL) ->
 							       CurrCRL
@@ -1186,9 +1192,14 @@ pkix_crls_validate(OtpCert, [{DP, CRL, DeltaCRL} | Rest],  All, Options, Revoked
 do_pkix_crls_validate(OtpCert, [{DP, CRL, DeltaCRL} | Rest],  All, Options, RevokedState0) ->
     OtherDPCRLs = All -- [{DP, CRL, DeltaCRL}],
     case pubkey_crl:validate(OtpCert, OtherDPCRLs, DP, CRL, DeltaCRL, Options, RevokedState0) of
-	{undetermined, _, _} when Rest == []->
-	    {bad_cert, revocation_status_undetermined};
-	{undetermined, _, RevokedState} when Rest =/= []->
+	{undetermined, unrevoked, #revoke_state{details = Details}} when Rest == []->
+            case proplists:get_value(undetermined_details, Options, false) of
+                false ->
+                    {bad_cert, revocation_status_undetermined};
+                true ->
+                    {bad_cert, {revocation_status_undetermined, {bad_crls, Details}}}
+            end;
+	{undetermined, unrevoked, RevokedState} when Rest =/= []->
 	    pkix_crls_validate(OtpCert, Rest, All, Options, RevokedState);
 	{finished, unrevoked} ->
 	    valid;
@@ -1461,3 +1472,7 @@ to_lower_ascii(C) -> C.
 to_string(S) when is_list(S) -> S;
 to_string(B) when is_binary(B) -> binary_to_list(B).
 
+format_details([]) ->
+    no_relevant_crls;
+format_details(Details) ->
+    Details.