Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#22189 closed task (fixed)

Document SSL dependency, recommend OpenSSL

Reported by: charpent Owned by:
Priority: major Milestone: sage-7.6
Component: documentation Keywords:
Cc: Merged in:
Authors: Emmanuel Charpentier Reviewers: Julian Rüth, André Apitzsch, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 8af2ca2 (Commits) Commit:
Dependencies: #22058 Stopgaps:

Description

The motivation has been discussed numerous times, and has become a somewhat urgent matter now that R stopped providing its own implementation.

This ticket aims at documenting this long known dependency, recommend using OpenSSL system-wide and document the possibility of compiling against other libraries with reduced functionality.

it depends on #22058, to allow for successful build against OpenSSL>1.1 (see this report) : we cannot decently recommend a library if its newer version breaks Sage building...

This ticket does not aim to modify any code, only the documentation (README.md and the Sage Installation Guide).

Change History (32)

comment:1 Changed 3 years ago by charpent

  • Branch set to u/charpent/document_ssl_dependency__recommend_openssl

comment:2 follow-up: Changed 3 years ago by charpent

  • Commit set to cd6b74e7d9016e029380ba70bd67bd208f6fa2de

Initial proposition.

I noted a few inconsistencies/inecxactitudes while revising the text. I did not try to fix them : they require more information and belong to other tickets :

  • I'm surprised to see Python as a prequisite in $SAGE_ROOT/src/doc/en/installation/source.rst...
  • Are the instruction about moving the Sage tree still valid ? Somehow, I doubt it !
  • Is a system git a prerequisite (as $SAGE_ROOT/src/doc/en/installation/source.rst seem to imply) or not ? If so, having a git package is pointless...

comment:3 in reply to: ↑ 2 Changed 3 years ago by charpent

  • Status changed from new to needs_review

Forgot to add :

  1. The resulting Sage passes ptestlong with two failures :
    ----------------------------------------------------------------------
    sage -t --long --warn-long 119.0 src/sage/homology/simplicial_complex.py  # 1 doctest failed
    sage -t --long --warn-long 119.0 src/sage/rings/polynomial/multi_polynomial_ideal.py  # 1 doctest failed
    ----------------------------------------------------------------------
    

The first one is transient (test passes fine when run standalone), the second one is an already-known problem.

  1. Forgot to set needs_review

comment:4 follow-up: Changed 3 years ago by saraedum

  • Reviewers set to Julian Rüth

At the end it should now probably say "Sage 7.6". Feel free to set to positive review once this is fixed.

comment:5 Changed 3 years ago by git

  • Commit changed from cd6b74e7d9016e029380ba70bd67bd208f6fa2de to a37c08e12c28568985e1269ce25cfabbb3b26871

Branch pushed to git repo; I updated commit sha1. New commits:

a37c08eChanged the target Sage version, as suggested by reviewer.

comment:6 in reply to: ↑ 4 Changed 3 years ago by charpent

Replying to saraedum:

At the end it should now probably say "Sage 7.6".

Done.

Feel free to set to positive review once this is fixed.

I'd rather follow the basic requirement of no self-review : it's a basic safeguard against hubris, and, as all basic rules, it can have any value if and only if rigidly followed...

comment:7 Changed 3 years ago by saraedum

  • Status changed from needs_review to positive_review

comment:8 follow-up: Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

Author is missing...

comment:9 in reply to: ↑ 8 Changed 3 years ago by charpent

  • Authors set to Emmanuel Charpentier
  • Status changed from needs_work to needs_review

Replying to vbraun:

Author is missing...

Wups !

Done...

comment:10 follow-up: Changed 3 years ago by tscrim

Looks like you picked up a merge conflict with the next beta. (For future reference, missing author/review name can immediately be set back to positive review.)

comment:11 in reply to: ↑ 10 ; follow-up: Changed 3 years ago by charpent

  • Status changed from needs_review to positive_review

Replying to tscrim:

Looks like you picked up a merge conflict with the next beta.

What do you mean ?

(For future reference, missing author/review name can immediately be set back to positive review.)

Done. Thanks.

comment:12 in reply to: ↑ 11 Changed 3 years ago by tscrim

  • Status changed from positive_review to needs_work

Replying to charpent:

Replying to tscrim:

Looks like you picked up a merge conflict with the next beta.

What do you mean ?

The branch is red, so there is (almost certainly) a non-trivial with the latest beta (7.6.beta1).

comment:13 Changed 3 years ago by saraedum

  • Work issues set to merge conflicts

comment:14 Changed 3 years ago by git

  • Commit changed from a37c08e12c28568985e1269ce25cfabbb3b26871 to c884e41ac51bb660074bf48cc6cb6577e8003eb1

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

c884e41From Sage 7.6, depend on system SSL, recommend OpenSSL.

comment:15 follow-up: Changed 3 years ago by charpent

I squashed the two commits and forced a push. This should fix the merge problem.

Last edited 3 years ago by charpent (previous) (diff)

comment:16 in reply to: ↑ 15 Changed 3 years ago by charpent

Replying to charpent:

I squashed the two commits and forced a push. This should fix the merge problem.

No, it doesn't : there is still a merge conflict, requiring to manually pick the "right" versin of the last line of src/doc/en/installation/source.rst (i. e. what was fixed by the second commit). I do not understand why, and I'm stuck.

Last edited 3 years ago by charpent (previous) (diff)

comment:17 Changed 3 years ago by tscrim

  • Branch changed from u/charpent/document_ssl_dependency__recommend_openssl to u/tscrim/22189
  • Commit changed from c884e41ac51bb660074bf48cc6cb6577e8003eb1 to 2ee7e18926b13d977e05981546706a275635a887
  • Status changed from needs_work to positive_review
  • Work issues merge conflicts deleted

I fixed the (trivial) merge conflict with 7.6.beta1.


New commits:

2ee7e18Merge branch 'u/charpent/document_ssl_dependency__recommend_openssl' of git://trac.sagemath.org/sage into u/tscrim/22189
Last edited 3 years ago by tscrim (previous) (diff)

comment:18 follow-up: Changed 3 years ago by aapitzsch

  • Status changed from positive_review to needs_work

There is a typo in

Sage's ``-pip`` facility (used to install some Sage packages) is
disabled when Sage is compiled agaonst those libraries.

agaonst -> against

comment:19 Changed 3 years ago by charpent

  • Branch changed from u/tscrim/22189 to u/charpent/22189

comment:20 in reply to: ↑ 18 Changed 3 years ago by charpent

  • Commit changed from 2ee7e18926b13d977e05981546706a275635a887 to 8af2ca2ad19f764c8c46891b8b808b3cd0747b09
  • Status changed from needs_work to needs_review

Replying to aapitzsch:

There is a typo in

Sage's ``-pip`` facility (used to install some Sage packages) is
disabled when Sage is compiled agaonst those libraries.

agaonst -> against

Done. Fixed a couple of other typos.

Passes ptestlong. needs_review.


New commits:

8af2ca2fixed a couple of typos.

comment:21 Changed 3 years ago by tscrim

  • Reviewers changed from Julian Rüth to Julian Rüth, André Apitzsch, Travis Scrimshaw
  • Status changed from needs_review to positive_review

Latest changes LGTM.

comment:22 Changed 3 years ago by vbraun

  • Branch changed from u/charpent/22189 to 8af2ca2ad19f764c8c46891b8b808b3cd0747b09
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:23 follow-up: Changed 3 years ago by jhpalmieri

  • Commit 8af2ca2ad19f764c8c46891b8b808b3cd0747b09 deleted

What part of the Sage build will break without ssl? Shouldn't we check for the presence of ssl in the top-level configure script and bail out right away if it's missing, rather than let Sage build for an hour or two before failing?

comment:24 in reply to: ↑ 23 Changed 3 years ago by charpent

Replying to jhpalmieri:

What part of the Sage build will break without ssl?

  • R will fail to build without an https-capable SSL
  • Python won't fail overtly, but the resulting pip won't be able to install or list packages in https-accessed repositories : nowadays, that means all pip directories.
  • ISTR thython license mention this and warns about "reduced functionalities".

The python license recommends openssl ; my tests showed that GnuTLS is able to create a functional R (but not a functional pip). Hence the dependency on "an SSL implementation) and the recommendation of OpenSSL. I didn't test the nss library...

Shouldn't we check for the presence of ssl in the top-level configure script and bail out right away if it's missing, rather than let Sage build for an hour or two before failing?

Indeed. But I don't know how (yet) how to do it cleanly (it is easy to test for OpenSSL, not so much for either OpenSSL or GnuTLS...). If I can find or devise a relevant minimal test program, I'll propose to add this test in the master configure file. If you have ideas...

Hope this helps,

comment:25 follow-up: Changed 3 years ago by jhpalmieri

I would suggest using python -c 'import ssl' (using the system's Python), but this succeeds on my OS X box on which I just uninstalled Sage's version of openssl, in order to test #22252. I don't know why it succeeds, but in any case, it's not a good test.

comment:26 in reply to: ↑ 25 ; follow-up: Changed 3 years ago by charpent

Replying to jhpalmieri:

I would suggest using python -c 'import ssl' (using the system's Python), but this succeeds on my OS X box on which I just uninstalled Sage's version of openssl, in order to test #22252. I don't know why it succeeds,

Don't you have some SSL library installed systemwide ? OpenSSL is pretty ubiquitous nowadays (even on Debian base (not desktop !) systems, to the dismay of Jeroen Demeyer....)

but in any case, it's not a good test.

Indeed. What I need is a program testing a library common to (at least) OpenSSL and GnuTLS, and able to test the "https-worthiness" of a library. Not exactly a trivial request...

comment:27 in reply to: ↑ 26 Changed 3 years ago by jhpalmieri

Replying to charpent:

Replying to jhpalmieri:

I would suggest using python -c 'import ssl' (using the system's Python), but this succeeds on my OS X box on which I just uninstalled Sage's version of openssl, in order to test #22252. I don't know why it succeeds,

Don't you have some SSL library installed systemwide ? OpenSSL is pretty ubiquitous nowadays (even on Debian base (not desktop !) systems, to the dismay of Jeroen Demeyer....)

I apparently do, since the system Python picks up something. I can't actually find it myself, nor does Sage's Python when it builds, since import ssl fails for it.

comment:28 follow-up: Changed 3 years ago by jdemeyer

I don't understand why this ticket got positive review. Sage does not depend on OpenSSL, so why document that it does? It might be recommended, but certainly not required. See #22620 for a follow-up.

comment:29 follow-up: Changed 3 years ago by dimpase

interestingly, curl sources include vtls, (almost) an API to do (the client side of) TLS/SSL with different backends, e.g. DarwinSSL, OpenSSL (and a dozen others).

comment:30 in reply to: ↑ 29 ; follow-up: Changed 3 years ago by charpent

Replying to dimpase:

interestingly, curl sources include vtls, (almost) an API to do (the client side of) TLS/SSL with different backends, e.g. DarwinSSL, OpenSSL (and a dozen others).

Do you think that this might ofar a solution to Jeroen's conudrum ? And our Mac OS/X woes ?

comment:31 in reply to: ↑ 28 Changed 3 years ago by charpent

Replying to jdemeyer:

I don't understand why this ticket got positive review. Sage does not depend on OpenSSL, so why document that it does?

Because Python does. Because R needs curl, which does need (some) SSL to reach https repositories.

It might be recommended, but certainly not required.

Indeed. The current documentation requires some SSL library and recommends OpenSSL : not quite the same thing.

I also proved conclusively that :

  • GnuTLS allowed for the successfil compilation of a https-capable R an
  • that this GnuTLL library is insufficient for a functional pip.

See #22620 for a follow-up.

See #22620 for my judgemement on this edit war...

comment:32 in reply to: ↑ 30 Changed 3 years ago by dimpase

Replying to charpent:

Replying to dimpase:

interestingly, curl sources include vtls, (almost) an API to do (the client side of) TLS/SSL with different backends, e.g. DarwinSSL, OpenSSL (and a dozen others).

Do you think that this might ofar a solution to Jeroen's conudrum ? And our Mac OS/X woes ?

It is sure a fun project to allow python's _ssl to use it, but it is a lot of work...

Note: See TracTickets for help on using tickets.