Secrets in containerized nodes are wrong due to encoding #5778
Conversation
Results in secret content being incorrect, service account ca.crt has \n instead of actual newlines.
smarterclayton
force-pushed
the
fix_secret_in_container
branch
from
24d0e5c to
b2e0262
Compare
|
[test] |
|
Evaluated for origin test up to b2e0262 |
|
@sdodson this resulted in containerized node being broken, which breaks builds and other things. If you were having problems with pushes, builds, anything that required auth to the master, this was it (was deploy broken for you?) |
smarterclayton
added
priority/P1
approved
| @@ -56,10 +58,11 @@ func (writer *NsenterWriter) WriteFile(filename string, data []byte, perm os.Fil | |||
| "--", | |||
| } | |||
|
|
|||
| echo_args := append(base_args, "sh", "-c", | |||
| fmt.Sprintf("echo %q | cat > %s", data, filename)) | |||
| echo_args := append(base_args, "sh", "-c", fmt.Sprintf("cat > %s", filename)) | |||
There was a problem hiding this comment.
I don't know all the things that call this. Is file name safe unquoted and unescaped?
There was a problem hiding this comment.
There is only one code path today that invokes this method, the inputs are the volume mount path (not user input) and secret data key (user input) but secret data key is strongly validated and path traversal should not be possible. Opened kubernetes/kubernetes#16971 to track resolution of that.
After review, for this code, in this spot, I believe we are safe unless someone else starts calling WriteFile.
|
Core change looks good, just the question on file name |
|
Question answered sufficiently? |
|
yeah, LGTM. Would like a follow up issue to revisit |
|
Upstream issue covers the revisit. |
|
[merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/6987/) (Image: devenv-rhel7_2657) |
|
Evaluated for origin merge up to b2e0262 |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/6987/) |
No, I wasn't running into any problems like that. I was able to build the quickstarts just fine. Though I'm slightly confused as to why we weren't having problems now. |
|
Builds are fine, deployments are likely to fail. On Nov 7, 2015, at 7:30 PM, Scott Dodson [email protected] wrote: @sdodson https://github.com/sdodson this resulted in containerized node No, I wasn't running into any problems like that. I was able to build — |
The upstream "fix" for writing secrets onto disk while containerized improperly mangled the content of the node.
@liggitt this is candidate because of containerized.