Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate TLS mutual auth support #27

Closed
artemredkin opened this issue May 5, 2019 · 25 comments
Closed

Investigate TLS mutual auth support #27

artemredkin opened this issue May 5, 2019 · 25 comments
Labels
kind/support Adopter support requests.

Comments

@artemredkin
Copy link
Collaborator

No description provided.

@Lukasa
Copy link
Collaborator

Lukasa commented May 5, 2019

Do you really mean MTLS, or do you mean mutual auth?

@markm77
Copy link

markm77 commented May 5, 2019

@Lukasa
Copy link
Collaborator

Lukasa commented May 6, 2019

Ok, cool: we should probably be consistent in calling that “TLS Mutual Auth” rather than MTLS, which is an ambiguous and uncommon abbreviation.

@markm77
Copy link

markm77 commented May 6, 2019

This requirement comes from a spec which mandates use of TLS mutual auth and refers to it as MTLS. But I can try to avoid that terminology here. : )

@Lukasa
Copy link
Collaborator

Lukasa commented May 6, 2019

Yeah, I saw that in the spec! It’s a strange choice. 😂 Regardless, the requirement is reasonable and NIO has the hooks to support it, though there are problems with the interaction with HTTP/2 and connection pooling that need to be considered.

@markm77
Copy link

markm77 commented May 6, 2019

Many thanks Cory. Actually I was referring to another API spec which I'll link by private email for your interest.

@weissi weissi changed the title Investigate MTLS support Investigate TLS mutual auth support May 6, 2019
@weissi
Copy link
Contributor

weissi commented May 6, 2019

@markfinlabs As Cory points out, NIOSSL does support mutual authentication today which works just fine for HTTP/1. I wasn't sure if that was clear from Cory's comment. This is how you would use it:

client

let key = try NIOSSLPrivateKey(file: "/tmp/clientkey/key.pem", format: .pem) { completion in
    completion("password for client's private key".utf8)
}

var tlsConfiguration = TLSConfiguration.forClient()
tlsConfiguration.trustRoots = .file("/tmp/serverkey/cert.pem") // only necessary if the server uses a self-signed cert or a custom CA that isn't trusted by the system. If your server has a real cert, you can delete this line
tlsConfiguration.privateKey = NIOSSLPrivateKeySource.privateKey(key)
tlsConfiguration.certificateChain = [.file("/tmp/otherclientkey/cert.pem")] // same here
tlsConfiguration.certificateVerification = .noHostnameVerification // or `.fullVerification`

And this TLSConfiguration you can just set in Configuration.tlsConfiguration. Therefore swift-nio-http-client will support this just fine today.

server

let key = try NIOSSLPrivateKey(file: "/tmp/serverkey/key.pem", format: .pem) { completion in
    completion("password for server's private key".utf8)
}

var tls = TLSConfiguration.forServer(certificateChain: [.file("/tmp/serverkey/cert.pem")],
                                     privateKey: .privateKey(key))
tls.trustRoots = .file("/tmp/clientkey/cert.pem") // again, only necessary if self-signed or own CA that isn't trusted by the system
tls.certificateVerification = .noHostnameVerification // or `.fullVerification`

let sslContext = try! NIOSSLContext(configuration: tls)

@markm77
Copy link

markm77 commented May 6, 2019

Thanks for this reply, very helpful. We should get a chance to test this next week and I'll report back.

@weissi weissi added the kind/support Adopter support requests. label May 8, 2019
@artemredkin
Copy link
Collaborator Author

@markfinlabs Did you get the chance to test it?

@markm77
Copy link

markm77 commented Jun 22, 2019

Hi @artemredkin. Sorry to drop the ball on this - our key engineer working on NIO suddenly resigned. I had a go myself to test this tonight but can you tell me something basic which I couldn't find quickly in the docs: how best to generate request.body for a form, i.e. Content-Type == "application/x-www-form-urlencoded" - i.e. is there a helper method to convert name/value pairs?

@artemredkin
Copy link
Collaborator Author

@markfinlabs hi, unfortunately there is no way to do that out of the box. If you need application/x-www-form-urlencoded, you can do the following (given you can convert your parameter values to string representation):

func escape(_ value: String) -> String? {
    return value.addingPercentEncoding(withAllowedCharacters: .urlHostAllowed)
}
func encode(key: String, value: String) -> String? {
    return escape("\(key)=\(value)")
}
func encode(parameters: [String: String]) -> String {
    return parameters.compactMap(encode).joined(separator: "&")
}

request.body = .string(encode(parameters: ["a": "b"]))

@ianpartridge
Copy link
Contributor

Interesting. Do you think we should add convenience API for application/x-www-form-urlencoded? application/json is another which might benefit from some API that accepts a Codable.

@artemredkin
Copy link
Collaborator Author

Good idea, filed #50

@markm77
Copy link

markm77 commented Jun 24, 2019

Thanks @artemredkin for code sample and just to update you. I tested with the external server (external company) I am working with and unfortunately got sslError([Error: 268435638 error:100000b6:SSL routines:OPENSSL_internal:NO_RENEGOTIATION]) which looking at s4.2 of https://tools.ietf.org/html/rfc5746 may be a problem with the external server and a security feature on your side? npm request worked fine with this server. Do you have any comment on this? I think I'll try to use a different external server (different company) but will take a few days to sort this out.

@weissi
Copy link
Contributor

weissi commented Jun 24, 2019

I think that’s because of this: https://github.com/google/boringssl/blob/master/PORTING.md#tls-renegotiation

Because renegotiation is a mess protocol and security wise, BoringSSL (and therefore nio-ssl) disables it by default. But @Lukasa will know in much more detail and if that’s something we would ever want to support despite the messiness. I’d say definitely not with at least the user asking for it to be turned on.

@markm77
Copy link

markm77 commented Jun 24, 2019

Thanks @weissi, that makes sense. I'll send you a private reply via the email on your GitHub home page to avoid putting some information into the public domain.

@Lukasa
Copy link
Collaborator

Lukasa commented Jun 25, 2019

We should add renegotiation support, but have it off by default, and require explicit user intervention to enable it. See apple/swift-nio-ssl#110 for tracking that. With that feature in place it should be possible to work around this issue.

@markm77
Copy link

markm77 commented Jun 25, 2019

Thanks @Lukasa, sounds super helpful as I don't think the server I am interfacing with is likely to change quickly. I will still endeavour to test with another server (different company) this week.

@weissi
Copy link
Contributor

weissi commented Jun 27, 2019

@markfinlabs, @Lukasa implemented this yesterday: apple/swift-nio-ssl#111 . So in theory you can test this today using swift package edit or you'll wait for the swift-nio-ssl version 2.2.0 release which shouldn't be long.

@markm77
Copy link

markm77 commented Jun 27, 2019

Thanks @weissi, great news. I can wait for v2.2.0 to re-check.

@markm77
Copy link

markm77 commented Jun 29, 2019

I have now tested with a second server and everything works fine (including server using custom CA). 😀

@artemredkin
Copy link
Collaborator Author

Great news! Thank you for testing this.

@markm77
Copy link

markm77 commented Jun 30, 2019

No pbs, I'll check the renegotiation support after NIO SSL v2.2.0 is released and report back here.

@markm77
Copy link

markm77 commented Aug 26, 2019

Just coming back to this for completeness. I successfully used TLS mutual auth with the original server I had a problem with using these options:

                tlsConfiguration.certificateVerification = .none
                tlsConfiguration.renegotiationSupport = .once

So thanks for your efforts here everyone.

@ozshabat
Copy link

Hey @weissi this issue is related to a TLS authentication. Can you help me with that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/support Adopter support requests.
Projects
None yet
Development

No branches or pull requests

6 participants