Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Restore lost portion of the bleichenbacher timing batch that addressed
review comments. All the review comments pertained to actual code comments,
so this patch only affects the comments.
  • Loading branch information
rjrelyea committed Dec 22, 2020
1 parent 3a48c7d commit 0dfd3aa
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 15 deletions.
12 changes: 6 additions & 6 deletions gtests/freebl_gtest/rsa_unittest.cc
Expand Up @@ -77,18 +77,18 @@ TEST_F(RSATest, DecryptBlockTestErrors) {
EXPECT_EQ(SECFailure, rv);

uint8_t in[256] = {0};
// This should fail because the padding checks will fail.
// however, Bleichenbacher preventions means that failure would be
// a different output.
// This should fail because the padding checks will fail,
// however, mitigations for Bleichenbacher attacks transform failures
// to a different output.
rv = RSA_DecryptBlock(key.get(), out, &outputLen, maxOutputLen, in,
sizeof(in));
EXPECT_EQ(SECSuccess, rv);
// outputLen should <= 256-11=245.
EXPECT_LE(outputLen, 245u);

// This should fail because the padding checks will fail.
// however, Bleichenbacher preventions means that failure would be
// a different output.
// This should fail because the padding checks will fail,
// however, mitigations for Bleichenbacher attacks transform failures
// to a different output.
uint8_t out_long[260] = {0};
maxOutputLen = sizeof(out_long);
rv = RSA_DecryptBlock(key.get(), out_long, &outputLen, maxOutputLen, in,
Expand Down
6 changes: 3 additions & 3 deletions gtests/pk11_gtest/pk11_rsaoaep_unittest.cc
Expand Up @@ -166,14 +166,14 @@ TEST(Pkcs11RsaOaepTest, TestOaepWrapUnwrap) {

// This assumes CKM_RSA_PKCS and doesn't understand OAEP.
// CKM_RSA_PKCS cannot safely return errors, however, as it can lead
// to Blecheinbaucher-like attacks. To solve this there's a new definition
// to Bleichenbacher-like attacks. To solve this there's a new definition
// that generates fake key material based on the message and private key.
// This returned key material will not be the key we were expecting, so
// make sure that's the case:
p_unwrapped_tmp = PK11_PubUnwrapSymKey(priv.get(), wrapped.get(), CKM_AES_CBC,
CKA_DECRYPT, 16);
// as long as the wrapped data is legal RSA length of the key
// (which is should be), then CKM_RSA_PKCS should not fail.
// As long as the wrapped data is the same length as the key
// (which it should be), then CKM_RSA_PKCS should not fail.
ASSERT_NE(p_unwrapped_tmp, nullptr);
ScopedPK11SymKey fakeUnwrapped;
fakeUnwrapped.reset(p_unwrapped_tmp);
Expand Down
2 changes: 0 additions & 2 deletions lib/freebl/alghmac.c
Expand Up @@ -37,12 +37,10 @@ HMAC_Destroy(HMACContext *cx, PRBool freeit)
PORT_Free(cx);
}

/* just setup the hmac key */
static SECStatus
hmac_initKey(HMACContext *cx, const unsigned char *secret,
unsigned int secret_len, PRBool isFIPS)
{

unsigned int i;
unsigned char hashed_secret[HASH_LENGTH_MAX];

Expand Down
9 changes: 5 additions & 4 deletions lib/freebl/rsapkcs.c
Expand Up @@ -1035,7 +1035,7 @@ rsa_HMACPrf(HMACContext *hmac, const char *label, int labelLen,
return rv;
}

/* This function takes an input number and
/* This function takes a 16-bit input number and
* creates the smallest mask which covers
* the whole number. Examples:
* 0x81 -> 0xff
Expand Down Expand Up @@ -1082,8 +1082,9 @@ rsa_GetErrorLength(HMACContext *hmac, int hashLen, int maxLegalLen)
/*
* This function can only fail in environmental cases: Programming errors
* and out of memory situations. It can't fail if the keys are valid and
* the inputs are the proper size. If the actual RSA decryption fails, then
* and generated return value is returned based on the key and input.
* the inputs are the proper size. If the actual RSA decryption fails, a
* fake value and a fake length, both of which have already been generated
* based on the key and input, are returned.
* Applications are expected to detect decryption failures based on the fact
* that the decrypted value (usually a key) doesn't validate. The prevents
* Blecheinbaucher style attacks against the key. */
Expand Down Expand Up @@ -1184,7 +1185,7 @@ RSA_DecryptBlock(RSAPrivateKey *key,
ep = errorBuffer + modulusLen - outLen;

/* at this point, outLen returns no information about decryption failures,
* no need to hide it's value. maxOutputLen is how much data the
* no need to hide its value. maxOutputLen is how much data the
* application is expecting, which is also not sensitive. */
if (outLen > maxOutputLen) {
outLen = maxOutputLen;
Expand Down

0 comments on commit 0dfd3aa

Please sign in to comment.