TT-12364 Secret wrongly imported into Tyk Redis

g-bohncke commented 5 months ago

I noticed that when a secret contains a full chain in the tls.crt it gets wrongly imported into Tyk OSS and stored in redis.

When I have a ApiDefinition that refers to a secret

apiVersion: tyk.tyk.io/v1alpha1
kind: ApiDefinition
metadata:
  name: myapideftemplate
  labels:
    name: my-ingress
    template: "true"
spec:
  name: foo
  use_keyless: true
  protocol: https
  listen_port: 8443
  certificate_secret_names:
    - test-tls
  active: true
  proxy:
    target_url: http://example.com
    strip_listen_path: true
  version_data:
    default_version: Default
    not_versioned: true
    versions:
      Default:
        name: Default
        paths: {}
---
apiVersion: v1
data:
  tls.crt: 
  tls.key:
kind: Secret
metadata:
  name: test-tls
  namespace: default
type: kubernetes.io/tls

The tls.crt would contain a leaf + intermediate + root

This will result in Redis in only storing the inter and the root. And a lot of error on the operator of trying to re-upload the cert and it not finding the cert with that same identifier and on the Gateway errors with not being able to find the cert.

{"level":"info","ts":1712180399.9318595,"logger":"controllers.SecretCert","msg":"Call","cert":"monitoring/server-cert-prometheus","Method":"GET","URL":"https://gateway-control-svc-tyk-oss-gw.tyk-oss.svc.cluster.local:9696/tyk/certs/81ea3bdd4c15ddedc27ca5d82f95f63143c043cc0273bdfa03e54543035e0f8e","Status":404}
{"level":"info","ts":1712180399.9319787,"logger":"controllers.SecretCert","msg":"Not Found","cert":"monitoring/server-cert-prometheus","body":"{\"status\":\"error\",\"message\":\"Certificate with given SHA256 fingerprint not found\"}\n"}
{"level":"error","ts":1712180399.932014,"logger":"controllers.SecretCert","msg":"failed to get certificate","cert":"monitoring/server-cert-prometheus","error":"Resource not found","stacktrace":"github.com/TykTechnologies/tyk-operator/pkg/client/gateway.Cert.Exists\n\t/workspace/pkg/client/gateway/cert.go:46\ngithub.com/TykTechnologies/tyk-operator/pkg/client/klient.Certificate.Exists\n\t/workspace/pkg/client/klient/universal_client.go:165\ngithub.com/TykTechnologies/tyk-operator/controllers.isCertificateAlreadyUploaded\n\t/workspace/controllers/secret_cert_controller.go:291\ngithub.com/TykTechnologies/tyk-operator/controllers.(*SecretCertReconciler).Reconcile\n\t/workspace/controllers/secret_cert_controller.go:198\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\t/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.9.0/pkg/internal/controller/controller.go:298\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.9.0/pkg/internal/controller/controller.go:253\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\t/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.9.0/pkg/internal/controller/controller.go:214"}
{"level":"info","ts":1712180399.9759226,"logger":"controllers.SecretCert","msg":"Call","cert":"monitoring/server-cert-prometheus","Method":"POST","URL":"https://gateway-control-svc-tyk-oss-gw.tyk-oss.svc.cluster.local:9696/tyk/certs","Status":403}

time="Apr 03 21:38:02" level=warning msg="Can't retrieve certificate:81ea3bdd4c15ddedc27ca5d82f95f63143c043cc0273bdfa03e54543035e0f8eopen 81ea3bdd4c15ddedc27ca5d82f95f63143c043cc0273bdfa03e54543035e0f8e: no such file or directory" prefix=cert_storage
time="Apr 03 21:38:03" level=warning msg="Can't retrieve certificate:81ea3bdd4c15ddedc27ca5d82f95f63143c043cc0273bdfa03e54543035e0f8eopen 81ea3bdd4c15ddedc27ca5d82f95f63143c043cc0273bdfa03e54543035e0f8e: no such file or directory" prefix=cert_storage
time="Apr 03 21:38:03" level=warning msg="Can't retrieve certificate:81ea3bdd4c15ddedc27ca5d82f95f63143c043cc0273bdfa03e54543035e0f8eopen 81ea3bdd4c15ddedc27ca5d82f95f63143c043cc0273bdfa03e54543035e0f8e: no such file or directory" prefix=cert_storage

-----BEGIN CERTIFICATE-----
some inter cert
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
some root cert
-----END CERTIFICATE---

Where as with a cert only containing the leaf it would result in and a working application and the following result in Redis

-----BEGIN CERTIFICATE-----
some inter cert
-----END CERTIFICATE-----
-----BEGIN RSA PRIVATE KEY-----
the private key
-----BEGIN RSA PRIVATE KEY-----

Are intermediates not supported? even if the root is stored in the ca.crt and not in the tls.crt. the same issue persists.

This issue is Tyk specific since the same secrets work fine with nginx ingress controller and is also according to the documentation of the certmanager.io https://stackoverflow.com/questions/45796058/how-do-i-add-an-intermediate-ssl-certificate-to-kubernetes-ingress-tls-configura https://cert-manager.io/docs/configuration/ca/ According to cert-manager: "Note: If your issuer represents an intermediate, ensure that tls.crt contains the issuer's full chain in the correct order: issuer -> intermediate(s) -> root. The root (self-signed) CA certificate is optional, but adding it will ensure that the correct CA certificate is stored in the secrets for issued Certificates under the ca.crt key. If you fail to provide a complete chain, it might not be possible for consumers of issued Certificates to verify whether they're trusted."

g-bohncke commented 5 months ago

Looking more in the issue it doesn't seem to be an issue with the operator because this code https://github.com/TykTechnologies/tyk-operator/blob/master/pkg/client/dashboard/cert.go#L76 actualy does a correct upload. so it seems the root of this is in the GW and Dashboard code.

komalsukhani commented 5 months ago

Thank you @g-bohncke for raising it. Currently, we are investigating it.

g-bohncke commented 2 months ago

After a lot of trying I figured out what is going wrong and sadly it's due to line breaks. if the Secret tls.crt & tls.key are not created with a linebreak at the end the import will go wrong.

resulting into trying to import

-----BEGIN CERTIFICATE----- some inter cert -----END CERTIFICATE----------BEGIN RSA PRIVATE KEY----- the private key -----BEGIN RSA PRIVATE KEY-----

I would suggest to make the code more robust. by altering https://github.com/TykTechnologies/tyk-operator/blob/master/pkg/client/dashboard/cert.go#L76 combined := make([]byte, 0) combined = strings.TrimSpace(append(combined, key...)) combined = append(combined, "\n") combined = strings.TrimSpace(append(combined, crt...)) body := &bytes.Buffer{}

TykTechnologies / tyk-operator

TT-12364 Secret wrongly imported into Tyk Redis #688