Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Server loads incorrect certificate from Windows Certificate Store #6024

Closed
wooti opened this issue Oct 21, 2024 · 8 comments · Fixed by #6042
Closed

Server loads incorrect certificate from Windows Certificate Store #6024

wooti opened this issue Oct 21, 2024 · 8 comments · Fixed by #6042
Assignees
Labels
defect Suspected defect such as a bug or regression

Comments

@wooti
Copy link

wooti commented Oct 21, 2024

Observed behavior

I start nats-server.exe with the following configuration:

tls: {
  cert_store: "WindowsLocalMachine"
  cert_match_by: "Subject"
  cert_match: "MyCertificate"
  timeout: 2
}
jetstream: enabled
host: my.local.machine

My Windows Certificate Store has multiple test certificates defined, with similar subject names. One is called MyCertificate, another MyCertificateForClient. The former is fully trusted (as it is also registered as a Trusted Root CA), and is the certificate I expect the server to load.

Expected behavior

The server loads certificate MyCertificateForClient. I am unable to connect to the server, with the following error:

nats: error: tls: failed to verify certificate: x509: certificate signed by unknown authority

Server and client version

nats-server: v2.10.22

Host environment

Windows 10, AMD64

Steps to reproduce

It looks like this is due to use of CERT_FIND_ISSUER_STR or CERT_FIND_SUBJECT_STR, more details here. The lookup is for a certificate that "contains the specified string", which both of my test certificates satisfy.

Furthermore, certificates within the store are unordered, and I also suspect that the server could retrieve expired, revoked or untrusted certificates in favour of one that can actually be used.

I found a comment with similar issue here. The follow up here makes sense to me - if I could specify a certificate by thumbprint I could guarantee that the correct certificate is loaded.

See also

#2130
https://github.com/nats-io/nats-architecture-and-design/blob/main/adr/ADR-39.md

@wooti wooti added the defect Suspected defect such as a bug or regression label Oct 21, 2024
@Jarema Jarema self-assigned this Oct 21, 2024
neilalexander added a commit that referenced this issue Oct 25, 2024
Input value should be hex-encoded string of the SHA1 thumbprint.

See: #6024

Signed-off-by: Neil Twigg <neil@nats.io>
@neilalexander
Copy link
Member

Have prepared a branch that should let you specify the certificate by the SHA1 thumbprint instead:

tls: {
  cert_store: "WindowsLocalMachine"
  cert_match_by: "thumbprint"
  cert_match: "2fd4e1c67a2d28fced849ee1bb76e7391b93eb12"
  timeout: 2
}

Can you please give it a try?

https://binaries.nats.dev/binary/github.com/nats-io/nats-server/v2?os=windows&arch=amd64&version=21ad69a5

derekcollison added a commit that referenced this issue Oct 25, 2024
This PR:

* Removes some magic numbers from the certstore code in favour of
constants already defined in `x/sys/windows`
* Adds the `thumbprint` option to `cert_match_by` by allowing matching a
specific certificate my SHA1 thumbprint rather than possibly matching
multiple certificates by name
* Adds the `cert_match_skip_invalid` option by integrating & rebasing a
community PR along with some fix-ups

Fixes #6024 
Fixes #4383
Closes #4384

Signed-off-by: Neil Twigg <neil@nats.io>
@wooti
Copy link
Author

wooti commented Oct 28, 2024

Can you please give it a try?

Hi @neilalexander, many thanks for having a look at this issue!
Unfortunately I wasn't able to make this work with the binary provided.

nats-server: .\nats-streaming.conf:1:0: unable to find certificate in store

@wooti
Copy link
Author

wooti commented Oct 28, 2024

I wonder if CERT_FIND_SHA1_HASH or CERT_FIND_HASH should be used instead of CERT_FIND_HASH_STR in https://github.com/nats-io/nats-server/blob/main/server/certstore/certstore_windows.go#L63 ?

@neilalexander
Copy link
Member

@wooti
Copy link
Author

wooti commented Oct 28, 2024

No luck I'm afraid, same error message

@neilalexander
Copy link
Member

Ah, I see the problem. CERT_FIND_HASH_STR is indeed correct but it needs the string input, not the decoded form.

Please give this one a go — I've tested it on a Windows virtual machine:

https://binaries.nats.dev/binary/github.com/nats-io/nats-server/v2?os=windows&arch=amd64&version=7eb387a5

@wooti
Copy link
Author

wooti commented Oct 28, 2024

Amazing! That one is working great

@neilalexander
Copy link
Member

Thanks for confirming, have raised PR #6047.

neilalexander added a commit that referenced this issue Oct 29, 2024
Input value should be hex-encoded string of the SHA1 thumbprint.

See: #6024

Signed-off-by: Neil Twigg <neil@nats.io>
neilalexander added a commit that referenced this issue Oct 29, 2024
Input value should be hex-encoded string of the SHA1 thumbprint.

See: #6024

Signed-off-by: Neil Twigg <neil@nats.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect Suspected defect such as a bug or regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants