-
Notifications
You must be signed in to change notification settings - Fork 1k
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
DataNode: Client Cert Generation #18522
Conversation
graylog2-server/src/main/java/org/graylog2/bootstrap/preflight/web/resources/CAResource.java
Show resolved
Hide resolved
graylog2-server/src/main/java/org/graylog2/bootstrap/preflight/web/resources/CAResource.java
Outdated
Show resolved
Hide resolved
graylog2-server/src/main/java/org/graylog2/bootstrap/preflight/web/resources/CAResource.java
Outdated
Show resolved
Hide resolved
graylog2-server/src/main/java/org/graylog/security/certutil/csr/ClientCertGenerator.java
Outdated
Show resolved
Hide resolved
} | ||
|
||
public void removeCertFor(final String role, final String principal) throws IOException { | ||
var certFile = certFilePath(principal); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If principal
has multiple role
values associated, those would collide, and we would remove the certificate for all of them.
I think it's safer to base the file name on the hash of role + principal
and use the resulting string everywhere in this class, including when create the private key store.
I'm not sure how the UI is going to represent the client certificates that exist, so we might need to store the role
, principal
pair somewhere else, or get it from the role mapping. But I think that is an independent question from this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general: how would using the file system work if you have multiple nodes, especially if those nodes might not be online at the same time?
I might be missing something but if this is a concern, the data needs to live somewhere else, probably in the database.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the first draft - to make it right, I think we have to figure out where we're heading with the CA stuff:
- If it's a one time, throw away cert, we don't have to save the PK anywhere because we might not want to reuse it at all
- we could let the user create their own CSR, no need to generate a PK then
- roles etc. could be more than one, we should save that state to MongoDB - but again: I'd like to give it at least a little thought before I'm creating sth. that needs a migration in the future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, that makes sense. In that case, we should consider a follow-on PR that doesn't store the certificate at all (maybe post-beta, so we don't hold this up too much).
If we never read it again, it doesn't make much of a difference whether it lives on a single node.
eea5e7c
to
1978a36
Compare
4791996
to
d0cfac8
Compare
d0cfac8
to
9723c3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, I've created a follow-on issue #18587 were we can see about the file system requirement for generated certs.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works as expected. lgtm.
thank you for adding this
Description
Generate Client Certs for 3rd party connection to OpenSearch
Exporting the certs from the JSON and calling
curl --cacert ca.pem --cert cert.pem --key key.pem https://localhost:9200/_aliases
works.
Grafana works if you only configure TLS client auth and disable SSL key checking. Otherwise it complains about the ca cert.
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: