Certstore: Added cert_match_skip_invalid option

Replaces #4384.

Co-authored-by: Daniel Modler <modler@linkbit.io>
Co-authored-by: Neil Twigg <neil@nats.io>
Signed-off-by: Neil Twigg <neil@nats.io>

server/certstore/certstore_other.go

@@ -26,8 +26,7 @@ var _ = MATCHBYEMPTY
 // otherKey implements crypto.Signer and crypto.Decrypter to satisfy linter on platforms that don't implement certstore
 type otherKey struct{}
 
-func TLSConfig(certStore StoreType, certMatchBy MatchByType, certMatch string, caCertsMatch []string, config *tls.Config) error {
-	_, _, _, _, _ = certStore, certMatchBy, certMatch, caCertsMatch, config
+func TLSConfig(_ StoreType, _ MatchByType, _ string, _ []string, _ bool, _ *tls.Config) error {
 	return ErrOSNotCompatCertStore
 }
 

server/certstore/certstore_windows.go

@@ -130,6 +130,7 @@ var (
 	winNCrypt  = windows.NewLazySystemDLL("ncrypt.dll")
 
 	winCertFindCertificateInStore        = winCrypt32.NewProc("CertFindCertificateInStore")
+	winCertVerifyTimeValidity            = winCrypt32.NewProc("CertVerifyTimeValidity")
 	winCryptAcquireCertificatePrivateKey = winCrypt32.NewProc("CryptAcquireCertificatePrivateKey")
 	winNCryptExportKey                   = winNCrypt.NewProc("NCryptExportKey")
 	winNCryptOpenStorageProvider         = winNCrypt.NewProc("NCryptOpenStorageProvider")
@@ -171,11 +172,11 @@ type winPSSPaddingInfo struct {
 // adding all matching certificates from the caCertsMatch array to the pool.
 // All matching certificates (vs first) are added to the pool based on a user
 // request. If no certificates are found an error is returned.
-func createCACertsPool(cs *winCertStore, storeType uint32, caCertsMatch []string) (*x509.CertPool, error) {
+func createCACertsPool(cs *winCertStore, storeType uint32, caCertsMatch []string, skipInvalid bool) (*x509.CertPool, error) {
 	var errs []error
 	caPool := x509.NewCertPool()
 	for _, s := range caCertsMatch {
-		lfs, err := cs.caCertsBySubjectMatch(s, storeType)
+		lfs, err := cs.caCertsBySubjectMatch(s, storeType, skipInvalid)
 		if err != nil {
 			errs = append(errs, err)
 		} else {
@@ -200,7 +201,7 @@ func createCACertsPool(cs *winCertStore, storeType uint32, caCertsMatch []string
 // Subjects matching the provided strings. If a match is found, the
 // certificate is added to the pool that is used to verify the certificate
 // chain.
-func TLSConfig(certStore StoreType, certMatchBy MatchByType, certMatch string, caCertsMatch []string, config *tls.Config) error {
+func TLSConfig(certStore StoreType, certMatchBy MatchByType, certMatch string, caCertsMatch []string, skipInvalid bool, config *tls.Config) error {
 	var (
 		leaf     *x509.Certificate
 		leafCtx  *windows.CertContext
@@ -227,11 +228,11 @@ func TLSConfig(certStore StoreType, certMatchBy MatchByType, certMatch string, c
 
 		// certByIssuer or certBySubject
 		if certMatchBy == matchBySubject || certMatchBy == MATCHBYEMPTY {
-			leaf, leafCtx, err = cs.certBySubject(certMatch, scope)
+			leaf, leafCtx, err = cs.certBySubject(certMatch, scope, skipInvalid)
 		} else if certMatchBy == matchByIssuer {
-			leaf, leafCtx, err = cs.certByIssuer(certMatch, scope)
+			leaf, leafCtx, err = cs.certByIssuer(certMatch, scope, skipInvalid)
 		} else if certMatchBy == matchByThumbprint {
-			leaf, leafCtx, err = cs.certByThumbprint(certMatch, scope)
+			leaf, leafCtx, err = cs.certByThumbprint(certMatch, scope, skipInvalid)
 		} else {
 			return ErrBadMatchByType
 		}
@@ -251,7 +252,7 @@ func TLSConfig(certStore StoreType, certMatchBy MatchByType, certMatch string, c
 		}
 		// Look for CA Certificates
 		if len(caCertsMatch) != 0 {
-			caPool, err := createCACertsPool(cs, scope, caCertsMatch)
+			caPool, err := createCACertsPool(cs, scope, caCertsMatch, skipInvalid)
 			if err != nil {
 				return err
 			}
@@ -339,6 +340,16 @@ func winFindCert(store windows.Handle, enc, findFlags, findType uint32, para *ui
 	return (*windows.CertContext)(unsafe.Pointer(h)), nil
 }
 
+// winVerifyCertValid wraps the CertVerifyTimeValidity and simply returns true if the certificate is valid
+func winVerifyCertValid(timeToVerify *windows.Filetime, certInfo *windows.CertInfo) bool {
+	// this function does not document returning errors / setting lasterror
+	r, _, _ := winCertVerifyTimeValidity.Call(
+		uintptr(unsafe.Pointer(timeToVerify)),
+		uintptr(unsafe.Pointer(certInfo)),
+	)
+	return r == 0
+}
+
 // winCertStore is a store implementation for the Windows Certificate Store
 type winCertStore struct {
 	Prov     uintptr
@@ -378,31 +389,31 @@ func winCertContextToX509(ctx *windows.CertContext) (*x509.Certificate, error) {
 // CertContext pointer returned allows subsequent key operations like Sign. Caller specifies
 // current user's personal certs or local machine's personal certs using storeType.
 // See CERT_FIND_ISSUER_STR description at https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certfindcertificateinstore
-func (w *winCertStore) certByIssuer(issuer string, storeType uint32) (*x509.Certificate, *windows.CertContext, error) {
-	return w.certSearch(winFindIssuerStr, issuer, winMyStore, storeType)
+func (w *winCertStore) certByIssuer(issuer string, storeType uint32, skipInvalid bool) (*x509.Certificate, *windows.CertContext, error) {
+	return w.certSearch(winFindIssuerStr, issuer, winMyStore, storeType, skipInvalid)
 }
 
 // certBySubject matches and returns the first certificate found by passed subject field.
 // CertContext pointer returned allows subsequent key operations like Sign. Caller specifies
 // current user's personal certs or local machine's personal certs using storeType.
 // See CERT_FIND_SUBJECT_STR description at https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certfindcertificateinstore
-func (w *winCertStore) certBySubject(subject string, storeType uint32) (*x509.Certificate, *windows.CertContext, error) {
-	return w.certSearch(winFindSubjectStr, subject, winMyStore, storeType)
+func (w *winCertStore) certBySubject(subject string, storeType uint32, skipInvalid bool) (*x509.Certificate, *windows.CertContext, error) {
+	return w.certSearch(winFindSubjectStr, subject, winMyStore, storeType, skipInvalid)
 }
 
 // certByThumbprint matches and returns the first certificate found by passed SHA1 thumbprint.
 // CertContext pointer returned allows subsequent key operations like Sign. Caller specifies
 // current user's personal certs or local machine's personal certs using storeType.
 // See CERT_FIND_SUBJECT_STR description at https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certfindcertificateinstore
-func (w *winCertStore) certByThumbprint(hash string, storeType uint32) (*x509.Certificate, *windows.CertContext, error) {
+func (w *winCertStore) certByThumbprint(hash string, storeType uint32, skipInvalid bool) (*x509.Certificate, *windows.CertContext, error) {
 	hb, err := hex.DecodeString(hash)
 	if err != nil {
 		return nil, nil, err
 	}
 	if len(hb) != sha1.Size {
 		return nil, nil, fmt.Errorf("incorrect thumbprint length %d", len(hb))
 	}
-	return w.certSearch(winFindHashStr, string(hb), winMyStore, storeType)
+	return w.certSearch(winFindHashStr, string(hb), winMyStore, storeType, skipInvalid)
 }
 
 // caCertsBySubjectMatch matches and returns all matching certificates of the subject field.
@@ -414,7 +425,7 @@ func (w *winCertStore) certByThumbprint(hash string, storeType uint32) (*x509.Ce
 //
 // Caller specifies current user's personal certs or local machine's personal certs using storeType.
 // See CERT_FIND_SUBJECT_STR description at https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certfindcertificateinstore
-func (w *winCertStore) caCertsBySubjectMatch(subject string, storeType uint32) ([]*x509.Certificate, error) {
+func (w *winCertStore) caCertsBySubjectMatch(subject string, storeType uint32, skipInvalid bool) ([]*x509.Certificate, error) {
 	var (
 		leaf            *x509.Certificate
 		searchLocations = [3]*uint16{winRootStore, winAuthRootStore, winIntermediateCAStore}
@@ -426,7 +437,7 @@ func (w *winCertStore) caCertsBySubjectMatch(subject string, storeType uint32) (
 	}
 	for _, sr := range searchLocations {
 		var err error
-		if leaf, _, err = w.certSearch(winFindSubjectStr, subject, sr, storeType); err == nil {
+		if leaf, _, err = w.certSearch(winFindSubjectStr, subject, sr, storeType, skipInvalid); err == nil {
 			rv = append(rv, leaf)
 		} else {
 			// Ignore the failed search from a single location. Errors we catch include
@@ -448,7 +459,7 @@ func (w *winCertStore) caCertsBySubjectMatch(subject string, storeType uint32) (
 
 // certSearch is a helper function to lookup certificates based on search type and match value.
 // store is used to specify which store to perform the lookup in (system or user).
-func (w *winCertStore) certSearch(searchType uint32, matchValue string, searchRoot *uint16, store uint32) (*x509.Certificate, *windows.CertContext, error) {
+func (w *winCertStore) certSearch(searchType uint32, matchValue string, searchRoot *uint16, store uint32, skipInvalid bool) (*x509.Certificate, *windows.CertContext, error) {
 	// store handle to "MY" store
 	h, err := w.storeHandle(store, searchRoot)
 	if err != nil {
@@ -465,23 +476,32 @@ func (w *winCertStore) certSearch(searchType uint32, matchValue string, searchRo
 
 	// pass 0 as the third parameter because it is not used
 	// https://msdn.microsoft.com/en-us/library/windows/desktop/aa376064(v=vs.85).aspx
-	nc, err := winFindCert(h, winEncodingX509ASN|winEncodingPKCS7, 0, searchType, i, prev)
-	if err != nil {
-		return nil, nil, err
-	}
-	if nc != nil {
-		// certificate found
-		prev = nc
 
-		// Extract the DER-encoded certificate from the cert context
-		xc, err := winCertContextToX509(nc)
-		if err == nil {
-			cert = xc
+	for {
+		nc, err := winFindCert(h, winEncodingX509ASN|winEncodingPKCS7, 0, searchType, i, prev)
+		if err != nil {
+			return nil, nil, err
+		}
+		if nc != nil {
+			// certificate found
+			prev = nc
+
+			var now *windows.Filetime
+			if skipInvalid && !winVerifyCertValid(now, nc.CertInfo) {
+				continue
+			}
+
+			// Extract the DER-encoded certificate from the cert context
+			xc, err := winCertContextToX509(nc)
+			if err == nil {
+				cert = xc
+				break
+			} else {
+				return nil, nil, ErrFailedX509Extract
+			}
 		} else {
-			return nil, nil, ErrFailedX509Extract
+			return nil, nil, ErrFailedCertSearch
 		}
-	} else {
-		return nil, nil, ErrFailedCertSearch
 	}
 
 	if cert == nil {

server/certstore/errors.go

@@ -71,6 +71,9 @@ var (
 	// ErrBadCaCertMatchField represents malformed cert_match option
 	ErrBadCaCertMatchField = errors.New("expected 'ca_certs_match' to be a valid non-empty string array")
 
+	// ErrBadCertMatchSkipInvalidField represents malformed cert_match_skip_invalid option
+	ErrBadCertMatchSkipInvalidField = errors.New("expected 'cert_match_skip_invalid' to be a boolean")
+
 	// ErrOSNotCompatCertStore represents cert_store passed that exists but is not valid on current OS
 	ErrOSNotCompatCertStore = errors.New("cert_store not compatible with current operating system")
 )

server/certstore_windows_test.go

@@ -28,9 +28,12 @@ import (
 )
 
 func runPowershellScript(scriptFile string, args []string) error {
-	_ = args
 	psExec, _ := exec.LookPath("powershell.exe")
+
 	execArgs := []string{psExec, "-command", fmt.Sprintf("& '%s'", scriptFile)}
+	if len(args) > 0 {
+		execArgs = append(execArgs, args...)
+	}
 
 	cmdImport := &exec.Cmd{
 		Path:   psExec,
@@ -251,3 +254,55 @@ func TestServerTLSWindowsCertStore(t *testing.T) {
 		})
 	}
 }
+
+// TestServerIgnoreExpiredCerts tests if the server skips expired certificates in configuration, and finds non-expired ones
+func TestServerIgnoreExpiredCerts(t *testing.T) {
+
+	// Server Identities: expired.pem; not-expired.pem
+	// Issuer: OU = NATS.io, CN = localhost
+	// Subject: OU = NATS.io Operators, CN = localhost
+
+	testCases := []struct {
+		certFile string
+		expect   bool
+	}{
+		{"expired.p12", false},
+		{"not-expired.p12", true},
+	}
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("Server certificate: %s", tc.certFile), func(t *testing.T) {
+			// Make sure windows cert store is reset to avoid conflict with other tests
+			err := runPowershellScript("../test/configs/certs/tlsauth/certstore/delete-cert-from-store.ps1", nil)
+			if err != nil {
+				t.Fatalf("expected powershell cert delete to succeed: %s", err.Error())
+			}
+
+			// Provision Windows cert store with server cert and secret
+			err = runPowershellScript("../test/configs/certs/tlsauth/certstore/import-p12-server.ps1", []string{tc.certFile})
+			if err != nil {
+				t.Fatalf("expected powershell provision to succeed: %s", err.Error())
+			}
+			// Fire up the server
+			srvConfig := createConfFile(t, []byte(`
+			listen: "localhost:-1"
+			tls {
+				cert_store: "WindowsCurrentUser"
+				cert_match_by: "Subject"
+				cert_match: "NATS.io Operators"
+				cert_match_skip_invalid: true
+				timeout: 5
+			}
+			`))
+			defer removeFile(t, srvConfig)
+			cfg, _ := ProcessConfigFile(srvConfig)
+			if (cfg != nil) == tc.expect {
+				return
+			}
+			if tc.expect == false {
+				t.Fatalf("expected server start to fail with expired certificate")
+			} else {
+				t.Fatalf("expected server to start with non expired certificate")
+			}
+		})
+	}
+}

server/opts.go

@@ -714,27 +714,28 @@ type authorization struct {
 // TLSConfigOpts holds the parsed tls config information,
 // used with flag parsing
 type TLSConfigOpts struct {
-	CertFile          string
-	KeyFile           string
-	CaFile            string
-	Verify            bool
-	Insecure          bool
-	Map               bool
-	TLSCheckKnownURLs bool
-	HandshakeFirst    bool          // Indicate that the TLS handshake should occur first, before sending the INFO protocol.
-	FallbackDelay     time.Duration // Where supported, indicates how long to wait for the handshake before falling back to sending the INFO protocol first.
-	Timeout           float64
-	RateLimit         int64
-	Ciphers           []uint16
-	CurvePreferences  []tls.CurveID
-	PinnedCerts       PinnedCertSet
-	CertStore         certstore.StoreType
-	CertMatchBy       certstore.MatchByType
-	CertMatch         string
-	CaCertsMatch      []string
-	OCSPPeerConfig    *certidp.OCSPPeerConfig
-	Certificates      []*TLSCertPairOpt
-	MinVersion        uint16
+	CertFile             string
+	KeyFile              string
+	CaFile               string
+	Verify               bool
+	Insecure             bool
+	Map                  bool
+	TLSCheckKnownURLs    bool
+	HandshakeFirst       bool          // Indicate that the TLS handshake should occur first, before sending the INFO protocol.
+	FallbackDelay        time.Duration // Where supported, indicates how long to wait for the handshake before falling back to sending the INFO protocol first.
+	Timeout              float64
+	RateLimit            int64
+	Ciphers              []uint16
+	CurvePreferences     []tls.CurveID
+	PinnedCerts          PinnedCertSet
+	CertStore            certstore.StoreType
+	CertMatchBy          certstore.MatchByType
+	CertMatch            string
+	CertMatchSkipInvalid bool
+	CaCertsMatch         []string
+	OCSPPeerConfig       *certidp.OCSPPeerConfig
+	Certificates         []*TLSCertPairOpt
+	MinVersion           uint16
 }
 
 // TLSCertPairOpt are the paths to a certificate and private key.
@@ -4819,6 +4820,12 @@ func parseTLS(v any, isClientCtx bool) (t *TLSConfigOpts, retErr error) {
 			default:
 				return nil, &configErr{tk, fmt.Sprintf("field %q should be a boolean or a string, got %T", mk, mv)}
 			}
+		case "cert_match_skip_invalid":
+			certMatchSkipInvalid, ok := mv.(bool)
+			if !ok {
+				return nil, &configErr{tk, certstore.ErrBadCertMatchSkipInvalidField.Error()}
+			}
+			tc.CertMatchSkipInvalid = certMatchSkipInvalid
 		case "ocsp_peer":
 			switch vv := mv.(type) {
 			case bool:
@@ -5217,7 +5224,7 @@ func GenTLSConfig(tc *TLSConfigOpts) (*tls.Config, error) {
 		}
 		config.Certificates = []tls.Certificate{cert}
 	case tc.CertStore != certstore.STOREEMPTY:
-		err := certstore.TLSConfig(tc.CertStore, tc.CertMatchBy, tc.CertMatch, tc.CaCertsMatch, &config)
+		err := certstore.TLSConfig(tc.CertStore, tc.CertMatchBy, tc.CertMatch, tc.CaCertsMatch, tc.CertMatchSkipInvalid, &config)
 		if err != nil {
 			return nil, err
 		}

test/configs/certs/tlsauth/certstore/delete-cert-from-store.ps1

@@ -2,4 +2,4 @@ $issuer="NATS CA"
 Get-ChildItem Cert:\CurrentUser\My | Where-Object {$_.Issuer -match $issuer} | Remove-Item
 Get-ChildItem Cert:\CurrentUser\CA| Where-Object {$_.Issuer -match $issuer} | Remove-Item
 Get-ChildItem Cert:\CurrentUser\AuthRoot | Where-Object {$_.Issuer -match $issuer} | Remove-Item
-Get-ChildItem Cert:\CurrentUser\Root | Where-Object {$_.Issuer -match $issuer} | Remove-Item
+Get-ChildItem Cert:\CurrentUser\Root | Where-Object {$_.Issuer -match $issuer} | Remove-Item

test/configs/certs/tlsauth/certstore/import-p12-server.ps1

@@ -1,4 +1,7 @@
-$fileLocale = $PSScriptRoot + "\server.p12"
+$file=$args[0]
+if (!$file) { $file="server.p12 "}
+$fileLocale = $PSScriptRoot + "\" + $file
+echo "Installing certificate $fileLocale"
 $Pass = ConvertTo-SecureString -String 's3cr3t' -Force -AsPlainText
 $User = "whatever"
 $Cred = New-Object -TypeName "System.Management.Automation.PSCredential" -ArgumentList $User, $Pass