Forwarding Server #1482

russjones · 2017-11-29T00:20:21Z

Purpose

To support recording sessions with OpenSSH nodes Teleport needs to create a in-memory SSH server in the proxy that forwards connections to a remote host. This PR creates this server but does not use it yet.

Implementation

Consolidate handlers between the regular and forwarding server further in lib/srv/authhandlers.go.
Created new forwarding server in lib/srv/forward/sshserver.go.

Related Issues

Fixes #1481
Fixes #1486
Fixes #1487

russjones · 2017-11-29T00:30:44Z

lib/events/api.go

@@ -86,6 +86,11 @@ const (
 	ExecEventCode    = "exitCode"
 	ExecEventError   = "exitError"

+	// SubsystemEvent is the result of the execution of a subsystem.
+	SubsystemEvent = "subsystem"
+	SubsystemName  = "subsystem.name"


Should be name.

russjones · 2017-11-29T00:33:41Z

lib/srv/authhandlers.go

+
+		return nil
+	}
+


Debug log here saying that we are bypassing security checks.

How does this work when you can add a user CA, but not a host CA to a host?

russjones · 2017-11-29T00:44:33Z

lib/srv/authhandlers.go

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

+	// if it's a forwarding node checking permissions, then we don't do any RBAC


Instead create checkPermissionToLoginRBAC as well.

Or remove it because the below RBAC check is against the proxy itself.

russjones · 2017-11-29T00:49:54Z

lib/srv/ctx.go

+func (c *ServerContext) ProxyPublicAddress() string {
+	proxyHost := "<proxyhost>:3080"
+
+	if c.srv != nil {


Return here.

russjones · 2017-11-29T00:50:56Z

lib/srv/ctx.go

@@ -325,7 +363,8 @@ func closeAll(closers ...io.Closer) error {
 		if cl == nil {
 			continue
 		}
-		if e := cl.Close(); e != nil {
+
+		if e := cl.Close(); err != nil {


Revert and use trace.Aggregate.

russjones · 2017-11-29T01:47:18Z

lib/srv/forward/subsystem.go

+		serverContext: serverContext,
+		subsytemName:  subsytemName,
+		ctx:           ctx,
+		errorCh:       make(chan error),


Should have size 3.

russjones · 2017-11-29T01:48:46Z

lib/srv/regular/sshserver.go

+			// recording proxy mode.
+			err := s.handleAgentForwardProxy(req, ctx)
+			if err != nil {
+				log.Info(err)


russjones · 2017-11-29T01:50:57Z

lib/srv/regular/sshserver.go

 	}
-	err = s.reg.NotifyWinChange(*params, ctx)
+
+	// check if the users rbac role allows agent forwarding


check if the user's RBAC role allows agent forwarding

russjones · 2017-11-29T01:56:26Z

lib/srv/term.go

+	if t.termType == "" {
+		t.termType = defaultTerm
+	}
+	if err := t.session.RequestPty(t.termType, t.params.W, t.params.H, ssh.TerminalModes{}); err != nil {


See if we get ssh.TerminalModes in the request.

russjones · 2017-11-29T02:00:25Z

lib/srv/term.go

+		Wpx: uint32(w * 8),
+		Hpx: uint32(h * 8),
+	}
+	_, err := t.session.SendRequest("window-change", false, ssh.Marshal(&req))


klizhentas · 2017-11-30T02:56:33Z

lib/srv/forward/sshserver.go

+			}
+
+			// if we miss 3 in a row the connections dead, call cancel and cleanup
+			missed = missed + 1


why not missed += 1

russjones · 2017-12-01T20:12:32Z

@r0mant Can you take a look at lib/auth/init.go I made some changes to how ClusterID is set.

russjones · 2017-12-02T00:44:04Z

lib/srv/authhandlers.go

 // client) to see if this certificate can be allowed to login as user:login
 // pair to requested server.
-func (h *AuthHandlers) checkPermissionToLogin(cert *ssh.Certificate, clusterName string, teleportUser, osUser string) error {
-	h.Debugf("CheckPermsissionToLogin(%v, %v)", teleportUser, osUser)
+func (h *AuthHandlers) canLoginOpenSSH(cert *ssh.Certificate, clusterName string, teleportUser, osUser string) error {


canLoginWithoutRBAC

russjones · 2017-12-02T00:44:23Z

lib/srv/authhandlers.go

+// canLoginTeleport checks the given certificate (supplied by a connected
+// client) to see if this certificate can be allowed to login as user:login
+// pair to requested server and if RBAC rules allow login.
+func (h *AuthHandlers) canLoginTeleport(cert *ssh.Certificate, clusterName string, teleportUser, osUser string) error {


canLoginWithRBAC

russjones · 2017-12-02T00:56:37Z

lib/srv/forward/sshserver.go

+	}
+	ctx.Debugf("Opening direct-tcpip channel from %v to %v", srcAddr, dstAddr)
+
+	conn, err := s.remoteClient.DialTCP("tcp", srcAddr, dstAddr)


russjones · 2017-12-02T00:58:06Z

lib/srv/forward/sshserver.go

+// client. A manual timeout is needed here because SendRequest will wait for a
+// response forever.
+func (s *Server) sendKeepAliveWithTimeout(timeout time.Duration) bool {
+	errorCh := make(chan error)


errorCh := make(chan error, 1)

russjones · 2017-12-02T01:14:34Z

lib/auth/init.go

 	// AuditLog is used for emitting events to audit log
 	AuditLog events.IAuditLog
+
+	// SessionRecording determines where the session is recorded: node, proxy, or off.
+	SessionRecording string


Move to lib/config/configuration.go

russjones · 2017-12-02T01:17:47Z

lib/services/clusterconfig.go

+const (
+	// HostKeyCheckOn is the default. The proxy will check the host key of the
+	// target node it connects to.
+	HostKeyCheckOn HostKeyCheckType = "on"


russjones · 2017-12-02T01:19:24Z

lib/services/clusterconfig.go

 	RecordAtProxy RecordingType = "proxy"

 	// RecordOff is used to disable session recording completely.
 	RecordOff RecordingType = "off"
 )

+// HostKeyCheckType holds how the recording proxy will check host keys.
+type HostKeyCheckType string


russjones · 2017-12-02T01:21:33Z

lib/services/clusterconfig.go

@@ -218,6 +264,9 @@ const ClusterConfigSpecSchemaTemplate = `{
    "session_recording": {
      "type": "string"
    },
+    "proxy_checks_host_keys": {
+      "type": "string"
+	},


russjones · 2017-12-02T01:43:48Z

lib/srv/authhandlers.go

+// CheckPortForward checks if the certificate was signed by a valid certificate
+// authority, fetches the appropriate roles, and checks if port forwarding is
+// allowed.
+func (h *AuthHandlers) CheckPortForward(ctx *ServerContext) error {


Get CA set and RoleSet and pass identity earlier.

russjones · 2017-12-02T01:46:09Z

lib/srv/authhandlers.go

-	// a valid public key
+	// if we are strictly checking host keys then reject this request right away
+	if clusterConfig.GetProxyChecksHostKeys() == services.HostKeyCheckOn {
+		return trace.AccessDenied("certificate not presented")


proxy expects certificate signed by certificate authority, got public key

russjones · 2017-12-02T01:46:52Z

lib/srv/authhandlers.go

+
+	// if we are not stricting rejecting host keys, we need to log that we
+	// trusted a insecure key and then return nil
+	log.Infof("Strict host key checking disabled, allowing login without checking host key.")


Insecure warning prefix.

russjones · 2017-12-02T01:47:44Z

lib/srv/forward/sshserver.go

+	// check if the role allows port forwarding for this user
+	err := s.authHandlers.CheckPortForward(ctx)
+	if err != nil {
+		s.log.Infof(err.Error())


Emit Audit Log.

russjones · 2017-12-02T01:48:35Z

lib/srv/regular/sshserver.go

+	// check if the role allows port forwarding for this user
+	err := s.authHandlers.CheckPortForward(ctx)
+	if err != nil {
+		log.Info(err.Error())


russjones · 2017-12-05T00:08:43Z

lib/srv/authhandlers.go

+			events.LocalAddr:          ctx.Conn.LocalAddr().String(),
+			events.RemoteAddr:         ctx.Conn.RemoteAddr().String(),
+		})
+		h.Warnf("Port forwarding request denied: %v", systemErrorMessage)


* Group all modal stories under ModalsHost * Fix type of DialogClusterConnect, rename DialogBase to DialogNone

russjones requested review from klizhentas, kontsevoy and r0mant November 29, 2017 00:20

russjones commented Nov 29, 2017

View reviewed changes

klizhentas reviewed Nov 30, 2017

View reviewed changes

russjones force-pushed the rjones/fwd-srv branch from d8f0ab4 to 5f45532 Compare December 1, 2017 20:08

russjones commented Dec 2, 2017

View reviewed changes

russjones commented Dec 5, 2017

View reviewed changes

russjones force-pushed the rjones/fwd-srv branch from 99240ee to f0db0a0 Compare December 5, 2017 00:53

Added forwarding SSH server.

7018852

russjones force-pushed the rjones/fwd-srv branch from f0db0a0 to 7018852 Compare December 5, 2017 01:02

klizhentas approved these changes Dec 5, 2017

View reviewed changes

russjones merged commit e870a73 into master Dec 5, 2017

russjones deleted the rjones/fwd-srv branch December 5, 2017 01:33

hatched pushed a commit that referenced this pull request Jan 30, 2023

Connect: Add missing modal stories, misc modal fixes (#1479) (#1482)

c716408

* Group all modal stories under ModalsHost * Fix type of DialogClusterConnect, rename DialogBase to DialogNone

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Forwarding Server #1482

Forwarding Server #1482

russjones commented Nov 29, 2017 •

edited

Loading

russjones Nov 29, 2017

russjones Nov 29, 2017

russjones Nov 29, 2017

russjones Nov 29, 2017

russjones Nov 29, 2017

russjones Nov 29, 2017

russjones Nov 29, 2017

russjones Nov 29, 2017

russjones Nov 29, 2017

russjones Nov 29, 2017

russjones Nov 29, 2017

klizhentas Nov 30, 2017

russjones commented Dec 1, 2017

russjones Dec 2, 2017

russjones Dec 2, 2017

russjones Dec 2, 2017

russjones Dec 2, 2017

russjones Dec 2, 2017

russjones Dec 2, 2017

russjones Dec 2, 2017

russjones Dec 2, 2017

russjones Dec 2, 2017

russjones Dec 2, 2017

russjones Dec 2, 2017

russjones Dec 2, 2017

russjones Dec 2, 2017

russjones Dec 2, 2017

russjones Dec 2, 2017

russjones Dec 5, 2017

Forwarding Server #1482

Forwarding Server #1482

Conversation

russjones commented Nov 29, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

russjones commented Dec 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

russjones commented Nov 29, 2017 •

edited

Loading