Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Dec 31, 2017

... for #269 ... it's horrible but compiles and seems to work ... needs some more testing ...

@codecov-io
Copy link

codecov-io commented Dec 31, 2017

Codecov Report

Merging #271 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #271   +/-   ##
=======================================
  Coverage   90.53%   90.53%           
=======================================
  Files           1        1           
  Lines         507      507           
=======================================
  Hits          459      459           
  Misses         48       48

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 807addf...cab4a58. Read the comment docs.

mptre added a commit that referenced this pull request Jan 1, 2018
@mptre
Copy link
Owner

mptre commented Jan 1, 2018

Thanks a great start @DBOTW! See #273, I wonder if we instead of
mimicking pledge on different OSs should come up with a different
abstraction and reason about the privilegies related to the state of
pick. Would really appreciated your thoughts.

/cc @DBOTW @mike-burns @calleerlandsson

@mike-burns
Copy link
Collaborator

Oh wow this is cool as heck! And I do agree that #273 is probably the right direction for this project but I wonder whether something like this pledge implementation belongs in libbsd.

Also it was illuminating to me to see pledge defined in terms of seccomp.

I'm not qualified to have detected non-obvious bugs in this code.

@ghost
Copy link
Author

ghost commented Jan 2, 2018

Thanks @mptre and @mike-burns, but this was just a quick hack based on
https://github.com/aggsol/linux-pledge/ and modified for the needs of pick ...

strace was used to determine the syscalls that terminate the execution of
pick and these were added to the "rules" (marked with /* Necessary */).

This is probably not the right approach when it comes to "security": by adding
more exceptions to a packet filter, after a certain time it's useless anyway ...

Even worse: I'm using seccomp for the first time, so chances are high that the
added exceptions "undermine" the whole "security" (RULE(prctl);) ...

So I'm concerned about the patch (that said I'm also not qualified for this) ...

@ghost
Copy link
Author

ghost commented Jan 2, 2018

OK, not that bad: RULE(prctl); is necessary because pledge is called a second time (of course without prctl no rules can be defined) ...

I have removed rpath wpath cpath from the first call and it still works here, so with #273 there is a better way to "fine tune" privilegies depending on the OS ...

The new patch contains only the minimum of necessary rules to run pick ...

Patch for the new sandbox-layer is in progress ...

mptre added a commit that referenced this pull request Jan 2, 2018
@ghost
Copy link
Author

ghost commented Jan 2, 2018

Closing this in favor of #274 ...

@ghost ghost closed this Jan 2, 2018
@ghost ghost deleted the pledge branch January 2, 2018 18:57
This pull request was closed.
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.

3 participants