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

capng_apply: apply CAPNG_SELECT_CAPS before CAPNG_SELECT_BOUNDS #23

Closed

Conversation

diabonas
Copy link

When using capng_apply(CAPNG_SELECT_BOTH), libcap-ng before version 0.8.1 silently skipped applying bounding capabilities if CAP_SETPCAP was not given and continued to apply CAPNG_SELECT_CAPS.

libcap-ng 0.8.1 changed this behaviour in commits 6a24a9c and 2ab6a03 to return with an error when trying to set bounding capabilities without having CAP_SETPCAP. While this change generally is useful to be able to detect more errors, it means that capng_apply(CAPNG_SELECT_BOTH) now does not even apply CAPNG_SELECT_CAPS any more since the function returns early after failing to apply the bounding capabilities.

This change in behaviour might be surprising for legacy code that simply copied capng_apply(CAPNG_SELECT_BOTH) from the example code in the README and does not check the return value to see whether something went wrong. In this case, no capabilities are applied at all any more, which is a breaking change.

Applying CAPNG_SELECT_CAPS before CAPNG_SELECT_BOUNDS means that at least the CAPNG_SELECT_CAPS are applied, while still failing with an error of -4 when CAPNG_SELECT_BOTH is used without CAP_SETPCAP.

I hope this strikes a balance between the need for better error checking and backwards compatibility with legacy code that does not perform any error checking on capng_apply, thus silently failing to apply any capabilities at all.

When using capng_apply(CAPNG_SELECT_BOTH), libcap-ng before version 0.8.1
silently skipped applying bounding capabilities if CAP_SETPCAP was not given
and continued to apply CAPNG_SELECT_CAPS.

libcap-ng 0.8.1 changed this behaviour in commits
6a24a9c and
2ab6a03 to return with an error when trying to
set bounding capabilities without having CAP_SETPCAP. While this change
generally is useful to be able to detect more errors, it means that
capng_apply(CAPNG_SELECT_BOTH) now does not even apply CAPNG_SELECT_CAPS any
more since the function returns early after failing to apply the bounding
capabilities.

This change in behaviour might be surprising for legacy code that simply copied
capng_apply(CAPNG_SELECT_BOTH) from the example code in the README and does not
check the return value to see whether something went wrong. In this case, no
capabilities are applied at all any more, which is a breaking change.

Applying CAPNG_SELECT_CAPS before CAPNG_SELECT_BOUNDS means that at least the
CAPNG_SELECT_CAPS are applied, while still failing with an error of -4 when
CAPNG_SELECT_BOTH is used without CAP_SETPCAP.

I hope this strikes a balance between the need for better error checking and
backwards compatibility with legacy code that does not perform any error
checking on capng_apply, thus silently failing to apply any capabilities at
all.
@stevegrubb
Copy link
Owner

The big issue with this approach is that if capabilities are applied and drops CAP_SETPCAP, now you can't change the bounding set. So, it needs to come first. Perhaps we can continue and apply what we can. I'll look more at this.

@diabonas
Copy link
Author

The big issue with this approach is that if capabilities are applied and drops CAP_SETPCAP, now you can't change the bounding set. So, it needs to come first.

I see, I am going to close this PR then.

Perhaps we can continue and apply what we can. I'll look more at this.

That would be great, since as of now, a lot of software just silently fails to drop capabilities since it does not have CAP_SETPCAP and uses CAPNG_SELECT_BOUNDS without error checking.

@diabonas diabonas closed this Nov 20, 2020
@diabonas diabonas deleted the apply-caps-before-bounds branch November 20, 2020 21:14
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.

2 participants