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 SSL_VERIFY_CLIENT_ONCE (1.0.2) #1984

Conversation

mattcaswell
Copy link
Member

@mattcaswell mattcaswell commented Nov 22, 2016

Checklist
  • documentation is added or updated
  • tests are added or updated
  • CLA is signed
Description of change

The flag SSL_VERIFY_CLIENT_ONCE is documented as follows:

  B<Server mode:> only request a client certificate on the initial TLS/SSL
  handshake. Do not ask for a client certificate again in case of a
  renegotiation. This flag must be used together with SSL_VERIFY_PEER.

  B<Client mode:> ignored

But the implementation actually did nothing. After the server sends its
ServerKeyExchange message, the code was checking s->session->peer to see if
it is NULL. If it was set then it did not ask for another client
certificate. However s->session->peer will only be set in the event of a
resumption, but a ServerKeyExchange message is only sent in the event of a
full handshake (i.e. no resumption).

The documentation suggests that the original intention was for this to
have an effect on renegotiation, and resumption doesn't come into it.

The fix is to properly check for renegotiation, not whether there is already
a client certificate in the session.

As far as I can tell this has been broken for a long time.

This is the one bug fixed in #1982 and #1983 which also impacts 1.0.2

@mattcaswell mattcaswell added the branch: 1.0.2 Merge to OpenSSL_1_0_2-stable branch label Nov 22, 2016
@dot-asm
Copy link
Contributor

dot-asm commented Nov 27, 2016

It's all very confusing. I mean starting with documentation. Or in other words is it possible that it's bug in documentation? Or misconception? You also refer to ServerKeyExchange message, but what does it have to do with client authentication? You say that peer is set on resumption, but client authentication is actually option on resumption. I mean if you resume session, then you don't have an option to ask for client certificate, do you? And if you renegotiate, then you go through complete handshake. One can also wonder if it's actually sane to provide such option. Indeed, if man-in-the-middle highjacks connection and initiates renegotiation, why should server award renegotiated context original client identity?

@dot-asm
Copy link
Contributor

dot-asm commented Nov 27, 2016

You say that peer is set on resumption, but client authentication is actually option on resumption.

I naturally meant "client authentication is not actually an option on resumption", as it should have been obvious from next sentence.

@mattcaswell
Copy link
Member Author

It's all very confusing. I mean starting with documentation. Or in other words is it possible that it's bug in documentation? Or misconception?

Yes, that is always possible. But I can't think of another interpretation of what this option might have been intended to do. The description provided in the documentation makes perfect sense to me as to something you might want to do.

You also refer to ServerKeyExchange message, but what does it have to do with client authentication?

Only that sending of the CertificateRequest comes immediately after sending the ServerKeyExchange in the state machine. So perhaps my original description would have been clearer if, instead of saying "After the server sends its ServerKeyExchange message", I had said "Before the server sends its CertificateRequest message"

You say that peer is set on resumption, but client authentication is not actually an option on resumption. I mean if you resume session, then you don't have an option to ask for client certificate, do you?

No, but if you did client-auth in the original handshake then the cert gets saved away in the session, and gets reinstated from the session when you resume. So, if you did client-auth in the original handshake, then in a resumption handshake s->session->peer is set.

And if you renegotiate, then you go through complete handshake. One can also wonder if it's actually sane to provide such option. Indeed, if man-in-the-middle highjacks connection and initiates renegotiation, why should server award renegotiated context original client identity?

But it doesn't? This option is only about whether a certificate request gets sent on a reneg handshake or not, i.e. if you set the option then you only get a CertificateRequest on the original handshake not on any subsequent renegs. The client identity from the original handshake does not get carried forward to the new one in either case.

@dot-asm
Copy link
Contributor

dot-asm commented Nov 28, 2016

As I said from the start, it's all very confusing. What is this option supposed to mean? Let's consider common usage pattern. Normally you'd demand client authentication to protect a resource, right? Provided that application is oblivious to renegotiation, would it be appropriate to offer an option not to ask for client authentication on initially authenticated connection? What if rogue renegotiation takes place in between decision to send sensitive data and actual transmission of the said data?

@dot-asm
Copy link
Contributor

dot-asm commented Nov 28, 2016

Of course there is secure renegotiation, but then maybe this option should be offered only in such cases...

@dot-asm
Copy link
Contributor

dot-asm commented Nov 28, 2016

Oh! I feel foolish. Renegotiation is encrypted, so it's not directly susceptible to man-in-the-middle... Duh...

@mattcaswell
Copy link
Member Author

Renegotiation is encrypted, so it's not directly susceptible to man-in-the-middle

Quite. The case where non-secure renegotiation is being run is insecure in any event, regardless of client auth, so I don't think we need to make any special allowances for that.

What is this option supposed to mean?

It means only assert the client identity on an initial handshake. Presumably this is because the application does not require the overhead of checking it on subsequent renegs. It is sufficient to assert it once. As long as you trust that renegs are secure and your identity cannot be hijacked by a reneg (which secure renegotiation is supposed to do) then that should be ok.

@richsalz
Copy link
Contributor

And unsafe legacy renegotiation is off by default, right? :)

@mattcaswell
Copy link
Member Author

And unsafe legacy renegotiation is off by default, right? :)

Yes!! :-)

The flag SSL_VERIFY_CLIENT_ONCE is documented as follows:

  B<Server mode:> only request a client certificate on the initial TLS/SSL
  handshake. Do not ask for a client certificate again in case of a
  renegotiation. This flag must be used together with SSL_VERIFY_PEER.

  B<Client mode:> ignored

But the implementation actually did nothing. After the server sends its
ServerKeyExchange message, the code was checking s->session->peer to see if
it is NULL. If it was set then it did not ask for another client
certificate. However s->session->peer will only be set in the event of a
resumption, but a ServerKeyExchange message is only sent in the event of a
full handshake (i.e. no resumption).

The documentation suggests that the original intention was for this to
have an effect on renegotiation, and resumption doesn't come into it.

The fix is to properly check for renegotiation, not whether there is already
a client certificate in the session.

As far as I can tell this has been broken for a *long* time.
@mattcaswell
Copy link
Member Author

Updated to have the same reneg check as #1982 and #1983 for consistency reasons. Ping?

@levitte
Copy link
Member

levitte commented Jan 12, 2017

Is this still current?

@mattcaswell
Copy link
Member Author

Yes - it's still current.

levitte pushed a commit that referenced this pull request Jan 23, 2017
The flag SSL_VERIFY_CLIENT_ONCE is documented as follows:

  B<Server mode:> only request a client certificate on the initial TLS/SSL
  handshake. Do not ask for a client certificate again in case of a
  renegotiation. This flag must be used together with SSL_VERIFY_PEER.

  B<Client mode:> ignored

But the implementation actually did nothing. After the server sends its
ServerKeyExchange message, the code was checking s->session->peer to see if
it is NULL. If it was set then it did not ask for another client
certificate. However s->session->peer will only be set in the event of a
resumption, but a ServerKeyExchange message is only sent in the event of a
full handshake (i.e. no resumption).

The documentation suggests that the original intention was for this to
have an effect on renegotiation, and resumption doesn't come into it.

The fix is to properly check for renegotiation, not whether there is already
a client certificate in the session.

As far as I can tell this has been broken for a *long* time.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #1984)
@mattcaswell
Copy link
Member Author

Pushed. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: 1.0.2 Merge to OpenSSL_1_0_2-stable branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants