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] TLS: Handshake First for client connections #4642

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

kozlovic
Copy link
Member

@kozlovic kozlovic commented Oct 9, 2023

A new option instructs the server to perform the TLS handshake first, that is prior to sending the INFO protocol to the client.

Only clients that implement equivalent option would be able to connect if the server runs with this option enabled.

The configuration would look something like this:

...
tls {
    cert_file: ...
    key_file: ...

    handshake_first: true
}

The same option can be set to "auto" or a Go time duration to fallback to the old behavior. This is intended for deployments where it is known that not all clients have been upgraded to a client library providing the TLS handshake first option.

After the delay has elapsed without receiving the TLS handshake from the client, the server reverts to sending the INFO protocol so that older clients can connect. Clients that do connect with the "TLS first" option will be marked as such in the monitoring's Connz page/result. It will allow the administrator to keep track of applications still needing to upgrade.

The configuration would be similar to:

...
tls {
    cert_file: ...
    key_file: ...

    handshake_first: auto
}

With the above value, the fallback delay used by the server is 50ms.

The duration can be explcitly set, say 300 milliseconds:

...
tls {
    cert_file: ...
    key_file: ...

    handshake_first: "300ms"
}

It is understood that any configuration other that "true" will result in the server sending the INFO protocol after the elapsed amount of time without the client initiating the TLS handshake. Therefore, for administrators that do not want any data transmitted in plain text, the value must be set to "true" only. It will require applications to be updated to a library that provides the option, which may or may not be readily available.

Signed-off-by: Ivan Kozlovic [email protected]

@kozlovic kozlovic requested a review from a team as a code owner October 9, 2023 22:10
@kozlovic kozlovic requested a review from derekcollison October 9, 2023 22:10
@kozlovic kozlovic marked this pull request as draft October 10, 2023 12:25
@kozlovic
Copy link
Member Author

@derekcollison I need to revisit something so I converted the PR to draft so it doesn't get merged as-is. I will switch it back to opened when I am ready with the changes. Thanks!

A new option instructs the server to perform the TLS handshake first,
that is prior to sending the INFO protocol to the client.

Only clients that implement equivalent option would be able to
connect if the server runs with this option enabled.

The configuration would look something like this:
```
...
tls {
    cert_file: ...
    key_file: ...

    handshake_first: true
}
```

The same option can be set to "auto" or a Go time duration to fallback
to the old behavior. This is intended for deployments where it is known
that not all clients have been upgraded to a client library providing
the TLS handshake first option.

After the delay has elapsed without receiving the TLS handshake from
the client, the server reverts to sending the INFO protocol so that
older clients can connect. Clients that do connect with the "TLS first"
option will be marked as such in the monitoring's Connz page/result.
It will allow the administrator to keep track of applications still
needing to upgrade.

The configuration would be similar to:
```
...
tls {
    cert_file: ...
    key_file: ...

    handshake_first: auto
}
```
With the above value, the fallback delay used by the server is 50ms.

The duration can be explcitly set, say 300 milliseconds:
```
...
tls {
    cert_file: ...
    key_file: ...

    handshake_first: "300ms"
}
```

It is understood that any configuration other that "true" will result
in the server sending the INFO protocol after the elapsed amount of
time without the client initiating the TLS handshake. Therefore, for
administrators that do not want any data transmitted in plain text,
the value must be set to "true" only. It will require applications
to be updated to a library that provides the option, which may or
may not be readily available.

Signed-off-by: Ivan Kozlovic <[email protected]>
@kozlovic kozlovic marked this pull request as ready for review October 10, 2023 16:03
@kozlovic
Copy link
Member Author

@derekcollison Ok, this is ready for review now.

@Jarema
Copy link
Member

Jarema commented Oct 11, 2023

@kozlovic @derekcollison Tested with non-Go client (Rust) and both TLS-first options work (with and without auto mode) as expected.

I will work on the client-side ADR.

@Jarema
Copy link
Member

Jarema commented Oct 16, 2023

@derekcollison @kozlovic can we merge this? Otherwise clients need to depend on a server branch to run tests against this feature.

related ADR spec containing tls first is here:
nats-io/nats-architecture-and-design#247

@derekcollison
Copy link
Member

@kozlovic could you review the client ADR?

Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM

@derekcollison derekcollison merged commit 4df6c9a into main Oct 16, 2023
@derekcollison derekcollison deleted the client_tls_first branch October 16, 2023 14:49
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