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

fix racy use of tls configs #41114

Merged
merged 1 commit into from
May 1, 2024
Merged

Conversation

fspmarshall
Copy link
Contributor

Fixes a race condition introduced in #41077.

The client tls config generator was being passed references to the same TLS config instance that would then subsequently be configured to call back into the client TLS config generator to get client configs. This introduced a race because assignment of the GetConfigForClient field happens after the generator has been started:

generator, err := auth.NewClientTLSConfigGenerator(auth.ClientTLSConfigGeneratorConfig{
	TLS:                  tlsConfig, // background goroutine started that here that will read 'tslConfig'
	// ...
})

tlsConfig.GetConfigForClient = generator.GetConfigForClient // write occurs here

The fix for this issue is to clone the root config before passing it to the generator.

@fspmarshall fspmarshall added backport/branch/v14 no-changelog Indicates that a PR does not require a changelog entry backport/branch/v15 labels May 1, 2024
@fspmarshall fspmarshall requested a review from GavinFrazar May 1, 2024 19:15
@fspmarshall fspmarshall enabled auto-merge May 1, 2024 19:15
@fspmarshall fspmarshall added this pull request to the merge queue May 1, 2024
Merged via the queue into master with commit 2684949 May 1, 2024
41 checks passed
@fspmarshall fspmarshall deleted the fspmarshall/fix-tls-config-race branch May 1, 2024 19:56
@public-teleport-github-review-bot

@fspmarshall See the table below for backport results.

Branch Result
branch/v14 Create PR
branch/v15 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v14 backport/branch/v15 no-changelog Indicates that a PR does not require a changelog entry size/sm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants