AES_set_encrypt_key and variants return error codes, but they are ignored

[briansmith]

Look at

ring/crypto/cipher/e_aes.c

Line 152 in 80400f5

AES_set_encrypt_key(key, key_len * 8, aes_key);

and other instances where those functions are called. The return value is ignored. It seems like the functions will never fail if given valid inputs, so they should have their return types changed to void.

However, some variants aren't so clear as to whether they can fail on valid inputs or not. For example, asm_AES_set_encrypt_key/_armv4_AES_set_encrypt_key have code to jump to .Labrt. Is this just the "verify inputs are not null" check? If so, that code can be removed.

Note that in BoringSSL, but not OpenSSL, it is assumed that input parameters are not NULL unless the function's documentation specifically states they may be NULL, and NULL checks on parameters are not done.

[ecstatic-morse]

I'd like to look into this.

Findings
(updated as I get new information)

The hand-coded C and assembly implementations of AES_set_encrypt_key are consistent. They return an error code if and only if one of the following invariants is violated:

• The input pointers must be non-null (C, x86_64, i586, ARMv4)
• bits (the key length) must be either 128 or 256 (C, x86_64, i586, ARMv4)

However, the accelerated implementations have different semantics.

The vectorized implementation (vpaes) treats all keys greater than 192 bits as 256-bit, all keys less than or equal to 192 bits as 128 bit (x86, x86_64). As far as I can tell, these versions don't check their input pointers either and thus will never return an error.

The AES-NI version has the same error semantics as the C version but it also handles 192 bit keys (x86, x86_64).