SSL/TLS Support on Connections

[AlainODea]

It would be very useful to have SSL/TLS support.

I was looking at Network.AMQP.Internal.openConnection''. It seems like it should be practical to use Network.Connection.connectionSetSecure to achieve TLS encrypted communications.

Something along the lines of this answer on StackOverflow may be applicable.

I'll take a crack at this tonight, but I imagine you'll beat me to a working implementation :)

[michaelklishin]

Feel free to use tls-gen to generate some development certificates.

[AlainODea]

Thank you @michaelklishin. tls-gen looks incredibly convenient!

[AlainODea]

No dice on my first attempt. It looks like Network.Connection.connectionSetSecure is intended for negotiating an upgrade to TLS. I've learned that's not how AMQPS works. It expects TLS to be established immediately on connection similar to HTTPS. It looks like Network.Connection.connectTo needs to be handed a ConnectionParams with connectionUseSecure = Just TLSSettingsSimple. I'm going to take another crack at this tonight.

[AlainODea]

@michaelklishin tls-gen is incredible. What a time saver!

It looks like GHC.IO.Handle is used in a few places, but is encapsulated. It can fairly readily be replaced with Network.Connection.Connection

Making connHandle a Network.Connection.Connection is no big deal. It requires changes to readFrame and writeFrame and some minor tweaks the handshake.

I will give this another shot tonight.

[AlainODea]

I'm pretty sure I've made quite a mess of this.

It compiles, but I can't get it to work properly. I've looked at PCAPs in Wireshark until I've gone a little cross-eyed and have come up with very little other than that traces before and after my changes are dramatically different for reasons I can't identify.

The nearest indication I have of trouble is my dubious use of toStrict and fromStrict in Network.AMQP.Internal.readFrame and Network.AMQP.Internal.writeFrame:
https://github.com/AlainODea-haskell/amqp/compare/TLS_Support#diff-f64900e08cee94a8e4c16974feda0226R386

On second though it's more likely the significant difference between Network.Connection.connectionGet and GHC.IO.Handle.hGet.
http://hackage.haskell.org/package/connection-0.1.3.1/docs/Network-Connection.html#v:connectionGet
http://hackage.haskell.org/package/bytestring-0.10.4.0/docs/Data-ByteString-Lazy.html#v:hGet

Network.Connection.connectionGet acts like GHC.IO.Handle.hGetNonBlocking.

Ouch. I should get an RTFM tattoo :D

At least I have a good idea why it is not working now :)

[hreinhardt]

In my testing the program gets stuck in the writeFrame method. Apparently the 'connection' library uses an MVar internally which gets deadlocked because we try to read and write concurrently.

[AlainODea]

@hreinhardt thank you for taking a look. I'll investigate. I may need to ask @vincenthz how to work around that.

[vincenthz]

it's fixed in tls 1.2 (not released to hackage yet) and dev branch of connection (not on hackage either)

[AlainODea]

@vincenthz that's great news. Thank you. I'll give my code a try with the dev branch.

[AlainODea]

@vincenthz @hreinhardt thank you for all your help on this. I got it working using tls 1.2 (from hs-tls' master@692aaf01a5) and connection dev (from hs-connection dev@289d72bf63). Both plain and TLS connections work and are included in the TLS_Support branch on my fork.

@hreinhardt should I proceed with a Pull Request or wait for the release of tls 1.2 and the connection package that uses it?

[hreinhardt]

There's still the problem with the difference between hGet and connectionGet'. My understanding is that the size parameter on connectionGet' specifies the maximum number of bytes to read instead of the exact number to read.

[AlainODea]

@hreinhardt I think that's covered by Ankur Dhama's solution:
http://stackoverflow.com/questions/20414948/how-do-i-use-network-connection-connectionget-in-a-blocking-manner-like-data-byt/20416436#20416436

I integrated and tested Ankur's solution on my TLS_Support branch last night. I'll rebase to a single commit before sending a Pull Request. It's a bit of a mess right now.

[AlainODea]

I know for sure that hard-coding certificate validation to disabled is very wrong so I'm definitely going to change that.

The challenge with making TLS certificate validation configurable is that I would need a reasonable way for users to express it. My addition of openTLSConnection and openTLSConnection' to the module interface seems clumsy. Adding another axis of Unsecured versions of those functions would be even more clumsy.

Should I hard-code certificate validation as enabled or make it user configurable?

[hreinhardt]

You don't need to create new connect-functions. You can add a TLS flag to ConnectionOpts and people can then use openConnection''.
I'm not sure if it makes sense to enable it by default. Are there any drawbacks?

[AlainODea]

That makes sense. I'll add a TLS flag to ConnectionOpts. TLS should be off by default to preserve existing client code, but TLS Certificate Validation (essential for secure use of TLS) will be on and not configurable when the TLS flag is True.

The drawback of TLS is the latency of establishing a TLS session and the overhead of encryption. It's usually not a huge factor in practice unless the AMQP server is experiencing CPU saturation (spikes of CPU scheduling delays or load averages exceeding 1.0). The major drawback is that it requires changes to the AMQP server configuration and these can be a bit of a hassle.

I'll make those changes and send a clean Pull Request in the next few days.

[woffs]

Is it possible to have TLS with a client certificate?

[alain-odea-vgh]

@woffs not as of now. It's only providing server authentication right now. The underlying library supports mutual TLS, but I don't expose a method of using it.

[alain-odea-vgh]

@woffs what AMQP server are you using that supports mutual TLS? I'd love to have that.

[woffs]

just rabbitmq-3.4.3

[alain-odea-vgh]

Cool. I did not realize RabbitMQ support that. Thanks :)

[woffs]

nb, at the moment, with haskell as client I use
socat -lm tcp-listen:5672,fork,reuseaddr openssl:amqp-server:5673,verify=0,cert=clientcert.pem
and SASLMechanism "EXTERNAL"

Bit maybe this is just getting off-topic. ;-)

[woffs]

Is it possible to expose the TLS method to use a client certificate; or is there a way I could do it by hand? Should I open a new ticket regarding this feature request?

[AlainODea]

@woffs coTLSSettings does not currently expose the means for doing this:
a578c13

It only allows toggling TLS server validation on and off.

It should be straight-forward to adapt TLSSettings to include an option for supplying a more complex Conn.TLSSettings value:
https://github.com/AlainODea-haskell/amqp/blob/a578c1396abf09c92d2c350f79ef5bee8d6b45c0/Network/AMQP/Internal.hs#L154-163

Broad strokes on this would be:

-- | Represents the kind of TLS connection to establish.
data TLSSettings =
    TLSTrusted   -- ^ Require trusted certificates (Recommended).
  | TLSUntrusted -- ^ Allow untrusted certificates (Discouraged. Vulnerable to man-in-the-middle attacks)
  | TLSCustom Conn.TLSSettings -- ^ Provide your own custom TLS settings

connectionTLSSettings :: TLSSettings -> Maybe Conn.TLSSettings
connectionTLSSettings tlsSettings =
  Just $ case tlsSettings of
    TLSTrusted -> Conn.TLSSettingsSimple False False False
    TLSUntrusted -> Conn.TLSSettingsSimple True False False
    TLSCustom x -> x

Want to give that a shot?

[woffs]

Thanks! I applied that and am trying to get it working. Now I have the problem of constructing a reasonable and working TLSSettings and how to debug. I still get handshake failure and my onCertificateRequest routine don't get called yet. I guess there's no possibility to let TLSSettingsSimple do the work and just add the clientHooks onCertificateRequest?

[woffs]

It works! I'll take the freedom to put your suggestion into a PR?

[AlainODea]

@woffs that sounds reasonable. I'm not the maintainer. @michaelklishin what do you think?

[AlainODea]

Sorry. I meant to tag @hreinhardt. What do you think of @woffs putting in a PR for exposing full TLS settings customization? It should be pretty simple.

[hreinhardt]

Sounds like a good idea to add this.