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

[docker] don't set m3dbnode binary caps in build #1672

Merged
merged 3 commits into from
May 29, 2019

Conversation

schallert
Copy link
Collaborator

What this PR does / why we need it:

Fixes #1671

Special notes for your reviewer:

Does this PR introduce a user-facing and/or backwards incompatible change?:

NONE

Does this PR require updating code package or user-facing documentation?:

NONE

@codecov-io
Copy link

codecov-io commented May 29, 2019

Codecov Report

Merging #1672 into master will increase coverage by 39.6%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1672      +/-   ##
=========================================
+ Coverage    32.2%   71.9%   +39.6%     
=========================================
  Files           4     974     +970     
  Lines         341   81215   +80874     
=========================================
+ Hits          110   58401   +58291     
- Misses        221   18980   +18759     
- Partials       10    3834    +3824
Flag Coverage Δ
#aggregator 82.4% <ø> (?)
#cluster 85.7% <ø> (?)
#collector 63.9% <ø> (+31.6%) ⬆️
#dbnode 80% <ø> (?)
#m3em 73.2% <ø> (?)
#m3ninx 74% <ø> (?)
#m3nsch 51.1% <ø> (?)
#metrics 17.6% <ø> (?)
#msg 74.7% <ø> (?)
#query 66.3% <ø> (?)
#x 85.4% <ø> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c4fd5a...5f83473. Read the comment docs.

@schallert schallert requested a review from robskillington May 29, 2019 02:30
Copy link
Contributor

@richardartoul richardartoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@schallert was this what was triggering the permissions error? Its definitely not the syscall itself that M3DB is making now? Wonder if we should also set PROCESS_LIMITS_RAISE to false in the default image as well although not sure if that had any effect.

Also how did you encounter this? I ran into it a few hours ago with location store's C.I

@schallert
Copy link
Collaborator Author

schallert commented May 29, 2019

@richardartoul my understanding is that Linux won't even let you exec a file if it has capabilities on it that you don't have (not sure which set this considers, "bounding" or another), way before you even get to making the syscall.

For example, removing Docker from the equation, since my current bash shell has cap_sys_resource in its bounding set I can exec m3db:

# check my current caps
$ /sbin/capsh --print
Current: =
Bounding set =cap_chown,cap_dac_override,cap_dac_read_search,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_linux_immutable,cap_net_bind_service,cap_net_broadcast,cap_net_admin,cap_net_raw,cap_ipc_lock,cap_ipc_owner,cap_sys_module,cap_sys_rawio,cap_sys_chroot,cap_sys_ptrace,cap_sys_pacct,cap_sys_admin,cap_sys_boot,cap_sys_nice,cap_sys_resource,cap_sys_time,cap_sys_tty_config,cap_mknod,cap_lease,cap_audit_write,cap_audit_control,cap_setfcap,cap_mac_override,cap_mac_admin,cap_syslog,cap_wake_alarm,cap_block_suspend,cap_audit_read

# exec m3db
$ ./m3dbnode 2>&1 | head -n1
2019/05/29 04:11:50 Go Runtime version: go1.10.8

However if I use capsh to drop that capability, I'll get the same error (I have to be root to drop a cap):

$ capsh --drop=cap_sys_resource --user=mschalle -- -c '/home/mschalle/m3dbnode'
/bin/bash: /home/mschalle/m3dbnode: Operation not permitted

# un-setcap
sudo setcap cap_sys_resource-ep m3dbnode

# all good
$ capsh --drop=cap_sys_resource --user=mschalle -- -c '/home/mschalle/m3dbnode' 2>&1 | head -n1
2019/05/29 04:19:36 Go Runtime version: go1.10.8

According to Docker's docs, SYS_RESOURCE isn't included by default and must be explicitly added: https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities

I happened to encounter this while doing some testing with the new setcap image and a Kubernetes cluster with locked down privs lol

@schallert schallert force-pushed the schallert/docker_no_setcap branch from fd72f89 to ca0072f Compare May 29, 2019 04:17
@schallert schallert force-pushed the schallert/docker_no_setcap branch from ca0072f to 5f83473 Compare May 29, 2019 04:47
@schallert schallert merged commit c982091 into master May 29, 2019
@schallert schallert deleted the schallert/docker_no_setcap branch May 29, 2019 05:01
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.

Setcap in docker build causes un-runnable container with docker defaults
3 participants