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

Fix readiness probe script to let xpack.security.http.ssl.client_authentication be set to required #5762

Closed

Conversation

ryanhockstad
Copy link
Contributor

It looks like currently the xpack.security.http.ssl.client_authentication setting won't work if it is set to 'required.' This is caused when the readiness_probe.sh tries to ping https://localhost:9200, but it doesn't present a cert or key file. The fix is to simply set the --key and --cert flags of the curl request. This still works when ECK is configured to use custom certs because the operator always puts them in the /usr/share/elasticsearch/config/http-certs/ directory, and it also continue to work even when SSL is disabled because the cert files will still be there.

It's worth pointing out that this setting isn't listed in the Settings Managed by ECK doc page, which is why I decided to fix it.

@botelastic botelastic bot added the triage label Jun 12, 2022
@ryanhockstad ryanhockstad added >bug Something isn't working and removed triage labels Jun 13, 2022
Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR! I think the problem I see with this is that changing the readiness probe alone won't put us into a position to enable client certificate verification. I think we would also have to at least adjust the HTTP client used by the operator to present a certificate to still be able to reconcile clusters that have xpack.security.http.ssl.client_authentication enabled.

@ryanhockstad
Copy link
Contributor Author

Oh yeah, that makes sense. I only tested this out on a 3 node cluster to see if it would spin up, but I didn't think about the operator making requests to the cluster.

That also makes me realize the other CRDs would need to be edited to present certs when connecting to ES, too.

Yeah so this is a way bigger lift than I realized at first. Should we edit this doc page to have xpack.security.http.ssl.client_authentication as a setting users shouldn't change, then?

@pebrc
Copy link
Collaborator

pebrc commented Jun 15, 2022

Yes I think we can do two things. We can update the documentation to highlight this limitation and we can open an issue to address this in a more comprehensive manner. Is there a specific use case you had where you strictly required client certificate verification or was this just an experiment?

@ryanhockstad
Copy link
Contributor Author

I had a client who wanted client certificate verification enabled, but they were ok moving on without it when I realized ECK doesn't support it. After that I was just interested in figuring out how we could get it to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants