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

Introduced ENABLE_ENCRYPTION flag, ON by default. #670

Merged
merged 5 commits into from
May 16, 2019

Conversation

ethouris
Copy link
Collaborator

Fixes #666

This introduces the compile option for encryption (cmake: SRT_ENABLE_ENCRYPTION=OFF, configure: --disable-encryption). When used:

  • haicrypt subproject is not added to SRT
  • SSL library dependencies are off
  • setting a password or pbkeylen is not enabled (setting an empty password is tolerated and ignored)
  • strict encryption works as before - when off, it allows connections from a password-requiring site

@ethouris ethouris requested review from maxsharabayko and rndi April 30, 2019 06:43
@maxsharabayko maxsharabayko added this to the v.1.3.3 milestone Apr 30, 2019
@maxsharabayko maxsharabayko added the [core] Area: Changes in SRT library core label Apr 30, 2019
Copy link
Collaborator

@maxsharabayko maxsharabayko left a comment

Choose a reason for hiding this comment

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

@ethouris Can you please check that when two peers have different encryption build definition, the expected behavior is respected.
E.g. two srt-live-transmit instances. One built with encryption enabled, the second- with the encryption disabled.
Caller with encryption enabled. Listener with encryption disabled.

Caller Listener Connection
no strict no pwd no strict established
no strict no pwd strict established
no strict pwd no strict established
no strict pwd strict rejected

configure-data.tcl Outdated Show resolved Hide resolved
srtcore/core.cpp Show resolved Hide resolved
srtcore/core.cpp Show resolved Hide resolved
srtcore/core.cpp Show resolved Hide resolved
@ethouris
Copy link
Collaborator Author

ethouris commented May 2, 2019

@ethouris Can you please check that when two peers have different encryption build definition, the expected behavior is respected.
E.g. two srt-live-transmit instances. One built with encryption enabled, the second- with the encryption disabled.
Caller with encryption enabled. Listener with encryption disabled.

Caller Listener Connection
no strict no pwd no strict established
no strict no pwd strict established
no strict pwd no strict established
no strict pwd strict rejected

According to what I have tested, the only case when it matters is when you have one side with encryption supported and configured, and the other side with encryption unsupported. The current state is that with such a combination you need to have both sides strict encryption off, otherwise the connection is rejected.

@maxsharabayko
Copy link
Collaborator

@ethouris

According to what I have tested,

Please specify which cases you've tested.

@ethouris
Copy link
Collaborator Author

ethouris commented May 2, 2019

@ethouris

According to what I have tested,

Please specify which cases you've tested.

  1. Listener/NOENC vs. Caller/ENC+password - all combinations of STRICTENC flag.
  2. Caller/NOENC vs. Listener/ENC+password - asabov
  3. Any NOENC+password -> error in the beginning, as expected

@rndi rndi merged commit ed67060 into Haivision:master May 16, 2019
@hgs-gmicros
Copy link

Disabling the encryption throws an error and ends the build configuration.

Running: ./configure --disable-encryption

Throws the following error:
No SOURCES given to target: haicrypt_virtual

It appears that this issue should not be labeled as DONE. Not sure is it requires a separate issue.

@maxsharabayko
Copy link
Collaborator

Hi @hgs-gmicros
Thanks for reporting. Could you please create an issue for that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[core] Area: Changes in SRT library core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add build option to disable encryption
4 participants