Skip to content

Commit

Permalink
Modernize pkinit_get_certs_pkcs11
Browse files Browse the repository at this point in the history 
Remove unusable PKINIT_USE_MECH_LIST code since there's no build
system support and it would leak mechp.  Factor out the code to load a
cert from the card.  Avoid mixing PKCS11 and krb5 error codes.  Fix
leaks of cert and cert_id reported by Coverity.
  • Loading branch information
@frozencemetery @greghudson
frozencemetery authored and greghudson committed Jul 1, 2021
1 parent 8c88def commit cddd5c5
Showing 1 changed file with 113 additions and 117 deletions.
230 changes: 113 additions & 117 deletions src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4510,6 +4510,89 @@ reassemble_pkcs11_name(pkinit_identity_opts *idopts)
return ret;
}

static krb5_error_code
load_one_cert(CK_FUNCTION_LIST_PTR p11, CK_SESSION_HANDLE session,
pkinit_identity_opts *idopts, pkinit_cred_info *cred_out)
{
krb5_error_code ret;
CK_ATTRIBUTE attrs[2];
CK_BYTE_PTR cert = NULL, cert_id = NULL;
CK_RV pret;
const unsigned char *cp;
CK_OBJECT_HANDLE obj;
CK_ULONG count;
X509 *x = NULL;
pkinit_cred_info cred;

*cred_out = NULL;

/* Look for X.509 cert. */
pret = p11->C_FindObjects(session, &obj, 1, &count);
if (pret != CKR_OK || count <= 0)
return 0;

/* Get cert and id len. */
attrs[0].type = CKA_VALUE;
attrs[0].pValue = NULL;
attrs[0].ulValueLen = 0;
attrs[1].type = CKA_ID;
attrs[1].pValue = NULL;
attrs[1].ulValueLen = 0;
pret = p11->C_GetAttributeValue(session, obj, attrs, 2);
if (pret != CKR_OK && pret != CKR_BUFFER_TOO_SMALL) {
pkiDebug("C_GetAttributeValue: %s\n", pkcs11err(pret));
ret = KRB5KDC_ERR_PREAUTH_FAILED;
goto cleanup;
}

/* Allocate buffers and read the cert and id. */
cert = k5alloc(attrs[0].ulValueLen + 1, &ret);
if (cert == NULL)
goto cleanup;
cert_id = k5alloc(attrs[1].ulValueLen + 1, &ret);
if (cert_id == NULL)
goto cleanup;
attrs[0].type = CKA_VALUE;
attrs[0].pValue = cert;
attrs[1].type = CKA_ID;
attrs[1].pValue = cert_id;
pret = p11->C_GetAttributeValue(session, obj, attrs, 2);
if (pret != CKR_OK) {
pkiDebug("C_GetAttributeValue: %s\n", pkcs11err(pret));
ret = KRB5KDC_ERR_PREAUTH_FAILED;
goto cleanup;
}

pkiDebug("cert: size %d, id %d, idlen %d\n", (int)attrs[0].ulValueLen,
(int)cert_id[0], (int)attrs[1].ulValueLen);

cp = (unsigned char *)cert;
x = d2i_X509(NULL, &cp, (int)attrs[0].ulValueLen);
if (x == NULL) {
ret = KRB5KDC_ERR_PREAUTH_FAILED;
goto cleanup;
}

cred = k5alloc(sizeof(struct _pkinit_cred_info), &ret);
if (cred == NULL)
goto cleanup;

cred->name = reassemble_pkcs11_name(idopts);
cred->cert = x;
cred->key = NULL;
cred->cert_id = cert_id;
cred->cert_id_len = attrs[1].ulValueLen;

*cred_out = cred;
cert_id = NULL;
ret = 0;

cleanup:
free(cert);
free(cert_id);
return ret;
}

static krb5_error_code
pkinit_get_certs_pkcs11(krb5_context context,
pkinit_plg_crypto_context plg_cryptoctx,
Expand All @@ -4518,20 +4601,13 @@ pkinit_get_certs_pkcs11(krb5_context context,
pkinit_identity_crypto_context id_cryptoctx,
krb5_principal princ)
{
#ifdef PKINIT_USE_MECH_LIST
CK_MECHANISM_TYPE_PTR mechp;
CK_MECHANISM_INFO info;
#endif
CK_OBJECT_CLASS cls;
CK_OBJECT_HANDLE obj;
CK_ATTRIBUTE attrs[4];
CK_ULONG count;
CK_CERTIFICATE_TYPE certtype;
CK_BYTE_PTR cert = NULL, cert_id;
const unsigned char *cp;
int i, r;
int i;
unsigned int nattrs;
X509 *x = NULL;
krb5_error_code ret;
CK_RV pret;

/* Copy stuff from idopts -> id_cryptoctx */
if (idopts->p11_module_name != NULL) {
Expand All @@ -4552,20 +4628,20 @@ pkinit_get_certs_pkcs11(krb5_context context,
}
/* Convert the ascii cert_id string into a binary blob */
if (idopts->cert_id_string != NULL) {
r = k5_hex_decode(idopts->cert_id_string,
&id_cryptoctx->cert_id, &id_cryptoctx->cert_id_len);
if (r != 0) {
ret = k5_hex_decode(idopts->cert_id_string, &id_cryptoctx->cert_id,
&id_cryptoctx->cert_id_len);
if (ret) {
pkiDebug("Failed to convert certid string [%s]\n",
idopts->cert_id_string);
return r;
return ret;
}
}
id_cryptoctx->slotid = idopts->slotid;
id_cryptoctx->pkcs11_method = 1;

r = pkinit_open_session(context, id_cryptoctx);
if (r != 0)
return r;
ret = pkinit_open_session(context, id_cryptoctx);
if (ret)
return ret;
if (id_cryptoctx->defer_id_prompt) {
/*
* We need to reset all of the PKCS#11 state, so that the next time we
Expand All @@ -4577,56 +4653,23 @@ pkinit_get_certs_pkcs11(krb5_context context,
return 0;
}

#ifndef PKINIT_USE_MECH_LIST
/*
* We'd like to use CKM_SHA1_RSA_PKCS for signing if it's available, but
* many cards seems to be confused about whether they are capable of
* this or not. The safe thing seems to be to ignore the mechanism list,
* always use CKM_RSA_PKCS and calculate the sha1 digest ourselves.
*/

id_cryptoctx->mech = CKM_RSA_PKCS;
#else
if ((r = id_cryptoctx->p11->C_GetMechanismList(id_cryptoctx->slotid, NULL,
&count)) != CKR_OK || count <= 0) {
pkiDebug("C_GetMechanismList: %s\n", pkcs11err(r));
return KRB5KDC_ERR_PREAUTH_FAILED;
}
mechp = malloc(count * sizeof (CK_MECHANISM_TYPE));
if (mechp == NULL)
return ENOMEM;
if ((r = id_cryptoctx->p11->C_GetMechanismList(id_cryptoctx->slotid,
mechp, &count)) != CKR_OK)
return KRB5KDC_ERR_PREAUTH_FAILED;
for (i = 0; i < count; i++) {
if ((r = id_cryptoctx->p11->C_GetMechanismInfo(id_cryptoctx->slotid,
mechp[i], &info)) != CKR_OK)
return KRB5KDC_ERR_PREAUTH_FAILED;
#ifdef DEBUG_MECHINFO
pkiDebug("mech %x flags %x\n", (int) mechp[i], (int) info.flags);
if ((info.flags & (CKF_SIGN|CKF_DECRYPT)) == (CKF_SIGN|CKF_DECRYPT))
pkiDebug(" this mech is good for sign & decrypt\n");
#endif
if (mechp[i] == CKM_RSA_PKCS) {
/* This seems backwards... */
id_cryptoctx->mech =
(info.flags & CKF_SIGN) ? CKM_SHA1_RSA_PKCS : CKM_RSA_PKCS;
}
}
free(mechp);

pkiDebug("got %d mechs from card\n", (int) count);
#endif

cls = CKO_CERTIFICATE;
attrs[0].type = CKA_CLASS;
attrs[0].pValue = &cls;
attrs[0].ulValueLen = sizeof cls;
attrs[0].ulValueLen = sizeof(cls);

certtype = CKC_X_509;
attrs[1].type = CKA_CERTIFICATE_TYPE;
attrs[1].pValue = &certtype;
attrs[1].ulValueLen = sizeof certtype;
attrs[1].ulValueLen = sizeof(certtype);

nattrs = 2;

Expand All @@ -4644,80 +4687,33 @@ pkinit_get_certs_pkcs11(krb5_context context,
nattrs++;
}

r = id_cryptoctx->p11->C_FindObjectsInit(id_cryptoctx->session, attrs, nattrs);
if (r != CKR_OK) {
pkiDebug("C_FindObjectsInit: %s\n", pkcs11err(r));
pret = id_cryptoctx->p11->C_FindObjectsInit(id_cryptoctx->session, attrs,
nattrs);
if (pret != CKR_OK) {
pkiDebug("C_FindObjectsInit: %s\n", pkcs11err(pret));
return KRB5KDC_ERR_PREAUTH_FAILED;
}

for (i = 0; ; i++) {
if (i >= MAX_CREDS_ALLOWED)
return KRB5KDC_ERR_PREAUTH_FAILED;

/* Look for x.509 cert */
if ((r = id_cryptoctx->p11->C_FindObjects(id_cryptoctx->session,
&obj, 1, &count)) != CKR_OK || count <= 0) {
id_cryptoctx->creds[i] = NULL;
break;
}

/* Get cert and id len */
attrs[0].type = CKA_VALUE;
attrs[0].pValue = NULL;
attrs[0].ulValueLen = 0;

attrs[1].type = CKA_ID;
attrs[1].pValue = NULL;
attrs[1].ulValueLen = 0;

if ((r = id_cryptoctx->p11->C_GetAttributeValue(id_cryptoctx->session,
obj, attrs, 2)) != CKR_OK && r != CKR_BUFFER_TOO_SMALL) {
pkiDebug("C_GetAttributeValue: %s\n", pkcs11err(r));
return KRB5KDC_ERR_PREAUTH_FAILED;
}
cert = (CK_BYTE_PTR) malloc((size_t) attrs[0].ulValueLen + 1);
cert_id = (CK_BYTE_PTR) malloc((size_t) attrs[1].ulValueLen + 1);
if (cert == NULL || cert_id == NULL)
return ENOMEM;

/* Read the cert and id off the card */

attrs[0].type = CKA_VALUE;
attrs[0].pValue = cert;

attrs[1].type = CKA_ID;
attrs[1].pValue = cert_id;

if ((r = id_cryptoctx->p11->C_GetAttributeValue(id_cryptoctx->session,
obj, attrs, 2)) != CKR_OK) {
pkiDebug("C_GetAttributeValue: %s\n", pkcs11err(r));
return KRB5KDC_ERR_PREAUTH_FAILED;
}

pkiDebug("cert %d size %d id %d idlen %d\n", i,
(int) attrs[0].ulValueLen, (int) cert_id[0],
(int) attrs[1].ulValueLen);

cp = (unsigned char *) cert;
x = d2i_X509(NULL, &cp, (int) attrs[0].ulValueLen);
if (x == NULL)
return KRB5KDC_ERR_PREAUTH_FAILED;
id_cryptoctx->creds[i] = malloc(sizeof(struct _pkinit_cred_info));
for (i = 0; i < MAX_CREDS_ALLOWED; i++) {
ret = load_one_cert(id_cryptoctx->p11, id_cryptoctx->session, idopts,
&id_cryptoctx->creds[i]);
if (ret)
return ret;
if (id_cryptoctx->creds[i] == NULL)
return KRB5KDC_ERR_PREAUTH_FAILED;
id_cryptoctx->creds[i]->name = reassemble_pkcs11_name(idopts);
id_cryptoctx->creds[i]->cert = x;
id_cryptoctx->creds[i]->key = NULL;
id_cryptoctx->creds[i]->cert_id = cert_id;
id_cryptoctx->creds[i]->cert_id_len = attrs[1].ulValueLen;
free(cert);
break;
}
if (i == MAX_CREDS_ALLOWED)
return KRB5KDC_ERR_PREAUTH_FAILED;

id_cryptoctx->p11->C_FindObjectsFinal(id_cryptoctx->session);
if (cert == NULL)

/* Check if we found no certs. */
if (id_cryptoctx->creds[0] == NULL)
return KRB5KDC_ERR_PREAUTH_FAILED;
return 0;
}
#endif

#endif /* !WITHOUT_PKCS11 */


static void
Expand Down

0 comments on commit cddd5c5

Please sign in to comment.