external-secrets / bitwarden-sdk-server

This repository contains a simple REST wrapper for the Bitwarden Rust SDK
https://external-secrets.io
Apache License 2.0
9 stars 2 forks source link

CA injection via cert-manager in ClusterSecretStore do not work #9

Closed volschin closed 2 months ago

volschin commented 3 months ago

Describe the bug I tried to setup ClusterSecretStore after deployment of the self signed certificate chain and the helm installation. I tried to inject the ca instead of using the caBundle field statically.

apiVersion: external-secrets.io/v1beta1
kind: ClusterSecretStore
metadata:
  name: bitwarden-secrets-manager
  annotations:
    cert-manager.io/inject-ca-from: external-secrets/bitwarden-tls-certs

But it tells me caBundle is a required field. Full message from reconciliation:

ClusterSecretStore/external-secrets/bitwarden-secrets-manager dry-run failed (Invalid): ClusterSecretStore.external-secrets.io "bitwarden-secrets-manager" is invalid: spec.provider.bitwardensecretsmanager.caBundle: Required value

To Reproduce Steps to reproduce the behavior: My deployment is made with flux. My yaml code can be seen here: https://github.com/volschin/home-ops/blob/main/kubernetes/apps/external-secrets/external-secrets/stores/bitwarden-secrets/clustersecretstore.yaml

Expected behavior It should be possible to inject the ca from the generated certificate in the ClusterSecretStore.

Additional In my connected research I found this linkerd issue an pr from some years ago that goes in a same direction an perhaps helps. https://github.com/linkerd/linkerd2/issues/7353

Skarlso commented 3 months ago

Oh interesting. I don't know of that approach. :) I'll take a look on how to implement that.

volschin commented 3 months ago

I‘m surprised, because your hack script explicitly waited for the ca-injector to be ready. ;)

volschin commented 3 months ago

To be honest, at the moment I‘m in a brainstorming phase to use mTLS instead of normal TLS. It would be helpful to have the kind of communication between the services open to such concepts.

Skarlso commented 3 months ago

Hmmm. Not entirely sure how that would work since external secrets isn't necessarily aware of the service. It's handling it as a generic something that has an api.

If it would start checking any certificates it would impact the setup of the whole project too.

It's not impossible I just think it's not necessarily worth the effort. 🤣 but maybe I'm wrong. 😀

Skarlso commented 3 months ago

Hmmm. Then again we are talking about secrets here that potentially could destroy a system. So scratch what I said. Setting up mTLS would be nice.

Skarlso commented 3 months ago

Btw I don't think this ca injector annotation would work here since the caBundle field isn't in the spec. It's under spec provider bitwardenprovider auth. The thing we configure is neither a webhook nor an api service. So I'm not sure how this would work.

Skarlso commented 3 months ago

That said... eso does have it's own injector. I wonder if I could implement something there. I will have to check that out.

Skarlso commented 3 months ago

Hm, this is most likely not going to happen as it requires a bunch of rewrites to our injector and the recommended approach is to either point to a secret or use a bundled certificate. I will add the secret approach as well.

Skarlso commented 3 months ago

Meaning it will get something like this:

// Used to provide custom certificate authority (CA) certificates
// for a secret store. The CAProvider points to a Secret or ConfigMap resource
// that contains a PEM-encoded certificate.
type CAProvider struct {
    // The type of provider to use such as "Secret", or "ConfigMap".
    // +kubebuilder:validation:Enum="Secret";"ConfigMap"
    Type CAProviderType `json:"type"`

    // The name of the object located at the provider type.
    Name string `json:"name"`

    // The key where the CA certificate can be found in the Secret or ConfigMap.
    // +kubebuilder:validation:Optional
    Key string `json:"key,omitempty"`

    // The namespace the Provider type is in.
    // Can only be defined when used in a ClusterSecretStore.
    // +optional
    Namespace *string `json:"namespace,omitempty"`
}

So the source can be a secret or a configmap too.

Skarlso commented 3 months ago

https://github.com/external-secrets/external-secrets/issues/3676

volschin commented 3 months ago

Looks like a really fine & clean solution. 👍

brianramseyau commented 2 months ago

I've just hit into this one as well as the caProvider option (which is in the docs: https://github.com/external-secrets/bitwarden-sdk-server?tab=readme-ov-file#external-secrets) is not in the CRD https://github.com/external-secrets/external-secrets/blob/4f62fb396397bc4973f54dce1744e8de39900a9c/apis/externalsecrets/v1beta1/secretsstore_bitwarden_types.go#L20 also lacking the ref in the upstream api docs as well https://external-secrets.io/latest/api/spec/#external-secrets.io/v1beta1.CAProvider

I have resorted to (for now, for testing) copying the CA bundle from the CertManager output secret and adding in into the caBundle string as a bypass, though this is not ideal as any changes upstream (CertManager) could result in a new CA being required and then its invalidated.

I can see you have an MR thats open which looks at a glace to be addressing this, however :)

volschin commented 2 months ago

@brianramseyau Yes, the upcoming solution would be able to handle a ca certificate rotation also.

Skarlso commented 2 months ago

This will have to be released first in ESO to take effect. So keep an eye out for 0.10.1 for ESO.

Skarlso commented 1 month ago

Finally, ESO 0.10.1 was released :)