Hardcoded Secret in Dex Authenticator ConfigMap

[VF-mbrauer]

While reviewing the ConfigMaps of the dex Kubernetes namespace, it was found that a secret is hardcoded and stored in clear text inside the ConfigMap dex-auth-dex-k8sauthenticator. Storing secret values in clear text within ConfigMaps potentially allows anyone with permissions to review ConfigMaps to obtain sensitive information, potentially causing other/unspecified harm.

#kubectl get cm -n dex dex-auth-dex-k8s-authenticator -oyaml

data:
  config.yaml: |-
    listen: http://0.0.0.0:5555
    web_path_prefix: /
    debug: false
    logo_uri: mylogo.logo.com
    clusters:
    - client_id: dex-k8s-authenticator
      client_secret: <mysecret-key>
      description: Please click here to generate the 24h token...
      issuer: https://my-url-to-dex```                              

[xunholy]

Agreed, this did seem strange that we'd store the client secret in the configmap resource.

[VF-mbrauer]

@xunholy something which will be solved soon?

[xunholy]

I'm not a contributor or maintainer in this particular repo. I'm sure a pull request fix would always be welcomed though.

[VF-mbrauer]

Hi @nabadger, can you check the PR, please?

cc: @xunholy

[nabadger]

@VF-mbrauer @xunholy I'm not really sure what this solves? b64 is just encoding and doesn't encrypt/protect data anymore (i.e. the user can simply b64decode it).

[VF-mbrauer]

Yes, there is still more to do. But some Security departments would be still happy to see at least an Encoding and not the real secrets. And yes @nabadger you are right, it is possible to decode, but at the first glance, we do not have the real secret in the config map. With the next version, we could also split some of the sensitive content and move it completely to the secrets area of k8s.

[nabadger]

I'm not really convinced on going down the b64 route as it doesn't increase security (dexidp don't do this: https://github.com/dexidp/dex/blob/766fc7ad990b51f656e03f03e157ba81da132552/examples/config-dev.yaml#L130).

There is talk about whether client-secret is needed at all (i.e. PKCE could avoid this...but I'm not sure that's right either).

See #80

---

For this area, I would rather jump straight to a solution which allows the secret to be referenced from a k8s Secret - these are not secret either (they are just b64 too), but at least it would allow the user more choice in how that secret is managed, and would not be part of the configmap - you would have some extra control over blocking access to that secret via RBAC too if desired).

I think this might be a better approach (given that if we did b64 encode this, that would be a breaking change anyway in its current form).

[nabadger]

Actually, we might already have a solution in place for this.

The configuration values can be environment variables, which means you should be able to do this:

data:
  config.yaml: |-
    listen: http://0.0.0.0:5555
    web_path_prefix: /
    debug: false
    logo_uri: mylogo.logo.com
    clusters:
    - client_id: dex-k8s-authenticator
      client_secret: ${A_SECRET_FROM_THE_ENV}
      description: Please click here to generate the 24h token...
      issuer: https://my-url-to-dex```                    

You can then set this via the new envFrom support (which can reference a secret) - This is soon to be merged ( #175 )

How does that sound?

[xunholy]

Hi @nabadger thanks for sharing your views; I want to confirm that secrets although are base64 encoded which is not en encryption should be using the secret kubernetes resource type as intended. There is no real good argument against this - secret resources are designed to create a separation of duties and distinguish between application config and sensitive data, this can be imposed using RBAC to create fine grained control on what is appropriate. Additionally secrets in most organisations are all encrypted at rest - only the encoded value is presented to a client that has appropriate authN and authZ. Using sensitive information / secrets in ENV vars is highly also discouraged within the kubernetes security CIS benchmark (best explanation can be found directly from the CIS report).

EDIT* This particular topic is mostly about following best security practices and standards; There are a number of existing applications that do not adhere to this standard as you've already highlighted one but that should not be the reason.

[nabadger]

@xunholy I agree with all that, but not sure how the current PR helps this?

#179 (comment) achieves this except it provides secrets via env-vars. This isn't ideal as per CIS, but is essentially what most apps support these days, since the latter is somewhat more work for developers and can increase code complexity.

If we want to support secrets via files (via a secret-volume mount with desired perms), we need to come up with a way to map the values in that file (most likely a mapping of client-id->client-secret) into the desired structure in code.

I imagine this might involve making the dex-k8s-auth process accept both a config file and another associated-secrets file.

Maybe the process (or entrypoint) can just source the creds file (which can be locked down via permissions), then it can re-use the existing env-var substitution impl. but the vars (secrets) will only be visible to the process (not the whole pod env).

[xunholy]

I haven't actually reviewed the proposed PR but if it doesn't align to what I mentioned above then I agree with you.

#179 (comment) achieves this except it provides secrets via env-vars. This isn't ideal as per CIS, but is essentially what most apps support these days, since the latter is somewhat more work for developers and can increase code complexity.

In terms of this change, we should just be reading in from a secret which should be created as some particular mount path, validating the file exists and if so reading in the secret - You might also support ENVs, perhaps this is a phases approach and you could support ENVs with the longer term goal of reading it from the secret instead?

You could have an ENV that points to the secret location if indeed you made it dynamic but because we control the chart in this repo it might not be that hard to enforce its location.

Maybe the process (or entrypoint) can just source the creds file (which can be locked down via permissions), then it can re-use the existing env-var substitution impl. but the vars (secrets) will only be visible to the process (not the whole pod env).

Not sure on the implications this has but could also work?

[VF-mbrauer]

I'm not really convinced on going down the b64 route as it doesn't increase security (dexidp don't do this: https://github.com/dexidp/dex/blob/766fc7ad990b51f656e03f03e157ba81da132552/examples/config-dev.yaml#L130).

There is talk about whether client-secret is needed at all (i.e. PKCE could avoid this...but I'm not sure that's right either).

See #80

---

Hi @nabadger, dexidp does also store its secrets plain in the configuration. BUT it then stores it in secrets of k8s. That's the difference in this case.

So if you guys don't agree on the PR, and the way it does the implementation then fine. But then I would propose the solution to split that part out of configmap and make it a Mount of secrets on k8s.

ENV I don't think that this should be used from my view. It creates additional complexity.

What do you think?

[nabadger]

@VF-mbrauer simplest solution is to have the entrypoint source a credentials file I think from the docker-entrypoint.

credentials-file=/var/run/secrets/dex-k8s-authenticator/credentials
if $credentials file-exists
  source  $credentials-file
fi

Your credentials file can just be a k8s secret volume, and could look something like this:

MY_CLIENT_SECRET=foo
MY_OTHER_CLIENT_SECRET=foo2

This is also generic and can work for multiple clusters (multiple secrets).

Your configmap can look like this:

data:
  config.yaml: |-
    listen: http://0.0.0.0:5555
    web_path_prefix: /
    debug: false
    logo_uri: mylogo.logo.com
    clusters:
    - client_id: dex-k8s-authenticator
      client_secret: ${MY_CLIENT_SECRET}
      description: Please click here to generate the 24h token...
      issuer: https://my-url-to-dex
    - client_id: dex-k8s-authenticator-other
      client_secret: ${MY_OTHER_CLIENT_SECRET}
      description: Please click here to generate the 24h token...
      issuer: https://my-url-to-dex-other

The code will already handle the substituion..so this should just work.

The reason not to turn the whole configmap into a secret is that it means you can still store the configmap in version control (and code-review it etc) without having to worry about encrypting it (at in version control).

[VF-mbrauer]

Hi @nabadger, thanks for the reply and sorry for the silence here from my side. Had some stuff Todo which kicked in until the end of 2021 :-)

Let continue here then. So yes, it makes sense just to pass the credentials out of the config and make them readable via a secret in k8s which we can mount as usual in parallel to the config map mounts. It makes sense and does also not change the complete logic/config for now.

So, for this we need also to adapt the entrypoint.sha and also create a secret.yaml as part of the helm chart templates. Also, we need to adapt the deployment.yaml to consume the new secret as a Volume Mount.

I have another question to the comment of

MY_CLIENT_SECRET=foo
MY_OTHER_CLIENT_SECRET=foo2

Don't we need to run it with an export to properly source it? So, something like this:

EXPORT MY_CLIENT_SECRET=foo
EXPORT MY_OTHER_CLIENT_SECRET=foo2