Fix permissions on secrets directory #3702
Conversation
The permission on this directory will prevent user namespaces from using secrets. Should be world searchable [NO TESTS NEEDED] Signed-off-by: Daniel J Walsh <[email protected]>
| @@ -2627,7 +2627,7 @@ func getSecretMount(tokens []string, secrets map[string]define.Secret, mountlabe | |||
|
|
|||
| // Copy secrets to container working dir (or tmp dir if it's an env), since we need to chmod, | |||
| // chown and relabel it for the container user and we don't want to mess with the original file | |||
| if err := os.MkdirAll(filepath.Dir(ctrFileOnHost), 0644); err != nil { | |||
| if err := os.MkdirAll(filepath.Dir(ctrFileOnHost), 0755); err != nil { | |||
| return nil, "", err | |||
| } | |||
| if err := ioutil.WriteFile(ctrFileOnHost, data, 0644); err != nil { | |||
There was a problem hiding this comment.
Do we also need to change the perms on the file itself or just the dir? Not sure
There was a problem hiding this comment.
File is fine, it is world readable, This issue is 644 means a direcory it listable but not searchable.
So a non root user would not be able to get to the data file on disk. Although they would be able to list it.
|
LGTM |
|
@rhatdan I think the overlay test failure might be real. It looks like a perm issue trying to get to a directory. |
|
Code LGTM, maybe a test case adjustment is needed? |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
The permission on this directory will prevent user namespaces from using
secrets. Should be world searchable
[NO NEW TESTS REQUIRED]
Signed-off-by: Daniel J Walsh [email protected]
What type of PR is this?
What this PR does / why we need it:
How to verify it
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?