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

elastic-agent elasticsearch CA fingerprint support #29128

Merged

Conversation

michel-laterman
Copy link
Contributor

@michel-laterman michel-laterman commented Nov 24, 2021

What does this PR do?

Add support to the elastic-agent for specifying the elasticsearch CA by fingerprint with the --fleet-server-es-ca-trusted-fingerprint flag.

Why is it important?

The elastic stack in 8.0 will turn on security by default. If no certs are provided ES will generate self-signed certs. Using the fingerprint of the CA will let components know what certs to trust.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

  1. Run an Elasticsearch 8.0 Instance with self signed certs
  2. Start a Kibana (8.0) instance the generated enrolment credentials
  3. Enrol an elastic-agent with the --fleet-server-es-ca-sha256 flag
  4. Update obs doc for Elastic Agent and Fleet Server

Related issues

@michel-laterman michel-laterman added enhancement backport-v8.0.0 Automated backport with mergify Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team labels Nov 24, 2021
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Nov 24, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Nov 24, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-12-10T00:45:23.408+0000

  • Duration: 95 min 24 sec

  • Commit: 787cdd5

Test stats 🧪

Test Results
Failed 0
Passed 7136
Skipped 16
Total 7152

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@jlind23
Copy link
Collaborator

jlind23 commented Dec 1, 2021

@belimawr this may be interesting for you :-)

@@ -51,7 +51,7 @@ func verifyCAPin(hashes []string, verifiedChains [][]*x509.Certificate) error {

// Fingerprint takes a certificate and create a hash of the DER encoded public key.
func Fingerprint(certificate *x509.Certificate) string {
hash := sha256.Sum256(certificate.RawSubjectPublicKeyInfo)
hash := sha256.Sum256(certificate.Raw)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@simitt, I've tried the change you suggested here and my tests have rejected the certificate

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you be a bit more specific what rejected the certificate means? And did you convert the ES fingerprint from hex to base64 before configuring it?

Copy link
Contributor

Choose a reason for hiding this comment

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

And since it is self-sigend, you need to also configure the CA with the current libbeat code. I mentioned this in my first comment, as it might be a problem if only the fingerprint is passed down from Fleat.

Copy link
Collaborator

Choose a reason for hiding this comment

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

both hex and basic should still be accepted right? Can we know if certs are hex or basic in order to avoid convertion when not needed?

@jlind23
Copy link
Collaborator

jlind23 commented Dec 2, 2021

@michel-laterman could you also perform some test with the standalone agent? Certificate should then be pass into the elastic agent configuration file.
ping @mostlyjason @joshdover

@michel-laterman
Copy link
Contributor Author

michel-laterman commented Dec 2, 2021

I have tested in fleet-mode and got it to work* with a custom snapshot build of the elastic-agent and fleet-server binary.

The agent->ES communication is done through fleet-server.
The fleet-server uses libbeat to parse tls config and passes the output of

func (c *TLSConfig) ToConfig() *tls.Config {

as the http.RoundTripper.TLSClientConfig when creating a go-elasticsearch client. This means that all certificate verification is currently done by libbeat instead of go-elasticsearch.

To build my test binaries I cherry-picked my current PR commits to 8.0 and built a snapshot for the elastic-agent.
When building the fleet-server binary I based off of 8.0 and updated the libbeat version to the latest release (v7.15.2), injected the fingerprinting code from #29229, and forced it to use the new VerificationFingerprintOnly mode.

The binaries "worked" as fleet-server was able to connect to ES and get cluster info (the agent then failed at DNS stuff).

I'll clear the fingerprint handling from this PR and let @belimawr's pr handle it.
However, in order to get fingerprinting working with elastic-agent/fleet we will need to make sure that when a sha256 fingerprint and no cert is detected the new VerificationFingerprintOnly mode is used (likely as another commit to this PR) and we will need to make another PR on fleet-server to update the version of beats/libbeat it uses to one that includes a backport from #29229.

@joshdover
Copy link
Contributor

As the discussion has progress some things have changed. We should instead be using a CLI arg name called fleet-server-es-ca-trusted-fingerprint to match the ca_trusted_fingerprint config added in libbeat: #29229

@michel-laterman michel-laterman marked this pull request as ready for review December 8, 2021 16:01
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

@michel-laterman michel-laterman requested a review from a team December 8, 2021 16:06
@mergify
Copy link
Contributor

mergify bot commented Dec 9, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b agent-es-ca-fingerprint upstream/agent-es-ca-fingerprint
git merge upstream/master
git push upstream agent-es-ca-fingerprint

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@blakerouse blakerouse 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 adding it to the container command. This overall looks great and ready to go.

@michel-laterman michel-laterman merged commit fcb7b81 into elastic:master Dec 14, 2021
@michel-laterman michel-laterman deleted the agent-es-ca-fingerprint branch December 14, 2021 19:28
mergify bot pushed a commit that referenced this pull request Dec 14, 2021
* initial commit

* Update Fingerprint method

* Switch to using newly added CATrustedFingerprint attribute

* rename flag

* Fix broken flag

* Add CHANGELOG

* Add container support

(cherry picked from commit fcb7b81)
michel-laterman added a commit that referenced this pull request Dec 15, 2021
* initial commit

* Update Fingerprint method

* Switch to using newly added CATrustedFingerprint attribute

* rename flag

* Fix broken flag

* Add CHANGELOG

* Add container support

(cherry picked from commit fcb7b81)

Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v8.0.0 Automated backport with mergify enhancement Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants