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

Block Wildcard Address Client Connections #3483

Merged
merged 5 commits into from
Mar 21, 2023

Conversation

nibanks
Copy link
Member

@nibanks nibanks commented Mar 7, 2023

Description

MsQuic was allowing a client to "connect" to 0.0.0.0 which the stack actually just treats as a wildcard (unconnected) socket.

Testing

Existing CI

Documentation

We don't currently document whether this was allowed or disallowed. Maybe we should be explicit there?

@nibanks nibanks requested a review from a team as a code owner March 7, 2023 19:10
@nibanks nibanks added Area: API Area: Core Related to the shared, core protocol logic labels Mar 7, 2023
@jdcole
Copy link
Contributor

jdcole commented Mar 7, 2023

For the history book, this issue was discovered running quicsample -client -unsecure -target:0. Thanks for the fix @nibanks!

@ami-GS
Copy link
Contributor

ami-GS commented Mar 8, 2023

required test is failing?

[03/08/2023 12:32:33] Process had nonzero exit code: 1
##vso[task.LogIssue type=error;][03/08/2023 12:32:33] ParameterValidation.ValidateConnectionParam failed:
Note: Google Test filter = ParameterValidation.ValidateConnectionParam
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
Initializing for User Mode tests
[----------] 1 test from ParameterValidation
[ RUN      ] ParameterValidation.ValidateConnectionParam
/__w/1/s/src/test/lib/ApiTest.cpp:3373: Failure
Connection.SetParam( QUIC_PARAM_CONN_REMOTE_ADDRESS, sizeof(Dummy), &Dummy) failed, 0x[16](https://github.com/microsoft/msquic/actions/runs/4364285556/jobs/7631421949#step:8:17)
[  FAILED  ] ParameterValidation.ValidateConnectionParam ([20](https://github.com/microsoft/msquic/actions/runs/4364285556/jobs/7631421949#step:8:21)5 ms)
[----------] 1 test from ParameterValidation (205 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (366 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ParameterValidation.ValidateConnectionParam

 1 FAILED TEST

@nibanks
Copy link
Member Author

nibanks commented Mar 8, 2023

required test is failing?

[03/08/2023 12:32:33] Process had nonzero exit code: 1
##vso[task.LogIssue type=error;][03/08/2023 12:32:33] ParameterValidation.ValidateConnectionParam failed:
Note: Google Test filter = ParameterValidation.ValidateConnectionParam
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
Initializing for User Mode tests
[----------] 1 test from ParameterValidation
[ RUN      ] ParameterValidation.ValidateConnectionParam
/__w/1/s/src/test/lib/ApiTest.cpp:3373: Failure
Connection.SetParam( QUIC_PARAM_CONN_REMOTE_ADDRESS, sizeof(Dummy), &Dummy) failed, 0x[16](https://github.com/microsoft/msquic/actions/runs/4364285556/jobs/7631421949#step:8:17)
[  FAILED  ] ParameterValidation.ValidateConnectionParam ([20](https://github.com/microsoft/msquic/actions/runs/4364285556/jobs/7631421949#step:8:21)5 ms)
[----------] 1 test from ParameterValidation (205 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (366 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ParameterValidation.ValidateConnectionParam

 1 FAILED TEST

Is fixed (down-level) by #3487. We will need to take that first and make a new release to fix the test.

@nibanks nibanks merged commit b4819f2 into main Mar 21, 2023
@nibanks nibanks deleted the nibanks/block-wildcard-addr-connect branch March 21, 2023 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: API Area: Core Related to the shared, core protocol logic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants