Skip to content

Commit

Permalink
*: allow periods (.) in usernames
Browse files Browse the repository at this point in the history
Requested by a user:

> Currently, there is a restriction for the database username which
> will limit the certificate-based authentication. It's very common to
> include .local (e.g.: internal-service2.local) in the CN (Common Name)
> of a certificate.  The AWS Certificate Manager (ACM) won't even issue
> a certificate if the "dot" (.) is not present.

Release note (sql change): Usernames can now contain periods, for
compatibility with certificate managers that require domain names to
be used as usernames.

Release note (security update): The validation rule for principal
names was extended to support periods and thus allow domainname-like
principals. For reference, the validation rule is currently the regular
expression `^[\p{Ll}0-9_][---\p{Ll}0-9_.]+$` and a size limit of 63
UTF-8 bytes (larger usernames are rejected with an error); for
comparison, PostgreSQL allows many more characters and truncates at 63
chacters silently.
  • Loading branch information
knz committed Mar 2, 2020
1 parent 860c137 commit f978076
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 9 deletions.
2 changes: 1 addition & 1 deletion pkg/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1339,7 +1339,7 @@ func Example_cert() {
// cert create-client Ομηρος
// cert create-client 0foo
// cert create-client ,foo
// failed to generate client certificate and key: username ",foo" invalid; usernames are case insensitive, must start with a letter, digit or underscore, may contain letters, digits, dashes, or underscores, and must not exceed 63 characters
// failed to generate client certificate and key: username ",foo" invalid
}

// TestFlagUsage is a basic test to make sure the fragile
Expand Down
13 changes: 8 additions & 5 deletions pkg/sql/create_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,10 @@ func (*CreateUserNode) Close(context.Context) {}
// FastPathResults implements the planNodeFastPath interface.
func (n *CreateUserNode) FastPathResults() (int, bool) { return n.run.rowsAffected, true }

const usernameHelp = "usernames are case insensitive, must start with a letter, " +
"digit or underscore, may contain letters, digits, dashes, or underscores, and must not exceed 63 characters"
const usernameHelp = "Usernames are case insensitive, must start with a letter, " +
"digit or underscore, may contain letters, digits, dashes, periods, or underscores, and must not exceed 63 characters."

var usernameRE = regexp.MustCompile(`^[\p{Ll}0-9_][\p{Ll}0-9_-]{0,62}$`)
var usernameRE = regexp.MustCompile(`^[\p{Ll}0-9_][---\p{Ll}0-9_.]+$`)

var blacklistedUsernames = map[string]struct{}{
security.NodeUser: {},
Expand All @@ -214,7 +214,7 @@ func NormalizeAndValidateUsername(username string) (string, error) {
return "", err
}
if _, ok := blacklistedUsernames[username]; ok {
return "", errors.Errorf("username %q reserved", username)
return "", pgerror.Newf(pgcode.ReservedName, "username %q reserved", username)
}
return username, nil
}
Expand All @@ -224,7 +224,10 @@ func NormalizeAndValidateUsername(username string) (string, error) {
func NormalizeAndValidateUsernameNoBlacklist(username string) (string, error) {
username = tree.Name(username).Normalize()
if !usernameRE.MatchString(username) {
return "", errors.Errorf("username %q invalid; %s", username, usernameHelp)
return "", errors.WithHint(pgerror.Newf(pgcode.InvalidName, "username %q invalid", username), usernameHelp)
}
if len(username) > 63 {
return "", errors.WithHint(pgerror.Newf(pgcode.NameTooLong, "username %q is too long", username), usernameHelp)
}
return username, nil
}
Expand Down
63 changes: 63 additions & 0 deletions pkg/sql/create_user_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright 2020 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package sql_test

import (
"testing"

"github.com/cockroachdb/cockroach/pkg/sql"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
)

func TestUserName(t *testing.T) {
defer leaktest.AfterTest(t)()

testCases := []struct {
username string
normalized string
err string
sqlstate string
}{
{"Abc123", "abc123", "", ""},
{"0123121132", "0123121132", "", ""},
{"HeLlO", "hello", "", ""},
{"Ομηρος", "ομηρος", "", ""},
{"_HeLlO", "_hello", "", ""},
{"a-BC-d", "a-bc-d", "", ""},
{"A.Bcd", "a.bcd", "", ""},
{"WWW.BIGSITE.NET", "www.bigsite.net", "", ""},
{"", "", `username "" invalid`, pgcode.InvalidName},
{"-ABC", "", `username "-abc" invalid`, pgcode.InvalidName},
{".ABC", "", `username ".abc" invalid`, pgcode.InvalidName},
{"*.wildcard", "", `username "\*.wildcard" invalid`, pgcode.InvalidName},
{"foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoof", "", `username "foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoof" is too long`, pgcode.NameTooLong},
}

for _, tc := range testCases {
normalized, err := sql.NormalizeAndValidateUsername(tc.username)
if !testutils.IsError(err, tc.err) {
t.Errorf("%q: expected %q, got %v", tc.username, tc.err, err)
continue
}
if err != nil {
if pgcode := pgerror.GetPGCode(err); pgcode != tc.sqlstate {
t.Errorf("%q: expected SQLSTATE %s, got %s", tc.username, tc.sqlstate, pgcode)
continue
}
}
if normalized != tc.normalized {
t.Errorf("%q: expected %q, got %q", tc.username, tc.normalized, normalized)
}
}
}
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/drop_user
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ DROP USER IF EXISTS user1
statement error username "node" reserved
DROP USER node

statement error pq: username "foo☂" invalid; usernames are case insensitive, must start with a letter, digit or underscore, may contain letters, digits, dashes, or underscores, and must not exceed 63 characters
statement error pq: username "foo☂" invalid
DROP USER foo☂

statement ok
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/user
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ CREATE USER uSEr2 WITH PASSWORD 'cockroach'
statement ok
CREATE USER user3 WITH PASSWORD '蟑螂'

statement error pq: username "foo☂" invalid; usernames are case insensitive, must start with a letter, digit or underscore, may contain letters, digits, dashes, or underscores, and must not exceed 63 characters
statement error pq: username "foo☂" invalid
CREATE USER foo☂

statement error pq: username "-foo" invalid; usernames are case insensitive, must start with a letter, digit or underscore, may contain letters, digits, dashes, or underscores, and must not exceed 63 characters
statement error pq: username "-foo" invalid
CREATE USER "-foo"

statement error at or near "-": syntax error
Expand Down

0 comments on commit f978076

Please sign in to comment.