m/n/core/rpc: limit API footgun availability
This unifies the interface of the
New{Ephemeral,Authenticated}Credentials calls. They now use the same set
of CredentialsOpt options which allows both calls to request a
particular verification of the remote side of the connection.
NewEphemeralCredentials also now requires an explicit WantInsecure
option which surfaces attempts to dial the cluster without CA/node
verification.
Change-Id: Ibb65cb0952f6ff2092a3f55fe1c5a31bd2b72b36
Reviewed-on: https://review.monogon.dev/c/monogon/+/2741
Tested-by: Jenkins CI
Reviewed-by: Lorenz Brun <lorenz@monogon.tech>
diff --git a/cloud/bmaas/server/agent_callback_service_test.go b/cloud/bmaas/server/agent_callback_service_test.go
index 3bd3df9..4a3a9f4 100644
--- a/cloud/bmaas/server/agent_callback_service_test.go
+++ b/cloud/bmaas/server/agent_callback_service_test.go
@@ -58,7 +58,7 @@
}
heartbeat := func(mid uuid.UUID) error {
- creds, err := rpc.NewEphemeralCredentials(priv, nil)
+ creds, err := rpc.NewEphemeralCredentials(priv, rpc.WantInsecure())
if err != nil {
t.Fatalf("could not generate ephemeral credentials: %v", err)
}
@@ -136,7 +136,7 @@
}
heartbeat := func(mid uuid.UUID, report *apb.OSInstallationReport) (*apb.AgentHeartbeatResponse, error) {
- creds, err := rpc.NewEphemeralCredentials(priv, nil)
+ creds, err := rpc.NewEphemeralCredentials(priv, rpc.WantInsecure())
if err != nil {
t.Fatalf("could not generate ephemeral credentials: %v", err)
}
diff --git a/metropolis/cli/metroctl/core/rpc.go b/metropolis/cli/metroctl/core/rpc.go
index 4aeda75..e519906 100644
--- a/metropolis/cli/metroctl/core/rpc.go
+++ b/metropolis/cli/metroctl/core/rpc.go
@@ -48,7 +48,7 @@
}
if ocert == nil {
- creds, err := rpc.NewEphemeralCredentials(opkey, nil)
+ creds, err := rpc.NewEphemeralCredentials(opkey, rpc.WantInsecure())
if err != nil {
return nil, fmt.Errorf("while building ephemeral credentials: %v", err)
}
diff --git a/metropolis/node/core/cluster/cluster_join.go b/metropolis/node/core/cluster/cluster_join.go
index 9a940a2..53ca815 100644
--- a/metropolis/node/core/cluster/cluster_join.go
+++ b/metropolis/node/core/cluster/cluster_join.go
@@ -58,7 +58,7 @@
return fmt.Errorf("no remote node available, cannot join cluster")
}
- ephCreds, err := rpc.NewEphemeralCredentials(jpriv, ca)
+ ephCreds, err := rpc.NewEphemeralCredentials(jpriv, rpc.WantRemoteCluster(ca))
if err != nil {
return fmt.Errorf("could not create ephemeral credentials: %w", err)
}
diff --git a/metropolis/node/core/cluster/cluster_register.go b/metropolis/node/core/cluster/cluster_register.go
index 64f1cdf..61cea56 100644
--- a/metropolis/node/core/cluster/cluster_register.go
+++ b/metropolis/node/core/cluster/cluster_register.go
@@ -96,7 +96,7 @@
return fmt.Errorf("no remote node available, cannot register into cluster")
}
- ephCreds, err := rpc.NewEphemeralCredentials(priv, ca)
+ ephCreds, err := rpc.NewEphemeralCredentials(priv, rpc.WantRemoteCluster(ca))
if err != nil {
return fmt.Errorf("could not create ephemeral credentials: %w", err)
}
diff --git a/metropolis/node/core/curator/impl_leader_test.go b/metropolis/node/core/curator/impl_leader_test.go
index 87bd36c..e89d4a1 100644
--- a/metropolis/node/core/curator/impl_leader_test.go
+++ b/metropolis/node/core/curator/impl_leader_test.go
@@ -214,7 +214,7 @@
}
// Create an ephemeral node gRPC client for the 'other node'.
- otherEphCreds, err := rpc.NewEphemeralCredentials(otherPriv, ca)
+ otherEphCreds, err := rpc.NewEphemeralCredentials(otherPriv, rpc.WantRemoteCluster(ca))
if err != nil {
t.Fatalf("NewEphemeralCredentials: %v", err)
}
@@ -789,7 +789,7 @@
withLocalDialer := grpc.WithContextDialer(func(_ context.Context, _ string) (net.Conn, error) {
return cl.curatorLis.Dial()
})
- ephCreds, err := rpc.NewEphemeralCredentials(jpriv, cl.ca)
+ ephCreds, err := rpc.NewEphemeralCredentials(jpriv, rpc.WantRemoteCluster(cl.ca))
if err != nil {
t.Fatalf("NewEphemeralCredentials: %v", err)
}
@@ -1723,7 +1723,7 @@
// Node registered as expected. Now make sure it can only join with the same seal
// state as it was supposed to use.
- ephCreds, err := rpc.NewEphemeralCredentials(nodeJoinPriv, cl.ca)
+ ephCreds, err := rpc.NewEphemeralCredentials(nodeJoinPriv, rpc.WantRemoteCluster(cl.ca))
if err != nil {
t.Fatalf("NewEphemeralCredentials: %v", err)
}
diff --git a/metropolis/node/core/rpc/client.go b/metropolis/node/core/rpc/client.go
index aeaed7e..8619d19 100644
--- a/metropolis/node/core/rpc/client.go
+++ b/metropolis/node/core/rpc/client.go
@@ -73,10 +73,10 @@
// 2. Escrow of initial owner credentials into a proper manager
// certificate
//
-// If 'ca' is given, the remote side will be cryptographically verified to be a
-// node that's part of the cluster represented by the ca. Otherwise, no
-// verification is performed and this function is unsafe.
-func NewEphemeralCredentials(private ed25519.PrivateKey, ca *x509.Certificate) (credentials.TransportCredentials, error) {
+// The given opts can be used to lock down the remote side of the connection, eg.
+// expecting a given cluster CA certificate or disabling remote side verification
+// by using WantInsecure().
+func NewEphemeralCredentials(private ed25519.PrivateKey, opts ...CredentialsOpt) (credentials.TransportCredentials, error) {
template := x509.Certificate{
SerialNumber: big.NewInt(1),
NotBefore: time.Now(),
@@ -94,24 +94,19 @@
Certificate: [][]byte{certificateBytes},
PrivateKey: private,
}
- var opts []AuthenticatedCredentialsOpt
- if ca != nil {
- opts = append(opts, WantRemoteCluster(ca))
- } else {
- opts = append(opts, WantInsecure())
- }
+ opts = append(opts)
return NewAuthenticatedCredentials(certificate, opts...), nil
}
-// AuthenticatedCredentialsOpt are created using WantXXX functions and used in
-// NewAuthenticatedCredentials.
-type AuthenticatedCredentialsOpt struct {
+// CredentialsOpt are created using WantXXX functions and used in
+// NewCredentials.
+type CredentialsOpt struct {
wantCA *x509.Certificate
wantNodeID string
insecureOkay bool
}
-func (a *AuthenticatedCredentialsOpt) merge(o *AuthenticatedCredentialsOpt) {
+func (a *CredentialsOpt) merge(o *CredentialsOpt) {
if a.wantNodeID == "" && o.wantNodeID != "" {
a.wantNodeID = o.wantNodeID
}
@@ -129,58 +124,62 @@
//
// This is the bare minimum option required to implement secure connections to
// clusters.
-func WantRemoteCluster(ca *x509.Certificate) AuthenticatedCredentialsOpt {
- return AuthenticatedCredentialsOpt{
+func WantRemoteCluster(ca *x509.Certificate) CredentialsOpt {
+ return CredentialsOpt{
wantCA: ca,
}
}
// WantRemoteNode enables the verification of the remote node identity when using
-// NewAuthenticatedCredentials. If the connection is not terminated at the node
+// NewCredentials. If the connection is not terminated at the node
// ID 'id', an error will be returned. For this function to work,
// WantRemoteCluster must also be set.
-func WantRemoteNode(id string) AuthenticatedCredentialsOpt {
- return AuthenticatedCredentialsOpt{
+func WantRemoteNode(id string) CredentialsOpt {
+ return CredentialsOpt{
wantNodeID: id,
}
}
// WantInsecure disables the verification of the remote side of the connection
-// via NewAuthenticatedCredentials. This is unsafe.
-func WantInsecure() AuthenticatedCredentialsOpt {
- return AuthenticatedCredentialsOpt{
+// via NewCredentials. This is unsafe.
+func WantInsecure() CredentialsOpt {
+ return CredentialsOpt{
insecureOkay: true,
}
}
-// NewAuthenticatedCredentials returns gRPC TransportCredentials that can be
-// used to dial a cluster with a given TLS certificate (from node or manager
+// NewAuthenticatedCredentials returns gRPC TransportCredentials that can be used
+// to dial a cluster with a given TLS certificate (from node or manager
// credentials).
//
-// The provided AuthenticatedCredentialsOpt specify the verification of the
-// remote side of the connection. When connecting to a cluster (any node), use
-// WantRemoteCluster. If you also want to verify the connection to a particular
-// node, specify WantRemoteNode alongside it. If no verification should be
-// performed use WantInsecure.
+// The provided CredentialsOpt specify the verification of the remote side of the
+// connection. When connecting to a cluster (any node), use WantRemoteCluster. If
+// you also want to verify the connection to a particular node, specify
+// WantRemoteNode alongside it. If no verification should be performed use
+// WantInsecure.
//
// The given options are parsed on a first-wins basis.
-func NewAuthenticatedCredentials(cert tls.Certificate, opts ...AuthenticatedCredentialsOpt) credentials.TransportCredentials {
+func NewAuthenticatedCredentials(cert tls.Certificate, opts ...CredentialsOpt) credentials.TransportCredentials {
config := &tls.Config{
Certificates: []tls.Certificate{cert},
InsecureSkipVerify: true,
}
- var merged AuthenticatedCredentialsOpt
+ var merged CredentialsOpt
for _, o := range opts {
merged.merge(&o)
}
if merged.insecureOkay {
- if merged.wantNodeID != "" || merged.wantCA != nil {
- config.VerifyPeerCertificate = verifyFail(fmt.Errorf("WantInsecure specified alongside WantRemoteNode/WantRemoteCluster"))
+ if merged.wantNodeID != "" {
+ config.VerifyPeerCertificate = verifyFail(fmt.Errorf("WantInsecure specified alongside WantRemoteNode"))
+ } else if merged.wantCA != nil {
+ config.VerifyPeerCertificate = verifyFail(fmt.Errorf("WantInsecure specified alongside WantRemoteCluster"))
}
} else {
switch {
+ case merged.wantNodeID == "" && merged.wantCA == nil:
+ config.VerifyPeerCertificate = verifyFail(fmt.Errorf("WantRemoteNode/WantRemoteCluster/WantInsecure not specified"))
case merged.wantNodeID != "" && merged.wantCA == nil:
config.VerifyPeerCertificate = verifyFail(fmt.Errorf("WantRemoteNode also requires WantRemoteCluster"))
case merged.wantCA == nil:
diff --git a/metropolis/node/core/rpc/server_authentication_test.go b/metropolis/node/core/rpc/server_authentication_test.go
index 326b59e..b0da0a9 100644
--- a/metropolis/node/core/rpc/server_authentication_test.go
+++ b/metropolis/node/core/rpc/server_authentication_test.go
@@ -106,7 +106,7 @@
t.Fatalf("GenerateKey: %v", err)
}
- ephCreds, err := NewEphemeralCredentials(sk, eph.CA)
+ ephCreds, err := NewEphemeralCredentials(sk, WantRemoteCluster(eph.CA))
if err != nil {
t.Fatalf("NewEphemeralCredentials: %v", err)
}
diff --git a/metropolis/test/launch/cluster/cluster.go b/metropolis/test/launch/cluster/cluster.go
index 1066d4d..06b928b 100644
--- a/metropolis/test/launch/cluster/cluster.go
+++ b/metropolis/test/launch/cluster/cluster.go
@@ -580,7 +580,7 @@
func firstConnection(ctx context.Context, socksDialer proxy.Dialer) (*tls.Certificate, *NodeInCluster, error) {
// Dial external service.
remote := fmt.Sprintf("10.1.0.2:%s", node.CuratorServicePort.PortString())
- initCreds, err := rpc.NewEphemeralCredentials(InsecurePrivateKey, nil)
+ initCreds, err := rpc.NewEphemeralCredentials(InsecurePrivateKey, rpc.WantInsecure())
if err != nil {
return nil, nil, fmt.Errorf("NewEphemeralCredentials: %w", err)
}