Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Security audit #2190

Merged
merged 2 commits into from
Jan 31, 2020
Merged

Adding Security audit #2190

merged 2 commits into from
Jan 31, 2020

Conversation

amye
Copy link

@amye amye commented Dec 20, 2019

Adding security audit, editing readme with new security.md file.
This would be super to pass the DCO bot as everything matches now. :)

@cyphar
Copy link
Member

cyphar commented Dec 21, 2019

LGTM, though I guess publishing the report counts as public disclosure of the double-volume issue.

Approved with PullApprove

@caniszczyk
Copy link
Contributor

@cyphar is there a fix in flight for the issue yet?

@cyphar
Copy link
Member

cyphar commented Dec 21, 2019

@caniszczyk I'm still working on it. I'll try to arrange a hotfix, but as mentioned on the mailing list the only real solution is to wholesale port everything to libpathrs (which is going to take a while).

EDIT: Also this report has a slight inaccuracy when talking about the double-volume bug -- there are actually two ways to mount over a file descriptor (you just mount over /proc/self/fd/... or you can use the new mount API's move_mount which actually has a MOVE_MOUNT_T_EMPTY_PATH flag).

@cyphar
Copy link
Member

cyphar commented Dec 22, 2019

CVE-2019-19921 has been assigned for the issue.

@carnil
Copy link

carnil commented Dec 31, 2019

@cyphar, when will be details on CVE-2019-19921 be public? I see the CVE is as well mentioned in https://lore.kernel.org/stable/[email protected]/

Edit: nevermind, I see the Security-Audit.pdf contains the report and is accessible.

@cyphar
Copy link
Member

cyphar commented Dec 31, 2019

@carnil I plan to (re)publish the details on the relevant security mailing lists after I have a PR open for the issue (there won't be an embargo because it was publicly disclosed by this PR).

@leoluk
Copy link

leoluk commented Jan 1, 2020

Here's the original report: #2197

@estesp
Copy link
Contributor

estesp commented Jan 28, 2020

needs a rebase

@caniszczyk
Copy link
Contributor

caniszczyk commented Jan 28, 2020

LGTM

Approved with PullApprove

@cyphar cyphar force-pushed the amye-securityaudit branch 2 times, most recently from 86f246a to c0b301a Compare January 28, 2020 17:38
@cyphar
Copy link
Member

cyphar commented Jan 28, 2020

@caniszczyk You merged master, rather than rebasing. Fixed.

LGTM.

Approved with PullApprove

Amye Scavarda Perrin added 2 commits January 31, 2020 10:59
Signed-off-by: Amye Scavarda Perrin <[email protected]>
Signed-off-by: Amye Scavarda Perrin <[email protected]>
Signed-off-by: Aleksa Sarai <[email protected]>
@cyphar
Copy link
Member

cyphar commented Jan 31, 2020

Travis failure was because @amye's Signed-off-by lines were lowercase (pushed fixed commits). To be honest, I think we should disable git-validation because this is getting to be a pain in the ass. Who cares if the Signed-off-by is not capitalised!?

@cyphar
Copy link
Member

cyphar commented Jan 31, 2020

LGTM.

@amye
Copy link
Author

amye commented Jan 31, 2020

Removing redundant checks++

@cyphar
Copy link
Member

cyphar commented Jan 31, 2020

And now PullApprove is broken. 😡 Screw it, I'm just going to merge once Travis succeeds and count @caniszczyk's LGTM as applying to the newest commits.

@cyphar cyphar closed this in ff107ee Jan 31, 2020
@cyphar cyphar merged commit 7d23d1e into opencontainers:master Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants