Skip to content
Permalink
Browse files Browse the repository at this point in the history
[Backport] Update topo {Get,Create}Keyspace to prevent invalid keyspa…
…ce names (#12732) (#12771)

* Update topo {Get,Create}Keyspace to prevent invalid keyspace names (#12732)

* Update topo {Get,Create}Keyspace to prevent invalid keyspace names

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* embarrassing

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* docs, release notes

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* only validate, do not correct

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* broader restrictions via allow-list

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* Revert "broader restrictions via allow-list"

This reverts commit 5354d4f.

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* tighten up release notes scope

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* Update go/vt/topo/keyspace.go

Co-authored-by: Florent Poinsard <35779988+frouioui@users.noreply.github.com>
Signed-off-by: Andrew Mason <andrew@planetscale.com>

---------

Signed-off-by: Andrew Mason <andrew@planetscale.com>
Co-authored-by: Florent Poinsard <35779988+frouioui@users.noreply.github.com>

* update notes and comments

Signed-off-by: Andrew Mason <andrew@planetscale.com>

---------

Signed-off-by: Andrew Mason <andrew@planetscale.com>
Co-authored-by: Florent Poinsard <35779988+frouioui@users.noreply.github.com>
  • Loading branch information
ajm188 and frouioui committed Mar 30, 2023
1 parent 098913a commit adf1019
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 0 deletions.
6 changes: 6 additions & 0 deletions changelog/16.0/16.0.1/summary.md
Expand Up @@ -7,3 +7,9 @@ Below is a summary of this Go patch release. You can learn more [here](https://g

> go1.20.2 (released 2023-03-07) includes a security fix to the crypto/elliptic package, as well as bug fixes to the compiler, the covdata command, the linker, the runtime, and the crypto/ecdh, crypto/rsa, crypto/x509, os, and syscall packages.
### Keyspace name validation in TopoServer

Prior to v16.0.1, it was possible to create a keyspace with invalid characters, which would then be inaccessible to various cluster management operations.

Keyspace names may no longer contain the forward slash ("/") character, and TopoServer's `GetKeyspace` and `CreateKeyspace` methods return an error if given such a name.

23 changes: 23 additions & 0 deletions go/vt/topo/keyspace.go
Expand Up @@ -18,6 +18,7 @@ package topo

import (
"path"
"strings"

"google.golang.org/protobuf/proto"

Expand Down Expand Up @@ -54,6 +55,20 @@ func (ki *KeyspaceInfo) SetKeyspaceName(name string) {
ki.keyspace = name
}

var invalidKeyspaceNameChars = "/"

// ValidateKeyspaceName checks if the provided name is a valid name for a
// keyspace.
//
// As of v16.0.1, "all invalid characters" is just the forward slash ("/").
func ValidateKeyspaceName(name string) error {
if strings.ContainsAny(name, invalidKeyspaceNameChars) {
return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "keyspace name %s contains invalid characters; may not contain any of the following: %+v", name, strings.Split(invalidKeyspaceNameChars, ""))
}

return nil
}

// GetServedFrom returns a Keyspace_ServedFrom record if it exists.
func (ki *KeyspaceInfo) GetServedFrom(tabletType topodatapb.TabletType) *topodatapb.Keyspace_ServedFrom {
for _, ksf := range ki.ServedFroms {
Expand Down Expand Up @@ -161,6 +176,10 @@ func (ki *KeyspaceInfo) ComputeCellServedFrom(cell string) []*topodatapb.SrvKeys
// CreateKeyspace wraps the underlying Conn.Create
// and dispatches the event.
func (ts *Server) CreateKeyspace(ctx context.Context, keyspace string, value *topodatapb.Keyspace) error {
if err := ValidateKeyspaceName(keyspace); err != nil {
return vterrors.Wrapf(err, "CreateKeyspace: %s", err)
}

data, err := proto.Marshal(value)
if err != nil {
return err
Expand All @@ -181,6 +200,10 @@ func (ts *Server) CreateKeyspace(ctx context.Context, keyspace string, value *to

// GetKeyspace reads the given keyspace and returns it
func (ts *Server) GetKeyspace(ctx context.Context, keyspace string) (*KeyspaceInfo, error) {
if err := ValidateKeyspaceName(keyspace); err != nil {
return nil, vterrors.Wrapf(err, "GetKeyspace: %s", err)
}

keyspacePath := path.Join(KeyspacesPath, keyspace, KeyspaceFile)
data, version, err := ts.globalCell.Get(ctx, keyspacePath)
if err != nil {
Expand Down
72 changes: 72 additions & 0 deletions go/vt/topo/topotests/keyspace_test.go
@@ -0,0 +1,72 @@
/*
Copyright 2023 The Vitess Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package topotests

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"vitess.io/vitess/go/vt/topo/memorytopo"
"vitess.io/vitess/go/vt/vterrors"

topodatapb "vitess.io/vitess/go/vt/proto/topodata"
"vitess.io/vitess/go/vt/proto/vtrpc"
)

func TestCreateKeyspace(t *testing.T) {
ts := memorytopo.NewServer("zone1")
ctx := context.Background()

t.Run("valid name", func(t *testing.T) {
err := ts.CreateKeyspace(ctx, "ks", &topodatapb.Keyspace{})
require.NoError(t, err)
})
t.Run("invalid name", func(t *testing.T) {
err := ts.CreateKeyspace(ctx, "no/slashes/allowed", &topodatapb.Keyspace{})
assert.Error(t, err)
assert.Equal(t, vtrpc.Code_INVALID_ARGUMENT, vterrors.Code(err), "%+v", err)
})
}

func TestGetKeyspace(t *testing.T) {
ts := memorytopo.NewServer("zone1")
ctx := context.Background()

t.Run("valid name", func(t *testing.T) {
// First, create the keyspace.
err := ts.CreateKeyspace(ctx, "ks", &topodatapb.Keyspace{})
require.NoError(t, err)

// Now, get it.
ks, err := ts.GetKeyspace(ctx, "ks")
require.NoError(t, err)
assert.NotNil(t, ks)
})

t.Run("invalid name", func(t *testing.T) {
// We can't create the keyspace (because we can't create a keyspace
// with an invalid name), so we'll validate the error we get is *not*
// NOT_FOUND.
ks, err := ts.GetKeyspace(ctx, "no/slashes/allowed")
assert.Error(t, err)
assert.Equal(t, vtrpc.Code_INVALID_ARGUMENT, vterrors.Code(err), "%+v", err)
assert.Nil(t, ks)
})
}
4 changes: 4 additions & 0 deletions go/vt/vtorc/inst/keyspace_dao.go
Expand Up @@ -30,6 +30,10 @@ var ErrKeyspaceNotFound = errors.New("keyspace not found")

// ReadKeyspace reads the vitess keyspace record.
func ReadKeyspace(keyspaceName string) (*topo.KeyspaceInfo, error) {
if err := topo.ValidateKeyspaceName(keyspaceName); err != nil {
return nil, err
}

query := `
select
keyspace_type,
Expand Down

0 comments on commit adf1019

Please sign in to comment.