Skip to content

Conversation

pabuhler
Copy link
Member

The kdf for AES 192 is currently broken, it mistakenly uses the kdf in AES 256 mode as described in #763.

First will commit a failing test and then the fix.

The test vectors are generated based on RFC 6188
section 7.4 . It demonstrates that the kdf for
AES 192 is currently broken. It mistakenly uses the
kdf in AES 256 mode as described in cisco#763.
Set the kdf key length to be the determined input key length.
This will set it correctly for AES ICM 128/192/256.
In the case of AES GCM 128 & 256 it needs to be increased
by 2 to match the corresponding AES ICM cipher.

fixes cisco#763

srtp_err_status_t srtp_validate_aes_256(void);

#ifdef GCM

Choose a reason for hiding this comment

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

I don't think this ifdef is needed here, you may want to run this test even when GCM is not enabled

Copy link
Member Author

Choose a reason for hiding this comment

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

The GCM is miss used in this context, it indicates that the crypto engine is external as there is no internal implementation of AES 192 :( ... this should also be changed at some point.

Choose a reason for hiding this comment

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

sorry I missed that trick.

Copy link
Contributor

@paulej paulej left a comment

Choose a reason for hiding this comment

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

LGTM

@pabuhler pabuhler merged commit 8016607 into cisco:main Oct 2, 2025
39 checks passed
pabuhler added a commit to pabuhler/libsrtp that referenced this pull request Oct 2, 2025
Back ported cisco#765 from main.
fixes cisco#763
@pabuhler pabuhler mentioned this pull request Oct 2, 2025
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