From 9c9ee9500190c63bc0802bbf4918bca5c9c2c358 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Mon, 3 May 2021 16:48:11 +0200 Subject: [PATCH] server,cli/demo: make the SQL and HTTP listeners configurable Prior to this change, a `demo` instance would restrict all its network listeners to 127.0.0.1. Since this choice was initially made, we discovered the utility of embedding a `demo` shell inside a Katakoda instance, where the Katakoda infrastructure is able to provide a secure HTTPS proxy with a public CA for a service running inside the sandbox. Since that proxying requires the service to be exposed on the sandbox's external address, we wanted the `demo` shell to make its listeners configurable. Release note (cli change): It is now possible to enable TLS for the web UI console of `cockroach demo` with the new flag `--secure-http`. This makes it possible to expose the HTTP interface to a shared network. Note however that this configuration still uses an ephemeral self-signed CA generated by `demo` itself, and is thus insufficient for use by end users web browsers directly: it should still be combined with a HTTP proxy that exposes the service using a public CA. This feature is not enabled by default. Release note (cli change): The network address bound by `cockroach demo` simulated nodes is now configurable via the new command-line flags `--http-addr` (for the HTTP interface) and `--sql-addr` (for the TCP interface for SQL clients). The default for these flags results in the same configuration as in previous versions, that is, the `demo` nodes only listen on 127.0.0.1. Note that `--http-addr` cannot be used without passing `--secure-http` explicitly (either via `--secure-http=true` or `--secure-http=false`), so as to clearly signal intent. Release note (cli change): The CommonName (CN) field of the TLS server certificate generated for the HTTP port in `cockroach demo` (when `--secure-http` is used) is now configurable via the new flag `--http-advertise-addr`. This is also the address displayed in the web UI URLs printed to the user via the `\demo ls` client-side command. The default for this value is the same as `--http-addr`. --- pkg/base/test_server_args.go | 3 ++ pkg/cli/cliflags/flags.go | 46 ++++++++++++++++++++++++++++++ pkg/cli/context.go | 9 ++++++ pkg/cli/demo.go | 25 ++++++++++++++++ pkg/cli/demo_cluster.go | 55 ++++++++++++++++++++++++++++++------ pkg/cli/flags.go | 5 ++++ pkg/server/testserver.go | 3 ++ 7 files changed, 137 insertions(+), 9 deletions(-) diff --git a/pkg/base/test_server_args.go b/pkg/base/test_server_args.go index eccb9d68c18f..2222f96f0821 100644 --- a/pkg/base/test_server_args.go +++ b/pkg/base/test_server_args.go @@ -59,6 +59,9 @@ type TestServerArgs struct { TenantAddr *string // HTTPAddr (if nonempty) is the HTTP address to use for the test server. HTTPAddr string + // HTTPAdvertiseAddr (if nonempty) is the advertised HTTP address to use. + HTTPAdvertiseAddr string + // DisableTLSForHTTP if set, disables TLS for the HTTP interface. DisableTLSForHTTP bool diff --git a/pkg/cli/cliflags/flags.go b/pkg/cli/cliflags/flags.go index a68dcc59bd81..4b7253eb202a 100644 --- a/pkg/cli/cliflags/flags.go +++ b/pkg/cli/cliflags/flags.go @@ -1125,6 +1125,13 @@ There should be as many TCP ports available as the value of --nodes starting at the specified value.`, } + DemoSQLAddr = FlagInfo{ + Name: "sql-addr", + Description: `The network address for demo nodes +to listen for connection requests from SQL clients. +The port number must not be included.`, + } + DemoHTTPPort = FlagInfo{ Name: "http-port", Description: `First port number for HTTP servers. @@ -1132,6 +1139,45 @@ There should be as many TCP ports available as the value of --nodes starting at the specified value.`, } + DemoHTTPAddr = FlagInfo{ + Name: "http-addr", + Description: `The network address for demo nodes +to listen for connection requests from HTTP clients. +The port number must not be included. + +Attention: if using --secure-http=false with a non-default +--http-addr, the resulting configuration may expose an unsecured +HTTP service over a shared network, resulting in possible +remote access to the local machine.`, + } + + DemoHTTPAdvertiseAddr = FlagInfo{ + Name: "http-advertise-addr", + Description: `The address to include in the web UI URL +displayed upon server startup and \demo ls. It is also +the address embedded in the server TLS certificate as +Common Name (CN) when the HTTPS console is enabled with +--secure-http. +The port number should be included; add :0 after the network +address to reuse the same HTTP port as each simulated node. + +The default is the same as --http-addr.`, + } + + DemoSecureHTTP = FlagInfo{ + Name: "secure-http", + Description: `If set, enable TLS for accessing the web UI. +This causes the demo nodes to use a self-signed +TLS certificate for the HTTP port. +Use --http-advertise-addr to control the network +address listed as Common Name (CN) in the TLS certificate. + +Attention: if using --secure-http=false with a non-default +--http-addr, the resulting configuration may expose an unsecured +HTTP service over a shared network, resulting in possible +remote access to the local machine.`, + } + DemoNodes = FlagInfo{ Name: "nodes", Description: `How many in-memory nodes to create for the demo.`, diff --git a/pkg/cli/context.go b/pkg/cli/context.go index 73586fbb8251..eb154a8ff410 100644 --- a/pkg/cli/context.go +++ b/pkg/cli/context.go @@ -564,6 +564,11 @@ var demoCtx struct { insecure bool sqlPort int httpPort int + + sqlAddr string + httpAddr string + httpAdvertiseAddr string + secureHTTP bool } // setDemoContextDefaults set the default values in demoCtx. This @@ -584,6 +589,10 @@ func setDemoContextDefaults() { demoCtx.insecure = false demoCtx.sqlPort, _ = strconv.Atoi(base.DefaultPort) demoCtx.httpPort, _ = strconv.Atoi(base.DefaultHTTPPort) + demoCtx.sqlAddr = "127.0.0.1" + demoCtx.httpAddr = "127.0.0.1" + demoCtx.httpAdvertiseAddr = "" + demoCtx.secureHTTP = false } // stmtDiagCtx captures the command-line parameters of the 'statement-diag' diff --git a/pkg/cli/demo.go b/pkg/cli/demo.go index bc7265087bed..3b448b0fd8d0 100644 --- a/pkg/cli/demo.go +++ b/pkg/cli/demo.go @@ -189,6 +189,31 @@ func checkDemoConfiguration( gen = defaultGenerator } + // Since --insecure disables all security controls, --secure-http would + // not work properly. Better prevent the combination altogether. + if demoCtx.insecure && demoCtx.secureHTTP { + return nil, errors.New("cannot enable HTTPS in insecure mode") + } + + fs := flagSetForCmd(cmd) + + // Ensure that an explicit --http-addr is always combined with + // and explicit --secure-http. + if fs.Lookup(cliflags.DemoHTTPAddr.Name).Changed && + !fs.Lookup(cliflags.DemoSecureHTTP.Name).Changed { + return nil, errors.WithHintf( + errors.Newf("using --%[1]s requires either --%[2]s=true or --%[2]s=false", + cliflags.DemoHTTPAddr.Name, cliflags.DemoSecureHTTP.Name), + "Note that --%[2]s=false with a non-default --%[1]s can result in undesired network accesses.", + cliflags.DemoHTTPAddr.Name, cliflags.DemoSecureHTTP.Name) + } + + // If --http-advertise-addr was not specified, fall back + // to the http addr. + if !fs.Lookup(cliflags.DemoHTTPAdvertiseAddr.Name).Changed { + demoCtx.httpAdvertiseAddr = demoCtx.httpAddr + ":0" + } + // Make sure that the user didn't request a workload and an empty database. if demoCtx.runWorkload && demoCtx.noExampleDatabase { return nil, errors.New("cannot run a workload when generation of the example database is disabled") diff --git a/pkg/cli/demo_cluster.go b/pkg/cli/demo_cluster.go index 80e3149e0ae4..c0a6bb04347f 100644 --- a/pkg/cli/demo_cluster.go +++ b/pkg/cli/demo_cluster.go @@ -105,7 +105,7 @@ func (c *transientCluster) checkConfigAndSetupLogging( } if !demoCtx.insecure { - if err := generateCerts(c.demoDir); err != nil { + if err := generateCerts(c.demoDir, demoCtx.secureHTTP, demoCtx.httpAdvertiseAddr); err != nil { return err } } @@ -560,11 +560,15 @@ func testServerArgsForTransientCluster( storeSpec.StickyInMemoryEngineID = fmt.Sprintf("demo-node%d", nodeID) args := base.TestServerArgs{ - SocketFile: sock.filename(), - PartOfCluster: true, - Stopper: stop.NewStopper(), - JoinAddr: joinAddr, - DisableTLSForHTTP: true, + SocketFile: sock.filename(), + PartOfCluster: true, + Stopper: stop.NewStopper(), + JoinAddr: joinAddr, + + SQLAddr: demoCtx.sqlAddr + ":0", // The port number may be overridden below. + HTTPAddr: demoCtx.httpAddr + ":0", // The port number may be overridden below. + HTTPAdvertiseAddr: demoCtx.httpAdvertiseAddr, + StoreSpecs: []base.StoreSpec{storeSpec}, SQLMemoryPoolSize: demoCtx.sqlPoolMemorySize, CacheSize: demoCtx.cacheSize, @@ -580,19 +584,27 @@ func testServerArgsForTransientCluster( }, } + if !demoCtx.secureHTTP { + args.DisableTLSForHTTP = true + } + if !testingForceRandomizeDemoPorts { // Unit tests can be run with multiple processes side-by-side with // `make stress`. This is bound to not work with fixed ports. sqlPort := sqlBasePort + int(nodeID) - 1 if sqlBasePort == 0 { + // In case the user specified zero explicitly, this is an + // instruction to revert to automatic allocation. sqlPort = 0 } httpPort := httpBasePort + int(nodeID) - 1 if httpBasePort == 0 { + // In case the user specified zero explicitly, this is an + // instruction to revert to automatic allocation. httpPort = 0 } - args.SQLAddr = fmt.Sprintf(":%d", sqlPort) - args.HTTPAddr = fmt.Sprintf(":%d", httpPort) + args.SQLAddr = fmt.Sprintf("%s:%d", demoCtx.sqlAddr, sqlPort) + args.HTTPAddr = fmt.Sprintf("%s:%d", demoCtx.httpAddr, httpPort) } if demoCtx.localities != nil { @@ -846,7 +858,32 @@ This server is running at increased risk of memory-related failures.`, } // generateCerts generates some temporary certificates for cockroach demo. -func generateCerts(certsDir string) (err error) { +func generateCerts(certsDir string, generateHTTPCerts bool, httpAdvAddr string) (err error) { + if generateHTTPCerts { + uicaKeyPath := filepath.Join(certsDir, security.EmbeddedUICAKey) + // Create a CA-Key for the UI certificates. + if err := security.CreateUICAPair( + certsDir, + uicaKeyPath, + defaultKeySize, + defaultCALifetime, + false, /* allowKeyReuse */ + false, /*overwrite */ + ); err != nil { + return err + } + // Generate a certificate for the demo nodes's HTTP interface. + if err := security.CreateUIPair( + certsDir, + uicaKeyPath, + defaultKeySize, + defaultCertLifetime, + false, /* overwrite */ + []string{httpAdvAddr}, + ); err != nil { + return err + } + } caKeyPath := filepath.Join(certsDir, security.EmbeddedCAKey) // Create a CA-Key. if err := security.CreateCAPair( diff --git a/pkg/cli/flags.go b/pkg/cli/flags.go index 1a448357ea26..3a16980cb704 100644 --- a/pkg/cli/flags.go +++ b/pkg/cli/flags.go @@ -780,6 +780,11 @@ func init() { intFlag(f, &demoCtx.sqlPort, cliflags.DemoSQLPort) intFlag(f, &demoCtx.httpPort, cliflags.DemoHTTPPort) + + stringFlag(f, &demoCtx.sqlAddr, cliflags.DemoSQLAddr) + stringFlag(f, &demoCtx.httpAddr, cliflags.DemoHTTPAddr) + stringFlag(f, &demoCtx.httpAdvertiseAddr, cliflags.DemoHTTPAdvertiseAddr) + boolFlag(f, &demoCtx.secureHTTP, cliflags.DemoSecureHTTP) } // statement-diag command. diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go index adb744b4bbe6..02b0781c0560 100644 --- a/pkg/server/testserver.go +++ b/pkg/server/testserver.go @@ -218,6 +218,9 @@ func makeTestConfigFromParams(params base.TestServerArgs) Config { if params.HTTPAddr != "" { cfg.HTTPAddr = params.HTTPAddr } + if params.HTTPAdvertiseAddr != "" { + cfg.HTTPAdvertiseAddr = params.HTTPAdvertiseAddr + } cfg.DisableTLSForHTTP = params.DisableTLSForHTTP if params.DisableWebSessionAuthentication { cfg.EnableWebSessionAuthentication = false