[ARVADOS] updated: 2.1.0-262-g5d05dd3ef

Git user git at public.arvados.org
Wed Jan 13 22:18:55 UTC 2021


Summary of changes:
 cmd/arvados-client/container_gateway.go          |  32 +++---
 cmd/arvados-client/container_gateway_test.go     |  26 +++++
 lib/controller/localdb/container_gateway.go      |  82 +++++++++++++--
 lib/controller/localdb/container_gateway_test.go | 123 +++++++++++++++++++++++
 lib/controller/rpc/conn.go                       |  39 +++++--
 lib/crunchrun/container_gateway.go               |  75 ++++++++++----
 lib/crunchrun/crunchrun.go                       |  17 ++--
 lib/dispatchcloud/worker/pool.go                 |   2 +-
 lib/selfsigned/cert.go                           |  76 ++++++++++++++
 lib/selfsigned/cert_test.go                      |  26 +++++
 sdk/go/arvados/api.go                            |   1 +
 11 files changed, 437 insertions(+), 62 deletions(-)
 create mode 100644 cmd/arvados-client/container_gateway_test.go
 create mode 100644 lib/controller/localdb/container_gateway_test.go
 create mode 100644 lib/selfsigned/cert.go
 create mode 100644 lib/selfsigned/cert_test.go

       via  5d05dd3ef4822dfb3b476777ba1aacaafed8278d (commit)
       via  bdd9b68e133bb3b3ef7914893ad15f311ce2de24 (commit)
       via  0a6c5d8a8758df0395e40b3cf1d125e59d6c88dd (commit)
       via  5c34cfad641b76ffaa43be8234af1da9d5736ec2 (commit)
       via  03141ccc3015a04f2f171b5a532a9dfd57b8bcd6 (commit)
       via  a2c539196d3b76b466039dfd00e7595ec9dd6abc (commit)
       via  4cc07d6ab4ae16bd6bc69b2c8022414d66ee4113 (commit)
       via  6be5fe90dc9a5452b432176bfa83cba83070f8cf (commit)
       via  fed1d9f5e7a442abb6b2e86114e8c3d05cff5329 (commit)
       via  2c3418d59cd27806322e07519ef9483c2cb433c2 (commit)
      from  0a39317d5ec49ad439833df0a70965394cafb6e8 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 5d05dd3ef4822dfb3b476777ba1aacaafed8278d
Author: Tom Clegg <tom at curii.com>
Date:   Wed Jan 13 17:18:08 2021 -0500

    17170: "arvados-client shell" accepts container request UUID.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/cmd/arvados-client/container_gateway.go b/cmd/arvados-client/container_gateway.go
index b5fde6cf1..1ed6dc279 100644
--- a/cmd/arvados-client/container_gateway.go
+++ b/cmd/arvados-client/container_gateway.go
@@ -125,18 +125,24 @@ Options:
 		func(context.Context) ([]string, error) {
 			return []string{os.Getenv("ARVADOS_API_TOKEN")}, nil
 		})
-	// if strings.Contains(targetUUID, "-xvhdp-") {
-	// 	cr, err := rpcconn.ContainerRequestGet(context.TODO(), arvados.GetOptions{UUID: targetUUID})
-	// 	if err != nil {
-	// 		fmt.Fprintln(stderr, err)
-	// 		return 1
-	// 	}
-	// 	if cr.ContainerUUID == "" {
-	// 		fmt.Fprintf(stderr, "no container assigned, container request state is %s\n", strings.ToLower(cr.State))
-	// 		return 1
-	// 	}
-	// 	targetUUID = cr.ContainerUUID
-	// }
+	if strings.Contains(targetUUID, "-xvhdp-") {
+		crs, err := rpcconn.ContainerRequestList(context.TODO(), arvados.ListOptions{Limit: -1, Filters: []arvados.Filter{{"uuid", "=", targetUUID}}})
+		if err != nil {
+			fmt.Fprintln(stderr, err)
+			return 1
+		}
+		if len(crs.Items) < 1 {
+			fmt.Fprintf(stderr, "container request %q not found\n", targetUUID)
+			return 1
+		}
+		cr := crs.Items[0]
+		if cr.ContainerUUID == "" {
+			fmt.Fprintf(stderr, "no container assigned, container request state is %s\n", strings.ToLower(string(cr.State)))
+			return 1
+		}
+		targetUUID = cr.ContainerUUID
+		fmt.Fprintln(stderr, "connecting to container", targetUUID)
+	}
 	sshconn, err := rpcconn.ContainerSSH(context.TODO(), arvados.ContainerSSHOptions{
 		UUID:          targetUUID,
 		DetachKeys:    *detachKeys,
diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go
index a9a638759..5fecf662f 100644
--- a/lib/controller/rpc/conn.go
+++ b/lib/controller/rpc/conn.go
@@ -361,6 +361,13 @@ func (conn *Conn) ContainerSSH(ctx context.Context, options arvados.ContainerSSH
 	return
 }
 
+func (conn *Conn) ContainerRequestList(ctx context.Context, options arvados.ListOptions) (arvados.ContainerRequestList, error) {
+	ep := arvados.EndpointContainerRequestList
+	var resp arvados.ContainerRequestList
+	err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+	return resp, err
+}
+
 func (conn *Conn) SpecimenCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Specimen, error) {
 	ep := arvados.EndpointSpecimenCreate
 	var resp arvados.Specimen
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index e8baa01b4..4675906e7 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -46,6 +46,7 @@ var (
 	EndpointContainerLock                 = APIEndpoint{"POST", "arvados/v1/containers/{uuid}/lock", ""}
 	EndpointContainerUnlock               = APIEndpoint{"POST", "arvados/v1/containers/{uuid}/unlock", ""}
 	EndpointContainerSSH                  = APIEndpoint{"GET", "arvados/v1/connect/{uuid}/ssh", ""} // move to /containers after #17014 fixes routing
+	EndpointContainerRequestList          = APIEndpoint{"GET", "arvados/v1/container_requests", ""}
 	EndpointUserActivate                  = APIEndpoint{"POST", "arvados/v1/users/{uuid}/activate", ""}
 	EndpointUserCreate                    = APIEndpoint{"POST", "arvados/v1/users", "user"}
 	EndpointUserCurrent                   = APIEndpoint{"GET", "arvados/v1/users/current", ""}

commit bdd9b68e133bb3b3ef7914893ad15f311ce2de24
Author: Tom Clegg <tom at curii.com>
Date:   Wed Jan 13 17:17:50 2021 -0500

    17170: Improve error messages.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go
index 48e67e274..a9a638759 100644
--- a/lib/controller/rpc/conn.go
+++ b/lib/controller/rpc/conn.go
@@ -313,6 +313,7 @@ func (conn *Conn) ContainerSSH(ctx context.Context, options arvados.ContainerSSH
 
 	u, err := conn.baseURL.Parse("/" + strings.Replace(arvados.EndpointContainerSSH.Path, "{uuid}", options.UUID, -1))
 	if err != nil {
+		err = fmt.Errorf("tls.Dial: %w", err)
 		return
 	}
 	u.RawQuery = url.Values{
@@ -334,12 +335,20 @@ func (conn *Conn) ContainerSSH(ctx context.Context, options arvados.ContainerSSH
 	bufw.Flush()
 	resp, err := http.ReadResponse(bufr, &http.Request{Method: "GET"})
 	if err != nil {
+		err = fmt.Errorf("http.ReadResponse: %w", err)
 		return
 	}
 	if resp.StatusCode != http.StatusSwitchingProtocols {
 		defer resp.Body.Close()
 		body, _ := ioutil.ReadAll(resp.Body)
-		err = fmt.Errorf("server did not provide a tunnel: %d %q", resp.StatusCode, body)
+		var message string
+		var errDoc httpserver.ErrorResponse
+		if err := json.Unmarshal(body, &errDoc); err != nil {
+			message = strings.Join(errDoc.Errors, "; ")
+		} else {
+			message = fmt.Sprintf("%q", body)
+		}
+		err = fmt.Errorf("server did not provide a tunnel: %q (HTTP %d)", message, resp.StatusCode)
 		return
 	}
 	if strings.ToLower(resp.Header.Get("Upgrade")) != "ssh" ||

commit 0a6c5d8a8758df0395e40b3cf1d125e59d6c88dd
Author: Tom Clegg <tom at curii.com>
Date:   Wed Jan 13 17:06:57 2021 -0500

    17170: Don't reuse transport that might have http2 enabled.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go
index 7dd89452b..48e67e274 100644
--- a/lib/controller/rpc/conn.go
+++ b/lib/controller/rpc/conn.go
@@ -298,8 +298,9 @@ func (conn *Conn) ContainerSSH(ctx context.Context, options arvados.ContainerSSH
 		// hostname or ::1 or 1::1
 		addr = net.JoinHostPort(addr, "https")
 	}
-	netconn, err := tls.Dial("tcp", addr, conn.httpClient.Transport.(*http.Transport).TLSClientConfig)
+	netconn, err := tls.Dial("tcp", addr, &tls.Config{InsecureSkipVerify: conn.httpClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify})
 	if err != nil {
+		err = fmt.Errorf("tls.Dial: %w", err)
 		return
 	}
 	defer func() {

commit 5c34cfad641b76ffaa43be8234af1da9d5736ec2
Author: Tom Clegg <tom at curii.com>
Date:   Wed Jan 13 17:06:11 2021 -0500

    17170: Dry up close-connection-on-error.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go
index 2accfd8f2..7dd89452b 100644
--- a/lib/controller/rpc/conn.go
+++ b/lib/controller/rpc/conn.go
@@ -302,12 +302,16 @@ func (conn *Conn) ContainerSSH(ctx context.Context, options arvados.ContainerSSH
 	if err != nil {
 		return
 	}
+	defer func() {
+		if err != nil {
+			netconn.Close()
+		}
+	}()
 	bufr := bufio.NewReader(netconn)
 	bufw := bufio.NewWriter(netconn)
 
 	u, err := conn.baseURL.Parse("/" + strings.Replace(arvados.EndpointContainerSSH.Path, "{uuid}", options.UUID, -1))
 	if err != nil {
-		netconn.Close()
 		return
 	}
 	u.RawQuery = url.Values{
@@ -316,11 +320,9 @@ func (conn *Conn) ContainerSSH(ctx context.Context, options arvados.ContainerSSH
 	}.Encode()
 	tokens, err := conn.tokenProvider(ctx)
 	if err != nil {
-		netconn.Close()
 		return
 	} else if len(tokens) < 1 {
 		err = httpserver.ErrorWithStatus(errors.New("unauthorized"), http.StatusUnauthorized)
-		netconn.Close()
 		return
 	}
 	bufw.WriteString("GET " + u.String() + " HTTP/1.1\r\n")
@@ -331,20 +333,17 @@ func (conn *Conn) ContainerSSH(ctx context.Context, options arvados.ContainerSSH
 	bufw.Flush()
 	resp, err := http.ReadResponse(bufr, &http.Request{Method: "GET"})
 	if err != nil {
-		netconn.Close()
 		return
 	}
 	if resp.StatusCode != http.StatusSwitchingProtocols {
 		defer resp.Body.Close()
 		body, _ := ioutil.ReadAll(resp.Body)
 		err = fmt.Errorf("server did not provide a tunnel: %d %q", resp.StatusCode, body)
-		netconn.Close()
 		return
 	}
 	if strings.ToLower(resp.Header.Get("Upgrade")) != "ssh" ||
 		strings.ToLower(resp.Header.Get("Connection")) != "upgrade" {
 		err = fmt.Errorf("bad response from server: Upgrade %q Connection %q", resp.Header.Get("Upgrade"), resp.Header.Get("Connection"))
-		netconn.Close()
 		return
 	}
 	sshconn.Conn = netconn

commit 03141ccc3015a04f2f171b5a532a9dfd57b8bcd6
Author: Tom Clegg <tom at curii.com>
Date:   Wed Jan 13 17:03:40 2021 -0500

    17170: Allow shell only if this user submitted all associated CRs.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/localdb/container_gateway.go b/lib/controller/localdb/container_gateway.go
index f6eaea6d9..ff3e3de5b 100644
--- a/lib/controller/localdb/container_gateway.go
+++ b/lib/controller/localdb/container_gateway.go
@@ -18,6 +18,7 @@ import (
 	"strings"
 
 	"git.arvados.org/arvados.git/sdk/go/arvados"
+	"git.arvados.org/arvados.git/sdk/go/auth"
 	"git.arvados.org/arvados.git/sdk/go/ctxlog"
 	"git.arvados.org/arvados.git/sdk/go/httpserver"
 )
@@ -29,6 +30,26 @@ import (
 // If the returned error is nil, the caller is responsible for closing
 // sshconn.Conn.
 func (conn *Conn) ContainerSSH(ctx context.Context, opts arvados.ContainerSSHOptions) (sshconn arvados.ContainerSSHConnection, err error) {
+	user, err := conn.railsProxy.UserGetCurrent(ctx, arvados.GetOptions{})
+	if err != nil {
+		return
+	}
+	ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}})
+	crs, err := conn.railsProxy.ContainerRequestList(ctxRoot, arvados.ListOptions{Limit: -1, Filters: []arvados.Filter{{"container_uuid", "=", opts.UUID}}})
+	if err != nil {
+		return
+	}
+	for _, cr := range crs.Items {
+		if cr.ModifiedByUserUUID != user.UUID {
+			err = httpserver.ErrorWithStatus(errors.New("permission denied: container is associated with requests submitted by other users"), http.StatusForbidden)
+			return
+		}
+	}
+	if crs.ItemsAvailable != len(crs.Items) {
+		err = httpserver.ErrorWithStatus(errors.New("incomplete response while checking permission"), http.StatusInternalServerError)
+		return
+	}
+
 	ctr, err := conn.railsProxy.ContainerGet(ctx, arvados.GetOptions{UUID: opts.UUID})
 	if err != nil {
 		return

commit a2c539196d3b76b466039dfd00e7595ec9dd6abc
Author: Tom Clegg <tom at curii.com>
Date:   Wed Jan 13 00:13:48 2021 -0500

    17170: Test arvados-client shell command.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/cmd/arvados-client/container_gateway.go b/cmd/arvados-client/container_gateway.go
index d15b9d7b5..b5fde6cf1 100644
--- a/cmd/arvados-client/container_gateway.go
+++ b/cmd/arvados-client/container_gateway.go
@@ -143,7 +143,7 @@ Options:
 		LoginUsername: loginUsername,
 	})
 	if err != nil {
-		fmt.Fprintln(stderr, err)
+		fmt.Fprintln(stderr, "error setting up tunnel:", err)
 		return 1
 	}
 	defer sshconn.Conn.Close()
diff --git a/cmd/arvados-client/container_gateway_test.go b/cmd/arvados-client/container_gateway_test.go
new file mode 100644
index 000000000..deff7b170
--- /dev/null
+++ b/cmd/arvados-client/container_gateway_test.go
@@ -0,0 +1,26 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package main
+
+import (
+	"bytes"
+	"os"
+	"os/exec"
+
+	"git.arvados.org/arvados.git/sdk/go/arvadostest"
+	check "gopkg.in/check.v1"
+)
+
+func (s *ClientSuite) TestShellGatewayNotAvailable(c *check.C) {
+	var stdout, stderr bytes.Buffer
+	cmd := exec.Command("go", "run", ".", "shell", arvadostest.QueuedContainerUUID, "-o", "controlpath=none", "echo", "ok")
+	cmd.Env = append(cmd.Env, os.Environ()...)
+	cmd.Env = append(cmd.Env, "ARVADOS_API_TOKEN="+arvadostest.ActiveTokenV2)
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+	c.Check(cmd.Run(), check.NotNil)
+	c.Log(stderr.String())
+	c.Check(stderr.String(), check.Matches, `(?ms).*gateway is not available, container is queued.*`)
+}
diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go
index 9b2695b55..2accfd8f2 100644
--- a/lib/controller/rpc/conn.go
+++ b/lib/controller/rpc/conn.go
@@ -337,13 +337,13 @@ func (conn *Conn) ContainerSSH(ctx context.Context, options arvados.ContainerSSH
 	if resp.StatusCode != http.StatusSwitchingProtocols {
 		defer resp.Body.Close()
 		body, _ := ioutil.ReadAll(resp.Body)
-		err = fmt.Errorf("tunnel connection failed: %d %q", resp.StatusCode, body)
+		err = fmt.Errorf("server did not provide a tunnel: %d %q", resp.StatusCode, body)
 		netconn.Close()
 		return
 	}
 	if strings.ToLower(resp.Header.Get("Upgrade")) != "ssh" ||
 		strings.ToLower(resp.Header.Get("Connection")) != "upgrade" {
-		err = fmt.Errorf("bad response: Upgrade %q Connection %q", resp.Header.Get("Upgrade"), resp.Header.Get("Connection"))
+		err = fmt.Errorf("bad response from server: Upgrade %q Connection %q", resp.Header.Get("Upgrade"), resp.Header.Get("Connection"))
 		netconn.Close()
 		return
 	}

commit 4cc07d6ab4ae16bd6bc69b2c8022414d66ee4113
Author: Tom Clegg <tom at curii.com>
Date:   Wed Jan 13 00:10:09 2021 -0500

    17170: Fix dial when API host var is host:port.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go
index 75603bb59..9b2695b55 100644
--- a/lib/controller/rpc/conn.go
+++ b/lib/controller/rpc/conn.go
@@ -293,7 +293,12 @@ func (conn *Conn) ContainerUnlock(ctx context.Context, options arvados.GetOption
 // a running container. If the returned error is nil, the caller is
 // responsible for closing sshconn.Conn.
 func (conn *Conn) ContainerSSH(ctx context.Context, options arvados.ContainerSSHOptions) (sshconn arvados.ContainerSSHConnection, err error) {
-	netconn, err := tls.Dial("tcp", net.JoinHostPort(conn.baseURL.Host, "https"), nil)
+	addr := conn.baseURL.Host
+	if strings.Index(addr, ":") < 1 || (strings.Contains(addr, "::") && addr[0] != '[') {
+		// hostname or ::1 or 1::1
+		addr = net.JoinHostPort(addr, "https")
+	}
+	netconn, err := tls.Dial("tcp", addr, conn.httpClient.Transport.(*http.Transport).TLSClientConfig)
 	if err != nil {
 		return
 	}

commit 6be5fe90dc9a5452b432176bfa83cba83070f8cf
Author: Tom Clegg <tom at curii.com>
Date:   Tue Jan 12 23:22:46 2021 -0500

    17170: Test controller->crunch-run tunnel.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/localdb/container_gateway_test.go b/lib/controller/localdb/container_gateway_test.go
new file mode 100644
index 000000000..336d5b634
--- /dev/null
+++ b/lib/controller/localdb/container_gateway_test.go
@@ -0,0 +1,123 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package localdb
+
+import (
+	"context"
+	"crypto/hmac"
+	"crypto/sha256"
+	"fmt"
+	"io"
+	"time"
+
+	"git.arvados.org/arvados.git/lib/config"
+	"git.arvados.org/arvados.git/lib/crunchrun"
+	"git.arvados.org/arvados.git/sdk/go/arvados"
+	"git.arvados.org/arvados.git/sdk/go/arvadostest"
+	"git.arvados.org/arvados.git/sdk/go/auth"
+	"git.arvados.org/arvados.git/sdk/go/ctxlog"
+	check "gopkg.in/check.v1"
+)
+
+var _ = check.Suite(&ContainerGatewaySuite{})
+
+type ContainerGatewaySuite struct {
+	cluster *arvados.Cluster
+	localdb *Conn
+	ctx     context.Context
+	ctrUUID string
+	gw      *crunchrun.Gateway
+}
+
+func (s *ContainerGatewaySuite) TearDownSuite(c *check.C) {
+	// Undo any changes/additions to the user database so they
+	// don't affect subsequent tests.
+	arvadostest.ResetEnv()
+	c.Check(arvados.NewClientFromEnv().RequestAndDecode(nil, "POST", "database/reset", nil, nil), check.IsNil)
+}
+
+func (s *ContainerGatewaySuite) SetUpSuite(c *check.C) {
+	cfg, err := config.NewLoader(nil, ctxlog.TestLogger(c)).Load()
+	c.Assert(err, check.IsNil)
+	s.cluster, err = cfg.GetCluster("")
+	c.Assert(err, check.IsNil)
+	s.localdb = NewConn(s.cluster)
+	s.ctx = auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{arvadostest.ActiveTokenV2}})
+
+	s.ctrUUID = arvadostest.QueuedContainerUUID
+
+	h := hmac.New(sha256.New, []byte(s.cluster.SystemRootToken))
+	fmt.Fprint(h, s.ctrUUID)
+	authKey := fmt.Sprintf("%x", h.Sum(nil))
+
+	s.gw = &crunchrun.Gateway{
+		DockerContainerID: new(string),
+		ContainerUUID:     s.ctrUUID,
+		AuthSecret:        authKey,
+		Address:           "localhost:0",
+		Log:               ctxlog.TestLogger(c),
+	}
+	c.Assert(s.gw.Start(), check.IsNil)
+	rootctx := auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{s.cluster.SystemRootToken}})
+	_, err = s.localdb.ContainerUpdate(rootctx, arvados.UpdateOptions{
+		UUID: s.ctrUUID,
+		Attrs: map[string]interface{}{
+			"state": arvados.ContainerStateLocked}})
+	c.Assert(err, check.IsNil)
+	_, err = s.localdb.ContainerUpdate(rootctx, arvados.UpdateOptions{
+		UUID: s.ctrUUID,
+		Attrs: map[string]interface{}{
+			"state":           arvados.ContainerStateRunning,
+			"gateway_address": s.gw.Address}})
+	c.Assert(err, check.IsNil)
+}
+
+func (s *ContainerGatewaySuite) TestConnect(c *check.C) {
+	c.Logf("connecting to %s", s.gw.Address)
+	sshconn, err := s.localdb.ContainerSSH(s.ctx, arvados.ContainerSSHOptions{UUID: s.ctrUUID})
+	c.Assert(err, check.IsNil)
+	c.Assert(sshconn.Conn, check.NotNil)
+	defer sshconn.Conn.Close()
+
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+
+		// Receive text banner
+		buf := make([]byte, 12)
+		_, err := io.ReadFull(sshconn.Conn, buf)
+		c.Check(err, check.IsNil)
+		c.Check(string(buf), check.Equals, "SSH-2.0-Go\r\n")
+
+		// Send text banner
+		_, err = sshconn.Conn.Write([]byte("SSH-2.0-Fake\r\n"))
+		c.Check(err, check.IsNil)
+
+		// Receive binary
+		_, err = io.ReadFull(sshconn.Conn, buf[:4])
+		c.Check(err, check.IsNil)
+		c.Check(buf[:4], check.DeepEquals, []byte{0, 0, 1, 0xfc})
+
+		// If we can get this far into an SSH handshake...
+		c.Log("success, tunnel is working")
+	}()
+	select {
+	case <-done:
+	case <-time.After(time.Second):
+		c.Fail()
+	}
+}
+
+func (s *ContainerGatewaySuite) TestConnectFail(c *check.C) {
+	c.Log("trying with no token")
+	ctx := auth.NewContext(context.Background(), &auth.Credentials{})
+	_, err := s.localdb.ContainerSSH(ctx, arvados.ContainerSSHOptions{UUID: s.ctrUUID})
+	c.Check(err, check.ErrorMatches, `.* 401 .*`)
+
+	c.Log("trying with anonymous token")
+	ctx = auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{arvadostest.AnonymousToken}})
+	_, err = s.localdb.ContainerSSH(ctx, arvados.ContainerSSHOptions{UUID: s.ctrUUID})
+	c.Check(err, check.ErrorMatches, `.* 404 .*`)
+}

commit fed1d9f5e7a442abb6b2e86114e8c3d05cff5329
Author: Tom Clegg <tom at curii.com>
Date:   Tue Jan 12 21:57:40 2021 -0500

    17170: Fixup gateway auth secret.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/localdb/container_gateway.go b/lib/controller/localdb/container_gateway.go
index dd296c569..f6eaea6d9 100644
--- a/lib/controller/localdb/container_gateway.go
+++ b/lib/controller/localdb/container_gateway.go
@@ -63,7 +63,7 @@ func (conn *Conn) ContainerSSH(ctx context.Context, opts arvados.ContainerSSHOpt
 				return errors.New("no certificate received, cannot compute authorization header")
 			}
 			h := hmac.New(sha256.New, []byte(conn.cluster.SystemRootToken))
-			fmt.Fprint(h, "%s", opts.UUID)
+			fmt.Fprint(h, opts.UUID)
 			authKey := fmt.Sprintf("%x", h.Sum(nil))
 			h = hmac.New(sha256.New, []byte(authKey))
 			h.Write(rawCerts[0])
diff --git a/lib/dispatchcloud/worker/pool.go b/lib/dispatchcloud/worker/pool.go
index e092e7ada..6a74280ca 100644
--- a/lib/dispatchcloud/worker/pool.go
+++ b/lib/dispatchcloud/worker/pool.go
@@ -996,7 +996,7 @@ func (wp *Pool) waitUntilLoaded() {
 
 func (wp *Pool) gatewayAuthSecret(uuid string) string {
 	h := hmac.New(sha256.New, []byte(wp.systemRootToken))
-	fmt.Fprint(h, "%s", uuid)
+	fmt.Fprint(h, uuid)
 	return fmt.Sprintf("%x", h.Sum(nil))
 }
 

commit 2c3418d59cd27806322e07519ef9483c2cb433c2
Author: Tom Clegg <tom at curii.com>
Date:   Tue Jan 12 21:03:40 2021 -0500

    17170: Use TLS for controller->crunch-run traffic.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/localdb/container_gateway.go b/lib/controller/localdb/container_gateway.go
index 31d44e5e0..dd296c569 100644
--- a/lib/controller/localdb/container_gateway.go
+++ b/lib/controller/localdb/container_gateway.go
@@ -9,9 +9,10 @@ import (
 	"context"
 	"crypto/hmac"
 	"crypto/sha256"
+	"crypto/tls"
+	"crypto/x509"
 	"errors"
 	"fmt"
-	"net"
 	"net/http"
 	"net/url"
 	"strings"
@@ -36,20 +37,53 @@ func (conn *Conn) ContainerSSH(ctx context.Context, opts arvados.ContainerSSHOpt
 		err = httpserver.ErrorWithStatus(fmt.Errorf("gateway is not available, container is %s", strings.ToLower(string(ctr.State))), http.StatusBadGateway)
 		return
 	}
-	netconn, err := net.Dial("tcp", ctr.GatewayAddress)
+	// crunch-run uses a self-signed / unverifiable TLS
+	// certificate, so we use the following scheme to ensure we're
+	// not talking to a MITM.
+	//
+	// 1. Compute ctrKey = HMAC-SHA256(sysRootToken,ctrUUID) --
+	// this will be the same ctrKey that a-d-c supplied to
+	// crunch-run in the GatewayAuthSecret env var.
+	//
+	// 2. Compute requestAuth = HMAC-SHA256(ctrKey,serverCert) and
+	// send it to crunch-run as the X-Arvados-Authorization
+	// header, proving that we know ctrKey. (Note a MITM cannot
+	// replay the proof to a real crunch-run server, because the
+	// real crunch-run server would have a different cert.)
+	//
+	// 3. Compute respondAuth = HMAC-SHA256(ctrKey,requestAuth)
+	// and ensure the server returns it in the
+	// X-Arvados-Authorization-Response header, proving that the
+	// server knows ctrKey.
+	var requestAuth, respondAuth string
+	netconn, err := tls.Dial("tcp", ctr.GatewayAddress, &tls.Config{
+		InsecureSkipVerify: true,
+		VerifyPeerCertificate: func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
+			if len(rawCerts) == 0 {
+				return errors.New("no certificate received, cannot compute authorization header")
+			}
+			h := hmac.New(sha256.New, []byte(conn.cluster.SystemRootToken))
+			fmt.Fprint(h, "%s", opts.UUID)
+			authKey := fmt.Sprintf("%x", h.Sum(nil))
+			h = hmac.New(sha256.New, []byte(authKey))
+			h.Write(rawCerts[0])
+			requestAuth = fmt.Sprintf("%x", h.Sum(nil))
+			h.Reset()
+			h.Write([]byte(requestAuth))
+			respondAuth = fmt.Sprintf("%x", h.Sum(nil))
+			return nil
+		},
+	})
 	if err != nil {
 		return
 	}
+	if respondAuth == "" {
+		err = httpserver.ErrorWithStatus(errors.New("BUG: no respondAuth"), http.StatusInternalServerError)
+		return
+	}
 	bufr := bufio.NewReader(netconn)
 	bufw := bufio.NewWriter(netconn)
 
-	// Note this auth header does not protect from replay/mitm
-	// attacks (TODO: use TLS for that). It only authenticates us
-	// to crunch-run.
-	h := hmac.New(sha256.New, []byte(conn.cluster.SystemRootToken))
-	fmt.Fprint(h, "%s", opts.UUID)
-	auth := fmt.Sprintf("%x", h.Sum(nil))
-
 	u := url.URL{
 		Scheme: "http",
 		Host:   ctr.GatewayAddress,
@@ -59,14 +93,19 @@ func (conn *Conn) ContainerSSH(ctx context.Context, opts arvados.ContainerSSHOpt
 	bufw.WriteString("Host: " + u.Host + "\r\n")
 	bufw.WriteString("Upgrade: ssh\r\n")
 	bufw.WriteString("X-Arvados-Target-Uuid: " + opts.UUID + "\r\n")
-	bufw.WriteString("X-Arvados-Authorization: " + auth + "\r\n")
+	bufw.WriteString("X-Arvados-Authorization: " + requestAuth + "\r\n")
 	bufw.WriteString("X-Arvados-Detach-Keys: " + opts.DetachKeys + "\r\n")
 	bufw.WriteString("X-Arvados-Login-Username: " + opts.LoginUsername + "\r\n")
 	bufw.WriteString("\r\n")
 	bufw.Flush()
 	resp, err := http.ReadResponse(bufr, &http.Request{Method: "GET"})
 	if err != nil {
-		err = fmt.Errorf("error reading http response from gateway: %w", err)
+		err = httpserver.ErrorWithStatus(fmt.Errorf("error reading http response from gateway: %w", err), http.StatusBadGateway)
+		netconn.Close()
+		return
+	}
+	if resp.Header.Get("X-Arvados-Authorization-Response") != respondAuth {
+		err = httpserver.ErrorWithStatus(errors.New("bad X-Arvados-Authorization-Response header"), http.StatusBadGateway)
 		netconn.Close()
 		return
 	}
diff --git a/lib/crunchrun/container_gateway.go b/lib/crunchrun/container_gateway.go
index 92c11383f..8b2fe4144 100644
--- a/lib/crunchrun/container_gateway.go
+++ b/lib/crunchrun/container_gateway.go
@@ -5,8 +5,11 @@
 package crunchrun
 
 import (
+	"crypto/hmac"
 	"crypto/rand"
 	"crypto/rsa"
+	"crypto/sha256"
+	"crypto/tls"
 	"fmt"
 	"io"
 	"net"
@@ -16,17 +19,32 @@ import (
 	"sync"
 	"syscall"
 
+	"git.arvados.org/arvados.git/lib/selfsigned"
 	"git.arvados.org/arvados.git/sdk/go/httpserver"
 	"github.com/creack/pty"
 	"github.com/google/shlex"
 	"golang.org/x/crypto/ssh"
 )
 
+type Gateway struct {
+	DockerContainerID *string
+	ContainerUUID     string
+	Address           string // listen host:port; if port=0, Start() will change it to the selected port
+	AuthSecret        string
+	Log               interface {
+		Printf(fmt string, args ...interface{})
+	}
+
+	sshConfig   ssh.ServerConfig
+	requestAuth string
+	respondAuth string
+}
+
 // startGatewayServer starts an http server that allows authenticated
 // clients to open an interactive "docker exec" session and (in
 // future) connect to tcp ports inside the docker container.
-func (runner *ContainerRunner) startGatewayServer() error {
-	runner.gatewaySSHConfig = &ssh.ServerConfig{
+func (gw *Gateway) Start() error {
+	gw.sshConfig = ssh.ServerConfig{
 		NoClientAuth: true,
 		PasswordCallback: func(c ssh.ConnMetadata, pass []byte) (*ssh.Permissions, error) {
 			if c.User() == "_" {
@@ -47,7 +65,7 @@ func (runner *ContainerRunner) startGatewayServer() error {
 			}
 		},
 	}
-	pvt, err := rsa.GenerateKey(rand.Reader, 4096)
+	pvt, err := rsa.GenerateKey(rand.Reader, 2048)
 	if err != nil {
 		return err
 	}
@@ -59,20 +77,34 @@ func (runner *ContainerRunner) startGatewayServer() error {
 	if err != nil {
 		return err
 	}
-	runner.gatewaySSHConfig.AddHostKey(signer)
+	gw.sshConfig.AddHostKey(signer)
 
-	// GatewayAddress (provided by arvados-dispatch-cloud) is
+	// Address (typically provided by arvados-dispatch-cloud) is
 	// HOST:PORT where HOST is our IP address or hostname as seen
 	// from arvados-controller, and PORT is either the desired
-	// port where we should run our gateway server, or "0" if
-	// we should choose an available port.
-	host, port, err := net.SplitHostPort(os.Getenv("GatewayAddress"))
+	// port where we should run our gateway server, or "0" if we
+	// should choose an available port.
+	host, port, err := net.SplitHostPort(gw.Address)
 	if err != nil {
 		return err
 	}
+	cert, err := selfsigned.CertGenerator{}.Generate()
+	if err != nil {
+		return err
+	}
+	h := hmac.New(sha256.New, []byte(gw.AuthSecret))
+	h.Write(cert.Certificate[0])
+	gw.requestAuth = fmt.Sprintf("%x", h.Sum(nil))
+	h.Reset()
+	h.Write([]byte(gw.requestAuth))
+	gw.respondAuth = fmt.Sprintf("%x", h.Sum(nil))
+
 	srv := &httpserver.Server{
 		Server: http.Server{
-			Handler: http.HandlerFunc(runner.handleSSH),
+			Handler: http.HandlerFunc(gw.handleSSH),
+			TLSConfig: &tls.Config{
+				Certificates: []tls.Certificate{cert},
+			},
 		},
 		Addr: ":" + port,
 	}
@@ -90,7 +122,7 @@ func (runner *ContainerRunner) startGatewayServer() error {
 	// gateway_address to "HOST:PORT" where HOST is our
 	// external hostname/IP as provided by arvados-dispatch-cloud,
 	// and PORT is the port number we ended up listening on.
-	runner.gatewayAddress = net.JoinHostPort(host, port)
+	gw.Address = net.JoinHostPort(host, port)
 	return nil
 }
 
@@ -104,15 +136,15 @@ func (runner *ContainerRunner) startGatewayServer() error {
 // Connection: upgrade
 // Upgrade: ssh
 // X-Arvados-Target-Uuid: uuid of container
-// X-Arvados-Authorization: must match GatewayAuthSecret provided by
-// a-d-c (this prevents other containers and shell nodes from
-// connecting directly)
+// X-Arvados-Authorization: must match
+// hmac(AuthSecret,certfingerprint) (this prevents other containers
+// and shell nodes from connecting directly)
 //
 // Optional header:
 //
 // X-Arvados-Detach-Keys: argument to "docker attach --detach-keys",
 // e.g., "ctrl-p,ctrl-q"
-func (runner *ContainerRunner) handleSSH(w http.ResponseWriter, req *http.Request) {
+func (gw *Gateway) handleSSH(w http.ResponseWriter, req *http.Request) {
 	// In future we'll handle browser traffic too, but for now the
 	// only traffic we expect is an SSH tunnel from
 	// (*lib/controller/localdb.Conn)ContainerSSH()
@@ -120,11 +152,11 @@ func (runner *ContainerRunner) handleSSH(w http.ResponseWriter, req *http.Reques
 		http.Error(w, "path not found", http.StatusNotFound)
 		return
 	}
-	if want := req.Header.Get("X-Arvados-Target-Uuid"); want != runner.Container.UUID {
-		http.Error(w, fmt.Sprintf("misdirected request: meant for %q but received by crunch-run %q", want, runner.Container.UUID), http.StatusBadGateway)
+	if want := req.Header.Get("X-Arvados-Target-Uuid"); want != gw.ContainerUUID {
+		http.Error(w, fmt.Sprintf("misdirected request: meant for %q but received by crunch-run %q", want, gw.ContainerUUID), http.StatusBadGateway)
 		return
 	}
-	if req.Header.Get("X-Arvados-Authorization") != runner.gatewayAuthSecret {
+	if req.Header.Get("X-Arvados-Authorization") != gw.requestAuth {
 		http.Error(w, "bad X-Arvados-Authorization header", http.StatusUnauthorized)
 		return
 	}
@@ -146,15 +178,16 @@ func (runner *ContainerRunner) handleSSH(w http.ResponseWriter, req *http.Reques
 	defer netconn.Close()
 	w.Header().Set("Connection", "upgrade")
 	w.Header().Set("Upgrade", "ssh")
+	w.Header().Set("X-Arvados-Authorization-Response", gw.respondAuth)
 	netconn.Write([]byte("HTTP/1.1 101 Switching Protocols\r\n"))
 	w.Header().Write(netconn)
 	netconn.Write([]byte("\r\n"))
 
 	ctx := req.Context()
 
-	conn, newchans, reqs, err := ssh.NewServerConn(netconn, runner.gatewaySSHConfig)
+	conn, newchans, reqs, err := ssh.NewServerConn(netconn, &gw.sshConfig)
 	if err != nil {
-		runner.CrunchLog.Printf("ssh.NewServerConn: %s", err)
+		gw.Log.Printf("ssh.NewServerConn: %s", err)
 		return
 	}
 	defer conn.Close()
@@ -166,7 +199,7 @@ func (runner *ContainerRunner) handleSSH(w http.ResponseWriter, req *http.Reques
 		}
 		ch, reqs, err := newch.Accept()
 		if err != nil {
-			runner.CrunchLog.Printf("accept channel: %s", err)
+			gw.Log.Printf("accept channel: %s", err)
 			return
 		}
 		var pty0, tty0 *os.File
@@ -217,7 +250,7 @@ func (runner *ContainerRunner) handleSSH(w http.ResponseWriter, req *http.Reques
 							// Send our own debug messages to tty as well.
 							logw = tty0
 						}
-						cmd.Args = append(cmd.Args, runner.ContainerID)
+						cmd.Args = append(cmd.Args, *gw.DockerContainerID)
 						cmd.Args = append(cmd.Args, execargs...)
 						cmd.SysProcAttr = &syscall.SysProcAttr{
 							Setctty: tty0 != nil,
diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go
index b252e0dce..f28593fe0 100644
--- a/lib/crunchrun/crunchrun.go
+++ b/lib/crunchrun/crunchrun.go
@@ -33,7 +33,6 @@ import (
 	"git.arvados.org/arvados.git/sdk/go/arvadosclient"
 	"git.arvados.org/arvados.git/sdk/go/keepclient"
 	"git.arvados.org/arvados.git/sdk/go/manifest"
-	"golang.org/x/crypto/ssh"
 	"golang.org/x/net/context"
 
 	dockertypes "github.com/docker/docker/api/types"
@@ -180,9 +179,7 @@ type ContainerRunner struct {
 
 	containerWatchdogInterval time.Duration
 
-	gatewayAddress    string
-	gatewaySSHConfig  *ssh.ServerConfig
-	gatewayAuthSecret string
+	gateway Gateway
 }
 
 // setupSignals sets up signal handling to gracefully terminate the underlying
@@ -1474,7 +1471,7 @@ func (runner *ContainerRunner) UpdateContainerRunning() error {
 		return ErrCancelled
 	}
 	return runner.DispatcherArvClient.Update("containers", runner.Container.UUID,
-		arvadosclient.Dict{"container": arvadosclient.Dict{"state": "Running", "gateway_address": runner.gatewayAddress}}, nil)
+		arvadosclient.Dict{"container": arvadosclient.Dict{"state": "Running", "gateway_address": runner.gateway.Address}}, nil)
 }
 
 // ContainerToken returns the api_token the container (and any
@@ -1873,9 +1870,15 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s
 		return 1
 	}
 
-	cr.gatewayAuthSecret = os.Getenv("GatewayAuthSecret")
+	cr.gateway = Gateway{
+		Address:           os.Getenv("GatewayAddress"),
+		AuthSecret:        os.Getenv("GatewayAuthSecret"),
+		ContainerUUID:     containerID,
+		DockerContainerID: &cr.ContainerID,
+		Log:               cr.CrunchLog,
+	}
 	os.Unsetenv("GatewayAuthSecret")
-	err = cr.startGatewayServer()
+	err = cr.gateway.Start()
 	if err != nil {
 		log.Printf("error starting gateway server: %s", err)
 		return 1
diff --git a/lib/selfsigned/cert.go b/lib/selfsigned/cert.go
new file mode 100644
index 000000000..ae521dd17
--- /dev/null
+++ b/lib/selfsigned/cert.go
@@ -0,0 +1,76 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package selfsigned
+
+import (
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/tls"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"fmt"
+	"math/big"
+	"net"
+	"time"
+)
+
+type CertGenerator struct {
+	Bits  int
+	Hosts []string
+	IsCA  bool
+}
+
+func (gen CertGenerator) Generate() (cert tls.Certificate, err error) {
+	keyUsage := x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment
+	if gen.IsCA {
+		keyUsage |= x509.KeyUsageCertSign
+	}
+	notBefore := time.Now()
+	notAfter := time.Now().Add(time.Hour * 24 * 365)
+	snMax := new(big.Int).Lsh(big.NewInt(1), 128)
+	sn, err := rand.Int(rand.Reader, snMax)
+	if err != nil {
+		err = fmt.Errorf("Failed to generate serial number: %w", err)
+		return
+	}
+	template := x509.Certificate{
+		SerialNumber: sn,
+		Subject: pkix.Name{
+			Organization: []string{"N/A"},
+		},
+		NotBefore:             notBefore,
+		NotAfter:              notAfter,
+		KeyUsage:              keyUsage,
+		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+		BasicConstraintsValid: true,
+		IsCA:                  gen.IsCA,
+	}
+	for _, h := range gen.Hosts {
+		if ip := net.ParseIP(h); ip != nil {
+			template.IPAddresses = append(template.IPAddresses, ip)
+		} else {
+			template.DNSNames = append(template.DNSNames, h)
+		}
+	}
+	bits := gen.Bits
+	if bits == 0 {
+		bits = 4096
+	}
+	priv, err := rsa.GenerateKey(rand.Reader, bits)
+	if err != nil {
+		err = fmt.Errorf("error generating key: %w", err)
+		return
+	}
+	certder, err := x509.CreateCertificate(rand.Reader, &template, &template, &priv.PublicKey, priv)
+	if err != nil {
+		err = fmt.Errorf("error creating certificate: %w", err)
+		return
+	}
+	cert = tls.Certificate{
+		Certificate: [][]byte{certder},
+		PrivateKey:  priv,
+	}
+	return
+}
diff --git a/lib/selfsigned/cert_test.go b/lib/selfsigned/cert_test.go
new file mode 100644
index 000000000..16ed8bd91
--- /dev/null
+++ b/lib/selfsigned/cert_test.go
@@ -0,0 +1,26 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package selfsigned
+
+import (
+	"testing"
+)
+
+func TestCert(t *testing.T) {
+	cert, err := CertGenerator{Bits: 1024, Hosts: []string{"localhost"}, IsCA: false}.Generate()
+	if err != nil {
+		t.Error(err)
+	}
+	if len(cert.Certificate) < 1 {
+		t.Error("no certificate!")
+	}
+	cert, err = CertGenerator{Bits: 2048, Hosts: []string{"localhost"}, IsCA: true}.Generate()
+	if err != nil {
+		t.Error(err)
+	}
+	if len(cert.Certificate) < 1 {
+		t.Error("no certificate!")
+	}
+}

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list