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

Introduce T_COSE_OPT_ENABLE_NON_AEAD #284

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

kentakayama
Copy link
Contributor

@kentakayama kentakayama commented Jul 29, 2024

This PR mitigates AEAD to non-AEAD downgrade attacks by warning the library uses to comply with RFC 9459.
Non AEAD algorithms are disabled by default, and will be enabled only when the T_COSE_OPT_ENABLE_NON_AEAD option is specified by the library caller.

The library callers using AES-CTR or AES-CBC will lose the backward compatibility, but it is crucial avoiding them to be vulnerable.
For example, they must be updated from

t_cose_encrypt_dec_init(&dec_context,
                         T_COSE_OPT_MESSAGE_TYPE_ENCRYPT0);

to

/**
 * NOTE: The library caller MUST validate the authentication and integrity of data by itself outside t_cose library
 * because the COSE message doesn't provide them with non-AEAD content encryption algorithms.
 * See security considerations of RFC 9459.
 */
validate_authentication_and_integrity(cose_message_buf, digital_signature);

t_cose_encrypt_dec_init(&dec_context,
                         T_COSE_OPT_MESSAGE_TYPE_ENCRYPT0 | T_COSE_OPT_ENABLE_NON_AEAD);

…PT_ENABLE_NON_AEAD to avoid unintentional use of them
@laurencelundblade
Copy link
Owner

I'm going to hold on this for a few days to see the outcome of the errata proposal separately in email.

Thanks! Really appreciate the contribution

Copy link
Owner

@laurencelundblade laurencelundblade left a comment

Choose a reason for hiding this comment

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

Really appreciate the PR and like the design (and test coverage!)

Would like to add a specific error code (error codes cost nothing) to help the caller figure out better what went wrong and to find the solution.

Also, some wording changes. Maybe discuss in email instead of here.

Thx!

src/t_cose_encrypt_enc.c Outdated Show resolved Hide resolved
inc/t_cose/t_cose_encrypt_dec.h Outdated Show resolved Hide resolved
inc/t_cose/t_cose_encrypt_enc.h Outdated Show resolved Hide resolved
inc/t_cose/t_cose_common.h Outdated Show resolved Hide resolved
inc/t_cose/t_cose_common.h Outdated Show resolved Hide resolved
Copy link
Contributor Author

@kentakayama kentakayama left a comment

Choose a reason for hiding this comment

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

I've reflected all your comments. Please check the updated files.

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