OS-7955 CKM_AES_CBC_PAD multi-part updates w/ multiple full blocks incorrectly

Reviewed by: Dan McDonald <danmcd@joyent.com>
Approved by: Tim Foster <tim.foster@joyent.com>

1 files changed, 199 insertions, 13 deletions

diff --git a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softAESCrypt.c b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softAESCrypt.c
index 4aff7660c5..c57fc4c442 100644
--- a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softAESCrypt.c
+++ b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softAESCrypt.c
@@ -878,6 +878,7 @@ soft_aes_decrypt_update(soft_session_t *session_p, CK_BYTE_PTR pEncryptedData,
     CK_ULONG ulEncryptedDataLen, CK_BYTE_PTR pData, CK_ULONG_PTR pulDataLen)
 {
 	aes_ctx_t *aes_ctx = session_p->decrypt.context;
+	uint8_t *buffer_block = NULL;
 	crypto_data_t out = {
 		.cd_format = CRYPTO_DATA_RAW,
 		.cd_offset = 0,
@@ -887,6 +888,7 @@ soft_aes_decrypt_update(soft_session_t *session_p, CK_BYTE_PTR pEncryptedData,
 	};
 	CK_MECHANISM_TYPE mech = session_p->decrypt.mech.mechanism;
 	CK_RV rv = CKR_OK;
+	size_t in_len = ulEncryptedDataLen;
 	size_t out_len;
 	int rc = CRYPTO_SUCCESS;
 
@@ -896,44 +898,207 @@ soft_aes_decrypt_update(soft_session_t *session_p, CK_BYTE_PTR pEncryptedData,
 		out_len = 0;
 		break;
 	case CKM_AES_CBC_PAD:
-		out_len = aes_ctx->ac_remainder_len + ulEncryptedDataLen;
 		/*
-		 * Since we cannot know which data is the final block
-		 * of data until C_DecryptFinal() is called, we must
-		 * always buffer the most recent full block of
-		 * ciphertext.  Once C_DecryptFinal() is called, we should
-		 * have a full block of ciphertext buffered that we can
-		 * decrypt and remove the padding.
+		 * For CKM_AES_CBC_PAD, we use the existing code for CBC
+		 * mode in libsoftcrypto (which itself uses the code in
+		 * usr/src/common/crypto/modes for CBC mode).  For
+		 * non-padding AES CBC mode, aes_decrypt_contiguous_blocks()
+		 * will accumulate ciphertext in aes_ctx->ac_remainder until
+		 * there is at least AES_BLOCK_LEN bytes of ciphertext available
+		 * to decrypt.  At that point, as many blocks of AES_BLOCK_LEN
+		 * sized ciphertext blocks are decrypted.  Any remainder is
+		 * copied into aes_ctx->ac_remainder for decryption in
+		 * subsequent calls to aes_decrypt_contiguous_blocks().
+		 *
+		 * When PKCS#7 padding is used, the buffering
+		 * aes_decrypt_contigous_blocks() performs is insufficient.
+		 * PKCS#7 padding always adds [1..AES_BLOCK_LEN] bytes of
+		 * padding to plaintext, so the resulting ciphertext is always
+		 * larger than the input plaintext.  However we cannot know
+		 * which block is the final block (and needs its padding
+		 * stripped) until C_DecryptFinal() is called.  Additionally,
+		 * it is permissible for a caller to use buffers sized to the
+		 * output plaintext -- i.e. smaller than the input ciphertext.
+		 * This leads to a more complicated buffering/accumulation
+		 * strategy than what aes_decrypt_contiguous_blocks() provides
+		 * us.
+		 *
+		 * Our buffering strategy works as follows:
+		 *  For each call to C_DecryptUpdate, we calculate the
+		 *  total amount of ciphertext available (buffered plus what's
+		 *  passed in) as the initial output size (out_len). Based
+		 *  on the value of out_len, there are three possibilties:
+		 *
+		 *  1. We have less than AES_BLOCK_LEN + 1 bytes of
+		 *  ciphertext available. Accumulate the ciphertext in
+		 *  aes_ctx->ac_remainder. Note that while we could let
+		 *  aes_decrypt_contiguous_blocks() buffer the input for us
+		 *  when we have less than AES_BLOCK_LEN bytes, we would still
+		 *  need to buffer when we have exactly AES_BLOCK_LEN
+		 *  bytes available, so we just handle both situations with
+		 *  one if clause.
+		 *
+		 *  2. We have at least AES_BLOCK_LEN + 1 bytes of
+		 *  ciphertext, and the total amount available is also an
+		 *  exact multiple of AES_BLOCK_LEN. We cannot know if the
+		 *  last block of input is the final block (yet), but we
+		 *  are an exact multiple of AES_BLOCK_LEN, and we have
+		 *  at least AES_BLOCK_LEN + 1 bytes available, therefore
+		 *  there must be at least 2 * AES_BLOCK_LEN bytes of input
+		 *  ciphertext available. It also means there's at least one
+		 *  full block of input ciphertext that can be decrypted. We
+		 *  reduce the size of the input (in_len) given to
+		 *  aes_decrypt_contiguous_bytes() by AES_BLOCK_LEN to prevent
+		 *  it from decrypting the last full block of data.
+		 *  aes_decrypt_contiguous_blocks() will when decrypt any
+		 *  buffered data in aex_ctx->ac_remainder, and then any
+		 *  input data passed. Since we have an exact multiple of
+		 *  AES_BLOCK_LEN, aes_ctx->ac_remainder will be empty
+		 *  (aes_ctx->ac_remainder_len == 0), once
+		 *  aes_decrypt_contiguout_block() completes, and we can
+		 *  copy the last block of data into aes_ctx->ac_remainder.
+		 *
+		 *  3. We have at least AES_BLOCK_LEN + 1 bytes of
+		 *  ciphertext, but the total amount available is not an
+		 *  exact multiple of AES_BLOCK_LEN. We decrypt all of
+		 *  full blocks of data we have. The remainder will be
+		 *  less than AES_BLOCK_LEN bytes. We let
+		 *  aes_decrypt_contiguous_blocks() buffer the remainder
+		 *  for us since it would normally do this anyway. Since there
+		 *  is a remainder, the full blocks that are present cannot
+		 *  be the last block, so we can safey decrypt all of them.
+		 *
+		 * Some things to note:
+		 *  - The above semantics will cause aes_ctx->ac_remainder to
+		 *  never accumulate more than AES_BLOCK_LEN bytes of ciphertext.
+		 *  Once we reach at least AES_BLOCK_LEN + 1 bytes, we will
+		 *  decrypt the contents of aes_ctx->ac_remainder by one of
+		 *  the last two scenarios described above.
+		 *
+		 *  - We must always end up with AES_BLOCK_LEN bytes of data
+		 *  in aes_ctx->ac_remainder when C_DecryptFinal() is called.
+		 *  The first and third scenarios above may leave
+		 *  aes_ctx->ac_remainder with less than AES_BLOCK_LEN bytes,
+		 *  however the total size of the input ciphertext that's
+		 *  been decrypted must end up a multiple of AES_BLOCK_LEN.
+		 *  Therefore, we can always assume when there is a
+		 *  remainder that more data is coming.  If we do end up
+		 *  with a remainder that's not AES_BLOCK_LEN bytes long
+		 *  when C_DecryptFinal() is called, the input is assumed
+		 *  invalid and we return CKR_DATA_LEN_RANGE (see
+		 *  soft_aes_decrypt_final()).
 		 */
+
+		VERIFY3U(aes_ctx->ac_remainder_len, <=, AES_BLOCK_LEN);
+		if (in_len >= SIZE_MAX - AES_BLOCK_LEN)
+			return (CKR_ENCRYPTED_DATA_LEN_RANGE);
+
+		out_len = aes_ctx->ac_remainder_len + in_len;
+
 		if (out_len <= AES_BLOCK_LEN) {
+			/*
+			 * The first scenario detailed above, accumulate
+			 * ciphertext in ac_remainder_len and return.
+			 */
 			uint8_t *dest = (uint8_t *)aes_ctx->ac_remainder +
 			    aes_ctx->ac_remainder_len;
 
-			bcopy(pEncryptedData, dest, ulEncryptedDataLen);
-			aes_ctx->ac_remainder_len += ulEncryptedDataLen;
+			bcopy(pEncryptedData, dest, in_len);
+			aes_ctx->ac_remainder_len += in_len;
 			*pulDataLen = 0;
+
+			/*
+			 * Since we aren't writing an output, and are returning
+			 * here, we don't need to adjust out_len -- we never
+			 * reach the output buffer size checks after the
+			 * switch statement.
+			 */
 			return (CKR_OK);
+		} else if (out_len % AES_BLOCK_LEN == 0) {
+			/*
+			 * The second scenario decribed above. The total amount
+			 * available is a multiple of AES_BLOCK_LEN, and
+			 * we have more than one block.  We reduce the
+			 * input size (in_len) by AES_BLOCK_LEN. We also
+			 * reduce the output size (out_len) by AES_BLOCK_LEN
+			 * for the output buffer size checks that follow
+			 * the switch statement. In certain situations,
+			 * PKCS#11 requires this to be an exact value, so
+			 * the size check cannot occur for CKM_AES_CBC_PAD
+			 * until after we've determine which scenario we
+			 * have.
+			 *
+			 * Because we never accumulate more than AES_BLOCK_LEN
+			 * bytes in aes_ctx->ac_remainder, when we are in
+			 * this scenario, the following VERIFYs should always
+			 * be true (and serve as a final safeguard against
+			 * underflow).
+			 */
+			VERIFY3U(in_len, >=, AES_BLOCK_LEN);
+
+			buffer_block = pEncryptedData + in_len - AES_BLOCK_LEN;
+
+			in_len -= AES_BLOCK_LEN;
+
+			/*
+			 * This else clause explicity checks
+			 * out_len > AES_BLOCK_LEN, so this is also safe.
+			 */
+			out_len -= AES_BLOCK_LEN;
+		} else {
+			/*
+			 * The third scenario above.  We have at least
+			 * AES_BLOCK_LEN + 1 bytes, but the total amount of
+			 * input ciphertext available is not an exact
+			 * multiple of AES_BLOCK_LEN.  Let
+			 * aes_decrypt_contiguous_blocks() handle the
+			 * buffering of the remainder.  Update the
+			 * output size to reflect the actual amount of output
+			 * we want to emit for the checks after the switch
+			 * statement.
+			 */
+			out_len &= ~(AES_BLOCK_LEN - 1);
 		}
-		out_len &= ~(AES_BLOCK_LEN - 1);
 		break;
 	default:
-		out_len = aes_ctx->ac_remainder_len + ulEncryptedDataLen;
+		out_len = aes_ctx->ac_remainder_len + in_len;
 		out_len &= ~(AES_BLOCK_LEN - 1);
 		break;
 	}
 
+	/*
+	 * C_DecryptUpdate() verifies that pulDataLen is not NULL prior
+	 * to calling soft_decrypt_common() (which calls us).
+	 */
+
 	if (pData == NULL) {
+		/*
+		 * If the output buffer (pData) is NULL, that means the
+		 * caller is inquiring about the size buffer needed to
+		 * complete the C_DecryptUpdate() request.  While we are
+		 * permitted to set *pulDataLen to an estimated value that can
+		 * be 'slightly' larger than the actual value required,
+		 * since we know the exact size we need, we stick with the
+		 * exact size.
+		 */
 		*pulDataLen = out_len;
 		return (CKR_OK);
 	}
 
 	if (*pulDataLen < out_len) {
+		/*
+		 * Not an inquiry, but the output buffer isn't large enough.
+		 * PKCS#11 requires that this scenario not fail fatally (as
+		 * well as return a different error value). This situation
+		 * also requires us to set *pulDataLen to the _exact_ size
+		 * required.
+		 */
 		*pulDataLen = out_len;
 		return (CKR_BUFFER_TOO_SMALL);
 	}
 
 	rc = aes_decrypt_contiguous_blocks(aes_ctx, (char *)pEncryptedData,
-	    ulEncryptedDataLen, &out);
+	    in_len, &out);
 
 	if (rc != CRYPTO_SUCCESS) {
 		rv = CKR_FUNCTION_FAILED;
@@ -942,10 +1107,31 @@ soft_aes_decrypt_update(soft_session_t *session_p, CK_BYTE_PTR pEncryptedData,
 
 	*pulDataLen = out.cd_offset;
 
-	if ((aes_ctx->ac_flags & CTR_MODE) && (aes_ctx->ac_remainder_len > 0)) {
+	switch (mech) {
+	case CKM_AES_CTR:
+		if (aes_ctx->ac_remainder_len == 0) {
+			break;
+		}
 		rc = ctr_mode_final((ctr_ctx_t *)aes_ctx, &out,
 		    aes_encrypt_block);
 		rv = crypto2pkcs11_error_number(rc);
+		break;
+	case CKM_AES_CBC_PAD:
+		if (buffer_block == NULL) {
+			break;
+		}
+
+		VERIFY0(aes_ctx->ac_remainder_len);
+
+		/*
+		 * We had multiple blocks of data to decrypt with nothing
+		 * left over and deferred decrypting the last block of data.
+		 * Copy it into aes_ctx->ac_remainder to decrypt on the
+		 * next update call (or final).
+		 */
+		bcopy(buffer_block, aes_ctx->ac_remainder, AES_BLOCK_LEN);
+		aes_ctx->ac_remainder_len = AES_BLOCK_LEN;
+		break;
 	}
 
 done: