It was found that fix for CVE-2016-3095 was incomplete, introducing new vulnerabilities due to insecure way of creating the temporary directory when generating new CA key.
Name: Florian Weimer (Red Hat), Sander Bos
Created attachment 1144778 [details]
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]
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
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
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: