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

Added support for AES CM 256 crypto suites #282

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

sirzooro
Copy link
Contributor

@sirzooro sirzooro commented Jul 8, 2024

Added support for AES_256_CM_HMAC_SHA1_80 and AES_256_CM_HMAC_SHA1_32 crypto suites.

Reference encrypted packets used in tests are the same as generated by libsrtp2.

@sirzooro sirzooro requested a review from at-wat July 8, 2024 22:17
Copy link

codecov bot commented Jul 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.91%. Comparing base (e3e6d11) to head (59e8b1e).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #282      +/-   ##
==========================================
+ Coverage   78.83%   78.91%   +0.08%     
==========================================
  Files          17       17              
  Lines         992      996       +4     
==========================================
+ Hits          782      786       +4     
  Misses        118      118              
  Partials       92       92              
Flag Coverage Δ
go 78.91% <100.00%> (+0.08%) ⬆️
wasm 78.41% <100.00%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sirzooro sirzooro force-pushed the add_support_for_aes_cm_256 branch from c2a74d5 to d87f5d1 Compare July 8, 2024 22:24
@sirzooro sirzooro requested review from adamroach and digitalix July 10, 2024 07:35
Copy link
Member

@at-wat at-wat left a comment

Choose a reason for hiding this comment

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

Looks nice!

Comment on lines 19 to 21
// These are not IANA registered, so used high values to avoid potential conflict
ProtectionProfileAes256CmHmacSha1_80 ProtectionProfile = 0xfff0
ProtectionProfileAes256CmHmacSha1_32 ProtectionProfile = 0xfff1
Copy link
Member

Choose a reason for hiding this comment

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

In the RFC draft,

      SRTPProtectionProfile SRTP_AES256_CM_SHA1_80 = {0x00, 0x03};
      SRTPProtectionProfile SRTP_AES256_CM_SHA1_32 = {0x00, 0x04};

https://datatracker.ietf.org/doc/html/draft-ietf-avt-dtls-srtp-03#section-4.1.2

libsrtp in chromium code seems also using them: https://chromium.googlesource.com/chromium/deps/libsrtp/+/84122798bb16927b1e676bd4f938a6e48e5bf2fe/srtp/include/srtp.h#694

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for info! Updated this in code.

Added support for AES_256_CM_HMAC_SHA1_80 and AES_256_CM_HMAC_SHA1_32
crypto suites.
@sirzooro sirzooro force-pushed the add_support_for_aes_cm_256 branch from d87f5d1 to 59e8b1e Compare July 11, 2024 15:33
Copy link
Member

@at-wat at-wat left a comment

Choose a reason for hiding this comment

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

LGTM

@sirzooro sirzooro merged commit 111a7b0 into pion:master Jul 12, 2024
14 checks passed
@sirzooro sirzooro deleted the add_support_for_aes_cm_256 branch July 12, 2024 07:52
@sirzooro sirzooro mentioned this pull request Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants