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

http: Add support for enabling automatic sending of SSL client certificate #3293

Conversation

pascalmuller
Copy link

@pascalmuller pascalmuller commented Jun 23, 2021

This adds support for a new http.sslAutoClientCert config value.

In cURL 7.77 or later the schannel backend does not automatically send
client certificates from the Windows Certificate Store anymore.

This config value is only used if http.sslBackend is set to "schannel",
and can be used to opt in to the old behavior and force cURL to send
client certificates.

This fixes #3292

@pascalmuller pascalmuller force-pushed the http-support-automatically-sending-client-certificate branch from dd63e64 to 3de1d46 Compare June 23, 2021 20:41
@pascalmuller
Copy link
Author

I have some doubts, including:

  • Should this behavior be opt-in (as I have currently implemented it), or opt-out?
  • I'm unsure about variable naming
  • I'm unsure if the documentation I added is good enough to be understandable by anyone.

I apologize if this PR is not up to the standards that are expected, but please be patient with me. This is my first attempt to contribute, so I might need help to get this Pull Request up to par and ready to merge.

I really hope we can get the bug fixed soon.

http.c Outdated Show resolved Hide resolved
http.c Outdated Show resolved Hide resolved
@dscho
Copy link
Member

dscho commented Jun 23, 2021

Thank you so much for working on this. I always like it when users reporting an issue take the opportunity to become contributors and bring about the change they'd like to see. It makes maintaining a lot more fun that way.

Apart from the comments I added to the diff, could I ask for the following changes in the commit message?

  1. we typically continue in lower-case after the ":" part, and try to keep the oneline shorter; I would suggest using the oneline "http: optionally send SSL client certificate"
  2. Could you write out the URL of the ticket in the "This fixes" line? I plan on shepherding this patch to the Git project, where the ticket number on its own would be misleading.

I have some doubts, including:

  • Should this behavior be opt-in (as I have currently implemented it), or opt-out?

Since cURL made it opt-in (for privacy reasons), I think you did the right thing.

  • I'm unsure about variable naming

As I said, I think we can drop the schannel_ infix. Who knows whether cURL will use this flag for other TLS backends, too?

  • I'm unsure if the documentation I added is good enough to be understandable by anyone.

Personally, I find it very readable. Thank you!

I apologize if this PR is not up to the standards that are expected, but please be patient with me. This is my first attempt to contribute, so I might need help to get this Pull Request up to par and ready to merge.

This PR is a really good start, and I believe that we'll need only one round of updates to get it in. You did pretty well, in particular as a first-time contributor!

@pascalmuller pascalmuller force-pushed the http-support-automatically-sending-client-certificate branch from 3de1d46 to 46c25af Compare June 24, 2021 07:14
@pascalmuller
Copy link
Author

I made the requested changes. Thanks for your time and your suggestions so far; I appreciate it.

@dscho
Copy link
Member

dscho commented Jun 24, 2021

I made the requested changes. Thanks for your time and your suggestions so far; I appreciate it.

I do not see the changes in https://github.com/git-for-windows/git/compare/3de1d46786a2326f65ab2e155c6e9762fac6a8e4..46c25af92ec6f9271691d5447c28d2d523358b81. Maybe you called git commit --amend without staging the changes first? Happens to me all the time...

This adds support for a new http.sslAutoClientCert config value.

In cURL 7.77 or later the schannel backend does not automatically send
client certificates from the Windows Certificate Store anymore.

This config value is only used if http.sslBackend is set to "schannel",
and can be used to opt in to the old behavior and force cURL to send
client certificates.

This fixes git-for-windows#3292

Signed-off-by: Pascal Muller <pascalmuller@gmail.com>
@pascalmuller pascalmuller force-pushed the http-support-automatically-sending-client-certificate branch from 46c25af to c129342 Compare June 24, 2021 12:29
@pascalmuller
Copy link
Author

I made the requested changes. Thanks for your time and your suggestions so far; I appreciate it.

I do not see the changes in https://github.com/git-for-windows/git/compare/3de1d46786a2326f65ab2e155c6e9762fac6a8e4..46c25af92ec6f9271691d5447c28d2d523358b81. Maybe you called git commit --amend without staging the changes first? Happens to me all the time...

You're right. Stupid mistake on my part. Should be fixed now.

@dscho
Copy link
Member

dscho commented Jun 24, 2021

You're right. Stupid mistake on my part. Should be fixed now.

Nah, it is a common mistake, no need to be hard on yourself. Thank you for taking care of it so swiftly!

Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

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

Excellent! I'll wait for the CI to pass, and then merge. That should trigger the "Git artifacts" Azure Pipeline, and once that run finished, an Azure Release Pipeline will upload the new snapshot).

@dscho dscho added this to the Next release milestone Jun 24, 2021
@dscho dscho merged commit c0919c9 into git-for-windows:main Jun 24, 2021
git-for-windows-ci pushed a commit that referenced this pull request Jun 24, 2021
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Jun 24, 2021
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Jun 24, 2021
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
@dscho
Copy link
Member

dscho commented Jun 24, 2021

Thank you so much @pascalmuller!

dscho added a commit that referenced this pull request Jun 25, 2021
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Jun 26, 2021
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Jun 26, 2021
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Jun 26, 2021
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Jun 26, 2021
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
@pascalmuller pascalmuller deleted the http-support-automatically-sending-client-certificate branch June 28, 2021 11:49
dscho added a commit that referenced this pull request Jun 29, 2021
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Jun 29, 2021
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
dscho added a commit that referenced this pull request Jul 2, 2021
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
dscho added a commit that referenced this pull request Jul 2, 2021
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Jul 2, 2021
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Jul 2, 2021
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
dscho added a commit that referenced this pull request Oct 11, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
dscho added a commit that referenced this pull request Oct 20, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
dscho added a commit that referenced this pull request Oct 20, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
dscho added a commit that referenced this pull request Oct 21, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Oct 21, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Oct 21, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Oct 22, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Oct 22, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Oct 22, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Oct 22, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Oct 22, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Oct 22, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
dscho added a commit that referenced this pull request Oct 23, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Oct 25, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Oct 25, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Oct 25, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Oct 30, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Nov 1, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Nov 6, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
dscho added a commit that referenced this pull request Nov 22, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
dscho added a commit that referenced this pull request Nov 22, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
dscho added a commit that referenced this pull request Nov 22, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
dscho added a commit that referenced this pull request Nov 22, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
dscho added a commit that referenced this pull request Nov 22, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Nov 25, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Nov 25, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Nov 25, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Nov 25, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
dscho added a commit to dscho/git that referenced this pull request Nov 25, 2024
…t-automatically-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
git-for-windows-ci pushed a commit that referenced this pull request Nov 25, 2024
…-sending-client-certificate

http: Add support for enabling automatic sending of SSL client certificate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Git 2.32 cannot access git repositories secured using client certificates when using cURL schannel backend
2 participants