googleforgames / agones

Dedicated Game Server Hosting and Scaling for Multiplayer Games on Kubernetes
https://agones.dev
Apache License 2.0
6.01k stars 796 forks source link

Helm chart: allocator service client TLS key is not stored anywhere #1698

Closed endragor closed 4 years ago

endragor commented 4 years ago

Is your feature request related to a problem? Please describe. With agones.allocator.generateTLS=true the Helm chart generates client CA, but doesn't store the key anywhere. That seems to make it impossible to actually make a client that could connect to the allocator service. At the same time, setting generateTLS=false results in the need to also provide the server certificate/key pair, which is not really necessary for most use cases.

Describe the solution you'd like generateTLS=true could store both client certificate and key as a secret. That way the clients could use the secret or a CI job could copy it, if necessary.

Also, generateTLS could be split into generateServerTLS and generateClientTLS, so that users may choose to provide client certificate but let the chart generate server certificates.

pooneh-m commented 4 years ago

Thanks for reporting the issue. Regarding client TLS there is an issue already to provide a default secret #1686.

For the server TLS, the generated secret does not have the endpoint of the agones-allocator. The reason is that the allocator IP address of agones-allocator is only allocated after it is installed. However, we can provide a helm configuration such as agones.allocator.dnsName to generate a valid self-signed server certificate for the DNS name. Does this solve your problem for the server certificate?

The generated TLS is useless otherwise without the client skip the CN verification, which is not recommended. The agones-allocator documentation recommends regenerating agones-allocator server certificate.

endragor commented 4 years ago

I saw #1686, but wasn't sure if it was the same. And I'm still not sure.

What I reported here is that agones.allocator.generateTLS=true seems to be useless, because the client key gets discarded. So no one would be able to connect to the allocator service when using this option. Please correct me if I'm wrong. The suggestion is to not discard the key, but also store it in a secret.

pooneh-m commented 4 years ago

Please help me understand you scenario. Let's separate the two secrets of server certificate, which is stored in allocator-tls and client certificate whitelisting which is done by adding the public cert to allocator-client-ca.

It is correct that agones.allocator.generateTLS=true is useless because it does not have SAN for the service as the value is unknown when generating the certificate. Here is a related issue: https://github.com/googleforgames/agones/issues/1407. So there is an additional step of replacing the the server TLS certificate after installation.

For the client whitelisting, allocator-client-ca needs to be updated according to this. When there is a new client certificate to whitelist.

Is your problem with in-place upgrade that you need your secrets to remain untouched? Will solving https://github.com/googleforgames/agones/issues/1710 address your concern in this issue as well?

endragor commented 4 years ago
  1. For allocator-client-ca you pointed to the document which implies that generateTLS is false, i.e. the user generates certificates manually. However, if generateTLS is true, the allocator-client-ca is created automatically, but the key is not stored anywhere. This is what this issue is about. Here is an excerpt from allocation.yml of the Helm chart:
    {{- $ca := genCA "allocation-ca" 3650 }}
    apiVersion: v1
    kind: Secret
    metadata:
    name: allocator-client-ca
    # <omitted>
    data:
    {{- if .Values.agones.allocator.generateTLS }}
    client-ca.crt: {{ b64enc $ca.Cert }}
    # <omitted>

As you can see, ca.Cert is stored, but ca.Key is discarded.

  1. SAN can be known in advance if #1709 is implemented. In such case, the certificates could be generated for specific IP address. Also, you could provide an option to specify the IP address(es) - this is useful for NodePort service type, which is often used in local environments where the address is known (usually either localhost or static VM address).
pooneh-m commented 4 years ago
  1. Thanks for the clarification. client-ca.crt: {{ b64enc $ca.Cert }} is whitelisting the CA cert that generates allocator-tls to also be accepted as the client certificate. However, I do not recommend getting a dependency to allocator-tls for the client certificate. Instead, there is a new default client certificate that is going to be added in Agones 1.8. You can export the client certificate and use it outside the cluster.

  2. loadBalancerIP sounds like a great idea to also help with generating a usable server certificate.

endragor commented 4 years ago

Ah, I see, thank you for the explanation. So it seems this issue is not currently relevant.