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

proxy_ssl_trusted_certificate getting cert and then crl on the same line #3732

Closed
brianehlert opened this issue Apr 6, 2023 Discussed in #3731 · 4 comments · Fixed by #3737
Closed

proxy_ssl_trusted_certificate getting cert and then crl on the same line #3732

brianehlert opened this issue Apr 6, 2023 Discussed in #3731 · 4 comments · Fixed by #3737
Assignees

Comments

@brianehlert
Copy link
Collaborator

Discussed in #3731

Originally posted by coolbry95 April 6, 2023
I believe this is breaking from this line here https://github.com/nginxinc/kubernetes-ingress/pull/3632/files#diff-0a308ab187fcddbbc7a73510aacc512d8a04022d75c6f95044b1e3fd4c483770R738.

Before it would just return the full path for the cert but now its returning the cert and the crl.

proxy_ssl_trusted_certificate /etc/nginx/secrets/asdf-trusted-ca-ca.crt /etc/nginx/secrets/asdf-trusted-ca-ca.crl;

apiVersion: k8s.nginx.org/v1
kind: Policy
metadata:
  name: egress-mtls-oauth2
spec:
  egressMTLS:
    trustedCertSecret: egress-trusted-ca
    verifyServer: true
    verifyDepth: 25
    serverName: true
    sessionReuse: true
    sslName: oauth2.googleapis.com
apiVersion: k8s.nginx.org/v1
kind: Policy
metadata:
  name: egress-mtls-pubsub
spec:
  egressMTLS:
    trustedCertSecret: egress-trusted-ca
    verifyServer: true
    verifyDepth: 25
    serverName: true
    sessionReuse: true
    sslName: pubsub.googleapis.com
apiVersion: k8s.nginx.org/v1
kind: VirtualServer
metadata:
  name: oauth
spec:
  host: oauth2.googleapis.com
  tls:
    secret: egress-proxy-tls
  policies:
    - name: ingress-mtls
    - name: egress-mtls-oauth2
  upstreams:
  - name: oauth
    service: oauth
    port: 443
    tls:
      enable: true
  routes:
  - path: /
    action:
      pass: oauth
apiVersion: k8s.nginx.org/v1
kind: VirtualServer
metadata:
  name: pubsub
spec:
  host: pubsub.googleapis.com
  tls:
    secret: egress-proxy-tls
  policies:
    - name: ingress-mtls
    - name: egress-mtls-pubsub
  upstreams:
  - name: pubsub
    service: pubsub
    port: 443
    type: grpc
    tls:
      enable: true
  routes:
  - path: /
    action:
      pass: pubsub
```</div>
@brianehlert
Copy link
Collaborator Author

@coolbry95 can you please describe "I believe this is breaking from this line here"

@coolbry95
Copy link
Contributor

coolbry95 commented Apr 6, 2023

Before the PR the code is like this

https://github.com/nginxinc/kubernetes-ingress/blob/v3.0.2/internal/configs/configurator.go#L724

func (cnf *Configurator) addOrUpdateCASecret(secret *api_v1.Secret) string {
	name := objectMetaToFileName(&secret.ObjectMeta)
	data := GenerateCAFileContent(secret)
	return cnf.nginxManager.CreateSecret(name, data, nginx.TLSSecretFileMode)
}

If you looks at what CreateSecret is returning its just the filename not the filename and the crl.
https://github.com/nginxinc/kubernetes-ingress/blob/v3.0.2/internal/nginx/manager.go#L207

func (lm *LocalManager) CreateSecret(name string, content []byte, mode os.FileMode) string {
	filename := lm.GetFilenameForSecret(name)

	glog.V(3).Infof("Writing secret to %v", filename)

	createFileAndWriteAtomically(filename, lm.secretsPath, mode, content)

	return filename
}

https://github.com/nginxinc/kubernetes-ingress/blob/v3.0.2/internal/nginx/manager.go#L229

func (lm *LocalManager) GetFilenameForSecret(name string) string {
	return path.Join(lm.secretsPath, name)
}

After the PR it is returning the crtFileName and crlFilename. This lines up with what we are seeing on the proxy_ssl_trusted line.
https://github.com/nginxinc/kubernetes-ingress/blob/v3.1.0/internal/configs/configurator.go#L731

func (cnf *Configurator) addOrUpdateCASecret(secret *api_v1.Secret) string {
	name := objectMetaToFileName(&secret.ObjectMeta)
	crtData, crlData := GenerateCAFileContent(secret)
	crtSecretName := fmt.Sprintf("%s-%s", name, CACrtKey)
	crlSecretName := fmt.Sprintf("%s-%s", name, CACrlKey)
	crtFileName := cnf.nginxManager.CreateSecret(crtSecretName, crtData, nginx.TLSSecretFileMode)
	crlFileName := cnf.nginxManager.CreateSecret(crlSecretName, crlData, nginx.TLSSecretFileMode)
	return fmt.Sprintf("%s %s", crtFileName, crlFileName)
}

Maybe I am wrong but looking at the PR that is the only code change that sticks out to me.

@coolbry95
Copy link
Contributor

Here is the error reported by nginx.

nginx@egress-proxy-aws-us-east-1-nginx-ingress-controller-777786xt7r8:/$ nginx -T
nginx: [emerg] invalid number of arguments in “proxy_ssl_trusted_certificate” directive in /etc/nginx/conf.d/vs_egress-proxy-qa_oauth.conf:100
nginx: configuration file /etc/nginx/nginx.conf test failed

@shaun-nx shaun-nx self-assigned this Apr 6, 2023
@shaun-nx
Copy link
Contributor

shaun-nx commented Apr 6, 2023

Thanks for reporting this @coolbry95 I'll be working to address this issue. I'll update this thread with the PR for the fix when it's available.

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

Successfully merging a pull request may close this issue.

3 participants