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

TLS passphrase is unnecessarily mandatory #299

Open
akunszt opened this issue Mar 17, 2021 · 0 comments
Open

TLS passphrase is unnecessarily mandatory #299

akunszt opened this issue Mar 17, 2021 · 0 comments

Comments

@akunszt
Copy link

akunszt commented Mar 17, 2021

I tried to use a private key and the certificate with the following configuration - do we really have to modify the _helpers.tpl for TLS configuration?.

tls:
  cas:
    - path: /etc/kraken/tls-server/ca.crt
  client:
    disabled: true
  server:
    cert:
      path: /etc/kraken/tls-server/tls.crt
    key:
      path: /etc/kraken/tls-server/tls.key

This failed with a not very informative error message.

2021-03-17T10:47:12.323331000Z 2021-03-17T10:47:12.323Z FATAL   cmd/cmd.go:187  invalid TLS config: stat : no such file or directory

After I checked the source it looks like the missing passphrase is correctly handled in the

// parseKey reads key from file and decrypts if passphrase is provided.
but not in the
return fmt.Errorf("invalid TLS config: %s", err)
. The nginx.go assumes that the passphrase is set all the time.

Adding /dev/null as a passphrase file is a viable - albeit ugly - workaround for this.

tls:
  cas:
    - path: /etc/kraken/tls-server/ca.crt
  client:
    disabled: true
  server:
    cert:
      path: /etc/kraken/tls-server/tls.crt
    key:
      path: /etc/kraken/tls-server/tls.key
    passphrase:
      path: /dev/null

I don't know if it's intentional or not. I didn't find any reference about the configuration at all, just some examples without any explanations - I still don't know what the tls.name is used for. The comment at the parseKey function indicates that the passphrase should be optional.

The expected behaviour should be

  • Fix the nginx.go to handle missing passphrase files.
  • Improve the error message to contain an information about which configuration element have issues and a filename if it's available.
  • If you think the passphrase should be mandatory then that should be documented and also the /dev/null trick can be mentioned there.
  • A configuration reference would be nice but this is not related to this issue.

I opened this issue as the error message was a bit misleading and it took me a lot of time to trace back what actually happened. Also I think a passphrase file should be only an optional thing. With cert-manager and correct RBAC in place the private key can't be read by anyone and if someone has shell/exec access to the running pod then he/she can fetch both the encrypted key and the passphrase too.

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

1 participant