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

Proposal: Provide a flag to disable mTLS for agones-allocator #1590

Closed
pooneh-m opened this issue May 27, 2020 · 8 comments
Closed

Proposal: Provide a flag to disable mTLS for agones-allocator #1590

pooneh-m opened this issue May 27, 2020 · 8 comments
Labels
area/operations Installation, updating, metrics etc help wanted We would love help on these issues. Please come help us! kind/feature New features for Agones
Milestone

Comments

@pooneh-m
Copy link
Contributor

pooneh-m commented May 27, 2020

Is your feature request related to a problem? Please describe.
agones-allocator service supports mTLS authentication built into the service, which is used for LoadBalancer service and for enabling multi-cluster allocation solution.

However, a NodePort service used by Ingress does not need an authentication mechanism built into the service. Ingress can provide mTLS support along with other ways of authentications. Also, Ingress in the cloud solutions handle the authentication. Side car proxies such as Envoy can provide mTLS support.

Therefore, mTLS being built into the service prevents other solutions' adaptation.

Describe the solution you'd like
Provide a flag to disable mTLS for agones-allocator.

Describe alternatives you've considered

  • The service only enables mTLS if it is of type LoadBalancer.

  • By default disable mTLS and change the service type to NodePort and document setting up Ingress resource. Currently, because the IP address is not allocated before Agones installation, Agones cannot issue a certificate that after install would be ready to use by the service and requires additional step.

@pooneh-m pooneh-m added kind/feature New features for Agones help wanted We would love help on these issues. Please come help us! labels May 27, 2020
@markmandel
Copy link
Collaborator

This sound interesting - how does this impact cross cluster communication (for allocation failover)? Would that also be unencrypted?

Or would there be multiple services - one with mTLS (for inter-cluster comms) and one without (for external system interaction through a load balancer)?

@pooneh-m
Copy link
Contributor Author

Still for multi-cluster allocation, mTLS is used to secure the connections. However, if mTLS authentication is disabled on agones-allocator, other means of mTLS support should be provided either through a cloud provider, Ingress or sidecar proxies.

@luna-duclos
Copy link
Contributor

luna-duclos commented May 28, 2020

Would very much be a fan of this, IAP or similar load balancer level solution can do the job of securing the communication!
This does mean however that this isn't useful without the ability to tell the agones allocator how to authenticate.

In that light, perhaps we should go for a flag that specifies how to authenticate instead ? Options can be disabled, mTLS, gcpserviceaccount, etc.

@markmandel
Copy link
Collaborator

This sounds like a good idea to me!

@markmandel markmandel added the area/operations Installation, updating, metrics etc label May 28, 2020
@TBBle
Copy link
Contributor

TBBle commented Jun 23, 2020

If we're able to use an Ingress for this service with mTLS disabled on the service, it should be possible to have CertManager take care of issuing the mTLS certificates for the server side, and have the Ingress validate the mTLS connection, assuming your Ingress implementation supports mTLS.

Note that I haven't actually tested mTLS as below, it's put together from the docs, and looking at our existing CertManager-based TLS-secured Ingresses. I do have pending use-cases for mTLS, but have not implemented any of them locally.

Similar to the current approach, you'd create your CA Certificate, and then load it into a CA Issuer, and then annotate your Ingress, and CertManager generates a certificate for you. (Note that this won't work with the ACME Issuer used with e.g., LetsEncrypt as the ca.crt is not populated, so (I think but haven't tested) you can't do client-certificate validation).

You can either issues client certs by-hand yourself, or use kubectl create to create Certificate resources with the right key usages which will populate a Secret you can extract and use elsewhere. This part is similar to the mTLS server-side setup flow, although that uses a self-signed certificate as the TLS certificate, and bears no relationship to the client-presented self-signed certificate.

In this flow, NGINX Ingress appears to lack any way to limit which certificates to accept beyond the depth of the path from your CA, a feature of the allocation service's own mTLS support. On the other hand, NGINX Ingress doesn't require you to list trusted certificates to accept, a cost of the allocation service's own mTLS support.

To block compromised certificates, a CRL list given to NGINX Ingress, or a CRL Distribution Point in the certificates you create with CertManager would work, although compared to allocator-service's accept-list, this is a reject-list. And apart from embedding the CRL Distribution Point URL, Cert Manager doesn't currently appear to support revocation in any way, so you have to run the SSL commands yourself. It also doesn't yet support things like OSCP Stapling or the OSCP server URL, so if you need to manage who can access your server more carefully that certificate expiry dates, this side will prove painful.


I'm curious if it would be hard to make the allocator service's mTLS support work with CAs and existing Secret layouts, rather than having specialised lists of trusted self-signed certificates in both directions. Or is that just the example?

@TBBle
Copy link
Contributor

TBBle commented Jun 23, 2020

More on-topic, NodePort seems like the wrong default if mTLS is disabled by default, since that's potentially open-to-the-world if you're not careful. It should default to ClusterIP, although even having it there by default with mTLS disabled allows bypassing the RBAC permissions for create GameServerAllocation to anyone on your cluster who can discover the Service/Endpoint. (Also true of NodePort and Ingress-based setups, so perhaps that's not something we're worried about, if we disable the built-in mTLS?)

Personally, I would have the allocation service off-by-default, while its primary purpose is serving multi-cluster allocation requests from the outside world, since any valid setup requires some configuration effort anyway, even if it's just a bunch of annotations in Helm. I've turned it off on my clusters as we're not using it.

@devloop0
Copy link
Contributor

#1645 partially address this issue. Note that we still need to get testing working for this feature and document the helm configuration parameter.

@markmandel
Copy link
Collaborator

Doing some cleanup - has this been completed? @pooneh-m @devloop0 ?

@markmandel markmandel added this to the 1.11.0 milestone Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/operations Installation, updating, metrics etc help wanted We would love help on these issues. Please come help us! kind/feature New features for Agones
Projects
None yet
Development

No branches or pull requests

5 participants