On 13/06/17 11:57, Mark Thomas wrote:
> On 13/06/17 10:13, Kreuser, Peter wrote:
>> Mark,
>>
>>> On 09/06/17 16:02, Kreuser, Peter wrote:
>>>> Hi all,
>>>>
>>>> Sorry for the long text. I hope somebody can help me track down the 
>>>> problem I'm facing with Tomcat (8.5.15), tcnative (1.2.12), openssl 
>>>> (1.1.0e) and HTTP/2. JVM is zulu-8.21.0.1 (1.8.0_131-b11)
>>>
>>> No need to apologise. There was enough information in this report for me
>>> to be 95% sure of what was going on before I tested things locally.
>>>
>>> <snip/>
>>>
>>>> Would be nice if somebody here could tell me if that is a bug or if I have 
>>>> some misconfiguration.
>>>
>>> It is a bug.
>>>
>>> The short explanation is that with NIO2, the style of the API (async
>>> read/write with CompletionHandlers) means that a TLS connection can get
>>> closed down (by a CompletionHandler) before the access log entry is
>>> written. This leads to passing a NULL reference to some native code
>>> which - unsurprisingly - falls over (Tomcat is meant to make sure it
>>> doesn't pass NULL references).
>>>
>>> There is an easy fix but the side-effect is that sometimes the cipher
>>> suite name won't appear in the access log. Depending on timing on your
>>> system that could be very frequent to almost never. I'd roughly expect
>>> it to happen as frequently as the crashes.
>>>
>>> The harder fix is to note that we want the info and to ensure we cache
>>> it before the TLS connection is closed down. In theory, we could do this
>>> in the access log but it means parsing the request attribute config.
>>> Performance wise, there will be an impact on users since the lookup to
>>> populate the cache will occur during request processing rather than
>>> afterwards. However, overall throughput should be unaffected.
>>>
>>> An alternative fix is for the application to trigger the caching of the
>>> TLS info by requesting one of the TLD attributes.
>>>
>>> Switching to NIO should also fix this. Note, I would not expect NIO and
>>> NIO2 to be much different performance wise.
>>>
>>> Would the easy fix be sufficient?
>>>
>>> Mark
>>
>> For a start the easy fix would be sufficient. Apparently this is only a real 
>> problem when I call the site via curl, as it does not happen in the various 
>> browsers (at least not on every call).
>>
>> We should at least make users aware that certain information can be missing 
>> in the accesslog and the cause of it (in the AccessLogValve documentation?). 
>> And are there any other implications of the asynchronicity? I guess the 
>> behavior of NIO2 and HTTP/2 processing is not obvious?
>>
>> Your alternative fix would be to copy the attributes during request 
>> processing (in a filter) and then log them from the filter request 
>> attributes instead of the predefined ssl attributes? Correct?
>>
>> Thank you for your support and the bug fix ;-)! Have you filed the bug 
>> internally or should I?
> 
> I need to look into this some more first. Remy's comments suggest that
> my first analysis might have been wrong.

I've been looking into this this afternoon. Rémy has made some changes
to trunk and 8.5.x that prevent the crash.

It does appear that a timing issue is involved. I'm still looking into
the details.

At the moment, the SSL_NULL_WITH_NULL_NULL cipher suite is recorded
rather than the crash when the TLS information is not available when the
access log needs it.

Rémy's changes are in the 9.0.x and 8.5.x releases being voted on at the
moment.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org

Reply via email to