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

Cannot Disable HTTP when using managed-certificates. HTTPS + HTTP are both provisioned #764

Closed
jakebolam opened this issue Jun 1, 2019 · 33 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@jakebolam
Copy link

jakebolam commented Jun 1, 2019

Cannot disable HTTP when using managed-certificates

When specifying the annotation kubernetes.io/ingress.allow-http: "false" the ingress still accepts HTTP traffic.

Versions

Kubernetes master/node-kube version: 1.12.7-gke.17 (Note: must be equal to or greater than this version, managed certificates had issues prior to this version see GoogleCloudPlatform/gke-managed-certs#2)

Additional Details:

I also tried (#738 (comment)):

  1. Creating an ingress without kubernetes.io/ingress.allow-http: "false".
  2. Verified the ingress was serving HTTPS
  3. Then setting allow-http to "false"

Configuration

apiVersion: networking.gke.io/v1beta1
kind: ManagedCertificate
metadata:
  name: api-certifcate
spec:
  domains:
    - example.com
---
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: my-ingress
  annotations:
    kubernetes.io/ingress.allow-http: "false"
    kubernetes.io/ingress.global-static-ip-name: "web-static-ip"
    networking.gke.io/managed-certificates: api-certifcate
spec:
  rules:
    - http:
        paths:
          - path: /my-service/*
            backend:
              serviceName: my-service
              servicePort: 8080
@jakebolam
Copy link
Author

This was previously reported, but the issue was masked by issues in managed-certificates up until recent versions. See #738

@jakebolam
Copy link
Author

Debugging config from provisioned ingress:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  annotations:
    ingress.gcp.kubernetes.io/pre-shared-cert: xxxxxx, xxxxx, xxxx
    ingress.kubernetes.io/backends: '{"xxxx":"HEALTHY","xxxx":"HEALTHY"}'
    ingress.kubernetes.io/forwarding-rule: xxxx
    ingress.kubernetes.io/https-forwarding-rule: xxxxx
    ingress.kubernetes.io/https-target-proxy: xxxxx
    ingress.kubernetes.io/ssl-cert: xxxxxxxx, xxxxxx, xxxx
    ingress.kubernetes.io/target-proxy: xxxxx
    ingress.kubernetes.io/url-map: xxxxxx
    kubernetes.io/ingress.allow-http: "false"
    kubernetes.io/ingress.global-static-ip-name: web-static-ip
    networking.gke.io/managed-certificates: api-certifcate
  creationTimestamp: 2019-05-23T21:54:39Z
  generation: 2
  name: my-ingress
  namespace: default
  resourceVersion: "2986739"
  selfLink: /apis/extensions/v1beta1/namespaces/default/ingresses/my-ingress
  uid:xxxxxx
spec:
  rules:
    - http:
        paths:
          - path: /my-service/*
            backend:
              serviceName: my-service
              servicePort: 8080
status:
  loadBalancer:
    ingress:
    - ip: xxx.xxx.xxx.xx

@rramkumar1
Copy link
Contributor

@jakebolam Ok, now I think you have discovered the bug. Turns out, we knew about this and were fixing it (#631) but the PR stalled a bit. I actually thought this PR was merged a while back and as a result I gave you the wrong advice in #738 (regarding adding the annotation after the frontend was configured) so apologies for that.

/assign

@rramkumar1 rramkumar1 added the kind/bug Categorizes issue or PR as related to a bug. label Jun 3, 2019
@rramkumar1
Copy link
Contributor

@jakebolam A potential workaround might be to delete the HTTP resources manually until we can ship the fix.

@jakebolam
Copy link
Author

jakebolam commented Jun 4, 2019

no worries, thanks for replying so fast. Looking forward to the PR getting merged.

Will do 😄

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 2, 2019
@AlienResidents
Copy link

Hi,

I'm also looking forward to getting PR #631 merged. Any update on that stalled PR?

Chris-

@bowei
Copy link
Member

bowei commented Sep 30, 2019

BTW : We are looking at fixing the issue in #631 not on that specific PR.

@bshibs
Copy link

bshibs commented Oct 15, 2019

Just to confirm: manually deleting the LB Frontend port/protocol is a viable workaround until the fix lands?

@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 14, 2019
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@svachmic
Copy link

svachmic commented Feb 4, 2020

Could be related to this:
https://issuetracker.google.com/issues/35904733

Any signs that it will finally be resolved?

@roobert
Copy link

roobert commented Mar 15, 2020

I'm also seeing this issue with GKE 1.14.10-gke.17.

Since it's not yet possible to redirect http->https (https://issuetracker.google.com/issues/35904733) and this annotation doesn't work when using GCP managed certificates, there doesn't appear to be a good way to easily disable the HTTP port left open by the GKE ingress controller.

@Setheck
Copy link

Setheck commented Mar 18, 2020

Unable to disable HTTP via annotation kubernetes.io/ingress.allow-http: "false" on 1.15.9-gke.12
Hoping for http->https rewrite 🤞
But should this issue be re-opened?

@pgp
Copy link

pgp commented Mar 18, 2020

I think this should be solved as soon as possible, it's a real shame a security issue of this size is still open... Even without rewrite, disabling http completely should suffice at least for most restricted scenarios

@svachmic
Copy link

svachmic commented Mar 18, 2020

Quoting a Googler from the parent issue in Google's bug tracker I linked above:

We have an Alpha available for HTTP(S) Load Balancing URL Rewrites and Redirects. All Alpha terms apply. You need to be white-listed for the alpha. Please contact your Sales Representative or Account Team for more information.

We are proactively working on collecting feedback for the Alpha and will then open it up for public use without any white-listing.

Help is on its way, unfortunately I am not white-listed and cannot provide any feedback for how good/bad the solution is. If anybody is, please let us know anything that an alpha NDA allows you to.

@jmound
Copy link

jmound commented Apr 22, 2020

/reopen

@k8s-ci-robot
Copy link
Contributor

@jmound: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@jmound
Copy link

jmound commented Apr 22, 2020

@jakebolam Feel free to reopen it, I don't think this issue should be closed.

@svachmic
Copy link

The 5year old underlying issue seems to have been resolved.

The question is - correctly asked as the next comment in the issue tracker - how can we make this work in GKE?

@jmound
Copy link

jmound commented Apr 24, 2020

@rramkumar1 Are you able to reopen this issue? If not, should a new one be opened?

@rramkumar1
Copy link
Contributor

@jmound Let me refresh myself on what the exact issue was here and get back to you. Thanks.

@rramkumar1
Copy link
Contributor

@jmound So there are a couple things to unpack here:

  1. The underlying issue that caused this issue to be filed has been fixed in clean up HTTP(S) resources when changing protocols #631. If you are not observing that the fix is working, then we can keep this open but otherwise this issue should be closed.
  2. Support for SSL-redirects in GKE Ingress is being tracked in HTTP to HTTPS redirection #1075 and we will be providing updates on that issue.

Hope that helps.

@skmatti
Copy link
Contributor

skmatti commented Apr 29, 2020

The fix(#894) is available in GKE version v1.16.4-gke.25 and above(or Ingress v1.8.2).

@pgp
Copy link

pgp commented May 13, 2020

Hi, just tried this fix (allowHttp: false) with cluster version 1.17.5-gke.0
It actually works, in the sense that when I try to send a plain HTTP request, the response is a 404 from GCP backend. Anyway, with this strategy, any sensitive content in a plain HTTP request is actually sent over the internet (e.g. the body of a POST request); I rather expected the server to deny the TCP handshake - i.e. respond immediately with a Connection Reset, in so preventing sending data over the network at all. Do you believe such an improvement is possible?

@rramkumar1
Copy link
Contributor

@pgp I think the HTTPS redirects feature is what you are after. See #1075

@pgp
Copy link

pgp commented May 13, 2020

No, in terms of actual security, http->https redirect is equivalent to the situation I described above, in that my plain HTTP request travels over the network, even if it shouldn't (the only difference is I receive a 302 response instead of a 404 one, but the concept is the same). Completely blocking the traffic on port 80 can be useful in case some REST API users do not comply with the given specifications (i.e. they deliberately or erroneously use http instead of https for an endpoint). Clearly, the steady-state situation is the same - a user receiving a 404 response will modify its client calls accordingly - but meanwhile some (possibly sensitive or private) data have been travelling in plaintext over the public internet.

@rramkumar1
Copy link
Contributor

rramkumar1 commented May 13, 2020

@pgp I see, thanks for the clarification. I'll bring this up with our load balancing teams to see if there is a feature request here (or solution). Until this sort of feature is supported in the upstream load balancing API there is nothing we can do here.

@rramkumar1
Copy link
Contributor

@pgp Found https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Strict-Transport-Security. Not sure if it helps in your case.

@pgp
Copy link

pgp commented May 13, 2020

That always regards the response a server sends, nothing prevents the client from sending any kind of data in plain text before knowing this... Anyway, I'll look for possible updates on this issue, thanks for now.

@Andrewangeta
Copy link

I'm also facing this issue on the latest version and I'm losing my mind.

@Andrewangeta
Copy link

Andrewangeta commented May 31, 2020

my hydrated yaml looks like this and the cert tries to create itself and every time it just ends up saying FAILED_NOT_VISIBLE after about 15 minutes of PROVISIONING

apiVersion: networking.gke.io/v1beta1
kind: ManagedCertificate
metadata:
  name: my-app-certificate
  namespace: my-app
spec:
  domains:
  - my.domain
---
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  annotations:
    ingress.kubernetes.io/force-ssl-redirect: "true"
    kubernetes.io/ingress.allow-http: "false"
    kubernetes.io/ingress.global-static-ip-name: my-app-static-ip
    networking.gke.io/managed-certificates: my-app-certificate
  name: my-app-ingress
  namespace: my-app
spec:
  backend:
    serviceName: my-app
    servicePort: 80

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests