Files
snapd/osutil/env_test.go
Zygmunt Krynicki 913d765818 many: improve environment handling, fixing duplicate entries (#8242)
* strutil: add Environment, EnvironmentDelta and RawEnvironment

This patch adds three new types and supporting methods.

Environment is an unordered map representing environment entries that is
convenient to modify.  EnvironmentDelta is an ordered map representing
changes to an environment that also supports variable substitution and
expansion. Finally RawEnvironment is a slice representing key=value
pairs that is useful for low-level system interfaces.

Environment and EnvironmentDelta can be modified with simple map-like
methods Get, Set, Del. RawEnvironment can only be parsed to an
Environment.

The patch also contains Environment.ApplyDelta function, which correctly
applies a single delta to a given environment.

The types are designed to avoid the mistake of appending two slices
representing environment strings together. This was the original cause
of bug https://bugs.launchpad.net/snapd/+bug/1860369

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

* many: use Environment, EnvironmentDelta and RawEnvironment

This patch replaces all ad-hoc environment handling with the three
aforementioned types. Environment is used whenever we are working
with the real complete environment (starting from OSEnvironment).
EnvironmentDelta is used to modify it according to snap-level and
application-level or hook-level overrides. Finally RawEnvironment
is only used by snap run and snap-exec, immediately before calling
execve.

This fixes incorrect handling of environment in snap-exec and also
avoids this class of bugs by making it hard, due to type system,
to express mistakes again.

Fixes: https://bugs.launchpad.net/snapd/+bug/1860369
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

* osutil/env: import all symbols from check.v1

This style is shorter and we use it nearly everywhere else.
I'm about to combine more tests into this file and I don't want
to invert their style over to the verbose one.

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

* many: move new environment code to osutil

There's a lot of churn but that's unavoidable in such a move.

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

* many: remove RawEnvironment type

The raw environment is necessary only for executing programs. We can
remove the type and replace it with []string in tests (where it has most
of the usage) and a few call sites.

The corresponding method from Environment was renamed to ForExec().

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

* many: make Environment a map[string]string

Some extra methods such as Get, Set and Delete are retained but they
are all arguably unnecessarily and will be removed in another pass.

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

* many: remove most map-like methods from Environment

This includes Get, Delete and Contains but not Set. Set has extra
semantics that will be removed in another pass.

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

* many: remove Environment.Set

This patch removes the leftover map-like method from the Environment
type. Some extra care was applied to ensure we don't try to write
through a nil map, something that Set protected against.

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

* cmd/snap-exec: use NewEnvironment in test code

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

* many: rename EnvironmentDelta to ExpandableEnv

The main feature of what was called EnvironmentDelta is the fact it
could expand expressions in variable definitions. This makes it more
obvious.

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

* many: make ExpandableEnv an immutable *strutil.OrderedMap

There's a lot of extra changes to make the journey more complete.

First of all both snap.AppInfo and snap.HookInfo provide a method
EnvStack() []osutil.ExpandableEnv which replaces the earlier
EnvironmentOverrides. Instead of merging environment entries the
entire chain is modeled. This will be relevant when we want to expand
values.

This also means that support code to make EnvStack mutable is gone.

Second of all, snap/snapenv.ExecEnv has reverted to using plain
Environment (aka map[string]string) for basicEnv and userEnv. The
values never had expressions so using the more sophisticated code
there is not really necessary.

Delta from master could be reduced further but I went for clarity
of the progression over the clarity of the end-to-end diff.

Lastly, a small but important change, snap-exec now expands each
ExpandableEnv in sequence. First for the snap-level and then for the
app or hook level. This should fix the issue that each level wanted
to expand but not completely erase the value set previously.

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

* osutil: remove NewEnvironment

With the current type it's not any better than just defining a map
locally.

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

* osutil: make ParseRawEnvironment private

It is only required for OSEnvironment and is not used anywhere outside
of the package.

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

* many: rename ApplyDelta to SetExpandableEnv

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

* osutil: update stale documentation referring to delta

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

* osutil: weaken SetExpandableEnv

When initially implemented as a part of this branch, SetExpandableEnv
(nee ApplyDelta) behaved in somewhat unusual and "strong" way, where
forward references _were_ expanded. This was not previously so and
arguably is not needed.

As the code stands now, we have proper chaining starting from system
environment, to snap-level environment declarations (that can expand
variables) and all the way down to application or hook-level environment
declarations (also with variable expansions). Since such declarations
are ordered this provides the equivalence of shell execution performing
the same expansions.

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

* many: special-case Environment.Transform

Instead of offering specialized environment transformation functions
offer just the things we need: escaping and un-escaping of unsafe
variables removed by the dynamic linker when loading binaries with
AT_SECURE flag.

Various bits move from snap/snapenv to osutil.

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

* many: EnvStack to EnvChain, SetExpandableEnv to ExtendWithExpanded

also adjust doc comments and cover ExtendWithExpanded for the nil
initial env case

* cmd/snap,snap/snapenv: change snapenv.ExecEnv into ExtendEnvForRun

* many: make preserving ld.so setuid unsafe vars a boundary concern

support the escaping/unescaping that we use for preserving vars
stripped out by ld.so for snap-confine, via variants of
OSEnvironment/ForExec so that is happens at the process boundaries,
loading os env and setting env for exec

add/change some tests related to this

adjust comments/doc comments or add

Co-authored-by: Samuele Pedroni <pedronis@lucediurna.net>
Co-authored-by: Michael Vogt <mvo@ubuntu.com>
2020-03-20 11:58:19 +01:00

333 lines
10 KiB
Go

// -*- Mode: Go; indent-tabs-mode: t -*-
/*
* Copyright (C) 2016 Canonical Ltd
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
package osutil_test
import (
"fmt"
"math"
"os"
"strings"
. "gopkg.in/check.v1"
"github.com/snapcore/snapd/osutil"
)
type envSuite struct{}
var _ = Suite(&envSuite{})
func (s *envSuite) TestGetenvBoolTrue(c *C) {
key := "__XYZZY__"
os.Unsetenv(key)
for _, s := range []string{
"1", "t", "TRUE",
} {
os.Setenv(key, s)
c.Assert(os.Getenv(key), Equals, s)
c.Check(osutil.GetenvBool(key), Equals, true, Commentf(s))
c.Check(osutil.GetenvBool(key, false), Equals, true, Commentf(s))
c.Check(osutil.GetenvBool(key, true), Equals, true, Commentf(s))
}
}
func (s *envSuite) TestGetenvBoolFalse(c *C) {
key := "__XYZZY__"
os.Unsetenv(key)
c.Assert(osutil.GetenvBool(key), Equals, false)
for _, s := range []string{
"", "0", "f", "FALSE", "potato",
} {
os.Setenv(key, s)
c.Assert(os.Getenv(key), Equals, s)
c.Check(osutil.GetenvBool(key), Equals, false, Commentf(s))
c.Check(osutil.GetenvBool(key, false), Equals, false, Commentf(s))
}
}
func (s *envSuite) TestGetenvBoolFalseDefaultTrue(c *C) {
key := "__XYZZY__"
os.Unsetenv(key)
c.Assert(osutil.GetenvBool(key), Equals, false)
for _, s := range []string{
"0", "f", "FALSE",
} {
os.Setenv(key, s)
c.Assert(os.Getenv(key), Equals, s)
c.Check(osutil.GetenvBool(key, true), Equals, false, Commentf(s))
}
for _, s := range []string{
"", "potato", // etc
} {
os.Setenv(key, s)
c.Assert(os.Getenv(key), Equals, s)
c.Check(osutil.GetenvBool(key, true), Equals, true, Commentf(s))
}
}
func (s *envSuite) TestGetenvInt64(c *C) {
key := "__XYZZY__"
os.Unsetenv(key)
c.Check(osutil.GetenvInt64(key), Equals, int64(0))
c.Check(osutil.GetenvInt64(key, -1), Equals, int64(-1))
c.Check(osutil.GetenvInt64(key, math.MaxInt64), Equals, int64(math.MaxInt64))
c.Check(osutil.GetenvInt64(key, math.MinInt64), Equals, int64(math.MinInt64))
for _, n := range []int64{
0, -1, math.MinInt64, math.MaxInt64,
} {
for _, tpl := range []string{"%d", " %d ", "%#x", "%#X", "%#o"} {
v := fmt.Sprintf(tpl, n)
os.Setenv(key, v)
c.Assert(os.Getenv(key), Equals, v)
c.Check(osutil.GetenvInt64(key), Equals, n, Commentf(v))
}
}
}
func (s *envSuite) TestParseRawEnvironmentHappy(c *C) {
for _, t := range []struct {
env []string
expected map[string]string
}{
{
[]string{"K=V"},
map[string]string{"K": "V"},
},
{
[]string{"K=V=V=V"},
map[string]string{"K": "V=V=V"},
},
{
[]string{"K1=V1", "K2=V2"},
map[string]string{"K1": "V1", "K2": "V2"},
},
} {
env, err := osutil.ParseRawEnvironment(t.env)
c.Assert(err, IsNil)
c.Check(env, DeepEquals, osutil.Environment(t.expected), Commentf("invalid result for %q, got %q expected %q", t.env, env, t.expected))
}
}
func (s *envSuite) TestParseRawEnvironmentNotKeyValue(c *C) {
env, err := osutil.ParseRawEnvironment([]string{"KEY"})
c.Assert(err, ErrorMatches, `cannot parse environment entry: "KEY"`)
c.Assert(env, IsNil)
}
func (s *envSuite) TestParseRawEnvironmentEmptyKey(c *C) {
env, err := osutil.ParseRawEnvironment([]string{"=VALUE"})
c.Assert(err, ErrorMatches, `environment variable name cannot be empty: "=VALUE"`)
c.Assert(env, IsNil)
}
func (s *envSuite) TestParseRawEnvironmentDuplicateKey(c *C) {
env, err := osutil.ParseRawEnvironment([]string{"K=1", "K=2"})
c.Assert(err, ErrorMatches, `cannot overwrite earlier value of "K"`)
c.Assert(env, IsNil)
}
func (s *envSuite) TestOSEnvironment(c *C) {
env, err := osutil.OSEnvironment()
c.Assert(err, IsNil)
c.Check(len(os.Environ()), Equals, len(env.ForExec()))
c.Check(os.Getenv("PATH"), Equals, env["PATH"])
}
func (s *envSuite) TestOSEnvironmentUnescapeUnsafe(c *C) {
os.Setenv("SNAPD_UNSAFE_PREFIX_A", "a")
defer os.Unsetenv("SNAPD_UNSAFE_PREFIX_A")
os.Setenv("SNAPDEXTRA", "2")
defer os.Unsetenv("SNAPDEXTRA")
os.Setenv("SNAPD_UNSAFE_PREFIX_SNAPDEXTRA", "1")
defer os.Unsetenv("SNAPD_UNSAFE_PREFIX_SNAPDEXTRA")
env, err := osutil.OSEnvironmentUnescapeUnsafe("SNAPD_UNSAFE_PREFIX_")
c.Assert(err, IsNil)
// -1 because only the unescaped SNAPDEXTRA is kept
c.Check(len(os.Environ())-1, Equals, len(env.ForExec()))
c.Check(os.Getenv("PATH"), Equals, env["PATH"])
c.Check("a", Equals, env["A"])
c.Check("2", Equals, env["SNAPDEXTRA"])
}
func (s *envSuite) TestGet(c *C) {
env := osutil.Environment{"K": "V"}
c.Assert(env["K"], Equals, "V")
c.Assert(env["missing"], Equals, "")
}
func (s *envSuite) TestDel(c *C) {
env := osutil.Environment{"K": "V"}
delete(env, "K")
c.Assert(env["K"], Equals, "")
delete(env, "missing")
c.Assert(env["missing"], Equals, "")
}
func (s *envSuite) TestForExec(c *C) {
env := osutil.Environment{"K1": "V1", "K2": "V2"}
c.Check(env.ForExec(), DeepEquals, []string{"K1=V1", "K2=V2"})
}
func (s *envSuite) TestNewExpandableEnv(c *C) {
eenv := osutil.NewExpandableEnv("K1", "V1", "K2", "$K1")
c.Check(eenv.Get("K1"), Equals, "V1")
c.Check(eenv.Get("K2"), Equals, "$K1")
}
func (s *envSuite) TestParseRawExpandableEnvHappy(c *C) {
eenv, err := osutil.ParseRawExpandableEnv([]string{"K1=V1", "K2=$K1"})
c.Assert(err, IsNil)
c.Check(eenv.Get("K1"), Equals, "V1")
c.Check(eenv.Get("K2"), Equals, "$K1")
}
func (s *envSuite) TestParseRawExpandableEnvNotKeyValue(c *C) {
eenv, err := osutil.ParseRawExpandableEnv([]string{"KEY"})
c.Assert(err, ErrorMatches, `cannot parse environment entry: "KEY"`)
c.Assert(eenv, DeepEquals, osutil.ExpandableEnv{})
}
func (s *envSuite) TestParseRawExpandableEnvEmptyKey(c *C) {
eenv, err := osutil.ParseRawExpandableEnv([]string{"=VALUE"})
c.Assert(err, ErrorMatches, `environment variable name cannot be empty: "=VALUE"`)
c.Assert(eenv, DeepEquals, osutil.ExpandableEnv{})
}
func (s *envSuite) TestParseRawExpandableEnvDuplicateKey(c *C) {
eenv, err := osutil.ParseRawExpandableEnv([]string{"K=1", "K=2"})
c.Assert(err, ErrorMatches, `cannot overwrite earlier value of "K"`)
c.Assert(eenv, DeepEquals, osutil.ExpandableEnv{})
}
func (s *envSuite) TestExtendWithExpanded(c *C) {
env := osutil.Environment{"A": "a"}
env.ExtendWithExpanded(osutil.NewExpandableEnv(
"B", "$C", // $C is undefined so it expands to ""
"C", "$A", // $A is defined in the environment so it expands to "a"
"D", "$D", // $D is undefined so it expands to ""
))
c.Check(env, DeepEquals, osutil.Environment{"A": "a", "B": "", "C": "a", "D": ""})
}
func (s *envSuite) TestExtendWithExpandedOfNil(c *C) {
var env osutil.Environment
env.ExtendWithExpanded(osutil.NewExpandableEnv(
"A", "a",
"B", "$C", // $C is undefined so it expands to ""
"C", "$A", // $A is defined in the environment so it expands to "a"
"D", "$D", // $D is undefined so it expands to ""
))
c.Check(env, DeepEquals, osutil.Environment{"A": "a", "B": "", "C": "a", "D": ""})
}
func (s *envSuite) TestExtendWithExpandedForEnvOverride(c *C) {
env := osutil.Environment{"PATH": "system-value"}
env.ExtendWithExpanded(osutil.NewExpandableEnv("PATH", "snap-level-override"))
env.ExtendWithExpanded(osutil.NewExpandableEnv("PATH", "app-level-override"))
c.Check(env, DeepEquals, osutil.Environment{"PATH": "app-level-override"})
}
func (s *envSuite) TestExtendWithExpandedForEnvExpansion(c *C) {
env := osutil.Environment{"PATH": "system-value"}
env.ExtendWithExpanded(osutil.NewExpandableEnv("PATH", "snap-ext:$PATH"))
env.ExtendWithExpanded(osutil.NewExpandableEnv("PATH", "app-ext:$PATH"))
c.Check(env, DeepEquals, osutil.Environment{"PATH": "app-ext:snap-ext:system-value"})
}
func (s *envSuite) TestExtendWithExpandedVarious(c *C) {
for _, t := range []struct {
env string
expected string
}{
// trivial
{"K1=V1,K2=V2", "K1=V1,K2=V2"},
// simple (order is preserved)
{"K=V,K2=$K", "K=V,K2=V"},
// simple from environment
{"K=$PATH", fmt.Sprintf("K=%s", os.Getenv("PATH"))},
// append to substitution from environment
{"K=${PATH}:/foo", fmt.Sprintf("K=%s", os.Getenv("PATH")+":/foo")},
// multi-level
{"A=1,B=$A/2,C=$B/3,D=$C/4", "A=1,B=1/2,C=1/2/3,D=1/2/3/4"},
// parsing is top down
{"A=$A", "A="},
{"A=$B,B=$A", "A=,B="},
{"A=$B,B=$C,C=$A", "A=,B=,C="},
} {
eenv, err := osutil.ParseRawExpandableEnv(strings.Split(t.env, ","))
c.Assert(err, IsNil)
env := osutil.Environment{}
if strings.Contains(t.env, "PATH") {
env["PATH"] = os.Getenv("PATH")
}
env.ExtendWithExpanded(eenv)
delete(env, "PATH")
c.Check(strings.Join(env.ForExec(), ","), DeepEquals, t.expected, Commentf("invalid result for %q, got %q expected %q", t.env, env, t.expected))
}
}
func (s *envSuite) TestForExecEscapeUnsafe(c *C) {
env := osutil.Environment{
"FOO": "foo",
"LD_PRELOAD": "/opt/lib/libfunky.so",
"SNAP_DATA": "snap-data",
"SNAP_SAVED_WHAT": "what", // will be dropped
"SNAP_SAVED": "snap-saved",
"SNAP_S": "snap-s",
"XDG_STUFF": "xdg-stuff", // will be prefixed
"TMPDIR": "/var/tmp", // will be prefixed
}
raw := env.ForExecEscapeUnsafe("SNAP_SAVED_")
c.Check(raw, DeepEquals, []string{
"FOO=foo",
"SNAP_DATA=snap-data",
"SNAP_S=snap-s",
"SNAP_SAVED=snap-saved",
"SNAP_SAVED_LD_PRELOAD=/opt/lib/libfunky.so",
"SNAP_SAVED_TMPDIR=/var/tmp",
"XDG_STUFF=xdg-stuff",
})
}
func (s *envSuite) TestForExecEscapeUnsafeNothingToEscape(c *C) {
env := osutil.Environment{
"FOO": "foo",
"SNAP_DATA": "snap-data",
"SNAP_SAVED_WHAT": "what",
"SNAP_SAVED": "snap-saved",
"SNAP_S": "snap-s",
"XDG_STUFF": "xdg-stuff",
}
raw := env.ForExecEscapeUnsafe("SNAP_SAVED_")
c.Check(raw, DeepEquals, []string{
"FOO=foo",
"SNAP_DATA=snap-data",
"SNAP_S=snap-s",
"SNAP_SAVED=snap-saved",
"XDG_STUFF=xdg-stuff",
})
}