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

Consider allowing custom certificates on the transport layer #6479

Closed
pebrc opened this issue Mar 2, 2023 · 9 comments · Fixed by #6727
Closed

Consider allowing custom certificates on the transport layer #6479

pebrc opened this issue Mar 2, 2023 · 9 comments · Fixed by #6727
Assignees
Labels
>enhancement Enhancement of existing functionality

Comments

@pebrc
Copy link
Collaborator

pebrc commented Mar 2, 2023

Existing functionality:

The user provided CA and the private key needs to be exposed to the ECK operator to issue certificates. It will create a Kubernetes secret to contain all certificates and private keys for each of the nodes in the Elasticsearch cluster.

Pre-generating the certificates is impractical because the number of Elasticsearch nodes may grow and shrink as the cluster scales. The operator can of course deal with this by updating the certificate secret as needed. But a user would not.

To allow an even tighter integration with existing certificate infrastructure and decoupling of the certificate generation from the ECK operator we should investigate if we can support https://cert-manager.io/docs/projects/csi-driver/
This would allow us to mount node specific certificates without sharing keys.

Trade-offs:

  • user experience: user has to setup and configure cert-manager and cert-manager CSI driver
  • we would no longer be able to use the otherName SAN required for trust restrictions in ESS (thorough testing of CCS/CCR with ESS would be part of this work) Support SAN/dnsName for restricted trust elasticsearch#91946 relaxes the requirement on newer versions of ES somewhat by allowing SAN/dnsName as well.
  • where do the users responsibilities end: does the user create the issuer and reference it in the ES custom resource? Are we going to build an explicit (optional) dependency on cert-manager into ECK
  • cert-manager performance at scale is problematic (not an issue for majority of users)

Plan:

  1. Introduce transport.tls.certificateAuthorities.configMapName and http.tls.certificateAuthorities.configMapName (for symmetry) to allow users to specify additionally trusted CAs (enables tools like trust-manager, and allows ECK to uses those in remote cluster associations)
  2. Remove the warning when users set xpack.security.transport.ssl.key, xpack.security.transport.ssl.certificate (to discuss: should this be only be allowed when they set transport.tls.certificate.trust ?)
  3. Add a page to our documentation on transport based on the example in Consider allowing custom certificates on the transport layer #6479 (comment) plus the certificateAuthorities attribute.
@botelastic botelastic bot added the triage label Mar 2, 2023
@pebrc pebrc added the >enhancement Enhancement of existing functionality label Mar 5, 2023
@botelastic botelastic bot removed the triage label Mar 5, 2023
@pebrc
Copy link
Collaborator Author

pebrc commented Mar 19, 2023

After some experimentation I was able to confirm my theory that using custom certificates with cert-manager csi-driver is already possible today. The overriding of the transport SSL config settings leads to warnings in the logs that we should maybe reconsider if we want to make this an officially supported way of running. Also worth noting that the existing setting for the transport level CA just happens to work as is because the CSI driver generated cert secrets contain the CA under the ca.crt key.

apiVersion: elasticsearch.k8s.elastic.co/v1
kind: Elasticsearch
metadata:
  name: es
spec:
  version: 8.6.2 
  nodeSets:
  - name: mixed
    count: 3
    config:
      xpack.security.transport.ssl.key: /usr/share/elasticsearch/config/transport-certs/tls.key
      xpack.security.transport.ssl.certificate: /usr/share/elasticsearch/config/transport-certs/tls.crt
      node.store.allow_mmap: false
    podTemplate:
      spec:
        containers:
        - name: elasticsearch
          volumeMounts: 
          - name: transport-certs
            mountPath: /usr/share/elasticsearch/config/transport-certs
        volumes: 
        - name: transport-certs
          csi: 
            driver: csi.cert-manager.io
            readOnly: true
            volumeAttributes: 
              csi.cert-manager.io/issuer-name: ca-cluster-issuer
              csi.cert-manager.io/dns-names: "${POD_NAME}.${POD_NAMESPACE}.svc.cluster.local"

What I have not tested yet is how well this setup works with remote clusters (both external ones and ones managed by ECK)

@pebrc
Copy link
Collaborator Author

pebrc commented Mar 20, 2023

Adding some more notes on remote cluster support in combination with the cert-manager csi-driver based approach to transport:

  • our remote cluster automation is broken with the above manifest unless the CA certificate +key is also declared in transport.tls.certificate.secretName
    • this means exposing the CA private key as well to the operator and kind of defeats the purpose of using the CSI driver to begin with (i.e. not having to expose the CA private key to ECK)
    • our current implementation/validation of transport TLS secrets does not allow the common format used by cert-manager with the keys ca.crt, tls.crt, tls.key (we should relax that independently of this issue)
    • A potential workaround could be to allow a secret with a ca.crt only and copy that to the transport-certs-public secret we use in the remote cluster automation.
    • However I question the usefulness of the latter as that would mean a customer/user would have to manually create a secret with only the ca.crt and then more importantly update it whenever the CA is rotated. All this effort is for very little gain when it comes to the improvement in security posture. The operator can read all secrets anyway.

I am assuming that remote cluster support works right away when set up manually given that the issue I described is purely a matter of pulling the right ca.crt out of an existing secret. Once I declared the TLS secret remote cluster support started working for the in-cluster type of remote cluster association.

@pebrc
Copy link
Collaborator Author

pebrc commented Mar 23, 2023

However I question the usefulness of the latter as that would mean a customer/user would have to manually create a secret with only the ca.crt and then more importantly update it whenever the CA is rotated.

I forgot that tools like https://cert-manager.io/docs/projects/trust-manager/ exist which can propagate the public part of a cert-manager created secret to other namespaces.

We should therefore support the following secret data contents variations:

  • tls.crt, tls.key (legacy)
  • ca.crt, ca.key (new ECK specific format)
  • tls.crt, tls.key, ca.crt (cert-manager default format)
  • ca.crt (trust-manager projection from a cert-manager secret)

UPDATE: this is incorrect. trust manager uses config maps

@pebrc
Copy link
Collaborator Author

pebrc commented Mar 26, 2023

Design decisions to make here:

  • should the operator generate the necessary CSI driver volumes as spelled out in the example above Consider allowing custom certificates on the transport layer #6479 (comment)
    • best user experience, we would add a single attribute e.g. transport.tls.certificate.issuer or similar that would turn on cert-manager mode
  • should we leave the specifics of the certificate sourcing to the user
    • are there other tools similar to cert-manager csi driver that we would exclude
    • if we do this how do we solve the config warning users get when they try to configure reserved configuration attributes related to transport (UX problem)
  • we need to trust the user provided CA (mainly for the remote cluster feature)
    • we need a way to express trust
      1. transport.tls.certificate.trust.configMapName or similar to allow users to specify a trust bundle e.g. coming from cert-manager's trust-manager
      2. try to figure out the CA from the issuer that we potentially have in transport.tls.certificate.issuer (that requires explicit reliance on cert-manager and would be the way to go for the first implementation option )

@pebrc
Copy link
Collaborator Author

pebrc commented Apr 23, 2023

I suggest we do the following:

  1. Introduce transport.tls.trust.configMapName and http.tls.trust.configMapName (for symmetry) to allow users to specify additionally trusted CAs (enables tools like trust-manager, and allows ECK to uses those in remote cluster associations)
  2. Remove the warning when users set xpack.security.transport.ssl.key, xpack.security.transport.ssl.certificate (to discuss: should this be only be allowed when they set transport.tls.certificate.trust ?)
  3. Add a page to our documentation on transport based on the example in Consider allowing custom certificates on the transport layer #6479 (comment) plus the trust attribute.

@barkbay
Copy link
Contributor

barkbay commented Apr 24, 2023

Introduce transport.tls.trust.configMapName and http.tls.trust.configMapName (for symmetry) to allow users to specify additionally trusted CAs (enables tools like trust-manager, and allows ECK to uses those in remote cluster associations)

Just to double check I understand correctly: would it automatically setup xpack.security.transport.ssl.certificate_authorities? Would we have any expectations as to the content of theseconfigMaps ?

Reading the Elasticsearch documentation about xpack.security.transport.ssl.certificate_authorities I wonder if each key/file in the configMap should contain only one certificate:

List of paths to PEM encoded certificate files that should be trusted.

@pebrc
Copy link
Collaborator Author

pebrc commented Apr 24, 2023

Just to double check I understand correctly: would it automatically setup xpack.security.transport.ssl.certificate_authorities? Would we have any expectations as to the content of these configMaps ?

Yes that would be the idea. The expectation for the contents would be one or more PEM encoded x509 certificates. Elsticsearch is able to parse more than one out of a single file. This would allow us to support tools like trust manager.

@barkbay
Copy link
Contributor

barkbay commented Apr 24, 2023

Thanks for your explanation on how xpack.security.transport.ssl.certificate_authorities is handled. I think your proposal makes sense 👍

Nit: naming, I wonder if we should try to reuse the word certificate_authorities to make it more explicit that we are expecting CAs here:

  transport:
    tls:
      ca:
        configMapName: my-bundle

or

  transport:
    tls:
      certificateAuthorities:
        configMapName: my-bundle

@pebrc
Copy link
Collaborator Author

pebrc commented Apr 24, 2023

I liked trust because it is nice and short, ca is singular and hides the fact that it could be more than one. I updated the main description with the plan and your proposal to call it certificateAuthorities

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement Enhancement of existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants