GLIBC_TUNABLES is parsed by glibc in root su/login environments with
AT_SECURE=0 (CVE-2023-4911 class); add it to the dangerous-env-var
denylist next to its GCONV_PATH/LOCPATH siblings.
The TERM/LANG/LC_* pairs accepted without policy (OpenSSH's default
AcceptEnv) had unvalidated values that reach root-exec'd login/su
environments, where glibc treats path-like locale names as locale-data
paths and leading dashes as its own options. Reject path- and
option-like values (and NUL, which would truncate on exec) instead of
trying to sanitize them.
Also cap accepted env var names at 256 bytes before any wildcard
matching, bounding the matcher input and the child's "su -w" allowlist;
real names are short (LOGIN_NAME_MAX).
Updates tailscale/corp#50380
Signed-off-by: James Tucker <james@tailscale.com>
GCONV_PATH, LOCPATH, NLSPATH, HOSTALIASES and RES_OPTIONS can make
privileged processes (root su/login environments built from forwarded
vars) load attacker-controlled files or redirect resolver lookups; add
them to the isDangerousEnvVar denylist, and apply the same check in the
incubator child when it re-filters the inherited forwarded-env payload
(old parents may not have filtered them).
Updates tailscale/corp#50380
Signed-off-by: James Tucker <james@tailscale.com>
matchAcceptEnvPattern recursively retried each '*' suffix, which is
exponential in the number of '*'s in an acceptEnv pattern (2.7s observed
on a 400-byte crafted variable name). Replace it with an iterative
matcher that only backtracks to the most recent '*', preserving the
exact OpenSSH-style glob semantics. Cross-check it against a naive
backtracking reference over random inputs, and pin the crafted
backtracking case as a timing regression test.
Updates tailscale/corp#50380
Signed-off-by: James Tucker <james@tailscale.com>
Previously the acceptEnv variables forwarded to the incubator child were
JSON-encoded onto its command line (--encoded-env), so their values were
visible in /proc/<pid>/cmdline to any other local user and were logged in
the session-start argv (locally and to log.tailscale.com except where
--no-logs-no-support was specified).
This change now carries those variables through an os.Pipe file
descriptor as a json encoded payload. Added end-to-end testing
helps validate secrets reach the session but are not in flags or logged.
Fixestailscale/corp#44903
Change-Id: I5b137b20e9c06feec6b70aaf4e6925e6db74017e
Signed-off-by: Mike Jensen <mikej@tailscale.com>
Co-authored-by: Mike Jensen <mikej@tailscale.com>
Block dynamic linker environment variables (LD_PRELOAD, LD_LIBRARY_PATH,
DYLD_INSERT_LIBRARIES, and friends) from being forwarded regardless of
acceptEnv policy, preventing privilege escalation via wildcard patterns
like "*".
We are not aware of any legitimate use of these variables so they are
safe to exclude from being passed.
Thanks to Tim Sageser (dtrsecurity) for this report.
Updates tailscale/corp#42033
Signed-off-by: Patrick O'Doherty <patrick@tailscale.com>
This file was never truly necessary and has never actually been used in
the history of Tailscale's open source releases.
A Brief History of AUTHORS files
---
The AUTHORS file was a pattern developed at Google, originally for
Chromium, then adopted by Go and a bunch of other projects. The problem
was that Chromium originally had a copyright line only recognizing
Google as the copyright holder. Because Google (and most open source
projects) do not require copyright assignemnt for contributions, each
contributor maintains their copyright. Some large corporate contributors
then tried to add their own name to the copyright line in the LICENSE
file or in file headers. This quickly becomes unwieldy, and puts a
tremendous burden on anyone building on top of Chromium, since the
license requires that they keep all copyright lines intact.
The compromise was to create an AUTHORS file that would list all of the
copyright holders. The LICENSE file and source file headers would then
include that list by reference, listing the copyright holder as "The
Chromium Authors".
This also become cumbersome to simply keep the file up to date with a
high rate of new contributors. Plus it's not always obvious who the
copyright holder is. Sometimes it is the individual making the
contribution, but many times it may be their employer. There is no way
for the proejct maintainer to know.
Eventually, Google changed their policy to no longer recommend trying to
keep the AUTHORS file up to date proactively, and instead to only add to
it when requested: https://opensource.google/docs/releasing/authors.
They are also clear that:
> Adding contributors to the AUTHORS file is entirely within the
> project's discretion and has no implications for copyright ownership.
It was primarily added to appease a small number of large contributors
that insisted that they be recognized as copyright holders (which was
entirely their right to do). But it's not truly necessary, and not even
the most accurate way of identifying contributors and/or copyright
holders.
In practice, we've never added anyone to our AUTHORS file. It only lists
Tailscale, so it's not really serving any purpose. It also causes
confusion because Tailscalars put the "Tailscale Inc & AUTHORS" header
in other open source repos which don't actually have an AUTHORS file, so
it's ambiguous what that means.
Instead, we just acknowledge that the contributors to Tailscale (whoever
they are) are copyright holders for their individual contributions. We
also have the benefit of using the DCO (developercertificate.org) which
provides some additional certification of their right to make the
contribution.
The source file changes were purely mechanical with:
git ls-files | xargs sed -i -e 's/\(Tailscale Inc &\) AUTHORS/\1 contributors/g'
Updates #cleanup
Change-Id: Ia101a4a3005adb9118051b3416f5a64a4a45987d
Signed-off-by: Will Norris <will@tailscale.com>
Add logic for parsing and matching against our planned format for
AcceptEnv values. Namely, this supports direct matches against string
values and matching where * and ? are treated as wildcard characters
which match against an arbitrary number of characters and a single
character respectively.
Actually using this logic in non-test code will come in subsequent
changes.
Updates https://github.com/tailscale/corp/issues/22775
Signed-off-by: Mario Minardi <mario@tailscale.com>
James Tucker