Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ot] hw/opentitan: ot_aes: fix IV updates in CFB mode #107

Open
wants to merge 1 commit into
base: ot-earlgrey-9.2.0
Choose a base branch
from

Conversation

luismarques
Copy link

@luismarques luismarques commented Jan 19, 2025

In AES CFB mode after a block operation the IV registers are updated with the ciphertext from the previous block operation. See:

LibTomCrypt does not provide this, so we just read it directly from our own data. Previously, we were using the updated IV value that LibTomCrypt provides, which is instead updated to be the output of the block operation (without the XOR with the plaintext).

This fix allows the aes_functest to pass.

Copy link

@rivos-eblot rivos-eblot left a comment

Choose a reason for hiding this comment

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

Could you also update something I missed from the previous AES MR circa line 441:

    trace_ot_aes_init("iv");
    if (randomize) {
        ot_aes_randomize(s, r->iv, ARRAY_SIZE(r->iv));
    }

with something like

    if (randomize) {
        trace_ot_aes_init("iv randomize");
        ot_aes_randomize(s, r->iv, ARRAY_SIZE(r->iv));
    } else {
        trace_ot_aes_init("iv");
    }

to differentiate both kinds. Thanks

@rivos-eblot
Copy link

Might be worth updating the file header as well?

from

 * Copyright (c) 2022-2024 Rivos, Inc.

to

 * Copyright (c) 2022-2024 Rivos, Inc.
 * Copyright (c) 2025 lowRISC contributors.

or the like

@luismarques luismarques changed the title [ot] hw/opentitan: ot_aes: update IV registers after operation in CFB mode [ot] hw/opentitan: ot_aes: fix IV updates in CFB mode Feb 6, 2025
@luismarques luismarques force-pushed the cfb-iv branch 2 times, most recently from b5c2edb to e8b3509 Compare February 6, 2025 16:12
@luismarques
Copy link
Author

I've checked that OpenSSL returns the expected output IV value, so this seems to be a libtomcrypt oddity.

@rivos-eblot
Copy link

I'll have a look.

No changes in libtomcrypt AES implementation for about 1.5 years.

In AES CFB mode after a block operation the IV registers are updated with
the ciphertext from the previous block operation. See:

- https://opentitan.org/book/hw/ip/aes/doc/theory_of_operation.html#datapath-architecture-and-operation
- https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Cipher_feedback_(CFB)

LibTomCrypt does not provide this, so we just read it directly from our
own data. Previously, we were using the updated IV value that LibTomCrypt
provides, which is instead updated to be the output of the block operation
(without the XOR with the plaintext).

This fix allows the aes_functest to pass.

Signed-off-by: Luís Marques <[email protected]>
@luismarques luismarques changed the base branch from ot-earlgrey-9.1.0 to ot-earlgrey-9.2.0 February 23, 2025 15:18
Copy link

@rivos-eblot rivos-eblot left a comment

Choose a reason for hiding this comment

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

LGTM

@luismarques
Copy link
Author

The QEMU CI is broken. Let's merge this after fixing it and letting it run.

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.

3 participants