Commit 59b1129e authored by Dr. Stephen Henson's avatar Dr. Stephen Henson Committed by Andy Polyakov
Browse files

Fix IV check and padding removal. 

Fix the calculation that checks there is enough room in a record
after removing padding and optional explicit IV. (by Steve)

For AEAD remove the correct number of padding bytes (by Andy)
(cherry picked from commit be125aa5)
parent fb092ef4

ssl/s3_cbc.c

+15 −23
Original line number Diff line number Diff line
@@ -139,14 +139,21 @@ int tls1_cbc_remove_padding(const SSL* s, 
			    unsigned mac_size)

	{

	unsigned padding_length, good, to_check, i;

	const char has_explicit_iv = s->version == DTLS1_VERSION;

	const unsigned overhead = 1 /* padding length byte */ +

				  mac_size +

				  (has_explicit_iv ? block_size : 0);



	/* These lengths are all public so we can test them in non-constant

	 * time. */

	if (overhead > rec->length)

	const unsigned overhead = 1 /* padding length byte */ + mac_size;

	/* Check if version requires explicit IV */

	if (s->version == DTLS1_VERSION)

		{

		/* These lengths are all public so we can test them in

		 * non-constant time.

		 */

		if (overhead + block_size > rec->length)

			return 0;

		/* We can now safely skip explicit IV */

		rec->data += block_size;

		rec->input += block_size;

		rec->length -= block_size;

		}

	else if (overhead > rec->length)

		return 0;



	padding_length = rec->data[rec->length-1];
@@ -208,21 +215,6 @@ int tls1_cbc_remove_padding(const SSL* s, 
	rec->length -= padding_length;

	rec->type |= padding_length<<8;	/* kludge: pass padding length */



	/* We can always safely skip the explicit IV. We check at the beginning

	 * of this function that the record has at least enough space for the

	 * IV, MAC and padding length byte. (These can be checked in

	 * non-constant time because it's all public information.) So, if the

	 * padding was invalid, then we didn't change |rec->length| and this is

	 * safe. If the padding was valid then we know that we have at least

	 * overhead+padding_length bytes of space and so this is still safe

	 * because overhead accounts for the explicit IV. */

	if (has_explicit_iv)

		{

		rec->data += block_size;

		rec->input += block_size;

		rec->length -= block_size;

		}



	return (int)((good & 1) | (~good & -1));

	}