Bug 1324926 (CVE-2016-3106)
| Summary: | CVE-2016-3106 pulp: Insecure creation of temporary directory when generating new CA key | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Other] Security Response | Reporter: | Adam Mariš <amaris> | ||||||||||
| Component: | vulnerability | Assignee: | Red Hat Product Security <security-response-team> | ||||||||||
| Status: | CLOSED NOTABUG | QA Contact: | |||||||||||
| Severity: | low | Docs Contact: | |||||||||||
| Priority: | low | ||||||||||||
| Version: | unspecified | CC: | fweimer, rbarlow, sean.myers, security-response-team | ||||||||||
| Target Milestone: | --- | Keywords: | Security | ||||||||||
| Target Release: | --- | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | Linux | ||||||||||||
| Whiteboard: | |||||||||||||
| Fixed In Version: | Doc Type: | Bug Fix | |||||||||||
| Doc Text: | Story Points: | --- | |||||||||||
| Clone Of: | Environment: | ||||||||||||
| Last Closed: | 2019-06-08 02:50:25 UTC | Type: | --- | ||||||||||
| Regression: | --- | Mount Type: | --- | ||||||||||
| Documentation: | --- | CRM: | |||||||||||
| Verified Versions: | Category: | --- | |||||||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||||||
| Embargoed: | |||||||||||||
| Bug Depends On: | |||||||||||||
| Bug Blocks: | 1322708 | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Adam Mariš
2016-04-07 15:22:21 UTC
Acknowledgments: Name: Florian Weimer (Red Hat), Sander Bos Created attachment 1144778 [details]
Proposed patch
For the record, Sander Bos has also independently reported this issue to the Pulp team. Thanks to both Florian and Sander! Created attachment 1145757 [details]
Second patch proposal
Sander Bos recommended also setting the umask on the patch, which I think is a good addition to the patch. I am attaching that version of the patch here as well.
Created attachment 1145973 [details]
Second patch proposal
Here is the same patch with attribution included in the AUTHORS file.
Created attachment 1146458 [details]
Proposed patch
I discovered that the umask statement in my second revision of the patch was unnecessary, so this revision removes that. Additionally, this version adds a -Z to the mv statements at the end so that the correct SELinux label will get applied when the certificate and key are moved to their final destination.
(In reply to Randy Barlow from comment #6) > Created attachment 1146458 [details] > Proposed patch > > I discovered that the umask statement in my second revision of the patch was > unnecessary, so this revision removes that. Additionally, this version adds > a -Z to the mv statements at the end so that the correct SELinux label will > get applied when the certificate and key are moved to their final > destination. Unfortunately, “mv -Z” has a race condition and is unsafe if the destination directory is owned by an untrusted user. If the directory is owned by root, it is fine in this context, but I don't know if this applies here. Hello Florian, thanks for letting me know about the race condition, I was not aware. Here are the permissions on the destination folder: $ ls -lah /etc/pki/pulp | head -n 6 total 28K drwxr-xr-x. 3 root root 4.0K Apr 10 13:31 . drwxr-xr-x. 10 root root 4.0K Apr 10 13:29 .. -rw-r-----. 1 root apache 1.8K Apr 10 13:31 ca.crt -rw-r-----. 1 root apache 3.2K Apr 10 13:31 ca.key drwxr-xr-x. 2 root root 4.0K Apr 10 13:29 nodes Thus this CA does get installed to a directory owned by root. However, do you have a proposal of a better way to get the correct SELinux contexts applied that would be safe in all conditions? This issue is filed upstream as #1827 and is fixed by PR #2527: https://pulp.plan.io/issues/1827 https://github.com/pulp/pulp/pull/2527 |