From: Tom Clegg Date: Thu, 20 Apr 2023 14:49:16 +0000 (-0400) Subject: Merge branch '20241-validate-ssh-keys' X-Git-Tag: 2.7.0~135 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/432ec6a698a5ff2616541745aa7afa9074b970c3?hp=02ae62dcdc5d5cd9231b32b407bd525e74633003 Merge branch '20241-validate-ssh-keys' fixes #20241 Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- diff --git a/.licenseignore b/.licenseignore index 3d24c4ee3a..4456725dca 100644 --- a/.licenseignore +++ b/.licenseignore @@ -94,3 +94,4 @@ sdk/cwl/tests/chipseq/data/Genomes/* CITATION.cff SECURITY.md */testdata/fakestat/* +lib/controller/localdb/testdata/*.pub diff --git a/apps/workbench/test/integration/user_settings_menu_test.rb b/apps/workbench/test/integration/user_settings_menu_test.rb index 99076bbaf7..6f3ca9dd5c 100644 --- a/apps/workbench/test/integration/user_settings_menu_test.rb +++ b/apps/workbench/test/integration/user_settings_menu_test.rb @@ -46,7 +46,7 @@ class UserSettingsMenuTest < ActionDispatch::IntegrationTest page.find_field('public_key').set 'first test with an incorrect ssh key value' click_button 'Submit' - assert_text 'Public key does not appear to be a valid ssh-rsa or dsa public key' + assert_text 'Public key does not appear to be valid' public_key_str = api_fixture('authorized_keys')['active']['public_key'] page.find_field('public_key').set public_key_str diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go index 268b9eefb8..7a9f7cb999 100644 --- a/lib/controller/federation/conn.go +++ b/lib/controller/federation/conn.go @@ -270,6 +270,26 @@ func (conn *Conn) Logout(ctx context.Context, options arvados.LogoutOptions) (ar return arvados.LogoutResponse{RedirectLocation: target.String()}, nil } +func (conn *Conn) AuthorizedKeyCreate(ctx context.Context, options arvados.CreateOptions) (arvados.AuthorizedKey, error) { + return conn.chooseBackend(options.ClusterID).AuthorizedKeyCreate(ctx, options) +} + +func (conn *Conn) AuthorizedKeyUpdate(ctx context.Context, options arvados.UpdateOptions) (arvados.AuthorizedKey, error) { + return conn.chooseBackend(options.UUID).AuthorizedKeyUpdate(ctx, options) +} + +func (conn *Conn) AuthorizedKeyGet(ctx context.Context, options arvados.GetOptions) (arvados.AuthorizedKey, error) { + return conn.chooseBackend(options.UUID).AuthorizedKeyGet(ctx, options) +} + +func (conn *Conn) AuthorizedKeyList(ctx context.Context, options arvados.ListOptions) (arvados.AuthorizedKeyList, error) { + return conn.generated_AuthorizedKeyList(ctx, options) +} + +func (conn *Conn) AuthorizedKeyDelete(ctx context.Context, options arvados.DeleteOptions) (arvados.AuthorizedKey, error) { + return conn.chooseBackend(options.UUID).AuthorizedKeyDelete(ctx, options) +} + func (conn *Conn) CollectionGet(ctx context.Context, options arvados.GetOptions) (arvados.Collection, error) { if len(options.UUID) == 27 { // UUID is really a UUID diff --git a/lib/controller/federation/generate.go b/lib/controller/federation/generate.go index 86bbf9d9e3..2dc2918f79 100644 --- a/lib/controller/federation/generate.go +++ b/lib/controller/federation/generate.go @@ -53,7 +53,7 @@ func main() { defer out.Close() out.Write(regexp.MustCompile(`(?ms)^.*package .*?import.*?\n\)\n`).Find(buf)) io.WriteString(out, "//\n// -- this file is auto-generated -- do not edit -- edit list.go and run \"go generate\" instead --\n//\n\n") - for _, t := range []string{"Container", "ContainerRequest", "Group", "Specimen", "User", "Link", "Log", "APIClientAuthorization"} { + for _, t := range []string{"AuthorizedKey", "Container", "ContainerRequest", "Group", "Specimen", "User", "Link", "Log", "APIClientAuthorization"} { _, err := out.Write(bytes.ReplaceAll(orig, []byte("Collection"), []byte(t))) if err != nil { panic(err) diff --git a/lib/controller/federation/generated.go b/lib/controller/federation/generated.go index 637a1ce919..8c8666fea1 100755 --- a/lib/controller/federation/generated.go +++ b/lib/controller/federation/generated.go @@ -17,6 +17,47 @@ import ( // -- this file is auto-generated -- do not edit -- edit list.go and run "go generate" instead -- // +func (conn *Conn) generated_AuthorizedKeyList(ctx context.Context, options arvados.ListOptions) (arvados.AuthorizedKeyList, error) { + var mtx sync.Mutex + var merged arvados.AuthorizedKeyList + var needSort atomic.Value + needSort.Store(false) + err := conn.splitListRequest(ctx, options, func(ctx context.Context, _ string, backend arvados.API, options arvados.ListOptions) ([]string, error) { + options.ForwardedFor = conn.cluster.ClusterID + "-" + options.ForwardedFor + cl, err := backend.AuthorizedKeyList(ctx, options) + if err != nil { + return nil, err + } + mtx.Lock() + defer mtx.Unlock() + if len(merged.Items) == 0 { + merged = cl + } else if len(cl.Items) > 0 { + merged.Items = append(merged.Items, cl.Items...) + needSort.Store(true) + } + uuids := make([]string, 0, len(cl.Items)) + for _, item := range cl.Items { + uuids = append(uuids, item.UUID) + } + return uuids, nil + }) + if needSort.Load().(bool) { + // Apply the default/implied order, "modified_at desc" + sort.Slice(merged.Items, func(i, j int) bool { + mi, mj := merged.Items[i].ModifiedAt, merged.Items[j].ModifiedAt + return mj.Before(mi) + }) + } + if merged.Items == nil { + // Return empty results as [], not null + // (https://github.com/golang/go/issues/27589 might be + // a better solution in the future) + merged.Items = []arvados.AuthorizedKey{} + } + return merged, err +} + func (conn *Conn) generated_ContainerList(ctx context.Context, options arvados.ListOptions) (arvados.ContainerList, error) { var mtx sync.Mutex var merged arvados.ContainerList diff --git a/lib/controller/handler.go b/lib/controller/handler.go index 7b7378ac55..bfcb98b9d9 100644 --- a/lib/controller/handler.go +++ b/lib/controller/handler.go @@ -140,6 +140,8 @@ func (h *Handler) setup() { mux.Handle("/arvados/v1/groups/", rtr) mux.Handle("/arvados/v1/links", rtr) mux.Handle("/arvados/v1/links/", rtr) + mux.Handle("/arvados/v1/authorized_keys", rtr) + mux.Handle("/arvados/v1/authorized_keys/", rtr) mux.Handle("/login", rtr) mux.Handle("/logout", rtr) mux.Handle("/arvados/v1/api_client_authorizations", rtr) diff --git a/lib/controller/handler_test.go b/lib/controller/handler_test.go index fcd70d7cc5..0c50a6c4be 100644 --- a/lib/controller/handler_test.go +++ b/lib/controller/handler_test.go @@ -639,7 +639,7 @@ func (s *HandlerSuite) TestGetObjects(c *check.C) { testCases := map[string]map[string]bool{ "api_clients/" + arvadostest.TrustedWorkbenchAPIClientUUID: nil, "api_client_authorizations/" + auth.UUID: {"href": true, "modified_by_client_uuid": true, "modified_by_user_uuid": true}, - "authorized_keys/" + arvadostest.AdminAuthorizedKeysUUID: nil, + "authorized_keys/" + arvadostest.AdminAuthorizedKeysUUID: {"href": true}, "collections/" + arvadostest.CollectionWithUniqueWordsUUID: {"href": true}, "containers/" + arvadostest.RunningContainerUUID: nil, "container_requests/" + arvadostest.QueuedContainerRequestUUID: nil, diff --git a/lib/controller/localdb/authorized_key.go b/lib/controller/localdb/authorized_key.go new file mode 100644 index 0000000000..4d858c8fa7 --- /dev/null +++ b/lib/controller/localdb/authorized_key.go @@ -0,0 +1,59 @@ +// Copyright (C) The Arvados Authors. All rights reserved. +// +// SPDX-License-Identifier: AGPL-3.0 + +package localdb + +import ( + "context" + "errors" + "fmt" + "net/http" + "strings" + + "git.arvados.org/arvados.git/sdk/go/arvados" + "git.arvados.org/arvados.git/sdk/go/httpserver" + "golang.org/x/crypto/ssh" +) + +// AuthorizedKeyCreate checks that the provided public key is valid, +// then proxies to railsproxy. +func (conn *Conn) AuthorizedKeyCreate(ctx context.Context, opts arvados.CreateOptions) (arvados.AuthorizedKey, error) { + if err := validateKey(opts.Attrs); err != nil { + return arvados.AuthorizedKey{}, httpserver.ErrorWithStatus(err, http.StatusBadRequest) + } + return conn.railsProxy.AuthorizedKeyCreate(ctx, opts) +} + +// AuthorizedKeyUpdate checks that the provided public key is valid, +// then proxies to railsproxy. +func (conn *Conn) AuthorizedKeyUpdate(ctx context.Context, opts arvados.UpdateOptions) (arvados.AuthorizedKey, error) { + if err := validateKey(opts.Attrs); err != nil { + return arvados.AuthorizedKey{}, httpserver.ErrorWithStatus(err, http.StatusBadRequest) + } + return conn.railsProxy.AuthorizedKeyUpdate(ctx, opts) +} + +func validateKey(attrs map[string]interface{}) error { + in, _ := attrs["public_key"].(string) + if in == "" { + return nil + } + in = strings.TrimSpace(in) + if strings.IndexAny(in, "\r\n") >= 0 { + return errors.New("Public key does not appear to be valid: extra data after key") + } + pubkey, _, _, rest, err := ssh.ParseAuthorizedKey([]byte(in)) + if err != nil { + return fmt.Errorf("Public key does not appear to be valid: %w", err) + } + if len(rest) > 0 { + return errors.New("Public key does not appear to be valid: extra data after key") + } + if i := strings.Index(in, " "); i < 0 { + return errors.New("Public key does not appear to be valid: no leading type field") + } else if in[:i] != pubkey.Type() { + return fmt.Errorf("Public key does not appear to be valid: leading type field %q does not match actual key type %q", in[:i], pubkey.Type()) + } + return nil +} diff --git a/lib/controller/localdb/authorized_key_test.go b/lib/controller/localdb/authorized_key_test.go new file mode 100644 index 0000000000..44fa3cf94e --- /dev/null +++ b/lib/controller/localdb/authorized_key_test.go @@ -0,0 +1,114 @@ +// Copyright (C) The Arvados Authors. All rights reserved. +// +// SPDX-License-Identifier: AGPL-3.0 + +package localdb + +import ( + _ "embed" + "errors" + "io/ioutil" + "net/http" + "os" + "strings" + + "git.arvados.org/arvados.git/sdk/go/arvados" + "git.arvados.org/arvados.git/sdk/go/arvadostest" + "git.arvados.org/arvados.git/sdk/go/httpserver" + . "gopkg.in/check.v1" +) + +var _ = Suite(&authorizedKeySuite{}) + +type authorizedKeySuite struct { + localdbSuite +} + +//go:embed testdata/rsa.pub +var testPubKey string + +func (s *authorizedKeySuite) TestAuthorizedKeyCreate(c *C) { + ak, err := s.localdb.AuthorizedKeyCreate(s.userctx, arvados.CreateOptions{ + Attrs: map[string]interface{}{ + "name": "testkey", + "key_type": "SSH", + }}) + c.Assert(err, IsNil) + c.Check(ak.KeyType, Equals, "SSH") + defer s.localdb.AuthorizedKeyDelete(s.userctx, arvados.DeleteOptions{UUID: ak.UUID}) + updated, err := s.localdb.AuthorizedKeyUpdate(s.userctx, arvados.UpdateOptions{ + UUID: ak.UUID, + Attrs: map[string]interface{}{"name": "testkeyrenamed"}}) + c.Check(err, IsNil) + c.Check(updated.UUID, Equals, ak.UUID) + c.Check(updated.Name, Equals, "testkeyrenamed") + c.Check(updated.ModifiedByUserUUID, Equals, arvadostest.ActiveUserUUID) + + _, err = s.localdb.AuthorizedKeyCreate(s.userctx, arvados.CreateOptions{ + Attrs: map[string]interface{}{ + "name": "testkey", + "public_key": "ssh-dsa boguskey\n", + }}) + c.Check(err, ErrorMatches, `Public key does not appear to be valid: ssh: no key found`) + _, err = s.localdb.AuthorizedKeyUpdate(s.userctx, arvados.UpdateOptions{ + UUID: ak.UUID, + Attrs: map[string]interface{}{ + "public_key": strings.Replace(testPubKey, "A", "#", 1), + }}) + c.Check(err, ErrorMatches, `Public key does not appear to be valid: ssh: no key found`) + _, err = s.localdb.AuthorizedKeyUpdate(s.userctx, arvados.UpdateOptions{ + UUID: ak.UUID, + Attrs: map[string]interface{}{ + "public_key": testPubKey + testPubKey, + }}) + c.Check(err, ErrorMatches, `Public key does not appear to be valid: extra data after key`) + _, err = s.localdb.AuthorizedKeyUpdate(s.userctx, arvados.UpdateOptions{ + UUID: ak.UUID, + Attrs: map[string]interface{}{ + "public_key": testPubKey + "# extra data\n", + }}) + c.Check(err, ErrorMatches, `Public key does not appear to be valid: extra data after key`) + _, err = s.localdb.AuthorizedKeyUpdate(s.userctx, arvados.UpdateOptions{ + UUID: ak.UUID, + Attrs: map[string]interface{}{ + "public_key": strings.Replace(testPubKey, "ssh-rsa", "ssh-dsa", 1), + }}) + c.Check(err, ErrorMatches, `Public key does not appear to be valid: leading type field "ssh-dsa" does not match actual key type "ssh-rsa"`) + var se httpserver.HTTPStatusError + if c.Check(errors.As(err, &se), Equals, true) { + c.Check(se.HTTPStatus(), Equals, http.StatusBadRequest) + } + + dirents, err := os.ReadDir("./testdata") + c.Assert(err, IsNil) + c.Assert(dirents, Not(HasLen), 0) + for _, dirent := range dirents { + if !strings.HasSuffix(dirent.Name(), ".pub") { + continue + } + pubkeyfile := "./testdata/" + dirent.Name() + c.Logf("checking public key from %s", pubkeyfile) + pubkey, err := ioutil.ReadFile(pubkeyfile) + if !c.Check(err, IsNil) { + continue + } + updated, err := s.localdb.AuthorizedKeyUpdate(s.userctx, arvados.UpdateOptions{ + UUID: ak.UUID, + Attrs: map[string]interface{}{ + "public_key": string(pubkey), + }}) + c.Check(err, IsNil) + c.Check(updated.PublicKey, Equals, string(pubkey)) + + _, err = s.localdb.AuthorizedKeyUpdate(s.userctx, arvados.UpdateOptions{ + UUID: ak.UUID, + Attrs: map[string]interface{}{ + "public_key": strings.Replace(string(pubkey), " ", "-bogus ", 1), + }}) + c.Check(err, ErrorMatches, `.*type field ".*" does not match actual key type ".*"`) + } + + deleted, err := s.localdb.AuthorizedKeyDelete(s.userctx, arvados.DeleteOptions{UUID: ak.UUID}) + c.Check(err, IsNil) + c.Check(deleted.UUID, Equals, ak.UUID) +} diff --git a/lib/controller/localdb/testdata/dsa.pub b/lib/controller/localdb/testdata/dsa.pub new file mode 100644 index 0000000000..8a2743d91d --- /dev/null +++ b/lib/controller/localdb/testdata/dsa.pub @@ -0,0 +1 @@ +ssh-dss AAAAB3NzaC1kc3MAAACBAIS5sFWjsFPK5yEa/TjXEEudJrBaFjQ6WvYLiJmh8AmCqWlC83ETv5gEFeIwJo8om8bat4n6l6IKkG4wDo7uxNN0lEWGnOBXatpWOcrJphb0PgYMstZnW7K5GBpTY52TDShx5OS5nvb9iJiQjd1/WQ63knmYoVZH3Ijhv6vDikL3AAAAFQDotNYD4D4IjS8BjJFk8qCGg1FWGQAAAIBlqZ/KwlJpJiekR2Yv+8k456kiFhPUasjeDqx+zGP//+0xNGx2yYzdkPlmvYrdG3YvRjA8KX5C+qJT9CfS1FMcY8/3cXWmDCxi3zKvaXjUcLk1nfVbhsPHdaebpSX3N+C6meehjoQIhYIgZghdPuWOgyGjwIavO9DYMlTGVhHRCgAAAIAjqJonYsmaSd3/0SoD2NGKBvRhngKcaTu63OLIY/V2kdg4Zrph7Ptx//S994rlhugLq68c0wnNoeq4vjVoRY8gDaCy8KXsk9Sq8THbxNseFeqa04txJJXe7g8/6nopfqrhi0NgpIyaNn/0BfqjWOErQuhzxhMqZ5if0aRi1k+g5A== tom@slab diff --git a/lib/controller/localdb/testdata/ecdsa-sk.pub b/lib/controller/localdb/testdata/ecdsa-sk.pub new file mode 100644 index 0000000000..9f18e6b65c --- /dev/null +++ b/lib/controller/localdb/testdata/ecdsa-sk.pub @@ -0,0 +1 @@ +sk-ecdsa-sha2-nistp256@openssh.com AAAAInNrLWVjZHNhLXNoYTItbmlzdHAyNTZAb3BlbnNzaC5jb20AAAAIbmlzdHAyNTYAAABBBFj1zodcmSKWeUgNxzDOv7m9TeLhNRb64wa9oQwQK4tFZzLQRgcsmaVQmMx/ZbY+ThZbHLHSpKRxaByINu99NKUAAAAEc3NoOg== tom@slab diff --git a/lib/controller/localdb/testdata/ecdsa.pub b/lib/controller/localdb/testdata/ecdsa.pub new file mode 100644 index 0000000000..b34e821e6f --- /dev/null +++ b/lib/controller/localdb/testdata/ecdsa.pub @@ -0,0 +1 @@ +ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBDLajzRPnSI3FBChDvvNJyIBPdyA/nC7GWFWwizK93XL8HkQ5+X6D/xaqowq6iIPq/XHSdbZ3ebdb0OH81ovrCQ= tom@slab diff --git a/lib/controller/localdb/testdata/ed25519-sk.pub b/lib/controller/localdb/testdata/ed25519-sk.pub new file mode 100644 index 0000000000..0aa08f57bd --- /dev/null +++ b/lib/controller/localdb/testdata/ed25519-sk.pub @@ -0,0 +1 @@ +sk-ssh-ed25519@openssh.com AAAAGnNrLXNzaC1lZDI1NTE5QG9wZW5zc2guY29tAAAAIJMteBo9BvwQTeiBq4FvS4qJ83YjoCvKrH6EnvrOCILmAAAABHNzaDo= test key diff --git a/lib/controller/localdb/testdata/ed25519.pub b/lib/controller/localdb/testdata/ed25519.pub new file mode 100644 index 0000000000..ffcde15401 --- /dev/null +++ b/lib/controller/localdb/testdata/ed25519.pub @@ -0,0 +1 @@ +ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIElzlGk8QUevhJQ2mhf8p73lUAh044icWqssl3bMoCaT tom@slab diff --git a/lib/controller/localdb/testdata/generate b/lib/controller/localdb/testdata/generate new file mode 100755 index 0000000000..d39d72a91d --- /dev/null +++ b/lib/controller/localdb/testdata/generate @@ -0,0 +1,25 @@ +#!/bin/bash +# Copyright (C) The Arvados Authors. All rights reserved. +# +# SPDX-License-Identifier: AGPL-3.0 + +# This script uses ssh-keygen to generate an example public key for +# each supported type, to be used by test cases. Private keys are +# discarded. If ${keytype}.pub already exists, it is left alone. + +set -e + +err= +keytypes=$(ssh-keygen -_ 2>&1 | grep -- -t | tr -d '[|]' | tr ' ' '\n' | grep -vw t) +for keytype in ${keytypes[@]}; do + if [[ ! -e "./${keytype}.pub" ]]; then + if ssh-keygen -t "${keytype}" -f "./${keytype}" -N ""; then + # discard private key + rm "./${keytype}" + else + echo >&2 "ssh-keygen -t ${keytype} failed" + err=1 + fi + fi +done +exit $err diff --git a/lib/controller/localdb/testdata/rsa.pub b/lib/controller/localdb/testdata/rsa.pub new file mode 100644 index 0000000000..4b5ab75ec7 --- /dev/null +++ b/lib/controller/localdb/testdata/rsa.pub @@ -0,0 +1 @@ +ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQCtlBJsNterzUR26k/3tbXi2LViRj0vPyyJ7msqyGtRjJKuMqZkVJz6GN42/+aESeHfJw9FNlwW4oMa3Z4BB5llvZSG8yhY1HXbBlK5sURjSo9tid/U+PlKPGqteiXTguXLj5PAwoAoQ4JnGKR/+YphWxuWy+VR4toLcuKG9pX5d6iwkmWU1/smUnF6+vq38Xrhv94EpeNmyTEPC6OijDdmcas3rwDGW/I2Vij/Bxdj9DY/tHLv9V+yznbV1YB9yxda0YeIGMa2d35dOIxBeWmXzAGczVNQeXE7ooFOH6zCyoJZ4HH/AhAZ9GHyNGsf72CM+WkTBUEYmBmRIDHtMXY32KxyreRWUU1l47md5gefkb4c57OI369AQed154SVQaoiiVqIXinXGGezmfa09nnaSelD54Hky71GC/qqMvzkv7pXkETB37hYC2z2NixXQ6pf21vRHZLAtA8LK9OB5yxdr9b5buMIdTLViKufr3pPk8bcJrlB7tilw5X/PUioWws= tom@slab diff --git a/lib/controller/router/router.go b/lib/controller/router/router.go index 2cbd9b88dc..9a91c69a6b 100644 --- a/lib/controller/router/router.go +++ b/lib/controller/router/router.go @@ -86,6 +86,41 @@ func (rtr *router) addRoutes() { return rtr.backend.Logout(ctx, *opts.(*arvados.LogoutOptions)) }, }, + { + arvados.EndpointAuthorizedKeyCreate, + func() interface{} { return &arvados.CreateOptions{} }, + func(ctx context.Context, opts interface{}) (interface{}, error) { + return rtr.backend.AuthorizedKeyCreate(ctx, *opts.(*arvados.CreateOptions)) + }, + }, + { + arvados.EndpointAuthorizedKeyUpdate, + func() interface{} { return &arvados.UpdateOptions{} }, + func(ctx context.Context, opts interface{}) (interface{}, error) { + return rtr.backend.AuthorizedKeyUpdate(ctx, *opts.(*arvados.UpdateOptions)) + }, + }, + { + arvados.EndpointAuthorizedKeyGet, + func() interface{} { return &arvados.GetOptions{} }, + func(ctx context.Context, opts interface{}) (interface{}, error) { + return rtr.backend.AuthorizedKeyGet(ctx, *opts.(*arvados.GetOptions)) + }, + }, + { + arvados.EndpointAuthorizedKeyList, + func() interface{} { return &arvados.ListOptions{Limit: -1} }, + func(ctx context.Context, opts interface{}) (interface{}, error) { + return rtr.backend.AuthorizedKeyList(ctx, *opts.(*arvados.ListOptions)) + }, + }, + { + arvados.EndpointAuthorizedKeyDelete, + func() interface{} { return &arvados.DeleteOptions{} }, + func(ctx context.Context, opts interface{}) (interface{}, error) { + return rtr.backend.AuthorizedKeyDelete(ctx, *opts.(*arvados.DeleteOptions)) + }, + }, { arvados.EndpointCollectionCreate, func() interface{} { return &arvados.CreateOptions{} }, diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go index 70a936a6f6..3621f42d0e 100644 --- a/lib/controller/rpc/conn.go +++ b/lib/controller/rpc/conn.go @@ -220,6 +220,41 @@ func (conn *Conn) relativeToBaseURL(location string) string { return location } +func (conn *Conn) AuthorizedKeyCreate(ctx context.Context, options arvados.CreateOptions) (arvados.AuthorizedKey, error) { + ep := arvados.EndpointAuthorizedKeyCreate + var resp arvados.AuthorizedKey + err := conn.requestAndDecode(ctx, &resp, ep, nil, options) + return resp, err +} + +func (conn *Conn) AuthorizedKeyUpdate(ctx context.Context, options arvados.UpdateOptions) (arvados.AuthorizedKey, error) { + ep := arvados.EndpointAuthorizedKeyUpdate + var resp arvados.AuthorizedKey + err := conn.requestAndDecode(ctx, &resp, ep, nil, options) + return resp, err +} + +func (conn *Conn) AuthorizedKeyGet(ctx context.Context, options arvados.GetOptions) (arvados.AuthorizedKey, error) { + ep := arvados.EndpointAuthorizedKeyGet + var resp arvados.AuthorizedKey + err := conn.requestAndDecode(ctx, &resp, ep, nil, options) + return resp, err +} + +func (conn *Conn) AuthorizedKeyList(ctx context.Context, options arvados.ListOptions) (arvados.AuthorizedKeyList, error) { + ep := arvados.EndpointAuthorizedKeyList + var resp arvados.AuthorizedKeyList + err := conn.requestAndDecode(ctx, &resp, ep, nil, options) + return resp, err +} + +func (conn *Conn) AuthorizedKeyDelete(ctx context.Context, options arvados.DeleteOptions) (arvados.AuthorizedKey, error) { + ep := arvados.EndpointAuthorizedKeyDelete + var resp arvados.AuthorizedKey + err := conn.requestAndDecode(ctx, &resp, ep, nil, options) + return resp, err +} + func (conn *Conn) CollectionCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Collection, error) { ep := arvados.EndpointCollectionCreate var resp arvados.Collection diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go index 861b8e6ceb..c79616c676 100644 --- a/sdk/go/arvados/api.go +++ b/sdk/go/arvados/api.go @@ -27,6 +27,11 @@ var ( EndpointVocabularyGet = APIEndpoint{"GET", "arvados/v1/vocabulary", ""} EndpointLogin = APIEndpoint{"GET", "login", ""} EndpointLogout = APIEndpoint{"GET", "logout", ""} + EndpointAuthorizedKeyCreate = APIEndpoint{"POST", "arvados/v1/authorized_keys", "authorized_key"} + EndpointAuthorizedKeyUpdate = APIEndpoint{"PATCH", "arvados/v1/authorized_keys/{uuid}", "authorized_key"} + EndpointAuthorizedKeyGet = APIEndpoint{"GET", "arvados/v1/authorized_keys/{uuid}", ""} + EndpointAuthorizedKeyList = APIEndpoint{"GET", "arvados/v1/authorized_keys", ""} + EndpointAuthorizedKeyDelete = APIEndpoint{"DELETE", "arvados/v1/authorized_keys/{uuid}", ""} EndpointCollectionCreate = APIEndpoint{"POST", "arvados/v1/collections", "collection"} EndpointCollectionUpdate = APIEndpoint{"PATCH", "arvados/v1/collections/{uuid}", "collection"} EndpointCollectionGet = APIEndpoint{"GET", "arvados/v1/collections/{uuid}", ""} @@ -268,6 +273,11 @@ type API interface { VocabularyGet(ctx context.Context) (Vocabulary, error) Login(ctx context.Context, options LoginOptions) (LoginResponse, error) Logout(ctx context.Context, options LogoutOptions) (LogoutResponse, error) + AuthorizedKeyCreate(ctx context.Context, options CreateOptions) (AuthorizedKey, error) + AuthorizedKeyUpdate(ctx context.Context, options UpdateOptions) (AuthorizedKey, error) + AuthorizedKeyGet(ctx context.Context, options GetOptions) (AuthorizedKey, error) + AuthorizedKeyList(ctx context.Context, options ListOptions) (AuthorizedKeyList, error) + AuthorizedKeyDelete(ctx context.Context, options DeleteOptions) (AuthorizedKey, error) CollectionCreate(ctx context.Context, options CreateOptions) (Collection, error) CollectionUpdate(ctx context.Context, options UpdateOptions) (Collection, error) CollectionGet(ctx context.Context, options GetOptions) (Collection, error) diff --git a/sdk/go/arvados/authorized_key.go b/sdk/go/arvados/authorized_key.go new file mode 100644 index 0000000000..642fc11261 --- /dev/null +++ b/sdk/go/arvados/authorized_key.go @@ -0,0 +1,31 @@ +// Copyright (C) The Arvados Authors. All rights reserved. +// +// SPDX-License-Identifier: Apache-2.0 + +package arvados + +import "time" + +// AuthorizedKey is an arvados#authorizedKey resource. +type AuthorizedKey struct { + UUID string `json:"uuid"` + Etag string `json:"etag"` + OwnerUUID string `json:"owner_uuid"` + CreatedAt time.Time `json:"created_at"` + ModifiedAt time.Time `json:"modified_at"` + ModifiedByClientUUID string `json:"modified_by_client_uuid"` + ModifiedByUserUUID string `json:"modified_by_user_uuid"` + Name string `json:"name"` + AuthorizedUserUUID string `json:"authorized_user_uuid"` + PublicKey string `json:"public_key"` + KeyType string `json:"key_type"` + ExpiresAt time.Time `json:"expires_at"` +} + +// AuthorizedKeyList is an arvados#authorizedKeyList resource. +type AuthorizedKeyList struct { + Items []AuthorizedKey `json:"items"` + ItemsAvailable int `json:"items_available"` + Offset int `json:"offset"` + Limit int `json:"limit"` +} diff --git a/sdk/go/arvadostest/api.go b/sdk/go/arvadostest/api.go index 483832de53..9c70e9776a 100644 --- a/sdk/go/arvadostest/api.go +++ b/sdk/go/arvadostest/api.go @@ -48,6 +48,26 @@ func (as *APIStub) Logout(ctx context.Context, options arvados.LogoutOptions) (a as.appendCall(ctx, as.Logout, options) return arvados.LogoutResponse{}, as.Error } +func (as *APIStub) AuthorizedKeyCreate(ctx context.Context, options arvados.CreateOptions) (arvados.AuthorizedKey, error) { + as.appendCall(ctx, as.AuthorizedKeyCreate, options) + return arvados.AuthorizedKey{}, as.Error +} +func (as *APIStub) AuthorizedKeyUpdate(ctx context.Context, options arvados.UpdateOptions) (arvados.AuthorizedKey, error) { + as.appendCall(ctx, as.AuthorizedKeyUpdate, options) + return arvados.AuthorizedKey{}, as.Error +} +func (as *APIStub) AuthorizedKeyGet(ctx context.Context, options arvados.GetOptions) (arvados.AuthorizedKey, error) { + as.appendCall(ctx, as.AuthorizedKeyGet, options) + return arvados.AuthorizedKey{}, as.Error +} +func (as *APIStub) AuthorizedKeyList(ctx context.Context, options arvados.ListOptions) (arvados.AuthorizedKeyList, error) { + as.appendCall(ctx, as.AuthorizedKeyList, options) + return arvados.AuthorizedKeyList{}, as.Error +} +func (as *APIStub) AuthorizedKeyDelete(ctx context.Context, options arvados.DeleteOptions) (arvados.AuthorizedKey, error) { + as.appendCall(ctx, as.AuthorizedKeyDelete, options) + return arvados.AuthorizedKey{}, as.Error +} func (as *APIStub) CollectionCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Collection, error) { as.appendCall(ctx, as.CollectionCreate, options) return arvados.Collection{}, as.Error diff --git a/services/api/Gemfile b/services/api/Gemfile index 9b401cc6ac..3abc1d3b28 100644 --- a/services/api/Gemfile +++ b/services/api/Gemfile @@ -53,7 +53,6 @@ gem 'themes_for_rails', git: 'https://github.com/arvados/themes_for_rails' gem 'arvados', '~> 2.1.5' gem 'httpclient' -gem 'sshkey' gem 'safe_yaml' gem 'lograge' gem 'logstash-event' diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock index bdfaaf4efe..0fdfb8d1da 100644 --- a/services/api/Gemfile.lock +++ b/services/api/Gemfile.lock @@ -217,7 +217,6 @@ GEM actionpack (>= 5.2) activesupport (>= 5.2) sprockets (>= 3.0.0) - sshkey (2.0.0) test-unit (3.3.1) power_assert thor (1.2.1) @@ -260,7 +259,6 @@ DEPENDENCIES simplecov (~> 0.7.1) simplecov-rcov sprockets (~> 3.0) - sshkey test-unit (~> 3.0) themes_for_rails! diff --git a/services/api/app/models/authorized_key.rb b/services/api/app/models/authorized_key.rb index a5c5081c40..ce348e0f8a 100644 --- a/services/api/app/models/authorized_key.rb +++ b/services/api/app/models/authorized_key.rb @@ -37,17 +37,11 @@ class AuthorizedKey < ArvadosModel def public_key_must_be_unique if self.public_key - valid_key = SSHKey.valid_ssh_public_key? self.public_key - - if not valid_key - errors.add(:public_key, "does not appear to be a valid ssh-rsa or dsa public key") - else - # Valid if no other rows have this public key - if self.class.where('uuid != ? and public_key like ?', - uuid || '', "%#{self.public_key}%").any? - errors.add(:public_key, "already exists in the database, use a different key.") - return false - end + # Valid if no other rows have this public key + if self.class.where('uuid != ? and public_key like ?', + uuid || '', "%#{self.public_key}%").any? + errors.add(:public_key, "already exists in the database, use a different key.") + return false end end return true diff --git a/services/api/test/fixtures/authorized_keys.yml b/services/api/test/fixtures/authorized_keys.yml index 1c14204d98..b2b2c8be1b 100644 --- a/services/api/test/fixtures/authorized_keys.yml +++ b/services/api/test/fixtures/authorized_keys.yml @@ -5,6 +5,7 @@ active: uuid: zzzzz-fngyi-12nc9ov4osp8nae owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz + modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz authorized_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz key_type: SSH name: active @@ -13,6 +14,7 @@ active: admin: uuid: zzzzz-fngyi-g290j3i3u701duh owner_uuid: zzzzz-tpzed-d9tiejq69daie8f + modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f authorized_user_uuid: zzzzz-tpzed-d9tiejq69daie8f key_type: SSH name: admin @@ -21,6 +23,7 @@ admin: spectator: uuid: zzzzz-fngyi-3uze1ipbnz2c2c2 owner_uuid: zzzzz-tpzed-l1s2piq4t4mps8r + modified_by_user_uuid: zzzzz-tpzed-l1s2piq4t4mps8r authorized_user_uuid: zzzzz-tpzed-l1s2piq4t4mps8r key_type: SSH name: spectator @@ -29,6 +32,7 @@ spectator: project_viewer: uuid: zzzzz-fngyi-5d3av1396niwcej owner_uuid: zzzzz-tpzed-projectviewer1a + modified_by_user_uuid: zzzzz-tpzed-projectviewer1a authorized_user_uuid: zzzzz-tpzed-projectviewer1a key_type: SSH name: project_viewer