Repository navigation
fix(packaging): start every sudoers grant's argument regex at the argument text - #480
Merged
Merged
Conversation
…ument text sudo reads a rule's arguments as a regular expression only when the whole argument text starts with `^`; otherwise it is a wildcard pattern in which `^`, `+` and `$` are literal. Every grant built with a regex put fixed tokens before it (`/usr/bin/date -s ^@[0-9]+$`), so none could match, and each needed a password wherever the service user had no blanket sudo: - system.set_clock, so the automatic time sync never set the clock - the two usermod group joins - camera.config_write, camera.overlay_load, camera.overlay_unload - media.write Each rule is now built by `_regex_args`, which puts the anchor in front of the whole argument text (`/usr/bin/date ^-s @[0-9]+$`). Inside a regex the group list's and camera token's commas need no sudoers escape, so the escaped constants go. The probes already send a token each regex accepts. Checked with sudo 1.9.16p2 on a Pi, for a user holding only these rules: the old rules refused all seven valid calls; the new ones allow them and refuse every malformed one (a non-numeric or suffixed epoch, an extra argument, an injected `-aG sudo`, a widened group list, a non-USB device). Tests pin, for every regex rule: the argument text starts with `^`, ends with `$` and holds no other `^`; the words before the regex are plain; the rule admits its own probe; and the hardware cases. The playbook's drop-in is mirrored to keep its sync test. Fixes #256 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The camera setup grant test still expected the escaped, regex-after-the-verb form, so the suite failed. It now pins each rendered line whole, ending at the newline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sudo runs regexec over the space-joined arguments, which anchors only where the rule says so. The tests matched with re.fullmatch, which anchors both ends itself, so a rule that lost its closing `$` (and so admitted trailing arguments) still passed them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The words in front of a rule's regex bind to its first alternative only if the alternation is grouped: a top-level `|` would render `^write a|b$`, which sudo reads as "starts with write a" or "ends with b". The in-process argv check matches the bare token and would not notice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
Review: four roundsAgent review of the whole change, fixes committed each round. Every new or changed test was mutation-checked and fails against the old form or the broken variant. Round 1 (
Round 2: nothing beyond cosmetic. Checked:
Round 3 (independent reviewer, Round 4: nothing beyond cosmetic or theoretical. Left as is:
Results:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #256.
sudo reads a rule's arguments as a regular expression only when the whole argument text starts with
^. Otherwise it is a wildcard pattern in which^,+and$are literal. Every grant built with a regex put fixed tokens in front of it (/usr/bin/date -s ^@[0-9]+$), so none could ever match. Wherever the service user has no blanket sudo, each of these needed a password:system.set_clock: the automatic time sync never set the clock;usermodgroup joins (journal access, hardware groups);camera.config_write,camera.overlay_load,camera.overlay_unload;media.write.The fix:
_regex_args, which puts the anchor in front of the whole argument text:/usr/bin/date ^-s @[0-9]+$,…write-to-media ^write /dev/sd[a-z]+[0-9]*$.Why it went unnoticed: the OpenFollow image and the test stations give the service user blanket passwordless sudo.
Test plan
-aG sudo, a widened group list,automaticfor a load, a non-USB device.visudo -cfon the Pi.^, ends with$and holds no other^;re.searchas sudo'sregexecruns.regcomp/regexecagrees with that model on every rule's probe and on 29 edge strings.make ci-remoteon the testing Pi..debinstall without blanket sudo: the Device permissions view shows these grants as passwordless, and the time sync sets the clock..deb) with the blanket rule removed, timesyncd stopped and the clock set 120 s back. Control with the packaged drop-in: Device permissions listed these 7 grants asneeds_passwordand the time sync reported "answered, but the clock was NOT set (drift 120.1s; needs the system.set_clock grant)". With the drop-in rendered from this branch (visudo -cfOK): 31 passwordless, 0 needs password, and the time sync reported "clock set (drift was 119.8s)".🤖 Generated with Claude Code