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

AES-192 (ICM) approach B #170

Closed
wants to merge 2 commits into from
Closed

AES-192 (ICM) approach B #170

wants to merge 2 commits into from

Conversation

traud
Copy link
Contributor

@traud traud commented Jun 15, 2016

In the original libSRTP (without OpenSSL), support for AES-256 was added in May 2010 with commit 5df951a, but AES-192 was not implemented (see crypto/cipher/aes.c:aes_expand_encryption_key). Since 2013 with OpenSSL enabled (see pull request #34), libSRTP supports AES-192 as well. Or stated differently:

If you want the crypto suite AES_192_CM_HMAC_SHA1_80 in your VoIP application, you have to go for
./configure --enable-openssl
otherwise, just AES-256 and AES-128 work. As a side-effect, you get AES-NI and AES-GCM.

Even with the current source code, AES-192 works only between VoIP applications which use the same library (for example locally on the same computer). I did not squash the two commits on purpose, hopefully to make the overall pull request easier to understand.

This is approach/alternative B to solve this issue. Because I am just a contributor and not a member of the team, I cannot decide whether the approach A or B is the correct one. If you do not like the other approach A, please, consider this simpler change-set here for inclusion, especially because several VoIP/SIP clients leverage AES-192 already (like Acrobits Groundwire and Media5-fone).

enable AES-192 and AES-256 for SRTCP
allow AES_256_ICM as policy cipher_type
prominent examples like jfigus/PJSIP@eb1b22c suggest that usage, therefore allow it
backport of b8cb577
@pabuhler
Copy link
Member

Prefer this approach as it is more consistent with rest of code for now.
Also would prefer it was against master, any reason for 1.5.x?

@pabuhler
Copy link
Member

Hi @traud , any chance of get this this against master? We are planing on a 2.1 release and would be good to get it in. If I don't hear back soon I might try to apply to master myself.

@pabuhler pabuhler added this to the Version 2.1.0 milestone Jan 25, 2017
@paulej
Copy link
Contributor

paulej commented Jan 28, 2017

It would be good if test code was added for AES-192. There is a test for 256 (srtp_validate_aes_256()) and having a version for 192 would be good.

pabuhler pushed a commit to pabuhler/libsrtp that referenced this pull request Feb 21, 2017
@pabuhler
Copy link
Member

This has been applied to master in #253 as 4a74b82, if that commit is satisfactory then this should be closed.

@traud
Copy link
Contributor Author

traud commented Feb 26, 2017

Fine. There should be a release note that AES-192 only works with libSRTP 2.1 or newer; and only when OpenSSL was configured. Furthermore, there should be a note with the next libSRTP 1.5.x that AES-192 does not work with that.

@traud traud closed this Feb 26, 2017
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.

None yet

3 participants