cisco / libsrtp

Library for SRTP (Secure Realtime Transport Protocol)
Other
1.23k stars 476 forks source link

AES-GCM Session Key/Salt Derivation is Incorrect #179

Closed imhotepisinvisible closed 4 years ago

imhotepisinvisible commented 8 years ago

RFC 7714 Sec 11. states that AEAD_AES_128_GCM must use the KDF as defined in RFC 3711 Sec 4.3.1.

IV generation for this KDF is defined in RFC 3711 Sec 4.3.3: IV [is] equal to (x*2^16) where x is the master salt.

The problem arises because the master IV for AES-GCM is 12 bytes, as opposed to 14 bytes for the default AES-CM algorithm specified in RFC 3711. Appendix B.3 has a test vector for IV generation. A 14 byte master salt of 0EC675AD498AFEEBB6960B3AABE6 will result in an IV of 0EC675AD498AFEEBB6960B3AABE60000.

For AES-GCM then, a 12 byte master salt, e.g. 0EC675AD498AFEEBB6960B3A should result in an IV of 00000EC675AD498AFEEBB6960B3A0000. Unfortunately libSRTP uses an incorrect IV of 0EC675AD498AFEEBB6960B3A00000000, leading to an incorrect session key and salt being derived.

paulej commented 8 years ago

As Alexander noted in his reply, changing the current behavior will break existing deployments.

We do need to get clarity on this issue. I hope to see Dave McGrew at the IETF next week. Perhaps I can get his view on this while there.

I can easily see arguments for both ways on this. Section 4.1.1/RFC 3711 describes working with a salt with AES CM which is a 14 octet value. Since AES GCM uses a 12 octet value, two additional octets were needed somewhere. Libsrtp took the option of adding those on the right.

Equally valid given there appears to be no explicit statement on this, the padding could have been on the left.

The specs simply do not state where padding should be added, but padding is definitely need to use that salt with the KDF. At least I cannot find it.

JonathanLennox commented 8 years ago

Paul: this should probably be brought up on the AVTCore mailing list, rather than just hoping to talk to Dave at the IETF. If we have a spec ambiguity that's leading to interoperability failure, it needs higher visibility than just a libsrtp bug report.

paulej commented 8 years ago

I agree it might need to be, but David can certainly tell me if I've overlooked something. If the question goes to the list, I think folks will still look to David for an answer.

paulej commented 8 years ago

I raised this to the AVT list. You can see the message here: https://www.ietf.org/mail-archive/web/avt/current/msg17354.html. Hopefully, I presented the issue properly. Please feel free to respond to correct any misstatement.

paulej commented 8 years ago

The dialog on the IETF "avtcore" mailing list continues. The question raised today is, "how many products are impacted? LibSRTP with AES-GCM is deployed in shipping products, so to change the way the salt is used would be a problem. Are there any other deployments in the field using different logic?

traud commented 8 years ago

Acrobits Groundwire offers AES-GCM since April 2016. In the Apple iOS variant, one enables AES-GCM via Settings → Preferences → Security → (SRTP) SDES. Their implementation aligns the Master Salt to the right and therefore is incompatible with libSRTP: Voice calls produce static noise or silence (authentication failures).

paulej commented 8 years ago

Would you mind responding to the email from Cullen on the AVTCORE mailing list? If you're not subscribed, there's a link: https://www.ietf.org/mail-archive/web/avt/current/msg17373.html. (You can export messages here: https://mailarchive.ietf.org/arch/search/?email_list=avt).

I'll reply, too. Cisco has a few million phones shipped using the current logic. So changing it is going to be a problem.

paulej commented 7 years ago

As a follow-up to this, Cullen posted his opinion on this to the avtcore mailing list. Here is the message: https://mailarchive.ietf.org/arch/msg/avt/-C1cIWQXpyzS2KfBjGR6B2kK92w.

In light of the fact this could break nearly every product out there and no objections to keeping this behavior, I think we can close this issue.

pabuhler commented 4 years ago

@Sean-Der , I am not sure why your post does not show up here but the errata is available here, https://www.rfc-editor.org/errata_search.php?rfc=7714