Project

General

Profile

Bug #2608

Ensure fetched CRLs are signed by certificate issuer unless cRLIssuer is specified in the CDP

Added by Luka Logar about 1 year ago. Updated about 1 year ago.

Status:
Closed
Priority:
Normal
Category:
libstrongswan
Target version:
Start date:
Due date:
Estimated time:
Affected version:
5.6.2
Resolution:
Fixed

Description

Hi,

We are using strongswan with certificate based authentication. Some of the certificates are issued by CA "A" and some by CA "B" ("A" and "B" CAs are completely distinct). We have also CRL checking enabled (CRLs for both "A" and "B" CAs are fetched from a web server) and revocation policy set to "strict". As we (by mistake) replaced the "A" CRL with "B" CRL, the certificates signed by "A" were still valid (that is, accepted by strongSwan during authentication) although there was no "A" CRL available anymore and the "A" certificates were verified against "B" CRL?

Indeed, after checking the revocation plugin, I couldn't find the check that the certificate and the CRL (the certificate is verified against) belong to the same CA?

There is a "issuer of fetched CRL '%Y' does not match CRL issuer" error (which of course is not relevant in this case), but I think a similar "issuer of fetched CRL '%Y' does not match certificate issuer" error is also needed...

As I mentioned in my previous posts, we are using some slightly patched version of strongSwan (and revocation plugin), but I think the above applies also to the original/unmodified version.

Associated revisions

Revision 21553276 (diff)
Added by Tobias Brunner about 1 year ago

revocation: Make sure issuer of fetched CRL matches that of the certificate

Unless there is a cRLIssuer listed in the CDP, the CRL should be issued
by the same issuer as the checked certificate.

Fixes #2608.

History

#1 Updated by Tobias Brunner about 1 year ago

  • Status changed from New to Feedback

There is a "issuer of fetched CRL '%Y' does not match CRL issuer" error (which of course is not relevant in this case)

Why should it not be relevant? Depending on the check you are referring to the issuer checked is actually the end-entity certificate's issuer, not the crlIssuer in the CDP. But there is the check in check_crl() (and probably check_delta_crl()) that might need some tweaking.

#2 Updated by Luka Logar about 1 year ago

Well, "issuer of fetched CRL '%Y' does not match CRL issuer" error wasn't triggered in our case. We solved our problem with this patch to function check_crl():

--- a/src/libstrongswan/plugins/revocation/revocation_validator.c
+++ b/src/libstrongswan/plugins/revocation/revocation_validator.c
@@ -692,6 +695,14 @@
             current = fetch_crl(cdp->uri);
             if (current)
             {
+                id = ((certificate_t*)subject)->get_issuer((certificate_t*)subject);
+                if (!id->equals(id, current->get_issuer(current))) {
+                    DBG1(DBG_CFG, "issuer of fetched CRL '%Y' does not match " 
+                         "certificate issuer '%Y'",
+                         current->get_issuer(current), id);
+                    current->destroy(current);
+                    continue;
+                }
                 if (cdp->issuer && !current->has_issuer(current, cdp->issuer))
                 {
                     DBG1(DBG_CFG, "issuer of fetched CRL '%Y' does not match " 

It is however possible, that the unpatched revocation plugin behaves differently?
Btw, the rest of our patch forces CRL fetching on every authentication (yes, it's quite inefficient, but this way one always has latest CRL)
I'll try to recreate this issue with stock strongSwan later in the evening...

#3 Updated by Tobias Brunner about 1 year ago

It is however possible, that the unpatched revocation plugin behaves differently?

I don't think so. As mentioned, that's one of the checks that needs tweaking. Your patch, however, isn't working as an upstream solution. It should definitely follow the cdp->issuer check and the issuer cert is actually passed to check_crl() (the subjectKeyIdentifier is already used once, could perhaps be reused), and to check it you should probably use current->has_issuer(), which properly handles subjectKeyIdentifier IDs if the CRL contains an authorityKeyIdentifier. A similar check is probably also needed in check_delta_crl().

Btw, the rest of our patch forces CRL fetching on every authentication (yes, it's quite inefficient, but this way one always has latest CRL)

You should consider using OCSP.

#4 Updated by Tobias Brunner about 1 year ago

I've pushed a fix for this to the 2608-crl-issuer branch. Let me know if that works for you.

#5 Updated by Luka Logar about 1 year ago

Yes, the CRL checking works now as it should.

Thanks.

#6 Updated by Tobias Brunner about 1 year ago

  • Tracker changed from Issue to Bug
  • Subject changed from CRL checking with multiple CAs. to Ensure fetched CRLs are signed by certificate issuer unless cRLIssuer is specified in the CDP
  • Target version set to 5.6.3

Yes, the CRL checking works now as it should.

Great, thanks for testing. I'll line this up for the next release.

#7 Updated by Tobias Brunner about 1 year ago

  • Category set to libstrongswan
  • Status changed from Feedback to Closed
  • Assignee set to Tobias Brunner
  • Resolution set to Fixed

Also available in: Atom PDF