Add CA Search functionality for the windows cert store

[ColinSullivan1]

This PR adds a new field for searching the windows Certificate Store for CA certificates.

When specifying a value in the ca_certs_match field, the following locations are searched:

• Trusted Root Certification Authorities
• Third-Party Root Certification Authorities
• Intermediate Certification Authorities

Based on user input, the first matching certificate from subject in the list, from each location, is added to the certificate pool. If no certificates can be found an error is reported.

Example usage:

tls {
  cert_store: "WindowsLocalMachine"
  cert_match_by: "Subject"
  cert_match: "client001"
  ca_certs_match: ["CA1","CA2"]
  timeout: 3s
}

Additional Notes:
Test scripts have been added to add and remove the test CA certificate, and existing windows certificates have been updated as they were expired.

Signed-off-by: Colin Sullivan colin@luxantsolutions.com

[ColinSullivan1]

I made a few assumptions, happy to make changes if we want to only include the first discovered matching cert, etc.

EDIT (2/26/24): After thinking a bit more about this over the weekend, I'm wondering if we always stop at the first match, going from Trusted Root -> Third Party -> Intermediates vs continuing to add from all three as the PR is now. wdyt?

[bruth]

Surface level pass on functionality (which looks good!) and a few basic comments.

Not for this PR, but I think we need to carve out Windows-specific tests in a GitHub Action job.

[bruth]

My suggestion for this one is for @derekcollison to do a review. Once its approved and merged, we can include in a preview.2 release of 2.11 for folks to test.

[derekcollison]

I doubt I am the right person to do this review.

[ripienaar]

I doubt I am the right person to do this review.

feature wise this seems fine to me - pity about the breaking API change but for this partcular code I dont think its a problem.

[bruth]

Agree with @ripienaar. @derekcollison Then let's approve and merge once green, cut a new preview shortly, and then iterate based on feedback.

[bruth]

The failure appears to be some change in CLI arguments for MQTT testing. I don't see it necessary to rebase for that test to pass, so approve and merge when ready.

[derekcollison]

LGTM

[ripienaar]

@ColinSullivan1 I am not seeing here a test that shows a client loading CA from certstore can connect to a server thats same - we do have these

nats-server/server/certstore_windows_test.go

Line 227 in 6e768e8

testCases := []struct {

but none with CA loaded from a certstore.

I am having some trouble getting this scenario going, so a test showing its possible would be great!

[ripienaar]

To make it work I think we need to add here

nats-server/server/certstore/certstore_windows.go

Line 235 in 6e768e8

config.ClientCAs = caPool

			config.RootCAs = caPool

But I am not sure if thats the right thing to do? I can also do it on my side basically swap those 2 for client purposes?

[ripienaar]

So for me to use it in a client without modifying the server, it would be:

err = certstore.TLSConfig(storeType, matchBy, c.config.WinCertStoreMatch, c.config.WinCertStoreCaMatch, tlsc)
	if err != nil {
		return nil, err
	}

	if tlsc.ClientCAs != nil {
		tlsc.RootCAs = tlsc.ClientCAs
		tlsc.ClientCAs = nil
	}

I am not sure whats the right thing to do though

[ripienaar]

Seems we do this already in the server here:

nats-server/server/opts.go

Lines 1398 to 1400 in f1cd3ed

	// GenTLSConfig loads the CA file into ClientCAs, but since this will
	// be used as a client connection, we need to set RootCAs.
	o.AccountResolverTLSConfig.RootCAs = tlsConfig.ClientCAs

So I am ok doing that unless you have other oppionions @ColinSullivan1

[ColinSullivan1]

but none with CA loaded from a certstore.

Works for me...

[ColinSullivan1]

@ColinSullivan1 I am not seeing here a test that shows a client loading CA from certstore can connect to a server thats same - we do have these

nats-server/server/certstore_windows_test.go

Line 227 in 6e768e8

testCases := []struct {

but none with CA loaded from a certstore.
I am having some trouble getting this scenario going, so a test showing its possible would be great!

Thanks R.I. - I'll take a look. The CAs added to the certstore were generated from the same CA the client should be using, but I understand we want to future-proof the tests. I'll aim for today, or if not later next week.

[ColinSullivan1]

@ColinSullivan1 I am not seeing here a test that shows a client loading CA from certstore can connect to a server thats same - we do have these

nats-server/server/certstore_windows_test.go

Line 227 in 6e768e8

testCases := []struct {

but none with CA loaded from a certstore.
I am having some trouble getting this scenario going, so a test showing its possible would be great!

Thanks R.I. - I'll take a look. The CAs added to the certstore were generated from the same CA the client should be using, but I understand we want to future-proof the tests. I'll aim for today, or if not later next week.

Per our slack convo I'll hold off on this. lmk if there are any other updates/changes - happy to make them.