Skip to content

Commit

Permalink
security,server: Misc fixes in node-join code
Browse files Browse the repository at this point in the history
A couple changes in this commit:
 - Capitalize CA consistently in protobuf structs and method names
 - Write all CAs/certs from the right places in InitializeFromConfig
   instead of writing the InterNode one everywhere
 - Create a `client.node.crt` signed by `ca-client.crt`, otherwise
   nodes wouldn't be able to connect to each other. Fixes cockroachdb#61624.

Release note: None.
  • Loading branch information
itsbilal committed May 5, 2021
1 parent 089511a commit 822b75b
Show file tree
Hide file tree
Showing 15 changed files with 2,389 additions and 3,222 deletions.
16 changes: 8 additions & 8 deletions docs/generated/http/full.md
Original file line number Diff line number Diff line change
Expand Up @@ -2887,7 +2887,7 @@ Support status: [reserved](#support-status)



CaRequest requests the CA cert anchoring this service.
CARequest requests the CA cert anchoring this service.

No information needed.

Expand All @@ -2903,12 +2903,12 @@ No information needed.



CaResponse contains a PEM encoded copy of the CA cert for this service.
CAResponse contains a PEM encoded copy of the CA cert for this service.


| Field | Type | Label | Description | Support status |
| ----- | ---- | ----- | ----------- | -------------- |
| ca_cert | [bytes](#cockroach.server.serverpb.CaResponse-bytes) | | query is the SQL query string. | [reserved](#support-status) |
| ca_cert | [bytes](#cockroach.server.serverpb.CAResponse-bytes) | | | [reserved](#support-status) |



Expand All @@ -2929,15 +2929,15 @@ Support status: [reserved](#support-status)



BundleRequest requests the bundle of initialization CAs for a new node.
CertBundleRequest requests the bundle of initialization CAs for a new node.
It provides authentication in the form of a joinToken containing a
sharedSecret.


| Field | Type | Label | Description | Support status |
| ----- | ---- | ----- | ----------- | -------------- |
| token_id | [string](#cockroach.server.serverpb.BundleRequest-string) | | sharedSecret | [reserved](#support-status) |
| shared_secret | [string](#cockroach.server.serverpb.BundleRequest-string) | | | [reserved](#support-status) |
| token_id | [string](#cockroach.server.serverpb.CertBundleRequest-string) | | | [reserved](#support-status) |
| shared_secret | [bytes](#cockroach.server.serverpb.CertBundleRequest-bytes) | | | [reserved](#support-status) |



Expand All @@ -2950,13 +2950,13 @@ sharedSecret.



BundleResponse contains a copy of all CAs needed to intialize TLS for
CertBundleResponse contains a copy of all CAs needed to intialize TLS for
a new node.


| Field | Type | Label | Description | Support status |
| ----- | ---- | ----- | ----------- | -------------- |
| bundle | [bytes](#cockroach.server.serverpb.BundleResponse-bytes) | | query is the SQL query string. | [reserved](#support-status) |
| bundle | [bytes](#cockroach.server.serverpb.CertBundleResponse-bytes) | | | [reserved](#support-status) |



Expand Down
3 changes: 3 additions & 0 deletions pkg/cli/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ load("//build:STRINGER.bzl", "stringer")
go_library(
name = "cli",
srcs = [
"addjoin.go",
"auth.go",
"cert.go",
"cli.go",
Expand Down Expand Up @@ -262,6 +263,7 @@ go_test(
name = "cli_test",
size = "large",
srcs = [
"addjoin_test.go",
"cli_debug_test.go",
"cli_test.go",
"debug_check_store_test.go",
Expand Down Expand Up @@ -310,6 +312,7 @@ go_test(
"//pkg/server/serverpb",
"//pkg/server/status",
"//pkg/server/status/statuspb:statuspb_go_proto",
"//pkg/settings/cluster",
"//pkg/sql",
"//pkg/sql/catalog/descpb",
"//pkg/sql/lex",
Expand Down
120 changes: 68 additions & 52 deletions pkg/cli/addjoin.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import (
"encoding/json"
"encoding/pem"

"github.com/cockroachdb/cockroach/pkg/cli/cliflags"
"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/errors"

"github.com/cockroachdb/cockroach/pkg/rpc"
Expand All @@ -26,13 +29,13 @@ import (
)

var nodeJoinCmd = &cobra.Command{
Use: "node-join <remote-addr>",
Use: "node-join <join-token>",
Short: "request the TLS certs for a new node from an existing node",
Args: cobra.MinimumNArgs(1),
RunE: MaybeDecorateGRPCError(runNodeJoin),
}

func requestPeerCA(ctx context.Context, peer string, joinToken string) (*x509.CertPool, error) {
func requestPeerCA(ctx context.Context, peer string, jt security.JoinToken) (*x509.CertPool, error) {
var dialOpts []grpc.DialOption

dialOpts = rpc.GetAddJoinDialOptions(nil)
Expand All @@ -44,19 +47,14 @@ func requestPeerCA(ctx context.Context, peer string, joinToken string) (*x509.Ce

s := serverpb.NewAdminClient(conn)

req := serverpb.CaRequest{}
req := serverpb.CARequest{}
resp, err := s.RequestCA(ctx, &req)
if err != nil {
return nil, errors.Wrap(
err, "failed grpc call to request CA from peer")
}

// Verify that the received bytes match our expected MAC.
isTrustedCA, err := server.IsCATrustedByJoinToken(resp.CaCert, joinToken)
if err != nil {
return nil, errors.Wrap(err, "failed to validate remote CA")
}
if !isTrustedCA {
if !jt.VerifySignature(resp.CaCert) {
return nil, errors.New("resp.CaCert failed cryptologic validation")
}

Expand All @@ -75,50 +73,9 @@ func requestPeerCA(ctx context.Context, peer string, joinToken string) (*x509.Ce
return certPool, nil
}

// runNodeJoin will attempt to connect to peers from the list provided and
// request a certificate initialization bundle if it is able to validate a
// peer.
// TODO(aaron-crl): Parallelize this and handle errors.
func runNodeJoin(cmd *cobra.Command, args []string) error {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

// TODO(aaron-crl) plumb this into the connect or start commands or both.
var err error
peerAddr := args[0]
jt := args[1]

// TODO(aaron-crl): loop over peers here.
// For each peer, attempt to validate it's CA, then request a bundle with
// the supplied join-token. For now loop this to reduce complexity.
certPool, err := requestPeerCA(ctx, peerAddr, jt)
if err != nil {
return errors.Wrapf(
err, "failed requesting peer CA from %q", peerAddr)
}

// TODO(aaron-crl): Update add/join to signal to client when a token IS
// consumed.
certBundle, err := requestCertBundle(ctx, peerAddr, certPool)
if err != nil {
return errors.Wrapf(
err, "failed requesting certBundle from peer %q, token may have been consumed", peerAddr)
}

// Use the bundle to initialize the node.
err = certBundle.InitializeNodeFromBundle(ctx, *baseCfg)
if err != nil {
return errors.Wrap(
err,
"failed to initialize node after consuming join-token",
)
}

return nil
}

func requestCertBundle(
ctx context.Context, peerAddr string, certPool *x509.CertPool,
jt security.JoinToken,
) (*server.CertificateBundle, error) {
var dialOpts []grpc.DialOption
dialOpts = rpc.GetAddJoinDialOptions(certPool)
Expand All @@ -129,7 +86,10 @@ func requestCertBundle(
}

s := serverpb.NewAdminClient(conn)
req := serverpb.BundleRequest{}
req := serverpb.CertBundleRequest{
TokenID: jt.TokenID.String(),
SharedSecret: jt.SharedSecret,
}
resp, err := s.RequestCertBundle(ctx, &req)
if err != nil {
return nil, errors.Wrapf(
Expand All @@ -151,3 +111,59 @@ func requestCertBundle(

return &certBundle, nil
}

// runNodeJoin will attempt to connect to peers from the join list provided and
// request a certificate initialization bundle if it is able to validate a
// peer.
// TODO(aaron-crl): Parallelize this and handle errors.
func runNodeJoin(cmd *cobra.Command, args []string) error {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

if err := validateNodeJoinFlags(cmd); err != nil {
return err
}

joinTokenArg := args[0]
var jt security.JoinToken
if err := jt.UnmarshalText([]byte(joinTokenArg)); err != nil {
return errors.Wrap(err, "failed to parse join token")
}

for _, peer := range serverCfg.JoinList {
certPool, err := requestPeerCA(ctx, peer, jt)
if err != nil {
// Try a different peer.
log.Errorf(ctx, "failed requesting peer CA from %s: %s", peer, err)
continue
}

// TODO(aaron-crl): Update add/join to signal to client when a token IS
// consumed.
certBundle, err := requestCertBundle(ctx, peer, certPool, jt)
if err != nil {
return errors.Wrapf(err,
"failed requesting certBundle from peer %q, token may have been consumed", peer)
}

// Use the bundle to initialize the node.
err = certBundle.InitializeNodeFromBundle(ctx, *baseCfg)
if err != nil {
return errors.Wrap(
err,
"failed to initialize node after consuming join-token",
)
}
return nil
}

return errors.New("could not successfully authenticate with any listed nodes")
}

func validateNodeJoinFlags(_ *cobra.Command) error {
if len(serverCfg.JoinList) == 0 {
return errors.Newf("flag --%s must specify address of at least one node to join",
cliflags.Join.Name)
}
return nil
}
2 changes: 0 additions & 2 deletions pkg/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,6 @@ func init() {
connectCmd,
initCmd,
certCmd,
// TODO(bilal): Uncomment this when the connect command does something useful.
// connectCmd,
nodeJoinCmd,
quitCmd,

Expand Down
1 change: 1 addition & 0 deletions pkg/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1094,6 +1094,7 @@ Available Commands:
connect auto-build TLS certificates for use with the start command
init initialize a cluster
cert create ca, node, and client certs
node-join request the TLS certs for a new node from an existing node
sql open a sql shell
statement-diag commands for managing statement diagnostics bundles
auth-session log in and out of HTTP sessions
Expand Down
12 changes: 9 additions & 3 deletions pkg/cli/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func init() {
// multi-tenancy related commands that start long-running servers.
// Also for `connect` which does not really start a server but uses
// all the networking flags.
for _, cmd := range append(serverCmds, connectCmd) {
for _, cmd := range append(serverCmds, connectCmd, nodeJoinCmd) {
AddPersistentPreRunE(cmd, func(cmd *cobra.Command, _ []string) error {
// Finalize the configuration of network settings.
return extraServerFlagInit(cmd)
Expand Down Expand Up @@ -339,8 +339,9 @@ func init() {
// avoid printing some messages to standard output in that case.
_, startCtx.inBackground = envutil.EnvString(backgroundEnvVar, 1)

// Flags common to the start commands and the connect command.
for _, cmd := range append(StartCmds, connectCmd) {
// Flags common to the start commands, the connect command, and the node join
// command.
for _, cmd := range append(StartCmds, connectCmd, nodeJoinCmd) {
f := cmd.Flags()

varFlag(f, addrSetter{&startCtx.serverListenAddr, &serverListenPort}, cliflags.ListenAddr)
Expand All @@ -359,6 +360,11 @@ func init() {
// 'start' will check that the flag is properly defined.
varFlag(f, &serverCfg.JoinList, cliflags.Join)
boolFlag(f, &serverCfg.JoinPreferSRVRecords, cliflags.JoinPreferSRVRecords)
}

// Flags common to the start commands and the connect command.
for _, cmd := range append(StartCmds, connectCmd) {
f := cmd.Flags()

// The initialization token and expected peers. For 'start' commands this is optional.
stringFlag(f, &startCtx.initToken, cliflags.InitToken)
Expand Down
1 change: 1 addition & 0 deletions pkg/rpc/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
go_library(
name = "rpc",
srcs = [
"addjoin.go",
"auth.go",
"auth_tenant.go",
"breaker.go",
Expand Down
Loading

0 comments on commit 822b75b

Please sign in to comment.