Skip to content

Commit

Permalink
Remove X509_CRL_METHOD.
Browse files Browse the repository at this point in the history 
This has no callers, and seems to be practically unusable. The only way
to set an X509_CRL_METHOD is X509_CRL_set_default_method, which is not
thread-safe and globally affects the CRL implementation across the
application.

The comment says it's to handle large CRLs, so lots of processes don't
have to store the same CRL in memory. As far as I can tell,
X509_CRL_METHOD cannot be used to help with this. It doesn't swap out
storage of the CRL, just signature verification and lookup into it. But
by the time we call into X509_CRL_METHOD, the CRL has already been
downloaded and the data stored on the X509_CRL structure. (Perhaps this
made more sense before the structure was made opaque?)

Update-Note: APIs relating to X509_CRL_METHOD are removed.
Change-Id: Ia5befa2a0e4f4416c2fb2febecad99fa31c1c6ac
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52687
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
  • Loading branch information
@davidben
davidben authored and Boringssl LUCI CQ committed May 24, 2022
1 parent 71573dc commit 1530333
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 133 deletions.
2 changes: 0 additions & 2 deletions crypto/x509/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,6 @@ struct X509_crl_st {
ASN1_INTEGER *base_crl_number;
unsigned char crl_hash[SHA256_DIGEST_LENGTH];
STACK_OF(GENERAL_NAMES) *issuers;
const X509_CRL_METHOD *meth;
void *meth_data;
} /* X509_CRL */;

struct X509_VERIFY_PARAM_st {
Expand Down
134 changes: 16 additions & 118 deletions crypto/x509/x_crl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,24 +68,6 @@
#include "../internal.h"
#include "internal.h"

/*
* Method to handle CRL access. In general a CRL could be very large (several
* Mb) and can consume large amounts of resources if stored in memory by
* multiple processes. This method allows general CRL operations to be
* redirected to more efficient callbacks: for example a CRL entry database.
*/

#define X509_CRL_METHOD_DYNAMIC 1

struct x509_crl_method_st {
int flags;
int (*crl_init) (X509_CRL *crl);
int (*crl_free) (X509_CRL *crl);
int (*crl_lookup) (X509_CRL *crl, X509_REVOKED **ret,
ASN1_INTEGER *ser, X509_NAME *issuer);
int (*crl_verify) (X509_CRL *crl, EVP_PKEY *pk);
};

static int X509_REVOKED_cmp(const X509_REVOKED **a, const X509_REVOKED **b);
static int setup_idp(X509_CRL *crl, ISSUING_DIST_POINT *idp);

Expand All @@ -95,19 +77,8 @@ ASN1_SEQUENCE(X509_REVOKED) = {
ASN1_SEQUENCE_OF_OPT(X509_REVOKED,extensions, X509_EXTENSION)
} ASN1_SEQUENCE_END(X509_REVOKED)

static int def_crl_verify(X509_CRL *crl, EVP_PKEY *r);
static int def_crl_lookup(X509_CRL *crl,
X509_REVOKED **ret, ASN1_INTEGER *serial,
X509_NAME *issuer);

static const X509_CRL_METHOD int_crl_meth = {
0,
0, 0,
def_crl_lookup,
def_crl_verify
};

static const X509_CRL_METHOD *default_crl_method = &int_crl_meth;
static int crl_lookup(X509_CRL *crl, X509_REVOKED **ret, ASN1_INTEGER *serial,
X509_NAME *issuer);

/*
* The X509_CRL_INFO structure needs a bit of customisation. Since we cache
Expand Down Expand Up @@ -237,8 +208,6 @@ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
crl->flags = 0;
crl->idp_flags = 0;
crl->idp_reasons = CRLDP_ALL_REASONS;
crl->meth = default_crl_method;
crl->meth_data = NULL;
crl->issuers = NULL;
crl->crl_number = NULL;
crl->base_crl_number = NULL;
Expand Down Expand Up @@ -332,24 +301,12 @@ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
if (!crl_set_issuers(crl))
return 0;

if (crl->meth->crl_init) {
if (crl->meth->crl_init(crl) == 0)
return 0;
}
break;
}

case ASN1_OP_FREE_POST:
/* |crl->meth| may be NULL if constructing the object failed before
* |ASN1_OP_NEW_POST| was run. */
if (crl->meth && crl->meth->crl_free) {
if (!crl->meth->crl_free(crl))
return 0;
}
if (crl->akid)
AUTHORITY_KEYID_free(crl->akid);
if (crl->idp)
ISSUING_DIST_POINT_free(crl->idp);
AUTHORITY_KEYID_free(crl->akid);
ISSUING_DIST_POINT_free(crl->idp);
ASN1_INTEGER_free(crl->crl_number);
ASN1_INTEGER_free(crl->base_crl_number);
sk_GENERAL_NAMES_pop_free(crl->issuers, GENERAL_NAMES_free);
Expand Down Expand Up @@ -431,37 +388,25 @@ int X509_CRL_add0_revoked(X509_CRL *crl, X509_REVOKED *rev)

int X509_CRL_verify(X509_CRL *crl, EVP_PKEY *pkey)
{
if (crl->meth->crl_verify)
return crl->meth->crl_verify(crl, pkey);
return 0;
if (X509_ALGOR_cmp(crl->sig_alg, crl->crl->sig_alg) != 0) {
OPENSSL_PUT_ERROR(X509, X509_R_SIGNATURE_ALGORITHM_MISMATCH);
return 0;
}

return ASN1_item_verify(ASN1_ITEM_rptr(X509_CRL_INFO),
crl->sig_alg, crl->signature, crl->crl, pkey);
}

int X509_CRL_get0_by_serial(X509_CRL *crl,
X509_REVOKED **ret, ASN1_INTEGER *serial)
{
if (crl->meth->crl_lookup)
return crl->meth->crl_lookup(crl, ret, serial, NULL);
return 0;
return crl_lookup(crl, ret, serial, NULL);
}

int X509_CRL_get0_by_cert(X509_CRL *crl, X509_REVOKED **ret, X509 *x)
{
if (crl->meth->crl_lookup)
return crl->meth->crl_lookup(crl, ret,
X509_get_serialNumber(x),
X509_get_issuer_name(x));
return 0;
}

static int def_crl_verify(X509_CRL *crl, EVP_PKEY *r)
{
if (X509_ALGOR_cmp(crl->sig_alg, crl->crl->sig_alg) != 0) {
OPENSSL_PUT_ERROR(X509, X509_R_SIGNATURE_ALGORITHM_MISMATCH);
return 0;
}

return (ASN1_item_verify(ASN1_ITEM_rptr(X509_CRL_INFO),
crl->sig_alg, crl->signature, crl->crl, r));
return crl_lookup(crl, ret, X509_get_serialNumber(x),
X509_get_issuer_name(x));
}

static int crl_revoked_issuer_match(X509_CRL *crl, X509_NAME *nm,
Expand Down Expand Up @@ -493,9 +438,8 @@ static int crl_revoked_issuer_match(X509_CRL *crl, X509_NAME *nm,

static struct CRYPTO_STATIC_MUTEX g_crl_sort_lock = CRYPTO_STATIC_MUTEX_INIT;

static int def_crl_lookup(X509_CRL *crl,
X509_REVOKED **ret, ASN1_INTEGER *serial,
X509_NAME *issuer)
static int crl_lookup(X509_CRL *crl, X509_REVOKED **ret, ASN1_INTEGER *serial,
X509_NAME *issuer)
{
X509_REVOKED rtmp, *rev;
size_t idx;
Expand Down Expand Up @@ -534,49 +478,3 @@ static int def_crl_lookup(X509_CRL *crl,
}
return 0;
}

void X509_CRL_set_default_method(const X509_CRL_METHOD *meth)
{
if (meth == NULL)
default_crl_method = &int_crl_meth;
else
default_crl_method = meth;
}

X509_CRL_METHOD *X509_CRL_METHOD_new(int (*crl_init) (X509_CRL *crl),
int (*crl_free) (X509_CRL *crl),
int (*crl_lookup) (X509_CRL *crl,
X509_REVOKED **ret,
ASN1_INTEGER *ser,
X509_NAME *issuer),
int (*crl_verify) (X509_CRL *crl,
EVP_PKEY *pk))
{
X509_CRL_METHOD *m;
m = OPENSSL_malloc(sizeof(X509_CRL_METHOD));
if (!m)
return NULL;
m->crl_init = crl_init;
m->crl_free = crl_free;
m->crl_lookup = crl_lookup;
m->crl_verify = crl_verify;
m->flags = X509_CRL_METHOD_DYNAMIC;
return m;
}

void X509_CRL_METHOD_free(X509_CRL_METHOD *m)
{
if (!(m->flags & X509_CRL_METHOD_DYNAMIC))
return;
OPENSSL_free(m);
}

void X509_CRL_set_meth_data(X509_CRL *crl, void *dat)
{
crl->meth_data = dat;
}

void *X509_CRL_get_meth_data(X509_CRL *crl)
{
return crl->meth_data;
}
1 change: 0 additions & 1 deletion include/openssl/base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,6 @@ typedef struct trust_token_issuer_st TRUST_TOKEN_ISSUER;
typedef struct trust_token_method_st TRUST_TOKEN_METHOD;
typedef struct v3_ext_ctx X509V3_CTX;
typedef struct x509_attributes_st X509_ATTRIBUTE;
typedef struct x509_crl_method_st X509_CRL_METHOD;
typedef struct x509_lookup_st X509_LOOKUP;
typedef struct x509_lookup_method_st X509_LOOKUP_METHOD;
typedef struct x509_object_st X509_OBJECT;
Expand Down
12 changes: 0 additions & 12 deletions include/openssl/x509.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -472,17 +472,6 @@ OPENSSL_EXPORT void X509_SIG_get0(const X509_SIG *sig,
OPENSSL_EXPORT void X509_SIG_getm(X509_SIG *sig, X509_ALGOR **out_alg,
ASN1_OCTET_STRING **out_digest);

OPENSSL_EXPORT void X509_CRL_set_default_method(const X509_CRL_METHOD *meth);
OPENSSL_EXPORT X509_CRL_METHOD *X509_CRL_METHOD_new(
int (*crl_init)(X509_CRL *crl), int (*crl_free)(X509_CRL *crl),
int (*crl_lookup)(X509_CRL *crl, X509_REVOKED **ret, ASN1_INTEGER *ser,
X509_NAME *issuer),
int (*crl_verify)(X509_CRL *crl, EVP_PKEY *pk));
OPENSSL_EXPORT void X509_CRL_METHOD_free(X509_CRL_METHOD *m);

OPENSSL_EXPORT void X509_CRL_set_meth_data(X509_CRL *crl, void *dat);
OPENSSL_EXPORT void *X509_CRL_get_meth_data(X509_CRL *crl);

// X509_get_X509_PUBKEY returns the public key of |x509|. Note this function is
// not const-correct for legacy reasons. Callers should not modify the returned
// object.
Expand Down Expand Up @@ -2388,7 +2377,6 @@ BORINGSSL_MAKE_DELETER(X509_ALGOR, X509_ALGOR_free)
BORINGSSL_MAKE_DELETER(X509_ATTRIBUTE, X509_ATTRIBUTE_free)
BORINGSSL_MAKE_DELETER(X509_CRL, X509_CRL_free)
BORINGSSL_MAKE_UP_REF(X509_CRL, X509_CRL_up_ref)
BORINGSSL_MAKE_DELETER(X509_CRL_METHOD, X509_CRL_METHOD_free)
BORINGSSL_MAKE_DELETER(X509_EXTENSION, X509_EXTENSION_free)
BORINGSSL_MAKE_DELETER(X509_INFO, X509_INFO_free)
BORINGSSL_MAKE_DELETER(X509_LOOKUP, X509_LOOKUP_free)
Expand Down

0 comments on commit 1530333

Please sign in to comment.