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

Ambassador doesn't like Istio mTLS secrets #1475

Closed
ovk opened this issue Apr 29, 2019 · 13 comments
Closed

Ambassador doesn't like Istio mTLS secrets #1475

ovk opened this issue Apr 29, 2019 · 13 comments
Milestone

Comments

@ovk
Copy link

ovk commented Apr 29, 2019

Describe the bug
The Ambassador with Istio guide (https://www.getambassador.io/user-guide/with-istio) suggests to use the following annotation to attach Istio secrets:

apiVersion: ambassador/v1
  kind:  TLSContext
  name: istio-upstream
  hosts: []
  secret: istio.default

Which does not work with the following error:

ambassador.default.1: TLSContext istio-upstream found no certificate in secret istio in namespace default, ignoring...

Upon inspection of the source code https://github.com/datawire/ambassador/blob/528b16c14d26cece0be7719403debf6577e2e097/ambassador/ambassador/ir/ir.py#L291 it looks like the secret name istio.default is parsed incorrectly because the routine assumes that secret name is everything before dot (e.g. istio).

To Reproduce
Just try to follow the guide: https://www.getambassador.io/user-guide/with-istio
It cannot work because istio.default secret name will be parsed as istio.

Expected behavior
If you want to have namespace qualification for secrets, than I think it should be possible to do dot escaping, something like istio\.default to support cases where secret has dot in its name.

Versions (please complete the following information):

  • Ambassador: 0.60.1
  • Kubernetes environment: Minikube
  • Version: k8s 1.14.0
@tomburns
Copy link

tomburns commented Apr 29, 2019

+1, my team ran into this on Friday -- is the Istio mutual TLS scenario described in the documentation no longer supported?

@zombor
Copy link

zombor commented Apr 29, 2019

This is related to #1255

@ovk
Copy link
Author

ovk commented Apr 30, 2019

Upon some investigation I also discovered some inconsistencies between how Istio stores secret istio.default and how Ambassador attempts to read it:

  1. Secret type: Ambassador expects Opaque while Istio has istio.io/key-and-cert.
  2. Key name. Ambassador expects tls.key while Istio has key.pem.
  3. Cert name. Ambassador expects tls.crt while Istio has cert-chain.pem.

Given all this, looks like the mutual TLS between Ambassador and Istio is completely broken at the moment.

There is an easy temporary workaround though: just create secret manually, copy key and cert from the istio.default secret and name everything as mentioned above (also, don't put any dots into the secret name). Then use this secret for Ambassador.

@iNoahNothing
Copy link
Contributor

Thank you for reporting this issue. It appears I did not completely check the correctness of the change to the documentation before publishing it.

Short term, the solution is to simply rollback to the old method of mounting the secret in a volume in the Ambassador container and use it there. Long term, we should work on getting this to work using a TLSContext for the sake of configuration uniformity.

The documentation rollback is happening now and should be completed soon. You can then follow that document to enable mTLS between Ambassador and Istio. I apologize for the inconvenience this has caused and appreciate the effort put in to reporting this issue.

@kflynn kflynn changed the title Secret name in TLSContext is parsed incorrectly: dot in the name always treated as namespace qualification Ambassador doesn't like Istio mTLS secrets May 9, 2019
@kflynn
Copy link
Member

kflynn commented May 9, 2019

Changed the title to reflect that this is distinct from #1255.

@kflynn kflynn added this to the la-sagrada-familia milestone May 9, 2019
@kflynn kflynn modified the milestones: pedrera, sagrada-familia May 20, 2019
@kflynn kflynn modified the milestones: sagrada-familia, santa-cruz Jun 6, 2019
@kflynn kflynn modified the milestones: santa-cruz, santander Jul 2, 2019
@kflynn kflynn modified the milestones: santander, santurtzi Jul 11, 2019
@diptimanadak
Copy link

There is an easy temporary workaround though: just create secret manually, copy key and cert from the istio.default secret and name everything as mentioned above (also, don't put any dots into the secret name). Then use this secret for Ambassador.

Upon some investigation I also discovered some inconsistencies between how Istio stores secret istio.default and how Ambassador attempts to read it:

1. Secret type: Ambassador expects `Opaque` while Istio has ` istio.io/key-and-cert`.

2. Key name. Ambassador expects `tls.key` while Istio has `key.pem`.

3. Cert name. Ambassador expects `tls.crt` while Istio has `cert-chain.pem`.

Given all this, looks like the mutual TLS between Ambassador and Istio is completely broken at the moment.

There is an easy temporary workaround though: just create secret manually, copy key and cert from the istio.default secret and name everything as mentioned above (also, don't put any dots into the secret name). Then use this secret for Ambassador.

I am also stuck with the same issue, could you elaborate the steps, how we can do that

@diptimanadak
Copy link

diptimanadak commented Jul 25, 2019

To give more details about my configuration:

apiVersion: extensions/v1beta1 kind: Deployment metadata: name: ambassador spec: replicas: 3 template: metadata: annotations: sidecar.istio.io/inject: "false" labels: service: ambassador spec: serviceAccountName: ambassador containers: - name: ambassador image: quay.io/datawire/ambassador:0.73.0 resources: limits: cpu: 1 memory: 400Mi requests: cpu: 200m memory: 100Mi env: - name: AMBASSADOR_NAMESPACE valueFrom: fieldRef: fieldPath: metadata.namespace livenessProbe: httpGet: path: /ambassador/v0/check_alive port: 8877 initialDelaySeconds: 30 periodSeconds: 3 readinessProbe: httpGet: path: /ambassador/v0/check_ready port: 8877 initialDelaySeconds: 30 periodSeconds: 3 volumeMounts: - mountPath: /etc/istiocerts/ name: istio-certs readOnly: true restartPolicy: Always volumes: - name: istio-certs secret: optional: true secretName: istio.default

ambassador-service.yaml:
`apiVersion: v1
kind: Service
metadata:
labels:
service: ambassador
name: ambassador
annotations:
getambassador.io/config: |
---
apiVersion: ambassador/v1
kind: Mapping
name: httpbin_mapping
prefix: /httpbin/
service: httpbin.org:80
host_rewrite: httpbin.org
---
apiVersion: ambassador/v1
kind: Module
name: tls
config:
server:
enabled: True
redirect_cleartext_from: 8080
secret: istio.default
---
apiVersion: ambassador/v1
kind: TLSContext
name: istio-upstream
cert_chain_file: /etc/istiocerts/cert-chain.pem
private_key_file: /etc/istiocerts/key.pem
cacert_chain_file: /etc/istiocerts/root-cert.pem
spec:
type: LoadBalancer
ports:

  • name: ambassador
    port: 80
    targetPort: 8080
  • name: ambassador-https
    port: 443
    targetPort: 80
    selector:
    service: ambassador
    `

I am getting configuration error in ambassador dashboard:

CONFIGURATION ERRORS

ambassador.default.2: TLSContext server found no certificate in secret istio in namespace default, ignoring...

@ovk
Copy link
Author

ovk commented Jul 26, 2019

As I mentioned, just creating secret without dot in the name (e.g. istio-default) with contents identical to istio.default, and referencing it in your ambassador deployment volume should help.

@diptimanadak
Copy link

diptimanadak commented Jul 28, 2019

I forgot to mention I was using ambassador version 0.73.0, which does the behave similarly like 0.50.1. Anyway reverting back to 0.50.1 and steps you have mentioned worked for me. There are no errors coming in ambassador logs. But none of the deployed services I am able to access. I am using EKS to deploy the services along with istio and ambassador. Can you guys point out the probable configuration mistakes:

apiVersion: v1
kind: Service
metadata:
  name: customer-portal
  annotations:
    getambassador.io/config: |
      ---
      apiVersion: ambassador/v1
      kind: Mapping
      name: customer-portal_mapping
      prefix: /customer-portal/
      rewrite: /customer-portal
      service:  https://customer-portal:8080
      tls: istio-upstream
spec:
  ports:
  - name: customer-portal
    port: 8080
    targetPort: 8080
    protocol: TCP
  selector:
    app: customer-portal```

@stale
Copy link

stale bot commented Oct 25, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Issue is stale and will be closed label Oct 25, 2019
@ovk
Copy link
Author

ovk commented Oct 25, 2019

I believe the issue is still there.

@stale stale bot removed the stale Issue is stale and will be closed label Oct 25, 2019
@stale
Copy link

stale bot commented Dec 24, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Issue is stale and will be closed label Dec 24, 2019
@richarddli richarddli removed the stale Issue is stale and will be closed label Dec 24, 2019
@richarddli
Copy link
Contributor

#2277 is merged, this will be in the next Ambassador release, out shortly. Thanks @ppeble !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants