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

Fix: rendezvous: do not reject incoming packet with id=0 when rendezvous #2548

Merged
merged 2 commits into from Nov 25, 2022

Conversation

ethouris
Copy link
Collaborator

@ethouris ethouris commented Nov 22, 2022

Fixes #2539

@codecov-commenter
Copy link

codecov-commenter commented Nov 22, 2022

Codecov Report

Merging #2548 (dc77986) into master (be1ccf5) will decrease coverage by 0.15%.
The diff coverage is 57.14%.

@@            Coverage Diff             @@
##           master    #2548      +/-   ##
==========================================
- Coverage   66.37%   66.21%   -0.16%     
==========================================
  Files         100      100              
  Lines       19772    19778       +6     
==========================================
- Hits        13123    13096      -27     
- Misses       6649     6682      +33     
Impacted Files Coverage Δ
srtcore/queue.cpp 81.15% <57.14%> (+1.00%) ⬆️
srtcore/core.h 72.15% <0.00%> (-6.33%) ⬇️
srtcore/cache.h 80.00% <0.00%> (-3.08%) ⬇️
srtcore/core.cpp 60.39% <0.00%> (-0.73%) ⬇️
srtcore/list.cpp 81.84% <0.00%> (-0.55%) ⬇️
haicrypt/hcrypt.c 79.87% <0.00%> (-0.39%) ⬇️
test/test_socketdata.cpp 100.00% <0.00%> (+6.89%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ethouris ethouris added Type: Bug Indicates an unexpected problem or unintended behavior [core] Area: Changes in SRT library core labels Nov 22, 2022
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.

Minor suggestion.

srtcore/queue.cpp Outdated Show resolved Hide resolved
Co-authored-by: Maxim Sharabayko <maxlovic@gmail.com>
@maxsharabayko
Copy link
Collaborator

Wow, segmentation fault. Not sure if it is related to the changes in the PR.

[ RUN      ] TestEnforcedEncryption.CASE_D_4_NonBlocking_Enforced_Off_Off_Pwd_None_Set
09:05:31.484224/SRT:RcvQ:w75!W:SRT.cn: @254575997: HS EXT: Agent declares encryption, but Peer does not (Agent can still receive unencrypted packets from Peer).
09:05:31.485487/SRT:RcvQ:w75!W:SRT.cn: @254575997: createSrtHandshake: Agent has PW, but Peer sent no KMREQ. Sending error KMRSP response
Epoll wait result: 1 FOUND: @254575998 in [R]
09:05:31.485936/SRT:RcvQ:w76!W:SRT.cn: @254575999: HS KMREQ: Peer declares encryption, but agent does not - still allowing connection.
09:05:31.485958/SRT:RcvQ:w76!W:SRT.cn: processSrtMsg_KMRSP: received failure report. STATE: NOSECRET
ACCEPT: done, result=254575997
[T] ACCEPT SUCCEEDED: @254575997
/home/travis/.travis/functions: line 109:  7067 Segmentation fault      ./test-srt --gtest_filter="-$TESTS_IPv6"

@ethouris
Copy link
Collaborator Author

Well, that's kinda weird. I tried this now and no problems are reported on the local machine. I also haven't changed anything that would influence this flow. This might influence the handshake processing on the listener side, but here in the test the segfault is reported after the accept call has succeeded on both endpoints, so it's after the handshake. Worth researching, but I doubt we can retrieve enough data from Travis.

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 Type: Bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rendezvous Connection in the Non Blocking Mode
3 participants