Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cli: add --cert-principal-map to client commands #47449

Merged
merged 1 commit into from
Apr 29, 2020
Merged

cli: add --cert-principal-map to client commands #47449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions pkg/cli/cert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,6 @@ List certificates and keys found in the certificate directory.

// runListCerts loads and lists all certs.
func runListCerts(cmd *cobra.Command, args []string) error {
if err := security.SetCertPrincipalMap(certCtx.certPrincipalMap); err != nil {
return err
}
cm, err := security.NewCertificateManager(baseCfg.SSLCertsDir)
if err != nil {
return errors.Wrap(err, "cannot load certificates")
Expand Down
12 changes: 4 additions & 8 deletions pkg/cli/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ func initCLIDefaults() {
cliCtx.cmdTimeout = 0 // no timeout
cliCtx.clientConnHost = ""
cliCtx.clientConnPort = base.DefaultPort
cliCtx.certPrincipalMap = nil
cliCtx.sqlConnURL = ""
cliCtx.sqlConnUser = ""
cliCtx.sqlConnPasswd = ""
Expand Down Expand Up @@ -173,8 +174,6 @@ func initCLIDefaults() {

authCtx.validityPeriod = 1 * time.Hour

certCtx.certPrincipalMap = nil

initPreFlagsDefaults()

// Clear the "Changed" state of all the registered command-line flags.
Expand Down Expand Up @@ -217,6 +216,9 @@ type cliContext struct {
// clientConnPort is the port name/number to use to connect to a server.
clientConnPort string

// certPrincipalMap is the cert-principal:db-principal map.
certPrincipalMap []string

// for CLI commands that use the SQL interface, these parameters
// determine how to connect to the server.
sqlConnURL, sqlConnUser, sqlConnDBName string
Expand Down Expand Up @@ -405,9 +407,3 @@ var demoCtx struct {
insecure bool
geoLibsDir string
}

// certCtx captures the command-line parameters of the `cert` command.
// Defaults set by InitCLIDefaults() above.
var certCtx struct {
certPrincipalMap []string
}
20 changes: 14 additions & 6 deletions pkg/cli/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,9 @@ func init() {

// Every command but start will inherit the following setting.
AddPersistentPreRunE(cockroachCmd, func(cmd *cobra.Command, _ []string) error {
extraClientFlagInit()
if err := extraClientFlagInit(); err != nil {
return err
}
return setDefaultStderrVerbosity(cmd, log.Severity_WARNING)
})

Expand Down Expand Up @@ -441,12 +443,11 @@ func init() {
f := cmd.Flags()
// All certs commands need the certificate directory.
StringFlag(f, &baseCfg.SSLCertsDir, cliflags.CertsDir, baseCfg.SSLCertsDir)
// All certs commands get the certificate principal map.
StringSlice(f, &cliCtx.certPrincipalMap,
cliflags.CertPrincipalMap, cliCtx.certPrincipalMap)
}

// The list certs command needs the certificate principal map.
StringSlice(listCertsCmd.Flags(), &certCtx.certPrincipalMap,
cliflags.CertPrincipalMap, certCtx.certPrincipalMap)

for _, cmd := range []*cobra.Command{createCACertCmd, createClientCACertCmd} {
f := cmd.Flags()
// CA certificates have a longer expiration time.
Expand Down Expand Up @@ -495,6 +496,9 @@ func init() {

// Certificate flags.
StringFlag(f, &baseCfg.SSLCertsDir, cliflags.CertsDir, baseCfg.SSLCertsDir)
// Certificate principal map.
StringSlice(f, &cliCtx.certPrincipalMap,
cliflags.CertPrincipalMap, cliCtx.certPrincipalMap)
}

// Auth commands.
Expand Down Expand Up @@ -878,7 +882,10 @@ func extraServerFlagInit(cmd *cobra.Command) error {
return nil
}

func extraClientFlagInit() {
func extraClientFlagInit() error {
if err := security.SetCertPrincipalMap(cliCtx.certPrincipalMap); err != nil {
return err
}
serverCfg.Addr = net.JoinHostPort(cliCtx.clientConnHost, cliCtx.clientConnPort)
serverCfg.AdvertiseAddr = serverCfg.Addr
serverCfg.SQLAddr = net.JoinHostPort(cliCtx.clientConnHost, cliCtx.clientConnPort)
Expand All @@ -894,6 +901,7 @@ func extraClientFlagInit() {
if sqlCtx.debugMode {
sqlCtx.echo = true
}
return nil
}

func setDefaultStderrVerbosity(cmd *cobra.Command, defaultSeverity log.Severity) error {
Expand Down
8 changes: 6 additions & 2 deletions pkg/cli/flags_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,9 @@ func TestServerJoinSettings(t *testing.T) {
t.Fatalf("Parse(%#v) got unexpected error: %v", td.args, err)
}

extraClientFlagInit()
if err := extraClientFlagInit(); err != nil {
t.Fatal(err)
}

var actual []string
myHostname, _ := os.Hostname()
Expand Down Expand Up @@ -861,7 +863,9 @@ func TestClientConnSettings(t *testing.T) {
t.Fatalf("Parse(%#v) got unexpected error: %v", td.args, err)
}

extraClientFlagInit()
if err := extraClientFlagInit(); err != nil {
t.Fatal(err)
}
if td.expectedAddr != serverCfg.Addr {
t.Errorf("%d. serverCfg.Addr expected '%s', but got '%s'. td.args was '%#v'.",
i, td.expectedAddr, serverCfg.Addr, td.args)
Expand Down
33 changes: 27 additions & 6 deletions pkg/cli/interactive_tests/test_cert_advisory_validation.tcl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ set prompt ":/# "
eexpect $prompt

# create some cert without an IP address in there.
set certs_dir "./my-safe-directory"
set db_dir "logs/db"
set certs_dir "logs/my-safe-directory"
send "mkdir -p $certs_dir\r"
eexpect $prompt

Expand All @@ -21,7 +22,7 @@ send "$argv cert create-node localhost --certs-dir=$certs_dir --ca-key=$certs_di
eexpect $prompt

start_test "Check that the server reports a warning if attempting to advertise an IP address not in cert."
send "$argv start-single-node --certs-dir=$certs_dir --advertise-addr=127.0.0.1\r"
send "$argv start-single-node --store=$db_dir --certs-dir=$certs_dir --advertise-addr=127.0.0.1\r"
eexpect "advertise address"
eexpect "127.0.0.1"
eexpect "not in node certificate"
Expand All @@ -32,7 +33,7 @@ eexpect $prompt
end_test

start_test "Check that the server reports no warning if the avertise addr is in the cert."
send "$argv start-single-node --certs-dir=$certs_dir --advertise-addr=localhost\r"
send "$argv start-single-node --store=$db_dir --certs-dir=$certs_dir --advertise-addr=localhost\r"
expect {
"not in node certificate" {
report "unexpected warning"
Expand All @@ -51,21 +52,21 @@ send "COCKROACH_CERT_NODE_USER=foo.bar $argv cert create-node localhost --certs-
eexpect $prompt

start_test "Check that the server reports an error if the node cert does not contain a node principal."
send "$argv start-single-node --certs-dir=$certs_dir --advertise-addr=localhost\r"
send "$argv start-single-node --store=$db_dir --certs-dir=$certs_dir --advertise-addr=localhost\r"
eexpect "cannot load certificates"
expect $prompt
end_test

start_test "Check that the cert principal map can allow the use of non-standard cert principal."
send "$argv start-single-node --certs-dir=$certs_dir --cert-principal-map=foo.bar:node --advertise-addr=localhost\r"
send "$argv start-single-node --store=$db_dir --certs-dir=$certs_dir --cert-principal-map=foo.bar:node --advertise-addr=localhost\r"
eexpect "node starting"
interrupt
eexpect "interrupted"
expect $prompt
end_test

start_test "Check that the cert principal map can allow the use of a SAN principal."
send "$argv start-single-node --certs-dir=$certs_dir --cert-principal-map=localhost:node --advertise-addr=localhost\r"
send "$argv start-single-node --store=$db_dir --certs-dir=$certs_dir --cert-principal-map=localhost:node --advertise-addr=localhost\r"
eexpect "node starting"
interrupt
eexpect "interrupted"
Expand All @@ -77,3 +78,23 @@ send "$argv cert list --certs-dir=$certs_dir --cert-principal-map=foo.bar:node\r
eexpect "Certificate directory:"
expect $prompt
end_test

start_test "Check that 'cert create-client' can utilize cert principal map."
send "$argv cert create-client root.crdb.io --certs-dir=$certs_dir --ca-key=$certs_dir/ca.key --cert-principal-map=foo.bar:node\r"
eexpect $prompt
send "mv $certs_dir/client.root.crdb.io.crt $certs_dir/client.root.crt; mv $certs_dir/client.root.crdb.io.key $certs_dir/client.root.key\r"
eexpect $prompt
end_test

start_test "Check that the client commands can use cert principal map."
system "$argv start-single-node --store=$db_dir --certs-dir=$certs_dir --cert-principal-map=foo.bar:node,root.crdb.io:root --advertise-addr=localhost --background >>expect-cmd.log 2>&1"
send "$argv sql --certs-dir=$certs_dir --cert-principal-map=foo.bar:node,root.crdb.io:root -e \"select 'hello'\"\r"
eexpect "hello"
expect $prompt
send "$argv node ls --certs-dir=$certs_dir --cert-principal-map=foo.bar:node,root.crdb.io:root\r"
eexpect "1 row"
expect $prompt
send "$argv quit --certs-dir=$certs_dir --cert-principal-map=foo.bar:node,root.crdb.io:root\r"
eexpect "ok"
expect $prompt
end_test