Skip to content

Support HSM for Windows Desktop and database AD access#55429

Merged
zmb3 merged 17 commits intomasterfrom
probakowski/ad-hsm-main
Jul 17, 2025
Merged

Support HSM for Windows Desktop and database AD access#55429
zmb3 merged 17 commits intomasterfrom
probakowski/ad-hsm-main

Conversation

@probakowski
Copy link
Copy Markdown
Contributor

@probakowski probakowski commented Jun 4, 2025

This change implements RFD 210, supporting HMS for Windows Desktop and Database access with Active Directory.
Empty CRLs (valid for 10 years) are generated during each new TLS key pair creation and all existing cert authorities are updated during startup to also have one.

changelog: Fix certificate revocation failures in Active Directory environments when Teleport is using HSM-backed key material.

@probakowski probakowski requested a review from zmb3 June 4, 2025 17:58
@github-actions github-actions bot requested review from boxofrad and nklaassen June 4, 2025 17:58
@zmb3 zmb3 changed the title Support HMS for Windows Desktop and database AD access Support HSM for Windows Desktop and database AD access Jun 4, 2025
Comment thread integrations/event-handler/go.sum
Comment thread lib/auth/auth.go
Comment thread lib/auth/db.go Outdated
Comment thread lib/auth/db.go Outdated
Comment thread lib/auth/keystore/manager.go
Comment thread lib/auth/keystore/manager.go Outdated
Comment thread lib/tlsca/ca.go
Comment thread lib/winpki/certificate_authority.go Outdated
Comment thread lib/winpki/certificate_authority.go
Copy link
Copy Markdown
Contributor

@nklaassen nklaassen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Base automatically changed from probakowski/ad-hsm-proto to master June 10, 2025 21:21
Copy link
Copy Markdown
Contributor

@boxofrad boxofrad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@probakowski probakowski force-pushed the probakowski/ad-hsm-main branch from 54ae4e2 to 47e7504 Compare June 18, 2025 10:10
@probakowski probakowski force-pushed the probakowski/ad-hsm-main branch from 303f786 to 5bad0e8 Compare June 26, 2025 12:03
@probakowski probakowski requested a review from zmb3 June 26, 2025 12:07
@probakowski probakowski force-pushed the probakowski/ad-hsm-main branch 2 times, most recently from 6630e28 to a810a1e Compare June 27, 2025 12:43
@probakowski probakowski force-pushed the probakowski/ad-hsm-main branch from a810a1e to 63b346e Compare June 27, 2025 15:23
@zmb3 zmb3 enabled auto-merge July 17, 2025 19:40
@zmb3 zmb3 added this pull request to the merge queue Jul 17, 2025
Merged via the queue into master with commit 68cd267 Jul 17, 2025
48 checks passed
@zmb3 zmb3 deleted the probakowski/ad-hsm-main branch July 17, 2025 20:02
@backport-bot-workflows
Copy link
Copy Markdown
Contributor

@probakowski See the table below for backport results.

Branch Result
branch/v16 Failed
branch/v17 Failed
branch/v18 Failed

github-merge-queue bot pushed a commit that referenced this pull request Jul 23, 2025
* Proto changes for supporting HSM in Windows Desktop and Database access w/AD (#55428)

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Support HSM for Windows Desktop and database AD access (#55429)

* Support out parameter for `tctl auth crl` command (#55430)

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

---------

Co-authored-by: Przemko Robakowski <przemko.robakowski@goteleport.com>
github-merge-queue bot pushed a commit that referenced this pull request Jul 23, 2025
* Proto changes for supporting HSM in Windows Desktop and Database access w/AD

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Support HSM for Windows Desktop and database AD access (#55429)

* Support out parameter for `tctl auth crl` command (#55430)

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

---------

Co-authored-by: Przemko Robakowski <przemko.robakowski@goteleport.com>
zmb3 added a commit that referenced this pull request Aug 12, 2025
In #55429 we added an empty certificate revocation list to the
TLS keypairs in the certificate_authority backend resource.

The intent was that the CRL would be signed by the keypair it is
assocaited with, but the code mistakenly used a function that would
find _any_ suitable keypair, which could create CRLS signed by the
wrong key.

This commit corrects both the initial CRL generation as well as
any existing CRLs that have invalid signatures.
@zmb3 zmb3 mentioned this pull request Aug 12, 2025
zmb3 added a commit that referenced this pull request Aug 12, 2025
In #55429 we added an empty certificate revocation list to the
TLS keypairs in the certificate_authority backend resource.

The intent was that the CRL would be signed by the keypair it is
assocaited with, but the code mistakenly used a function that would
find _any_ suitable keypair, which could create CRLS signed by the
wrong key.

This commit corrects both the initial CRL generation as well as
any existing CRLs that have invalid signatures.
github-merge-queue bot pushed a commit that referenced this pull request Aug 14, 2025
* Fix CRLs in the backend

In #55429 we added an empty certificate revocation list to the
TLS keypairs in the certificate_authority backend resource.

The intent was that the CRL would be signed by the keypair it is
assocaited with, but the code mistakenly used a function that would
find _any_ suitable keypair, which could create CRLS signed by the
wrong key.

This commit corrects both the initial CRL generation as well as
any existing CRLs that have invalid signatures.

* Add some extra warnings re: CRL generation

In clusters with multiple active signers (like those using HSMs or
KMS for private keys), the only safe way to get a CRL is to use
the one that's already present in the certificate_authority resource.

Generating a CRL with GenerateCertAuthorityCRL will give you a valid
CRL, but you have no control over which certificate signs it, which
is likely to cause revocation failures later on.

* Rename GenerateCRLForCA

In most cases, we're looking up an existing CRL, not generating one.

* Improve DELETE IN comments while the context is fresh
backport-bot-workflows bot pushed a commit that referenced this pull request Aug 14, 2025
In #55429 we added an empty certificate revocation list to the
TLS keypairs in the certificate_authority backend resource.

The intent was that the CRL would be signed by the keypair it is
assocaited with, but the code mistakenly used a function that would
find _any_ suitable keypair, which could create CRLS signed by the
wrong key.

This commit corrects both the initial CRL generation as well as
any existing CRLs that have invalid signatures.
backport-bot-workflows bot pushed a commit that referenced this pull request Aug 14, 2025
In #55429 we added an empty certificate revocation list to the
TLS keypairs in the certificate_authority backend resource.

The intent was that the CRL would be signed by the keypair it is
assocaited with, but the code mistakenly used a function that would
find _any_ suitable keypair, which could create CRLS signed by the
wrong key.

This commit corrects both the initial CRL generation as well as
any existing CRLs that have invalid signatures.
github-merge-queue bot pushed a commit that referenced this pull request Aug 14, 2025
* Fix CRLs in the backend

In #55429 we added an empty certificate revocation list to the
TLS keypairs in the certificate_authority backend resource.

The intent was that the CRL would be signed by the keypair it is
assocaited with, but the code mistakenly used a function that would
find _any_ suitable keypair, which could create CRLS signed by the
wrong key.

This commit corrects both the initial CRL generation as well as
any existing CRLs that have invalid signatures.

* Add some extra warnings re: CRL generation

In clusters with multiple active signers (like those using HSMs or
KMS for private keys), the only safe way to get a CRL is to use
the one that's already present in the certificate_authority resource.

Generating a CRL with GenerateCertAuthorityCRL will give you a valid
CRL, but you have no control over which certificate signs it, which
is likely to cause revocation failures later on.

* Rename GenerateCRLForCA

In most cases, we're looking up an existing CRL, not generating one.

* Improve DELETE IN comments while the context is fresh
github-merge-queue bot pushed a commit that referenced this pull request Aug 14, 2025
* Fix CRLs in the backend

In #55429 we added an empty certificate revocation list to the
TLS keypairs in the certificate_authority backend resource.

The intent was that the CRL would be signed by the keypair it is
assocaited with, but the code mistakenly used a function that would
find _any_ suitable keypair, which could create CRLS signed by the
wrong key.

This commit corrects both the initial CRL generation as well as
any existing CRLs that have invalid signatures.

* Add some extra warnings re: CRL generation

In clusters with multiple active signers (like those using HSMs or
KMS for private keys), the only safe way to get a CRL is to use
the one that's already present in the certificate_authority resource.

Generating a CRL with GenerateCertAuthorityCRL will give you a valid
CRL, but you have no control over which certificate signs it, which
is likely to cause revocation failures later on.

* Rename GenerateCRLForCA

In most cases, we're looking up an existing CRL, not generating one.

* Improve DELETE IN comments while the context is fresh
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants