server,sql: Add new JoinToken struct, generator, feature flag#61505
server,sql: Add new JoinToken struct, generator, feature flag#61505craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
pkg/server/status.go
Outdated
| } | ||
|
|
||
| // TODO(bilal): This is definitely wrong. | ||
| f, err := os.Open(certLocator.CACertPath()) |
There was a problem hiding this comment.
@aaron-crl This likely calls for a "get me the CA cert" function inside `pkg/security, right? Is there an obvious way to get hold of a certificate manager to get this?
7be180b to
1bbe759
Compare
aaron-crl
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @itsbilal and @knz)
pkg/server/status.go, line 2568 at r1 (raw file):
Previously, itsbilal (Bilal Akhtar) wrote…
@aaron-crl This likely calls for a "get me the CA cert" function inside `pkg/security, right? Is there an obvious way to get hold of a certificate manager to get this?
I've got the same need in the GRPC endpoint. I'm using the below code to get it there.
Since you're in server, you should have access to loadCertificateFile, so something like this (copied from my GRPC code):
// RequestCA makes it possible for a node to request the node-to-node CA certificate.
func (s *adminServer) RequestCA(
ctx context.Context, req *serverpb.CaRequest,
) (*serverpb.CaResponse, error) {
cl := security.MakeCertsLocator(s.server.cfg.SSLCertsDir)
caCert, err := loadCertificateFile(cl.CACertPath())
if err != nil {
return nil, errors.Wrapf(
err,
"failed to read inter-node cert from disk at %q ",
cl.CACertPath(),
)
}
[...]
}
When a join token is generated, gossip it among other connected nodes with a short TTL. This allows that join token to be used when connecting to any node in the cluster, and not necessarily to the same node where the join token was generated. Also add an isValid function to the join token to confirm (on the receiver end) that the join token matches what was stored in gossip. First commit is cockroachdb#61505. Part of cockroachdb#60632. Release justification: Changes to new code paths, gated behind experimental feature flag. Release note: None.
1bbe759 to
4bc6c88
Compare
itsbilal
left a comment
There was a problem hiding this comment.
TFTR! Tests added.
Reviewable status:
pkg/server/status.go, line 2568 at r1 (raw file):
Previously, aaron-crl wrote…
I've got the same need in the GRPC endpoint. I'm using the below code to get it there.
Since you're in server, you should have access to
loadCertificateFile, so something like this (copied from my GRPC code):// RequestCA makes it possible for a node to request the node-to-node CA certificate. func (s *adminServer) RequestCA( ctx context.Context, req *serverpb.CaRequest, ) (*serverpb.CaResponse, error) { cl := security.MakeCertsLocator(s.server.cfg.SSLCertsDir) caCert, err := loadCertificateFile(cl.CACertPath()) if err != nil { return nil, errors.Wrapf( err, "failed to read inter-node cert from disk at %q ", cl.CACertPath(), ) } [...] }
This is done now - thanks!
aaron-crl
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/server/join_token_test.go, line 61 at r2 (raw file):
caCertFile := path.Join(tempDir, security.CACertFilename()) caCert := []byte("foobar") require.NoError(t, ioutil.WriteFile(caCertFile, caCert, 0666))
nit: This might eventually fall afoul of cert load tests that check permissions. Consider 0600.
4bc6c88 to
2c3b31c
Compare
2c3b31c to
20b6852
Compare
itsbilal
left a comment
There was a problem hiding this comment.
TFTR!
Reviewable status:
pkg/server/join_token_test.go, line 61 at r2 (raw file):
Previously, aaron-crl wrote…
nit: This might eventually fall afoul of cert load tests that check permissions. Consider 0600.
Done.
20b6852 to
df249b3
Compare
df249b3 to
cb48e0c
Compare
cb48e0c to
4d9cd5e
Compare
When a join token is generated, gossip it among other connected nodes with a short TTL. This allows that join token to be used when connecting to any node in the cluster, and not necessarily to the same node where the join token was generated. Also add an isValid function to the join token to confirm (on the receiver end) that the join token matches what was stored in gossip. First commit is cockroachdb#61505. Part of cockroachdb#60632. Release justification: Changes to new code paths, gated behind experimental feature flag. Release note: None.
This change adds a new JoinToken struct in the server package for use in TLS auto-join (see RFC cockroachdb#51991). It also adds a feature flag to hide this feature except for opt-in use, as well as a generator function to the status server that can be used by a future SQL statement that would generate and return join tokens. The feature flag is also in the SQL package so it can be used by both SQL and server. Part of cockroachdb#60632. Release justification: All changes are to new code paths, and are gated behind a feature flag. Release note: None.
4d9cd5e to
d738ecd
Compare
|
bors r=aaron-crl |
|
Build succeeded: |
When a join token is generated, gossip it among other connected nodes with a short TTL. This allows that join token to be used when connecting to any node in the cluster, and not necessarily to the same node where the join token was generated. Also add an isValid function to the join token to confirm (on the receiver end) that the join token matches what was stored in gossip. First commit is cockroachdb#61505. Part of cockroachdb#60632. Release justification: Changes to new code paths, gated behind experimental feature flag. Release note: None.
* removed stubs and dummy functions * cleaned up TODOs Release justification: low risk, feature flagged functional enhancement Release note: None
knz
left a comment
There was a problem hiding this comment.
Reviewed 1 of 4 files at r1, 6 of 6 files at r3.
Reviewable status:
pkg/server/join_token.go, line 81 at r3 (raw file):
// // The format of the text (after base64-decoding) is: // <tokenID:uuid.Size><sharedSecret:joinTokenSecretLen><fingerprint:variable><crc:4>
I would have recommended a version byte in front, so that we can handle multiple formats of join tokens. Can you process this in a followup PR?
pkg/server/status.go, line 2549 at r3 (raw file):
// subsystem directly. The response is a base64 marshaled form of the join token // that can be shared to new nodes that want to join this cluster. func (s *statusServer) GenerateJoinToken(ctx context.Context) (string, error) {
Can you explain the rationale of making this a method of statusServer? I though the entire join protocol would be part of the admin interface? Or the new API v2 tree?
* removed stubs and dummy functions * cleaned up TODOs Release justification: low risk, feature flagged functional enhancement Release note: None
* removed stubs and dummy functions * cleaned up TODOs Release justification: low risk, feature flagged functional enhancement Release note: None
This change adds a new JoinToken struct in the server
package for use in TLS auto-join (see RFC #51991).
It also adds a feature flag to hide this feature except
for opt-in use, as well as a generator function to the status
server that can be used by a future SQL statement that would
generate and return join tokens. The feature flag is also
in the SQL package so it can be used by both SQL and server.
Part of #60632.
WIP until:
server.JoinTokenRelease justification: All changes are to new code paths, and
are gated behind a feature flag.
Release note: None.