SSL Support

[Daniel-Svensson]

This provides SSL support to quickfix and thereby implements issue #18

The names of the different settings are similar to those suggested in the issue thread and are all documented, both in code and in tutorial\configuration

The fastest approach to get the examples running using SSL is to first install the following three certificates (double click on them in windows and follow the guide) into the personal store and then specifying the "_ssl" (eg tradeclient_ssl.cfg instead of tradeclient.cfg) variant of the configuration file for the exampel project

• Examples\QuickFixn-TestCA.pfx
• Examples\QuickFixn-TestServer.pfx
• Examples\QuickFixn-TestClient.pfx

My original ide was to specify filepath and password to the certificates in the example projects, but it did not seem 100% reliable when using .net 3.5 (It worked and then stopped working after cloning my reposit to a new directory so it may be to long filenamse or maybe I just needed a computer restart).
Having the certificates installed on the computer is better from a security point of view since you don't have to store the password in plain text in the configuration file, so I think we should use this approach anyway as we would otherwise maybe fool people into using it in production otherwise.

Note: Don't install the QuickFixn-TestCA.pfx as a trusted root CA unless you know what you are doing. (Anybody who have downloaded it from guithub could create a certificate for *.google.com, your bank or any other webbpage and you would trust it)

[gbirchmeier]

Thanks, @Daniel-Svensson . This is a feature that more and more people have been asking for, so we'll take a look at it when we start planning our v1.5 features.

[ligu]

This feature is very welcomed!
I've tested it in live environment and works great!

[ligu]

I suggest an extra config param:
in StreamFactory.GetCertificateFromStore in the first code line, the certificate store location should be parametrizable: LocalMachine or CurrentUser, and maybe Both.

[ligu]

I've added configuration settings for SSLStoreName and SSLStoreLocation.

Here it is:
https://github.com/Daniel-Svensson/quickfixn/pull/1

[JanKrivanek]

This looks very helpful - thanks!

Is there a way how to use .NET SslStream without installing the certificates?
When I try to point this to an UAT where client certificate validation is being performed (and connection tunneled through stunnel works just fine) the 'AuthenticateAsClient' code gets AuthenticationException. So I enabled verbose logging of Schannel (HKEY_LOCAL_MACHINE\System\CurrentControlSet\Control\SecurityProviders\SCHANNEL – EventLogging set to 7) and then in eventlog I saw following message:

The remote server has requested SSL client authentication, but no suitable client certificate could be found. An
anonymous connection will be attempted. This SSL connection request may succeed or fail, depending on the
server's policy settings.

Despite the fact that AuthenticateAsClient is getting list of certificates (X509CertificateCollection) containing certificate constructed from file.

Even though this is more question on .NET internals performing the actual SSL handshake - I'd still appreciate any opinions.

Thanks!
Jan

[Daniel-Svensson]

@jakrivan

Is there a way how to use .NET SslStream without installing the certificates?

Just export the certificate with key as a pfx file and set SSLCertificate to the filename (full path including extension is recommended). Take a look at the documentation for the SSL related settings which can be found at
https://github.com/Daniel-Svensson/quickfixn/blob/master/tutorial/configuration.md#ssl (unfortunately the formatting is not that pretty at the moment).

If you are still experiencing issues activate quickfixn logging and look into the quickfix events logfile and see if you get any relevant events. If that to fails you might get some help if you post your ssl related settings.

[JanKrivanek]

Thanks Daniel!

It turned out that I need to use certificates in pfx format (despite X509Certificate(2) can happily accept appropriately formatted .pem certificate).

During debugging through your code I found 2 places that I thought might benefit from adjustments - if I'm not just confused and you would agree I can code the suggested adjustments:

1. Server part can validate client certificate against 'custom' CA certificate that is configured by config file, however client side doesn't have this validation code in place (which prevents usage of custom CA certificates without installing them).

2. It is possible to construct server side that doesn't require client side certificate (SSLRequireClientCertificate) or/and also such a server side can exist as an endpoint we try to connect to - in such a case we can use arbitrary certificate on client side, however it might be easier to just use empty value (null) - which is accepted just fine by SslStream.AuthenticateAsCleint, however this code doesn't allow empty 'SSLCertificate' config value.

Thanks
Jan

[JanKrivanek]

During debugging through your code I found 2 places that I thought might benefit from adjustments

Actually - please just disregard the number 2 (I was somehow mistaken when reading the code).
The number 1 would be supper easy to fix - just using the same validation callback for both - client and server SslStream construction. I'm just wondering if there is any deeper reason why it was chosen to use 2 different validation callbacks (differing just in custom CA certificate validation)?

[Formator]

I have also test this PR and I must say @Daniel-Svensson you have done great work. I have also implement SSL support into engine in my branch dev_master but I didn't have time to finished it, so I will use your solution, thank you!

[Daniel-Svensson]

@jakrivan

The number 1 would be supper easy to fix - just using the same validation callback for both - client and server SslStream construction. I'm just wondering if there is any deeper reason why it was chosen to use 2 different validation callbacks (differing just in custom CA certificate validation)?

Actually the initial implementation was quite different for server/client, but I did some cleanup in order to unify it but did not go as far as to unify the functions partly because I find it a bit clearer with separate functions and partly becasue it was easier and took less time to leave the two functions.

Instead of merging the functions completelty I would personally go for an approach where the two separate functions exist but where all common code is moved to a separate certificate verification function.

I am planning to look into making the change in order to allow ca certificate to be specified by clients, but can not promise when I will get to it as I am currently working with some other projects.

[Daniel-Svensson]

@Formator
Thanks for your feedback :)

We have currently been running with the last committed version in production for a month now without being notified of any problems at all.

[gbirchmeier]

I've added this PR to the 1.5.0 milestone, as from your discussions it's looking pretty good (though I haven't tried it myself yet.)

[JanKrivanek]

The only issue we've hit so fare is with '_certificateCache' (QuickFixN\Transport\StreamFactory.cs@69 - lines might be a bit off as we have made a small number of custom changes) is static variable but it's not treated in thread-safe manner. Especially the 'LoadCertificate' static function takes some time between checking the collection and adding the value. In a case where 2 sessions (e.g. quotes and orders for same counterpart) share same certificates it's easy to hit a race condition causing 'object with the same key already exists' when adding into _certificateCache dictionary.
This can be easily solved with ConcurrentDictionary.

[Daniel-Svensson]

@jakrivan, Thanks for pointing that out. I've added some syncronization.

@gbirchmeier what are the current plans for moving to .Net 4, I ve seen a discussion but no actual choice to move the platform? Beeing able to use ConcurrentDictionary would be quite nice (as would a lot of the other features such as Tasks etc)

[JanKrivanek]

Why such a long timeout?
This can actually break heartbeat logic, as when there are no messages from other end, we'd wait 10 seconds to check if HB is needed - meaning we can be up to 10s seconds late with HB. This leads to unnecessary testrequest messages from other end - and some banks are really touchy about this behavior.
How about 1s?

[Daniel-Svensson]

Thanks, it should have been 1s so I've fixed the typo

[gbirchmeier]

@Daniel-Svensson - this stuff looks really great. It might be one of the biggest patches I've received from an outsider, and also one of the most complete. (And you documented stuff! Woohoo!)

Some questions:

• Looks like this code requires .NET 4.0, right?
• Is there any reason you changed the TradeClient to FIX4.3 (was 4.4)?
• I see a lot of new app.config files. What are these for?

[Daniel-Svensson]

@gbirchmeier

Looks like this code requires .NET 4.0, right?

The first version did, but I did backport it to work with .net 3.5 (client profile) again.

So actually the .net framework target of quickfix has changed from 3.5 full to 3.5 client profile meaning that the smaller client installation is enough.

Is there any reason you changed the TradeClient to FIX4.3 (was 4.4)?

During the development I've used a modified quickfix which only include the Fix4.3 messages (as it is the only version we use and the performance benefit during development was quite substantial). I just forgot I've made the change to the example.

I see a lot of new app.config files. What are these for?

They where generated when switching .Net version between 3.5, 4.0 and back again.
I haven't used them for any specific settings so the only purpose they serve right now is to say that the examples can run with only the .Net 3.5 client profile installed (since neither they nor quickfix require anything which is only present in the full/extended .net 3.5 install).

It might very well be possible to just remove the config files without it having any consequences, but since I have not researched the matter I just left them.

[gbirchmeier]

Upmerged to #240.