qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Rob Godfrey (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (QPID-7742) [Java Broker] [AMQP 1.0] Hostname should be taken from SNI or sasl-init if it is not present in open
Date Wed, 26 Apr 2017 11:30:04 GMT

    [ https://issues.apache.org/jira/browse/QPID-7742?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15984616#comment-15984616
] 

Rob Godfrey commented on QPID-7742:
-----------------------------------

{quote}
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.
{quote}
Agreed - I will change this to exclude SSLv3
{quote}
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 met:
{quote}
Agreed - I will change the getServerNameFromTLSClientHello to first call isSufficientToDetermineClientSNIHost,
and if the return value from that is false, I will throw an IllegalArgumentException
{quote}
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.
{quote}
My decision here was originally that the underlying buffers are going to be fed through an
SSL decryption engine which will properly report on any SSL encoding errors, and it would
be better to simply ignore invalid SSL in this SNI code.  I think that is still the correct
decision.
{quote}
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.
{quote}
Until it is done then we are not in compliance with the AMQP websocket binding specification.
 I agree it should probably be raised as a separate JIRA and prioritized on its own (and after
the Jetty upgrade)

> [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: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


Mime
View raw message