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 <mario@tailscale.com>
2 months ago · c90c9938c8
parent 9eb59c72c1
commit c90c9938c8
2 changed files with 250 additions and 0 deletions
--- a/ssh/tailssh/accept_env.go
+++ 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++
+	}
+}
--- a/ssh/tailssh/accept_env_test.go
+++ 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)
+			}
+		})
+	}
+}