feat: allow restricting to namespace

[woehrl01]

This PR adds the possibility to restrict to listening only to specific namespaces. This is required if you use IAM role for service account. Otherwise this would allow you to create any AWSPCAIssuer in any namespace and resolve that with the controllers permission.

See: https://sdk.operatorframework.io/docs/building-operators/golang/operator-scope/#watching-resources-in-a-single-namespace

[woehrl01]

/assign @irbekrm

[kollabpr]

Can you please add unit tests for the changes you have committed?

[jetstack-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: woehrl01
To complete the pull request process, please ask for approval from irbekrm after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[woehrl01]

@kollabpr I'm a bit unclear about what you're asking me to do regarding the unit test. It seems that the namespace variable is being utilized by the operator-sdk and is not directly relevant to this controller. Don't get me wrong, I'm definitely in favor of writing tests, but in this case, I don't believe there is any custom logic I need to test specifically within this controller's scope. Could you clarify what you had in mind so I can better understand how to proceed?

[divyansh-gupta]

Hi @woehrl01 - Thanks for the feature request and the PR! We will review with the team and get back to you soon. In the meantime, we would love to know more about your use-case, how you are using the plugin and why you need this feature, to help us better make this decision. Please feel free to include any supporting documentation or steps you did.

[woehrl01]

Hi @divyansh-gupta

I'm currently using the aws-privateca-issuer to issue certificates via an AWS Private CA. However, I want to limit the creation of certificates to a single application within a specific namespace while also leveraging IAM Roles for Service Accounts.

The current setup is not entirely secure because the controller assumes a role independently of the location of the AWSPCAIssuer. Therefore, I added additional restrictions so that the controller only operates within the trusted AWSPCAIssuer's namespace.

By implementing this additional restriction, I can ensure that the controller only listens to the single namespace where the trusted AWSPCAIssuer resides, ultimately improving the overall security of my system.

[divyansh-gupta]

Hi @woehrl01 - have you tried using a regular issuer (not a ClusterIssuer) with the namespace set? https://github.com/cert-manager/aws-privateca-issuer/blob/main/config/samples/awspcaissuer_ec/_v1beta1_awspcaissuer_ec.yaml#L5

[woehrl01]

@divyansh-gupta of course that's what I use. But without this change you can generate this in any namespace and still issue with the role of the controller.

[divyansh-gupta]

@woehrl01 Thank you for the response.

[divyansh-gupta]

Adding details for how to reproduce after a chat with @woehrl01 -

Install the controller in Namespace1 (which contains a ServiceAccount with the correct IAM permissions attached), and then you create an issuer in Namespace2. Next, if you create a cert in Namespace2, the cert will still issue even though Namespace2 doesn't have the permissions necessary since the IAM role was attached to Namespace1.

We will move forward with trying to reproduce this.

@woehrl01 can you confirm our understanding above? And when you say "install the controller to a namespace" do you mean using the -n {namespace} flag during the helm install?

[woehrl01]

@divyansh-gupta yes, exactly!

[divyansh-gupta]

@woehrl01 We were able to reproduce this namespacing issue and were able to apply your patch and test the fix. Your fix works, however our team is still discussing alternatives and next steps.

For example, we noticed that other cert-manager issuers are not supporting this feature, and are looking into that.

[woehrl01]

@divyansh-gupta thanks for the update, can you please elaborate which alternatives you are discussing?

I'm not sure if other issuers suffer of the same attack surface as this only affects issuers which use an iam role for authentication, I'm not aware of any other issuers using that.

[woehrl01]

@divyansh-gupta are there any updates from your side regarding this PR?

[divyansh-gupta]

@woehrl01 Hi there, we decided this approach makes sense. We haven't merged it due to having no automated tests for this scenario yet. This is something we would have to build before merging this PR.

[woehrl01]

Any updates on this @divyansh-gupta? Thank you!

[divyansh-gupta]

Hi @woehrl01 - This work hasn't been prioritized on the team yet, I will keep you updated when we do. I appreciate your patience.

[woehrl01]

Hi @divyansh-gupta is there any progress on your side regarding to having this PR merged? Thanks!

[woehrl01]

For other readers. We have now replaced this change with using cert-manager/approver-policy:

*please adapt the allowed fields to your needs.

apiVersion: policy.cert-manager.io/v1alpha1
kind: CertificateRequestPolicy
metadata:
  name: aws-pca-issuers
spec:
  allowed:
    commonName:
      required: true
      value: "*"
    isCA: true
    subject:
      countries:
        required: true
        values:
          - '*'
      organizationalUnits:
        required: true
        values:
          - '*'
      organizations:
        required: true
        values:
          - '*'
  selector:
    issuerRef:
      group: awspca.cert-manager.io
      kind: AWSPCAIssuer
      name: awspca-issuer
    namespace:
      matchNames:
        - istio-system

[jetstack-bot]

PR needs rebase.

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.