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

Add Azure Workload Identity support for certificate credentials #2364

Closed
wants to merge 7 commits into from

Conversation

jjlakis
Copy link
Contributor

@jjlakis jjlakis commented Dec 22, 2023

Description

This pull request enables Workload Identity for Azure provider.

Instead of client_secret, the token exchange request includes a certificate credential parameters. Certificate credential is a JWT token, which claims has to match a corresponding Federated Identity Credential added to the App Registration.

Azure Workload Identity Webhook Server running on the Kubernetes cluster, mutates the properly annotated pod and mounts the federated token in the default path stored in $AZURE_FEDERATED_TOKEN_FILE. Webhook server can be installed easily on AKS as well as elsewhere. See documentation of Azure Workload Identity.

New flag is added: - --azure-federated-token-auth-enabled. When set, oauth2-proxy does not require the client-secret setting anymore. Instead, it expects oauth2-proxy to run in a pod mutated by Azure Workload Identity Webhook Server. oauth2-proxy won't start if the flag is enabled but pod isn't properly configured.

This change allows user to not store plain secrets on Kubernetes / GitOps repositories.

This is a revisited version of #2070, however this PR does include only the certificate credential changes and does not include any changes regarding reading user groups.

Motivation and Context

Currently, oauth2-proxy needs to have hardcoded secret configured, which causes security issues and requires maintenance like secret rotation. This PR closes #1979 .

How Has This Been Tested?

This was tested on on-premises cluster with Azure's workload-identity-webhook helm chart installed. Cluster has OIDC endpoint publicly available. App registration used for testing:

resource "azuread_application" "auth" {
  display_name = "oauth2-proxy"

  web {
    redirect_uris = [
      "https://myapp.domain.com/oauth2/callback",
    ]
  }

  required_resource_access {
    resource_app_id = "00000003-0000-0000-c000-000000000000" # Microsoft Graph
    resource_access {
      id   = "e1fe6dd8-ba31-4d61-89e7-88639da4683d" # User.Read
      type = "Scope"
    }
  }
}
resource "azuread_application_federated_identity_credential" "oauth2-proxy" {
  application_id = azuread_application.auth.id
  display_name   = "oauth2-proxy"
  description    = "oauth2-proxy"
  audiences      = ["api://AzureADTokenExchange"]
  issuer         = "https://mycluster.oidc-endpoint.com"
  subject        = "system:serviceaccount:oauth2-proxy:oauth2-proxy"
}

Values for oauth2-proxy helm chart (installed in oauth2-proxy namespace):

serviceAccount:
  annotations:
    azure.workload.identity/client-id: 5b2583ce-9789-4fd2-970c-2cf711b648ed

podLabels:
  azure.workload.identity/use: "true"

extraArgs:
  - --provider=azure
  - --client-id=<my-client-id>
  - --azure-federated-token-auth-enabled
  - --oidc-issuer-url=<my-tenant-id>
  - --upstream=static://202
  - --reverse-proxy=true

Traefik middleware:

apiVersion: traefik.io/v1alpha1
kind: Middleware
metadata:
  name: entra
  namespace: oauth2-proxy
spec:
  forwardAuth:
    address: http://oauth2-proxy.oauth2-proxy

And ingress annotation for protected service:

traefik.ingress.kubernetes.io/router.middlewares: oauth2-proxy-entra@kubernetescrd

TODO:

  • My change requires a change to the documentation or CHANGELOG.
  • I have updated the documentation/CHANGELOG accordingly.
  • I have created a feature (non-master) branch for my PR.
  • I have written tests for my code changes.
  • Code climate

@jjlakis jjlakis requested a review from a team as a code owner December 22, 2023 16:50
@jjlakis jjlakis changed the title Add Azure Workload Identity support for certificate credentials WIP: Add Azure Workload Identity support for certificate credentials Dec 22, 2023
@jjlakis jjlakis changed the title WIP: Add Azure Workload Identity support for certificate credentials Add Azure Workload Identity support for certificate credentials Dec 22, 2023
@tuunit
Copy link
Member

tuunit commented Jan 6, 2024

Hi @jjlakis,

could you please test if the solution proposed in #2305 would work for your use case as well.

@jjlakis
Copy link
Contributor Author

jjlakis commented Jan 7, 2024

Hi @jjlakis,

could you please test if the solution proposed in #2305 would work for your use case as well.

Hey @tuunit . Looks like mentioned PR indeed allows me to use certificate credentials instead of secret for generic OIDC (Azure is OIDC compliant) so I would say - theoretically yes, however I couldn't test it due to invalid memory address or nil pointer dereference (see my comment).

But even if we could get #2305 working as expected, there is still an important reason to use Azure provider instead of OIDC, that is: Limit of 200 groups inside JWT. So in this corner case, we cannot fully rely on reading groups from the JWT claims, and need to call Graph API. Another (less important) reason could be a configuration experience for AKS users.

@tuunit
Copy link
Member

tuunit commented Jan 7, 2024

Hi @jjlakis,

it would be amazing if we could explore what would be necessary to get the AzureProvider to be a "child struct" of the OIDC provider similar to how we already do it for keycloak-oidc and others.

https://github.com/oauth2-proxy/oauth2-proxy/blob/master/providers/keycloak_oidc.go#L24-L26
https://github.com/oauth2-proxy/oauth2-proxy/blob/master/providers/keycloak_oidc.go#L63-L65
https://github.com/oauth2-proxy/oauth2-proxy/blob/master/providers/providers.go#L176-L185

My hope is that we can generalize the logic even more and reduce the complexity inside the provider specific files. Overall this is a broader topic and doesn't only concern the Azure provider.

@jjlakis
Copy link
Contributor Author

jjlakis commented Jan 8, 2024

Hey @tuunit , so let me dig into this topic deeper then 💪 . I see you will be taking over the client assertion changes in the OIDC provider, that's lovely 👌

@jjlakis
Copy link
Contributor Author

jjlakis commented Jan 15, 2024

Closed in favor of #2390 and #2378

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Azure AKS workload identity support
2 participants