Bug 1387306 - Secrets have S_ISGID flag set
Summary: Secrets have S_ISGID flag set
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: OpenShift Container Platform
Classification: Red Hat
Component: Node
Version: 3.2.0
Hardware: Unspecified
OS: Unspecified
unspecified
urgent
Target Milestone: ---
: ---
Assignee: Seth Jennings
QA Contact: Qixuan Wang
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-10-20 15:12 UTC by Francesco Marchioni
Modified: 2017-03-08 18:43 UTC (History)
9 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Openshift uses fsGroup in the pod spec to set volume permissions in unprivileged pods. The S_ISGID bit is set on all directories in the volume such that new files inherit the group id. However the bit is also set for files, for which it has a different meaning (mandatory file locking, see stat(2)). This is now fixed to only set the S_ISGID bit on directories.
Clone Of:
Environment:
Last Closed: 2017-01-18 12:43:53 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2017:0066 0 normal SHIPPED_LIVE Red Hat OpenShift Container Platform 3.4 RPM Release Advisory 2017-01-18 17:23:26 UTC

Description Francesco Marchioni 2016-10-20 15:12:52 UTC
Description of problem:

Starting in OSE 3.2, secret files have S_ISGID flag set.  This is causing issues with applications that check the flags set on secrets and prevents us from deploying OSE 3.2 (or 3.3 for that matter).

Version-Release number of selected component (if applicable): 3.2.0


How reproducible:

* Create a project

```
oc login --username=project-admin
oc new-project secret-project
```

* Create a secret:

```
oc create -f - <<EOF
apiVersion: "v1"
kind: "Secret"
metadata:
  name: "top-secret"
data:
  username: "dmFsdWUtMQ0K"
  password: "dmFsdWUtMg0KDQo="
EOF
```

* Consume the secret:

```
oc create -f - <<EOF
apiVersion: v1
kind: Pod
metadata:
  name: secret-example-pod
spec:
  containers:
    - name: secret-test-container
      image: busybox
      command: [ "/bin/sh", "-c", "echo -- 1 -------- ; ls -ld /etc/secret-volume ; echo -- 2 -------- ; ls -l /etc/secret-volume/*" ]
      volumeMounts:
          # name must match the volume name below
          - name: secret-volume
            mountPath: /etc/secret-volume
            readOnly: true
  volumes:
    - name: secret-volume
      secret:
        secretName: top-secret
  restartPolicy: Never
EOF
```

Pod logs from `oc logs secret-example-pod`:

```
-- 1 --------
drwxrwsrwt    2 root     10000500        80 Oct 19 16:10 /etc/secret-volume
-- 2 --------
-r--r-Sr--    1 root     10000500        11 Oct 19 16:10 /etc/secret-volume/password
-r--r-Sr--    1 root     10000500         9 Oct 19 16:10 /etc/secret-volume/username
```


Actual results:

Observe the setgid flag on the files in addition to the directory.

S_ISGID on the directories is normal and indicates that files created inside them should inherit the group ownership. S_ISGID on files indicates 'mandatory file locking', which is in any case deprecated, and looks like it is set accidentally as a side effect of setting the flag on the directories.


Expected results: Secrets shouldn't have S_ISGID flag set


Additional info: A patch is available here:

https://github.com/EricMountain-1A/kubernetes/commit/411ab85368517442a74af95ef6efbab5121530a0

Could it be incorporated with the upstream project ?


Reference: https://c.na7.visual.force.com/apex/Case_View?id=500A000000VX3mE&sfdc.override=1

Comment 1 Paul Morie 2016-10-20 15:21:10 UTC
We do need to get this upstream, yes.

Comment 2 Paul Morie 2016-10-31 19:15:13 UTC
*** Bug 1384458 has been marked as a duplicate of this bug. ***

Comment 4 Seth Jennings 2016-11-08 15:53:37 UTC
Kube PR:
https://github.com/kubernetes/kubernetes/pull/36386

Origin cherry-pick PR:
https://github.com/openshift/origin/pull/11816

Comment 5 Troy Dawson 2016-11-11 19:28:04 UTC
This has been merged into ose and is in OSE v3.4.0.25 or newer.

Comment 7 Qixuan Wang 2016-11-14 10:06:08 UTC
Tested on OCP v3.4.0.25 (openshift v3.4.0.25+1f36858, kubernetes v1.4.0+776c994)

[root@dhcp-140-13 v3test]# oc logs secret-example-pod
-- 1 --------
drwxrwsrwt    3 root     10011300       120 Nov 14 08:01 /etc/secret-volume
-- 2 --------
lrwxrwxrwx    1 root     root            15 Nov 14 08:01 /etc/secret-volume/password -> ..data/password
lrwxrwxrwx    1 root     root            15 Nov 14 08:01 /etc/secret-volume/username -> ..data/username

Seems the S_ISGID on directory is kept and on file is moved.

However, comparing with upstream (I used test files on bug https://bugzilla.redhat.com/show_bug.cgi?id=1384458 for entering the pod), I still have two questions.

Question 1: Upstream doesn't set S_ISGID on the directory, processes under the directory may cover/effect each other, meanwhile, OpenShift introduces group operations, that's where OpenShift permission management is better than upstream. That's my understanding. Is it correct?

[Upstream]
# ls -ld /etc/foo
drwxrwxrwt. 3 root root 120 Nov 14 09:09 /etc/foo

[OpenShift]
$ ls -ld /etc/foo
drwxrwsrwt. 3 root 1000040000 120 Nov 14 08:43 /etc/foo


Question 2: I set secret defaultMode: 0744, so secrets permission in the volume is -rwxr--r--, that's correct. But in OpenShift it's still -rwxrwxrwx. Should I reopen https://bugzilla.redhat.com/show_bug.cgi?id=1384458 ?

[Upstream]
# ls -laR /etc/foo
/etc/foo:
total 4
drwxrwxrwt.  3 root root  120 Nov 14 09:09 .
drwxr-xr-x. 44 root root 4096 Nov 14 09:09 ..
drwxr-xr-x.  2 root root   80 Nov 14 09:09 ..119811_14_11_04_09_38.741096121
lrwxrwxrwx.  1 root root   33 Nov 14 09:09 ..data -> ..119811_14_11_04_09_38.741096121
lrwxrwxrwx.  1 root root   13 Nov 14 09:09 data-1 -> ..data/data-1
lrwxrwxrwx.  1 root root   13 Nov 14 09:09 data-2 -> ..data/data-2

/etc/foo/..119811_14_11_04_09_38.741096121:
total 8
drwxr-xr-x. 2 root root  80 Nov 14 09:09 .
drwxrwxrwt. 3 root root 120 Nov 14 09:09 ..
-rwxr--r--. 1 root root   9 Nov 14 09:09 data-1
-rwxr--r--. 1 root root  11 Nov 14 09:09 data-2

[OpenShift]
$ ls -laR /etc/foo
/etc/foo:
total 4
drwxrwsrwt.  3 root 1000040000  120 Nov 14 08:43 .
drwxr-xr-x. 44 root root       4096 Nov 14 06:25 ..
drwxrwxrwx.  2 root 1000040000   80 Nov 14 09:14 ..119811_14_11_03_43_31.527057293
lrwxrwxrwx.  1 root 1000040000   33 Nov 14 08:43 ..data -> ..119811_14_11_03_43_31.527057293
lrwxrwxrwx.  1 root root         13 Nov 14 06:24 data-1 -> ..data/data-1
lrwxrwxrwx.  1 root root         13 Nov 14 06:24 data-2 -> ..data/data-2

/etc/foo/..119811_14_11_03_43_31.527057293:
total 8
drwxrwxrwx. 2 root 1000040000  80 Nov 14 09:14 .
drwxrwsrwt. 3 root 1000040000 120 Nov 14 08:43 ..
-rwxrwxrwx. 1 root 1000040000   9 Nov 14 08:43 data-1
-rwxrwxrwx. 1 root 1000040000  11 Nov 14 08:43 data-2

Comment 8 Seth Jennings 2016-11-14 14:58:29 UTC
Regarding question 2, yes you should reopen as the fix for this bug did not address the issue in 1384458

Question 1, I'll need to do some more investigation.  I looks that Openshift uses user namespaces and upstream kubernetes does not.

Comment 9 Seth Jennings 2016-11-14 15:49:33 UTC
Yes, Openshift handles permissions differently i.e more securely.

In the interest of getting the fix marked as verified, I think the output you provided shows that the fix is in place.  Can we mark this as verified and track the defaultMode issue in a reopened 1384458?

$ ls -laR /etc/foo
/etc/foo:
total 4
drwxrwsrwt.  3 root 1000040000  120 Nov 14 08:43 . <- ISGID set
drwxr-xr-x. 44 root root       4096 Nov 14 06:25 ..
drwxrwxrwx.  2 root 1000040000   80 Nov 14 09:14 ..119811_14_11_03_43_31.527057293
lrwxrwxrwx.  1 root 1000040000   33 Nov 14 08:43 ..data -> ..119811_14_11_03_43_31.527057293
lrwxrwxrwx.  1 root root         13 Nov 14 06:24 data-1 -> ..data/data-1 <- ISGID not set
lrwxrwxrwx.  1 root root         13 Nov 14 06:24 data-2 -> ..data/data-2 <- ISGID not set

Comment 10 Zhang Cheng 2016-11-15 02:53:19 UTC
@Jennings 
Base on Qixuan Wang's test result, upstream doesn't set S_ISGID on the directory.
But, I found Kube PR have been merged: https://github.com/kubernetes/kubernetes/pull/36386

Comment 11 Andy Goldstein 2016-11-15 11:29:47 UTC
This is an OCP bug. No need to test Kube. If it works in OCP, please move to verified. Thanks!

Comment 13 errata-xmlrpc 2017-01-18 12:43:53 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2017:0066


Note You need to log in before you can comment on or make changes to this bug.