Add HTTPS support

[sirdarckcat]

Closes #78

This automatically provisions a certificate and builds a HTTPS load balancer.

Unfortunately this means we end up with 2 load balancers for the same task (2x the cost).

[sirdarckcat]

live at https://apache-others.kctf-testing.kctf.dev/nodejs/foo

[sroettger]

what happens if expose=false, https=true?

[sirdarckcat]

It creates the Ingress then, but it won't route traffic to anything (it won't know what's the service "chal"). This is OK, I think because it takes a while (5 mins) for the Ingress to fully setup and even more (10 mins) for the SSL cert to be issued, so this way you might not notice the delay

[sirdarckcat]

can I haz approve? :P

[sirdarckcat]

gree-gorey would you mind TAL? :) stephen is probably busy with kids :P

[gree-gorey]

IIUC, the problem with two LB is that Managed certs in GKE are only available for Ingress. But we can't change all public challenges to use Ingress instead of just TCP LB, because some of them are not using http protocol -- is that correct?

So, maybe in that case we would better specify challenge type in config, say smth like PROTOCOL=http|tcp, and for http we would create only Ingress, for others we would create only network LB as before. (Btw, I should adjust my PR with DNS so that it annotate Ingresses as well because right now it only annotates LB).)

What about challenges that would want to use TLS with TCP? They can't use Ingress. Or we don't expect such things?

[gree-gorey]

This is the same exact file as base/Makefile, right? Why it's not a symlink like in kctf/base/challenge-skeleton/Makefile? Should I also duplicate changes in two Makefiles in my PR?

[sirdarckcat]

oh, yea.. /samples/ is the equivalent of what an "upgrade" in infrastructure would be. this means, every CL needs to update /samples/, but we always forget :(. we can't make it a symlink because in "real life" users wouldn't do this.

the way I usually do the "upgrade" is that I delete the kctf-conf/base directory and then run setup-chal-dir, which recreates it from "head".

[sirdarckcat]

IIUC, the problem with two LB is that Managed certs in GKE are only available for Ingress. But we can't change all public challenges to use Ingress instead of just TCP LB, because some of them are not using http protocol -- is that correct?

Right, Ingress is only HTTPS.

So, maybe in that case we would better specify challenge type in config, say smth like PROTOCOL=http|tcp, and for http we would create only Ingress, for others we would create only network LB as before. (Btw, I should adjust my PR with DNS so that it annotate Ingresses as well because right now it only annotates LB).)

Makes sense. FWIW, Ingress has to point to a Service, so I'm currently pointing to the LoadBalancer Service, if we don't create the LB, then we will do a NodePort instead. Alternatively we could annotate it as internal https://cloud.google.com/kubernetes-engine/docs/how-to/internal-load-balancing

What about challenges that would want to use TLS with TCP? They can't use Ingress. Or we don't expect such things?

I think TLS+TCP would be the "responsibility" of the challenge author to setup. I think cases when this might be necessary (SMTP+STARTTLS or just encrypting traffic), it would be easier for the task author to provide the certificate or CA of the key being used, since the players would need to do the cert checks manually anyway.

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[sirdarckcat]

If you have a CTF tasks with 2 components (eg, 2 ports - say port 80/443 and port 22), then you can't easily make it happen with DNS. Maybe instead of letting the user select HTTPS or TCP, we can leave this as HTTPS=true/false and that just creates another DNS name (say, ${challenge-name}-web.${domain-name} and ${challenge-name}-tcp.${domain-name}). WDYT?

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

[gree-gorey]

If you have a CTF tasks with 2 components (eg, 2 ports - say port 80/443 and port 22), then you can't easily make it happen with DNS. Maybe instead of letting the user select HTTPS or TCP, we can leave this as HTTPS=true/false and that just creates another DNS name (say, ${challenge-name}-web.${domain-name} and ${challenge-name}-tcp.${domain-name}). WDYT?

I see. Yes, with current setup I think that would be the easiest way.

But ultimately I think would be better to assign each port a type of protocol it serves. Now it would be probably difficult to do with .conf files but when we migrate to CRD we could do smth like:

...
  spec:
    ...
    ports:
    - port: 22
      protocol: "TCP"
    - port: 443
      protocol: "HTTP"
      https: true

Then we would create Ingress for http ports, and tcp LB for tcp ports.

[sirdarckcat]

@googlebot I fixed it.

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[sirdarckcat]

@gree-gorey makes sense, let's do that on the CRD version! =)

I'm currently trying to get a demo running for this version of the code (got some IAM issues we need to resolve for me to test it see cl/318991759).

[sirdarckcat]

one issue with my naming proposal, is that we will end up with:

web-asm-web.domainname.com

I'm not super thrilled by that (double -web), maybe it should be -ssl instead or -https.

[gree-gorey]

one issue with my naming proposal, is that we will end up with:

web-asm-web.domainname.com

I'm not super thrilled by that (double -web), maybe it should be -ssl instead or -https.

Right, -http sounds better.

[gree-gorey]

samples/bash/config/chal.conf still has PROTOCOL=tcp

[sirdarckcat]

finally https://apache-others-http.kctf-testing.kctf.dev/nodejs/foo