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

Fail to access https://github.com/* if portable version is unpacked in a network share. #3266

Closed
1 task done
omirochn opened this issue Jun 10, 2021 · 6 comments · Fixed by git-for-windows/MINGW-packages#51
Milestone

Comments

@omirochn
Copy link

omirochn commented Jun 10, 2021

  • I was not able to find an open or closed issue matching what I'm seeing

Setup

  • Which version of Git for Windows are you using? Is it 32-bit or 64-bit?
$ git --version --build-options

git version 2.32.0.windows.1
cpu: x86_64
built from commit: 4c204998d0e156d13d81abe1d1963051b1418fc0
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
  • Which version of Windows are you running? Vista, 7, 8, 10? Is it 32-bit or 64-bit?
$ cmd.exe /c ver

Microsoft Windows [Version 6.2.9200]

  • What options did you set as part of the installation? Or did you choose the
    defaults?
# One of the following:
> type "C:\Program Files\Git\etc\install-options.txt"
> type "C:\Program Files (x86)\Git\etc\install-options.txt"
> type "%USERPROFILE%\AppData\Local\Programs\Git\etc\install-options.txt"
$ cat /etc/install-options.txt

Unpacked a portable package in a network share.
Found no install-options.txt in any of the places 
  • Any other interesting things about your environment that might be related
    to the issue you're seeing?

Unpacked into a network share. The same package unpacked in local drive works fine.

Details

  • Which terminal/shell are you running Git from? e.g Bash/CMD/PowerShell/other

CMD, bash

y:\user\tmp\PortableGit-2.32.0-64-bit\bin\git clone https://github.com/git/git
  • What did you expect to occur after running these commands?

Success cloning

  • What actually happened instead?

Cloning into 'git'...
fatal: unable to access 'https://github.com/git/git/': error setting certificate verify locations: CAfile: /fileserver/share/user/tmp/PortableGit-2.32.0-64-
bit/mingw64/ssl/certs/ca-bundle.crt CApath: none

  • If the problem was occurring with a specific repository, can you provide the
    URL to that repository to help us with testing?

Any repo with https:// URL

@rimrul
Copy link
Member

rimrul commented Jun 11, 2021

CAfile: /fileserver/share/...

This looks broken.

What does git config http.sslcainfo tell you?

Probably something like \\fileserver\share\…, right?

That probably gets converted to //fileserver/share/... at some point and then further mangled into /fileserver/share/.... Not ideal.

@omirochn
Copy link
Author

It tells nothing (empty line):

c:\Temp>y:\user\tmp\PortableGit-2.32.0-64-bit\bin\git config http.sslcainfo

c:\Temp>

@dscho
Copy link
Member

dscho commented Jun 14, 2021

I can reproduce (using my WSL setup to emulate a network drive, via \\wsl$\Ubuntu-18.04).

A good workaround seems to be:

y:\user\tmp\PortableGit-2.32.0-64-bit\bin\git config --system http.sslbackend schannel

@dscho
Copy link
Member

dscho commented Jun 14, 2021

fatal: unable to access 'https://github.com/git/git/': error setting certificate verify locations: CAfile: /fileserver/share/user/tmp/PortableGit-2.32.0-64-bit/mingw64/ssl/certs/ca-bundle.crt CApath: none

The error message "error setting certificate verify locations" does not actually come from Git, but from cURL: https://github.com/curl/curl/blob/8fa0a298c65548615a86a042b2661d637c532699/lib/vtls/openssl.c#L3087-L3090

However, we do not set the location of the certificate bundle explicitly, it is set implicitly by cURL itself. Or, to be more precise, by our version of it. And the problem seems to be that the first double-slash is turned into a single slash. Which makes me think that the culprit is here: https://github.com/git-for-windows/MINGW-packages/blob/dd2a97114e7f01807dd33fd163156c28e5a3e35a/mingw-w64-curl/0001-Make-cURL-relocatable.patch#L158-L163

I could imagine that the problem would be solved by replacing

  path_p = path;

with

  path_p = path + !!*path; /* skip first character, if any, to handle UNC paths correctly */

@omirochn
Copy link
Author

The workaround works!
Thanks!!!

@dscho
Copy link
Member

dscho commented Jun 14, 2021

I am testing a fix right now. It looks essentially like this:

diff --git a/mingw-w64-curl/0001-Make-cURL-relocatable.patch b/mingw-w64-curl/0001-Make-cURL-relocatable.patch
index 3c5bfd935..71b82a2e8 100644
--- a/mingw-w64-curl/0001-Make-cURL-relocatable.patch
+++ b/mingw-w64-curl/0001-Make-cURL-relocatable.patch
@@ -26,10 +26,10 @@ Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
  configure.ac         |   1 +
  lib/Makefile.inc     |   2 +
  lib/curl_config.h.in |   3 +
- lib/pathtools.c      | 556 +++++++++++++++++++++++++++++++++++++++++++
+ lib/pathtools.c      | 562 +++++++++++++++++++++++++++++++++++++++++++
  lib/pathtools.h      |  57 +++++
  lib/url.c            |  26 +-
- 6 files changed, 644 insertions(+), 1 deletion(-)
+ 6 files changed, 650 insertions(+), 1 deletion(-)
  create mode 100644 lib/pathtools.c
  create mode 100644 lib/pathtools.h
 
@@ -81,10 +81,10 @@ index 89a1d195a..8c8305754 100644
  
 diff --git a/lib/pathtools.c b/lib/pathtools.c
 new file mode 100644
-index 000000000..c09458e95
+index 000000000..53d0db00b
 --- /dev/null
 +++ b/lib/pathtools.c
-@@ -0,0 +1,556 @@
+@@ -0,0 +1,562 @@
 +/*
 +      .Some useful path tools.
 +        .ASCII only for now.
@@ -156,7 +156,7 @@ index 000000000..c09458e95
 +    *path_p = '/';
 +  }
 +  /* Replace any '//' with '/' */
-+  path_p = path;
++  path_p = path + !!*path; /* skip first character, if any, to handle UNC paths correctly */
 +  while ((path_p = strstr (path_p, "//")) != NULL)
 +  {
 +    memmove (path_p, path_p + 1, path_size--);
@@ -277,6 +277,12 @@ index 000000000..c09458e95
 +  char * result = path;
 +  char * result_p;
 +  char const ** toks;
++  if (path[0] == '/' && path[1] == '/') {
++    /* preserve UNC path */
++    path++;
++    in_size--;
++    result++;
++  }
 +  sanitise_path(result);
 +  result_p = result;
 +

dscho added a commit to dscho/MINGW-packages that referenced this issue Jun 14, 2021
The code to make cURL relocatable simplifies/normalizes paths, e.g.
reducing multiple forward slashes to a single one.

This is okay in general, but at the beginning of the path, a double
slash has a special meaning: it denotes a UNC path of the form
//<host>/<path>. It would therefore be a mistake to reduce those first
two slashes to a single one: otherwise it would refer to the absolute
path relative to the current directory's drive.

Let's preserve the double slashes.

This fixes git-for-windows/git#3266

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
lazka pushed a commit to msys2/MINGW-packages that referenced this issue Jun 18, 2021
The code to make cURL relocatable simplifies/normalizes paths, e.g.
reducing multiple forward slashes to a single one.

This is okay in general, but at the beginning of the path, a double
slash has a special meaning: it denotes a UNC path of the form
//<host>/<path>. It would therefore be a mistake to reduce those first
two slashes to a single one: otherwise it would refer to the absolute
path relative to the current directory's drive.

Let's preserve the double slashes.

Reported-in: git-for-windows/git#3266
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho added this to the Next release milestone Jun 22, 2021
dscho added a commit to git-for-windows/build-extra that referenced this issue Jun 25, 2021
Remote HTTPS repositories [could not be accessed
from within portable Git installed into a network
share](git-for-windows/git#3266). This [has
been fixed](git-for-windows/MINGW-packages#51).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
lazka pushed a commit to msys2/MINGW-packages that referenced this issue Jun 27, 2021
The code to make cURL relocatable simplifies/normalizes paths, e.g.
reducing multiple forward slashes to a single one.

This is okay in general, but at the beginning of the path, a double
slash has a special meaning: it denotes a UNC path of the form
//<host>/<path>. It would therefore be a mistake to reduce those first
two slashes to a single one: otherwise it would refer to the absolute
path relative to the current directory's drive.

Let's preserve the double slashes.

Reported-in: git-for-windows/git#3266
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
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 a pull request may close this issue.

3 participants