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

kube-rbac-proxy warn about deprecation and future breaking changes #3524

Closed
antonincms opened this issue Aug 7, 2023 · 23 comments · Fixed by #3899
Closed

kube-rbac-proxy warn about deprecation and future breaking changes #3524

antonincms opened this issue Aug 7, 2023 · 23 comments · Fixed by #3899
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Milestone

Comments

@antonincms
Copy link
Contributor

antonincms commented Aug 7, 2023

What do you want to happen?

Since kube-rbac-proxy 0.14.1, it warn that some of the flags that kubebuilder scaffolds are deprecated :

Flag --logtostderr has been deprecated, will be removed in a future release, see https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components
W0807 14:58:44.167326       1 kube-rbac-proxy.go:152]
==== Deprecation Warning ======================

Insecure listen address will be removed.
Using --insecure-listen-address won't be possible!

The ability to run kube-rbac-proxy without TLS certificates will be removed.
Not using --tls-cert-file and --tls-private-key-file won't be possible!

For more information, please go to https://github.com/brancz/kube-rbac-proxy/issues/187

===============================================
I0807 14:58:44.167366       1 kube-rbac-proxy.go:272] Valid token audiences:
I0807 14:58:44.167421       1 kube-rbac-proxy.go:363] Generating self signed cert as no cert is provided
I0807 14:58:44.947435       1 kube-rbac-proxy.go:414] Starting TCP socket on 0.0.0.0:8443
I0807 14:58:44.947768       1 kube-rbac-proxy.go:421] Listening securely on 0.0.0.0:8443

We could easily remove the --logtostderr=true but I don't know what would be the best solution for --tls-cert-file and --tls-private-key-file that will become mandatory.

I think we could use certificates from cert-manager, but it would make cert-manager required (maybe kube-rbac-proxy could be disabled by default to not force that requirement).

@antonincms antonincms added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 7, 2023
@camilamacedo86 camilamacedo86 added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Aug 14, 2023
@camilamacedo86

This comment was marked as outdated.

@camilamacedo86 camilamacedo86 added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. labels Aug 14, 2023
@camilamacedo86

This comment was marked as outdated.

@varshaprasad96

This comment was marked as outdated.

@ashutosh887

This comment was marked as outdated.

@Sajiyah-Salat

This comment was marked as outdated.

@Sajiyah-Salat

This comment was marked as off-topic.

@antonincms
Copy link
Contributor Author

antonincms commented Aug 17, 2023

I would be happy to work on this, but it seems there is already candidates (that's fine).
For the --logtostderr=true flag, I think it could simply be removed, but for the TLS related flags it is less clear what to do.

From the issue in kube-rbac-proxy it seems that the point is to remove ability to setup kube-rbac-proxy without configured certificates for the v1.x so that the project can be donated to sig-auth. That's why I was wondering wether using cert-manager for that would be a good fit, but it would make it a requirement (I guess we could also generate a certificate in a init container).

@camilamacedo86
Copy link
Member

@Sajiyah-Salat,

We need the information from the project maintainers first to know how we should update those flags, see: brancz/kube-rbac-proxy#196 (comment)

Please, feel free to reach out them to gathering this info.

@Sajiyah-Salat

This comment was marked as outdated.

@Sajiyah-Salat

This comment was marked as outdated.

@Sajiyah-Salat

This comment was marked as outdated.

@ibihim
Copy link

ibihim commented Aug 22, 2023

Hi everyone,

Apologies for my delayed response. Summer vacations are in full swing here in Europe!

Regarding the logging flags: they've been deprecated by k8s upstream. As a result, they're now deprecated in kube-rbac-proxy too, given our reliance on the k8s logging framework. We're actively working on kube-rbac-proxy to make it a sig-auth owned project. This means incorporating more Kubernetes-native solutions and reducing our custom implementations.

I'll delve into the upstream recommendations regarding these deprecations as our migration guide hasn't addressed this yet, if time is short, you might consider exploring upstream resources directly in the interim.

From a security perspective, our decision to mandate TLS certificates comes in alignment with recommendations from the sig-auth reviewers, a stance we agree with. Running a server with self-signed certificates or without TLS may be a deliberate choice, but it doesn't offer robust security. Given the security-critical nature of kube-rbac-proxy, it's essential. To this end, we've addressed concerns raised over the use of self-signed certificates (see comment) and the insecure listen address (see comment).

If you have certain expectations about our project, it is NOW the time to file issues or comment on the "review issues" (like this issue) and discuss with us the future outcome of the project! The current development branch is called sig-auth-acceptance.

Due to the high volume of notifications on GitHub, I occasionally miss a few. For a more direct line of communication, feel free to reach out to me on the Kubernetes Slack. I check it at least once a week.

@ibihim
Copy link

ibihim commented Aug 22, 2023

@Sajiyah-Salat, I am one of the maintainers. @brancz is not involved in its development. He was kind enough to let us migrate the project to sig-auth.

You can reach out to me through kubernetes slack or kube-rbac-proxy issue.

@Sajiyah-Salat

This comment was marked as outdated.

@camilamacedo86
Copy link
Member

Hi @ibihim,

Thank you for your input. Following some comments inline.

⚠️ Regarding the --tls-cert-file and --tls-private-key-file becoming mandatory, I'm concerned about the implications for our setup, @ibihim. Can you provide some clarity?

From my understanding, these flags can only be provided if there's a solution like cert-manager in place. Is this a correct assumption? Do we have these values by default in any scenario? Furthermore, I'd like to emphasize that we can't expect all users to adopt cert-manager. Could you guide me on what values we provide here by default or if there's another recommended approach?

🚀 > We're actively working on kube-rbac-proxy to make it a sig-auth owned project.

That would really be helpful because currently we have been facing a kind of burn because we need to re-build and republish the images in the k8s registry when it would be better done by the project itself. Have you any ETA for it occurs?

🆗 Regards this task, by looking at the migration guide pointed out

  • replace --secure-listen-address by use a combination of --bind-address and --secure-port
  • remove the --logtostderr=true

Thank you for all your help on this one.

@camilamacedo86 camilamacedo86 added triage/needs-information Indicates an issue needs more information in order to work on it. and removed good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. labels Aug 30, 2023
@camilamacedo86
Copy link
Member

camilamacedo86 commented Aug 31, 2023

To make clear, what we need to know here:

If using kube-rbac-proxy mandatorily requires certmanager, then we cannot feasibly continue to include it in the default scaffold:

So, we need to confirm that is the case which appears to be. @ibihim could you please help us with your input for we know how can we move forward?

Therefore, the solution might to be remove kube-rbac-proxy in prol of use NetworkPolicy More info

There are some compelling reasons to consider:

  • Maintainability: Integrating with projects is becoming challenging. At present, we have to rebuild the image for kube-rbac-proxy because it isn’t under the SIG. Then, note that the registry change we “kubebuilder maintainers” are responsable for its image when we should not. We are just a small number of people
  • Flexibility: By keeping the plugin maintained within their repositories, projects can still integrate with kubebuilder. This approach enables them to utilize kubebuilder as a library via external plugins. Then, the project can provide this solution for Kubebuilder as any other tool like SDK but the source code will be maintained by the project itself.
  • User Preferences: Some users have expressed dissatisfaction with kube-rbac-proxy. For instance, see: RFC: The kube-rbac-proxy is too opinionated to be opt-out. #3482.

We created a thread to discuss it in the channel as well: https://kubernetes.slack.com/archives/CAR30FCJZ/p1693377335373059

@ibihim
Copy link

ibihim commented Aug 31, 2023

TLS Certs

To maintain the exact same current behavior, you can generate a self-signed TLS certificate and provide it to the kube-rbac-proxy either within the container image or using an initContainer. It's worth noting that there's no need for a cert manager if you are fine to keep this behavior.

In the previous version of kube-rbac-proxy, this certificate was generated automatically. With the recent changes, we've shifted this responsibility to the user to ensure they are fully aware of the implications of using self-signed certificates.

I'll look deeper into this topic. It might take me until next week.

Sig-Auth

The transition to sig-auth is progressing, but it will likely take a few more months to complete. Currently, we are in what we hope to be the second and final review phase, addressing the issues that have been raised.

As a result, our users can expect several changes in the near future. Our aim is to meet the requirements set by the sig-auth community while also catering to the needs of our users. Constructive feedback and active discussions would be greatly appreciated to help us achieve this balance.

For context, this process was initiated in April 2022.

@camilamacedo86
Copy link
Member

HI @krzys (ibihim)

Thank you for the answers.

To clarify, you're suggesting that for kubebuilder, we'll need to scaffold code to generate the certs in order to continue using it. Could you please provide a link to the code so we can review it?

Also, couldn't the kube_rbac_proxy generate the certs by default when the values are not provided? Why have those fields been made mandatory?

@camilamacedo86
Copy link
Member

Context and considerations discussed in the slack

BY @ibihim

It seems you’re enabling users to create operators that are protected by kube-rbac-proxy. From the template you provided, I noticed a secure listen address, which indicates that kube-rbac-proxy was indeed running with self-signed certificates.
Before we go deeper, I’d like to ensure we’re aligned. I apologize in advance if I’m reiterating points you’re already familiar with. Written communication can sometimes miss nuances, and I want to ensure clarity.
When someone connects to kube-rbac-proxy, they’re presented with a self-signed certificate, allowing them to establish a secure TLS connection. The primary goal here is to ensure secure communication, especially since sensitive tokens are transmitted, and it’s crucial these aren’t exposed in plaintext.
However, while the connection is encrypted with self-signed certificates, there’s an inherent trust issue. Without a certificate signed by a trusted Certificate Authority (likely something within the cluster), there’s no way to verify the identity of kube-rbac-proxy. This opens the door for potential Man-In-The-Middle attacks, where an attacker could impersonate kube-rbac-proxy, intercepting and forwarding messages.
This is the core reason behind our recommendation to provide specific TLS keys and certificates. It’s not merely about encryption but also about trust and identity verification. We aim to raise awareness among users about this distinction. If a user opts to continue with self-signed certificates, they should be made aware of the risks and take the necessary precautions.

Next steps

  • Analyse the available options and propose solutions to be discussed (i.e. we might want to consider make it as an optional option that is only supported with cert-manager)

@camilamacedo86
Copy link
Member

Hi @krzys (ibihim),

Just to make clear, see:

Then, the first step and what we need your help with is:

How those configurations should be done after the deprecations? What is the ideal way to use kube-rbac-proxy considering its changes?

So that, it is the first step for we start to think what is the best approach for we still providing it for Kubebuilder users.

Again, thank you for all your attention and time.

@camilamacedo86
Copy link
Member

camilamacedo86 commented Nov 14, 2023

Proposal for Moving Forward with kube-rbac-proxy Integration in Kubebuilder

To move forward here we need to:

a) Assess kube-rbac-proxy Integration without Mandatory cert-manager Usage:

We need to investigate whether there is a recommended approach to incorporate kube-rbac-proxy into Kubebuilder without making cert-manager mandatory. Upon the discussions made I do not see any recommended option. See that move the code used by kube-rbac-proxy today to generate the certs to Kubebuilder is not an option, since they are removing it due the fact of it to be NOT recommended However, new features related introduced in controller-runtime might be an option. That is what is required to investigate.

OR

b) Make cert-manager Mandatory in the Default Layout:

Another option is to explore making cert-manager a mandatory component in the default layout of Kubebuilder. We should consider the implications of this change, especially in light of the related requests discussed here, see that for properly ensure security with the metrics it might be required : GitHub Issue Link.

OR

c) Remove kube-rbac-proxy from the Default Layout and Replace It:

It's worth exploring the possibility of replacing kube-rbac-proxy with an alternative solution in the default layout of Kubebuilder. This could address certain concerns, as detailed in GitHub Issue Link.

AND/OR

d) Move kube-rbac-proxy to a Plugin with Opt-In Choice for Users:

We should consider moving kube-rbac-proxy to a plugin, allowing users to opt-in or opt-out based on their requirements and preferences. The motivation behind this approach is elaborated in GitHub Issue Link.

To move forward effectively, we may need to create open pull requests (PRs) with the proposed changes or proofs of concept (POCs). Additionally, a proposal document via a PR could help provide a clear path for decision-making.

Let’s collaborate closely to determine the best way forward and ensure that our integration of kube-rbac-proxy within Kubebuilder aligns with the project’s goals and user needs.

@ibihim
Copy link

ibihim commented Apr 11, 2024

Cert-Manager isn't a necessity by itself. Obviously it is the best overall solution. A better solution would be to create a cert with a CA and put the cerst in a config map and the key in a secret. If necessary one could make the certs last 100 years. This solution would be way better than a self-signed certificate solution.

We are starting to do that in the new sig-auth-acceptance branch:

An example how to have the same behavior with init containers (self-signed cert with a validity of 100 years):

apiVersion: apps/v1
kind: Deployment
metadata:
  name: kube-rbac-proxy
  namespace: default
spec:
  replicas: 1
  selector:
    matchLabels:
      app: kube-rbac-proxy
  template:
    metadata:
      labels:
        app: kube-rbac-proxy
    spec:
      serviceAccountName: kube-rbac-proxy
      initContainers:
        - name: cert-generator
          image: alpine
          command:
            - sh
            - -c
            - |
              apk add openssl
              mkdir -p /etc/certs
              
              # Generate server key and self-signed certificate
              openssl genrsa -out /etc/certs/server.key 4096
              openssl req -new -x509 -sha256 -key /etc/certs/server.key -out /etc/certs/server.crt -days 36500 -subj "/CN=kube-rbac-proxy" -extensions req_ext -config <(echo "
                [req]
                distinguished_name=req_distinguished_name
                x509_extensions=req_ext
                [req_distinguished_name]
                [req_ext]
                subjectAltName=DNS:kube-rbac-proxy,DNS:localhost
                keyUsage=digitalSignature,keyEncipherment
                extendedKeyUsage=serverAuth
              ")

              # Set permissions so the kube-rbac-proxy can read the certs and key
              chmod 644 /etc/certs/*.crt
              chmod 644 /etc/certs/*.key
          volumeMounts:
            - name: cert-volume
              mountPath: /etc/certs
      containers:
        - name: kube-rbac-proxy
          image: quay.io/brancz/kube-rbac-proxy:local
          args:
            - "--secure-listen-address=0.0.0.0:8443"
            - "--upstream=http://127.0.0.1:8081/"
            - "--v=10"
            - "--tls-cert-file=/etc/certs/server.crt"
            - "--tls-private-key-file=/etc/certs/server.key"
          ports:
            - containerPort: 8443
              name: https
          volumeMounts:
            - name: cert-volume
              mountPath: /etc/certs
              readOnly: true
        - name: prometheus-example-app
          image: quay.io/brancz/prometheus-example-app:v0.1.0
          args:
            - "--bind=127.0.0.1:8081"
      volumes:
        - name: cert-volume
          emptyDir: {}

Disclaimer:

  • I am not an openssl expert, I prefer to create certificates through Go code.
  • I tested it only with the basic test case

@camilamacedo86
Copy link
Member

camilamacedo86 commented Apr 15, 2024

Hi @ibihim,

Thank you for sharing this option: kubebuilder issue #3524 comment.

However, I understand that this configuration is not aligned with best practices and is not considered safe by the auth-sig maintainers in their review. This is the reason why kube-rbac-proxy no longer manages the certs in this manner. Or Am I wrong here and missed something?

Assuming that my understand is accurate: Therefore, we cannot promote this configuration via Kubebuilder scaffolds or endorse and recommend it. Kubebuilder aims to abstract complexities, help users skill up, and accelerate the process of developing "Native k8s solutions" while following good practices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants