Is anyone working on MKI support?

[MartinBonner]

I am interesting in MKI support. Does anyone have any working code that supports this?
Or even sketches of what the changes should look like? (Yes, yes. I know I can fork the repo and make the changes myself, but I'm lazy - I'd rather use someone else's work.)

Is there a more appropriate place to ask a question like this? The Sourceforge mailing list seems full of tumbleweed.

[paulej]

I'm not aware of anyone working on that. I think if you want it, you might have to write it yourself. :)

[ulfolsson]

We (Axis Communications) have added MKI (in version 1.5.4) support in a prototype project. We are interested to upstream this but I'm not sure how to proceed + there are some issues to discuss

[ulfolsson]

Let me first describe our use case. We are manufacturing IP cameras so we are mostly sending SRTP data, receiving packages are limited to a few SRTCP RR:s per minute. The clients that connects to our products consists mostly of VMS:es (Video Management Systems) that are using RTSP/TLS to setup the stream(s) (server using the SDP attribute key-mgmt and the client using RTSP SETUP KeyMgmt to exchange the SRTP information).

We also want the VMS to control when the master key shall be changed so the number of keys that will used during a session are unknown when the streams starts, except for the initial ones. The change of the key can for an example be triggered by a MIKEY message contained in a RTSP SET_PARAMETER request.

When the key is changed it is easy a for a sender to just switch the key and the associated MKI value and continue from there. But for the receiver it is more complicated since it have to keep track of at least to two keys for a while (since the UDP packets can come out of order)

We have tested different MKI solutions. Our starting point have been that we wanted to keep the changes in libsrtp to a minimum + libsrtp should not have to know so much about the key management at all.

Tests implementations that we have done so far are;

1.

A very simplistic approach leaving most of the work to the caller, that is adding protect and unprotect functions that just insert or parse out the MKI value to/from a RTP packet, i.e

srtp_protect_with_mki(..., mki_len, mki_to_use)

srtp_unprotect_with_mki(..., mki_len, mki_parsed_from_packet)

and letting srtp_stream_init_keys to handle the different keys

Add MKI as part of the SRTP policy in different flavors, basically the same thing a couple of other people have tried to do in the past, see https://sourceforge.net/p/srtp/mailman/srtp-development/thread/3F7EE7F9-57CC-4992-B32F-858F2616BE24%40cisco.com/#msg2084622.

Anyway we are open for suggestions :)

[darkangel-ua]

We have adopted Per Cederqvist patch for 2.0.0 and tested with Microsoft Lync - it works :) The patch itself adds support only for ONE key. I see only one downside with this patch - it changes user level API.

It is possible to modify patch in the way it will not touch srtp_policy_t::key member and users will not be forced to change code.

Paul, do you interested to add such reduced MKI support to libsrtp?

[paulej]

Given there are at least two or three people (and companies) interested in having MKI support, it's probably good to do. I'm favorable to not requiring changes to APIs, but appreciate that's sometimes necessary. The best thing is to offer something and have a few people comment on the changes.

Either way, any PR submitted will get reviewed by the team. It's a slow process sometimes, because everyone is full time on something else.

[darkangel-ua]

Then I will try to prepare PR without user API changes. Thanks!

[paulej]

We're going to target a 2.1 release for the end of January. Would we have the MKI work done before then?

[darkangel-ua]

I hope yes. I'm planning to finish work this week.

[ulfolsson]

Sounds good. We will wait for your patch then. Have you tested to handle more than one key?

[darkangel-ua]

It cannot handle more than one key by design as I understand from code. But that was enough for our case - MS Lync.

Maybe there is a easy way to add support for handling multiple keys, but this task for more advanced libsrtp programmer :)

[rsith71]

In the recent month I have worked on an MKI patch that supports Multiple keys. I see the PR is for only one Key support. I understand the issue about adding/modifying APIs. To prevent change all the Unit Test I renamed the existing SRTP/SRTCP Protect/Unprotect function to take in MKI information. I then kept the original SRTP/SRTCP Protect/Unprotect functions that called my new MKI versions with MKI support turned off. I have unit test working in libSRTP. The product that is using the MKI patch old SRTP stack supported MKI and there where Unit Test at that level two which are passing.

If you want I can share my patch. It may not be up to libSRTP standards but there is nothing a good code review cannot fix.

[ulfolsson]

<If you want I can share my patch Yes please. We (Axis) would prefer multiple keys support

[rsith71]

I'll create a PR from my patch. The patch was for version 2.0.0 and I know there are some conflict issues with master. I'll start bringing the patch overt to master and create a PR by the end of the week.

[paulej]

Working on adding EKT support to SRTP, we introduced this API call in the 2_0_0_ekt_dev branch: https://github.com/cisco/libsrtp/blob/2_0_0_ekt_dev/srtp/srtp.c#L2234. I think something like that could be employed, if that was your thinking @rsith71. What we did is have the original srtp_protect() call this new function. So, if you have an srtp_protect_with_mki(), you could have the original srtp_protect() call srtp_protect_with_mki(), but pass NULL for the MKI value (to maintain current behavior). Was that the thinking?

[rsith71]

I create a PR for Multiple MKI support. https://github.com/cisco/libsrtp/pull/224

It handles up to four Master keys. The reason for this is the stack we where coming from only handled up to four Master keys.

[ulfolsson]

Couldn't srtp_session_keys_t session_keys[SRTP_MAX_NUM_MASTER_KEYS]; be a linked list instead? Then you don't have know the number of keys that will used during a session

[jesup]

If this was c++, perhaps a . In C we could have it as a (reallocated-as-needed) array ptr (master keys aren't added that often!), or a linked-list (if we think it will change often or need to remove/insert items in the middle often (and the list isn't short).

Overall - I think realloc is the simplest and lowest-overhead for the common cases

[rsith71]

I updated the PR to allow for more dynamic support of MKI keys.

[pabuhler]

Now that #224 is merge, can this issue be closed?

[MartinBonner]

@pabuhler Yes, this issue can be closed.

I did actually go the route of making changes myself. Sadly, I have gone for a rather different model which enables an indefinite stream of master keys, which is going to make an eventual merge painful. I am also not currently at liberty to share the code with the wider community :-( (I will keep pushing ...)