Opened 9 years ago

Last modified 9 years ago

#13385 closed enhancement

Remove OpenSSL dependency from Sage — at Version 30

Reported by: kini Owned by: tbd
Priority: major Milestone: sage-5.4
Component: packages: standard Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #13121, #13384, #13373 Stopgaps:

Status badges

Description (last modified by jdemeyer)

Task

  • no longer ship pyOpenSSL with sagenb (this will be taken care of in #13121)
  • no longer require OpenSSL dev headers in prereq

Rationale

notebook(secure=True) is rarely used, and is not even really that desirable to use, except for people setting up multiuser Sage servers, which is a small percentage of Sage users. Therefore we will require users who want to use notebook(secure=True) to perform the additional step of installing pyOpenSSL into Sage's Python. This allows us to get rid of our sort of problematic dependencies on OpenSSL.

Related: #13392 (Remove GNUTLS-related packages)


Apply:

Copy http://sage.math.washington.edu/home/palmieri/SPKG/prereq-1.1.tar.gz to spkg/base.

Still to come: new sagenb spkg (at #13121).

Change History (34)

comment:1 Changed 9 years ago by novoselt

It would be nice to have a clear description of what someone has to do to make secure=True work and if someone has not done it - there should be a clear error message directing to these instructions...

comment:2 Changed 9 years ago by novoselt

It also would be a bit annoying to perform some extra installation on every newly built version of Sage. Is it possible to have instead some system dependencies and an environment variable that will force Sage to add pyOpenSSL automatically during build?

comment:3 follow-up: Changed 9 years ago by kini

You can just install the SPKG from #11384 after building Sage. Is that sufficient?

comment:4 Changed 9 years ago by kini

Sorry, I mean #13384. Also you'd need to have OpenSSL and its dev headers installed on the system if you wanted to use notebook(secure=True).

comment:5 in reply to: ↑ 3 Changed 9 years ago by novoselt

Replying to kini:

You can just install the SPKG from #11384 after building Sage. Is that sufficient?

So far

sage -i openssl

was sufficient (in 5.2 and later), but if this could be done automatically during build, it would be awesome.

comment:6 Changed 9 years ago by kini

Personally I don't think it's worth adding another environment variable to the long list we already have, just to avoid a small percentage of users having to run sage -i openssl and sage -i pyopenssl after make. Furthermore this will set a new precedent. Currently we have no environment variables which control whether a certain package is installed or not, except SAGE_INSTALL_GCC which is kind of a special case because we use GCC in Sage regardless of whether it's installed via SPKG or used from the system. Also, no environment variable causes the Sage build process to need network connectivity, as this suggestion would (unless you also want to make pyOpenSSL a standard shipped package).

IMHO installing an SPKG is not a big deal, it's one line and the average user doesn't need to know about it because they are not running a multi-user notebook server.

comment:7 Changed 9 years ago by vbraun

Please no new environment variables. We could have an extra makefile target

ssl: all
    ./sage -i openssl
    ./sage -i pyOpenSSL

comment:8 Changed 9 years ago by jhpalmieri

So what needs doing here?

Root repo:

  • remove the spkgs, modify deps and spkg/install accordingly
  • modify prereq so it no longer checks for openssl (i.e., undo some of the changes at #13329)
  • in README.txt, document what needs to be done to use the secure notebook
  • possibly add a new target to the Makefile

Sage library:

  • in the installation guide, document what needs to be done to use the secure notebook

Sagenb:

  • make it work without openssl

Anything else?

Last edited 9 years ago by jhpalmieri (previous) (diff)

comment:9 Changed 9 years ago by kini

In the sage library there is some code that does something with GNUTLS. It appears to be legacy code in devel/sage/sage/server/notebook . So I guess one more task is to see if anything breaks after removing the SPKGs.

comment:10 Changed 9 years ago by kini

(That includes upgrading extremely old notebook data, I guess.)

comment:11 Changed 9 years ago by kini

Here's a patch for the root repo. Did I miss anything in deps or install?

comment:12 Changed 9 years ago by jhpalmieri

The changes look okay at first glance. I'll try a build (on a system with openssl headers) to see how it goes. Perhaps you should mention make ssl in your changes to README.txt? And add a comment to the Makefile briefly describing the purpose of that target?

Last edited 9 years ago by jhpalmieri (previous) (diff)

comment:13 Changed 9 years ago by jhpalmieri

Oh, and in README.txt and Makefile, you should point out that sage -i openssl, make ssl, etc., require internet access.

comment:14 Changed 9 years ago by jhpalmieri

When I built with the root repo patch, all tests passed, because the old notebook directory has a file nodoctest.py. The appropriate files (like server/notebook/gnutls_socket_ssl.py) say things like

# This file is part of the OLD Sage notebook and is NOT actively developed,
# maintained, or supported.  As of Sage v4.1.2, all notebook development has
# moved to the separate Sage Notebook project

at the top, so I think it's okay to completely break this by removing gnutls.

Changed 9 years ago by jhpalmieri

comment:15 Changed 9 years ago by jhpalmieri

Here are some documentation patches, one for the root repo, one for the Sage library.

comment:16 Changed 9 years ago by jhpalmieri

Here's a patch for the prereq tarball, and here's a new prereq tarball.

Last edited 9 years ago by jhpalmieri (previous) (diff)

Changed 9 years ago by jhpalmieri

for prereq package; for review only

comment:17 Changed 9 years ago by jhpalmieri

  • Description modified (diff)

Changed 9 years ago by kini

apply to $SAGE_ROOT

comment:18 Changed 9 years ago by kini

Updated the root patches to account for the decapitalization of the pyOpenSSL SPKG name (see comments on #13384).

comment:19 Changed 9 years ago by kini

  • Dependencies changed from #13121 to #13121, #13384

comment:20 Changed 9 years ago by kini

sagenb fix is at https://github.com/sagemath/sagenb/pull/95 if someone wants to review it.

comment:21 Changed 9 years ago by kini

Looking at sagenb's README.rst, I notice that besides HTTPS notebook login, OpenSSL is also used by OpenID authentication, this time through Python's bundled ssl module rather than through pyOpenSSL.

This means that if a user wanted to use OpenID authentication on their server, they'd have to install OpenSSL dev headers on their system (or install the optional openssl SPKG) and then rebuild Python, which is a bit involved. This should also be included in the "ssl" makefile target, I guess. Preferably the "ssl" makefile target shouldn't build Python twice.

comment:22 Changed 9 years ago by jhpalmieri

So the "ssl" target should install openssl, then build Sage, then install pyopenssl. Fortunately, the openssl spkg has very few requirements: patch should be enough, I think (and patch is necessary on Solaris, at least: the system's patch is brain-dead).

So if the ssl makefile target should include openssl, perhaps it should run a script to download the spkg and then maybe modify spkg/standard/deps, adding openssl in the appropriate place (after patch). Or maybe, given the change at #13373, you could add three (?) new targets:

  • "patch", which does ./sage -i patch
  • "openssl", which has patch as a prerequisite and does ./sage -i openssl
  • "sslbuild", which has openssl as a prerequisite and just builds Sage.

Then "ssl" would have "sslbuild" as a dependency and would do ./sage -i pyopenssl. This all seems kind of clunky, especially since we should only install openssl if necessary.

Other ideas:

  • force the user to read README.txt and install openssl manually (./sage -i patch; ./sage -i openssl), and then make ssl will take care of everything else, by building Sage and installing pyopenssl.
  • include a script to check whether openssl is present, and run it as part of make ssl. If openssl is not present, exit and tell the user to install it.

(Wouldn't it be nice if Sage used configure? Then the user could do ./configure --ssl=True; make and the script would figure out whether openssl was necessary, and also install pyopenssl at the end.)

comment:23 Changed 9 years ago by kini

Another complication is that make ssl (or whatever it's called) should rebuild Python if it has already been built but without an ssl module.

I'm inclined to just make the user do this stuff and detail what needs to be done in the README. I'm also a bit wary of automating the installation of OpenSSL because that seems dangerously close to "bundling" it (though IANAL of course).

comment:24 Changed 9 years ago by jhpalmieri

Note the variable SAGE_UPGRADING. You should be able to rebuild Python (./sage -f python), and set this variable to "yes" and rebuild Python's dependencies by doing make build (is the syntax SAGE_UPGRADING=yes make build?). This will take a while, of course.

By they way, how should users tell if they have openssl installed? Can we put some helpful information about this in the documentation?

comment:25 Changed 9 years ago by jhpalmieri

  • Description modified (diff)

Here's some documentation for README.txt (the "v2" patch). If it's okay, we can add something similar to the installation guide.

Changed 9 years ago by jhpalmieri

for prereq package; for review only

comment:26 Changed 9 years ago by kini

  • Dependencies changed from #13121, #13384 to #13121, #13384, #13373

Yup, that looks fine to me!

comment:27 Changed 9 years ago by jdemeyer

It seems to me that "remove GNUTLS-related packages" and "make sagenb work without OpenSSL" are two independent things. So I think we should have two tickets, unless there is a good reason to connect these two issues.

comment:28 Changed 9 years ago by jdemeyer

  • Description modified (diff)
  • Summary changed from Remove TLS/SSL-related packages to Remove OpenSSL dependency from Sage

comment:29 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:30 Changed 9 years ago by jdemeyer

  • Description modified (diff)
Note: See TracTickets for help on using tickets.