Skip to content

Fix error in CT compare, add negative tests for TLS pad checks#385

Open
ColtonWilley wants to merge 1 commit intowolfSSL:masterfrom
ColtonWilley:fix/ct-byte-mask-ne-negative-pad-tests
Open

Fix error in CT compare, add negative tests for TLS pad checks#385
ColtonWilley wants to merge 1 commit intowolfSSL:masterfrom
ColtonWilley:fix/ct-byte-mask-ne-negative-pad-tests

Conversation

@ColtonWilley
Copy link
Copy Markdown
Contributor

No description provided.

byte wp_ct_byte_mask_ne(byte a, byte b)
{
return (((int32_t)b - a) >> 31) & (((int32_t)a - b) >> 31);
return (((int32_t)b - a) >> 31) | (((int32_t)a - b) >> 31);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is potentially a more robust fix, with less bit math:

return ~wp_ct_byte_mask_eq(a, b);

/**
* Constant time, set mask when first value is not equal to second.
*
* @param [in] a First valuue.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semi-related nit: typo here in "valuue". Also on line 1155.

* Encrypt block-aligned plaintext (produces a full padding block), corrupt a
* ciphertext byte so the padding block is garbled, verify DecryptFinal rejects.
*/
int test_des3_cbc_bad_pad(void *data)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that DES3 CBC is the only path for testing wp_ct_byte_mask_ne, currently. So this is a good add, but...

Can we add some test vectors for wp_ct_* functions? There seem to be no direct tests of these today. Just to ensure coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants