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

add TLS support #11

Merged
merged 3 commits into from
Feb 18, 2025
Merged

add TLS support #11

merged 3 commits into from
Feb 18, 2025

Conversation

connorkuehl
Copy link
Collaborator

@connorkuehl connorkuehl commented Feb 14, 2025

When adding additional integration tests around dialing over TLS I
learned that the original implementation just simply never ever worked.

The NBD server state machine still expects to begin option negotiation
outside of TLS, one of which must be OPT_STARTTLS in order to upgrade
the existing connection to a secure one, rather than expecting a TLS
handshake and then engaging in option negotation.

I've added support for NBD_OPT_STARTTLS along with a corresponding
integration test.

@connorkuehl connorkuehl changed the title Ck/additional transport tests add TLS support Feb 14, 2025
@connorkuehl connorkuehl force-pushed the ck/additional_transport_tests branch from 9cb16d5 to dc9b86b Compare February 14, 2025 22:04
Current version seems to disagree with both Go 1.24 and the examples
directory.
@connorkuehl connorkuehl force-pushed the ck/additional_transport_tests branch 2 times, most recently from e1762ed to fa3abbf Compare February 14, 2025 22:18
When adding additional integration tests around dialing over TLS I
learned that the original implementation just simply never ever worked.

The NBD server state machine still expects to begin option negotiation
outside of TLS, one of which must be OPT_STARTTLS in order to upgrade
the existing connection to a secure one, rather than expecting a TLS
handshake and then engaging in option negotation.

I've added support for NBD_OPT_STARTTLS along with a corresponding
integration test.
@connorkuehl connorkuehl force-pushed the ck/additional_transport_tests branch from fa3abbf to c2cdc0a Compare February 14, 2025 23:57
@connorkuehl connorkuehl marked this pull request as ready for review February 18, 2025 16:30
@connorkuehl connorkuehl requested a review from a team as a code owner February 18, 2025 16:30
@connorkuehl connorkuehl merged commit 6e36452 into main Feb 18, 2025
7 checks passed
@connorkuehl connorkuehl deleted the ck/additional_transport_tests branch February 18, 2025 20:22
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.

3 participants