From 6537be9c8d56ba3f3855822c2f1b833f01a1bd25 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 20 May 2021 16:52:49 +0200 Subject: [PATCH] pkcs11: Change how unavailable attributes like CKA_TRUSTED are handled If a PKCS#11 library/token doesn't provide one or more attributes via C_GetAttributeValue(), we get back CKR_ATTRIBUTE_TYPE_INVALID (similar for protected attributes where CKR_ATTRIBUTE_SENSITIVE is returned). This is not an error as the spec demands that all attributes have been processed with the unavailable attributes having set their length field to CK_UNAVAILABLE_INFORMATION. We use this to handle the CKA_TRUSTED attribute, which some tokens apparently don't support. We previously used a version check to remove the attribute from the call but even the latest spec doesn't make the attribute mandatory (it's just in a list of "common" attributes for CKO_CERTIFICATE objects, without a default value), so there are current tokens that don't support it and prevent us from enumerating certificates. --- .../plugins/pkcs11/pkcs11_creds.c | 25 +++++----- .../plugins/pkcs11/pkcs11_library.c | 48 +++++++++++++++---- .../plugins/pkcs11/pkcs11_library.h | 8 ++-- .../plugins/pkcs11/pkcs11_manager.c | 1 - .../plugins/pkcs11/pkcs11_private_key.c | 29 +++++++---- .../plugins/pkcs11/pkcs11_public_key.c | 3 +- 6 files changed, 78 insertions(+), 36 deletions(-) diff --git a/src/libstrongswan/plugins/pkcs11/pkcs11_creds.c b/src/libstrongswan/plugins/pkcs11/pkcs11_creds.c index 76d69d5cb4..74d699a2ed 100644 --- a/src/libstrongswan/plugins/pkcs11/pkcs11_creds.c +++ b/src/libstrongswan/plugins/pkcs11/pkcs11_creds.c @@ -83,21 +83,21 @@ static void find_certificates(private_pkcs11_creds_t *this, /* store result in a temporary list, avoid recursive operation */ raw = linked_list_create(); - /* do not use trusted argument if not supported */ - if (!(this->lib->get_features(this->lib) & PKCS11_TRUSTED_CERTS)) - { - count--; - } enumerator = this->lib->create_object_enumerator(this->lib, session, tmpl, countof(tmpl), attr, count); while (enumerator->enumerate(enumerator, &object)) { - entry = malloc(sizeof(*entry)); - entry->value = chunk_clone( - chunk_create(attr[0].pValue, attr[0].ulValueLen)); - entry->label = chunk_clone( - chunk_create(attr[1].pValue, attr[1].ulValueLen)); - entry->trusted = trusted; + if (attr[0].ulValueLen == CK_UNAVAILABLE_INFORMATION || + attr[1].ulValueLen == CK_UNAVAILABLE_INFORMATION) + { + continue; + } + INIT(entry, + .value = chunk_clone(chunk_create(attr[0].pValue, attr[0].ulValueLen)), + .label = chunk_clone(chunk_create(attr[1].pValue, attr[1].ulValueLen)), + /* assume trusted certificates if attribute is not available */ + .trusted = attr[2].ulValueLen == CK_UNAVAILABLE_INFORMATION || trusted, + ); raw->insert_last(raw, entry); } enumerator->destroy(enumerator); @@ -339,7 +339,8 @@ certificate_t *pkcs11_creds_load(certificate_type_t type, va_list args) } certs = p11->create_object_enumerator(p11, session, tmpl, countof(tmpl), attr, countof(attr)); - if (certs->enumerate(certs, &object)) + if (certs->enumerate(certs, &object) && + attr[0].ulValueLen != CK_UNAVAILABLE_INFORMATION) { data = chunk_clone(chunk_create(attr[0].pValue, attr[0].ulValueLen)); } diff --git a/src/libstrongswan/plugins/pkcs11/pkcs11_library.c b/src/libstrongswan/plugins/pkcs11/pkcs11_library.c index 6c0dda3fa1..ae0a843482 100644 --- a/src/libstrongswan/plugins/pkcs11/pkcs11_library.c +++ b/src/libstrongswan/plugins/pkcs11/pkcs11_library.c @@ -624,6 +624,8 @@ typedef struct { pkcs11_library_t *lib; /* attributes to retrieve */ CK_ATTRIBUTE_PTR attr; + /* copy of the original attributes provided by the caller */ + CK_ATTRIBUTE_PTR orig_attr; /* number of attributes */ CK_ULONG count; /* object handle in case of a single object */ @@ -633,17 +635,36 @@ typedef struct { } object_enumerator_t; /** - * Free contents of attributes in a list + * Keep a copy of the original attribute values so we can restore them while + * enumerating e.g. if an attribute was unavailable for a particular object. + */ +static void init_attrs(object_enumerator_t *this) +{ + int i; + + this->orig_attr = calloc(this->count, sizeof(CK_ATTRIBUTE)); + for (i = 0; i < this->count; i++) + { + this->orig_attr[i] = this->attr[i]; + } +} + +/** + * Free contents of allocated attributes and reset them to their original + * values. */ static void free_attrs(object_enumerator_t *this) { CK_ATTRIBUTE_PTR attr; + int i; while (this->freelist->remove_last(this->freelist, (void**)&attr) == SUCCESS) { free(attr->pValue); - attr->pValue = NULL; - attr->ulValueLen = 0; + } + for (i = 0; i < this->count; i++) + { + this->attr[i] = this->orig_attr[i]; } } @@ -687,7 +708,9 @@ static bool get_attributes(object_enumerator_t *this, CK_OBJECT_HANDLE object) /* get length of objects first */ rv = this->lib->f->C_GetAttributeValue(this->session, object, this->attr, this->count); - if (rv != CKR_OK) + if (rv != CKR_OK && + rv != CKR_ATTRIBUTE_SENSITIVE && + rv != CKR_ATTRIBUTE_TYPE_INVALID) { DBG1(DBG_CFG, "C_GetAttributeValue(NULL) error: %N", ck_rv_names, rv); return FALSE; @@ -695,8 +718,12 @@ static bool get_attributes(object_enumerator_t *this, CK_OBJECT_HANDLE object) /* allocate required chunks */ for (i = 0; i < this->count; i++) { - if (this->attr[i].pValue == NULL && - this->attr[i].ulValueLen != 0 && this->attr[i].ulValueLen != -1) + if (this->attr[i].ulValueLen == CK_UNAVAILABLE_INFORMATION) + { /* reset this unavailable attribute before the next call */ + this->attr[i] = this->orig_attr[i]; + } + else if (this->attr[i].pValue == NULL && + this->attr[i].ulValueLen != 0) { this->attr[i].pValue = malloc(this->attr[i].ulValueLen); this->freelist->insert_last(this->freelist, &this->attr[i]); @@ -705,9 +732,10 @@ static bool get_attributes(object_enumerator_t *this, CK_OBJECT_HANDLE object) /* get the data */ rv = this->lib->f->C_GetAttributeValue(this->session, object, this->attr, this->count); - if (rv != CKR_OK) + if (rv != CKR_OK && + rv != CKR_ATTRIBUTE_SENSITIVE && + rv != CKR_ATTRIBUTE_TYPE_INVALID) { - free_attrs(this); DBG1(DBG_CFG, "C_GetAttributeValue() error: %N", ck_rv_names, rv); return FALSE; } @@ -774,6 +802,7 @@ METHOD(enumerator_t, object_destroy, void, } free_attrs(this); this->freelist->destroy(this->freelist); + free(this->orig_attr); free(this); } @@ -804,6 +833,7 @@ METHOD(pkcs11_library_t, create_object_enumerator, enumerator_t*, .count = acount, .freelist = linked_list_create(), ); + init_attrs(enumerator); return &enumerator->public; } @@ -826,6 +856,7 @@ METHOD(pkcs11_library_t, create_object_attr_enumerator, enumerator_t*, .object = object, .freelist = linked_list_create(), ); + init_attrs(enumerator); return &enumerator->public; } @@ -1033,7 +1064,6 @@ static void check_features(private_pkcs11_library_t *this, CK_INFO *info) { if (has_version(info, 2, 20)) { - this->features |= PKCS11_TRUSTED_CERTS; this->features |= PKCS11_ALWAYS_AUTH_KEYS; } } diff --git a/src/libstrongswan/plugins/pkcs11/pkcs11_library.h b/src/libstrongswan/plugins/pkcs11/pkcs11_library.h index 4038b7e8f6..f8aa0afad7 100644 --- a/src/libstrongswan/plugins/pkcs11/pkcs11_library.h +++ b/src/libstrongswan/plugins/pkcs11/pkcs11_library.h @@ -37,10 +37,8 @@ typedef struct pkcs11_library_t pkcs11_library_t; * Optional PKCS#11 features some libraries support, some not */ enum pkcs11_feature_t { - /** CKA_TRUSTED attribute supported for certificate objects */ - PKCS11_TRUSTED_CERTS = (1<<0), /** CKA_ALWAYS_AUTHENTICATE attribute supported for private keys */ - PKCS11_ALWAYS_AUTH_KEYS = (1<<1), + PKCS11_ALWAYS_AUTH_KEYS = (1<<0), }; /** @@ -73,6 +71,8 @@ struct pkcs11_library_t { * An optional attribute array is automatically filled in with the * objects associated attributes. If the value of an output attribute * is NULL, the value gets allocated/freed during enumeration. + * For attributes that are unavailable, the length field will be set to + * CK_UNAVAILABLE_INFORMATION. * * @param session session to use * @param tmpl search template @@ -92,6 +92,8 @@ struct pkcs11_library_t { * The given attribute array is automatically filled in with the * associated attributes. If the value of an output attribute is NULL, * the required memory gets allocated/freed during enumeration. + * For attributes that are unavailable, the length field will be set to + * CK_UNAVAILABLE_INFORMATION. * * @param session session to use * @param object object handle diff --git a/src/libstrongswan/plugins/pkcs11/pkcs11_manager.c b/src/libstrongswan/plugins/pkcs11/pkcs11_manager.c index c7dfe69d7c..a1400ad3b1 100644 --- a/src/libstrongswan/plugins/pkcs11/pkcs11_manager.c +++ b/src/libstrongswan/plugins/pkcs11/pkcs11_manager.c @@ -389,4 +389,3 @@ pkcs11_manager_t *pkcs11_manager_create(pkcs11_manager_token_event_t cb, return &this->public; } - diff --git a/src/libstrongswan/plugins/pkcs11/pkcs11_private_key.c b/src/libstrongswan/plugins/pkcs11/pkcs11_private_key.c index 6b8be62658..820ac3f315 100644 --- a/src/libstrongswan/plugins/pkcs11/pkcs11_private_key.c +++ b/src/libstrongswan/plugins/pkcs11/pkcs11_private_key.c @@ -627,13 +627,17 @@ static pkcs11_library_t* find_lib_and_keyid_by_skid(chunk_t keyid_chunk, attr, countof(attr)); while (certs->enumerate(certs, &object)) { - INIT(entry, - .value = chunk_clone( - chunk_create(attr[0].pValue, attr[0].ulValueLen)), - .ckaid = chunk_clone( - chunk_create(attr[1].pValue, attr[1].ulValueLen)), - ); - raw->insert_last(raw, entry); + if (attr[0].ulValueLen != CK_UNAVAILABLE_INFORMATION && + attr[1].ulValueLen != CK_UNAVAILABLE_INFORMATION) + { + INIT(entry, + .value = chunk_clone( + chunk_create(attr[0].pValue, attr[0].ulValueLen)), + .ckaid = chunk_clone( + chunk_create(attr[1].pValue, attr[1].ulValueLen)), + ); + raw->insert_last(raw, entry); + } } certs->destroy(certs); @@ -708,7 +712,8 @@ static bool find_key(private_pkcs11_private_key_t *this, chunk_t keyid) } enumerator = this->lib->create_object_enumerator(this->lib, this->session, tmpl, countof(tmpl), attr, count); - if (enumerator->enumerate(enumerator, &object)) + if (enumerator->enumerate(enumerator, &object) && + attr[0].ulValueLen != CK_UNAVAILABLE_INFORMATION) { this->type = KEY_RSA; switch (type) @@ -717,7 +722,10 @@ static bool find_key(private_pkcs11_private_key_t *this, chunk_t keyid) this->type = KEY_ECDSA; /* fall-through */ case CKK_RSA: - this->reauth = reauth; + if (attr[1].ulValueLen != CK_UNAVAILABLE_INFORMATION) + { + this->reauth = reauth; + } this->object = object; found = TRUE; break; @@ -803,7 +811,8 @@ static public_key_t* find_pubkey_in_certs(private_pkcs11_private_key_t *this, enumerator = this->lib->create_object_enumerator(this->lib, this->session, tmpl, countof(tmpl), attr, countof(attr)); - if (enumerator->enumerate(enumerator, &object)) + if (enumerator->enumerate(enumerator, &object) && + attr[0].ulValueLen != CK_UNAVAILABLE_INFORMATION) { data = chunk_clone(chunk_create(attr[0].pValue, attr[0].ulValueLen)); } diff --git a/src/libstrongswan/plugins/pkcs11/pkcs11_public_key.c b/src/libstrongswan/plugins/pkcs11/pkcs11_public_key.c index 5c7a9a102c..cbebc63644 100644 --- a/src/libstrongswan/plugins/pkcs11/pkcs11_public_key.c +++ b/src/libstrongswan/plugins/pkcs11/pkcs11_public_key.c @@ -888,7 +888,8 @@ static private_pkcs11_public_key_t *find_key_by_keyid(pkcs11_library_t *p11, enumerator = p11->create_object_enumerator(p11, session, tmpl, count, attr, countof(attr)); - if (enumerator->enumerate(enumerator, &object)) + if (enumerator->enumerate(enumerator, &object) && + attr[0].ulValueLen != CK_UNAVAILABLE_INFORMATION) { switch (type) {