feat: Add opt-in support for proxy CA cert installation (#318)

* feat: Add opt-in support for proxy CA cert installation

* fix: Code review fixes

* fix: Code review fixes

* fix: Code review fixes

* fix: Code review fixes

* fix: Code review fixes

* docs: Add limitation for MacOS MDM script

proxy/certmanager/certmanager_test.go

@@ -35,7 +35,7 @@ func TestGenerateCA(t *testing.T) {
 
 	assert.True(t, ca.X509Cert.IsCA, "Certificate should be marked as CA")
 
-	assert.Equal(t, "PMG Proxy CA", ca.X509Cert.Subject.CommonName, "Common name should be PMG Proxy CA")
+	assert.Equal(t, "SafeDep PMG Proxy CA", ca.X509Cert.Subject.CommonName, "Common name should be SafeDep PMG Proxy CA")
 
 	assert.Greater(t, ca.X509Cert.NotAfter.Sub(ca.X509Cert.NotBefore).Hours(),
 		float64(config.CAValidityDays*24-1), "CA certificate validity period should be greater than the configured validity days")

proxy/certmanager/generator.go

@@ -26,6 +26,11 @@ const (
 	goosWindows                          = "windows"
 )
 
+// CACommonName is the subject CN of the PMG CA. It is the single source of truth
+// for the CA's identity: GenerateCA stamps it onto the certificate and the
+// truststore package matches on it to find/remove the cert in OS trust stores.
+const CACommonName = "SafeDep PMG Proxy CA"
+
 // certManager implements the CertificateManager interface
 type certManager struct {
 	ca     *Certificate
@@ -207,7 +212,7 @@ func GenerateCA(config CertManagerConfig) (*Certificate, error) {
 	template := &x509.Certificate{
 		SerialNumber: serialNumber,
 		Subject: pkix.Name{
-			CommonName:   "PMG Proxy CA",
+			CommonName:   CACommonName,
 			Organization: []string{"SafeDep PMG"},
 		},
 		NotBefore: notBefore,
@@ -292,76 +297,68 @@ func ParseTLSCertificate(cert *Certificate) (tls.Certificate, error) {
 	return tlsCert, nil
 }
 
-// GenerateCAWithSystemCA generates a self-signed certificate and appends system CA bundle
-// content to the certificate bytes. If the system bundle is unavailable or too large to merge,
-// it falls back to the PMG CA so proxy startup remains functional.
+// MergeWithSystemCA appends the system CA bundle to certPEM so env-var trust
+// injection (SSL_CERT_FILE, NODE_EXTRA_CA_CERTS, ...) covers both the PMG CA and
+// the real world. It is best-effort: any problem locating, sizing, or reading the
+// system bundle results in certPEM being returned unchanged, so proxy startup is
+// never blocked by an optional enhancement.
+func MergeWithSystemCA(certPEM []byte) []byte {
+	systemBundlePath := firstReadablePath(systemCABundleCandidates()...)
+	if systemBundlePath == "" {
+		log.Warnf("Skipping system CA bundle merge: No system CA bundle file found")
+		return certPEM
+	}
+
+	info, err := os.Stat(systemBundlePath)
+	if err != nil {
+		log.Errorf("Skipping system CA bundle merge: failed to stat %s: %v", systemBundlePath, err)
+		return certPEM
+	}
+
+	if info.Size() > maxSystemCABundleBytes {
+		log.Errorf("Skipping system CA bundle merge: %s is too large (%d bytes > %d bytes)",
+			systemBundlePath, info.Size(), maxSystemCABundleBytes)
+		return certPEM
+	}
+
+	systemBundle, err := os.ReadFile(systemBundlePath)
+	if err != nil {
+		log.Errorf("Skipping system CA bundle merge: failed to read %s: %v", systemBundlePath, err)
+		return certPEM
+	}
+
+	const extra = int64(2)
+	totalCap := int64(len(certPEM)) + int64(len(systemBundle)) + extra
+	if totalCap > maxSystemCABundleBytes {
+		log.Errorf("Skipping system CA bundle merge: merged CA would be too large (%d bytes > %d bytes)",
+			totalCap, maxSystemCABundleBytes)
+		return certPEM
+	}
+
+	merged := make([]byte, 0, int(totalCap))
+	merged = append(merged, certPEM...)
+	if len(merged) > 0 && merged[len(merged)-1] != '\n' {
+		merged = append(merged, '\n')
+	}
+	merged = append(merged, systemBundle...)
+	if len(merged) > 0 && merged[len(merged)-1] != '\n' {
+		merged = append(merged, '\n')
+	}
+
+	return merged
+}
+
+// GenerateCAWithSystemCA generates a self-signed CA and merges the system CA
+// bundle into the certificate bytes (for env-var trust injection). Signing
+// always uses the pure CA via X509Cert/PrivKey.
 func GenerateCAWithSystemCA(config CertManagerConfig) (*Certificate, error) {
 	caCert, err := GenerateCA(config)
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate CA: %w", err)
 	}
 
-	caCertPEM := caCert.Certificate
-	systemBundlePath := firstReadablePath(systemCABundleCandidates()...)
-
-	// No system CA found. Continue using only PMG cert.
-	if systemBundlePath == "" {
-		log.Warnf("Skipping system CA bundle merge: No system CA bundle file found")
-		return caCert, nil
-	}
-
-	info, err := os.Stat(systemBundlePath)
-	if err != nil {
-		return nil, fmt.Errorf("failed to stat system CA bundle %s: %w", systemBundlePath, err)
-	}
-
-	// We make sure there is a boundary on the size of CA bundle loaded
-	// from the system. Beyond that, we just skip it and return PMG cert.
-	if info.Size() > maxSystemCABundleBytes {
-		log.Errorf(
-			"Skipping system CA bundle merge: %s is too large (%d bytes > %d bytes)",
-			systemBundlePath,
-			info.Size(),
-			maxSystemCABundleBytes,
-		)
-
-		return caCert, nil
-	}
-
-	systemBundle, err := os.ReadFile(systemBundlePath)
-	if err != nil {
-		return nil, fmt.Errorf("failed to read system CA bundle %s: %w", systemBundlePath, err)
-	}
-
-	caLen := int64(len(caCertPEM))
-	sysLen := int64(len(systemBundle))
-	const extra = int64(2)
-
-	totalCap := caLen + sysLen + extra
-	if totalCap > maxSystemCABundleBytes {
-		log.Errorf(
-			"Skipping system CA bundle merge: merged CA would be too large (%d bytes > %d bytes)",
-			totalCap,
-			maxSystemCABundleBytes,
-		)
-
-		return caCert, nil
-	}
-
-	merged := make([]byte, 0, int(totalCap))
-	merged = append(merged, caCertPEM...)
-
-	if len(merged) > 0 && merged[len(merged)-1] != '\n' {
-		merged = append(merged, '\n')
-	}
-	merged = append(merged, systemBundle...)
-
-	if len(merged) > 0 && merged[len(merged)-1] != '\n' {
-		merged = append(merged, '\n')
-	}
-
 	return &Certificate{
-		Certificate: merged,
+		Certificate: MergeWithSystemCA(caCert.Certificate),
 		PrivateKey:  caCert.PrivateKey,
 		X509Cert:    caCert.X509Cert,
 		PrivKey:     caCert.PrivKey,

proxy/certmanager/generator_merge_test.go

@@ -0,0 +1,28 @@
+package certmanager
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestMergeWithSystemCAContainsInput(t *testing.T) {
+	ca, err := GenerateCA(DefaultCertManagerConfig())
+	require.NoError(t, err)
+
+	merged := MergeWithSystemCA(ca.Certificate)
+
+	// The input cert must always be a prefix of the merged output, regardless
+	// of whether a system bundle was found.
+	assert.True(t, len(merged) >= len(ca.Certificate))
+	assert.Equal(t, ca.Certificate, merged[:len(ca.Certificate)])
+}
+
+func TestGenerateCAWithSystemCAStillWorks(t *testing.T) {
+	ca, err := GenerateCAWithSystemCA(DefaultCertManagerConfig())
+	require.NoError(t, err)
+	require.NotNil(t, ca.X509Cert)
+	require.NotNil(t, ca.PrivKey)
+	assert.NotEmpty(t, ca.Certificate)
+}

proxy/certmanager/persist.go

@@ -0,0 +1,73 @@
+package certmanager
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+)
+
+const (
+	caCertFileName = "ca-cert.pem"
+	caKeyFileName  = "ca-key.pem"
+)
+
+func CACertPath(dir string) string { return filepath.Join(dir, caCertFileName) }
+func CAKeyPath(dir string) string  { return filepath.Join(dir, caKeyFileName) }
+
+// PersistentCACertManagerConfig returns the config for the on-disk,
+// system-trusted CA. The root is long-lived (10 years) because rotating an
+// installed, trusted root is expensive; leaf certs remain short (1 day).
+func PersistentCACertManagerConfig() CertManagerConfig {
+	c := DefaultCertManagerConfig()
+	c.CAValidityDays = 3650
+	return c
+}
+
+// SaveCA writes the CA certificate (0644) and private key (0600) to dir.
+// Only the pure PMG CA is persisted — not the system-bundle-merged PEM.
+func SaveCA(dir string, ca *Certificate) error {
+	if ca == nil {
+		return fmt.Errorf("ca certificate is nil")
+	}
+
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		return fmt.Errorf("failed to create config dir %s: %w", dir, err)
+	}
+
+	if err := os.WriteFile(CACertPath(dir), ca.Certificate, 0o644); err != nil {
+		return fmt.Errorf("failed to write CA certificate: %w", err)
+	}
+
+	// Remove any pre-existing key first so the new file is created fresh with
+	// 0600. os.WriteFile preserves an existing file's mode, which could otherwise
+	// leave a group/world-readable private key behind on a --force re-install.
+	if err := os.Remove(CAKeyPath(dir)); err != nil && !os.IsNotExist(err) {
+		return fmt.Errorf("failed to reset CA private key file: %w", err)
+	}
+	if err := os.WriteFile(CAKeyPath(dir), ca.PrivateKey, 0o600); err != nil {
+		return fmt.Errorf("failed to write CA private key: %w", err)
+	}
+
+	return nil
+}
+
+// LoadCA reads and parses the persisted CA from dir. When a file is missing
+// the returned error wraps os.ErrNotExist so callers can use errors.Is.
+func LoadCA(dir string) (*Certificate, error) {
+	certPEM, err := os.ReadFile(CACertPath(dir))
+	if err != nil {
+		return nil, fmt.Errorf("failed to read CA certificate: %w", err)
+	}
+
+	keyPEM, err := os.ReadFile(CAKeyPath(dir))
+	if err != nil {
+		return nil, fmt.Errorf("failed to read CA private key: %w", err)
+	}
+
+	parsed, err := parseCertificate(&Certificate{Certificate: certPEM, PrivateKey: keyPEM})
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse persisted CA: %w", err)
+	}
+
+	return parsed, nil
+}

proxy/certmanager/persist_test.go

@@ -0,0 +1,48 @@
+package certmanager
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestSaveAndLoadCARoundTrip(t *testing.T) {
+	dir := t.TempDir()
+
+	ca, err := GenerateCA(DefaultCertManagerConfig())
+	require.NoError(t, err)
+
+	require.NoError(t, SaveCA(dir, ca))
+
+	if runtime.GOOS != "windows" {
+		keyInfo, err := os.Stat(CAKeyPath(dir))
+		require.NoError(t, err)
+		assert.Equal(t, os.FileMode(0o600), keyInfo.Mode().Perm())
+
+		certInfo, err := os.Stat(CACertPath(dir))
+		require.NoError(t, err)
+		assert.Equal(t, os.FileMode(0o644), certInfo.Mode().Perm())
+	}
+
+	loaded, err := LoadCA(dir)
+	require.NoError(t, err)
+	assert.Equal(t, ca.Certificate, loaded.Certificate)
+	assert.Equal(t, ca.PrivateKey, loaded.PrivateKey)
+	require.NotNil(t, loaded.X509Cert)
+	require.NotNil(t, loaded.PrivKey)
+}
+
+func TestLoadCAMissingIsNotExist(t *testing.T) {
+	_, err := LoadCA(filepath.Join(t.TempDir(), "nope"))
+	require.Error(t, err)
+	assert.True(t, errors.Is(err, os.ErrNotExist))
+}
+
+func TestPersistentConfigIsLongLived(t *testing.T) {
+	assert.Equal(t, 3650, PersistentCACertManagerConfig().CAValidityDays)
+}

proxy/certmanager/status.go

@@ -0,0 +1,84 @@
+package certmanager
+
+import (
+	"crypto/sha256"
+	"crypto/x509"
+	"encoding/hex"
+	"encoding/pem"
+	"fmt"
+	"os"
+	"time"
+)
+
+// ExpiryWarnWindow is how close to NotAfter a CA must be before status/doctor
+// surface an expiry warning.
+const ExpiryWarnWindow = 30 * 24 * time.Hour
+
+// CAStatus is the reusable status model shared by `cert status`, `setup doctor`,
+// and `setup info`. Disk-side fields are filled by InspectCA; trust fields are
+// filled by the caller from the truststore package (kept here so certmanager
+// stays free of any OS/trust-store import).
+type CAStatus struct {
+	KeyPresent    bool
+	CertPresent   bool
+	NotAfter      time.Time
+	Expired       bool
+	ExpiringSoon  bool
+	Fingerprint   string
+	UserTrusted   bool
+	SystemTrusted bool
+}
+
+func (s CAStatus) Trusted() bool { return s.UserTrusted || s.SystemTrusted }
+
+// Drift reports on-disk integrity / expiry problems independent of OS trust
+// policy. Trust-store presence is intentionally NOT evaluated here: "not in a
+// store" is normal on Linux (Go honors SSL_CERT_FILE), so consumers interpret
+// trust state per platform.
+func (s CAStatus) Drift() (bool, string) {
+	if s.CertPresent && !s.KeyPresent {
+		return true, "CA certificate on disk but private key missing"
+	}
+	if s.KeyPresent && !s.CertPresent {
+		return true, "CA private key on disk but certificate missing"
+	}
+	if s.KeyPresent && s.Expired {
+		return true, "CA expired; re-run `pmg setup cert install`"
+	}
+	return false, ""
+}
+
+// InspectCA gathers disk-side CA facts from dir. Absence of the files is not an
+// error; the returned CAStatus simply reports them as not present.
+func InspectCA(dir string) (CAStatus, error) {
+	var st CAStatus
+
+	if _, err := os.Stat(CAKeyPath(dir)); err == nil {
+		st.KeyPresent = true
+	}
+
+	certPEM, err := os.ReadFile(CACertPath(dir))
+	if err != nil {
+		return st, nil
+	}
+	st.CertPresent = true
+
+	block, _ := pem.Decode(certPEM)
+	if block == nil {
+		return st, fmt.Errorf("failed to decode CA certificate PEM")
+	}
+
+	x509Cert, err := x509.ParseCertificate(block.Bytes)
+	if err != nil {
+		return st, fmt.Errorf("failed to parse CA certificate: %w", err)
+	}
+
+	st.NotAfter = x509Cert.NotAfter
+	st.Expired = time.Now().After(x509Cert.NotAfter)
+	st.ExpiringSoon = !st.Expired && time.Until(x509Cert.NotAfter) < ExpiryWarnWindow
+
+	sum := sha256.Sum256(x509Cert.Raw)
+	st.Fingerprint = hex.EncodeToString(sum[:])
+
+	return st, nil
+}

proxy/certmanager/status_test.go

@@ -0,0 +1,60 @@
+package certmanager
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestInspectCAAbsent(t *testing.T) {
+	st, err := InspectCA(t.TempDir())
+	require.NoError(t, err)
+	assert.False(t, st.KeyPresent)
+	assert.False(t, st.CertPresent)
+}
+
+func TestInspectCAPresent(t *testing.T) {
+	dir := t.TempDir()
+	ca, err := GenerateCA(PersistentCACertManagerConfig())
+	require.NoError(t, err)
+	require.NoError(t, SaveCA(dir, ca))
+
+	st, err := InspectCA(dir)
+	require.NoError(t, err)
+	assert.True(t, st.KeyPresent)
+	assert.True(t, st.CertPresent)
+	assert.False(t, st.Expired)
+	assert.NotEmpty(t, st.Fingerprint)
+	assert.WithinDuration(t, ca.X509Cert.NotAfter, st.NotAfter, time.Second)
+}
+
+func TestCAStatusDrift(t *testing.T) {
+	cases := []struct {
+		name      string
+		status    CAStatus
+		wantDrift bool
+	}{
+		{"healthy", CAStatus{KeyPresent: true, CertPresent: true}, false},
+		{"cert without key", CAStatus{CertPresent: true}, true},
+		{"key without cert", CAStatus{KeyPresent: true}, true},
+		{"expired", CAStatus{KeyPresent: true, CertPresent: true, Expired: true}, true},
+		{"expiring soon is not drift", CAStatus{KeyPresent: true, CertPresent: true, ExpiringSoon: true}, false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			drift, reason := tc.status.Drift()
+			assert.Equal(t, tc.wantDrift, drift)
+			if tc.wantDrift {
+				assert.NotEmpty(t, reason)
+			}
+		})
+	}
+}
+
+func TestCAStatusTrusted(t *testing.T) {
+	assert.True(t, CAStatus{UserTrusted: true}.Trusted())
+	assert.True(t, CAStatus{SystemTrusted: true}.Trusted())
+	assert.False(t, CAStatus{}.Trusted())
+}