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

Add Permissions to Client Certificates #546

Closed
1 of 2 tasks
pivotal-jamil-shamy opened this issue Jul 18, 2017 · 24 comments
Closed
1 of 2 tasks

Add Permissions to Client Certificates #546

pivotal-jamil-shamy opened this issue Jul 18, 2017 · 24 comments
Assignees

Comments

@pivotal-jamil-shamy
Copy link

pivotal-jamil-shamy commented Jul 18, 2017

  • Defect
  • Feature Request or Change Proposal

Feature Request

Use Case:

When mutual TLS is enabled by specifying --tlsverify flag, in theory a client no longer needs to pass a username and password to authenticate, since that was already being taken care of when verifying the client certificate.
Looking at the code, we see that the username and password are still being used when mutual TLS is enabled; specifically to set the permissions for a client.

Proposed Change:

Allow the inclusion of a client permissions inside the client certificate itself. This way, each client is issued a cert with the permissions baked into its cert.

A X509 certificate can hold some metadata in itself (the RFC https://www.ietf.org/rfc/rfc5280.txt refers to that). The approach would be when creating a client certificate, add the permissions to the cert metadata itself, then after successfully authenticating the client gnatsd server can extract the permissions from the client certificate and injects these permissions into the user's session.

Who Benefits From The Change(s)?

With this approach we can technically add "users" dynamically to an already running server by creating a client certificate with the intended permissions.

Note: The client cert itself can also hold more info about the client (probably client name) for trace and audit purposes.

@tylertreat
Copy link
Contributor

I'm not dismissing this idea as it's interesting and I think worth consideration, but I did want to point out that as of v1.0 you can dynamically add/remove users and change permissions on a running server through config reload: https://github.com/nats-io/gnatsd#process-signaling. This might provide an alternative solution?

@xtreme-sameer-vohra
Copy link

Hi @tylertreat
The considerations of a certificate based solution vs. dynamically reloading config;
Pros
- NATs server is stateless
- Scales well in a clustered environment. It avoids the complex workflow of modifying nats.cfg file and signalling the NATs process, which is even more challenging in a clustered NATs setup

Cons
- Revoking individual certificates is harder. A CA would need to be removed from nats.cfg
- A new cert would have to be generated when an client's permissions would change

@derekcollison derekcollison self-assigned this Jun 13, 2018
@derekcollison
Copy link
Member

Have you considered JWTs? I am looking into these a bit more for some future work.

@pivotal-jamil-shamy
Copy link
Author

@derekcollison we ended up doing both authentication and authorization over Mutual TLS with a twist.

Each NATS client certificate has the client type and id embedded in the cert. So the certificate does not explicitly have the permissions.

Then when the server gets the client cert, it extracts the client ID and Type from it and applies it to a template of what it can publish/subscribe to.

For example: If client type is agent and client id is 456, then we have a template defined for client types agent that allows that client to only publish to channel agent.456 and listen to agent.456.* .

Let us know if you're interested more into what we have implemented.

@derekcollison
Copy link
Member

Interesting, thanks for the info. Do you allows updates to permissions after the fact? How do you deal with revocation? Thanks again.

@bruth
Copy link
Member

bruth commented Jul 2, 2018

@pivotal-jamil-shamy Interesting solution. I am assuming you applied this to a fork of gnatsd?

Per @derekcollison's question, my guess is that revocation isn't technically required since the indirection with the server templates are what ultimately dictate the permission? I could imagine a control plane layer that manages these template/permission mappings.

@derekcollison If you are looking for a more flexible solution beyond JWTs (which also suffer from the revocation issue), Open Policy Agent (OPA) may be good for this use case. It is also incubating in the CNCF. It is implemented in Go so can be embedded or it can be deployed as a sidecar.

@derekcollison
Copy link
Member

I will look at OPA (we presented together at an end-user meeting). JWTs will be most flexible IMO since they can be completely decoupled and self describing without additional operational components.

@pivotal-jamil-shamy
Copy link
Author

@bruth yep, this was implemented into a fork of gnatsd. Open Policy Agent seems inetersting, will look into that.
@derekcollison in our system, we usually don't need to revoke individual clients permissions. What we end up doing is a periodic rotation of the whole system Certificate Authorities which will ensure no old client certs can be valid forever.

@derekcollison
Copy link
Member

Agree with short lived permissions. We will encourage this in the JWT scheme we will deliver in Q3. We do feel a revocation system is needed and we are in the process of designing how that would work.

@nerdoftech
Copy link

Each NATS client certificate has the client type and id embedded in the cert. So the certificate does not explicitly have the permissions.

Then when the server gets the client cert, it extracts the client ID and Type from it and applies it to a template of what it can publish/subscribe to.

For example: If client type is agent and client id is 456, then we have a template defined for client types agent that allows that client to only publish to channel agent.456 and listen to agent.456.* .

@pivotal-jamil-shamy - Is there any documentation for this feature?

If not:

  • I'll see if my company might be willing to put someone's time for a documentation PR.
  • Can you point me to the you point me to the sections of code that handle this?

@derekcollison
Copy link
Member

We now support pulling user id from a client certificate, and will add a bit more custom version too. Still do not forsee adding in auth to the a x.509 client cert though.

@pivotal-jamil-shamy
Copy link
Author

@nerdoftech
The code is still in a fork, you can find it in this branch : https://github.com/bosh-dep-forks/gnatsd/tree/bosh-1.3.0
This diff is mostly the new code and tests that was added bosh-dep-forks@0a80ce5
This tests here explain the feature in details https://github.com/bosh-dep-forks/gnatsd/blob/bosh-1.3.0/test/certificate_authorization_test.go , although there is no official documentation about it yet.

cc @xtreme-andrew-su @h4xnoodle

@derekcollison
Copy link
Member

Would be great to work with you to merge necessary items back into master so you don't have to keep a fork. Merging to get to 2.0 release may be challenging. Be happy to discuss on a call.

@nerdoftech
Copy link

nerdoftech commented Feb 5, 2019

After doing a bit of research of who is who, I have a better understanding of everyone involved.

@pivotal-jamil-shamy - Thanks for sending that info over, however, I did not realize that it was in a forked repo. I appreciate your response anyhow.

@derekcollison - After spending a few hours researching, I think I found I was looking for like you mentioned and it is exactly what we need. I'm new to Go, so please let me know if I got this right.

In this config file there is an option verify_and_map :
https://github.com/nats-io/gnatsd/blob/f10020ed42b934b4fac7178b98c46ed4fa628cc4/test/configs/tls_cert_id.conf#L16

This sets tc.Map and then o.TLSMap to true:
https://github.com/nats-io/gnatsd/blob/8361155fb25cbe218958c59ce8f225abd5252f28/server/opts.go#L1841-L1847
https://github.com/nats-io/gnatsd/blob/8361155fb25cbe218958c59ce8f225abd5252f28/server/opts.go#L470

This eventually gets to the method isClientAuthorized were it picks up cert.EmailAddresses[0] from the Go TLS ConnectionState and Go x509 Certificate struct types and maps it to the user:
https://github.com/nats-io/gnatsd/blob/6b2b4d9bafe92a3e11bf434c8d860acf26766dd5/server/auth.go#L246
https://github.com/nats-io/gnatsd/blob/6b2b4d9bafe92a3e11bf434c8d860acf26766dd5/server/auth.go#L306
https://github.com/nats-io/gnatsd/blob/6b2b4d9bafe92a3e11bf434c8d860acf26766dd5/server/auth.go#L318
https://github.com/nats-io/gnatsd/blob/6b2b4d9bafe92a3e11bf434c8d860acf26766dd5/server/auth.go#L327-L328

One question I have, I saw that the user's password is still checked later in the method:
https://github.com/nats-io/gnatsd/blob/6b2b4d9bafe92a3e11bf434c8d860acf26766dd5/server/auth.go#L426-L427

Would I be correct to assume that user.Password and c.opts.Password are nil values with TLS map and would therefore match, then consider the user authenticated?

@derekcollison
Copy link
Member

Yes that is correct. We have also added an additional mapping type that uses Go's Subject for x.509 certs. /cc @wallyqs

@nerdoftech
Copy link

Yes that is correct. We have also added an additional mapping type that uses Go's Subject for x.509 certs. /cc @wallyqs

Awesome! While I cant make a commitment without getting permission from my company, I am going to ask about contributing a documentation PR to the docs repo according to the contribution guidelines.

It looks like add info to these two pages might be the right place:
https://github.com/nats-io/nats-site/blob/master/content/documentation/managing_the_server/security.md

https://github.com/nats-io/nats-site/blob/master/content/documentation/managing_the_server/authentication.md

@derekcollison
Copy link
Member

We appreciate any and all PRs, so if you are able please send away, and thanks in advance.

@nerdoftech
Copy link

Yes that is correct. We have also added an additional mapping type that uses Go's Subject for x.509 certs. /cc @wallyqs

Ah, I just noticed that this code is not yet in a release tag. I'm only allowed to use feature in an official release.

I saw this PR #896 and the changes are approved. Are you able to share an ETA on when this feature will be release?

@wallyqs
Copy link
Member

wallyqs commented Feb 7, 2019

Hi @nerdoftech I just merged that branch so would be part of the next release which would happen sometime in the next few weeks I think.

@nerdoftech
Copy link

Thanks, @wallyqs !

@JensRantil
Copy link

Can this be closed or should we wait for next release before closing?

@h4xnoodle
Copy link

Seems like 2.0.0-RCX has the functionality. We're trying it out in cloudfoundry/bosh

@derekcollison
Copy link
Member

@h4xnoodle Great, ping us directly on Slack or email if you have any questions.

@derekcollison
Copy link
Member

@h4xnoodle I wanted to check in with you. I wanted to make sure that using NATS inside of BOSH and other things at VMW was going well.

If not let's jump on a video call, I would like to understand the issues and anything we can do to make NATS better inside VMW.

@bruth bruth closed this as completed Dec 29, 2022
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

9 participants