cmd/snap-seccomp,osutil: make user/group lookup functions public #3966

Merged
merged 3 commits into from Oct 2, 2017

Conversation

Projects
None yet
4 participants
Contributor

zyga commented Sep 26, 2017

This branch moves the Find{Uid,Gid} functions out of the cmd/snap-seccomp
program and into the osutil package where they can also be used by other parts
of snapd. Sadly golang 1.6 doesn't implement user.LookupGroup yet so we need
this kind of hackery.

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

zyga added some commits Sep 26, 2017

osutil: add code for FindUid/FindGid from snap-seccomp
This patch adds an easily accessible pair of functions for performing
user and group lookups. Group lookups are not in golang 1.6 so we cherry
picked part of the future implementation but now we need to use them
from more than once place, hence the new entry in the osutil package.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
cmd/snap-seccomp: switch to osutil for group lookup
This just completes the previous commit.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

codecov-io commented Sep 26, 2017

Codecov Report

Merging #3966 into master will decrease coverage by 0.15%.
The diff coverage is 11.62%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #3966      +/-   ##
=========================================
- Coverage   75.96%   75.8%   -0.16%     
=========================================
  Files         423     424       +1     
  Lines       36525   36577      +52     
=========================================
- Hits        27748   27729      -19     
- Misses       6836    6904      +68     
- Partials     1941    1944       +3
Impacted Files Coverage Δ
osutil/group.go 0% <0%> (ø)
cmd/snap-seccomp/main.go 54.4% <100%> (-2.48%) ⬇️
cmd/snap-repair/cmd_run.go 53.7% <0%> (-28.12%) ⬇️
cmd/snap-repair/main.go 44.44% <0%> (-5.56%) ⬇️
daemon/api.go 72.34% <0%> (-0.07%) ⬇️
interfaces/builtin/opengl.go 100% <0%> (ø) ⬆️
dirs/dirs.go 98.11% <0%> (+0.01%) ⬆️
cmd/snap-repair/runner.go 82.67% <0%> (+0.07%) ⬆️
overlord/snapstate/snapstate.go 80.29% <0%> (+0.19%) ⬆️
... and 1 more

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 41a23c8...26dff79. Read the comment docs.

-//
-// Use implementation from upcoming releases:
-// https://golang.org/src/os/user/lookup_unix.go
-func lookupGroup(groupname string) (string, error) {
@chipaca

chipaca Sep 26, 2017

Member

why are we doing it this way instead of using go's LookupGroup? it's a lot simpler fwiw

@zyga

zyga Sep 26, 2017

Contributor

Because it is not available in golang 1.6

@chipaca

chipaca Sep 26, 2017

Member

I understand that, but we can copy the LookupGroup function from go 1.9 or whatever, as we've done before for similar things

@chipaca

chipaca Sep 26, 2017

Member

ah. I looked at lookup_unix, but this is cgo_lookup_unix. Heh. Sorry.

@zyga

zyga Sep 26, 2017

Contributor

This is exactly that. I just took the existing copy @jdstrand added to snap-confine and made it public.

@zyga zyga requested a review from mvo5 Sep 26, 2017

Contributor

zyga commented Sep 26, 2017

@mvo5 please consider merging this soon as it will help me out with other PRs

osutil/group.go
+
+// FindUid returns the identifier of the given UNIX user name.
+func FindUid(username string) (uint64, error) {
+ if !userGroupNamePattern.MatchString(username) {
@mvo5

mvo5 Sep 28, 2017

Collaborator

I think now that this is a public method the check for userGroupNamePattern is misleading. This is about finding the user on the system and not longer about the usernames/groups specified in the policy. I think we need to move the check itself back into snap-seccomp but we need to remove it here.

@zyga

zyga Sep 28, 2017

Contributor

Thanks, I'll make that happen.

@zyga

zyga Sep 29, 2017

Contributor

Done

cmd/snap-seccomp,osutil: move user/group name enforcement
This patch moves the regular expression for checking valid user and
group names, as well as the assorted code back to the snap-seccomp
command. The lookup functions are generic and the extra validation is
specific to the snap-seccomp command.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

mvo5 approved these changes Sep 30, 2017

@zyga zyga merged commit b64270a into snapcore:master Oct 2, 2017

7 checks passed

artful-amd64 autopkgtest finished (success)
Details
artful-i386 autopkgtest finished (success)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-amd64 autopkgtest finished (success)
Details
xenial-i386 autopkgtest finished (success)
Details
xenial-ppc64el autopkgtest finished (success)
Details
zesty-amd64 autopkgtest finished (success)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment