Be suspicious of certificates without CRL DPs

Previously, if certificate revocation checking was turned on, and a
certificate didn't contain a CRL Distribution Points extension, and
there was no relevant CRL in the cache, then ssl_handshake:crl_check
would accept the certificate even if the crl_check option was set to
reject certificates for which the revocation status could not be
determined.  With this change, such certificates will only be accepted
if the crl_check option was set to best_effort.

The process for CRL validation is described in section 6.3 of RFC
5280.  The text doesn't mention any special treatment to be given to
certificates without distribution points: it just says "For each
distribution point..." (section 6.3.3), which would leave the
revocation status undetermined, unless there were "any available CRLs
not specified in a distribution point but issued by the certificate
issuer".  Thus the result of this algorithm should be UNDETERMINED in
this case, not UNREVOKED, and the crl_check option should govern how
the implementation reacts to this result.

2 files changed, 52 insertions, 7 deletions

diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl
index e9e140836b..e98073080a 100644
--- a/lib/ssl/src/ssl_handshake.erl
+++ b/lib/ssl/src/ssl_handshake.erl
@@ -2072,12 +2072,9 @@ crl_check(OtpCert, Check, CertDbHandle, CertDbRef, {Callback, CRLDbHandle}, _) -
 	      ],
     case dps_and_crls(OtpCert, Callback, CRLDbHandle, ext) of
 	no_dps ->
-	    case dps_and_crls(OtpCert, Callback, CRLDbHandle, same_issuer) of
-		[] ->
-		    valid; %% No relevant CRL existed
-		DpsAndCRls ->
-		    crl_check_same_issuer(OtpCert, Check, DpsAndCRls, Options)		
-	    end;
+	    crl_check_same_issuer(OtpCert, Check,
+				  dps_and_crls(OtpCert, Callback, CRLDbHandle, same_issuer),
+				  Options);
 	DpsAndCRLs ->  %% This DP list may be empty if relevant CRLs existed 
 	    %% but could not be retrived, will result in {bad_cert, revocation_status_undetermined}
 	    case public_key:pkix_crls_validate(OtpCert, DpsAndCRLs, Options) of
diff --git a/lib/ssl/test/ssl_crl_SUITE.erl b/lib/ssl/test/ssl_crl_SUITE.erl
index 68ed36caa3..5b86027210 100644
--- a/lib/ssl/test/ssl_crl_SUITE.erl
+++ b/lib/ssl/test/ssl_crl_SUITE.erl
@@ -53,7 +53,7 @@ groups() ->
      {idp_crl, [], basic_tests()}].
 
 basic_tests() ->
-    [crl_verify_valid, crl_verify_revoked].
+    [crl_verify_valid, crl_verify_revoked, crl_verify_no_crl].
 
 
 init_per_suite(Config) ->
@@ -204,6 +204,52 @@ crl_verify_revoked(Config)  when is_list(Config) ->
     crl_verify_error(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts,
                      "certificate revoked").
 
+crl_verify_no_crl() ->
+    [{doc,"Verify a simple CRL chain when the CRL is missing"}].
+crl_verify_no_crl(Config) when is_list(Config) ->
+    PrivDir = ?config(cert_dir, Config),
+    Check = ?config(crl_check, Config),
+    ServerOpts =  [{keyfile, filename:join([PrivDir, "server", "key.pem"])},
+      		  {certfile, filename:join([PrivDir, "server", "cert.pem"])},
+		   {cacertfile, filename:join([PrivDir, "server", "cacerts.pem"])}],
+    ClientOpts =  case ?config(idp_crl, Config) of 
+		      true ->	       
+			  [{cacertfile, filename:join([PrivDir, "server", "cacerts.pem"])},
+			   {crl_check, Check},
+			   {crl_cache, {ssl_crl_cache, {internal, [{http, 5000}]}}},
+			   {verify, verify_peer}];
+		      false ->
+			  [{cacertfile, filename:join([PrivDir, "server", "cacerts.pem"])},
+			   {crl_check, Check},
+			   {verify, verify_peer}]
+		  end,			  
+    {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
+
+    %% In case we're running an HTTP server that serves CRLs, let's
+    %% rename those files, so the CRL is absent when we try to verify
+    %% it.
+    %%
+    %% If we're not using an HTTP server, we just need to refrain from
+    %% adding the CRLs to the cache manually.
+    rename_crl(filename:join([PrivDir, "erlangCA", "crl.pem"])),
+    rename_crl(filename:join([PrivDir, "otpCA", "crl.pem"])),
+
+    %% The expected outcome when the CRL is missing depends on the
+    %% crl_check setting.
+    case Check of
+        true ->
+            %% The error "revocation status undetermined" gets turned
+            %% into "bad certificate".
+            crl_verify_error(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts,
+                             "bad certificate");
+        peer ->
+            crl_verify_error(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts,
+                             "bad certificate");
+        best_effort ->
+            %% In "best effort" mode, we consider the certificate not
+            %% to be revoked if we can't find the appropriate CRL.
+            crl_verify_valid(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts)
+    end.
 
 crl_verify_valid(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts) ->
     Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0}, 
@@ -263,3 +309,5 @@ make_dir_path(PathComponents) ->
 		"",
 		PathComponents).
 
+rename_crl(Filename) ->
+    file:rename(Filename, Filename ++ ".notfound").