Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read user environment when creating child session #1020

Merged
merged 1 commit into from
May 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,6 @@ const (
// we don't have another status code for it.
RemoteCommandFailure = 255
)

// MaxEnvironmentFileLines is the maximum number of lines in a environment file.
const MaxEnvironmentFileLines = 1000
12 changes: 12 additions & 0 deletions lib/config/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ type CommandLineFlags struct {
GopsAddr string
// DiagnosticAddr is listen address for diagnostic endpoint
DiagnosticAddr string
// PermitUserEnvironment enables reading of ~/.tsh/environment
// when creating a new session.
PermitUserEnvironment bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not make this a boolean flag, rather than a list with files to read with default to .tsh/environment so folks can actually set to something else if they want, e.g. will close the issue #1011

}

// readConfigFile reads /etc/teleport.yaml (or whatever is passed via --config flag)
Expand Down Expand Up @@ -419,6 +422,10 @@ func ApplyFileConfig(fc *FileConfig, cfg *service.Config) error {
if fc.SSH.Namespace != "" {
cfg.SSH.Namespace = fc.SSH.Namespace
}
if fc.SSH.PermitUserEnvironment {
cfg.SSH.PermitUserEnvironment = true
}

// read 'trusted_clusters' section:
if fc.Auth.Enabled() && len(fc.Auth.TrustedClusters) > 0 {
if err := readTrustedClusters(fc.Auth.TrustedClusters, cfg); err != nil {
Expand Down Expand Up @@ -703,6 +710,11 @@ func Configure(clf *CommandLineFlags, cfg *service.Config) error {
}
cfg.Auth.StorageConfig.Params["data_dir"] = cfg.DataDir

// command line flag takes precedence over file config
if clf.PermitUserEnvironment {
cfg.SSH.PermitUserEnvironment = true
}

return nil
}

Expand Down
46 changes: 46 additions & 0 deletions lib/config/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -544,3 +544,49 @@ func makeConfigFixture() string {

return conf.DebugDumpToYAML()
}

func (s *ConfigTestSuite) TestPermitUserEnvironment(c *check.C) {
tests := []struct {
inConfigString string
inPermitUserEnvironment bool
outPermitUserEnvironment bool
}{
// 0 - set on the command line, expect PermitUserEnvironment to be true
{
``,
true,
true,
},
// 1 - set in config file, expect PermitUserEnvironment to be true
{
`
ssh_service:
permit_user_env: true
`,
false,
true,
},
// 2 - not set anywhere, expect PermitUserEnvironment to be false
{
``,
false,
false,
},
}

// run tests
for i, tt := range tests {
comment := check.Commentf("Test %v", i)

clf := CommandLineFlags{
ConfigString: base64.StdEncoding.EncodeToString([]byte(tt.inConfigString)),
PermitUserEnvironment: tt.inPermitUserEnvironment,
}
cfg := service.MakeDefaultConfig()

err := Configure(&clf, cfg)
c.Assert(err, check.IsNil, comment)

c.Assert(cfg.SSH.PermitUserEnvironment, check.Equals, tt.outPermitUserEnvironment, comment)
}
}
10 changes: 6 additions & 4 deletions lib/config/fileconf.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ var (
"cache": true,
"ttl": false,
"issuer": false,
"permit_user_env": false,
}
)

Expand Down Expand Up @@ -526,10 +527,11 @@ func (u *UniversalSecondFactor) Parse() (services.UniversalSecondFactor, error)

// SSH is 'ssh_service' section of the config file
type SSH struct {
Service `yaml:",inline"`
Namespace string `yaml:"namespace,omitempty"`
Labels map[string]string `yaml:"labels,omitempty"`
Commands []CommandLabel `yaml:"commands,omitempty"`
Service `yaml:",inline"`
Namespace string `yaml:"namespace,omitempty"`
Labels map[string]string `yaml:"labels,omitempty"`
Commands []CommandLabel `yaml:"commands,omitempty"`
PermitUserEnvironment bool `yaml:"permit_user_env,omitempty"`
}

// CommandLabel is `command` section of `ssh_service` in the config file
Expand Down
15 changes: 8 additions & 7 deletions lib/service/cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,13 +262,14 @@ type AuthConfig struct {

// SSHConfig configures SSH server node role
type SSHConfig struct {
Enabled bool
Addr utils.NetAddr
Namespace string
Shell string
Limiter limiter.LimiterConfig
Labels map[string]string
CmdLabels services.CommandLabels
Enabled bool
Addr utils.NetAddr
Namespace string
Shell string
Limiter limiter.LimiterConfig
Labels map[string]string
CmdLabels services.CommandLabels
PermitUserEnvironment bool
}

// MakeDefaultConfig creates a new Config structure and populates it with defaults
Expand Down
1 change: 1 addition & 0 deletions lib/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,7 @@ func (process *TeleportProcess) initSSH() error {
srv.SetSessionServer(conn.Client),
srv.SetLabels(cfg.SSH.Labels, cfg.SSH.CmdLabels),
srv.SetNamespace(namespace),
srv.SetPermitUserEnvironment(cfg.SSH.PermitUserEnvironment),
)
if err != nil {
return trace.Wrap(err)
Expand Down
11 changes: 11 additions & 0 deletions lib/srv/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,17 @@ func prepareCommand(ctx *ctx) (*exec.Cmd, error) {
c.Env = append(c.Env, fmt.Sprintf("%s=%s", teleport.SSHSessionID, ctx.session.id))
}
}

// if the server allows reading in of ~/.tsh/environment read it in
// and pass environment variables along to new session
if ctx.srv.PermitUserEnvironment() {
filename := filepath.Join(osUser.HomeDir, ".tsh", "environment")
userEnvs, err := utils.ReadEnvironmentFile(filename)
if err != nil {
return nil, trace.Wrap(err)
}
c.Env = append(c.Env, userEnvs...)
}
return c, nil
}

Expand Down
18 changes: 18 additions & 0 deletions lib/srv/sshserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ type Server struct {

// clock is a system clock
clock clockwork.Clock

// permitUserEnvironment controls if this server will read ~/.tsh/environment
// before creating a new session.
permitUserEnvironment bool
}

// ServerOption is a functional option passed to the server
Expand Down Expand Up @@ -188,6 +192,14 @@ func SetNamespace(namespace string) ServerOption {
}
}

// SetPermitUserEnvironment allows you to set the value of permitUserEnvironment.
func SetPermitUserEnvironment(permitUserEnvironment bool) ServerOption {
return func(s *Server) error {
s.permitUserEnvironment = permitUserEnvironment
return nil
}
}

// New returns an unstarted server
func New(addr utils.NetAddr,
hostname string,
Expand Down Expand Up @@ -275,6 +287,12 @@ func (s *Server) ID() string {
return s.uuid
}

// PermitUserEnvironment returns if ~/.tsh/environment will be read before a
// session is created by this server.
func (s *Server) PermitUserEnvironment() bool {
return s.permitUserEnvironment
}

func (s *Server) setAdvertiseIP(ip net.IP) {
s.Lock()
defer s.Unlock()
Expand Down
67 changes: 67 additions & 0 deletions lib/utils/environment.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package utils

import (
"bufio"
"os"
"strings"

"github.com/gravitational/teleport"

log "github.com/Sirupsen/logrus"
"github.com/gravitational/trace"
)

// ReadEnvironmentFile will read environment variables from a passed in location.
// Lines that start with "#" or empty lines are ignored. Assignments are in the
// form name=value and no variable expansion occurs.
func ReadEnvironmentFile(filename string) ([]string, error) {
file, err := os.Open(filename)
if err != nil {
return nil, trace.ConvertSystemError(err)
}
defer file.Close()

var lineno int
var envs []string

scanner := bufio.NewScanner(file)
for scanner.Scan() {
line := strings.TrimSpace(scanner.Text())

// follow the lead of OpenSSH and don't allow more than 1,000 environment variables
// https://github.com/openssh/openssh-portable/blob/master/session.c#L873-L874
lineno = lineno + 1
if lineno > teleport.MaxEnvironmentFileLines {
return nil, trace.BadParameter("too many lines in environment file %v", filename)
}

// empty lines or lines that start with # are ignored
if line == "" || line[0] == '#' {
continue
}

// split on first =, if not found, log it and continue
idx := strings.Index(line, "=")
if idx == -1 {
log.Debugf("Bad line %v while reading %v: no = separator found", lineno, filename)
continue
}

// split key and value and make sure that key has a name
key := line[:idx]
value := line[idx+1:]
if strings.TrimSpace(key) == "" {
log.Debugf("Bad line %v while reading %v: key without name", lineno, filename)
continue
}

envs = append(envs, key+"="+value)
}

err = scanner.Err()
if err != nil {
return nil, trace.Wrap(err)
}

return envs, nil
}
66 changes: 66 additions & 0 deletions lib/utils/environment_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
Copyright 2017 Gravitational, Inc.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package utils

import (
"fmt"
"io/ioutil"
"os"

"gopkg.in/check.v1"
)

type EnvironmentSuite struct{}

var _ = check.Suite(&EnvironmentSuite{})
var _ = fmt.Printf

func (s *EnvironmentSuite) SetUpSuite(c *check.C) {
InitLoggerForTests()
}
func (s *EnvironmentSuite) TearDownSuite(c *check.C) {}
func (s *EnvironmentSuite) SetUpTest(c *check.C) {}
func (s *EnvironmentSuite) TearDownTest(c *check.C) {}

func (s *EnvironmentSuite) TestReadEnvironmentFile(c *check.C) {
// contents of environment file
rawenv := []byte(`
foo=bar
# comment
foo=bar=baz
# comment 2
=
foo=

=bar
`)

// create a temp file with an environment in it
f, err := ioutil.TempFile("", "teleport-environment-")
c.Assert(err, check.IsNil)
defer os.Remove(f.Name())
_, err = f.Write(rawenv)
c.Assert(err, check.IsNil)
err = f.Close()
c.Assert(err, check.IsNil)

// read in the temp file
env, err := ReadEnvironmentFile(f.Name())
c.Assert(err, check.IsNil)

// check we parsed it correctly
c.Assert(env, check.DeepEquals, []string{"foo=bar", "foo=bar=baz", "foo="})
}
2 changes: 2 additions & 0 deletions tool/teleport/common/teleport.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ func Run(cmdlineArgs []string, testRun bool) (executedCommand string, conf *serv
"Specify gops addr to listen on").Hidden().StringVar(&ccf.GopsAddr)
start.Flag("diag-addr",
"Start diangonstic endpoint on this address").Hidden().StringVar(&ccf.DiagnosticAddr)
start.Flag("permit-user-env",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make this a flag that accepts values with paths to read, defaulting to ~/.tsh/environment

"Enables reading of ~/.tsh/environment when creating a session").BoolVar(&ccf.PermitUserEnvironment)

// define start's usage info (we use kingpin's "alias" field for this)
start.Alias(usageNotes + usageExamples)
Expand Down