Skip to content

Fix for AES-CFB1 encrypt/decrypt on size (8*x-1) bits#7431

Merged
JacobBarthelmeh merged 1 commit intowolfSSL:masterfrom
lealem47:aes_cfb
Apr 19, 2024
Merged

Fix for AES-CFB1 encrypt/decrypt on size (8*x-1) bits#7431
JacobBarthelmeh merged 1 commit intowolfSSL:masterfrom
lealem47:aes_cfb

Conversation

@lealem47
Copy link
Copy Markdown
Contributor

@lealem47 lealem47 commented Apr 16, 2024

Description

Prior to this commit, AES-CFB1 encrypt/decrypts of size (8*x-1) bits would leave the last 7 bits of the input the same as the output

Testing

SRTP-KDF harness

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@lealem47 lealem47 self-assigned this Apr 16, 2024
@kaleb-himes kaleb-himes requested a review from SparkiDev April 16, 2024 18:44
@kaleb-himes
Copy link
Copy Markdown
Contributor

Great find @lealem47! Adding @SparkiDev for visibility.

@lealem47
Copy link
Copy Markdown
Contributor Author

Great find @lealem47! Adding @SparkiDev for visibility.

All credit to @JacobBarthelmeh

@lealem47
Copy link
Copy Markdown
Contributor Author

Retest this please

@lealem47 lealem47 assigned wolfSSL-Bot and unassigned lealem47 Apr 16, 2024
Copy link
Copy Markdown
Contributor

@JacobBarthelmeh JacobBarthelmeh left a comment

Choose a reason for hiding this comment

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

Thanks for adding in the test cases!

Comment thread wolfcrypt/src/aes.c Outdated
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.

Reviewing this more, I don't think there is a case here where 'sz' does not equal 0 once it hits this check? Having broken out of the while loop (sz > 0) and 'ret' is 0. Think the bounds check on 'bit' might just need to be inclusive with 0 if ((bit >= 0 && bit < 7)) { . Cases where 'bit' is 7 then out[0] is updated in the while loop and all other cases out[0] should be updated with the last value of cur after exiting the while loop.

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.

Just check bit < 7.
7 indicates no bits encrypted and not outputted.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread wolfcrypt/src/aes.c Outdated
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.

Just check bit < 7.
7 indicates no bits encrypted and not outputted.

@lealem47
Copy link
Copy Markdown
Contributor Author

Retest this please

@JacobBarthelmeh JacobBarthelmeh merged commit 69be7a7 into wolfSSL:master Apr 19, 2024
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.

5 participants