Inter-broker link with SSL

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Inter-broker link with SSL

Chris Richardson
Hi,

As you see from the subject I'm trying to set up an SSL link between two
brokers (version 0.28). To cut to the chase, both brokers are able to
communicate locally over SSL - for instance I can use

qpid-stat -b amqps://localhost:5671 -e
and
qpid-stat -b amqps://localhost:5674 -e

(these being my broker SSL ports) to retrieve lists of exchanges. This
works fine.

The problem comes when I attempt to add a route. To simplify, I'm just
trying to add just the link here with
qpid-route -t ssl link add localhost:5673 <hostname>:5671

Note that the destination is using a non-ssl port as per Gordon's
suggestion in this thread
http://qpid.2158936.n2.nabble.com/SSL-between-c-brokers-td7581914.html
and that <hostname> matches the "cn=" part of my ssl-cert's subject. Sadly
that thread related to version 0.14 and culminated in a bug fix related to
that version and did not continue along the same lines as my problem.

Following the addition of the link, the command
qpid-route link list localhost:5673
shows
Host            Port    Transport Durable  State             Last Error
=============================================================================
<hostname>       5671    ssl          N     Waiting           Closed by peer

and the source broker repeatedly reports
[System] error framing-error: Framing version unsupported (AMQPFrame.cpp:93)

I've done some digging into the state of the broker around this area of the
code, specifically in the AMQPFrame::decode method. It seems that the
framing version is derived from a flags byte and should always be zero, but
in this case has a value of 1. The byte stream being decoded is 134 bytes
long and begins (in hex)
 {41 4d 51 50 01 01 00 0a 0f 00 00 7e 00 00 00 00 00 00 00 00 01 01 07 00
00 00 00 3f 00 00 00 0 13 71 70 69 64 2e 66 65 64 65 ... ...}
This appears to be a decrypted version of the frame as it begins "AMQP, 1,
1, 0, 10..." which matches the specification exactly down to the class,
instance and protocol version of the message being received. However, the
flags byte from which the frame version is derived has a value of 41 ie.
appears to be incorrectly read from the "A" of "AMQP"... clearly something
has gone awry! Other packets arriving at the AMQPFrame::decode method
appear to have had the header already removed and begin { 0f ...} (assuming
those flags are set).

At this point I thought it may be time to hand over to someone with a
better understanding of the broker internals. Help... please? :)

/Chris

--

*Chris Richardson*, System Architect
[hidden email]


*FourC AS, Vestre Rosten 81, Trekanten, NO-7075 Tiller, Norwaywww.fourc.eu
<http://www.fourc.eu/>*

*Follow us on LinkedIn <http://bit.ly/fourcli>, Facebook
<http://bit.ly/fourcfb>, Google+ <http://bit.ly/fourcgp> and Twitter
<http://bit.ly/fourctw>!*
Reply | Threaded
Open this post in threaded view
|

Re: Inter-broker link with SSL

Chris Richardson
I think I've tracked this one down.

My theory is that the qpid::amqp_1_10::Connection::decode method, which
contains the code
if (isClient && !initialized) {
        //read in protocol header
        framing::ProtocolInitiation pi;
        if (pi.decode(in)) {
            if(!(pi==version))
                throw Exception(QPID_MSG("Unsupported version: " << pi
                                         << " supported version " <<
version));
            QPID_LOG(trace, "RECV [" << identifier << "]: INIT(" << pi <<
")");
        }
        initialized = true;
    }

can incorrectly set the connection status to "initialised" when the
pi.decode() method fails (which it does if <8 bytes are available). This
does not happen in practice without SSL, but when SSL is switched on I have
seen zero, 1 and 4 bytes turn up in the input buffer on first invocation.
Moving the "initialized = true" line to inside the "if (pi.decode(int) {"
block seems to fix the issue (this is because subsequent calls to the
Connection::decode() method (when sufficient data is available in the input
buffer) will result in the pi.decode() method correctly consuming the
"AMQP..." header).

A further optimisation might be to skip the decode() method body if
insufficient data is available thereby avoiding the overhead of the various
objects that are created and destroyed and the exceptions that are
thrown... but that's extra lines of code and "less is more", as they say ;)

Could a qpid dev please confirm this fix and, if approved, could we be
informed of when a fix will be released?

Thanks

Chris




On 4 August 2014 13:22, Chris Richardson <[hidden email]> wrote:

> Hi,
>
> As you see from the subject I'm trying to set up an SSL link between two
> brokers (version 0.28). To cut to the chase, both brokers are able to
> communicate locally over SSL - for instance I can use
>
> qpid-stat -b amqps://localhost:5671 -e
> and
> qpid-stat -b amqps://localhost:5674 -e
>
> (these being my broker SSL ports) to retrieve lists of exchanges. This
> works fine.
>
> The problem comes when I attempt to add a route. To simplify, I'm just
> trying to add just the link here with
> qpid-route -t ssl link add localhost:5673 <hostname>:5671
>
> Note that the destination is using a non-ssl port as per Gordon's
> suggestion in this thread
> http://qpid.2158936.n2.nabble.com/SSL-between-c-brokers-td7581914.html
> and that <hostname> matches the "cn=" part of my ssl-cert's subject. Sadly
> that thread related to version 0.14 and culminated in a bug fix related to
> that version and did not continue along the same lines as my problem.
>
> Following the addition of the link, the command
> qpid-route link list localhost:5673
> shows
> Host            Port    Transport Durable  State             Last Error
>
> =============================================================================
> <hostname>       5671    ssl          N     Waiting           Closed by
> peer
>
> and the source broker repeatedly reports
> [System] error framing-error: Framing version unsupported
> (AMQPFrame.cpp:93)
>
> I've done some digging into the state of the broker around this area of
> the code, specifically in the AMQPFrame::decode method. It seems that the
> framing version is derived from a flags byte and should always be zero, but
> in this case has a value of 1. The byte stream being decoded is 134 bytes
> long and begins (in hex)
>  {41 4d 51 50 01 01 00 0a 0f 00 00 7e 00 00 00 00 00 00 00 00 01 01 07 00
> 00 00 00 3f 00 00 00 0 13 71 70 69 64 2e 66 65 64 65 ... ...}
> This appears to be a decrypted version of the frame as it begins "AMQP, 1,
> 1, 0, 10..." which matches the specification exactly down to the class,
> instance and protocol version of the message being received. However, the
> flags byte from which the frame version is derived has a value of 41 ie.
> appears to be incorrectly read from the "A" of "AMQP"... clearly something
> has gone awry! Other packets arriving at the AMQPFrame::decode method
> appear to have had the header already removed and begin { 0f ...} (assuming
> those flags are set).
>
> At this point I thought it may be time to hand over to someone with a
> better understanding of the broker internals. Help... please? :)
>
> /Chris
>
> --
>
> *Chris Richardson*, System Architect
> [hidden email]
>
>
> *FourC AS, Vestre Rosten 81, Trekanten, NO-7075 Tiller, Norwaywww.fourc.eu
> <http://www.fourc.eu/>*
>
> *Follow us on LinkedIn <http://bit.ly/fourcli>, Facebook
> <http://bit.ly/fourcfb>, Google+ <http://bit.ly/fourcgp> and Twitter
> <http://bit.ly/fourctw>!*
>



--

*Chris Richardson*, System Architect
[hidden email]


*FourC AS, Vestre Rosten 81, Trekanten, NO-7075 Tiller, Norwaywww.fourc.eu
<http://www.fourc.eu/>*

*Follow us on LinkedIn <http://bit.ly/fourcli>, Facebook
<http://bit.ly/fourcfb>, Google+ <http://bit.ly/fourcgp> and Twitter
<http://bit.ly/fourctw>!*
Reply | Threaded
Open this post in threaded view
|

Re: Inter-broker link with SSL

Gordon Sim
On 08/05/2014 05:18 PM, Chris Richardson wrote:
> I think I've tracked this one down.

Good job! Thanks for chasing this down!

> My theory is that the qpid::amqp_1_10::Connection::decode method, which
> contains the code
> if (isClient && !initialized) {
>          //read in protocol header
>          framing::ProtocolInitiation pi;
>          if (pi.decode(in)) {
>              if(!(pi==version))
>                  throw Exception(QPID_MSG("Unsupported version: " << pi
>                                           << " supported version " <<
> version));
>              QPID_LOG(trace, "RECV [" << identifier << "]: INIT(" << pi <<
> ")");
>          }
>          initialized = true;
>      }
>
> can incorrectly set the connection status to "initialised" when the
> pi.decode() method fails (which it does if <8 bytes are available). This
> does not happen in practice without SSL, but when SSL is switched on I have
> seen zero, 1 and 4 bytes turn up in the input buffer on first invocation.
> Moving the "initialized = true" line to inside the "if (pi.decode(int) {"
> block seems to fix the issue (this is because subsequent calls to the
> Connection::decode() method (when sufficient data is available in the input
> buffer) will result in the pi.decode() method correctly consuming the
> "AMQP..." header).

That sounds like the right fix. Thank you once again!

(The description reminded me of
https://issues.apache.org/jira/browse/QPID-5488, but though related this
is a slightly different issue that only affects 'outgoing' connections).

> A further optimisation might be to skip the decode() method body if
> insufficient data is available thereby avoiding the overhead of the various
> objects that are created and destroyed and the exceptions that are
> thrown... but that's extra lines of code and "less is more", as they say ;)

The first line of the decode is in any case exactly such a check so I
don't think anything is gained by this.

> Could a qpid dev please confirm this fix and, if approved, could we be
> informed of when a fix will be released?

I'll get this in for the upcoming 0.30 release.

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

Reply | Threaded
Open this post in threaded view
|

Re: Inter-broker link with SSL

Gordon Sim
On 08/05/2014 05:51 PM, Gordon Sim wrote:
> On 08/05/2014 05:18 PM, Chris Richardson wrote:
>> Could a qpid dev please confirm this fix and, if approved, could we be
>> informed of when a fix will be released?
>
> I'll get this in for the upcoming 0.30 release.

Fyi: I checked your fix into trunk as https://svn.apache.org/r1615991 
(also raised an accompanying JIRA:
https://issues.apache.org/jira/browse/QPID-5963).

Thanks again!

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