Skip to content

export all tls keypairs with tctl or webapi#35754

Closed
GavinFrazar wants to merge 9 commits intomasterfrom
gavinfrazar/tctl-auth-export-all-keypairs
Closed

export all tls keypairs with tctl or webapi#35754
GavinFrazar wants to merge 9 commits intomasterfrom
gavinfrazar/tctl-auth-export-all-keypairs

Conversation

@GavinFrazar
Copy link
Copy Markdown
Contributor

@GavinFrazar GavinFrazar commented Dec 14, 2023

fixes #35444

changelog: All trusted TLS certs for a given CA are now exported when using tctl auth export --type=<ca> or curl https://<proxy>/webapi/auth/export?type=<ca>, rather than only one active cert.

If you tctl auth export multiple certs/keys in der format, you must provide an --out/-o=<filename> flag, because concatenated DER doesn't make sense.
Similar to the tctl auth sign command, the path given as --out is used to build the full name of the output file(s), i.e.:
tctl auth export --out=/path/to/filename results in these files:

  • /path/to/filename-0.cer
  • /path/to/filename-1.cer

Whereas when there is only one file to output, it just adds the file extension like: /path/to/filename.cer.

With the webapi, when there are multiple DER format certs to export, it creates a compressed archive attachment.
For db-der and tls-user-der, that will be a .tar.gz file.
For windows, it's a .zip file.

The archive contents will be named as:

  • teleport-ca-0.cer
  • teleport-ca-1.cer
  • ...etc...

Browsers get a normal download popup.
Curl needs -o or -OJ when downloading the DER attachments since they're binary.

@GavinFrazar GavinFrazar force-pushed the gavinfrazar/tctl-auth-export-all-keypairs branch from 4ad8f38 to 87ce21c Compare December 19, 2023 01:04
@GavinFrazar GavinFrazar force-pushed the gavinfrazar/tctl-auth-export-all-keypairs branch from 87ce21c to 97cb6b4 Compare December 19, 2023 01:22
@GavinFrazar GavinFrazar requested review from greedy52 and zmb3 December 19, 2023 01:22
@GavinFrazar GavinFrazar marked this pull request as ready for review December 19, 2023 01:23
@github-actions github-actions Bot added size/md tctl tctl - Teleport admin tool labels Dec 19, 2023
@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented Dec 19, 2023

For windows, it's a .zip file.

I think we should check whether using PKCS#7 / PKCS#12 as a container makes more sense here. I have a feeling this might allow you import the cert(s) on the Windows side in a single operation rather than unzipping and repeating the same operation for each file in the archive. If this does in fact work, it's a much friendlier UX and makes it less likely that customers do the wrong thing during/after a CA rotation.

@GavinFrazar
Copy link
Copy Markdown
Contributor Author

GavinFrazar commented Dec 19, 2023

For windows, it's a .zip file.

I think we should check whether using PKCS#7 / PKCS#12 as a container makes more sense here. I have a feeling this might allow you import the cert(s) on the Windows side in a single operation rather than unzipping and repeating the same operation for each file in the archive. If this does in fact work, it's a much friendlier UX and makes it less likely that customers do the wrong thing during/after a CA rotation.

I tried to use pkcs7 initially, but certutil does not work with multiple certs in a pkcs7 file, nor does our teleport-windows-auth-setup.exe installer, which was bizarre, because we do have windows.PKCS_7_ASN_ENCODING set:
https://github.com/gravitational/teleport-windows-auth/blob/main/installer/main.go#L192

Windows does work with right-click > import if it's a .p7b file however 🤷‍♂️ .

Edit: specifically, certutil -dspublish cas.p7b doesn't work with multiple certs in the .p7b file. It will accept a single cert in a .p7b file though.

Copy link
Copy Markdown
Contributor

@Tener Tener left a comment

Choose a reason for hiding this comment

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

The fact that we switch format depending on the number of keypairs available is not much of a problem for a human operator, but it will complicate things for scripting. There is also a backward compatibility issue: this is very much a breaking change for anyone expecting to get a single result. Can we actually introduce it in a patch release?

Perhaps a flag/http param to get multiple results would be an acceptable alternative result.

Also, note: there are some unit test failures.

@GavinFrazar
Copy link
Copy Markdown
Contributor Author

Also, note: there are some unit test failures.

oops, caused by my hasty patch to appease the linter error last night. fixed.

The fact that we switch format depending on the number of keypairs available is not much of a problem for a human operator, but it will complicate things for scripting. There is also a backward compatibility issue: this is very much a breaking change for anyone expecting to get a single result. Can we actually introduce it in a patch release?

Perhaps a flag/http param to get multiple results would be an acceptable alternative result.

I think we'd need two flags to handle both scriptability and backwards compat then.
For scripting: ?format=[file | zip | tarball] (default: file - original behavior)
For backwards compat: ?keypairs=[active | all] (default: active - original behavior)

With default flags, if there are multiple active keypairs to be exported in DER format, it can either return an error as it does originally, or we can output a pkcs7 file. The pkcs7 file won't work with certutil -dspublish on windows though.

Do you guys have any further suggestions? I'm torn here. On one hand backwards compat/scripting support, on the other: exporting a CA should export all its certs so that rotations work and HSM keys work...

@Tener
Copy link
Copy Markdown
Contributor

Tener commented Dec 20, 2023

This is a sensitive part of the API, so I'm very hesitant about introducing a breaking change without a major release. Perhaps we can highlight this in the docs, and change the defaults in the next major release?

@GavinFrazar
Copy link
Copy Markdown
Contributor Author

ok i'm going to rework this - I'll ping reviewers when it's ready for review

@codingllama
Copy link
Copy Markdown
Contributor

Superseded by #51298 and #51301, so I'll take the liberty of closing it.

@GavinFrazar GavinFrazar deleted the gavinfrazar/tctl-auth-export-all-keypairs branch January 23, 2025 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/md tctl tctl - Teleport admin tool

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tctl and webapi auth export commands fail with multiple active CAs

4 participants