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

Add client cert in insecure mode #10899

Merged
merged 8 commits into from
Apr 6, 2022
Merged

Add client cert in insecure mode #10899

merged 8 commits into from
Apr 6, 2022

Conversation

jakule
Copy link
Contributor

@jakule jakule commented Mar 7, 2022

Currently Database Access TLS insecure mode doesn't include client certificate when connecting to a database. This change adds client certificate to requests in insecure mode.

Closes #10803

@jakule jakule marked this pull request as ready for review March 8, 2022 02:03
@github-actions github-actions bot added the database-access Database access related issues and PRs label Mar 8, 2022
@github-actions github-actions bot requested review from codingllama and hatched March 8, 2022 02:03
@jakule jakule force-pushed the jakule/insecure-db-cert branch from 9f88244 to a86ea74 Compare March 8, 2022 02:04
@jakule jakule requested a review from r0mant March 8, 2022 02:04
Copy link
Contributor

@codingllama codingllama left a comment

Choose a reason for hiding this comment

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

Please add a brief description to the PR, but otherwise looks alright.

@jakule jakule force-pushed the jakule/insecure-db-cert branch from a86ea74 to 10eef50 Compare March 17, 2022 16:05
@jakule jakule enabled auto-merge (squash) March 23, 2022 20:03
@jakule jakule disabled auto-merge March 23, 2022 21:36
Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

I think there's still a bug here. Basically, it looks like getTLSConfigInsecure now tries to append client cert in all scenarios, not just when connecting to self-hosted databases.

@jakule jakule requested a review from r0mant April 4, 2022 19:51
@r0mant
Copy link
Collaborator

r0mant commented Apr 5, 2022

Let's backport to v8 and v9.

@jakule jakule enabled auto-merge (squash) April 6, 2022 00:06
@jakule jakule merged commit 8b00134 into master Apr 6, 2022
@jakule jakule deleted the jakule/insecure-db-cert branch April 6, 2022 00:28
jakule added a commit that referenced this pull request Apr 6, 2022
Add client certificate to DB insecure mode.
jakule added a commit that referenced this pull request Apr 6, 2022
Add client certificate to DB insecure mode.
jakule added a commit that referenced this pull request Apr 8, 2022
Add client certificate to DB insecure mode.

Backport of #10899
jakule added a commit that referenced this pull request Apr 11, 2022
Add client certificate to DB insecure mode.

Backport of #10899
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-required database-access Database access related issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Database TLS "insecure" mode does not generate client certificate
5 participants