Misc security related fixes for AWS and Azure

src/cloud-providers/aws/provider.go

@@ -27,6 +27,7 @@ var errNotReady = errors.New("address not ready")
 const (
 	maxInstanceNameLen = 63
 	maxWaitTime        = 120 * time.Second
+	maxInt32           = 1<<31 - 1
 )
 
 // Make ec2Client a mockable interface
@@ -103,6 +104,15 @@ func NewProvider(config *Config) (provider.Provider, error) {
 			config.RootVolumeSize = int(deviceSize)
 		}
 
+		// Ensure RootVolumeSize is not more than max int32
+		// The AWS apis accepts only int32, however the flags package has only IntVar.
+		// So we can't make RootVolumeSize as int32, hence checking for overflow here.
+
+		if config.RootVolumeSize > maxInt32 {
+			logger.Printf("RootVolumeSize %d exceeds max int32 value, setting to max int32", config.RootVolumeSize)
+			config.RootVolumeSize = maxInt32
+		}
+
 		// Update the serviceConfig with the device name
 		config.RootDeviceName = deviceName
 
@@ -249,6 +259,8 @@ func (p *awsProvider) CreateInstance(ctx context.Context, podName, sandboxID str
 			{
 				DeviceName: aws.String(p.serviceConfig.RootDeviceName),
 				Ebs: &types.EbsBlockDevice{
+					// We have already ensured RootVolumeSize is not more than max int32 in NewProvider
+					// Hence we can safely convert it to int32
 					VolumeSize: aws.Int32(int32(p.serviceConfig.RootVolumeSize)),
 				},
 			},

src/cloud-providers/azure/provider.go

@@ -11,6 +11,7 @@ import (
 	"log"
 	"net/netip"
 	"os"
+	"path/filepath"
 	"regexp"
 	"strings"
 
@@ -40,6 +41,9 @@ func NewProvider(config *Config) (provider.Provider, error) {
 
 	logger.Printf("azure config %+v", config.Redact())
 
+	// Clean the config.SSHKeyPath to avoid bad paths
+	config.SSHKeyPath = filepath.Clean(config.SSHKeyPath)
+
 	azureClient, err := NewAzureClient(*config)
 	if err != nil {
 		logger.Printf("creating azure client: %v", err)
@@ -295,6 +299,11 @@ func (p *azureProvider) ConfigVerifier() error {
 	if len(ImageId) == 0 {
 		return fmt.Errorf("ImageId is empty")
 	}
+
+	// Verify it's an SSH key file with the right permissions
+	if err := provider.VerifySSHKeyFile(p.serviceConfig.SSHKeyPath); err != nil {
+		return fmt.Errorf("SSH key is invalid: %s", err)
+	}
 	return nil
 }
 

src/cloud-providers/go.mod

@@ -23,6 +23,7 @@ require (
 	github.com/kdomanski/iso9660 v0.4.0
 	github.com/stretchr/testify v1.9.0
 	github.com/vmware/govmomi v0.33.1
+	golang.org/x/crypto v0.24.0
 	golang.org/x/oauth2 v0.17.0
 	google.golang.org/api v0.149.0
 	google.golang.org/protobuf v1.33.0
@@ -110,7 +111,6 @@ require (
 	go.opentelemetry.io/otel/metric v1.25.0 // indirect
 	go.opentelemetry.io/otel/sdk v1.25.0 // indirect
 	go.opentelemetry.io/otel/trace v1.25.0 // indirect
-	golang.org/x/crypto v0.24.0 // indirect
 	golang.org/x/net v0.26.0 // indirect
 	golang.org/x/sys v0.21.0 // indirect
 	golang.org/x/term v0.21.0 // indirect

src/cloud-providers/util.go

@@ -11,6 +11,7 @@ import (
 	"sort"
 
 	"github.com/confidential-containers/cloud-api-adaptor/src/cloud-providers/util"
+	"golang.org/x/crypto/ssh"
 )
 
 var logger = log.New(log.Writer(), "[adaptor/cloud] ", log.LstdFlags|log.Lmsgprefix)
@@ -142,3 +143,39 @@ func WriteUserData(instanceName string, userData string, dataDir string) (string
 	// Return the file path
 	return filePath, nil
 }
+
+// Verify SSH public key file
+// Check the permissions and the content of the file to ensure it is a valid SSH public key
+func VerifySSHKeyFile(sshKeyFile string) error {
+
+	// Check if the file exists
+	_, err := os.Stat(sshKeyFile)
+	if err != nil {
+		return fmt.Errorf("failed to verify SSH key file: %w", err)
+	}
+
+	// Check the permissions of the file
+	fileInfo, err := os.Stat(sshKeyFile)
+	if err != nil {
+		return fmt.Errorf("failed to verify SSH key file: %w", err)
+	}
+
+	// Check if the file permissions are exactly 600
+	if fileInfo.Mode().Perm() != 0600 {
+		return fmt.Errorf("SSH key file permissions are not 600: %s", sshKeyFile)
+	}
+
+	// Read the content of the file
+	content, err := os.ReadFile(sshKeyFile)
+	if err != nil {
+		return fmt.Errorf("failed to read SSH key file: %w", err)
+	}
+
+	// Check if the content is a valid SSH public key
+	_, _, _, _, err = ssh.ParseAuthorizedKey(content)
+	if err != nil {
+		return fmt.Errorf("invalid SSH public key: %w", err)
+	}
+
+	return nil
+}

src/cloud-providers/util_test.go

@@ -2,6 +2,7 @@ package provider
 
 import (
 	"fmt"
+	"os"
 	"reflect"
 	"testing"
 	"time"
@@ -373,3 +374,86 @@ func TestGetBestFitInstanceType(t *testing.T) {
 		})
 	}
 }
+
+func TestVerifySSHKeyFile(t *testing.T) {
+	tests := []struct {
+		name        string
+		setup       func() (string, error)
+		expectedErr bool
+	}{
+		{
+			name: "File does not exist",
+			setup: func() (string, error) {
+				return "/non/existent/file", nil
+			},
+			expectedErr: true,
+		},
+		{
+			name: "File with incorrect permissions",
+			setup: func() (string, error) {
+				file, err := os.CreateTemp("", "sshkey")
+				if err != nil {
+					return "", err
+				}
+				defer file.Close()
+				if err := os.Chmod(file.Name(), 0644); err != nil {
+					return "", err
+				}
+				return file.Name(), nil
+			},
+			expectedErr: true,
+		},
+		{
+			name: "File with invalid SSH key content",
+			setup: func() (string, error) {
+				file, err := os.CreateTemp("", "sshkey")
+				if err != nil {
+					return "", err
+				}
+				defer file.Close()
+				if _, err := file.WriteString("invalid-key-content"); err != nil {
+					return "", err
+				}
+				if err := os.Chmod(file.Name(), 0600); err != nil {
+					return "", err
+				}
+				return file.Name(), nil
+			},
+			expectedErr: true,
+		},
+		{
+			name: "File with valid SSH key content",
+			setup: func() (string, error) {
+				file, err := os.CreateTemp("", "sshkey")
+				if err != nil {
+					return "", err
+				}
+				defer file.Close()
+				if _, err := file.WriteString("ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAYgc9x91raNF1kh/7+XA9EpN4IoQnWC5kv1g107wVmt"); err != nil {
+					return "", err
+				}
+				if err := os.Chmod(file.Name(), 0600); err != nil {
+					return "", err
+				}
+				return file.Name(), nil
+
+			},
+			expectedErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			sshKeyFile, err := tt.setup()
+			if err != nil {
+				t.Errorf("Error setting up test: %v", err)
+				return
+			}
+			// Display the file content
+			fmt.Printf("File content: %s\n", sshKeyFile)
+			err = VerifySSHKeyFile(sshKeyFile)
+			if (err != nil) != tt.expectedErr {
+				t.Errorf("VerifySSHKeyFile() error = %v, expectedErr %v", err, tt.expectedErr)
+			}
+		})
+	}
+}