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

Remove needlessly complex key generation scheme #12113

Merged
merged 13 commits into from
Apr 25, 2022
Merged

Conversation

xacrimon
Copy link
Contributor

@xacrimon xacrimon commented Apr 20, 2022

While looking into TEL-Q122-11 which recommends we encrypt keys in the keygen function, I discovered that the suggested encryption is insecure and the entire encryption scheme is deprecated as of Go 1.16. To prevent confusion, I decided to remove the pass parameter from the keygen functions since it isn't actually used anywhere. Upon doing that, I discovered a host of wierd things like remote keygen HTTP APIs and overused indirection that I can't see they serve any purpose. This PR cleans all that up and uses native.GenerateKeyPair() for all keypair generation in Teleport.

@github-actions github-actions bot added application-access database-access Database access related issues and PRs kubernetes tctl tctl - Teleport admin tool labels Apr 20, 2022
@github-actions github-actions bot requested review from hatched and smallinsky April 20, 2022 14:29
@zmb3
Copy link
Collaborator

zmb3 commented Apr 20, 2022

Should we leverage

func (k *Keygen) precomputeKeys() {
more?

This basically always has a keypair ready for you, waiting for you to pull it from a channel. I recall cloud using it in a few places to decrease latency.

@xacrimon
Copy link
Contributor Author

xacrimon commented Apr 20, 2022

@zmb3 Rewritten the system quite a bit, it now keeps a global key cache which should decrease latency application wide, including tests.

lib/auth/native/native.go Outdated Show resolved Hide resolved
@xacrimon xacrimon requested a review from zmb3 April 20, 2022 21:35
@xacrimon xacrimon enabled auto-merge (squash) April 25, 2022 09:02
@xacrimon xacrimon merged commit 9911640 into master Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
application-access database-access Database access related issues and PRs kubernetes regression tctl tctl - Teleport admin tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants