Server TLS renegotiation issues with tc-native

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Server TLS renegotiation issues with tc-native

markt
Hi,

The good news is I have managed to unpick the various TLS issues I've
been struggling with.

The Chrome not selecting the user cert issue looks to be related to how
many of the fields were complete in the DN. That has been resolved by
recreating the test keys and certs I have been using.

I was also seeing some ugly stack traces in the console during
renegotiation. The root cause is the final issue in this mail but the
stack traces were a sign of some edge cases in the error handling. These
have been fixed in r1804463.

NIO and NIO2 with JSSE work as expected.

NIO and NIO2 with OpenSSL mostly work as expected. The only glitch is
that because we don't specify a set of trusted certs, OpenSSL doesn't
send any CA names to the user agent which in turn opts to display all of
the available client certs. This is more a usability issue for users
with lots of certs than a bug. I'll create a Bugzilla entry for this one.

The final issue is the one I have tried, and failed, to solve. The
renegotiation sequence with APR/native doesn't work as expected. I've
been debugging may way through the call to SSL.renegotiate(long) and
this is what happens:

SSL_renegotiate() returns 1 (success)
SSL_do_handshake() returns 1 (success)
SSL_is_init_finished() returns 1 (success)
SSL_peek() returns -1 (failure)
  At this point the client prompts the user to select a certificate
  The return code is not currently tested so the code continues
  (If SSL_get_error() is called here, SSL_ERROR_WANT_READ is returned)
SSL_is_init_finished() returns 1 (success)

Execution returns to the Java code where no certs are found on the
connection so a 401 response is sent back to the client.

Shortly afterwards the user selects a certificate, the handshake
continues and then the connection fails as soon as the handshake completes.

I think the behaviour we are seeing is because the connection is in
non-blocking mode. I have tried various ways to handle this without success.

Help with a solution very much appreciated.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Server TLS renegotiation issues with tc-native

remm
On Tue, Aug 8, 2017 at 10:07 PM, Mark Thomas <[hidden email]> wrote:

> Hi,
>
> The good news is I have managed to unpick the various TLS issues I've
> been struggling with.
>
> The Chrome not selecting the user cert issue looks to be related to how
> many of the fields were complete in the DN. That has been resolved by
> recreating the test keys and certs I have been using.
>
> I was also seeing some ugly stack traces in the console during
> renegotiation. The root cause is the final issue in this mail but the
> stack traces were a sign of some edge cases in the error handling. These
> have been fixed in r1804463.
>
> NIO and NIO2 with JSSE work as expected.
>
> NIO and NIO2 with OpenSSL mostly work as expected. The only glitch is
> that because we don't specify a set of trusted certs, OpenSSL doesn't
> send any CA names to the user agent which in turn opts to display all of
> the available client certs. This is more a usability issue for users
> with lots of certs than a bug. I'll create a Bugzilla entry for this one.
>
> The final issue is the one I have tried, and failed, to solve. The
> renegotiation sequence with APR/native doesn't work as expected. I've
> been debugging may way through the call to SSL.renegotiate(long) and
> this is what happens:
>
> SSL_renegotiate() returns 1 (success)
> SSL_do_handshake() returns 1 (success)
> SSL_is_init_finished() returns 1 (success)
> SSL_peek() returns -1 (failure)
>   At this point the client prompts the user to select a certificate
>   The return code is not currently tested so the code continues
>   (If SSL_get_error() is called here, SSL_ERROR_WANT_READ is returned)
> SSL_is_init_finished() returns 1 (success)
>
> Execution returns to the Java code where no certs are found on the
> connection so a 401 response is sent back to the client.
>
> Shortly afterwards the user selects a certificate, the handshake
> continues and then the connection fails as soon as the handshake completes.
>
> I think the behaviour we are seeing is because the connection is in
> non-blocking mode. I have tried various ways to handle this without
> success.
>
> Help with a solution very much appreciated.
>
> I think it would need the exact same algorithm as in OpenSSLEngine then
(which is a bit complex ...). renegociate actually does nothing, in
particular the SSL state remains as if the handshake was complete (as you
can see with the SSL isininit call). Actually, it needs to wrap/unwrap a
few times before it's actually done. To detect that the rehandshake is
done, you can either use the callback or hack using a statistics
(OpenSSLEngine calls  SSL.getHandshakeCount to see if a handshake was done
after IO operations). So that would probably be a significant amount of
native code changes.

IMO it's not really worth adding to APR, shouldn't we tell people to use
JSSE/OpenSSL instead now ?

Rémy
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Server TLS renegotiation issues with tc-native

markt
On 09/08/17 08:43, Rémy Maucherat wrote:

> On Tue, Aug 8, 2017 at 10:07 PM, Mark Thomas <[hidden email]> wrote:
>
>> Hi,
>>
>> The good news is I have managed to unpick the various TLS issues I've
>> been struggling with.
>>
>> The Chrome not selecting the user cert issue looks to be related to how
>> many of the fields were complete in the DN. That has been resolved by
>> recreating the test keys and certs I have been using.
>>
>> I was also seeing some ugly stack traces in the console during
>> renegotiation. The root cause is the final issue in this mail but the
>> stack traces were a sign of some edge cases in the error handling. These
>> have been fixed in r1804463.
>>
>> NIO and NIO2 with JSSE work as expected.
>>
>> NIO and NIO2 with OpenSSL mostly work as expected. The only glitch is
>> that because we don't specify a set of trusted certs, OpenSSL doesn't
>> send any CA names to the user agent which in turn opts to display all of
>> the available client certs. This is more a usability issue for users
>> with lots of certs than a bug. I'll create a Bugzilla entry for this one.
>>
>> The final issue is the one I have tried, and failed, to solve. The
>> renegotiation sequence with APR/native doesn't work as expected. I've
>> been debugging may way through the call to SSL.renegotiate(long) and
>> this is what happens:
>>
>> SSL_renegotiate() returns 1 (success)
>> SSL_do_handshake() returns 1 (success)
>> SSL_is_init_finished() returns 1 (success)
>> SSL_peek() returns -1 (failure)
>>   At this point the client prompts the user to select a certificate
>>   The return code is not currently tested so the code continues
>>   (If SSL_get_error() is called here, SSL_ERROR_WANT_READ is returned)
>> SSL_is_init_finished() returns 1 (success)
>>
>> Execution returns to the Java code where no certs are found on the
>> connection so a 401 response is sent back to the client.
>>
>> Shortly afterwards the user selects a certificate, the handshake
>> continues and then the connection fails as soon as the handshake completes.
>>
>> I think the behaviour we are seeing is because the connection is in
>> non-blocking mode. I have tried various ways to handle this without
>> success.
>>
>> Help with a solution very much appreciated.
>>
> I think it would need the exact same algorithm as in OpenSSLEngine then
> (which is a bit complex ...). renegociate actually does nothing, in
> particular the SSL state remains as if the handshake was complete (as you
> can see with the SSL isininit call). Actually, it needs to wrap/unwrap a
> few times before it's actually done. To detect that the rehandshake is
> done, you can either use the callback or hack using a statistics
> (OpenSSLEngine calls  SSL.getHandshakeCount to see if a handshake was done
> after IO operations). So that would probably be a significant amount of
> native code changes.

I'll take a look at this. I think the hint about the statistics hack
will help me get further than I have so far. Whether it is far enough,
we'll see.

> IMO it's not really worth adding to APR, shouldn't we tell people to use
> JSSE/OpenSSL instead now ?

That is certainly an option. I'm not quite ready to give up on fixing
this yet but if we aren't able to fix this then I'd be happy with that
work-around.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Server TLS renegotiation issues with tc-native

markt
On 09/08/17 15:51, Mark Thomas wrote:

> I'll take a look at this. I think the hint about the statistics hack
> will help me get further than I have so far. Whether it is far enough,
> we'll see.

As it happened, the statistics weren't the fix I needed but they got me
looking in the right direction where I found SSL_renegotiate_pending() -
thanks for the hint.

I have attached my proposed patch for review.

It seems to be moving back towards an a previous approach. The commit
history suggests this approach should not be necessary. I have tested
both 7.0.x and 9.0.x and both versions need the additional reads. I'm
not 100% sure what is going on.

If there aren't any objections, I plan to apply this patch roll 1.2.13
in time for it to be included in the September round of Tomcat releases.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

renegotiation-v1.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Server TLS renegotiation issues with tc-native

Konstantin Kolinko
2017-08-11 18:20 GMT+03:00 Mark Thomas <[hidden email]>:

> On 09/08/17 15:51, Mark Thomas wrote:
>
>> I'll take a look at this. I think the hint about the statistics hack
>> will help me get further than I have so far. Whether it is far enough,
>> we'll see.
>
> As it happened, the statistics weren't the fix I needed but they got me
> looking in the right direction where I found SSL_renegotiate_pending() -
> thanks for the hint.
>
> I have attached my proposed patch for review.

1. Sanity check:  You need to explicitly initialize new local variable
"error = 0".

Current code initializes it only when "if (retVal < 1)" and subsequent
if (error == SSL_ERROR_WANT_READ) reads an uninitialized value.

Local variables need explicit initialization in C (they do not default to 0).
https://stackoverflow.com/questions/14049777/why-are-global-variables-always-initialized-to-0-but-not-local-variables

2. Many new lines have tab characters and some have trailing whitespace.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Loading...