Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1576307 - Check mechanism param and param length before casting t…
…o mechanism-specific structs. r=jcj

This patch adds missing PKCS11 input parameter checks, which are needed prior to casting to mechanism-specific structs.

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

--HG--
extra : moz-landing-system : lando
  • Loading branch information
Kevin Jacobs committed Sep 30, 2019
1 parent deb6103 commit c071a2a
Show file tree
Hide file tree
Showing 2 changed files with 214 additions and 19 deletions.
84 changes: 84 additions & 0 deletions gtests/pk11_gtest/pk11_cbc_unittest.cc
Expand Up @@ -233,6 +233,90 @@ TEST_P(Pkcs11CbcPadTest, FailEncryptSimple) {
EXPECT_EQ(333U, output_len);
}

// It's a bit of a lie to put this in pk11_cbc_unittest, since we
// also test bounds checking in other modes. There doesn't seem
// to be an appropriately-generic place elsewhere.
TEST_F(Pkcs11CbcPadTest, FailEncryptShortParam) {
SECStatus rv = SECFailure;
uint8_t encrypted[sizeof(kInput)];
unsigned int encrypted_len = 0;
size_t input_len = AES_BLOCK_SIZE;

// CK_GCM_PARAMS is the largest param struct used across AES modes
uint8_t param_buf[sizeof(CK_GCM_PARAMS)];
SECItem param = {siBuffer, param_buf, sizeof(param_buf)};
SECItem key_item = {siBuffer, const_cast<uint8_t*>(kKeyData), 16};

// Setup (we use the ECB key for other modes)
ScopedPK11SlotInfo slot(PK11_GetInternalSlot());
ASSERT_NE(nullptr, slot);
ScopedPK11SymKey key(PK11_ImportSymKey(slot.get(), CKM_AES_ECB,
PK11_OriginUnwrap, CKA_ENCRYPT,
&key_item, nullptr));
ASSERT_TRUE(key.get());

// CTR should have a CK_AES_CTR_PARAMS
param.len = sizeof(CK_AES_CTR_PARAMS) - 1;
rv = PK11_Encrypt(key.get(), CKM_AES_CTR, &param, encrypted, &encrypted_len,
sizeof(encrypted), kInput, input_len);
EXPECT_EQ(SECFailure, rv);

param.len++;
reinterpret_cast<CK_AES_CTR_PARAMS*>(param.data)->ulCounterBits = 32;
rv = PK11_Encrypt(key.get(), CKM_AES_CTR, &param, encrypted, &encrypted_len,
sizeof(encrypted), kInput, input_len);
EXPECT_EQ(SECSuccess, rv);

// GCM should have a CK_GCM_PARAMS
param.len = sizeof(CK_GCM_PARAMS) - 1;
rv = PK11_Encrypt(key.get(), CKM_AES_GCM, &param, encrypted, &encrypted_len,
sizeof(encrypted), kInput, input_len);
EXPECT_EQ(SECFailure, rv);

param.len++;
reinterpret_cast<CK_GCM_PARAMS*>(param.data)->pIv = param_buf;
reinterpret_cast<CK_GCM_PARAMS*>(param.data)->ulIvLen = 12;
reinterpret_cast<CK_GCM_PARAMS*>(param.data)->pAAD = nullptr;
reinterpret_cast<CK_GCM_PARAMS*>(param.data)->ulAADLen = 0;
reinterpret_cast<CK_GCM_PARAMS*>(param.data)->ulTagBits = 128;
rv = PK11_Encrypt(key.get(), CKM_AES_GCM, &param, encrypted, &encrypted_len,
sizeof(encrypted), kInput, input_len);
EXPECT_EQ(SECSuccess, rv);

// CBC (and the below modes) should have a 16B IV
param.len = AES_BLOCK_SIZE - 1;
rv = PK11_Encrypt(key.get(), CKM_AES_CBC, &param, encrypted, &encrypted_len,
sizeof(encrypted), kInput, input_len);
EXPECT_EQ(SECFailure, rv);

param.len++;
rv = PK11_Encrypt(key.get(), CKM_AES_CBC, &param, encrypted, &encrypted_len,
sizeof(encrypted), kInput, input_len);
EXPECT_EQ(SECSuccess, rv);

// ECB
param.len = AES_BLOCK_SIZE - 1;
rv = PK11_Encrypt(key.get(), CKM_AES_CBC, &param, encrypted, &encrypted_len,
sizeof(encrypted), kInput, input_len);
EXPECT_EQ(SECFailure, rv);

param.len++;
rv = PK11_Encrypt(key.get(), CKM_AES_ECB, &param, encrypted, &encrypted_len,
sizeof(encrypted), kInput, input_len);
EXPECT_EQ(SECSuccess, rv);

// CTS
param.len = AES_BLOCK_SIZE - 1;
rv = PK11_Encrypt(key.get(), CKM_AES_CBC, &param, encrypted, &encrypted_len,
sizeof(encrypted), kInput, input_len);
EXPECT_EQ(SECFailure, rv);

param.len++;
rv = PK11_Encrypt(key.get(), CKM_AES_CTS, &param, encrypted, &encrypted_len,
sizeof(encrypted), kInput, input_len);
EXPECT_EQ(SECSuccess, rv);
}

TEST_P(Pkcs11CbcPadTest, ContextFailDecryptSimple) {
ScopedPK11Context dctx = MakeContext(CKA_DECRYPT);
uint8_t output[sizeof(kInput) + 64];
Expand Down

0 comments on commit c071a2a

Please sign in to comment.