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

make MKI a stream level setting #690

Merged
merged 2 commits into from
Feb 13, 2024
Merged

Conversation

pabuhler
Copy link
Member

@pabuhler pabuhler commented Feb 1, 2024

Move "use_mki" from a per packet parameter to a stream wide value. When using MKI all master keys need to have a valid MKI id.

Merge the MKI protect & unprotect functions. The MKI index is currently still passed per packet, this could also be moved up to stream level with a srtp_set_mki(ssrc) api as this is not likely to change per packet.

Move "use_mki" from a per packet parameter to a stream wide value.
When using MKI all master keys need to have a valid MKI id.

Merge the MKI protect & unprotect functions. The MKI index is currently still
passed per packet, this could also be moved up to stream level with a
srtp_set_mki(ssrc) api as this is not likely to change per packet.
@pabuhler pabuhler requested a review from paulej February 1, 2024 10:24
@pabuhler
Copy link
Member Author

pabuhler commented Feb 1, 2024

This started as an attempt to merge the protect & unprotect api's but became a more fundamental change to have MKI as a mode rather than a per packet thing.

The test vector was generated from the 2.5 branch.

include/srtp.h Show resolved Hide resolved
include/srtp_priv.h Show resolved Hide resolved
RFC 3711 section-3.2.1 states that the use of MKI and the size of
the MKI id must be fixed for the life time of stream. By moving
the mki_size value along with mki_use value to the stream level
allows this to be enforced for all keys.
Added a unit test to verify it can not be changed via an update.
{
srtp_err_status_t status = srtp_err_status_ok;
if (p->key != NULL) {
if (p->use_mki) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would a single key be a reason to not use MKI? Perhaps pointless, but illegal?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we will end up removing the single key option in the policy, it was only left there for ABI compatibility when MKI was first added.
It is "illegal" today as the single key option does not contain a mki id.

@pabuhler pabuhler merged commit 9a55910 into cisco:main Feb 13, 2024
33 checks passed
@pabuhler pabuhler deleted the use_mki-move-to-stream branch February 13, 2024 09:10
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