From c90c9938c8a20654bc10ffe0236e4f3580b7d684 Mon Sep 17 00:00:00 2001 From: Mario Minardi Date: Wed, 25 Sep 2024 21:09:05 -0600 Subject: [PATCH] ssh/tailssh: add logic for matching against AcceptEnv patterns (#13466) 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 --- ssh/tailssh/accept_env.go | 110 ++++++++++++++++++++++++++ ssh/tailssh/accept_env_test.go | 140 +++++++++++++++++++++++++++++++++ 2 files changed, 250 insertions(+) create mode 100644 ssh/tailssh/accept_env.go create mode 100644 ssh/tailssh/accept_env_test.go diff --git a/ssh/tailssh/accept_env.go b/ssh/tailssh/accept_env.go new file mode 100644 index 000000000..df4f1d010 --- /dev/null +++ b/ssh/tailssh/accept_env.go @@ -0,0 +1,110 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package tailssh + +import ( + "slices" + "strings" +) + +// filterEnv filters a passed in environ string slice (a slice with strings +// representing environment variables in the form "key=value") based on +// the supplied slice of acceptEnv values. +// +// acceptEnv is a slice of environment variable names that are allowlisted +// for the SSH rule in the policy file. +// +// acceptEnv values may contain * and ? wildcard characters which match against +// zero or one or more characters and a single character respectively. +func filterEnv(acceptEnv []string, environ []string) []string { + var acceptedPairs []string + + for _, envPair := range environ { + envVar := strings.Split(envPair, "=")[0] + + // Short circuit if we have a direct match between the environment + // variable and an AcceptEnv value. + if slices.Contains(acceptEnv, envVar) { + acceptedPairs = append(acceptedPairs, envPair) + continue + } + + // Otherwise check if we have a wildcard pattern that matches. + if matchAcceptEnv(acceptEnv, envVar) { + acceptedPairs = append(acceptedPairs, envPair) + continue + } + } + + return acceptedPairs +} + +// matchAcceptEnv is a convenience function that wraps calling matchAcceptEnvPattern +// with every value in acceptEnv for a given env that is being matched against. +func matchAcceptEnv(acceptEnv []string, env string) bool { + for _, pattern := range acceptEnv { + if matchAcceptEnvPattern(pattern, env) { + return true + } + } + + return false +} + +// matchAcceptEnvPattern returns true if the pattern matches against the target string. +// Patterns may include * and ? wildcard characters which match against zero or one or +// more characters and a single character respectively. +func matchAcceptEnvPattern(pattern string, target string) bool { + patternIdx := 0 + targetIdx := 0 + + for { + // If we are at the end of the pattern we can only have a match if we + // are also at the end of the target. + if patternIdx >= len(pattern) { + return targetIdx >= len(target) + } + + if pattern[patternIdx] == '*' { + // Optimization to skip through any repeated asterisks as they + // have the same net effect on our search. + for patternIdx < len(pattern) { + if pattern[patternIdx] != '*' { + break + } + + patternIdx++ + } + + // We are at the end of the pattern after matching the asterisk, + // implying a match. + if patternIdx >= len(pattern) { + return true + } + + // Search through the target sequentially for the next character + // from the pattern string, recursing into matchAcceptEnvPattern + // to try and find a match. + for ; targetIdx < len(target); targetIdx++ { + if matchAcceptEnvPattern(pattern[patternIdx:], target[targetIdx:]) { + return true + } + } + + // No match after searching through the entire target. + return false + } + + if targetIdx >= len(target) { + return false + } + + if pattern[patternIdx] != '?' && pattern[patternIdx] != target[targetIdx] { + return false + } + + patternIdx++ + targetIdx++ + } +} diff --git a/ssh/tailssh/accept_env_test.go b/ssh/tailssh/accept_env_test.go new file mode 100644 index 000000000..c67774447 --- /dev/null +++ b/ssh/tailssh/accept_env_test.go @@ -0,0 +1,140 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package tailssh + +import ( + "fmt" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestMatchAcceptEnvPattern(t *testing.T) { + testCases := []struct { + pattern string + target string + match bool + }{ + {pattern: "*", target: "EXAMPLE_ENV", match: true}, + {pattern: "***", target: "123456", match: true}, + + {pattern: "?", target: "A", match: true}, + {pattern: "?", target: "123", match: false}, + + {pattern: "?*", target: "EXAMPLE_2", match: true}, + {pattern: "?*", target: "", match: false}, + + {pattern: "*?", target: "A", match: true}, + {pattern: "*?", target: "", match: false}, + + {pattern: "??", target: "CC", match: true}, + {pattern: "??", target: "123", match: false}, + + {pattern: "*?*", target: "ABCDEFG", match: true}, + {pattern: "*?*", target: "C", match: true}, + {pattern: "*?*", target: "", match: false}, + + {pattern: "?*?", target: "ABCDEFG", match: true}, + {pattern: "?*?", target: "A", match: false}, + + {pattern: "**?TEST", target: "_TEST", match: true}, + {pattern: "**?TEST", target: "_TESTING", match: false}, + + {pattern: "TEST**?", target: "TEST_", match: true}, + {pattern: "TEST**?", target: "A_TEST_", match: false}, + + {pattern: "TEST_*", target: "TEST_A", match: true}, + {pattern: "TEST_*", target: "TEST_A_LONG_ENVIRONMENT_VARIABLE_NAME", match: true}, + {pattern: "TEST_*", target: "TEST", match: false}, + + {pattern: "EXAMPLE_?_ENV", target: "EXAMPLE_A_ENV", match: true}, + {pattern: "EXAMPLE_?_ENV", target: "EXAMPLE_ENV", match: false}, + + {pattern: "EXAMPLE_*_ENV", target: "EXAMPLE_aBcd2231---_ENV", match: true}, + {pattern: "EXAMPLE_*_ENV", target: "EXAMPLEENV", match: false}, + + {pattern: "COMPLICA?ED_PATTERN*", target: "COMPLICATED_PATTERN_REST", match: true}, + {pattern: "COMPLICA?ED_PATTERN*", target: "COMPLICATED_PATT", match: false}, + + {pattern: "COMPLICAT???ED_PATT??ERN", target: "COMPLICAT123ED_PATTggERN", match: true}, + {pattern: "COMPLICAT???ED_PATT??ERN", target: "COMPLICATED_PATTERN", match: false}, + + {pattern: "DIRECT_MATCH", target: "DIRECT_MATCH", match: true}, + {pattern: "DIRECT_MATCH", target: "MISS", match: false}, + + // OpenSSH compatibility cases + // See https://github.com/openssh/openssh-portable/blob/master/regress/unittests/match/tests.c + {pattern: "", target: "", match: true}, + {pattern: "aaa", target: "", match: false}, + {pattern: "", target: "aaa", match: false}, + {pattern: "aaaa", target: "aaa", match: false}, + {pattern: "aaa", target: "aaaa", match: false}, + {pattern: "*", target: "", match: true}, + {pattern: "?", target: "a", match: true}, + {pattern: "a?", target: "aa", match: true}, + {pattern: "*", target: "a", match: true}, + {pattern: "a*", target: "aa", match: true}, + {pattern: "?*", target: "aa", match: true}, + {pattern: "**", target: "aa", match: true}, + {pattern: "?a", target: "aa", match: true}, + {pattern: "*a", target: "aa", match: true}, + {pattern: "a?", target: "ba", match: false}, + {pattern: "a*", target: "ba", match: false}, + {pattern: "?a", target: "ab", match: false}, + {pattern: "*a", target: "ab", match: false}, + } + + for _, tc := range testCases { + name := fmt.Sprintf("pattern_%s_target_%s", tc.pattern, tc.target) + if tc.match { + name += "_should_match" + } else { + name += "_should_not_match" + } + + t.Run(name, func(t *testing.T) { + match := matchAcceptEnvPattern(tc.pattern, tc.target) + if match != tc.match { + t.Errorf("got %v, want %v", match, tc.match) + } + }) + } +} + +func TestFilterEnv(t *testing.T) { + testCases := []struct { + name string + acceptEnv []string + environ []string + expectedFiltered []string + }{ + { + name: "simple direct matches", + acceptEnv: []string{"FOO", "FOO2", "FOO_3"}, + environ: []string{"FOO=BAR", "FOO2=BAZ", "FOO_3=123", "FOOOO4-2=AbCdEfG"}, + expectedFiltered: []string{"FOO=BAR", "FOO2=BAZ", "FOO_3=123"}, + }, + { + name: "bare wildcard", + acceptEnv: []string{"*"}, + environ: []string{"FOO=BAR", "FOO2=BAZ", "FOO_3=123", "FOOOO4-2=AbCdEfG"}, + expectedFiltered: []string{"FOO=BAR", "FOO2=BAZ", "FOO_3=123", "FOOOO4-2=AbCdEfG"}, + }, + { + name: "complex matches", + acceptEnv: []string{"FO?", "FOOO*", "FO*5?7"}, + environ: []string{"FOO=BAR", "FOO2=BAZ", "FOO_3=123", "FOOOO4-2=AbCdEfG", "FO1-kmndGamc79567=ABC", "FO57=BAR2"}, + expectedFiltered: []string{"FOO=BAR", "FOOOO4-2=AbCdEfG", "FO1-kmndGamc79567=ABC"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + filtered := filterEnv(tc.acceptEnv, tc.environ) + if diff := cmp.Diff(tc.expectedFiltered, filtered); diff != "" { + t.Errorf("unexpected filter result (-got,+want): \n%s", diff) + } + }) + } +}