diff --git a/windows-agent/internal/proservices/landscape/connection.go b/windows-agent/internal/proservices/landscape/connection.go index 24f9b8f8f..1cad9b396 100644 --- a/windows-agent/internal/proservices/landscape/connection.go +++ b/windows-agent/internal/proservices/landscape/connection.go @@ -65,7 +65,7 @@ func newConnection(ctx context.Context, d serviceData) (conn *connection, err er cancel: cancel, } - creds, err := transportCredentials(conn.settings.certificatePath) + creds, err := transportCredentials(ctx, conn.settings.certificatePath) if err != nil { return nil, err } diff --git a/windows-agent/internal/proservices/landscape/executor_test.go b/windows-agent/internal/proservices/landscape/executor_test.go index f0eda2fcf..af95e5aff 100644 --- a/windows-agent/internal/proservices/landscape/executor_test.go +++ b/windows-agent/internal/proservices/landscape/executor_test.go @@ -746,6 +746,8 @@ func testReceiveCommand(t *testing.T, distrosettings distroSettings, homedir str ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) + ctx = context.WithValue(ctx, landscape.InsecureCredentials, true) + tb.wslMock = wslmock.New() ctx = wsl.WithMock(ctx, tb.wslMock) diff --git a/windows-agent/internal/proservices/landscape/landscape_test.go b/windows-agent/internal/proservices/landscape/landscape_test.go index 612ea885d..b6354b728 100644 --- a/windows-agent/internal/proservices/landscape/landscape_test.go +++ b/windows-agent/internal/proservices/landscape/landscape_test.go @@ -110,7 +110,8 @@ func TestConnect(t *testing.T) { emptyToken bool tokenErr bool - requireCertificate bool + clientUsesTLS bool + serverUsesTLS bool breakLandscapeClientConfig bool breakUIDFile bool @@ -123,7 +124,7 @@ func TestConnect(t *testing.T) { }{ "Success": {}, "Success in non-first contact": {uid: "123", wantSingleMessage: true}, - "Success with an SSL certificate": {requireCertificate: true}, + "Success with an SSL certificate": {clientUsesTLS: true, serverUsesTLS: true}, // These tests are for the error cases when the error is logged but not returned "Silent error when the config is empty": {wantNotConnected: true}, @@ -138,8 +139,9 @@ func TestConnect(t *testing.T) { "Error when the first-contact SendUpdatedInfo fails": {tokenErr: true, wantErr: true}, "Error when the config cannot be accessed": {breakLandscapeClientConfig: true, wantErr: true}, "Error when the config cannot be parsed": {wantErr: true}, - "Error when the SSL certificate cannot be read": {wantErr: true}, - "Error when the SSL certificate is not valid": {wantErr: true}, + "Error when the SSL certificate cannot be read": {clientUsesTLS: true, serverUsesTLS: true, wantErr: true}, + "Error when the SSL certificate is not valid": {clientUsesTLS: true, serverUsesTLS: true, wantErr: true}, + "Error when the SSL certificate is not trusted": {clientUsesTLS: true, serverUsesTLS: true, wantErr: true}, } for name, tc := range testCases { @@ -147,16 +149,20 @@ func TestConnect(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - if wsl.MockAvailable() { - t.Parallel() - ctx = wsl.WithMock(ctx, wslmock.New()) + if !tc.clientUsesTLS { + ctx = context.WithValue(ctx, landscape.InsecureCredentials, true) } p := "" - if tc.requireCertificate { + if tc.serverUsesTLS { p = certPath } + if wsl.MockAvailable() { + t.Parallel() + ctx = wsl.WithMock(ctx, wslmock.New()) + } + lis, server, mockService := setUpLandscapeMock(t, ctx, "localhost:", p) defer lis.Close() @@ -305,7 +311,7 @@ func TestSendUpdatedInfo(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { - ctx := context.Background() + ctx := context.WithValue(context.Background(), landscape.InsecureCredentials, true) if wsl.MockAvailable() { t.Parallel() mock := wslmock.New() @@ -519,7 +525,7 @@ func TestAutoReconnection(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(context.WithValue(context.Background(), landscape.InsecureCredentials, true)) defer cancel() if wsl.MockAvailable() { @@ -760,6 +766,8 @@ func TestReconnect(t *testing.T) { certPath = t.TempDir() testutils.GenerateTempCertificate(t, certPath) lcapeConfig = fmt.Sprintf("%s\nssl_public_key = {{ .CertPath }}/cert.pem", defaultLandscapeConfig) + } else { + ctx = context.WithValue(ctx, landscape.InsecureCredentials, true) } lis, server, mockServerService := setUpLandscapeMock(t, ctx, "localhost:", certPath) diff --git a/windows-agent/internal/proservices/landscape/testdata/TestConnect/error_when_the_SSL_certificate_is_not_trusted/landscape.conf b/windows-agent/internal/proservices/landscape/testdata/TestConnect/error_when_the_SSL_certificate_is_not_trusted/landscape.conf new file mode 100644 index 000000000..77f88a25c --- /dev/null +++ b/windows-agent/internal/proservices/landscape/testdata/TestConnect/error_when_the_SSL_certificate_is_not_trusted/landscape.conf @@ -0,0 +1,4 @@ +[client] + +[host] +url = {{ .HostURL }} diff --git a/windows-agent/internal/proservices/landscape/utils.go b/windows-agent/internal/proservices/landscape/utils.go index b053d8971..581e3b945 100644 --- a/windows-agent/internal/proservices/landscape/utils.go +++ b/windows-agent/internal/proservices/landscape/utils.go @@ -100,17 +100,39 @@ func newHostAgentInfo(ctx context.Context, c serviceData) (info *landscapeapi.Ho return info, nil } +type transportCredentialsType struct{} + +// InsecureCredentials is the key used in tests for insecure credentials. +var InsecureCredentials = transportCredentialsType{} + // transportCredentials reads the Landscape client config to check if a SSL public key is specified. // -// If this credential is not specified, an insecure credential is returned. -// If the credential is specified but erroneous, an error is returned. -func transportCredentials(sslPublicKeyPath string) (cred credentials.TransportCredentials, err error) { +// If this credential is not specified, credentials based on the system's certificate pool is returned. +// If the SSL public key is specified but invalid, an error is returned. +// If the context has the "InsecureCredentials" key set to "true", insecure credentials are returned (for testing purposes). +func transportCredentials(ctx context.Context, sslPublicKeyPath string) (cred credentials.TransportCredentials, err error) { defer decorate.OnError(&err, "Landscape credentials") - if sslPublicKeyPath == "" { + isInsecure := ctx.Value(InsecureCredentials) + // ctx.Value() returns 'any', thus this comparison is cleaner than a type assertion. + if isInsecure == true { + log.Warningf(ctx, "Landscape: context requires insecure credentials, ignoring server's public key %s", sslPublicKeyPath) return insecure.NewCredentials(), nil } + if sslPublicKeyPath == "" { + certPool, err := x509.SystemCertPool() + if err != nil { + return nil, fmt.Errorf("could not load system certificates: %v", err) + } + + return credentials.NewTLS(&tls.Config{ + RootCAs: certPool, + MinVersion: tls.VersionTLS12, + }), nil + } + + log.Infof(ctx, "Landscape: loading server's SSL public key %s", sslPublicKeyPath) cert, err := os.ReadFile(sslPublicKeyPath) if err != nil { return nil, fmt.Errorf("could not load SSL public key file: %v", err) @@ -118,9 +140,10 @@ func transportCredentials(sslPublicKeyPath string) (cred credentials.TransportCr certPool := x509.NewCertPool() if ok := certPool.AppendCertsFromPEM(cert); !ok { - return nil, fmt.Errorf("failed to add server CA's certificate: %v", err) + return nil, fmt.Errorf("failed to add server's certificate to the trust pool: %v", err) } + log.Infof(ctx, "Landscape: using server's SSL public key %s instead of system's certificate pool", sslPublicKeyPath) return credentials.NewTLS(&tls.Config{ RootCAs: certPool, MinVersion: tls.VersionTLS12,