Skip to content

Commit

Permalink
Bug 1612881 - Maintain PKCS11 C_GetAttributeValue semantics on attrib…
Browse files Browse the repository at this point in the history 
…utes that lack NSS database columns r=keeler,rrelyea

`sdb_GetAttributeValueNoLock` builds a query string from a list of attributes in the input template. Unfortunately, `sqlite3_prepare_v2` will fail the entire query if one of the attributes is missing from the underlying table. The PKCS #11 spec [[ https://www.cryptsoft.com/pkcs11doc/v220/pkcs11__all_8h.html#aC_GetAttributeValue | requires ]] setting the output `ulValueLen` field to -1 for such invalid attributes.

This patch reads and stores the columns of nssPublic/nssPrivate when opened, then filters an input template in `sdb_GetAttributeValueNoLock` for unbacked/invalid attributes, removing them from the query and setting their template output lengths to -1.

Differential Revision: https://phabricator.services.mozilla.com/D71622

--HG--
extra : moz-landing-system : lando
  • Loading branch information
Kevin Jacobs committed Apr 24, 2020
1 parent 9e091f5 commit 38eefc7
Show file tree
Hide file tree
Showing 6 changed files with 441 additions and 62 deletions.
3 changes: 2 additions & 1 deletion automation/abi-check/expected-report-libnss3.so.txt
View file
@@ -1,8 +1,9 @@
6 Added functions:
7 Added functions:

[A] 'function SECStatus PK11_AEADOp(PK11Context*, CK_GENERATOR_FUNCTION, int, unsigned char*, int, const unsigned char*, int, unsigned char*, int*, int, unsigned char*, int, const unsigned char*, int)' {PK11_AEADOp@@NSS_3.52}
[A] 'function SECStatus PK11_AEADRawOp(PK11Context*, void*, int, const unsigned char*, int, unsigned char*, int*, int, const unsigned char*, int)' {PK11_AEADRawOp@@NSS_3.52}
[A] 'function CK_OBJECT_HANDLE PK11_GetObjectHandle(PK11ObjectType, void*, PK11SlotInfo**)' {PK11_GetObjectHandle@@NSS_3.52}
[A] 'function SECStatus PK11_ReadRawAttributes(PLArenaPool*, PK11ObjectType, void*, CK_ATTRIBUTE*, unsigned int)' {PK11_ReadRawAttributes@@NSS_3.52}
[A] 'function SECStatus PK11_SymKeysToSameSlot(CK_MECHANISM_TYPE, CK_ATTRIBUTE_TYPE, CK_ATTRIBUTE_TYPE, PK11SymKey*, PK11SymKey*, PK11SymKey**, PK11SymKey**)' {PK11_SymKeysToSameSlot@@NSS_3.52}
[A] 'function PRBool _PK11_ContextGetAEADSimulation(PK11Context*)' {_PK11_ContextGetAEADSimulation@@NSS_3.52}
[A] 'function SECStatus _PK11_ContextSetAEADSimulation(PK11Context*)' {_PK11_ContextSetAEADSimulation@@NSS_3.52}
Expand Down
169 changes: 169 additions & 0 deletions gtests/softoken_gtest/softoken_gtest.cc
View file
Expand Up @@ -100,6 +100,175 @@ static const CK_ATTRIBUTE attributes[] = {
{CKA_TRUST_STEP_UP_APPROVED, (void *)&ck_false,
(PRUint32)sizeof(CK_BBOOL)}};

TEST_F(SoftokenTest, GetInvalidAttribute) {
ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
ASSERT_TRUE(slot);
EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, "password"));
ScopedPK11GenericObject obj(PK11_CreateGenericObject(
slot.get(), attributes, PR_ARRAY_SIZE(attributes), true));
ASSERT_NE(nullptr, obj);
SECItem out = {siBuffer, nullptr, 0};
SECStatus rv = PK11_ReadRawAttribute(PK11_TypeGeneric, obj.get(),
CKA_ALLOWED_MECHANISMS, &out);
EXPECT_EQ(SECFailure, rv);
// CKR_ATTRIBUTE_TYPE_INVALID maps to SEC_ERROR_BAD_DATA.
EXPECT_EQ(SEC_ERROR_BAD_DATA, PORT_GetError());
}

TEST_F(SoftokenTest, GetValidAttributes) {
ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
ASSERT_TRUE(slot);
EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, "password"));
ScopedPK11GenericObject obj(PK11_CreateGenericObject(
slot.get(), attributes, PR_ARRAY_SIZE(attributes), true));
ASSERT_NE(nullptr, obj);

CK_ATTRIBUTE template_attrs[] = {
{CKA_LABEL, NULL, 0},
{CKA_CERT_SHA1_HASH, NULL, 0},
{CKA_ISSUER, NULL, 0},
};
SECStatus rv =
PK11_ReadRawAttributes(nullptr, PK11_TypeGeneric, obj.get(),
template_attrs, PR_ARRAY_SIZE(template_attrs));
EXPECT_EQ(SECSuccess, rv);
ASSERT_EQ(attributes[4].ulValueLen, template_attrs[0].ulValueLen);
EXPECT_EQ(0, memcmp(attributes[4].pValue, template_attrs[0].pValue,
template_attrs[0].ulValueLen));
ASSERT_EQ(attributes[5].ulValueLen, template_attrs[1].ulValueLen);
EXPECT_EQ(0, memcmp(attributes[5].pValue, template_attrs[1].pValue,
template_attrs[1].ulValueLen));
ASSERT_EQ(attributes[7].ulValueLen, template_attrs[2].ulValueLen);
EXPECT_EQ(0, memcmp(attributes[7].pValue, template_attrs[2].pValue,
template_attrs[2].ulValueLen));
for (unsigned int i = 0; i < PR_ARRAY_SIZE(template_attrs); i++) {
PORT_Free(template_attrs[i].pValue);
}
}

TEST_F(SoftokenTest, GetOnlyInvalidAttributes) {
ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
ASSERT_TRUE(slot);
EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, "password"));
ScopedPK11GenericObject obj(PK11_CreateGenericObject(
slot.get(), attributes, PR_ARRAY_SIZE(attributes), true));
ASSERT_NE(nullptr, obj);

// Provide buffers of sufficient size, so that token
// will write the data. This is annoying, but PK11_GetAttributes
// won't allocate in the cases below when a single attribute
// is missing. So, just put them all on the stack.
unsigned char buf1[100];
unsigned char buf2[100];
CK_ATTRIBUTE template_attrs[] = {{0xffffffffUL, buf1, sizeof(buf1)},
{0xfffffffeUL, buf2, sizeof(buf2)}};
SECStatus rv =
PK11_ReadRawAttributes(nullptr, PK11_TypeGeneric, obj.get(),
template_attrs, PR_ARRAY_SIZE(template_attrs));
EXPECT_EQ(SECFailure, rv);
EXPECT_EQ(SEC_ERROR_BAD_DATA, PORT_GetError());

// MSVC rewards -1UL with a C4146 warning...
ASSERT_EQ(0UL, template_attrs[0].ulValueLen + 1);
ASSERT_EQ(0UL, template_attrs[1].ulValueLen + 1);
}

TEST_F(SoftokenTest, GetAttributesInvalidInterspersed1) {
ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
ASSERT_TRUE(slot);
EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, "password"));
ScopedPK11GenericObject obj(PK11_CreateGenericObject(
slot.get(), attributes, PR_ARRAY_SIZE(attributes), true));
ASSERT_NE(nullptr, obj);

unsigned char buf1[100];
unsigned char buf2[100];
unsigned char buf3[200];
CK_ATTRIBUTE template_attrs[] = {{0xffffffff, buf1, sizeof(buf1)},
{CKA_CERT_SHA1_HASH, buf2, sizeof(buf2)},
{CKA_ISSUER, buf3, sizeof(buf3)}};
SECStatus rv =
PK11_ReadRawAttributes(nullptr, PK11_TypeGeneric, obj.get(),
template_attrs, PR_ARRAY_SIZE(template_attrs));
EXPECT_EQ(SECFailure, rv);
EXPECT_EQ(SEC_ERROR_BAD_DATA, PORT_GetError());
ASSERT_EQ(0UL, template_attrs[0].ulValueLen + 1);
ASSERT_EQ(attributes[5].ulValueLen, template_attrs[1].ulValueLen);
EXPECT_EQ(0, memcmp(attributes[5].pValue, template_attrs[1].pValue,
template_attrs[1].ulValueLen));
ASSERT_EQ(attributes[7].ulValueLen, template_attrs[2].ulValueLen);
EXPECT_EQ(0, memcmp(attributes[7].pValue, template_attrs[2].pValue,
template_attrs[2].ulValueLen));
}

TEST_F(SoftokenTest, GetAttributesInvalidInterspersed2) {
ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
ASSERT_TRUE(slot);
EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, "password"));
ScopedPK11GenericObject obj(PK11_CreateGenericObject(
slot.get(), attributes, PR_ARRAY_SIZE(attributes), true));
ASSERT_NE(nullptr, obj);

unsigned char buf1[100];
unsigned char buf2[100];
unsigned char buf3[100];
CK_ATTRIBUTE template_attrs[] = {{CKA_LABEL, buf1, sizeof(buf1)},
{CKA_CERT_SHA1_HASH, buf2, sizeof(buf2)},
{0xffffffffUL, buf3, sizeof(buf3)}};
SECStatus rv =
PK11_ReadRawAttributes(nullptr, PK11_TypeGeneric, obj.get(),
template_attrs, PR_ARRAY_SIZE(template_attrs));
EXPECT_EQ(SECFailure, rv);
EXPECT_EQ(SEC_ERROR_BAD_DATA, PORT_GetError());
ASSERT_EQ(attributes[4].ulValueLen, template_attrs[0].ulValueLen);
EXPECT_EQ(0, memcmp(attributes[4].pValue, template_attrs[0].pValue,
template_attrs[0].ulValueLen));
ASSERT_EQ(attributes[5].ulValueLen, template_attrs[1].ulValueLen);
EXPECT_EQ(0, memcmp(attributes[5].pValue, template_attrs[1].pValue,
template_attrs[1].ulValueLen));
ASSERT_EQ(0UL, template_attrs[2].ulValueLen + 1);
}

TEST_F(SoftokenTest, GetAttributesInvalidInterspersed3) {
ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
ASSERT_TRUE(slot);
EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, "password"));
ScopedPK11GenericObject obj(PK11_CreateGenericObject(
slot.get(), attributes, PR_ARRAY_SIZE(attributes), true));
ASSERT_NE(nullptr, obj);

unsigned char buf1[100];
unsigned char buf2[100];
unsigned char buf3[100];
unsigned char buf4[100];
unsigned char buf5[100];
unsigned char buf6[200];
CK_ATTRIBUTE template_attrs[6] = {{CKA_LABEL, buf1, sizeof(buf1)},
{0xffffffffUL, buf2, sizeof(buf2)},
{0xfffffffeUL, buf3, sizeof(buf3)},
{CKA_CERT_SHA1_HASH, buf4, sizeof(buf4)},
{0xfffffffdUL, buf5, sizeof(buf5)},
{CKA_ISSUER, buf6, sizeof(buf6)}};
SECStatus rv =
PK11_ReadRawAttributes(nullptr, PK11_TypeGeneric, obj.get(),
template_attrs, PR_ARRAY_SIZE(template_attrs));
EXPECT_EQ(SECFailure, rv);
EXPECT_EQ(SEC_ERROR_BAD_DATA, PORT_GetError());

ASSERT_EQ(attributes[4].ulValueLen, template_attrs[0].ulValueLen);
EXPECT_EQ(0, memcmp(attributes[4].pValue, template_attrs[0].pValue,
template_attrs[0].ulValueLen));
ASSERT_EQ(0UL, template_attrs[1].ulValueLen + 1);
ASSERT_EQ(0UL, template_attrs[2].ulValueLen + 1);
ASSERT_EQ(attributes[5].ulValueLen, template_attrs[3].ulValueLen);
EXPECT_EQ(0, memcmp(attributes[5].pValue, template_attrs[3].pValue,
template_attrs[3].ulValueLen));
ASSERT_EQ(0UL, template_attrs[4].ulValueLen + 1);
ASSERT_EQ(attributes[7].ulValueLen, template_attrs[5].ulValueLen);
EXPECT_EQ(0, memcmp(attributes[7].pValue, template_attrs[5].pValue,
template_attrs[5].ulValueLen));
}

TEST_F(SoftokenTest, CreateObjectNonEmptyPassword) {
ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
ASSERT_TRUE(slot);
Expand Down
1 change: 1 addition & 0 deletions lib/nss/nss.def
View file
Expand Up @@ -1170,6 +1170,7 @@ _PK11_ContextSetAEADSimulation;
PK11_AEADOp;
PK11_AEADRawOp;
PK11_GetObjectHandle;
PK11_ReadRawAttributes;
PK11_SymKeysToSameSlot;
;+ local:
;+ *;
Expand Down
20 changes: 20 additions & 0 deletions lib/pk11wrap/pk11obj.c
View file
Expand Up @@ -1783,6 +1783,26 @@ PK11_ReadRawAttribute(PK11ObjectType objType, void *objSpec,
return PK11_ReadAttribute(slot, handle, attrType, NULL, item);
}

SECStatus
PK11_ReadRawAttributes(PLArenaPool *arena, PK11ObjectType objType, void *objSpec,
CK_ATTRIBUTE *pTemplate, unsigned int count)
{
PK11SlotInfo *slot = NULL;
CK_OBJECT_HANDLE handle = 0;

handle = PK11_GetObjectHandle(objType, objSpec, &slot);
if (handle == CK_INVALID_HANDLE) {
PORT_SetError(SEC_ERROR_UNKNOWN_OBJECT_TYPE);
return SECFailure;
}
CK_RV crv = PK11_GetAttributes(arena, slot, handle, pTemplate, count);
if (crv != CKR_OK) {
PORT_SetError(PK11_MapError(crv));
return SECFailure;
}
return SECSuccess;
}

/*
* return the object handle that matches the template
*/
Expand Down
2 changes: 2 additions & 0 deletions lib/pk11wrap/pk11pub.h
View file
Expand Up @@ -893,6 +893,8 @@ PK11GenericObject *PK11_CreateGenericObject(PK11SlotInfo *slot,
*/
SECStatus PK11_ReadRawAttribute(PK11ObjectType type, void *object,
CK_ATTRIBUTE_TYPE attr, SECItem *item);
SECStatus PK11_ReadRawAttributes(PLArenaPool *arena, PK11ObjectType type, void *object,
CK_ATTRIBUTE *pTemplate, unsigned int count);
SECStatus PK11_WriteRawAttribute(PK11ObjectType type, void *object,
CK_ATTRIBUTE_TYPE attr, SECItem *item);
/* get the PKCS #11 handle and slot for a generic object */
Expand Down

0 comments on commit 38eefc7

Please sign in to comment.