Add support for Windows CA certificate store by hlandau · Pull Request #18070 · openssl/openssl

hlandau · 2022-04-08T12:46:58Z

This PR adds support for the Windows CA certificate store. It does this by introducing a new store in the default provider which recognises the capi:// URI scheme. The URI used is just capi://, there are no arguments. This new store also becomes the default store when capistore is enabled at compile time (i.e., on Windows).

A build option is added (capistore) which can be disabled if this functionality is not desired.

This fixes #18020 but also implements my proposal in #18068 and is built on top of that fix. I can split these out into separate PRs if desired, but the former would preferably be merged before the latter.

hlandau · 2022-04-08T14:44:15Z

Manpage.

petrovr · 2022-04-08T17:24:22Z

I'm not convinced that proposed solution should change existing methods.
Implementation of Store2 API must be enough.

petrovr · 2022-04-08T17:36:42Z

About implementation - it is good to see a provider based sample. I have a number of engine based implementation including own by_store x509 lookup for openssl before 3.
Also I expect not so generic function names, i.e. with store as part of name. Perhaps current names are acceptable as static functions.
I could not see "error", "expect" and "find" methods. May be "expect" is superseded by "set...param...". If think that other two are required.

petrovr · 2022-04-08T17:42:02Z

P.S.I think that store name could be extracted from URI.

include/internal/common.h

kaduk · 2022-04-11T23:56:59Z

Please register any new URI schemes with IANA: https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml -- provisional registrations are given on a "first come first served" policy.

openssl-machine · 2022-05-12T00:07:14Z

This PR is in a state where it requires action by @openssl/otc but the last update was 30 days ago

beldmit · 2022-05-12T15:38:45Z

I strongly support this idea. If I remember correctly, windows has more than one storage (at least, system and user's). Will this code work with it?

hlandau · 2022-05-13T11:21:28Z

@beldmit It uses the system root CA store by default. Since this code uses an URI to specify the Windows store there is the possibility we could allow other stores to be specified.

beldmit · 2022-05-13T11:26:53Z

How do we override this default URI in run-time?

hlandau · 2022-05-13T11:35:04Z

The same way the URI is currently specified, e.g. via an environment variable, see this PR. I could add support for specifying different stores.

beldmit · 2022-05-13T11:48:13Z

It looks better to me if it is overrideable via API.

hlandau · 2022-05-13T11:52:10Z

That is also feasible.

levitte · 2022-05-19T07:56:04Z

As mentioned here, capi: isn't a good scheme name: #18068 (comment)

crypto/x509/by_store.c

levitte · 2022-05-23T09:03:36Z

As mentioned here, capi: isn't a good scheme name: #18068 (comment)

Suggestion regarding the name: winstore:...
non-registration alternative: org.openssl.winstore: (same as we did with engines, in #13570)

levitte · 2022-06-16T09:08:58Z

As mentioned here, capi: isn't a good scheme name: #18068 (comment)

Suggestion regarding the name: winstore:... non-registration alternative: org.openssl.winstore: (same as we did with engines, in #13570)

Fair warning: I'm going to be quite insistent on this

hlandau · 2022-06-29T17:23:48Z

Rebased. Updated to implement option C. URI scheme renamed to winstore://.

levitte

Looking good at this point.

openssl-machine · 2022-09-14T13:00:38Z

This pull request is ready to merge

hlandau · 2022-09-14T13:11:15Z

Merged to master.

Fixes #18068. Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #18070)

Fixes #18020. Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #18070)

Fixes openssl#18068. Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#18070)

Fixes openssl#18020. Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#18070)

Fixes openssl#18068. Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#18070)

Fixes openssl#18020. Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#18070)

…e store)

…tificate store)

Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #21190)

Complements: dfdbc11 "Partially revert openssl#18070 (Add support for Windows CA certificate store)" Signed-off-by: Norbert Pocs <norbertp@openssl.org>

Complements: dfdbc11 "Partially revert #18070 (Add support for Windows CA certificate store)" Signed-off-by: Norbert Pocs <norbertp@openssl.org> Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Saša Nedvědický <sashan@openssl.org> MergeDate: Mon Mar 16 11:24:06 2026 (Merged from #30390) (cherry picked from commit 00dcd45)

Complements: dfdbc11 "Partially revert #18070 (Add support for Windows CA certificate store)" Signed-off-by: Norbert Pocs <norbertp@openssl.org> Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Saša Nedvědický <sashan@openssl.org> MergeDate: Mon Mar 16 11:24:06 2026 (Merged from #30390)

Complements: dfdbc11 "Partially revert #18070 (Add support for Windows CA certificate store)" Signed-off-by: Norbert Pocs <norbertp@openssl.org> Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Saša Nedvědický <sashan@openssl.org> MergeDate: Mon Mar 16 11:24:06 2026 (Merged from #30390) (cherry picked from commit 00dcd45)

hlandau added branch: master Applies to master branch approval: review pending This pull request needs review by a committer approval: otc review pending triaged: feature The issue/pr requests/adds a feature labels Apr 8, 2022

hlandau requested review from levitte and mattcaswell April 8, 2022 12:46

hlandau self-assigned this Apr 8, 2022

github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Apr 8, 2022

hlandau force-pushed the fix-18020 branch from 8c1ffe4 to c079361 Compare April 8, 2022 14:44

t8m reviewed Apr 11, 2022

View reviewed changes

include/internal/common.h Outdated Show resolved Hide resolved

t8m requested changes May 19, 2022

View reviewed changes

crypto/x509/by_store.c Outdated Show resolved Hide resolved

hlandau mentioned this pull request Jun 14, 2022

Windows-ROOT as truststore #18020

Closed

hlandau force-pushed the fix-18020 branch from fbeda9d to e378ea2 Compare June 29, 2022 17:23

levitte approved these changes Jul 6, 2022

View reviewed changes

hlandau force-pushed the fix-18020 branch from 63b2f34 to 273e41c Compare September 13, 2022 10:28

levitte approved these changes Sep 13, 2022

View reviewed changes

t8m approved these changes Sep 13, 2022

View reviewed changes

t8m added approval: done This pull request has the required number of approvals and removed approval: done This pull request has the required number of approvals labels Sep 13, 2022

openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Sep 14, 2022

hlandau closed this Sep 14, 2022

openssl-machine pushed a commit that referenced this pull request Sep 14, 2022

Fix URI handling in SSL_CERT_DIR/introduce SSL_CERT_URI env

021859b

Fixes #18068. Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #18070)

openssl-machine pushed a commit that referenced this pull request Sep 14, 2022

Add support for loading root CAs from Windows crypto API

606e042

Fixes #18020. Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #18070)

vszakats mentioned this pull request Apr 28, 2023

curl: add --ca-native and --proxy-ca-native curl/curl#11049

Closed

This was referenced May 26, 2023

Optimise some X509_STORE related locking #20952

Closed

Unexpected logic for the default store lookup URI #21068

Closed

hlandau added a commit to hlandau/openssl that referenced this pull request Jun 13, 2023

Partially revert openssl#18070 (Add support for Windows CA certificat…

47b06fd

…e store)

hlandau added a commit to hlandau/openssl that referenced this pull request Jun 14, 2023

fixup! Partially revert openssl#18070 (Add support for Windows CA cer…

aa40a4f

…tificate store)

mattcaswell mentioned this pull request Feb 2, 2024

Support for new Apple trust API #23460

Open

jogme added a commit to jogme/openssl that referenced this pull request Mar 12, 2026

docs: Fix SSL_CERT_DIR env var

93a85f1

Complements: dfdbc11 "Partially revert openssl#18070 (Add support for Windows CA certificate store)" Signed-off-by: Norbert Pocs <norbertp@openssl.org>

Uh oh!

Conversation

hlandau commented Apr 8, 2022

Uh oh!

hlandau commented Apr 8, 2022

Uh oh!

petrovr commented Apr 8, 2022

Uh oh!

petrovr commented Apr 8, 2022

Uh oh!

petrovr commented Apr 8, 2022

Uh oh!

Uh oh!

kaduk commented Apr 11, 2022

Uh oh!

openssl-machine commented May 12, 2022

Uh oh!

beldmit commented May 12, 2022

Uh oh!

hlandau commented May 13, 2022

Uh oh!

beldmit commented May 13, 2022

Uh oh!

hlandau commented May 13, 2022

Uh oh!

beldmit commented May 13, 2022

Uh oh!

hlandau commented May 13, 2022

Uh oh!

levitte commented May 19, 2022

Uh oh!

Uh oh!

levitte commented May 23, 2022

Uh oh!

levitte commented Jun 16, 2022

Uh oh!

hlandau commented Jun 29, 2022

Uh oh!

levitte left a comment

Choose a reason for hiding this comment

Uh oh!

openssl-machine commented Sep 14, 2022

Uh oh!

hlandau commented Sep 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants