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

Fix security-context for apiserver - audit logs are supported only in Enterprise version #2906

Merged

Conversation

mihivagyok
Copy link
Contributor

@mihivagyok mihivagyok commented Sep 29, 2023

Description

kind/bug

It fixes the case when the operator installs apiserver OSS version. In case of OSS Calico, it shall run as non-root as audit logs are only supported in Enterprise version.
The change makes apiserver deployment more secure (least privilege).

For PR author

  • Tests for change.
  • If changing pkg/apis/, run make gen-files
  • If changing versions, run make gen-versions

For PR reviewers

A note for code reviewers - all pull requests must have the following:

  • Milestone set according to targeted release.
  • Appropriate labels:
    • kind/bug if this is a bugfix.
    • kind/enhancement if this is a a new feature.
    • enterprise if this PR applies to Calico Enterprise only.

@CLAassistant
Copy link

CLAassistant commented Sep 29, 2023

CLA assistant check
All committers have signed the CLA.

@mihivagyok mihivagyok changed the title Fix security-context - audit logs are supported only in Enterprise version Fix security-context for apiserver - audit logs are supported only in Enterprise version Sep 29, 2023
@mihivagyok mihivagyok force-pushed the run-apiserver-non-root-oss-openshift branch 2 times, most recently from 7e081ef to 6427e23 Compare September 29, 2023 09:09
@danudey danudey modified the milestones: v1.32.0, v1.32.1, v1.32.2 Dec 2, 2023
@danudey danudey modified the milestones: v1.32.2, v1.32.3 Dec 15, 2023
@danudey danudey modified the milestones: v1.32.3, v1.32.4 Jan 18, 2024
@tmjd
Copy link
Member

tmjd commented Feb 6, 2024

Have you tested this out in a real system (I'm guessing so because of your other PRs)?
I'm asking because I thought the User and Group IDs had to match the one built into the image but I am by no means confident in that thought. So if you've tried it out and it works then I'm obviously wrong.

pkg/render/apiserver.go Outdated Show resolved Hide resolved
…, it shall run as non-root

Signed-off-by: Adam Mihelcsik <18672841+mihivagyok@users.noreply.github.com>
@mihivagyok mihivagyok force-pushed the run-apiserver-non-root-oss-openshift branch from 6427e23 to f962a05 Compare February 6, 2024 18:59
@mihivagyok
Copy link
Contributor Author

@tmjd I will get back to you with some test result tomorrow! Thanks!

Copy link
Member

@tmjd tmjd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tmjd
Copy link
Member

tmjd commented Feb 13, 2024

/sem-approve

@mihivagyok
Copy link
Contributor Author

E0216 17:42:12.047708       1 run_server.go:77] open /tmp/ready: permission denied
F0216 17:42:12.047724       1 hooks.go:203] PostStartHook "apiserver-autoregistration" failed: open /tmp/ready: permission denied

Seems it is not really working. Try to figure out why:
https://github.com/projectcalico/calico/blob/release-v3.27/apiserver/cmd/apiserver/server/run_server.go#L75

@hjiawei
Copy link
Contributor

hjiawei commented Feb 16, 2024

E0216 17:42:12.047708       1 run_server.go:77] open /tmp/ready: permission denied
F0216 17:42:12.047724       1 hooks.go:203] PostStartHook "apiserver-autoregistration" failed: open /tmp/ready: permission denied

Seems it is not really working. Try to figure out why: https://github.com/projectcalico/calico/blob/release-v3.27/apiserver/cmd/apiserver/server/run_server.go#L75

That permission error could be caused by https://github.com/projectcalico/calico/blob/release-v3.27/apiserver/docker-image/Dockerfile.amd64#L11 so the recreated /tmp folder is 0755. Non-root apiserver process won't be able to write to that directory.

The /tmp/ready file is removed and /tmp folder is reworked from calico/base. However, these are only in the coming Calico v3.28. I believe this change will likely to work with latest Calico master apiserver.

@mihivagyok
Copy link
Contributor Author

mihivagyok commented Feb 16, 2024

@hjiawei Thanks for pointing out!
On the master branch, the Dockerfile is quite different:
https://github.com/projectcalico/calico/blob/master/apiserver/Dockerfile

Also, I don't really understand the comment here. https://github.com/projectcalico/calico/blob/release-v3.27/apiserver/docker-image/Dockerfile.amd64#L8 For me, it seems it does nothing special with /code.

Thanks!

@radTuti radTuti modified the milestones: v1.32.4, v1.32.5 Feb 16, 2024
@radTuti radTuti modified the milestones: v1.32.5, v1.32.6 Feb 16, 2024
@hjiawei
Copy link
Contributor

hjiawei commented Feb 20, 2024

Also, I don't really understand the comment here. https://github.com/projectcalico/calico/blob/release-v3.27/apiserver/docker-image/Dockerfile.amd64#L8 For me, it seems it does nothing special with /code.

Yeah. You are right about this @mihivagyok . It is related to /tmp folder permission and not /code. We reworked Dockerfiles on the main branch so it is different / unified for multi-arch.

@mihivagyok
Copy link
Contributor Author

With latest changes:

  • deployment:
➜ kubectl get deployment -n calico-apiserver calico-apiserver -o yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  annotations:
    deployment.kubernetes.io/revision: "1"
  creationTimestamp: "2024-02-20T16:35:18Z"
  generation: 1
  labels:
    apiserver: "true"
    app.kubernetes.io/name: calico-apiserver
    k8s-app: calico-apiserver
  name: calico-apiserver
  namespace: calico-apiserver
  ownerReferences:
  - apiVersion: operator.tigera.io/v1
    blockOwnerDeletion: true
    controller: true
    kind: APIServer
    name: default
    uid: c580d54c-30d8-449b-a0bf-5205c90296b2
  resourceVersion: "400266"
  uid: 87600bdc-7bf8-4f50-9c54-d93306caaefc
spec:
  progressDeadlineSeconds: 600
  replicas: 2
  revisionHistoryLimit: 10
  selector:
    matchLabels:
      apiserver: "true"
  strategy:
    type: Recreate
  template:
    metadata:
      annotations:
        tigera-operator.hash.operator.tigera.io/calico-apiserver-certs: 5f64c431bd397c8b87c8ebcc7721ac1c9f7206dc
      creationTimestamp: null
      labels:
        apiserver: "true"
        app.kubernetes.io/name: calico-apiserver
        k8s-app: calico-apiserver
      name: calico-apiserver
      namespace: calico-apiserver
    spec:
      affinity:
        podAntiAffinity:
          preferredDuringSchedulingIgnoredDuringExecution:
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  k8s-app: calico-apiserver
              namespaces:
              - calico-apiserver
              topologyKey: kubernetes.io/hostname
            weight: 100
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  k8s-app: calico-apiserver
              namespaces:
              - calico-apiserver
              topologyKey: topology.kubernetes.io/zone
            weight: 100
      containers:
      - args:
        - --secure-port=5443
        - --tls-private-key-file=/calico-apiserver-certs/tls.key
        - --tls-cert-file=/calico-apiserver-certs/tls.crt
        env:
        - name: DATASTORE_TYPE
          value: kubernetes
        - name: KUBERNETES_SERVICE_HOST
          value: 172.21.0.1
        - name: KUBERNETES_SERVICE_PORT
          value: "443"
        - name: MULTI_INTERFACE_MODE
          value: none
        image: us.icr.io/armada-master/calico/apiserver:v3.27.0
        imagePullPolicy: IfNotPresent
        livenessProbe:
          failureThreshold: 3
          httpGet:
            path: /version
            port: 5443
            scheme: HTTPS
          initialDelaySeconds: 90
          periodSeconds: 60
          successThreshold: 1
          timeoutSeconds: 10
        name: calico-apiserver
        readinessProbe:
          exec:
            command:
            - /code/filecheck
          failureThreshold: 5
          initialDelaySeconds: 5
          periodSeconds: 30
          successThreshold: 1
          timeoutSeconds: 10
        resources: {}
        securityContext:
          allowPrivilegeEscalation: false
          capabilities:
            drop:
            - ALL
          privileged: false
          runAsGroup: 10001
          runAsNonRoot: true
          runAsUser: 10001
          seccompProfile:
            type: RuntimeDefault
        terminationMessagePath: /dev/termination-log
        terminationMessagePolicy: File
        volumeMounts:
        - mountPath: /calico-apiserver-certs
          name: calico-apiserver-certs
          readOnly: true
        - mountPath: /tmp
          name: cache-volume
      dnsPolicy: ClusterFirst
      nodeSelector:
        kubernetes.io/os: linux
      restartPolicy: Always
      schedulerName: default-scheduler
      securityContext: {}
      serviceAccount: calico-apiserver
      serviceAccountName: calico-apiserver
      terminationGracePeriodSeconds: 30
      tolerations:
      - effect: NoSchedule
        key: node-role.kubernetes.io/master
      - effect: NoSchedule
        key: node-role.kubernetes.io/control-plane
      volumes:
      - name: calico-apiserver-certs
        secret:
          defaultMode: 420
          secretName: calico-apiserver-certs
      - emptyDir: {}
        name: cache-volume
status:
  availableReplicas: 2
  conditions:
  - lastTransitionTime: "2024-02-20T16:35:50Z"
    lastUpdateTime: "2024-02-20T16:35:50Z"
    message: Deployment has minimum availability.
    reason: MinimumReplicasAvailable
    status: "True"
    type: Available
  - lastTransitionTime: "2024-02-20T16:35:19Z"
    lastUpdateTime: "2024-02-20T16:35:50Z"
    message: ReplicaSet "calico-apiserver-58fb6b4bd7" has successfully progressed.
    reason: NewReplicaSetAvailable
    status: "True"
    type: Progressing
  observedGeneration: 1
  readyReplicas: 2
  replicas: 2
  updatedReplicas: 2
  • status
➜ kubectl get pods -n calico-apiserver
NAME                                READY   STATUS    RESTARTS   AGE
calico-apiserver-58fb6b4bd7-66crp   1/1     Running   0          3m28s
calico-apiserver-58fb6b4bd7-w8ls4   1/1     Running   0          3m28s

➜ kubectl get tigerastatus apiserver
NAME        AVAILABLE   PROGRESSING   DEGRADED   SINCE
apiserver   True        False         False      2m58s

hjiawei added a commit to projectcalico/calico that referenced this pull request Feb 28, 2024
This change backports changes from [1] for `calico/apiserver` only to
fix the `/tmp` folder permission. Currently, when `/tmp` folder is
copied form ubi stage to scratch, the folder permission is 0755 so it isn't
writable for non-root processes. With the changes in [2], apiserver won't
be ready because `/tmp/ready` flag can't be written.

[1] #8299
[2] tigera/operator#2906
hjiawei added a commit to projectcalico/calico that referenced this pull request Feb 28, 2024
This change backports changes from [1] for `calico/apiserver` only to
fix the `/tmp` folder permission. Currently, when `/tmp` folder is
copied form ubi stage to scratch, the folder permission is 0755 so it isn't
writable for non-root processes. With the changes in [2], apiserver won't
be ready because `/tmp/ready` flag can't be written.

[1] #8299
[2] tigera/operator#2906
hjiawei added a commit to projectcalico/calico that referenced this pull request Feb 28, 2024
This change backports changes from [1] for `calico/apiserver` only to
fix the `/tmp` folder permission. Currently, when `/tmp` folder is
copied form ubi stage to scratch, the folder permission is 0755 so it isn't
writable for non-root processes. With the changes in [2], apiserver won't
be ready because `/tmp/ready` flag can't be written.

[1] #8299
[2] tigera/operator#2906
hjiawei added a commit to projectcalico/calico that referenced this pull request Feb 28, 2024
This change backports changes from [1] for `calico/apiserver` only to
fix the `/tmp` folder permission. Currently, when `/tmp` folder is
copied form ubi stage to scratch, the folder permission is 0755 so it isn't
writable for non-root processes. With the changes in [2], apiserver won't
be ready because `/tmp/ready` flag can't be written.

[1] #8299
[2] tigera/operator#2906
@radTuti radTuti modified the milestones: v1.32.6, v1.32.7 Mar 25, 2024
@tmjd
Copy link
Member

tmjd commented Mar 29, 2024

AFAICT we're still waiting to drop the last/2nd commit that added an emptyDir volume.

@mihivagyok
Copy link
Contributor Author

@tmjd Let me revert those. Sorry for the delay!

@mihivagyok mihivagyok force-pushed the run-apiserver-non-root-oss-openshift branch from 6c3ecea to f962a05 Compare March 29, 2024 20:22
@tmjd
Copy link
Member

tmjd commented Mar 29, 2024

/sem-approve

@mihivagyok
Copy link
Contributor Author

All tests are passed :) Thank you!

@tmjd tmjd merged commit 21356ae into tigera:master Apr 1, 2024
7 checks passed
@tmjd
Copy link
Member

tmjd commented Apr 1, 2024

Thank you @mihivagyok for this PR

mihivagyok pushed a commit to mihivagyok/operator that referenced this pull request Apr 12, 2024
…-oss-openshift

Fix security-context for apiserver - audit logs are supported only in Enterprise version
mihivagyok pushed a commit to mihivagyok/operator that referenced this pull request Apr 12, 2024
…-oss-openshift

Fix security-context for apiserver - audit logs are supported only in Enterprise version
mihivagyok pushed a commit to mihivagyok/operator that referenced this pull request Apr 12, 2024
…-oss-openshift

Fix security-context for apiserver - audit logs are supported only in Enterprise version
rene-dekker pushed a commit that referenced this pull request Apr 26, 2024
…penshift (#3310)

Fix security-context for apiserver - audit logs are supported only in Enterprise version

Co-authored-by: Erik Stidham <erik@tigera.io>
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.

7 participants