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

Several 2.0 regression fixes #876

Merged
merged 9 commits into from
Mar 30, 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
2 changes: 1 addition & 1 deletion build.assets/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,4 @@ enter: bbox
#
.PHONY:release
release:
docker run $(DOCKERFLAGS) -i $(NOROOT) $(BBOX) /usr/bin/make release -e VERSION="$(VERSION)" -e ADDFLAGS="$(ADDFLAGS)"
docker run $(DOCKERFLAGS) -i $(NOROOT) $(BBOX) /usr/bin/make release -e ADDFLAGS="$(ADDFLAGS)"
5 changes: 4 additions & 1 deletion docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@ FROM teleport-buildbox:latest
ENV DEBUG=1 GOPATH=/root/go PATH=$PATH:/root/go/src/github.com/gravitational/teleport/build

# htop is useful for testing terminal resizing
RUN apt-get install -y htop screen; \
RUN apt-get install -y htop vim screen; \
mkdir -p /root/go/src/github.com/gravitational/teleport

# allows ansible testing
RUN apt-get install -y ansible

VOLUME ["/teleport", "/var/lib/teleport"]
COPY .bashrc /root/.bashrc
COPY .screenrc /root/.screenrc
Expand Down
59 changes: 59 additions & 0 deletions docker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,65 @@ To setup Trusted Clusters:
tctl -c /root/go/src/github.com/gravitational/teleport/docker/two-auth.yaml create -f docker/two-role-admin.yaml
tctl -c /root/go/src/github.com/gravitational/teleport/docker/two-auth.yaml create -f docker/two-tc.yaml
```

### Ansible

To setup Ansible:

1. Follow steps in Trusted Cluster section to setup Trusted Clusters.
1. Use `tctl` to issue create user command and follow link on screen to create user.

```bash
tctl users add {username} root
```
1. Configure Ansible.

```bash
# add two-node to ansible hosts file
echo "172.10.1.2:3022" >> /etc/ansible/hosts

# setup ssh_args that ansible will use to access trusted cluster nodes
sed -i '/ssh_args = -o ControlMaster=auto -o ControlPersist=60s/assh_args = -o "ProxyCommand ssh -p 3023 one -s proxy:%h:%p@two"' /etc/ansible/ansible.cfg

# use scp over sftp
sed -i '/scp_if_ssh/s/^#//g' /etc/ansible/ansible.cfg
```

1. Start and load OpenSSH agent with keys.

```bash
# create directory for ssh config
mkdir ~/.ssh && chmod 700 ~/.ssh

# start ssh-agent
eval `ssh-agent`

# log in with the user created before
tsh --proxy=localhost --user=rjones login

# load keys into ssh-agent
tsh --proxy=localhost --user=rjones agent --load
```

1. Verify Ansible works:

```bash
$ ansible all -m ping
172.10.1.2 | success >> {
"changed": false,
"ping": "pong"
}
```

1. Run an simple playbook:

```bash
# cd to directory that contains playbook
cd /root/go/src/github.com/gravitational/teleport/docker/ansible

# run playbook
ansible-playbook playbook.yaml
```

### Interactive Usage

Expand Down
26 changes: 26 additions & 0 deletions docker/ansible/playbook.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
- hosts: all
tasks:
- name: "simple copy"
copy:
src: /root/go/src/github.com/gravitational/teleport/docker/ansible/simple.txt
dest: /tmp/simple.txt.out
owner: root
group: root
mode: 0644

- name: recursive copy
copy:
src: /root/go/src/github.com/gravitational/teleport/docker/ansible/rdir
dest: /tmp"
owner: root
group: root
mode: 0644

- name: simple template
template:
src: /root/go/src/github.com/gravitational/teleport/docker/ansible/template.j2
dest: /tmp/template.out
owner: root
group: root
mode: 0644
1 change: 1 addition & 0 deletions docker/ansible/rdir/rdir/rdir
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
rdir
1 change: 1 addition & 0 deletions docker/ansible/simple.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
hello, world
2 changes: 2 additions & 0 deletions docker/ansible/template.j2
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
template_host: {{ template_host }}
template_uid: {{ template_uid }}
13 changes: 2 additions & 11 deletions lib/client/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ type Config struct {
// HostLogin is a user login on a remote host
HostLogin string

// HostPort is a remote host port to connect to
// HostPort is a remote host port to connect to. This is used for **explicit**
// port setting via -p flag, otherwise '0' is passed which means "use server default"
HostPort int

// ProxyHostPort is a host or IP of the proxy (with optional ":ssh_port,https_port").
Expand Down Expand Up @@ -276,16 +277,6 @@ func (c *Config) ProxySSHPort() (retval int) {
return retval
}

// NodeHostPort returns host:port string based on user supplied data
// either if user has set host:port in the connection string,
// or supplied the -p flag. If user has set both, -p flag data is ignored
func (c *Config) NodeHostPort() string {
if strings.Contains(c.Host, ":") {
return c.Host
}
return net.JoinHostPort(c.Host, strconv.Itoa(c.HostPort))
}

// ProxySpecified returns true if proxy has been specified
func (c *Config) ProxySpecified() bool {
return len(c.ProxyHostPort) > 0
Expand Down
1 change: 0 additions & 1 deletion lib/client/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ func (s *APITestSuite) TestNew(c *check.C) {

la := tc.LocalAgent()
c.Assert(la, check.NotNil)
c.Assert(tc.NodeHostPort(), check.Equals, "localhost:22")
}

func (s *APITestSuite) TestParseLabels(c *check.C) {
Expand Down
13 changes: 12 additions & 1 deletion lib/config/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"golang.org/x/crypto/ssh"

"github.com/gravitational/teleport"
"github.com/gravitational/teleport/lib/backend"
"github.com/gravitational/teleport/lib/client"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/limiter"
Expand Down Expand Up @@ -191,6 +192,10 @@ func ApplyFileConfig(fc *FileConfig, cfg *service.Config) error {
return trace.Errorf("unsupported logger severity: '%v'", fc.Logger.Severity)
}

if strings.ToLower(fc.Logger.Output) == "syslog" {
utils.SwitchLoggingtoSyslog()
}

// apply connection throttling:
limiters := []limiter.LimiterConfig{
cfg.SSH.Limiter,
Expand Down Expand Up @@ -545,7 +550,7 @@ func Configure(clf *CommandLineFlags, cfg *service.Config) error {
}

cfg.Console = ioutil.Discard
utils.InitLoggerDebug()
utils.InitLogger(utils.LoggingForDaemon, log.DebugLevel)
}

// apply --roles flag:
Expand Down Expand Up @@ -608,6 +613,12 @@ func Configure(clf *CommandLineFlags, cfg *service.Config) error {
cfg.AuthServers = append(cfg.AuthServers, cfg.Auth.SSHAddr)
}

// add data_dir to the backend config:
if cfg.Auth.StorageConfig.Params == nil {
cfg.Auth.StorageConfig.Params = backend.Params{}
}
cfg.Auth.StorageConfig.Params["data_dir"] = cfg.DataDir
Copy link
Contributor

Choose a reason for hiding this comment

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

@russjones this overwrites the user-supplied value at all times, what should not be the case. We should only overwrite the value if user haven't supplied the value. You can fix that yourself as @kontsevoy is OOO at the moment.

Copy link
Contributor Author

@kontsevoy kontsevoy Mar 28, 2017

Choose a reason for hiding this comment

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

@klizhentas there are no user-supplied values in StorageConfig.Params [1]. Storage config is a static, backend-specific YAML configuration, with "data_dir" being a reserved word. In fact, we guarantee the presence of "data_dir" in .Params (and back-end code expects it to be there) This needs to be reflected in backend developer README. In other words, this behavior is as-designed.

[1] Bolt used to accept an arbitrary JSON string, so that's why you think there are user-supplied values. This is no longer true since the Great Christmas-2016 Back-end Redesign

Copy link
Contributor

Choose a reason for hiding this comment

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

I think users can supply Params["path"] for backend, at least what I have observed in configs and the code, so I don't quite understand the fix then. Why is it setting "data_dir" and not "path" like for example "boltdb" requires.

Copy link
Contributor

Choose a reason for hiding this comment

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

My question basically is - can you explain to me how does this change fixes the case reported here:

#867

Copy link
Contributor

Choose a reason for hiding this comment

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

@russjones: @kontsevoy explained to me how it works, so this part lgtm, please fix the part with socket path and merge the whole thing


return nil
}

Expand Down
4 changes: 2 additions & 2 deletions lib/services/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,8 @@ func (r *RoleV2) CheckAndSetDefaults() error {
}

func (r *RoleV2) String() string {
return fmt.Sprintf("Role(Name=%v,MaxSessionTTL=%v,Logins=%v,NodeLabels=%v,Namespaces=%v,Resources=%v)",
r.GetName(), r.GetMaxSessionTTL(), r.GetLogins(), r.GetNodeLabels(), r.GetNamespaces(), r.GetResources())
return fmt.Sprintf("Role(Name=%v,MaxSessionTTL=%v,Logins=%v,NodeLabels=%v,Namespaces=%v,Resources=%v,CanForwardAgent=%v)",
r.GetName(), r.GetMaxSessionTTL(), r.GetLogins(), r.GetNodeLabels(), r.GetNamespaces(), r.GetResources(), r.CanForwardAgent())
}

// RoleSpecV2 is role specification for RoleV2
Expand Down
17 changes: 16 additions & 1 deletion lib/srv/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ package srv
import (
"fmt"
"net"
"os"
"os/user"
"path"
"path/filepath"

"gopkg.in/check.v1"

Expand Down Expand Up @@ -81,7 +84,7 @@ func (s *ExecSuite) TestOSCommandPrep(c *check.C) {
cmd, err = prepareCommand(s.ctx)
c.Assert(err, check.IsNil)
c.Assert(cmd, check.NotNil)
c.Assert(cmd.Path, check.Equals, "/bin/ls")
c.Assert(cmd.Path, check.Equals, findExecutable("ls"))
c.Assert(cmd.Args, check.DeepEquals, []string{"ls", "-lh", "/etc"})
c.Assert(cmd.Dir, check.Equals, s.usr.HomeDir)
c.Assert(cmd.Env, check.DeepEquals, expectedEnv)
Expand Down Expand Up @@ -112,3 +115,15 @@ func (s *ExecSuite) OpenChannel(string, []byte) (ssh.Channel, <-chan *ssh.Reques
return nil, nil, nil
}
func (s *ExecSuite) Wait() error { return nil }

// findExecutable helper finds a given executable name (like 'ls') in $PATH
// and returns the full path
func findExecutable(execName string) string {
for _, dir := range filepath.SplitList(os.Getenv("PATH")) {
fp := path.Join(dir, execName)
if utils.IsFile(fp) {
return fp
}
}
return "not found in $PATH: " + execName
}
6 changes: 5 additions & 1 deletion lib/srv/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,9 @@ func (t *proxySubsys) proxyToHost(
// if port is 0, it means the client wants us to figure out
// which port to use
useExactPort := len(t.port) > 0 && t.port != "0"
ips, _ := net.LookupHost(t.host)
log.Debugf("proxy connecting to host=%v port=%v, exact port=%v\n", t.host, t.port, useExactPort)

ips, err := net.LookupHost(t.host)

// enumerate and try to find a server with self-registered with a matching name/IP:
var server services.Server
Expand All @@ -257,6 +259,7 @@ func (t *proxySubsys) proxyToHost(
log.Error(err)
continue
}

if t.host == ip || t.host == servers[i].GetHostname() || utils.SliceContainsStr(ips, ip) {
server = servers[i]
// found the server. see if we need to match the port
Expand All @@ -275,6 +278,7 @@ func (t *proxySubsys) proxyToHost(
t.port = strconv.Itoa(defaults.SSHServerListenPort)
}
serverAddr = net.JoinHostPort(t.host, t.port)
log.Warnf("server lookup failed: using default=%v", serverAddr)
}

// we must dial by server IP address because hostname
Expand Down
10 changes: 7 additions & 3 deletions lib/srv/sshserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package srv
import (
"fmt"
"io"
"io/ioutil"
"net"
"os"
"os/exec"
Expand All @@ -42,7 +43,6 @@ import (
"github.com/gravitational/teleport/lib/sshutils"
"github.com/gravitational/teleport/lib/teleagent"
"github.com/gravitational/teleport/lib/utils"
"github.com/pborman/uuid"

log "github.com/Sirupsen/logrus"
"github.com/gravitational/trace"
Expand Down Expand Up @@ -872,13 +872,17 @@ func (s *Server) handleAgentForward(ch ssh.Channel, req *ssh.Request, ctx *ctx)

authChan, _, err := ctx.conn.OpenChannel("[email protected]", nil)
if err != nil {
return err
return trace.Wrap(err)
}
clientAgent := agent.NewClient(authChan)
ctx.setAgent(clientAgent, authChan)

pid := os.Getpid()
socketPath := filepath.Join(os.TempDir(), fmt.Sprintf("teleport-agent-%v.socket", uuid.New()))
socketDir, err := ioutil.TempDir(os.TempDir(), "teleport-")
if err != nil {
return trace.Wrap(err)
}
socketPath := filepath.Join(socketDir, fmt.Sprintf("teleport-%v.socket", pid))

agentServer := &teleagent.AgentServer{Agent: clientAgent}
err = agentServer.ListenUnixSocket(socketPath, uid, gid, 0600)
Expand Down
16 changes: 12 additions & 4 deletions lib/srv/sshserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ import (
"fmt"
"io"
"net"
"os"
"os/user"
"path/filepath"
"regexp"
"strings"
"testing"
Expand Down Expand Up @@ -238,13 +240,16 @@ func (s *SrvSuite) TestAgentForward(c *C) {
_, err = io.WriteString(writer, fmt.Sprintf("printenv %v\n\r", teleport.SSHAuthSock))
c.Assert(err, IsNil)

re := regexp.MustCompile(`/tmp/[^\s]+`)
pattern := filepath.Join(os.TempDir(), `teleport-[0-9]+`, `teleport-[0-9]+.socket`)
re := regexp.MustCompile(pattern)
buf := make([]byte, 4096)
result := make([]byte, 0)
var matches []string
for i := 0; i < 3; i++ {
_, err = reader.Read(buf)
n, err := reader.Read(buf)
c.Assert(err, IsNil)
matches = re.FindStringSubmatch(string(buf))
result = append(result, buf[0:n]...)
matches = re.FindStringSubmatch(string(result))
if len(matches) != 0 {
break
}
Expand Down Expand Up @@ -688,8 +693,11 @@ func (s *SrvSuite) TestPTY(c *C) {
c.Assert(err, IsNil)
defer se.Close()

// request PTY
// request PTY with valid size
c.Assert(se.RequestPty("xterm", 30, 30, ssh.TerminalModes{}), IsNil)

// request PTY with invalid size, should still work (selects defaults)
c.Assert(se.RequestPty("xterm", 0, 0, ssh.TerminalModes{}), IsNil)
}

// TestEnv requests setting environment variables. (We are currently ignoring these requests)
Expand Down
7 changes: 6 additions & 1 deletion lib/srv/term.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,12 @@ func requestPTY(req *ssh.Request) (*terminal, *rsession.TerminalParams, error) {
log.Warnf("failed to parse PTY request: %v", err)
return nil, nil, trace.Wrap(err)
}
log.Debugf("Parsed pty request pty(enn=%v, w=%v, h=%v)", r.Env, r.W, r.H)
err := r.CheckAndSetDefaults()
if err != nil {
return nil, nil, trace.Wrap(err)
}
log.Debugf("Parsed pty request pty(env=%v, w=%v, h=%v)", r.Env, r.W, r.H)

t, err := newTerminal()
if err != nil {
log.Warnf("failed to create term: %v", err)
Expand Down
Loading