Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Bool file sanitization #324
Conversation
|
/me goes to fix daemon code tests to use allowed paths. |
zyga
added some commits
Jan 14, 2016
|
Dear mo, retest this please. |
|
How relevant to what we're building is the fact that $ tail /sys/class/leds/dell::kbd_backlight/*brightness
==> /sys/class/leds/dell::kbd_backlight/brightness <==
3
==> /sys/class/leds/dell::kbd_backlight/max_brightness <==
10 |
niemeyer
reviewed
Jan 18, 2016
| @@ -53,6 +54,13 @@ func (t *BoolFileType) Name() string { | ||
| return "bool-file" | ||
| } | ||
| +var boolFileAllowedPathPatterns = []*regexp.Regexp{ | ||
| + // The brightness of standard LED class device | ||
| + regexp.MustCompile("^/sys/class/leds/[^/]+/brightness$"), |
zyga
Jan 19, 2016
Contributor
I'd like to keep it for two reasons:
- it is useful for common keyboard LEDs (caps lock, scroll lock, etc) and for testing on x86
- vast majority of LEDs have only two states 0 and !0 (no PWM support)
On a given dell laptop, the gadget snap can suppress that LED capability if it doesn't behave correctly. In every other case LEDs are a valid candidate and thus should be allowed.
niemeyer
Jan 20, 2016
Contributor
Please just drop it. This is a very simple test capability that we can improve at will later. This branch could already be merged by now.
niemeyer
Jan 20, 2016
Contributor
Sorry, I take that back. Yes, these cases indeed work with 0 or 1, so we can instead blacklist whatever we don't want later.
niemeyer
reviewed
Jan 18, 2016
| + valid := false | ||
| + for _, pattern := range boolFileAllowedPathPatterns { | ||
| + if pattern.MatchString(path) { | ||
| + valid = true |
|
LGTM considering these points.. I'd drop brightness altogether for now. |
added a commit
that referenced
this pull request
Jan 20, 2016
niemeyer
merged commit 634c221
into
snapcore:master
Jan 20, 2016
|
And it's merged by now! ;) |
zyga commentedJan 14, 2016
Note: this pull request depends on #322
To enhance the security of the bool-file capability it must not be allowed to create an arbitrary bool-file (e.g. one pointing to /dev/sda or /dev/mem). This is achieved with simple validation against a set of regular expressions describing allowed values.
Currently two kinds of bool file can be created. They can point to:
Additional values can be added as we discover their utility.