Skip to content

Commit

Permalink
feat(landscape): Implement support for SSL certificates in Landscape …
Browse files Browse the repository at this point in the history
…connection (#320)

Three changes in this PR:
- Up until now, the landscape `computer_name` override took place in the
agent, when creating the task. This is now moved to the WSL Pro Service.
- On top of overriding the `computer_name`, the WSL Pro Service will now
also rewrite `ssl_public_key` so that it is converted from Windows path
to WSL path. This step is skipped if this key is not present.
- The Windows Agent will now read the landscape config and use the
`ssl_public_key` when available. This key will be used to secure the
connection to the Hostagent server.



UDENG-1544
  • Loading branch information
EduardGomezEscandell authored Oct 18, 2023
2 parents 19ce4be + 9b04178 commit ff38d12
Show file tree
Hide file tree
Showing 30 changed files with 426 additions and 156 deletions.
4 changes: 1 addition & 3 deletions windows-agent/internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,8 @@ func (c *Config) ProvisioningTasks(ctx context.Context, distroName string) ([]ta

if conf, err := c.LandscapeClientConfig(ctx); err != nil {
log.Errorf(ctx, "Could not generate provisioning task LandscapeConfigure: %v", err)
} else if landscape, err := tasks.NewLandscapeConfigure(ctx, conf, distroName); err != nil {
log.Errorf(ctx, "Could not generate provisioning task LandscapeConfigure: %v", err)
} else {
// Success
landscape := tasks.LandscapeConfigure{Config: conf}
taskList = append(taskList, landscape)
}

Expand Down
66 changes: 65 additions & 1 deletion windows-agent/internal/proservices/landscape/landscape.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@ package landscape

import (
"context"
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"io/fs"
"os"
"path/filepath"
"strings"
"sync"
"sync/atomic"
"time"
Expand All @@ -19,7 +22,9 @@ import (
"github.com/ubuntu/decorate"
"google.golang.org/grpc"
"google.golang.org/grpc/connectivity"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/credentials/insecure"
"gopkg.in/ini.v1"
)

// Client is a client to the landscape service, served remotely.
Expand Down Expand Up @@ -58,6 +63,7 @@ const cacheFileBase = "landscape.conf"

// Config is a configuration provider for ProToken and the Landscape URL.
type Config interface {
LandscapeClientConfig(context.Context) (string, error)
LandscapeAgentURL(context.Context) (string, error)
Subscription(context.Context) (string, config.SubscriptionSource, error)
}
Expand Down Expand Up @@ -198,7 +204,12 @@ func (c *Client) connect(ctx context.Context, address string) (conn *connection,
dialCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
defer cancel()

grpcConn, err := grpc.DialContext(dialCtx, address, grpc.WithTransportCredentials(insecure.NewCredentials()))
creds, err := c.transportCredentials(ctx)
if err != nil {
return nil, err
}

grpcConn, err := grpc.DialContext(dialCtx, address, grpc.WithTransportCredentials(creds))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -377,3 +388,56 @@ func (c *Client) getUID() string {
func (c *Client) setUID(s string) {
c.uid.Store(s)
}

// 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 (c *Client) transportCredentials(ctx context.Context) (cred credentials.TransportCredentials, err error) {
defer decorate.OnError(&err, "Landscape credentials")

conf, err := c.conf.LandscapeClientConfig(ctx)
if err != nil {
return nil, fmt.Errorf("could not obtain Landscape config: %v", err)
}

if conf == "" {
// No Landscape config: default to insecure
return insecure.NewCredentials(), nil
}

ini, err := ini.Load(strings.NewReader(conf))
if err != nil {
return insecure.NewCredentials(), fmt.Errorf("could not parse Landscape config file: %v", err)
}

const section = "client"
const key = "ssl_public_key"

sec, err := ini.GetSection(section)
if err != nil {
// No SSL public key provided: default to insecure
return insecure.NewCredentials(), nil
}

k, err := sec.GetKey(key)
if err != nil {
// No SSL public key provided: default to insecure
return insecure.NewCredentials(), nil
}

cert, err := os.ReadFile(k.String())
if err != nil {
return nil, fmt.Errorf("could not load SSL public key file: %v", err)
}

certPool := x509.NewCertPool()
if ok := certPool.AppendCertsFromPEM(cert); !ok {
return nil, fmt.Errorf("failed to add server CA's certificate: %v", err)
}

return credentials.NewTLS(&tls.Config{
RootCAs: certPool,
MinVersion: tls.VersionTLS12,
}), nil
}
90 changes: 71 additions & 19 deletions windows-agent/internal/proservices/landscape/landscape_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package landscape_test

import (
"context"
"crypto/tls"
"errors"
"fmt"
"net"
Expand All @@ -14,6 +15,7 @@ import (
"time"

landscapeapi "github.com/canonical/landscape-hostagent-api"
"github.com/canonical/ubuntu-pro-for-windows/common/golden"
"github.com/canonical/ubuntu-pro-for-windows/common/wsltestutils"
"github.com/canonical/ubuntu-pro-for-windows/mocks/landscape/landscapemockservice"
"github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/config"
Expand All @@ -27,6 +29,7 @@ import (
wsl "github.com/ubuntu/gowsl"
wslmock "github.com/ubuntu/gowsl/mock"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
)

func TestMain(m *testing.M) {
Expand Down Expand Up @@ -94,24 +97,32 @@ func TestConnect(t *testing.T) {
}

testCases := map[string]struct {
precancelContext bool
serverNotAvailable bool
landscapeURLErr bool
tokenErr bool
precancelContext bool
serverNotAvailable bool
landscapeURLErr bool
tokenErr bool
requireCertificate bool
breakLandscapeClientConfig bool

breakUIDFile bool
uid string

wantErr bool
wantDistroSkipped bool
}{
"Success in first contact": {},
"Success in non-first contact": {uid: "123"},
"Success in first contact": {},
"Success in non-first contact": {uid: "123"},
"Success with non-empty config": {},
"Success with an SSL certificate": {requireCertificate: true},

"Error when the context is cancelled before Connected": {precancelContext: true, wantErr: true},
"Error when the landscape URL cannot be retrieved": {landscapeURLErr: true, wantErr: true},
"Error when the server cannot be reached": {serverNotAvailable: true, wantErr: true},
"Error when the first-contact SendUpdatedInfo fails ": {tokenErr: true, wantErr: true},
"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},
}

for name, tc := range testCases {
Expand All @@ -123,7 +134,7 @@ func TestConnect(t *testing.T) {
ctx = wsl.WithMock(ctx, wslmock.New())
}

lis, server, mockService := setUpLandscapeMock(t, ctx, "localhost:")
lis, server, mockService := setUpLandscapeMock(t, ctx, "localhost:", tc.requireCertificate)
defer lis.Close()

conf := &mockConfig{
Expand All @@ -135,7 +146,19 @@ func TestConnect(t *testing.T) {

// We trigger an earlier error by erroring out on LandscapeAgentURL
landscapeURLErr: tc.landscapeURLErr,

// We trigger an error when deciding to use a certificate or not
landscapeConfigErr: tc.breakLandscapeClientConfig,
}

out, err := os.ReadFile(filepath.Join(golden.TestFixturePath(t), "landscape.conf"))
if errors.Is(err, os.ErrNotExist) {
// This fixture is not compulsory
out = []byte{}
err = nil
}
require.NoError(t, err, "Setup: could not load landscape config")
conf.landscapeClientConfig = string(out)

if !tc.serverNotAvailable {
//nolint:errcheck // We don't care about these errors
Expand Down Expand Up @@ -188,7 +211,7 @@ func TestConnect(t *testing.T) {

confFile := filepath.Join(dir, landscape.CacheFileBase)
require.FileExists(t, confFile, "Landscape config file should be created after disconnecting")
out, err := os.ReadFile(confFile)
out, err = os.ReadFile(confFile)
require.NoError(t, err, "Could not read landscape config file")

wantUID := tc.uid
Expand Down Expand Up @@ -242,7 +265,7 @@ func TestSendUpdatedInfo(t *testing.T) {
ctx = wsl.WithMock(ctx, mock)
}

lis, server, mockService := setUpLandscapeMock(t, ctx, "localhost:")
lis, server, mockService := setUpLandscapeMock(t, ctx, "localhost:", false)

conf := &mockConfig{
proToken: "TOKEN",
Expand Down Expand Up @@ -408,7 +431,7 @@ func TestAutoReconnection(t *testing.T) {
ctx = wsl.WithMock(ctx, mock)
}

lis, server, mockService := setUpLandscapeMock(t, ctx, "localhost:")
lis, server, mockService := setUpLandscapeMock(t, ctx, "localhost:", false)
defer lis.Close()
defer server.Stop()

Expand Down Expand Up @@ -460,7 +483,7 @@ func TestAutoReconnection(t *testing.T) {
}, 5*time.Second, 100*time.Millisecond, "Client should have disconnected after the server is stopped")

// Restart server at the same address
lis, server, _ = setUpLandscapeMock(t, ctx, lis.Addr().String())
lis, server, _ = setUpLandscapeMock(t, ctx, lis.Addr().String(), false)
defer lis.Close()

//nolint:errcheck // We don't care
Expand Down Expand Up @@ -541,7 +564,7 @@ func TestReceiveCommands(t *testing.T) {
t.Skip("This test can only run with the mock")
}

lis, server, service := setUpLandscapeMock(t, ctx, "localhost:")
lis, server, service := setUpLandscapeMock(t, ctx, "localhost:", false)
defer lis.Close()

//nolint:errcheck // We don't care about these errors
Expand Down Expand Up @@ -781,30 +804,59 @@ func isAppxInstalled(t *testing.T, appxPackage string) bool {
}

//nolint:revive // Context goes after testing.T
func setUpLandscapeMock(t *testing.T, ctx context.Context, addr string) (lis net.Listener, server *grpc.Server, service *landscapemockservice.Service) {
func setUpLandscapeMock(t *testing.T, ctx context.Context, addr string, requireCertificate bool) (lis net.Listener, server *grpc.Server, service *landscapemockservice.Service) {
t.Helper()

var cfg net.ListenConfig
lis, err := cfg.Listen(ctx, "tcp", addr)
require.NoError(t, err, "Setup: can't listen")

server = grpc.NewServer()
var opts []grpc.ServerOption
if requireCertificate {
certPath := filepath.Join(golden.TestFamilyPath(t), "certificates/cert.pem")
keyPath := filepath.Join(golden.TestFamilyPath(t), "certificates/key.pem")

serverCert, err := tls.LoadX509KeyPair(certPath, keyPath)
require.NoError(t, err, "Setup: could not load Landscape mock server credentials")

config := &tls.Config{
Certificates: []tls.Certificate{serverCert},
ClientAuth: tls.NoClientCert,
MinVersion: tls.VersionTLS12,
}

opts = append(opts, grpc.Creds(credentials.NewTLS(config)))
}

server = grpc.NewServer(opts...)
service = landscapemockservice.New()
landscapeapi.RegisterLandscapeHostAgentServer(server, service)

return lis, server, service
}

type mockConfig struct {
proToken string
landscapeAgentURL string
proToken string
landscapeAgentURL string
landscapeClientConfig string

proTokenErr bool
landscapeURLErr bool
proTokenErr bool
landscapeURLErr bool
landscapeConfigErr bool

mu sync.Mutex
}

func (m *mockConfig) LandscapeClientConfig(ctx context.Context) (string, error) {
m.mu.Lock()
defer m.mu.Unlock()

if m.landscapeConfigErr {
return "", errors.New("Mock error")
}
return m.landscapeClientConfig, nil
}

func (m *mockConfig) ProvisioningTasks(ctx context.Context, distroName string) ([]task.Task, error) {
return nil, nil
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This is not a valid certificate
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
-----BEGIN CERTIFICATE-----
MIIFdDCCA1ygAwIBAgIUQ2XcjKTRgC0xuZMtwMAa0OvDxRswDQYJKoZIhvcNAQEL
BQAwQTELMAkGA1UEBhMCVVMxEjAQBgNVBAoMCUNhbm9uaWNhbDEeMBwGA1UEAwwV
Q2Fub25pY2FsR3JvdXBMaW1pdGVkMB4XDTIzMTAxMDEwMDcyM1oXDTIzMTEwOTEw
MDcyM1owQTELMAkGA1UEBhMCVVMxEjAQBgNVBAoMCUNhbm9uaWNhbDEeMBwGA1UE
AwwVQ2Fub25pY2FsR3JvdXBMaW1pdGVkMIICIjANBgkqhkiG9w0BAQEFAAOCAg8A
MIICCgKCAgEAzfI1KuyZgkgrIc48UMN5cHNEchOQlWrP9eo9+s2wTb56U7zyIdlq
4azsDXVpdL9ObhI1aLOoIMogRF1ZPgXvCH1JlGTnfjAGtChLUF2+rraUf6HGGESG
A09qFWB+JfeBrYNzRAL3rGFlttuNpW59WQIKAZ9hgKZTmBInjTNFrxMvjrAF9cYX
ebQqN6u29/+c8gH6Rf0mpPYSMahvdRT5IKZvLyaQNwQaT1UxsxSTsxAqsrXQ/o+G
UQcJpUnG3rc49B6jIDdTpiptF5Ey/f9maOjY/txQEKqLO5N04PuW7mDq17i625yJ
HAGI7ukNHQcMhcG475EUJuMLWNooozL0zBp/WAZs1cOSg0MH0TY6Oj2xET8HXoUW
T2gM/XkmWpeX9WmQBfkegbVALuKzDNnUWUYpIfCpvBL8u+9Rew+Dq7vIwmbPUAAf
0KssF+9KSRpDkmqDKZiGwbA6sAKXkLk47JDneeQ9SvANCox4fE0+qh+O2yJGygcO
T8CI0+fVZXdbS6Db5yzn8ZwbNpDw2lBahwpq1aCIwBmJ3P1ksEQXnTsyv3UKQz/F
x/zgSSqZWqgSwqKERYHxjSjSjjAORSNN+XTcKzK9Q9eA2exzw7mI1cR5mztPIRz2
r2DVTFwLAE/ykR2t31wse5wCLiXno9I0Zm/M2Be9xw0+A4VTbFz1nwkCAwEAAaNk
MGIwHQYDVR0OBBYEFAUAonELhcOl0sZfHnTLTZNkKzmaMB8GA1UdIwQYMBaAFAUA
onELhcOl0sZfHnTLTZNkKzmaMA8GA1UdEwEB/wQFMAMBAf8wDwYDVR0RBAgwBocE
fwAAATANBgkqhkiG9w0BAQsFAAOCAgEAbmUCtPUY9KoEucjLcf3bMmk4asWKTJIr
5kkJL/s7E1WaMudNHovunk4Zx3X62FzJHR6Z04zJjdWxhmdDbE+bO2LrkwsXJNxX
NBbdP7qkSgRUvgigSTu2kc7nAQgDS7dIxyCZyQTC1opaj8t0uuxzqHpG82zzrmQ1
8Guz1sWpBeBKgwyBM+okFvo4OcZV905hR1+sHzE1aLhoOnpX3uNwFgGh8z8jXc7p
e5FrgoEzaHYZfriioU+Lqf+92nAmtdFtNTP/g2OWunuOhEvYUI+EfPpjqSB19hvi
vD5qnV/euuWbP9mnoGVXmRcg5ZWVqgm6Yh+syRXYDEf8zwkpjod5DOKHVA0oK4hy
1zp5ufVwVETB/rdfUUS8cPXx8HqqtZienHauH/BO5OZTwSNmLrie7l/v1WUgvXBP
5k8p8wxtvXf0JNdTtNFPL82Q5W0f/4GE6PGAet4TxjNVLs9DolnTdBN7isntm9iI
d6BuuQLzNcd9J2p+7qm0Uu8gh7TeNbnBgaJnJiwCtogYMsOGFiimrF3ce3vJz/Qq
tN/od94ffjun+hWgCGQUIPZNFNNOAx7oUQmjQi0ubm/XiCEvNJHBNkvSImTidrrn
sHTu/FLkntCWzHTA8MfA3eZmqsNNUojQvVcSxs01Bwy/UKgYhcpuijRve5HsloSz
IT0QVhzpIdM=
-----END CERTIFICATE-----
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/bin/bash

openssl req \
-x509 \
-newkey rsa:4096 \
-keyout key.pem \
-out cert.pem \
-sha256 \
-nodes \
-addext 'subjectAltName = IP:127.0.0.1' \
-subj "/C=US/O=Canonical/CN=CanonicalGroupLimited"

echo This is not a valid certificate > bad-certificate.pem
Loading

0 comments on commit ff38d12

Please sign in to comment.