Skip to content

Commit

Permalink
Bug 1354488 - RSA_CheckSign() doesn't set proper error values r=franz…
Browse files Browse the repository at this point in the history 
…iskus

Differential Revision: https://nss-review.dev.mozaws.net/D302

--HG--
extra : amend_source : 2ba663b72d6f47e03a8337579a8d760b468a924a
  • Loading branch information
Tim Taubert committed May 4, 2017
1 parent 4c62732 commit ef9c14d
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 73 deletions.
12 changes: 2 additions & 10 deletions gtests/ssl_gtest/ssl_damage_unittest.cc
View file
Expand Up @@ -70,11 +70,7 @@ TEST_P(TlsConnectGenericPre13, DamageServerSignature) {
server_->SetTlsRecordFilter(filter);
ExpectAlert(client_, kTlsAlertDecryptError);
ConnectExpectFail();
// TODO(ttaubert@mozilla.com): This is the wrong error code in
// 1.1 and below. Bug 1354488.
client_->CheckErrorCode(version_ >= SSL_LIBRARY_VERSION_TLS_1_2
? SEC_ERROR_BAD_SIGNATURE
: SEC_ERROR_PKCS11_DEVICE_ERROR);
client_->CheckErrorCode(SEC_ERROR_BAD_SIGNATURE);
server_->CheckErrorCode(SSL_ERROR_DECRYPT_ERROR_ALERT);
}

Expand Down Expand Up @@ -117,11 +113,7 @@ TEST_P(TlsConnectGeneric, DamageClientSignature) {
? TlsAgent::STATE_CONNECTED
: TlsAgent::STATE_CONNECTING,
client_->state());
// TODO(ttaubert@mozilla.com): This is the wrong error code in
// 1.1 and below. Bug 1354488.
server_->CheckErrorCode(version_ >= SSL_LIBRARY_VERSION_TLS_1_2
? SEC_ERROR_BAD_SIGNATURE
: SEC_ERROR_PKCS11_DEVICE_ERROR);
server_->CheckErrorCode(SEC_ERROR_BAD_SIGNATURE);
}

} // namespace nspr_test
149 changes: 86 additions & 63 deletions lib/freebl/rsapkcs.c
View file
Expand Up @@ -1211,6 +1211,7 @@ RSA_SignPSS(RSAPrivateKey *key,
if (rv != SECSuccess)
goto done;

// This sets error codes upon failure.
rv = RSA_PrivateKeyOpDoubleChecked(key, output, pssEncoded);
*outputLen = modulusLen;

Expand Down Expand Up @@ -1270,7 +1271,6 @@ RSA_CheckSignPSS(RSAPublicKey *key,
return rv;
}

/* XXX Doesn't set error code */
SECStatus
RSA_Sign(RSAPrivateKey *key,
unsigned char *output,
Expand All @@ -1279,95 +1279,106 @@ RSA_Sign(RSAPrivateKey *key,
const unsigned char *input,
unsigned int inputLen)
{
SECStatus rv = SECSuccess;
SECStatus rv = SECFailure;
unsigned int modulusLen = rsa_modulusLen(&key->modulus);
SECItem formatted;
SECItem unformatted;
SECItem formatted = { siBuffer, NULL, 0 };
SECItem unformatted = { siBuffer, (unsigned char *)input, inputLen };

if (maxOutputLen < modulusLen)
return SECFailure;
if (maxOutputLen < modulusLen) {
PORT_SetError(SEC_ERROR_OUTPUT_LEN);
goto done;
}

unformatted.len = inputLen;
unformatted.data = (unsigned char *)input;
formatted.data = NULL;
rv = rsa_FormatBlock(&formatted, modulusLen, RSA_BlockPrivate,
&unformatted);
if (rv != SECSuccess)
if (rv != SECSuccess) {
PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
goto done;
}

// This sets error codes upon failure.
rv = RSA_PrivateKeyOpDoubleChecked(key, output, formatted.data);
*outputLen = modulusLen;

goto done;

done:
if (formatted.data != NULL)
if (formatted.data != NULL) {
PORT_ZFree(formatted.data, modulusLen);
}
return rv;
}

/* XXX Doesn't set error code */
SECStatus
RSA_CheckSign(RSAPublicKey *key,
const unsigned char *sig,
unsigned int sigLen,
const unsigned char *data,
unsigned int dataLen)
{
SECStatus rv;
SECStatus rv = SECFailure;
unsigned int modulusLen = rsa_modulusLen(&key->modulus);
unsigned int i;
unsigned char *buffer;
unsigned char *buffer = NULL;

if (sigLen != modulusLen) {
PORT_SetError(SEC_ERROR_BAD_SIGNATURE);
goto done;
}

if (sigLen != modulusLen)
goto failure;
/*
* 0x00 || BT || Pad || 0x00 || ActualData
*
* The "3" below is the first octet + the second octet + the 0x00
* octet that always comes just before the ActualData.
*/
if (dataLen > modulusLen - (3 + RSA_BLOCK_MIN_PAD_LEN))
goto failure;
if (dataLen > modulusLen - (3 + RSA_BLOCK_MIN_PAD_LEN)) {
PORT_SetError(SEC_ERROR_BAD_DATA);
goto done;
}

buffer = (unsigned char *)PORT_Alloc(modulusLen + 1);
if (!buffer)
goto failure;
if (!buffer) {
PORT_SetError(SEC_ERROR_NO_MEMORY);
goto done;
}

rv = RSA_PublicKeyOp(key, buffer, sig);
if (rv != SECSuccess)
goto loser;
if (RSA_PublicKeyOp(key, buffer, sig) != SECSuccess) {
PORT_SetError(SEC_ERROR_BAD_SIGNATURE);
goto done;
}

/*
* check the padding that was used
*/
if (buffer[0] != RSA_BLOCK_FIRST_OCTET ||
buffer[1] != (unsigned char)RSA_BlockPrivate) {
goto loser;
PORT_SetError(SEC_ERROR_BAD_SIGNATURE);
goto done;
}
for (i = 2; i < modulusLen - dataLen - 1; i++) {
if (buffer[i] != RSA_BLOCK_PRIVATE_PAD_OCTET)
goto loser;
if (buffer[i] != RSA_BLOCK_PRIVATE_PAD_OCTET) {
PORT_SetError(SEC_ERROR_BAD_SIGNATURE);
goto done;
}
}
if (buffer[i] != RSA_BLOCK_AFTER_PAD_OCTET) {
PORT_SetError(SEC_ERROR_BAD_SIGNATURE);
goto done;
}
if (buffer[i] != RSA_BLOCK_AFTER_PAD_OCTET)
goto loser;

/*
* make sure we get the same results
*/
if (PORT_Memcmp(buffer + modulusLen - dataLen, data, dataLen) != 0)
goto loser;

PORT_Free(buffer);
return SECSuccess;
if (PORT_Memcmp(buffer + modulusLen - dataLen, data, dataLen) == 0) {
rv = SECSuccess;
}

loser:
PORT_Free(buffer);
failure:
return SECFailure;
done:
if (buffer) {
PORT_Free(buffer);
}
return rv;
}

/* XXX Doesn't set error code */
SECStatus
RSA_CheckSignRecover(RSAPublicKey *key,
unsigned char *output,
Expand All @@ -1376,50 +1387,62 @@ RSA_CheckSignRecover(RSAPublicKey *key,
const unsigned char *sig,
unsigned int sigLen)
{
SECStatus rv;
SECStatus rv = SECFailure;
unsigned int modulusLen = rsa_modulusLen(&key->modulus);
unsigned int i;
unsigned char *buffer;
unsigned char *buffer = NULL;

if (sigLen != modulusLen)
goto failure;
if (sigLen != modulusLen) {
PORT_SetError(SEC_ERROR_BAD_SIGNATURE);
goto done;
}

buffer = (unsigned char *)PORT_Alloc(modulusLen + 1);
if (!buffer)
goto failure;
if (!buffer) {
PORT_SetError(SEC_ERROR_NO_MEMORY);
goto done;
}

if (RSA_PublicKeyOp(key, buffer, sig) != SECSuccess) {
PORT_SetError(SEC_ERROR_BAD_SIGNATURE);
goto done;
}

rv = RSA_PublicKeyOp(key, buffer, sig);
if (rv != SECSuccess)
goto loser;
*outputLen = 0;

/*
* check the padding that was used
*/
if (buffer[0] != RSA_BLOCK_FIRST_OCTET ||
buffer[1] != (unsigned char)RSA_BlockPrivate) {
goto loser;
PORT_SetError(SEC_ERROR_BAD_SIGNATURE);
goto done;
}
for (i = 2; i < modulusLen; i++) {
if (buffer[i] == RSA_BLOCK_AFTER_PAD_OCTET) {
*outputLen = modulusLen - i - 1;
break;
}
if (buffer[i] != RSA_BLOCK_PRIVATE_PAD_OCTET)
goto loser;
if (buffer[i] != RSA_BLOCK_PRIVATE_PAD_OCTET) {
PORT_SetError(SEC_ERROR_BAD_SIGNATURE);
goto done;
}
}
if (*outputLen == 0) {
PORT_SetError(SEC_ERROR_BAD_SIGNATURE);
goto done;
}
if (*outputLen > maxOutputLen) {
PORT_SetError(SEC_ERROR_OUTPUT_LEN);
goto done;
}
if (*outputLen == 0)
goto loser;
if (*outputLen > maxOutputLen)
goto loser;

PORT_Memcpy(output, buffer + modulusLen - *outputLen, *outputLen);
rv = SECSuccess;

PORT_Free(buffer);
return SECSuccess;

loser:
PORT_Free(buffer);
failure:
return SECFailure;
done:
if (buffer) {
PORT_Free(buffer);
}
return rv;
}

0 comments on commit ef9c14d

Please sign in to comment.