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

feat: Support multiple active CAs in Web exports #51301

Merged
merged 7 commits into from
Jan 23, 2025

Conversation

codingllama
Copy link
Contributor

@codingllama codingllama commented Jan 21, 2025

Add support for exporting multiple active CAs via the "format=zip" param.

  • If a single active CA exists the behavior is the same as before
  • If multiple active CAs exist the error message is changed to refer to the "format=zip" param
  • Any number of active CAs may be exported using "format=zip"

Error before this PR:

GET /auth/export?type=tls-user

"export returned 2 authorities, expected exactly one"

After this PR:

GET /auth/export?type=tls-user

"found 2 authorities to export, use format=zip to export all"

If format=zip is supplied (for example, "/auth/export?type=tls-user&format=zip") then a zip file called "Teleport_CA.zip" is returned as an attachment. The file contains various "ca$i.cer" files, one for each exported CA, in whatever format it would have as a single-file export.

Follow up from #51189. Sibling PR to #51298.

#35444

Changelog: Added support for multiple active CAs in the /auth/export endpoint

@codingllama codingllama added the no-changelog Indicates that a PR does not require a changelog entry label Jan 21, 2025
@codingllama
Copy link
Contributor Author

@codingllama
Copy link
Contributor Author

The PR is smaller than it seems, as there's a big move from the larger apiserver{,_test}.go files to ca_export{,_test}.go files. Because of that I suggest reviewing commit-by-commit.

  • The move commit is a plain move without changes (768da03)
  • The test refactor commit doesn't change any functionality (1cfebdc)

@codingllama
Copy link
Contributor Author

Takes inspiration from Gavin's #35754, although it has my own spin on it.

@zmb3
Copy link
Collaborator

zmb3 commented Jan 22, 2025

Do we have a way to put multiple DER-encoded certs in the zip file for windows, or will the zip always use PEM?

The use case that led to this issue was exporting the windows CA when using HSMs. In order to import the CA on the windows side it needs to be DER.

@codingllama
Copy link
Contributor Author

Do we have a way to put multiple DER-encoded certs in the zip file for windows, or will the zip always use PEM?

The use case that led to this issue was exporting the windows CA when using HSMs. In order to import the CA on the windows side it needs to be DER.

I used type=tls-user as an example, but it works the same for type=windows - you'd get the same zip as a result, but the underlying .cer files are DER and not PEM.

@codingllama
Copy link
Contributor Author

Friendly ping @mvbrock @probakowski @GavinFrazar.

Base automatically changed from codingllama/export-all-funcs to master January 22, 2025 15:44
@codingllama codingllama force-pushed the codingllama/export-all-web branch from 806d6b6 to e2f08d1 Compare January 22, 2025 16:44
@codingllama
Copy link
Contributor Author

Rebased onto master, no changes.

lib/web/ca_export.go Outdated Show resolved Hide resolved
lib/web/ca_export.go Outdated Show resolved Hide resolved
@codingllama
Copy link
Contributor Author

Thanks, Zac!

@codingllama codingllama force-pushed the codingllama/export-all-web branch from ccec73a to 9e1b309 Compare January 22, 2025 19:55
@codingllama
Copy link
Contributor Author

Rebased onto master so I can pull #51298 (the tctl PR) and add a final cleanup commit - 9e1b309. PTAL.

@codingllama
Copy link
Contributor Author

Friendly ping @mvbrock @probakowski @GavinFrazar ?

@codingllama codingllama added this pull request to the merge queue Jan 23, 2025
Merged via the queue into master with commit 2e89fd7 Jan 23, 2025
41 checks passed
@codingllama codingllama deleted the codingllama/export-all-web branch January 23, 2025 14:54
@public-teleport-github-review-bot

@codingllama See the table below for backport results.

Branch Result
branch/v15 Failed
branch/v16 Failed
branch/v17 Failed

codingllama added a commit that referenced this pull request Jan 23, 2025
* Move /auth/export code to own file

* Implement "/auth/export?format=zip"

* Refactor existing tests

* Test format=zip

* Fix comment

* Use bytes.NewReader

* Remove lib/client.ExportAuthorities
codingllama added a commit that referenced this pull request Jan 23, 2025
* Move /auth/export code to own file

* Implement "/auth/export?format=zip"

* Refactor existing tests

* Test format=zip

* Fix comment

* Use bytes.NewReader

* Remove lib/client.ExportAuthorities
codingllama added a commit that referenced this pull request Jan 23, 2025
* Move /auth/export code to own file

* Implement "/auth/export?format=zip"

* Refactor existing tests

* Test format=zip

* Fix comment

* Use bytes.NewReader

* Remove lib/client.ExportAuthorities
codingllama added a commit that referenced this pull request Jan 23, 2025
* Move /auth/export code to own file

* Implement "/auth/export?format=zip"

* Refactor existing tests

* Test format=zip

* Fix comment

* Use bytes.NewReader

* Remove lib/client.ExportAuthorities
github-merge-queue bot pushed a commit that referenced this pull request Jan 23, 2025
* Move /auth/export code to own file

* Implement "/auth/export?format=zip"

* Refactor existing tests

* Test format=zip

* Fix comment

* Use bytes.NewReader

* Remove lib/client.ExportAuthorities
github-merge-queue bot pushed a commit that referenced this pull request Jan 24, 2025
* Move /auth/export code to own file

* Implement "/auth/export?format=zip"

* Refactor existing tests

* Test format=zip

* Fix comment

* Use bytes.NewReader

* Remove lib/client.ExportAuthorities
github-merge-queue bot pushed a commit that referenced this pull request Jan 24, 2025
* Move /auth/export code to own file

* Implement "/auth/export?format=zip"

* Refactor existing tests

* Test format=zip

* Fix comment

* Use bytes.NewReader

* Remove lib/client.ExportAuthorities
zmb3 added a commit that referenced this pull request Jan 27, 2025
Now that #51301 is merged, we have the ability to get all of the
active user CA certificates.
zmb3 added a commit that referenced this pull request Jan 29, 2025
Now that #51301 is merged, we have the ability to get all of the
active user CA certificates.
zmb3 added a commit that referenced this pull request Jan 29, 2025
Now that #51301 is merged, we have the ability to get all of the
active user CA certificates.
github-merge-queue bot pushed a commit that referenced this pull request Jan 29, 2025
Now that #51301 is merged, we have the ability to get all of the
active user CA certificates.
github-actions bot pushed a commit that referenced this pull request Jan 29, 2025
Now that #51301 is merged, we have the ability to get all of the
active user CA certificates.
zmb3 added a commit that referenced this pull request Jan 30, 2025
Now that #51301 is merged, we have the ability to get all of the
active user CA certificates.
github-merge-queue bot pushed a commit that referenced this pull request Jan 30, 2025
* docs: update CA import instructions for HSMs (#51530)

Now that #51301 is merged, we have the ability to get all of the
active user CA certificates.

* docs: add a dedicated section to desktop LDAP discovery (#51515)

Closes #51485
github-merge-queue bot pushed a commit that referenced this pull request Jan 30, 2025
Now that #51301 is merged, we have the ability to get all of the
active user CA certificates.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants