sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.4k stars 473 forks source link

sage-download-file: Fix certificate problems with https downloads from upstream_url when sage-system-python is XCode's python3 on macOS #29418

Closed mkoeppe closed 4 years ago

mkoeppe commented 4 years ago

26351 added an optional upstream_url field to build/pkgs/*/checksums.ini. It streamlines the procedure for testing upgrade tickets: Developers or automatic testing facilities can pass an extra flag -o to sage-spkg to allow downloading from upstream rather than from Sage mirrors (where the updated ticket will be made available later only).

Many upstream package URLs use the https protocol - in contrast to the http protocol used when downloading from the Sage mirrors. The downloading is done via build/bin/sage-download-file, which uses the urllib module. It supports the https protocol.

However, SSL certificate problems are common on test systems. For example, if one uses XCode's python3 as the system python, then urllib does not automatically uses the standard system certificates. (This is apparently a known issue -- which is considered "wontfix" by Apple as reported here: https://github.com/HandBrake/HandBrake/issues/2216#issuecomment-527114519)

We add an option --no-check-certificate to sage-download-file, disabling certificate checking (https://stackoverflow.com/questions/36600583/python-3-urllib-ignore-ssl-certificate-verification).

Developers can set this option using the environment variable SAGE_DOWNLOAD_FILE_OPTIONS when installing packages (either by make or by using sage -i).

We note that even with SSL certificates disabled, there is still cryptographic protection because of the checksums recorded in checksums.ini.


Other possible workarounds considered:

As of #29090 (sage-system-python fixup) prefers /usr/bin/python3 over /usr/bin/python, leading to:

  File "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.7/lib/python3.7/ssl.py", line 1117, in do_handshake
    self._sslobj.do_handshake()
OSError: [Errno socket error] [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1056)

(see https://github.com/mkoeppe/sage/runs/538432620)

CC: @vbraun @dimpase @kiwifb @jhpalmieri @videlec @fchapoton @kliem

Component: build

Author: Matthias Koeppe

Branch: 90ea00b

Reviewer: Jonathan Kliem

Issue created by migration from https://trac.sagemath.org/ticket/29418

mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -2,3 +2,10 @@

 As seen with `local-homebrew-macos-minimal` in https://github.com/mkoeppe/sage/runs/538432620

+As of #29090 (sage-system-python fixup) prefers `/usr/bin/python3` over `/usr/bin/python`, leading to:
+
+```
+  File "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.7/lib/python3.7/ssl.py", line 1117, in do_handshake
+    self._sslobj.do_handshake()
+OSError: [Errno socket error] [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1056)
+```
mkoeppe commented 4 years ago
comment:2

https://stackoverflow.com/questions/36600583/python-3-urllib-ignore-ssl-certificate-verification

mkoeppe commented 4 years ago
comment:3

https://stackoverflow.com/questions/57630314/ssl-certificate-verify-failed-error-with-python3-on-macos-10-15

mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -1,6 +1,13 @@
 This is for downloading from an https `upstream_url` when the host is poorly configured.

-As seen with `local-homebrew-macos-minimal` in https://github.com/mkoeppe/sage/runs/538432620
+In particular this happens with XCode's `python3` as the system python. This is apparently a known issue -- which is considered "wontfix" by Apple as reported here:
+https://github.com/HandBrake/HandBrake/issues/2216#issuecomment-527114519
+
+We should work around this by either:
+- switching from using urllib directly to the requests library
+- passing cafile=..., capath=... to urllib, perhaps coming from an environment variable
+- disabling certificate checking (https://stackoverflow.com/questions/36600583/python-3-urllib-ignore-ssl-certificate-verification), perhaps triggered by an option --no-check-certificate to sage-download-file
+- using python instead of python3 on macOS as system python

 As of #29090 (sage-system-python fixup) prefers `/usr/bin/python3` over `/usr/bin/python`, leading to:

@@ -9,3 +16,6 @@
     self._sslobj.do_handshake()
 OSError: [Errno socket error] [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1056)

+(see https://github.com/mkoeppe/sage/runs/538432620) + +

mkoeppe commented 4 years ago

Branch: u/mkoeppe/sage_download_file__fix_certificate_problems_with_https_downloads_on_macos_python3

dimpase commented 4 years ago
comment:6

that's why I suggested not to bother with MacOS/Xcode's python(3). Can't one get Python3 from python.org in MacOS by running a script?


New commits:

bd24641build/bin/sage-download-file: Add option --no-check-certificate
dimpase commented 4 years ago

Commit: bd24641

mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -1,12 +1,15 @@
-This is for downloading from an https `upstream_url` when the host is poorly configured.
+We add an option `--no-check-certificate` to `sage-download-file`, disabling certificate checking (https://stackoverflow.com/questions/36600583/python-3-urllib-ignore-ssl-certificate-verification).
+
+Users can set this option using the environment variable SAGE_DOWNLOAD_FILE_OPTIONS when using `sage -i`. This is for enabling them to download from an https `upstream_url` when the host is poorly configured.

 In particular this happens with XCode's `python3` as the system python. This is apparently a known issue -- which is considered "wontfix" by Apple as reported here:
 https://github.com/HandBrake/HandBrake/issues/2216#issuecomment-527114519

-We should work around this by either:
+---
+
+Other possible workarounds considered:
 - switching from using urllib directly to the requests library
 - passing cafile=..., capath=... to urllib, perhaps coming from an environment variable
-- disabling certificate checking (https://stackoverflow.com/questions/36600583/python-3-urllib-ignore-ssl-certificate-verification), perhaps triggered by an option --no-check-certificate to sage-download-file
 - using python instead of python3 on macOS as system python

 As of #29090 (sage-system-python fixup) prefers `/usr/bin/python3` over `/usr/bin/python`, leading to:
mkoeppe commented 4 years ago

Changed commit from bd24641 to none

mkoeppe commented 4 years ago

Changed branch from u/mkoeppe/sage_download_file__fix_certificate_problems_with_https_downloads_on_macos_python3 to none

mkoeppe commented 4 years ago

Author: Matthias Koeppe

mkoeppe commented 4 years ago
comment:8

Replying to @dimpase:

that's why I suggested not to bother with MacOS/Xcode's python(3). Can't one get Python3 from python.org in MacOS by running a script?

Please... the idea is to make Sage installation easier, not harder.

mkoeppe commented 4 years ago
comment:9

One often runs into these certificate errors also with random Linux images when I run them on Docker. So this workaround is valuable for the automatic testing in any case.

mkoeppe commented 4 years ago

Branch: u/mkoeppe/sage_download_file__fix_certificate_problems_with_https_downloads_on_macos_python3

mkoeppe commented 4 years ago

Commit: e143939

mkoeppe commented 4 years ago
comment:11

Testing this at https://github.com/mkoeppe/sage/actions/runs/65346154


New commits:

bd24641build/bin/sage-download-file: Add option --no-check-certificate
e143939build/bin/sage-spkg: Append to SAGE_DOWNLOAD_FILE_OPTIONS instead of overwriting it
dimpase commented 4 years ago
comment:12

Replying to @mkoeppe:

Replying to @dimpase:

that's why I suggested not to bother with MacOS/Xcode's python(3). Can't one get Python3 from python.org in MacOS by running a script?

Please... the idea is to make Sage installation easier, not harder.

well, it seems that using system's python would make a slighly cryptographically broken Sagemath, one way or another.

mkoeppe commented 4 years ago
comment:13

Replying to @dimpase:

well, it seems that using system's python would make a slighly cryptographically broken Sagemath, one way or another.

That's not a regression from the current status on macOS, see #29291 for example

mkoeppe commented 4 years ago
comment:14

And, of course, the present ticket is about sage-system-python, which is only used for bootstrapping and build. Unrelated to the use of a system python3 for venv (#27824), which has a completely separate test for what it accepts.

dimpase commented 4 years ago
comment:15

oy gevalt, pythons everywhere!

mkoeppe commented 4 years ago
comment:17

Replying to @dimpase:

oy gevalt, pythons everywhere!

This will be addressed in #45678 "switch Sage from Python to Julia"

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from e143939 to 90ea00b

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

cf27321build/bin/sage-download-file: Add option --no-check-certificate
90ea00bbuild/bin/sage-spkg: Append to SAGE_DOWNLOAD_FILE_OPTIONS instead of overwriting it
mkoeppe commented 4 years ago
comment:19

Rebased, needs review

jhpalmieri commented 4 years ago
comment:23

Okay, I'm really confused, or my Sage installation is broken, or both. If I use ./sage -i beautifulsoup4 with a build using the system Python on OS X, it fails, but not because of the issues here. Instead, I see this:

make[2]: *** No rule to make target `/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/SYSTEM/sage-9.1.beta9/local/var/lib/sage/installed/beautifulsoup4-none', needed by `all-sage'.  Stop.

If instead I run make beautifulsoup4, it succeeds. When I ran ./sage -i tornado it didn't seem to even try to install it, but ./sage -f tornado worked.

So: why does ./sage -i ... not work? And what is the point of this ticket: what problem does it solve?

mkoeppe commented 4 years ago
comment:24

sage -i was changed in #29113 -- unrelated to the present ticket

mkoeppe commented 4 years ago
comment:25

Could you try if the problem goes away if you run bootstrap?

mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -1,6 +1,6 @@
 We add an option `--no-check-certificate` to `sage-download-file`, disabling certificate checking (https://stackoverflow.com/questions/36600583/python-3-urllib-ignore-ssl-certificate-verification).

-Users can set this option using the environment variable SAGE_DOWNLOAD_FILE_OPTIONS when using `sage -i`. This is for enabling them to download from an https `upstream_url` when the host is poorly configured.
+Users can set this option using the environment variable SAGE_DOWNLOAD_FILE_OPTIONS when installing packages (either by `make` or by using `sage -i`). This is for enabling them to download from an https `upstream_url` when the host is poorly configured.

 In particular this happens with XCode's `python3` as the system python. This is apparently a known issue -- which is considered "wontfix" by Apple as reported here:
 https://github.com/HandBrake/HandBrake/issues/2216#issuecomment-527114519
jhpalmieri commented 4 years ago
comment:27

If I run bootstrap, then running ./sage -i benzene acts like ./sage -i tornado: it doesn't install. There is a message

running ./configure '--enable-beautifulsoup4' '--enable-biopython'  '--enable-benzene'

and then if I search my terminal window for benzene, the only other hit is

benzene-20130630:                            does not support check for system package; optional, use "./configure --enable-benzene" to install

I don't think I tested #29113 well enough when I was reviewing it.

jhpalmieri commented 4 years ago
comment:28

This fixes it for me:

diff --git a/src/bin/sage b/src/bin/sage
index 10acddcd96..6ccff3789c 100755
--- a/src/bin/sage
+++ b/src/bin/sage
@@ -404,7 +404,7 @@ if [ "$1" = '-i' ]; then
         #   'CC=gcc -Wall' '--enable-e_antic'
         CONFIG_CMD="./configure $(./config.status --config) $ENABLE_ARGS"
         echo >&2 "running $CONFIG_CMD"
-        bash -c "$CONFIG_CMD" && $MAKE all-build
+        bash -c "$CONFIG_CMD" && $MAKE $PACKAGES
     else
         echo "New packages may have been installed."
         echo "Re-running configure and make in case any dependent packages need updating."

(or maybe it could be $MAKE all-build $PACKAGES?)

By the way, I still don't understand the goal of this ticket.

mkoeppe commented 4 years ago
comment:29

Let's please take the issues with sage -i to the new ticket #29481. Thanks for catching this.

mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -1,9 +1,15 @@
+#26351 added an optional `upstream_url` field to `build/pkgs/*/checksums.ini`. It streamlines the procedure for testing upgrade tickets: Developers or automatic testing facilities can pass an extra flag `-o` to `sage-spkg` to allow downloading from upstream rather than from Sage mirrors (where the updated ticket will be made available later only).
+
+Many upstream package URLs use the `https` protocol - in contrast to the `http` protocol used when downloading from the Sage mirrors. The downloading is done via `build/bin/sage-download-file`, which uses the `urllib` module. It supports the https protocol. 
+
+However, SSL certificate problems are common on test systems. For example, if one uses XCode's `python3` as the system python, then `urllib` does not automatically uses the standard system certificates. (This is apparently a known issue -- which is considered "wontfix" by Apple as reported here:
+https://github.com/HandBrake/HandBrake/issues/2216#issuecomment-527114519)
+
 We add an option `--no-check-certificate` to `sage-download-file`, disabling certificate checking (https://stackoverflow.com/questions/36600583/python-3-urllib-ignore-ssl-certificate-verification).

-Users can set this option using the environment variable SAGE_DOWNLOAD_FILE_OPTIONS when installing packages (either by `make` or by using `sage -i`). This is for enabling them to download from an https `upstream_url` when the host is poorly configured.
+Developers can set this option using the environment variable SAGE_DOWNLOAD_FILE_OPTIONS when installing packages (either by `make` or by using `sage -i`).

-In particular this happens with XCode's `python3` as the system python. This is apparently a known issue -- which is considered "wontfix" by Apple as reported here:
-https://github.com/HandBrake/HandBrake/issues/2216#issuecomment-527114519
+We note that even with SSL certificates disabled, there is still cryptographic protection because of the checksums recorded in `checksums.ini`.

 ---
mkoeppe commented 4 years ago
comment:31

Replying to @jhpalmieri:

By the way, I still don't understand the goal of this ticket.

Reworked the ticket description, please take a look

jhpalmieri commented 4 years ago
comment:32

Replying to @mkoeppe:

Replying to @jhpalmieri:

By the way, I still don't understand the goal of this ticket.

Reworked the ticket description, please take a look

Sorry, I wasn't clear enough. What system configuration and commands lead to the problem in the description? I can't reproduce the problem, so I can't tell if the solution here works.

kliem commented 4 years ago
comment:33

From what I understand this is a problem that only occurs when one has python3 from Xcode. This doesn't ship certify and never will (at least Apple doesn't have any intentions). So it is really difficult to establish an ssl connection with that.

Do you have successfully tested this ticket?

Do I understand correctly that the problem occurring could be theoretically fixed by manually downloading the correct packages into the upstream folder? (Yes this is not a good approach for testing environments.)

Also this flag would never be needed for a normal user as long as the sage mirrors don't use ssl?

mkoeppe commented 4 years ago
comment:34

Replying to @kliem:

From what I understand this is a problem that only occurs when one has python3 from Xcode. This doesn't ship certify and never will (at least Apple doesn't have any intentions). So it is really difficult to establish an ssl connection with that.

That's a correct description for macOS. But the problem also appears on Linux distributions if one does not install ca-certificates, or those are outdated.

Do you have successfully tested this ticket?

Yes, I have been using this as part of #29417 since end of March.

Do I understand correctly that the problem occurring could be theoretically fixed by manually downloading the correct packages into the upstream folder? (Yes this is not a good approach for testing environments.)

That's correct for the macOS tests using tox local. For the Linux tests with tox docker note that /upstream is in .gitignore (therefore in .dockerignore) and is therefore not provided to the container.

Also this flag would never be needed for a normal user as long as the sage mirrors don't use ssl?

That's right, normal users would not use the upstream_url at all; it is a feature for developers only.

kliem commented 4 years ago

Reviewer: Jonathan Kliem

kliem commented 4 years ago
comment:35

LGTM.

Btw, I have been using this ticket to test my tickets, e.g. here: https://github.com/kliem/sage-test-27122/actions/runs/72102779

mkoeppe commented 4 years ago
comment:36

Thank you!

vbraun commented 4 years ago

Changed branch from u/mkoeppe/sage_download_file__fix_certificate_problems_with_https_downloads_on_macos_python3 to 90ea00b

mkoeppe commented 3 years ago
comment:38

Follow up: #30950

mkoeppe commented 3 years ago

Changed commit from 90ea00b to none