The contents of env_file are not intended to be subject to env_keep.  Basically, env_file is applied last as long as there are no values that would override an existing environment variable.  Since the env_file contents are controlled by the admin, not the user, they are considered to be safe.

(In reply to Todd C Miller from comment #1)
> The contents of env_file are not intended to be subject to env_keep.
> Basically, env_file is applied last as long as there are no values
> that would override an existing environment variable.  Since the
> env_file contents are controlled by the admin, not the user, they
> are considered to be safe.

Won't env_file accept any absolute file system path?  We can't even assume its to a text file let alone a protected one, correct?  The admin can control where in the file system it points to but maybe not have control over the file itself.  Maybe for business reasons they can't control who has access to it or who updates it.  That is my business case.  Non-admins (developers) need to be able to make legitimate changes to path for legitimate reasons but some parts of the path must conform security requirements and changing those things should cause a failure to introduce a non-conforming environment variable.  Having a security filter over the entire environment, not just what is inherited by the invoking user is important.  If a hacker finds their way into the env_file, its a backdoor to the security provided by the "env_*" filters in sudo.

I think what we can assume is that the sudoers file IS protected, so the filters I set in it cannot be altered.  Making env_file subject to env_keep provides the ability to remove a potential loophole in the configuration of a "safe environment" for sudo to operate.


Even if there is some other reason to disagree with my argument, I would still like to report a bug for "env_keep" not filtering on equality matching.
  
Using the same scenario as I posed in the original bug report ...

Manual States:
By default, environment variables are matched by name. However, if the pattern includes an equal sign (‘=’), both the variables name and value must match.

Actual Results:
In my sudoers file I have the following:
Defaults env_reset
Cmnd_Alias PGM=/my/pgm
#  Defaults!PGM env_file=/my/myenv  -- I'VE STUBBED THIS OUT
Defaults!PGM env_keep +="MYPATH"

PGM alias "/my/pgm" just spits out all the set environment variables.
When I run this:

export MYPATH=/mydir
sudo /usr/pgm | grep MYPATH  -- I get output: "MYPATH=/mydir"
This is good behavior.

But, now if I change the env_keep value in the sudoers file to:
Defaults!PGM env_keep +="MYPATH=/mydir"

and then re-run the export and sudo:
export MYPATH=/mydir
echo $MYPATH
/mydir
sudo /usr/pgm | grep MYPATH  -- I get no output
This bad behavior does not conform to the manual

Is there a workaround?  Can I get the tool to equality match on an environment variable in some other way?  Or is this just a bug?

BTW: If you want to try exact scenario yourself, here's the source to "pgm.c":

#include <unistd.h>
#include <stdio.h>

extern char **environ;

int main()
{
    int i = 0;
    while (environ[i])
    {
        printf("%s\n", environ[i++]);
    }
    return 0;
}

Changing the handling of env_file is likely to break other users of it.  I understand your use case where the file is untrusted input but I believe it would be better to have a separate setting for this, possible user_env_file or untrusted_env_file.

As for environment variable matching when the value is present, I'm unable to reproduce your problem.  You have:

Cmnd_Alias PGM=/my/pgm
Defaults!PGM env_file=/my/myenv

However, it appears you ran:

$ sudo /usr/pgm | grep MYPATH

Note that PGM is defined to /my/pgm, not /usr/pgm.  Perhaps this is the source of your issue.

/usr/pgm was a typo in the original test case and I accidentally cut and pasted it into both new and old test case.  Sorry.  Correcting to make proper below.  You should be able to repro.  My experience is that if I add anything besides the left side env. var. name "MYPATH" I can't get env_keep to filter in the env. var.  If I even put "MYPATH=", the MYPATH env. var. gets filtered out no matter what the value is or how well it matches the value in env_keep.  Is this not your experience?
________________________________________________________
In my sudoers file I have the following:
Defaults env_reset
Cmnd_Alias PGM=/my/pgm
#  Defaults!PGM env_file=/my/myenv  -- I'VE STUBBED THIS OUT
Defaults!PGM env_keep +="MYPATH"

PGM alias "/my/pgm" just spits out all the set environment variables.
When I run this:

export MYPATH=/mydir
sudo /my/pgm | grep MYPATH  -- I get output: "MYPATH=/mydir"
This is good behavior.

But, now if I change the env_keep value in the sudoers file to:
Defaults!PGM env_keep +="MYPATH=/mydir"

and then re-run the export and sudo:
export MYPATH=/mydir
echo $MYPATH
/mydir
sudo /my/pgm | grep MYPATH  -- I get no output
This bad behavior does not conform to the manual

NOTE: If I change 
Defaults!PGM env_keep +="MYPATH=/mydir"
to 
Defaults!PGM env_check +="MYPATH=/mydir"
in the sudo file, it still doesn't find a match with the MYPATH env. variable.  Both env_keep and env_check appear to perform the same task when "env_reset" is on.  I'm not sure if there is any difference between the two when env_reset is on?

I observed in the documentation for "env_check" (that I suspect is supposed to be the same for "env_keep" but isn't documented that way), the following statement: 

      For all variables except TZ, "safe" means that the variables's
      value does not contain any '%' or '/' characters.  

I went back to try to see if the forward slash in MYPATH was the cause of the problem I am seeing.  But, tests confirm it is not.  As follows:

#slash removed from "mydir" value just so I can test of sudoers
Defaults!PGM env_keep +="MYPATH=mydir"

and then re-run the export and sudo:
export MYPATH=mydir
echo $MYPATH
/mydir
sudo /my/pgm | grep MYPATH  -- I get no output!!!

This is unexpected behavior and it doesn't change when I change the default from "env_keep" to "env_check" either:
Defaults!PGM env_keep +="MYPATH=mydir"

and then re-run the export and sudo:
export MYPATH=mydir
echo $MYPATH
/mydir
sudo /my/pgm | grep MYPATH  -- I still get no output!!!
_______________________________________________________

I don't disagree with you that splitting up "env_file" into to files makes a lot of sense.  I'd suggest documenting "env_file" as untrusted, then creating two new Defaults:

1. "trusted_env_file" - This could work just like "env_file" does today only it could make some checks on the ownership and/or privileges of the file to ensure it can be trusted (just like the sudoers file is tested before it is trusted.  
2. "untrusted_env_file" - This would not be checked for permissions but could go through the same filters as other env. vars. coming from the invoking user so that if a match fails the untrusted_env_file becomes "trusted" in the sense that it doesn't violate "env_*" rules set by the sudoers file.  

NOTE1: I don't think "trusted_env_file" or "untrusted_env_file" should be checking for '%' or '/' characters like "env_file" claims it does.  What's the point?  How does that make the variable "safe".  In my case, if it worked as documented, it would just be a hindrance.  Why can't an env. var. have a path separator?

NOTE2: I don't actually know (yet) what sudo does with non-conforming env. variables since I can't get this to work.  Is it just supposed to not add the non-conforming env. var. to sudo execution?  Or is it supposed to fail sudo because of the non-conformance?

It works as expected for me with the environment you describe:

myuser@linux-build:~$ sudo -l
Matching Defaults entries for myuser on linux-build.courtesan.com:
    env_reset

Runas and Command-specific defaults for myuser:
    Defaults!/my/pgm env_keep+=MYPATH=/mydir

User myuser may run the following commands on linux-build.courtesan.com:
    (root) /my/pgm

myuser@linux-build:~$ export MYPATH=/mydir
myuser@linux-build:~$ sudo /my/pgm |grep MYPATH
MYPATH=/mydir

The env_keep setting matches the full path as expected.  This is on Ubuntu 16.04.1 LTS with Sudo version 1.8.16.

As for '%' and '/' in environment variables, that is to avoid paths and printf-style format strings.  Those restrictions are only in place for the specific environment variables listed in the env_check list, not env_keep.  For example, you may not wish to allow a program to open arbitrary files simply by setting TZ or TERM to a path.

Sudo will simply omit non-conforming environment variables unless they were explicitly listed on the command line, in which case sudo will display a warning and not run the command.

With my version:
root@vm-baseosu104x64:/# sudo -V
Sudo version 1.8.9p5

These are my results:
root@vm-baseosu104x64:/# sudo -l
Matching Defaults entries for root on vm-baseosu104x64:
    env_reset

Runas and Command-specific defaults for root:
    Defaults!/my/pgm env_keep+=MYPATH=/mydir

User root may run the following commands on vm-baseosu104x64:
    (ALL : ALL) ALL
root@vm-baseosu104x64:/# export MYPATH=/mydir
root@vm-baseosu104x64:/# echo $MYPATH
/mydir
root@vm-baseosu104x64:/# sudo /my/pgm | grep MYPATH

No results from running "pgm".

I tried to upgrade but I'm told the version I have is the latest version:

root@vm-baseosu104x64:/# apt-get install sudo
Reading package lists... Done
Building dependency tree
Reading state information... Done
sudo is already the newest version.

The only thing I can conclude is that something between version 1.8.9p5 and 1.8.19p1 changed and resolved this problem.
  
I downloaded the sudo-1.8.19p1.tar.gz file and expanded it into a local directory but not sure what to do next.  Do you know what I need to run to build a local version of sudo from the install file so I can see if this corrects the problem?

The simplest thing would be for you to download the pre-built Ubuntu 14.04 package and install it with dpkg.  https://www.sudo.ws/download.html#binary

If you'd like to build your own from source, you should just be able to run the "mkpkg" script in the source tree, but doing so will require a number of development packages which you may or may not have installed.

(In reply to Todd C Miller from comment #7)
> The simplest thing would be for you to download the pre-built Ubuntu
> 14.04 package and install it with dpkg. 
> https://www.sudo.ws/download.html#binary
> 
> If you'd like to build your own from source, you should just be able
> to run the "mkpkg" script in the source tree, but doing so will
> require a number of development packages which you may or may not
> have installed.

Thanks for the advice.  I pulled the pre-built and installed w/dpkg and can confirm that I have the latest package:

root@vm-baseosu104x64:/# sudo -V
Sudo version 1.8.19p1

Unfortunately, it didn't help:
root@vm-baseosu104x64:/# export MYPATH="/mydir"
root@vm-baseosu104x64:/# echo $MYPATH
/mydir
root@vm-baseosu104x64:/# sudo /my/pgm | grep MYPATH
root@vm-baseosu104x64:/#

Rather than show it with "sudo -l", below are the literal entries I have in the sudoers file that don't appear to be working.  Do they differ from yours in any way?

Defaults env_reset
Defaults!/my/pgm env_keep+="MYPATH=mydir"

The only other difference in our sudoers file as pertains to root is that I believe you get this from sudo -l:

User myuser may run the following commands on linux-build.courtesan.com:
    (root) /my/pgm

and I get this:

User root may run the following commands on vm-baseosu104x64:
    (ALL : ALL) ALL

Just in case it made a difference, I added:
root      ALL = (root) /my/pgm 

to my sudoers file but it didn't make any difference.

The default sudoers file in Sudo version 1.8.19p1 has a bunch more stuff in it.  So, here's what I get now from "sudo -l":

root@vm-baseosu104x64:/etc# sudo -l
Matching Defaults entries for root on vm-baseosu104x64:
    env_reset, env_keep+="LANG LANGUAGE LINGUAS LC_* _XKB_CHARSET",
    env_keep+="XAPPLRESDIR XFILESEARCHPATH XUSERFILESEARCHPATH",
    secure_path=/usr/local/sbin\:/usr/local/bin\:/usr/sbin\:/usr/bin\:/sbin\:/bin,
    mail_badpass

Runas and Command-specific defaults for root:
    Defaults!/my/pgm env_keep+=MYPATH=mydir

User root may run the following commands on vm-baseosu104x64:
    (root) /my/pgm
    (ALL) ALL
root@vm-baseosu104x64:/etc#

Regardless, nothing seems to matter as I rerun the test case with all these entries:

root@vm-baseosu104x64:/# echo $MYPATH
/mydir
root@vm-baseosu104x64:/# sudo /my/pgm | grep MYPATH
root@vm-baseosu104x64:/#

I have no idea what to do next.  I cannot get literal environment value matching to work with sudo no matter what I try.  If I strip the value off the environment variable in sudoers:

Defaults!/my/pgm env_keep+="MYPATH"

Then, it works:

root@vm-baseosu104x64:/etc# sudo /my/pgm | grep MYPATH
MYPATH=/mydir

So clearly the env. var is being recognized by sudo and will pass a match based strickly on the variable name rather than the value.
I tried some permutations of env_keep to see if others might worked.  I tried the following and got these results:

root@vm-baseosu104x64:/etc# export MYPATH="/mydir"
root@vm-baseosu104x64:/etc# sudo /my/pgm | grep MYPATH
with:
Defaults!/my/pgm env_keep+="MYPATH" - fails
Defaults!/my/pgm env_keep+="MYPATH=" - fails
Defaults!/my/pgm env_keep+="MYPATH=*" - succeeds
Defaults!/my/pgm env_keep+="MYPATH=/mydir" fails
Defaults!/my/pgm env_keep+="MYPATH=/mydir*" succeeds !!!!!
Defaults!/my/pgm env_keep+="MYPATH=*/mydir*" fails !!

root@vm-baseosu104x64:/etc# export MYPATH="xyz/mydirabc"
root@vm-baseosu104x64:/etc# sudo /my/pgm | grep MYPATH
with:
Defaults!/my/pgm env_keep+="MYPATH=*/mydir*" fails
Defaults!/my/pgm env_keep+="MYPATH=*" succeeds
Defaults!/my/pgm env_keep+="MYPATH=xyz/mydirabc" succeeds !!!!!!!!!!

root@vm-baseosu104x64:/etc# export MYPATH="/mydir/"
root@vm-baseosu104x64:/etc# sudo /my/pgm | grep MYPATH
with:
Defaults!/my/pgm env_keep+="MYPATH=/mydir/" succeeds !!!!!!!!!!!! 

root@vm-baseosu104x64:/etc# export MYPATH="/mydir"
root@vm-baseosu104x64:/etc# sudo /my/pgm | grep MYPATH
with:
Defaults!/my/pgm env_keep+="MYPATH=/mydir" NOW IT SUCCEEDS ???????

I suspect there's an uninitialized variable somewhere whose value got triggered somewhere along the way and affected behavior.

One more question ...

I know the symbols used for character matching are not regular expressions and wildcards but ... is it possible to get env_keep to match something in the middle of a string? like "/mydir" among a larger path string?

So if MYPATH=/dir1/subdir1:/mydir:/dir2:/dir3/subdir2

Can I get env_keep to match MYPATH and "/mydir" in the value?

*** Bug 767 has been marked as a duplicate of this bug. ***

(In reply to Todd C Miller from comment #10)
> *** Bug 767 has been marked as a duplicate of this bug. ***

My apologies for the duplicate bug.  I didn't mean to do that ... thought I was responding to the existing bug. 

-------------------------------------------------------------------

I suspect you'll have to close this bug since you can't reproduce except maybe to install fresh somewhere and hope that the default memory space exhibits the problem.  Very strange although I've seen this type of thing in my own code many, many times.

--------------------------------

As for follow-up, do you think you can answer the question I had at the end of the last bug reply, it was:

One more question ...

I know the symbols used for character matching are not regular expressions and wildcards but ... is it possible to get env_keep to match something in the middle of a string? like ":/mydir:" among a larger path string?

So if MYPATH=/dir1/subdir1:/mydir:/dir2:/dir3/subdir2

Can I get env_keep to match the MYPATH variable and the ":/mydir:" substring inside the environment value?

Thanks for all you do here Todd.  This really is brilliant software.  I only started using it a few weeks ago but I'm really impressed with its ingenuity and power.  I'm sure you hear it all the time but ... great work!

Sudo 1.8.20 has a restricted_env_file setting that can be used when the contents of the file is not trusted.

Currently, the matching of env_keep only supports a '*' at the end of the string.  I had a patch to support using '*' within a string which should be able to accomplish what you were asking about.  I'll dig it up and consider it for inclusion in 1.8.21.

Created attachment 493 [details]
Patch to match multiple '*' characters in an environment variable

The attached patch, relative to sudo 1.8.20, will allow you to use multiple stars when matching an environment variable.  For instance, you can use MYPATH=*:/mydir:* to match MYPATH=/dir1/subdir1:/mydir:/dir2:/dir3/subdir2

Sudo 1.8.21 is out now which includes the attached patch to support multiple '*' characters.