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 DN name to accept_result struct #20

Closed
wants to merge 1 commit into from

Conversation

Lazin
Copy link

@Lazin Lazin commented Apr 21, 2022

Previously, it was possible to get DN using dn_callback passed to
credentials object. This callback is invoked on every handshake (if
'client_auth' is set to true) and accepts two parameters, subject and
issuer. But there is no way for the user of the tls library to connect
particular client connection with the DN string. Also, it's not the best
way to obtain the DN string because the handshake is delayed until the
first read or write attempt is made.

This commit solves the issue by

  • adding a DN field to the accept_result struct
  • propagating DN name to the accept_result
  • forcing TLS handshake after the connection is established and before
    the accept_result is returned in order to get DN field populated

In order for this mechanism to work the user must use 'seastar::tls::listen'
method directly and not 'wrap_server'. Also, the user have to pass
the credentials with client-auth set to 'REQUIRED' to the 'listen'
function.

@CLAassistant
Copy link

CLAassistant commented Apr 21, 2022

CLA assistant check
All committers have signed the CLA.

@BenPope BenPope self-requested a review April 21, 2022 09:05
Copy link
Member

@BenPope BenPope left a comment

Choose a reason for hiding this comment

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

This looks like it will work.

If the handshake is more eager, does this make an exhaustion attack more easy?

Are there concerns over when the set_sockopt can be called? I.e., are there scenarios where socket options must be set before the handshake?

src/net/tls.cc Outdated
_creds->_dn_callback(t, subject, issuer);
}
if (fetch_dn) {
return session_dn{.subject=subject, .issuer=issuer};
Copy link
Member

Choose a reason for hiding this comment

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

subject and issuer can be moved.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed that

Previously, it was possible to get DN using dn_callback passed to
credentials object. This callback is invoked on every handshake (if
'client_auth' is set to true) and accepts two parameters, subject and
issuer. But there is no way for the user of the tls library to connect
particular client connection with the DN string. Also, it's not the best
way to obtain the DN string because the handshake is delayed until the
first read or write attempt is made.

This commit solves the issue by
- adding a DN field to the accept_result struct
- propagating DN name to the accept_result
- forcing TLS handshake after the connection is established and before
  the accept_result is returned in order to get DN field populated

In order for this mechanism to work the user must use 'seastar::tls::listen'
method directly and not 'wrap_server'. Also, the user have to pass
the credentials with client-auth set to 'REQUIRED' to the 'listen'
function.
@Lazin Lazin force-pushed the feature/pass-dn-on-accept branch from 8f9ec17 to b76092e Compare April 21, 2022 12:46
@Lazin
Copy link
Author

Lazin commented Apr 21, 2022

If the handshake is more eager, does this make an exhaustion attack more easy?

I'm curious as well. Normally TLS exhaustion attack will actually require a handshake anyway. Here we're just starting the handshake as soon as connection is established. Without this the attacker can open a lot of connection without doing the handshake. So in my point of view this looks like reduction of the attack surface.

Are there concerns over when the set_sockopt can be called? I.e., are there scenarios where socket options must be set before the handshake?

I don't think that there are any concerns regarding this.

@dotnwat dotnwat requested review from dotnwat and jcsp April 21, 2022 15:07
@Lazin Lazin closed this Apr 29, 2022
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