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

When using auth-tls-secret, ssl_verify_depth might also be needed #110

Closed
euank opened this issue Jan 5, 2017 · 8 comments
Closed

When using auth-tls-secret, ssl_verify_depth might also be needed #110

euank opened this issue Jan 5, 2017 · 8 comments

Comments

@euank
Copy link
Contributor

euank commented Jan 5, 2017

The ssl_verify_depth option will have to be set for many configurations. Typically, it should be set to a value equal to the length of the certificate chain.

The default of 1 works fine if the certificate infrastructure only includes a root + client certs.

However, many configurations will include a root and intermediate and sign client certs with the intermediate. In that case, the default of 1 will result in an unworking setup.

I think our options are:

  1. Change the ssl_verify_depth to match the length of the cert chain provided
  2. Allow the user to configure the verify depth (via an annotation on the controller? an annotation on the tls secret used?)

1 seems like a good idea. There might be some configurations where this new proposed default ends up being wrong and we obviously need to be careful about fiddling with client-cert related options, but none come to mind immediately.

@pieterlange
Copy link
Contributor

While we're on the subject (i wasn't able to get this to work yet with the native ingress annotations - it seems like this is not officially supported yet) i'd like to configure ssl_verify_client as well.

@euank - do you know what key in the secret needs to contain the CA chain? I tried getting it to work with ca.crt in a separate secret, but that required me to set a dummy tls crt/key as well.
I might note that i'm terminating the actual connection with a different chain than the client certs i'm verifying - just to complicate matters ;-). I ended up just adding the vhost to the nginx.tmpl manually 👎

@aledbf
Copy link
Member

aledbf commented Jan 9, 2017

@pieterlange you need to specify one secret with 3 keys:

tls.crt
tls.key
ca.crt

I need to add an example (before the release)

@pieterlange
Copy link
Contributor

I know - my problem here is that i'm actually validating the client certs against a different CA then is configured at the TLS section of the ingress - so i'm unsure what to add in the tls.crt and tls.key sections of the secret.

@euank
Copy link
Contributor Author

euank commented Jan 9, 2017

@pieterlange I'm using this currently and it's working for me. I entered the same values for tls.crt and tls.key as ca.crt. I'm also using a separate secret for tls auth with different certs. tls.crt and tls.key are ignored.

The only change I had to make was the verify depth (which I just hardcoded over here: https://github.com/euank/ingress/tree/verify-depth-2).

My ingress looks something like this:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: test-ingress-auth
  annotations:
    ingress.kubernetes.io/auth-tls-secret: "default/auth-ca-chain"
spec:
  tls:
  - hosts:
    - test.auth.host
    secretName: test.auth.host-le-cert
  rules:
  - host: test.auth.host
    http:
      paths:
      - path: "/"
        backend:
          serviceName: echoheaders-x
          servicePort: 80

And then my secret:

apiVersion: v1
kind: Secret
metadata:
  name: auth-ca-chain
data: 
  tls.key: $data
  tls.crt: $data
  ca.crt: $data

Where my $data is a base64 encoding of

-----BEGIN CERTIFICATE-----
INTERMEDIATE
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
ROOT
-----END CERTIFICATE-----

@rikatz
Copy link
Contributor

rikatz commented Feb 2, 2017

I don't know if this is a good behaviour, of setting tls.key and tls.crt also for a secret containing only CAs accepted in mutual authentication.

Maybe using a separate opaque secret wouldn't be better?

@rikatz
Copy link
Contributor

rikatz commented Feb 7, 2017

@euank Take a look at the PR #235 and see if that's the expected behaviour. :)

@euank
Copy link
Contributor Author

euank commented Mar 27, 2017

Confirming that @rikatz's PR fixes this for me. Setting an annotation of ingress.kubernetes.io/auth-tls-verify-depth: "2" in the ingress in question gets the behaviour I want.

Thanks a ton @rikatz 🙇

@euank euank closed this as completed Mar 27, 2017
@afsilvasantos
Copy link

Hi @euank

And then my secret:

kind: Secret
metadata:
  name: auth-ca-chain
data: 
 tls.key: $data
 tls.crt: $data
 ca.crt: $data 

Was the "ca.crt" the root CA, and tls.key and tls.crt the intermediate one? Or did you combine into the ca.crt both the root and intermediate CA?

Thanks!

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

5 participants