search

haskell-tls / hs-tls

TLS/SSL implementation in haskell
Other
403 stars 92 forks source link

Support for client certificates #13

Closed mgrabmueller closed 12 years ago

mgrabmueller commented 12 years ago

I would be interested in adding support for client certificates to the tls package. I've seen that you already have some infrastructure in place for that, but that it is currently disabled. Before I start to work on that: have there been any big challenges in adding client certificate support, or have you just postponed it because of lack of need or lack of time. So if you think it would be a good idea to work on that, or if you know of someone else who is doing it, drop me a line -- either here or at martin@grabmueller.de. Thanks for your good work!

vincenthz commented 12 years ago

Hi martin, adding working client certificates is a very welcome contribution. it's on my list but sadly lack of time means it's still missing. There's no big challenge to client certificate support, namely you need to finish the support for certificate request (relatively easy) and certificate verify (a bit harder).

certrequest is a just a "simple" list of authorized signature/hash algorithms and a list of CAs.

for certverify you need to have multiple cryptographic hash contexts "running" on every handshake packet (except hellorequest), and the list of supported hash is agreed at hello time between the client and server. It's perfectly acceptable and reasonable to not support every possible hashes at the same time, but it need to be a runtime option.

The last step of certverify is a carefully coded RSA signature(on client) and verify(on server).

Let me know if you have any questions/issues about the client certificate or the tls code itself.

mgrabmueller commented 12 years ago

For now, I have just one question: on which git branch should I base my patches?

vincenthz commented 12 years ago

significant additions should happens in the next branch.

mgrabmueller commented 12 years ago

Just wanted to tell you about my progress so far.

I have basic client certificate support working, in the sense that clients and servers based on the tls package can exchange client certificates. My patches are available on my repo at

https://github.com/mgrabmueller/hs-tls

and include the following:

Open issues:

[1] I'm not sure that the hash calculation is correct. I also haven't checked yet whether TLS1.2 with its SHA-256 hash must be supported. [2] I'm not sure about RSA signing. It seems to work when my implementation talks to itself, but nginx gives a DecryptError alert with the following debug output:

2012/07/12 21:08:24 [crit] 4616#0: *18 SSL_do_handshake() failed (SSL: error:04091068:rsa routines:INT_RSA_VERIFY:bad signature error:1408807A:SSL routines:SSL3_GET_CERT_VERIFY:bad rsa signature) while SSL handshaking, client: 127.0.0.1, server: localhost

I have not really understood the necessary PKCS#1 signing etc., so would appreciate some input.

[3] The coding style differs from yours. [4] The design of the configuration needs to be discussed. [5] The design of the callbacks needs to be discussed. [6] There is no support for non-RSA certificates. [7] There is no support for anonymous servers.

The two last ones are probably not so important, since the rest of the package does not seem to support them either.

I have merged your changes on branch next, and I will open a pull request so that you can check out the code without too much hassle. But if you prefer to check out my repo instead, that's fine with me and you can close the pull request at will.

For testing, I have extended the tls-simple-client from the tls-debug package with client certificate support. The code can be found here

https://github.com/mgrabmueller/hs-tls-debug/tree/next

Additionally, I have extended my simple TLS echo server, which is available here for testing:

darcs get http://community.haskell.org/~martin/repos/tls-examples/

I hope you can manage to have a look at the code, because I would like to have some feedback on the issues listed above before proceeding.

Thanks in advance for looking into this.

mgrabmueller commented 12 years ago

Oops, forgot something.

The patches are on branch ccert of my hs-tls repo.

vincenthz commented 12 years ago

Ok i've replied inline to your patches for hopefully some useful feedback. It's looking good in general, I think it's in a good state already.

couple of more generic feedback: it would really help to have smaller commits: It's much easier for me to integrate smaller commits, since i can merge straight away the non controversial (or the one without feedback) commits straight away. Which gives you the ability to see what has been merged too.

Note that it's not necessarily practical during development to commits only partial commits, but i would warmly recommend "git gui" which allow you to stage line by line or hook by hook (and commit the staged code), and would give you the ability to make smaller commits without changing your development flow.

mgrabmueller commented 12 years ago

Thanks for looking. I have answered some of your questions inline. I will look into the suggestions and fix whatever I can. With respect to smaller patches: I have learnt my way around the codebase while implementing everything, so it was difficult to factor out small patches. In addition, I am learning my way around git at the same times, which adds to the confusion. I will certainly look into "git gui", thanks for the pointer. For integration into your repo, I think I will probably start a new branch in my repo based on your master branch and port my changes back, committing them in more manageable chunks.

mgrabmueller commented 12 years ago

Just to keep you informed: I have started a new branch for my development here:

https://github.com/mgrabmueller/hs-tls/tree/client-certificate

It's based on your next branch and I have made smaller, more manageable commits.

mgrabmueller commented 12 years ago

Good news! I got basic client certificate support to work! Of course, there may still be some problems lurking in the code, but after a lot of debugging, I can show some working code.

I have tested the following scenarios:

In all cases, connections with valid client certificate and key work, and connections with invalid client certificate or key are rejected.

What is missing:

You can find the working TLS 1.0 under the following tag:

https://github.com/mgrabmueller/hs-tls/tree/client-certificate-TLS1.0

edsko commented 12 years ago

Hi Martin,

I'm very excited about your branch, it's precisely what I need -- thanks!

I having some difficulties getting it to work though :-/ I have cloned your forks of hs-certificate, hs-tls, hs-tls-extra, and hs-tls-debug, and checked out branches dnames, client-certificate, next, and next, respectively. I then use the simple client from hs-tls-debug to try and connect to Windows Azure, which requires a client certificate for authentication. I modified the simple client slightly so that it wouldn't ask for / 

main = do
  sStorage <- newIORef undefined
  args     <- getArgs
  let hostname = "management.core.windows.net"
  let port = 443 
  certs <- do
    cert    <- fileReadCertificate $ args !! 0
    pk      <- fileReadPrivateKey $ args !! 1
    return [(cert, Just pk)]
  runTLS (getDefaultParams sStorage Nothing certs) hostname (fromIntegral port) $ \ctx -> do
    handshake ctx
    sendData ctx $ LC.pack $ concat 
      [ "GET /<<my subscription id>>/services/hostedservices HTTP/1.1\r\n"
      , "host: " ++ hostname ++ "\r\n" 
      , "content-type: application/xml\r\n"
      , "x-ms-version: 2010-10-28\r\n"
      , "\r\n"
      ]
    d <- recvData' ctx
    bye ctx
    LC.putStrLn d
    return ()

(I can send precisely the same request using openssl s_client, and that works). However, when I then run the client, I get something like

debug: >> Handshake [ClientHello TLS10 (ClientRandom .. 
debug: << Handshake [ServerHello TLS10 (ServerRandom .. 
debug: >> Handshake [ClientKeyXchg .. 
debug: >> ChangeCipherSpec
debug: >> Handshake [Finished "\231\201\147\165\134\167\246\138\141\169L\155"]
debug: << ChangeCipherSpec
debug: << Handshake [Finished "\186'\151r\252\213\&3\EMjZ\159c"]
debug: >> AppData "GET /<<my subscription id>>/services/hostedservices HTTP/1.1\r\nhost: management.core.windows.net\r\ncontent-type: application/xml\r\nx-ms-version: 2010-10-28\r\n\r\n"
debug: << Handshake [HelloRequest]
debug: >> Handshake [ClientHello TLS10 (ClientRandom .. 
tls-simpleclient: Error_Packet_Parsing "Failed reading: certrequest distinguishname not of the correct size\nFrom:\thandshake\n\n"

Any suggestions?

vincenthz commented 12 years ago

@mgrabmueller This is looking very promising. I'm currently in holidays (until the end of the week), and i can't look and comment on the pull request in details, but will do as soon as possible. Thanks !

mgrabmueller commented 12 years ago

@edsko This results (at least I think so) from a strange restriction in the TLS 1.0 spec, which requires the list of distinguished names to be at least 3 bytes long. Vincent has coded this check into hs-tls, but other TLS implementations send empty distinguished name lists. You can try it at home with

openssl s_server -accept 4322 -key scripts/server.key -cert scripts/server.crt -verify 1 -tls1

(note that no -CAfile option is given) and connect with tls-simpleclient to localhost:4322

TLS1.1 has lifted this restriction (which should be fixed in hs-tls, which only allows empty lists for TLS1.2). @vincenthz Maybe we should allow empty lists even for TLS1.0? Maybe as an option?

vincenthz commented 12 years ago

yeah it's probably best to remove the restrictions. the spec quite often doesn't reflect reality with old ssl/tls.

edsko commented 12 years ago

Right, just tried it with simply commenting out that check and with your TLS version 1.0 I can connect to Azure and login successfully! That's great :) Thanks a lot.

vincenthz commented 12 years ago

@edsko: great news. I'll start merging things soon.

edsko commented 12 years ago

@vincenthz, @mgrabmueller: just tried Vincent's merged version (tls branch next, tls-extra branch next certificate branch master) and it worked :) Just FYI.

mgrabmueller commented 12 years ago

@edsko: great to hear that. @vincenthz: thanks for merging my commits. If you run into any issues or have reports wrt. client certificates, I would be glad to look into them. Additionally, if you have any comments on the code or spot any security problems, just let me know. I am still quite excited about this projects, so expect other pull requests from me :)

vincenthz commented 12 years ago

@mgrabmueller: thanks again for the client certificate feature ! I'm closing this issue, and will probably generalize the certificate management in the near future, hopefully to simplify user side for client and server certificates.

vincenthz commented 12 years ago

@mgrabmueller: is onUnverifiedClientCert really useful for anything or just for debug ? I'm not sure I understand why the server could accept the certificate chain if the client is not able to sign properly the request.

mgrabmueller commented 12 years ago

I implemented it first for debugging, but then had the idea that it might be interesting for the application to see when a handshake fails because of an improperly signed request. Other than that, I also don't see a use case, so you can remove it if you like. Another idea would be to replace it with a notification.