Introduce a ValidateGenerateOpts function to ensure that RSA key sizes are at least 2048 bits and EC curve sizes are supported. This validation is now applied at the start of the Generate methods for FileStorage, Credkit, and WinCertStore. Also includes minor refactoring, such as renaming createMode to userReadWrite and updating import order in the Windows implementation.

certtostore.go

@@ -37,7 +37,7 @@ import (
 )
 
 const (
-	createMode = os.FileMode(0600)
+	userReadWrite = os.FileMode(0600)
 )
 
 // Algorithm indicates an asymmetric algorithm used by the credential.
@@ -68,6 +68,23 @@ type GenerateOpts struct {
 	Size int
 }
 
+// ValidateGenerateOpts ensures that the provided GenerateOpts are valid.
+func ValidateGenerateOpts(opts GenerateOpts) error {
+	switch opts.Algorithm {
+	case RSA:
+		if opts.Size < 2048 {
+			return fmt.Errorf("RSA key size must be at least 2048 bits, got %d", opts.Size)
+		}
+	case EC:
+		if _, ok := ecdsaCurves[opts.Size]; !ok {
+			return fmt.Errorf("invalid EC curve size: %d", opts.Size)
+		}
+	default:
+		return fmt.Errorf("unsupported algorithm: %q", opts.Algorithm)
+	}
+	return nil
+}
+
 // CertStorage exposes the different backend storage options for certificates.
 type CertStorage interface {
 	// Cert returns the current X509 certificate or nil if no certificate is installed.
@@ -191,6 +208,9 @@ var ecdsaCurves = map[int]elliptic.Curve{
 
 // Generate creates a new ECDSA or RSA private key and returns a signer that can be used to make a CSR for the key.
 func (f *FileStorage) Generate(opts GenerateOpts) (crypto.Signer, error) {
+	if err := ValidateGenerateOpts(opts); err != nil {
+		return nil, err
+	}
 	var err error
 	switch opts.Algorithm {
 	case RSA:
@@ -212,7 +232,7 @@ func (f *FileStorage) Generate(opts GenerateOpts) (crypto.Signer, error) {
 // Store finishes our cert installation by PEM encoding the cert, intermediate, and key and storing them to disk.
 func (f *FileStorage) Store(cert *x509.Certificate, intermediate *x509.Certificate) error {
 	// Make sure our directory exists
-	if err := os.MkdirAll(filepath.Dir(f.certFile), createMode|0111); err != nil {
+	if err := os.MkdirAll(filepath.Dir(f.certFile), userReadWrite|0111); err != nil {
 		return err
 	}
 
@@ -226,10 +246,10 @@ func (f *FileStorage) Store(cert *x509.Certificate, intermediate *x509.Certifica
 	}
 
 	// Write the certificates out to files
-	if err := ioutil.WriteFile(f.certFile, certBuf.Bytes(), createMode); err != nil {
+	if err := ioutil.WriteFile(f.certFile, certBuf.Bytes(), userReadWrite); err != nil {
 		return err
 	}
-	if err := ioutil.WriteFile(f.caCertFile, intermediateBuf.Bytes(), createMode); err != nil {
+	if err := ioutil.WriteFile(f.caCertFile, intermediateBuf.Bytes(), userReadWrite); err != nil {
 		return err
 	}
 
@@ -246,7 +266,7 @@ func (f *FileStorage) Store(cert *x509.Certificate, intermediate *x509.Certifica
 		return fmt.Errorf("could not encode key to PEM: %v", err)
 	}
 
-	return ioutil.WriteFile(f.keyFile, keyBuf.Bytes(), createMode)
+	return ioutil.WriteFile(f.keyFile, keyBuf.Bytes(), userReadWrite)
 }
 
 // Sign returns a signature for the provided digest.

certtostore_test.go

@@ -387,3 +387,42 @@ func TestPEMToX509(t *testing.T) {
 		t.Fatalf("unexpected certificate issuer got:%v, want:%v", xCissuer, issuer)
 	}
 }
+
+func TestValidateGenerateOpts(t *testing.T) {
+	for _, tt := range []struct {
+		name    string
+		opts    GenerateOpts
+		wantErr bool
+	}{
+		{
+			name: "valid-rsa",
+			opts: GenerateOpts{Algorithm: RSA, Size: 2048},
+		},
+		{
+			name: "valid-ec",
+			opts: GenerateOpts{Algorithm: EC, Size: 256},
+		},
+		{
+			name:    "invalid-rsa-size",
+			opts:    GenerateOpts{Algorithm: RSA, Size: 1024},
+			wantErr: true,
+		},
+		{
+			name:    "invalid-ec-size",
+			opts:    GenerateOpts{Algorithm: EC, Size: 128},
+			wantErr: true,
+		},
+		{
+			name:    "unsupported-algorithm",
+			opts:    GenerateOpts{Algorithm: "DSA", Size: 2048},
+			wantErr: true,
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidateGenerateOpts(tt.opts)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ValidateGenerateOpts(%v) error = %v, wantErr %v", tt.opts, err, tt.wantErr)
+			}
+		})
+	}
+}

certtostore_windows.go

@@ -36,14 +36,13 @@ import (
 	"reflect"
 	"strings"
 	"sync"
-	"syscall"
 	"time"
 	"unicode/utf16"
 	"unsafe"
 
 	"github.com/google/deck"
-	"golang.org/x/crypto/cryptobyte"
 	"golang.org/x/crypto/cryptobyte/asn1"
+	"golang.org/x/crypto/cryptobyte"
 	"golang.org/x/sys/windows"
 )
 
@@ -129,7 +128,7 @@ const (
 	nCryptOverwriteKey = 0x80 // NCRYPT_OVERWRITE_KEY_FLAG
 
 	// winerror.h constants
-	cryptENotFound syscall.Errno = 0x80092004 // CRYPT_E_NOT_FOUND
+	cryptENotFound windows.Errno = 0x80092004 // CRYPT_E_NOT_FOUND
 
 	// ProviderMSPlatform represents the Microsoft Platform Crypto Provider
 	ProviderMSPlatform = "Microsoft Platform Crypto Provider"
@@ -275,7 +274,7 @@ func findCert(store windows.Handle, enc, findFlags, findType uint32, para *uint1
 	)
 	if h == 0 {
 		// Actual error, or simply not found?
-		if errno, ok := err.(syscall.Errno); ok && errno == cryptENotFound {
+		if errno, ok := err.(windows.Errno); ok && errno == cryptENotFound {
 			return nil, nil
 		}
 		return nil, err
@@ -632,12 +631,12 @@ func (w *WinCertStore) Close() error {
 	for _, v := range w.stores {
 		if v != nil {
 			if err := v.Close(); err != nil {
-				errors.Join(result, err)
+				result = errors.Join(result, err)
 			}
 		}
 	}
 	if err := freeObject(w.Prov); err != nil {
-		errors.Join(result, err)
+		result = errors.Join(result, err)
 	}
 	w.certChains = nil
 	return result
@@ -711,6 +710,7 @@ func (w *WinCertStore) Link() error {
 	return nil
 }
 
+// storeHandle provides thread-safe access to a Windows certificate store handle.
 type storeHandle struct {
 	handle *windows.Handle
 }
@@ -1220,6 +1220,9 @@ func (w *WinCertStore) CertKey(cert *windows.CertContext) (*Key, error) {
 // software backed key, depending on support from the host OS
 // key size is set to the maximum supported by Microsoft Software Key Storage Provider
 func (w *WinCertStore) Generate(opts GenerateOpts) (crypto.Signer, error) {
+	if err := ValidateGenerateOpts(opts); err != nil {
+		return nil, err
+	}
 	if w.isReadOnly() {
 		return nil, fmt.Errorf("cannot generate keys in a read-only store")
 	}
@@ -1288,7 +1291,7 @@ func (w *WinCertStore) generateRSA(keySize int) (crypto.Signer, error) {
 	}
 
 	var kh uintptr
-	var length = uint32(keySize)
+	length := uint32(keySize)
 	// Pass 0 as the fifth parameter because it is not used (legacy)
 	// https://msdn.microsoft.com/en-us/library/windows/desktop/aa376247(v=vs.85).aspx
 	r, _, err := nCryptCreatePersistedKey.Call(
@@ -1656,7 +1659,8 @@ func (w *WinCertStore) StoreWithDisposition(cert *x509.Certificate, intermediate
 	return nil
 }
 
-// Returns a handle to a given cert store, opening the handle as needed.
+// storeHandle returns a handle to a given cert store, opening the handle as needed.
+// This method is thread-safe and caches handles within the WinCertStore instance.
 func (w *WinCertStore) storeHandle(provider uint32, store *uint16) (windows.Handle, error) {
 	w.mu.Lock()
 	defer w.mu.Unlock()
@@ -1753,6 +1757,9 @@ func softwareKeyContainers(uniqueID string, storeDomain uint32) (string, string,
 
 // keyMatch takes a known path to a private key and searches for a
 // matching key in a provided directory.
+//
+// TODO: Find a more deterministic way to link CNG and CAPI keys than
+// comparing modification times.
 func keyMatch(keyPath, dir string) (string, error) {
 	key, err := os.Stat(keyPath)
 	if err != nil {