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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client authentication #3

Closed
tekjar opened this issue Jul 2, 2016 · 22 comments
Closed

Client authentication #3

tekjar opened this issue Jul 2, 2016 · 22 comments
Assignees

Comments

@tekjar
Copy link

tekjar commented Jul 2, 2016

Hi. If I use this library for writing a client. Can I set client certificate in the API for third party servers to do client authentication ?

To be more specific, I'm trying to write client code using your library (Thanks 馃槃 ) . I'm able to authenticate server by providing the CA (root_store .add_pem_file(ca.crt)). Can I also simultaneously add client certificate for 3rd party servers to authenticate this client?

(Didn't use tls much before. Plz let me know if my question itself doesn't make sense)

@ctz
Copy link
Member

ctz commented Jul 4, 2016

At the minute client auth isn't supported. Can I ask what API you're using that requires it?

@tekjar
Copy link
Author

tekjar commented Jul 4, 2016

I'm writing a mqtt client library where client authentication is pretty common

@ghost
Copy link

ghost commented Jul 10, 2016

  • Why is client authentication found in the list of

broken, obsolete, badly designed, underspecified, dangerous and/or insane

things (in the docs?); while it is true that client certs are uncommon, they don't seem any of the above things to me. There are indeed real-world applications that make use of this.

  • Are there technical issues with rustls at the moment preventing implementation of this feature?

@ctz
Copy link
Member

ctz commented Jul 10, 2016

(copying and pasting my explanation from elsewhere)

Why is client authentication found in the list of (..) things (in the docs?)

There's a few problems with it.

  1. client auth in TLS1.2 and earlier is done at the wrong time in the handshake. As a result the client's identity (which unlike the server identity usually identifies a user so is PII) is sent in the clear. That's a big privacy failure.
  2. to work around (1), some implementations do an initial server-auth handshake, then immediately renegotiate up to mutual-auth (renegotiations are encrypted). Renegotiation has quite a dismal history, and I definitely want to avoid it.
  3. as a follow on from (2), the standard never described what implementations are expected to do if client/server identities change during renegotiation. This (partially) resulted in https://mitls.org/pages/attacks/3SHAKE

All of these are fixed in TLS1.3: client identities are encrypted and renegotiation is dropped.

Actually implementing client auth is quite a minor amount of work. It's probably best to do as a opt-in feature.

@tekjar
Copy link
Author

tekjar commented Jul 10, 2016

Even as an opt in is awesome. Will wait for this feature :)

@sciyoshi
Copy link
Contributor

@ctz I've started taking a look at how you would implement TLS 1.2 client authentication. You can see the initial changes are here. The main issue I've run into now is that the CertificateVerify message requires computing a signature of all previous handshake messages - however, ring's API doesn't support passing a pre-computed digest. That means that computing a signature from a running digest as messages are exchanged won't work without some changes to ring.

Any thoughts on how you would approach that?

@briansmith
Copy link
Contributor

ring's API doesn't support passing a pre-computed digest. That means that computing a signature from a running digest

I would be happy for somebody to add this functionality to ring. It is needed for many reasons and it would be easy to do.

@ctz
Copy link
Member

ctz commented Jul 18, 2016

I've also been working on-and-off on client auth. I ought to push my in-progress branch. But I could use a lot of your progress so far, so thanks!

Any thoughts on how you would approach that?

At the minute I've made HandshakeHash keep the input when compiled with the clientauth feature.

The ideal solution is to have HandshakeHash compute all supported hashes in parallel, which would also obviate the need to buffer the whole ClientHello at the layer above as we do currently. As noted, that needs ring support for client auth.

@sciyoshi
Copy link
Contributor

@ctz if you push up your branch I can compare our approaches and see what still needs work. I've opened an issue in ring here: briansmith/ring#253.

@sciyoshi
Copy link
Contributor

@ctz got client auth working from your branch! Thanks for the work in implementing this. Are you also planning on adding client auth support from the server end?

There's also a small issue where the signature/hash algorithm for the CertificateVerify message is calculated based on the selected ciphersuite, and not based on the capabilities of the selected certificate.

@ctz
Copy link
Member

ctz commented Jul 19, 2016

Are you also planning on adding client auth support from the server end?

Yes that should be easier.

There's also a small issue where the signature/hash algorithm for the CertificateVerify message is calculated based on the selected ciphersuite, and not based on the capabilities of the selected certificate.

Yes, that issue is also present in the normal server authentication. There and here rustls treats certificates as opaque, so will happily use a key with a cert not containing appropriate key usages, expired certs, etc. That kind of misconfiguration is hopefully rare, but I agree it's not ideal for rustls to plough on regardless. In any case, that information is mostly for the relyer.

@samscott89
Copy link

Since supporting EdDSA signatures would require maintaining the full message transcript anyway, is the idea that certain configs could disable EdDSA and hence only need to store one (or several) rolling hashes?

@tekjar
Copy link
Author

tekjar commented Jul 31, 2016

Hi. I see a client auth branch 馃槃 Thanks a lot. Is this ready to be merged to master? (Also any plans for adding rustls to crates.io?)

@ctz
Copy link
Member

ctz commented Aug 4, 2016

It's close to done for client auth by clients, but not for servers yet. It's still in progress, but it'll be a week or so before there's more progress due to travel and stuff :)

@ctz ctz self-assigned this Aug 13, 2016
@ctz
Copy link
Member

ctz commented Aug 14, 2016

Now pushed.

@ctz ctz closed this as completed Aug 14, 2016
@tekjar
Copy link
Author

tekjar commented Aug 29, 2016

Thanks :)

@salmankhalid-plt
Copy link

I know this has been closed by any chance to having Server side client authentication as well ?

@djc
Copy link
Member

djc commented Jul 16, 2023

@salmankhalid-plt what do you mean? As I understand it, only servers can authenticate clients, so that's naturally what we already support. What use case do you have in mind that you think rustls doesn't yet support?

@mouse07410
Copy link

I've no idea what the OP meant. However:

As I understand it, only servers can authenticate clients

This does not come even close to the specification of TLS. Of course, both ends can authenticate each other - search for "mutual TLS" (which will also explain why there is no separate spec for mTLS).

@djc
Copy link
Member

djc commented Jul 17, 2023

I think that matches what I said? Servers can authenticate clients, and clients authenticate servers.

@ctz
Copy link
Member

ctz commented Jul 17, 2023

The current state is client auth is supported on both the client- and server-side.

@mouse07410
Copy link

mouse07410 commented Jul 17, 2023

I think that matches what I said? Servers can authenticate clients, and clients authenticate servers.

The way I understood your reply was that only server authenticates client, while the OP asked for the client to authenticate server:

As I understand it, only servers can authenticate clients

Otherwise, we seem to be on the same page.

ctz pushed a commit that referenced this issue Feb 9, 2024
There were two bugs. webpki didn't:

1. read the X.509 Name Constraints field in its entirety, nor

2. check the certificate subject against the constraints correctly

(1) is a simple fix, (2) requires reading the Common Name from the
certificate.

Requires lifting some DER parsing logic from ring to parse UTF8String
and Set fields. Ring doesn't support those and isn't likely to in the
near future, see briansmith/ring#1265.

Closes #3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants