[
https://issues.apache.org/jira/browse/QPID-7742?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15984541#comment-15984541
]
Alex Rudyy commented on QPID-7742:
----------------------------------
Rob,
I reviewed your changes and here are my review comments:
* {{SSLUtil#getServerNameFromTLSClientHello}}
** line 668 : {{if (first == 22 && third != 0x01)}}. The condition checks that
it is a TLS handshake and protocol version is not TLS 1.0. I believe you meant
to check that TLS version is not SSL 3.0. Otherwise SSL 3.0 connection would
try to explore TLS extension defined for TLS 1.0, 1.1 and 1.2 and TLS 1.0
connection would ignore server name when extension is present.
** Currently {{SSLUtil#getServerNameFromTLSClientHello}} silently returns null
if preconditions are not met. I think it should throw exception (for example
{{IllegalArgumentException}}) in order to notify caller that inappropriate
buffer is provided. The code in {{NonBlockingConnectionTLSDelegate}} checks
that conditions are met by calling
{{SSLUtil#isSufficientToDetermineClientSNIHost}}. Thus, it is not an issue as
such, but for code correctness it make sense to throw exception if the
following preconditions are not meat:
*** the buffer#remaining is less than 5
*** first byte is not handshake
*** second byte is not 3 (major version)
*** third byte is 0 (minor version)
*** the record size exceeds buffer#remaining
*** sixth byte is not client_hello (0)
** Currently {{SSLUtil#getServerNameFromTLSClientHello}} silently returns null
if extension has invalid format or server name extension has invalid format. I
think that it is right thing to do but I would log it using debug or trace log
level in order to simplify the debugging.
* Re WebSocket implementation. I think it make sense to do it after upgrade to
new version of jetty. I am not sure that it is even required for v7.
> [Java Broker] [AMQP 1.0] Hostname should be taken from SNI or sasl-init if it
> is not present in open
> ----------------------------------------------------------------------------------------------------
>
> Key: QPID-7742
> URL: https://issues.apache.org/jira/browse/QPID-7742
> Project: Qpid
> Issue Type: Bug
> Components: Java Broker
> Reporter: Rob Godfrey
> Assignee: Rob Godfrey
>
> As per [the AMQP
> specification|http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-transport-v1.0-os.html#type-open]
> when TLS and/or SASL are being used, the hostname provided by these layers
> should be used if the hostname is not explicitly provided in the open
> performative
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
