Skip to content

Commit

Permalink
Simplify PKINIT cert representation
Browse files Browse the repository at this point in the history 
In the _pkinit_identity_crypto_context structure, the my_certs field
is a stack which only ever contains one cert and is only ever used to
retrieve that one cert.  The cert_index field is always 0.  Replace
these fields with a my_cert field pointing directly to the X509
certificate.

Simplify crypto_cert_select_default() by making it call
crypto_cert_select() with index 0 after verifying the certificate
count.
  • Loading branch information
@greghudson
greghudson committed Mar 19, 2024
1 parent 9240a5b commit f95dfb7
Showing 1 changed file with 20 additions and 54 deletions.
74 changes: 20 additions & 54 deletions src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,9 @@ typedef struct _pkinit_cred_info *pkinit_cred_info;

struct _pkinit_identity_crypto_context {
pkinit_cred_info creds[MAX_CREDS_ALLOWED+1];
STACK_OF(X509) *my_certs; /* available user certs */
X509 *my_cert; /* selected user or KDC cert */
char *identity; /* identity name for user cert */
int cert_index; /* cert to use out of available certs*/
EVP_PKEY *my_key; /* available user keys if in filesystem */
EVP_PKEY *my_key; /* selected cert key if in filesystem */
STACK_OF(X509) *trustedCAs; /* available trusted ca certs */
STACK_OF(X509) *intermediateCAs; /* available intermediate ca certs */
STACK_OF(X509_CRL) *revoked; /* available crls */
Expand Down Expand Up @@ -1489,8 +1488,7 @@ pkinit_init_certs(pkinit_identity_crypto_context ctx)

for (i = 0; i < MAX_CREDS_ALLOWED; i++)
ctx->creds[i] = NULL;
ctx->my_certs = NULL;
ctx->cert_index = 0;
ctx->my_cert = NULL;
ctx->my_key = NULL;
ctx->trustedCAs = NULL;
ctx->intermediateCAs = NULL;
Expand All @@ -1506,8 +1504,8 @@ pkinit_fini_certs(pkinit_identity_crypto_context ctx)
if (ctx == NULL)
return;

if (ctx->my_certs != NULL)
sk_X509_pop_free(ctx->my_certs, X509_free);
if (ctx->my_cert != NULL)
X509_free(ctx->my_cert);

if (ctx->my_key != NULL)
EVP_PKEY_free(ctx->my_key);
Expand Down Expand Up @@ -1696,7 +1694,6 @@ cms_signeddata_create(krb5_context context,
ASN1_OCTET_STRING *digest = NULL;
unsigned int alg_len = 0, digest_len = 0;
unsigned char *y = NULL;
X509 *cert = NULL;
ASN1_OBJECT *oid = NULL, *oid_copy;

/* Start creating PKCS7 data. */
Expand All @@ -1715,7 +1712,7 @@ cms_signeddata_create(krb5_context context,
if (oid == NULL)
goto cleanup;

if (id_cryptoctx->my_certs != NULL) {
if (id_cryptoctx->my_cert != NULL) {
X509_STORE *certstore = NULL;
X509_STORE_CTX *certctx;
STACK_OF(X509) *certstack = NULL;
Expand All @@ -1726,8 +1723,6 @@ cms_signeddata_create(krb5_context context,
if ((cert_stack = sk_X509_new_null()) == NULL)
goto cleanup;

cert = sk_X509_value(id_cryptoctx->my_certs, id_cryptoctx->cert_index);

certstore = X509_STORE_new();
if (certstore == NULL)
goto cleanup;
Expand All @@ -1736,7 +1731,7 @@ cms_signeddata_create(krb5_context context,
certctx = X509_STORE_CTX_new();
if (certctx == NULL)
goto cleanup;
X509_STORE_CTX_init(certctx, certstore, cert,
X509_STORE_CTX_init(certctx, certstore, id_cryptoctx->my_cert,
id_cryptoctx->intermediateCAs);
X509_STORE_CTX_trusted_stack(certctx, id_cryptoctx->trustedCAs);
if (!X509_verify_cert(certctx)) {
Expand Down Expand Up @@ -1764,13 +1759,13 @@ cms_signeddata_create(krb5_context context,
if (!ASN1_INTEGER_set(p7si->version, 1))
goto cleanup;
if (!X509_NAME_set(&p7si->issuer_and_serial->issuer,
X509_get_issuer_name(cert)))
X509_get_issuer_name(id_cryptoctx->my_cert)))
goto cleanup;
/* because ASN1_INTEGER_set is used to set a 'long' we will do
* things the ugly way. */
ASN1_INTEGER_free(p7si->issuer_and_serial->serial);
if (!(p7si->issuer_and_serial->serial =
ASN1_INTEGER_dup(X509_get_serialNumber(cert))))
ASN1_INTEGER_dup(X509_get_serialNumber(id_cryptoctx->my_cert))))
goto cleanup;

/* will not fill-out EVP_PKEY because it's on the smartcard */
Expand Down Expand Up @@ -3311,7 +3306,7 @@ pkinit_check_kdc_pkid(krb5_context context,
PKCS7_ISSUER_AND_SERIAL *is = NULL;
const unsigned char *p = pdid_buf;
int status = 1;
X509 *kdc_cert = sk_X509_value(id_cryptoctx->my_certs, id_cryptoctx->cert_index);
X509 *kdc_cert = id_cryptoctx->my_cert;

*valid_kdcPkId = 0;
pkiDebug("found kdcPkId in AS REQ\n");
Expand Down Expand Up @@ -4783,7 +4778,8 @@ crypto_cert_get_matching_data(krb5_context context,
}

/*
* Set the certificate in idctx->creds[cred_index] as the selected certificate.
* Set the certificate in idctx->creds[cred_index] as the selected certificate,
* stealing pointers from it.
*/
krb5_error_code
crypto_cert_select(krb5_context context, pkinit_identity_crypto_context idctx,
Expand All @@ -4795,20 +4791,17 @@ crypto_cert_select(krb5_context context, pkinit_identity_crypto_context idctx,
return ENOENT;

ci = idctx->creds[cred_index];
/* copy the selected cert into our id_cryptoctx */
if (idctx->my_certs != NULL)
sk_X509_pop_free(idctx->my_certs, X509_free);
idctx->my_certs = sk_X509_new_null();
sk_X509_push(idctx->my_certs, ci->cert);
free(idctx->identity);

idctx->my_cert = ci->cert;
ci->cert = NULL;

/* hang on to the selected credential name */
free(idctx->identity);
if (ci->name != NULL)
idctx->identity = strdup(ci->name);
else
idctx->identity = NULL;

ci->cert = NULL; /* Don't free it twice */
idctx->cert_index = 0;
if (idctx->pkcs11_method != 1) {
idctx->my_key = ci->key;
ci->key = NULL; /* Don't free it twice */
Expand Down Expand Up @@ -4837,41 +4830,14 @@ crypto_cert_select_default(krb5_context context,

retval = crypto_cert_get_count(id_cryptoctx, &cert_count);
if (retval)
goto errout;
return retval;

if (cert_count != 1) {
TRACE_PKINIT_NO_DEFAULT_CERT(context, cert_count);
retval = EINVAL;
goto errout;
}
/* copy the selected cert into our id_cryptoctx */
if (id_cryptoctx->my_certs != NULL) {
sk_X509_pop_free(id_cryptoctx->my_certs, X509_free);
return EINVAL;
}
id_cryptoctx->my_certs = sk_X509_new_null();
sk_X509_push(id_cryptoctx->my_certs, id_cryptoctx->creds[0]->cert);
id_cryptoctx->creds[0]->cert = NULL; /* Don't free it twice */
id_cryptoctx->cert_index = 0;
/* hang on to the selected credential name */
if (id_cryptoctx->creds[0]->name != NULL)
id_cryptoctx->identity = strdup(id_cryptoctx->creds[0]->name);
else
id_cryptoctx->identity = NULL;

if (id_cryptoctx->pkcs11_method != 1) {
id_cryptoctx->my_key = id_cryptoctx->creds[0]->key;
id_cryptoctx->creds[0]->key = NULL; /* Don't free it twice */
}
#ifndef WITHOUT_PKCS11
else {
id_cryptoctx->cert_id = id_cryptoctx->creds[0]->cert_id;
id_cryptoctx->creds[0]->cert_id = NULL; /* Don't free it twice */
id_cryptoctx->cert_id_len = id_cryptoctx->creds[0]->cert_id_len;
}
#endif
retval = 0;
errout:
return retval;
return crypto_cert_select(context, id_cryptoctx, 0);
}


Expand Down

0 comments on commit f95dfb7

Please sign in to comment.