Merge branch 'crl-sign'

Enforces that the certificate that signed a CRL either encodes the
cRLSign keyUsage (even if it is a CA certificate) or is a CA certificate
without a keyUsage extension (which should rarely be the case nowadays).

This is in compliance with RFC 5280, section 6.3.3. (f):

  If a key usage extension is present in the CRL issuer's certificate,
  verify that the cRLSign bit is set.

strongSwan encodes a keyUsage extension with cRLSign bit set in all CA
certificates it generates since 1ec8f22de222 ("set Certificate Sign and
CRL Sign flags in keyUsage extension if CA is true"), which was 13 years
ago.  Before that the extension was not encoded so those CA certificates
would also still be accepted as CRL issuer (if they are still valid, but
considering the SHA-1 deprecation that happened since then, they were
most likely replaced anyway).

References strongswan/strongswan#1548
This commit is contained in:
Tobias Brunner 2023-03-24 17:34:05 +01:00
commit 7dc82dea34
4 changed files with 28 additions and 16 deletions

View File

@ -303,7 +303,7 @@ METHOD(certificate_t, issued_by, bool,
return FALSE;
}
x509 = (x509_t*)issuer;
if (!(x509->get_flags(x509) & (X509_CA | X509_CRL_SIGN)))
if (!(x509->get_flags(x509) & X509_CRL_SIGN))
{
return FALSE;
}

View File

@ -687,9 +687,6 @@ static bool parse_keyUsage_ext(private_openssl_x509_t *this,
{
ASN1_BIT_STRING *usage;
/* to be compliant with RFC 4945 specific KUs have to be included */
this->flags &= ~X509_IKE_COMPLIANT;
usage = X509V3_EXT_d2i(ext);
if (usage)
{
@ -1013,11 +1010,9 @@ static bool parse_subjectKeyIdentifier_ext(private_openssl_x509_t *this,
static bool parse_extensions(private_openssl_x509_t *this)
{
const STACK_OF(X509_EXTENSION) *extensions;
bool key_usage_parsed = FALSE;
int i, num;
/* unless we see a keyUsage extension we are compliant with RFC 4945 */
this->flags |= X509_IKE_COMPLIANT;
extensions = X509_get0_extensions(this->x509);
if (extensions)
{
@ -1051,6 +1046,7 @@ static bool parse_extensions(private_openssl_x509_t *this)
break;
case NID_key_usage:
ok = parse_keyUsage_ext(this, ext);
key_usage_parsed = TRUE;
break;
case NID_ext_key_usage:
ok = parse_extKeyUsage_ext(this, ext);
@ -1084,6 +1080,16 @@ static bool parse_extensions(private_openssl_x509_t *this)
}
}
}
if (!key_usage_parsed)
{
/* we are compliant with RFC 4945 without keyUsage extension */
this->flags |= X509_IKE_COMPLIANT;
/* allow CA certificates without keyUsage extension to sign CRLs */
if (this->flags & X509_CA)
{
this->flags |= X509_CRL_SIGN;
}
}
return TRUE;
}

View File

@ -715,9 +715,6 @@ static void parse_keyUsage(chunk_t blob, private_x509_cert_t *this)
KU_DECIPHER_ONLY = 8,
};
/* to be compliant with RFC 4945 specific KUs have to be included */
this->flags &= ~X509_IKE_COMPLIANT;
if (asn1_unwrap(&blob, &blob) == ASN1_BIT_STRING && blob.len)
{
int bit, byte, unused = blob.ptr[0];
@ -1391,14 +1388,11 @@ static bool parse_certificate(private_x509_cert_t *this)
int objectID;
int extn_oid = OID_UNKNOWN;
signature_params_t sig_alg = {};
bool critical = FALSE, key_usage_parsed = FALSE;
bool success = FALSE;
bool critical = FALSE;
parser = asn1_parser_create(certObjects, this->encoding);
/* unless we see a keyUsage extension we are compliant with RFC 4945 */
this->flags |= X509_IKE_COMPLIANT;
while (parser->iterate(parser, &objectID, &object))
{
u_int level = parser->get_level(parser)+1;
@ -1513,6 +1507,7 @@ static bool parse_certificate(private_x509_cert_t *this)
break;
case OID_KEY_USAGE:
parse_keyUsage(object, this);
key_usage_parsed = TRUE;
break;
case OID_EXTENDED_KEY_USAGE:
if (!x509_parse_eku_extension(object, level, &this->flags))
@ -1610,6 +1605,17 @@ end:
{
hasher_t *hasher;
if (!key_usage_parsed)
{
/* we are compliant with RFC 4945 without keyUsage extension */
this->flags |= X509_IKE_COMPLIANT;
/* allow CA certificates without keyUsage extension to sign CRLs */
if (this->flags & X509_CA)
{
this->flags |= X509_CRL_SIGN;
}
}
/* check if the certificate is self-signed */
if (this->public.interface.interface.issued_by(
&this->public.interface.interface,

View File

@ -462,12 +462,12 @@ METHOD(certificate_t, issued_by, bool,
x509_t *x509 = (x509_t*)issuer;
chunk_t keyid = chunk_empty;
/* check if issuer is an X.509 CA certificate */
/* check if issuer is an X.509 certificate with cRLSign keyUsage bit set */
if (issuer->get_type(issuer) != CERT_X509)
{
return FALSE;
}
if (!(x509->get_flags(x509) & (X509_CA | X509_CRL_SIGN)))
if (!(x509->get_flags(x509) & X509_CRL_SIGN))
{
return FALSE;
}