| Summary: | PermitUserEnvironment accepting pattern of allowed userenv variables | ||
|---|---|---|---|
| Product: | Portable OpenSSH | Reporter: | Daniel Allen <dada.da+mindrot> |
| Component: | sshd | Assignee: | Damien Miller <djm> |
| Status: | CLOSED FIXED | ||
| Severity: | enhancement | CC: | dada.da+mindrot, djm, dtucker |
| Priority: | P2 | ||
| Version: | 5.5p1 | ||
| Hardware: | All | ||
| OS: | All | ||
| Bug Depends on: | |||
| Bug Blocks: | 2852 | ||
| Attachments: | |||
Sorry to be a pest, but could you please resubmit your patch in unified diff format ("diff -u"). You only need submit the 5.5p1 version.
Thanks,
Damien
Created attachment 1903 [details]
patch replacement: diff -u
(In reply to comment #1) > unified diff format ("diff -u"). You only need submit the 5.5p1 version. Oops, my bad! Resubmitted. -Daniel Retarget unclosed bugs from 5.7=>5.8 Created attachment 2017 [details]
userenv patch for 5.8p1
redid patch for openssh-5.8p1
Retarget unresolved bugs/features to 6.0 release Retarget unresolved bugs/features to 6.0 release Retarget unresolved bugs/features to 6.0 release (try again - bugzilla's "change several" isn't) Created attachment 2098 [details]
patch for PermitUserEnvironment against 5.9p1
Keeping up with new version numbers.
The patch looks okay, but I'm a little reticent to add a method to control environment variables that doesn't look like any of the other ACL mechanisms that we use. Perhaps this should use match_pattern_list() (match.[ch]) to test environment variables when PermitUserEnvironment!=yes instead of a simple substring? (In reply to comment #10) > The patch looks okay, but I'm a little reticent to add a method to > control environment variables that doesn't look like any of the other > ACL mechanisms that we use. Perhaps this should use > match_pattern_list() (match.[ch]) to test environment variables when > PermitUserEnvironment!=yes instead of a simple substring? Oh! match_pattern_list() sounds like a much more robust solution. I'll see if I can code something up soon. I have two big deadlines in the next two weeks but I hope to have something to review soon. Thanks, Daniel Created attachment 2113 [details]
rewrite of patch to use match_pattern_list
New patch allows pattern lists for permitted user environment variables, including wildcards and negation.
New format to match exactly one variable:
PermitUserEnvironment "REMOTEUSER=*"
To match any variables starting with LOG and XTERM variables with values matching vt*:
PermitUserEnvironment "LOGNAME=*,XTERM=vt*"
(In reply to comment #12) > To match any variables starting with LOG and XTERM variables with > values matching vt*: the last line of the example should read: PermitUserEnvironment "LOG*,XTERM=vt*" instead of: > PermitUserEnvironment "LOGNAME=*,XTERM=vt*" Hi Damien, don't suppose you've had time to look at this patch yet? It's working well for our campus, and I'd love to see this making it into v6.0. Thanks, -Daniel Retarget from 6.0 to 6.1 Retarget 6.0 => 6.1 Retarget uncompleted bugs from 6.1 => 6.2 Retarget bugs from 6.1 => 6.2 retarget to openssh-6.3 Retarget to openssh-6.4 Retarget 6.3 -> 6.4 Retarget incomplete bugs / feature requests to 6.6 release Retarget incomplete bugs / feature requests to 6.6 release Retarget to 6.7 release, since 6.6 was mostly bugfixing. Remove from 6.6 tracking bug Retarget incomplete bugs to 6.8 release. These bugs are no longer targeted at the imminent 6.7 release OpenSSH 6.8 is approaching release and closed for major work. Retarget these bugs for the next release. Retarget to 6.9 Retarget pending bugs to openssh-7.1 Retarget to openssh-7.3 Retarget to openssh-7.3 retarget unfinished bugs to next release retarget unfinished bugs to next release retarget unfinished bugs to next release retarget unfinished bugs to next release OpenSSH 7.4 release is closing; punt the bugs to 7.5 Move incomplete bugs to openssh-7.6 target since 7.5 shipped a while back. To calibrate expectations, there's little chance all of these are going to make 7.6. remove 7.5 target Created attachment 3012 [details]
redo
This redoes the patch from scratch; I think this is considerably simpler.
It supports PermitUserEnvironment=(yes|no|pattern-list)
Created attachment 3013 [details]
include documentation, make whitelist apply to key/cert authorized_keys options
Move to OpenSSH 7.8 tracking bug This has been committed and will be in the next release. commit 95344c257412b51199ead18d54eaed5bafb75617 (HEAD -> master, origin/master, origin/HEAD) Author: djm@openbsd.org <djm@openbsd.org> Date: Tue Jul 3 10:59:35 2018 +0000 upstream: allow sshd_config PermitUserEnvironment to accept a pattern-list of whitelisted environment variable names in addition to yes|no. bz#1800, feedback and ok markus@ OpenBSD-Commit-ID: 77dc2b468e0bf04b53f333434ba257008a1fdf24 closing resolved bugs as of 8.6p1 release |
Created attachment 1901 [details] diff for patching 5.5p1 and 5.4p1 "PermitUserEnvironment=Yes" security risks could be mitigated by allowing sshd to allow selected user-environment variables. I have written a patch which allows sshd configuration to specify: "PermitUserEnvironment=VAR" This passes user environment variables (from $USER/.ssh/environment and/or $USER/.ssh/authorized_keys) starting with VAR, ignoring all other environment variables not previously copied by sshd. The default option for PermitUserEnvironment is unchanged; it still defaults to "No". As a second effect, if PermitUserEnvironment is set to the default "No", but an "environment=" option is specified in authorized_keys, the key is no longer rejected with a "Bad options in file" error, but instead silently ignores the "environment=" option, which is similar to the behaviour of other options such as "permitopen=".