From 3c625d301b88097623c55e18afb621d21f27cdcc Mon Sep 17 00:00:00 2001 From: Markus Rudy Date: Sat, 22 Feb 2025 15:23:49 +0100 Subject: [PATCH 1/2] atls: add handshake timeout to aTLS servers and clients --- coordinator/internal/authority/credentials.go | 23 +++++++++++-------- internal/constants/constants.go | 17 +++++++++++++- .../grpc/atlscredentials/atlscredentials.go | 20 +++++++++++++++- internal/grpc/dialer/dialer.go | 4 ++-- 4 files changed, 51 insertions(+), 13 deletions(-) diff --git a/coordinator/internal/authority/credentials.go b/coordinator/internal/authority/credentials.go index e7d8f208e..9eb5631fa 100644 --- a/coordinator/internal/authority/credentials.go +++ b/coordinator/internal/authority/credentials.go @@ -5,6 +5,7 @@ package authority import ( "context" + "crypto/tls" "errors" "fmt" "log/slog" @@ -17,6 +18,7 @@ import ( "github.com/edgelesssys/contrast/internal/attestation/certcache" "github.com/edgelesssys/contrast/internal/attestation/snp" "github.com/edgelesssys/contrast/internal/attestation/tdx" + "github.com/edgelesssys/contrast/internal/constants" "github.com/edgelesssys/contrast/internal/logger" "github.com/edgelesssys/contrast/internal/memstore" "github.com/prometheus/client_golang/prometheus" @@ -112,16 +114,19 @@ func (c *Credentials) ServerHandshake(rawConn net.Conn) (net.Conn, credentials.A return nil, nil, err } - conn, info, err := credentials.NewTLS(serverCfg).ServerHandshake(rawConn) - if err != nil { - log.Error("ServerHandshake failed", "error", err) - return nil, nil, err + ctx, cancel := context.WithTimeout(context.Background(), constants.ATLSServerTimeout) + defer cancel() + + conn := tls.Server(rawConn, serverCfg) + if err := conn.HandshakeContext(ctx); err != nil { + return nil, nil, fmt.Errorf("handshake error: %w", err) } - tlsInfo, ok := info.(credentials.TLSInfo) - if ok { - authInfo.TLSInfo = tlsInfo - } else { - log.Error("credentials.NewTLS returned unexpected AuthInfo", "obj", info) + + authInfo.TLSInfo = credentials.TLSInfo{ + State: conn.ConnectionState(), + CommonAuthInfo: credentials.CommonAuthInfo{ + SecurityLevel: credentials.PrivacyAndIntegrity, + }, } return conn, authInfo, nil diff --git a/internal/constants/constants.go b/internal/constants/constants.go index 7ae0081eb..e2d0b1f51 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -3,7 +3,11 @@ package constants -import "github.com/google/go-sev-guest/abi" +import ( + "time" + + "github.com/google/go-sev-guest/abi" +) // Version value is injected at build time. var ( @@ -26,3 +30,14 @@ var SNPPolicy = abi.SnpPolicy{ SMT: true, Debug: false, } + +const ( + // ATLSClientTimeout is the maximal amount of time spent by Coordinator clients for issuing + // and validation of attestation docs. + ATLSClientTimeout = 30 * time.Second + + // ATLSServerTimeout is the maximal amount of time that the Coordinator can spend for issuing + // attestation docs. It's deliberately smaller than ATLSClientTimeout to allow proper error + // propagation. + ATLSServerTimeout = ATLSClientTimeout - 5*time.Second +) diff --git a/internal/grpc/atlscredentials/atlscredentials.go b/internal/grpc/atlscredentials/atlscredentials.go index 1281c8944..f78470327 100644 --- a/internal/grpc/atlscredentials/atlscredentials.go +++ b/internal/grpc/atlscredentials/atlscredentials.go @@ -7,10 +7,13 @@ package atlscredentials import ( "context" "crypto" + "crypto/tls" "errors" + "fmt" "net" "github.com/edgelesssys/contrast/internal/atls" + "github.com/edgelesssys/contrast/internal/constants" "github.com/prometheus/client_golang/prometheus" "google.golang.org/grpc/credentials" ) @@ -58,7 +61,22 @@ func (c *Credentials) ServerHandshake(rawConn net.Conn) (net.Conn, credentials.A return nil, nil, err } - return credentials.NewTLS(serverCfg).ServerHandshake(rawConn) + ctx, cancel := context.WithTimeout(context.Background(), constants.ATLSServerTimeout) + defer cancel() + + conn := tls.Server(rawConn, serverCfg) + if err := conn.HandshakeContext(ctx); err != nil { + return nil, nil, fmt.Errorf("handshake error: %w", err) + } + + info := credentials.TLSInfo{ + State: conn.ConnectionState(), + CommonAuthInfo: credentials.CommonAuthInfo{ + SecurityLevel: credentials.PrivacyAndIntegrity, + }, + } + + return conn, info, nil } // Info provides information about the protocol. diff --git a/internal/grpc/dialer/dialer.go b/internal/grpc/dialer/dialer.go index 9c5ff4f43..779c0b8b3 100644 --- a/internal/grpc/dialer/dialer.go +++ b/internal/grpc/dialer/dialer.go @@ -8,9 +8,9 @@ import ( "context" "crypto" "net" - "time" "github.com/edgelesssys/contrast/internal/atls" + "github.com/edgelesssys/contrast/internal/constants" "github.com/edgelesssys/contrast/internal/grpc/atlscredentials" "github.com/prometheus/client_golang/prometheus" "google.golang.org/grpc" @@ -57,7 +57,7 @@ func (d *Dialer) Dial(_ context.Context, target string) (*grpc.ClientConn, error grpc.WithConnectParams(grpc.ConnectParams{ // We need a high initial timeout, because otherwise the client will get stuck in a reconnect loop // where the timeout is too low to get a full handshake done. - MinConnectTimeout: 30 * time.Second, + MinConnectTimeout: constants.ATLSClientTimeout, }), ) } From a0a4e2e7609040b46b079ba956de2ab6bc2d4f8a Mon Sep 17 00:00:00 2001 From: Markus Rudy Date: Fri, 28 Feb 2025 10:33:03 +0100 Subject: [PATCH 2/2] authority: include type information for bad TLSInfo --- coordinator/internal/authority/userapi.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coordinator/internal/authority/userapi.go b/coordinator/internal/authority/userapi.go index 614782ec5..28bb12e0f 100644 --- a/coordinator/internal/authority/userapi.go +++ b/coordinator/internal/authority/userapi.go @@ -301,7 +301,7 @@ func getPeerPublicKey(ctx context.Context) ([]byte, error) { } tlsInfo, ok := peer.AuthInfo.(credentials.TLSInfo) if !ok { - return nil, errors.New("peer auth info is not of type TLSInfo") + return nil, fmt.Errorf("peer auth info is not of type TLSInfo: got %T", peer.AuthInfo) } if len(tlsInfo.State.PeerCertificates) == 0 || tlsInfo.State.PeerCertificates[0] == nil { return nil, errors.New("no peer certificates found")