-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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.
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
[sec_scan][20] add ReportSecrets
forwarder to proxy's gRPC insecure server
#44324
Merged
Merged
Changes from 1 commit
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,178 @@ | ||
/* | ||
* Teleport | ||
* Copyright (C) 2024 Gravitational, Inc. | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU Affero General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU Affero General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU Affero General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
|
||
package client | ||
|
||
import ( | ||
"context" | ||
"crypto/tls" | ||
"log/slog" | ||
"slices" | ||
|
||
"github.com/gravitational/trace" | ||
"github.com/jonboulle/clockwork" | ||
"golang.org/x/net/http2" | ||
"google.golang.org/grpc" | ||
"google.golang.org/grpc/credentials" | ||
|
||
"github.com/gravitational/teleport/api/client" | ||
"github.com/gravitational/teleport/api/constants" | ||
apidefaults "github.com/gravitational/teleport/api/defaults" | ||
accessgraphsecretsv1pb "github.com/gravitational/teleport/api/gen/proto/go/teleport/accessgraph/v1" | ||
"github.com/gravitational/teleport/api/metadata" | ||
"github.com/gravitational/teleport/lib/srv/alpnproxy/common" | ||
"github.com/gravitational/teleport/lib/utils" | ||
) | ||
|
||
// Client is a client for the SecretsScannerService. | ||
type Client interface { | ||
// ReportSecrets is used by trusted devices to report secrets found on the host that could be used to bypass Teleport. | ||
// The client (device) should first authenticate using the [ReportSecretsRequest.device_assertion] flow. Please refer to | ||
// the [teleport.devicetrust.v1.AssertDeviceRequest] and [teleport.devicetrust.v1.AssertDeviceResponse] messages for more details. | ||
// | ||
// Once the device is asserted, the client can send the secrets using the [ReportSecretsRequest.private_keys] field | ||
// and then close the client side of the stream. | ||
// | ||
// -> ReportSecrets (client) [1 or more] | ||
// -> CloseStream (client) | ||
// <- TerminateStream (server) | ||
// | ||
// Any failure in the assertion ceremony will result in the stream being terminated by the server. All secrets | ||
// reported by the client before the assertion terminates will be ignored and result in the stream being terminated. | ||
ReportSecrets(ctx context.Context, opts ...grpc.CallOption) (accessgraphsecretsv1pb.SecretsScannerService_ReportSecretsClient, error) | ||
// Close closes the client connection. | ||
Close() error | ||
} | ||
|
||
// ClientConfig specifies parameters for the client to dial credentialless via the proxy. | ||
type ClientConfig struct { | ||
// ProxyServer is the address of the proxy server | ||
ProxyServer string | ||
// CipherSuites is a list of cipher suites to use for TLS client connection | ||
CipherSuites []uint16 | ||
// Clock specifies the time provider. Will be used to override the time anchor | ||
// for TLS certificate verification. | ||
// Defaults to real clock if unspecified | ||
Clock clockwork.Clock | ||
// Insecure trusts the certificates from the Auth Server or Proxy during registration without verification. | ||
Insecure bool | ||
// Log is the logger. | ||
Log *slog.Logger | ||
} | ||
|
||
// NewSecretsScannerServiceClient creates a new SecretsScannerServiceClient that connects to the proxy | ||
// gRPC server that does not require authentication (credentialless) to report secrets found during scanning. | ||
func NewSecretsScannerServiceClient(ctx context.Context, cfg ClientConfig) (Client, error) { | ||
if cfg.ProxyServer == "" { | ||
return nil, trace.BadParameter("missing ProxyServer") | ||
} | ||
if cfg.Clock == nil { | ||
cfg.Clock = clockwork.NewRealClock() | ||
} | ||
if cfg.Log == nil { | ||
cfg.Log = slog.Default() | ||
} | ||
|
||
grpcConn, err := proxyConn(ctx, cfg) | ||
if err != nil { | ||
return nil, trace.Wrap(err, "failed to connect to the proxy") | ||
} | ||
|
||
return &secretsSvcClient{ | ||
SecretsScannerServiceClient: accessgraphsecretsv1pb.NewSecretsScannerServiceClient(grpcConn), | ||
conn: grpcConn, | ||
}, nil | ||
} | ||
|
||
type secretsSvcClient struct { | ||
accessgraphsecretsv1pb.SecretsScannerServiceClient | ||
conn *grpc.ClientConn | ||
} | ||
|
||
func (c *secretsSvcClient) Close() error { | ||
return c.conn.Close() | ||
} | ||
|
||
// proxyConn attempts to connect to the proxy insecure grpc server. | ||
// The Proxy's TLS cert will be verified using the host's root CA pool | ||
// (PKI) unless the --insecure flag was passed. | ||
func proxyConn( | ||
tigrato marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ctx context.Context, params ClientConfig, | ||
) (*grpc.ClientConn, error) { | ||
tlsConfig := utils.TLSConfig(params.CipherSuites) | ||
tlsConfig.Time = params.Clock.Now | ||
// set NextProtos for TLS routing, the actual protocol will be h2 | ||
tlsConfig.NextProtos = []string{string(common.ProtocolProxyGRPCInsecure), http2.NextProtoTLS} | ||
|
||
if params.Insecure { | ||
tlsConfig.InsecureSkipVerify = true | ||
params.Log.WarnContext(ctx, "Connecting to the cluster without validating the identity of the Proxy Server.") | ||
} | ||
|
||
// Check if proxy is behind a load balancer. If so, the connection upgrade | ||
// will verify the load balancer's cert using system cert pool. This | ||
// provides the same level of security as the client only verifies Proxy's | ||
// web cert against system cert pool when connection upgrade is not | ||
// required. | ||
// | ||
// With the ALPN connection upgrade, the tunneled TLS Routing request will | ||
// skip verify as the Proxy server will present its host cert which is not | ||
// fully verifiable at this point since the client does not have the host | ||
// CAs yet before completing registration. | ||
alpnConnUpgrade := client.IsALPNConnUpgradeRequired(ctx, params.ProxyServer, params.Insecure) | ||
if alpnConnUpgrade && !params.Insecure { | ||
tlsConfig.InsecureSkipVerify = true | ||
tlsConfig.VerifyConnection = verifyALPNUpgradedConn(params.Clock) | ||
} | ||
|
||
dialer := client.NewDialer( | ||
ctx, | ||
apidefaults.DefaultIdleTimeout, | ||
apidefaults.DefaultIOTimeout, | ||
client.WithInsecureSkipVerify(params.Insecure), | ||
client.WithALPNConnUpgrade(alpnConnUpgrade), | ||
) | ||
|
||
conn, err := grpc.NewClient( | ||
params.ProxyServer, | ||
grpc.WithContextDialer(client.GRPCContextDialer(dialer)), | ||
grpc.WithUnaryInterceptor(metadata.UnaryClientInterceptor), | ||
grpc.WithStreamInterceptor(metadata.StreamClientInterceptor), | ||
grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)), | ||
) | ||
return conn, trace.Wrap(err) | ||
} | ||
|
||
// verifyALPNUpgradedConn is a tls.Config.VerifyConnection callback function | ||
// used by the tunneled TLS Routing request to verify the host cert of a Proxy | ||
// behind a L7 load balancer. | ||
// | ||
// Since the client has not obtained the cluster CAs at this point, the | ||
// presented cert cannot be fully verified yet. For now, this function only | ||
// checks if "teleport.cluster.local" is present as one of the DNS names and | ||
// verifies the cert is not expired. | ||
func verifyALPNUpgradedConn(clock clockwork.Clock) func(tls.ConnectionState) error { | ||
return func(server tls.ConnectionState) error { | ||
for _, cert := range server.PeerCertificates { | ||
if slices.Contains(cert.DNSNames, constants.APIDomain) && clock.Now().Before(cert.NotAfter) { | ||
return nil | ||
} | ||
} | ||
return trace.AccessDenied("server is not a Teleport proxy or server certificate is expired") | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,133 @@ | ||
/* | ||
* Teleport | ||
* Copyright (C) 2024 Gravitational, Inc. | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU Affero General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU Affero General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU Affero General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
|
||
// Package proxy implements a proxy service that proxies requests from the proxy unauthenticated | ||
// gRPC service to the Auth's secret service. | ||
package proxy | ||
|
||
import ( | ||
"context" | ||
"errors" | ||
"io" | ||
"log/slog" | ||
|
||
"github.com/gravitational/trace" | ||
|
||
accessgraphsecretsv1pb "github.com/gravitational/teleport/api/gen/proto/go/teleport/accessgraph/v1" | ||
) | ||
|
||
// AuthClient is a subset of the full Auth API that must be connected | ||
type AuthClient interface { | ||
tigrato marked this conversation as resolved.
Show resolved
Hide resolved
|
||
AccessGraphSecretsScannerClient() accessgraphsecretsv1pb.SecretsScannerServiceClient | ||
} | ||
|
||
// ServiceConfig is the configuration for the Service. | ||
type ServiceConfig struct { | ||
// AuthClient is the client to the Auth service. | ||
AuthClient AuthClient | ||
// Log is the logger. | ||
Log *slog.Logger | ||
} | ||
|
||
// New creates a new Service. | ||
func New(cfg ServiceConfig) (*Service, error) { | ||
if cfg.AuthClient == nil { | ||
return nil, trace.BadParameter("missing AuthClient") | ||
} | ||
if cfg.Log == nil { | ||
cfg.Log = slog.Default() | ||
} | ||
return &Service{ | ||
authClient: cfg.AuthClient, | ||
log: cfg.Log, | ||
}, nil | ||
} | ||
|
||
// Service is a service that proxies requests from the proxy to the Auth's secret service. | ||
// It only implements the ReportSecrets method of the SecretsScannerService because it is the only method that needs to be proxied | ||
// from the proxy to the Auth's secret service. | ||
type Service struct { | ||
accessgraphsecretsv1pb.UnimplementedSecretsScannerServiceServer | ||
// authClient is the client to the Auth service. | ||
authClient AuthClient | ||
|
||
log *slog.Logger | ||
} | ||
|
||
func (s *Service) ReportSecrets(client accessgraphsecretsv1pb.SecretsScannerService_ReportSecretsServer) error { | ||
tigrato marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ctx, cancel := context.WithCancel(client.Context()) | ||
defer cancel() | ||
upstream, err := s.authClient.AccessGraphSecretsScannerClient().ReportSecrets(ctx) | ||
if err != nil { | ||
return trace.Wrap(err) | ||
} | ||
|
||
errCh := make(chan error, 1) | ||
go func() { | ||
err := trace.Wrap(s.forwardClientToServer(ctx, client, upstream)) | ||
if err != nil { | ||
cancel() | ||
} | ||
errCh <- err | ||
}() | ||
|
||
err = s.forwardServerToClient(ctx, client, upstream) | ||
return trace.NewAggregate(err, <-errCh) | ||
} | ||
|
||
func (s *Service) forwardClientToServer(ctx context.Context, | ||
client accessgraphsecretsv1pb.SecretsScannerService_ReportSecretsServer, | ||
server accessgraphsecretsv1pb.SecretsScannerService_ReportSecretsClient) (err error) { | ||
for { | ||
req, err := client.Recv() | ||
if errors.Is(err, io.EOF) { | ||
if err := server.CloseSend(); err != nil { | ||
s.log.WarnContext(ctx, "Failed to close upstream stream", "error", err) | ||
} | ||
break | ||
} | ||
if err != nil { | ||
s.log.WarnContext(ctx, "Failed to receive from client stream", "error", err) | ||
return trace.Wrap(err) | ||
} | ||
if err := server.Send(req); err != nil { | ||
s.log.WarnContext(ctx, "Failed to send to upstream stream", "error", err) | ||
return trace.Wrap(err) | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
func (s *Service) forwardServerToClient(ctx context.Context, | ||
client accessgraphsecretsv1pb.SecretsScannerService_ReportSecretsServer, | ||
server accessgraphsecretsv1pb.SecretsScannerService_ReportSecretsClient) (err error) { | ||
for { | ||
out, err := server.Recv() | ||
if errors.Is(err, io.EOF) { | ||
return nil | ||
} | ||
if err != nil { | ||
s.log.WarnContext(ctx, "Failed to receive from upstream stream", "error", err) | ||
return trace.Wrap(err) | ||
} | ||
if err := client.Send(out); err != nil { | ||
s.log.WarnContext(ctx, "Failed to send to client stream", "error", err) | ||
return trace.Wrap(err) | ||
} | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason why the
ReportSecrets(ctx context.Context, opts ...grpc.CallOption) (accessgraphsecretsv1pb.SecretsScannerService_ReportSecretsClient, error)
doesn't need any auth ?Anyone will can connect and report arbitrary data - How we will protect againts DDOS where the attacker can flood artificial data to drain our backend throughput and drastically increase database size ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Devices do not have credentials so we can't use them.
ReportSecrets
authentication uses a device assertion flow where a cryptographic challenge proves the device is already enrolled.Teleport Auth server already allows any user even without credentials to dial to it. This is how joining exists and works. Even endpoints that allow credentials, you can dial to them and everything will be stoped at authn level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense.
The auth is done by auth ReportSecrets hanlder: https://github.com/gravitational/teleport.e/pull/4649/files#diff-346834ba5299731121f1d806e1b0ca2b2046c2ccc0157bcb41bcd2dff871e929R98 I have missed the reference to the server part.