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

Storage redis URL from Secret is transformed into plain container command leaking possible password #106

Closed
slopezz opened this issue Oct 5, 2023 · 2 comments · Fixed by #109
Assignees

Comments

@slopezz
Copy link

slopezz commented Oct 5, 2023

I have been successfully testing limitador-operator v0.6.0, and I have identified a possible not intended credentials leak in the deployment container command.

I deployed the following CR called cluster:

apiVersion: limitador.kuadrant.io/v1alpha1
kind: Limitador
metadata:
  name: cluster
spec:
  affinity:
    podAntiAffinity:
      preferredDuringSchedulingIgnoredDuringExecution:
        - podAffinityTerm:
            labelSelector:
              matchLabels:
                app: limitador
                limitador-resource: cluster
            topologyKey: kubernetes.io/hostname
          weight: 100
        - podAffinityTerm:
            labelSelector:
              matchLabels:
                app: limitador
                limitador-resource: cluster
            topologyKey: topology.kubernetes.io/zone
          weight: 99
  limits:
    - conditions: []
      max_value: 400
      namespace: kuard
      seconds: 1
      variables:
        - per_hostname_per_second_burst
  listener:
    grpc:
      port: 8081
    http:
      port: 8080
  pdb:
    maxUnavailable: 1
  replicas: 3
  resourceRequirements:
    limits:
      cpu: 500m
      memory: 64Mi
    requests:
      cpu: 250m
      memory: 32Mi
  storage:
    redis:
      configSecretRef:
        name: redisconfig

Then redis storage is configured on an external secret with the connection string set at URL, and I guess it is a secret and not a configmap, because connection string to connect to redis might have use user/password:

apiVersion: v1
kind: Secret
metadata:
  name: redisconfig
stringData:
  URL: redis://127.0.0.1/a # Redis URL of its running instance
type: Opaque

However, instead of mounting the secret on the deployment and extract the URL into possibly an ENVVAR, it is taking the URL from the secret, and configure it directly on the container command showing its plain value (even if it possibly has a secret password):

          command:
            - limitador-server
            - /home/limitador/etc/limitador-config.yaml
            - redis
            - 'redis://redis:6379'

My recommendation would be to extract its value like any standard deployment and inject it on an ENVVAR maybe, something similar to:

          env:
            - name: URL
              valueFrom:
                secretKeyRef:
                  name: limits-config-cluster
                  key: URL

And then, you will need also to update how to use its value from the container command.

@eguzki
Copy link
Contributor

eguzki commented Oct 5, 2023

This is tricky.

I think we can explore the expansion feature from container environment. From kubernetes 1.25 ref:

command
string array

Entrypoint array. Not executed within a shell. The container image's ENTRYPOINT is used if this is not provided. Variable references $(VAR_NAME) are expanded using the container's environment. If a variable cannot be resolved, the reference in the input string will be unchanged. Double $$ are reduced to a single $, which allows for escaping the $(VAR_NAME) syntax: i.e. "$$(VAR_NAME)" will produce the string literal "$(VAR_NAME)". Escaped references will never be expanded, regardless of whether the variable exists or not. Cannot be updated. More info: https://kubernetes.io/docs/tasks/inject-data-application/define-command-argument-container/#running-a-command-in-a-shell

@Boomatang
Copy link
Contributor

Passing the Redis URL via a environment variable and not exposed in the deployment works straight forward.

The one side affect of taken this approach seems to mean the secret with the redis credentials needs to be defined in the same namespace as the limitador deployment.

PR has being put together with the changes required. #109

@Boomatang Boomatang moved this from In Progress to Review in Kuadrant Service Protection Oct 25, 2023
@github-project-automation github-project-automation bot moved this from Review to To test in Kuadrant Service Protection Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: To test
Development

Successfully merging a pull request may close this issue.

3 participants