unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
* [bug#56867] [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7.
@ 2022-08-01  9:07 Ludovic Courtès
  2022-08-01  9:15 ` Ludovic Courtès
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Ludovic Courtès @ 2022-08-01  9:07 UTC (permalink / raw)
  To: 56867; +Cc: Ludovic Courtès, guile-devel

The custom input/output port wrapping the TLS session record port would
introduce overhead, and it would also prevent its uses in a non-blocking
context--e.g., with Fibers.  The port close mechanism added in GnuTLS
3.7.7 allows us to get rid of that wrapper.

* guix/build/download.scm (wrap-record-port-for-gnutls<3.7.7): New
procedure, with code formerly in 'tls-wrap'.
(tls-wrap): Check for 'set-session-record-port-close!' and use it when
available; otherwise call 'wrap-record-port-for-gnutls<3.7.7'.
---
 guix/build/download.scm | 102 +++++++++++++++++++++-------------------
 1 file changed, 54 insertions(+), 48 deletions(-)

Hello!

I'll land a similar change in Guile's (web client) module afterwards
if there are no objections.

Ludo'.

diff --git a/guix/build/download.scm b/guix/build/download.scm
index 41583e8143..de094890b3 100644
--- a/guix/build/download.scm
+++ b/guix/build/download.scm
@@ -245,6 +245,54 @@ (define (print-tls-certificate-error port key args default-printer)
 (set-exception-printer! 'tls-certificate-error
                         print-tls-certificate-error)
 
+(define (wrap-record-port-for-gnutls<3.7.7 record port)
+  "Return a port that wraps RECORD to ensure that closing it also closes PORT,
+the actual socket port, and its file descriptor.  Make sure it does not
+introduce extra buffering (custom ports are buffered by default as of Guile
+3.0.5).
+
+This wrapper is unnecessary with GnuTLS >= 3.7.7, which can automatically
+close SESSION's file descriptor when RECORD is closed."
+  (define (read! bv start count)
+    (define read
+      (catch 'gnutls-error
+        (lambda ()
+          (get-bytevector-n! record bv start count))
+        (lambda (key err proc . rest)
+          ;; When responding to "Connection: close" requests, some servers
+          ;; close the connection abruptly after sending the response body,
+          ;; without doing a proper TLS connection termination.  Treat it as
+          ;; EOF.  This is fixed in GnuTLS 3.7.7.
+          (if (eq? err error/premature-termination)
+              the-eof-object
+              (apply throw key err proc rest)))))
+
+    (if (eof-object? read)
+        0
+        read))
+  (define (write! bv start count)
+    (put-bytevector record bv start count)
+    (force-output record)
+    count)
+  (define (get-position)
+    (port-position record))
+  (define (set-position! new-position)
+    (set-port-position! record new-position))
+  (define (close)
+    (unless (port-closed? port)
+      (close-port port))
+    (unless (port-closed? record)
+      (close-port record)))
+
+  (define (unbuffered port)
+    (setvbuf port 'none)
+    port)
+
+  (unbuffered
+   (make-custom-binary-input/output-port "gnutls wrapped port" read! write!
+                                         get-position set-position!
+                                         close)))
+
 (define* (tls-wrap port server #:key (verify-certificate? #t))
   "Return PORT wrapped in a TLS connection to SERVER.  SERVER must be a DNS
 host name without trailing dot."
@@ -317,55 +365,13 @@ (define (log level str)
           (apply throw args))))
 
     (let ((record (session-record-port session)))
-      (define (read! bv start count)
-        (define read
-          (catch 'gnutls-error
-            (lambda ()
-              (get-bytevector-n! record bv start count))
-            (lambda (key err proc . rest)
-              ;; When responding to "Connection: close" requests, some
-              ;; servers close the connection abruptly after sending the
-              ;; response body, without doing a proper TLS connection
-              ;; termination.  Treat it as EOF.
-              (if (eq? err error/premature-termination)
-                  the-eof-object
-                  (apply throw key err proc rest)))))
-
-        (if (eof-object? read)
-            0
-            read))
-      (define (write! bv start count)
-        (put-bytevector record bv start count)
-        (force-output record)
-        count)
-      (define (get-position)
-        (port-position record))
-      (define (set-position! new-position)
-        (set-port-position! record new-position))
-      (define (close)
-        (unless (port-closed? port)
-          (close-port port))
-        (unless (port-closed? record)
-          (close-port record)))
-
-      (define (unbuffered port)
-        (setvbuf port 'none)
-        port)
-
       (setvbuf record 'block)
-
-      ;; Return a port that wraps RECORD to ensure that closing it also
-      ;; closes PORT, the actual socket port, and its file descriptor.
-      ;; Make sure it does not introduce extra buffering (custom ports
-      ;; are buffered by default as of Guile 3.0.5).
-      ;; XXX: This wrapper would be unnecessary if GnuTLS could
-      ;; automatically close SESSION's file descriptor when RECORD is
-      ;; closed, but that doesn't seem to be possible currently (as of
-      ;; 3.6.9).
-      (unbuffered
-       (make-custom-binary-input/output-port "gnutls wrapped port" read! write!
-                                             get-position set-position!
-                                             close)))))
+      (if (module-defined? (resolve-interface '(gnutls))
+                           'set-session-record-port-close!) ;GnuTLS >= 3.7.7
+          (let ((close-wrapped-port (lambda (_) (close-port port))))
+            (set-session-record-port-close! record close-wrapped-port)
+            record)
+          (wrap-record-port-for-gnutls<3.7.7 record port)))))
 
 (define (ensure-uri uri-or-string)                ;XXX: copied from (web http)
   (cond

base-commit: ab59155c5a38dda7efaceb47c7528578fcf0def4
-- 
2.37.1






^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [bug#56867] [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7.
  2022-08-01  9:07 [bug#56867] [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7 Ludovic Courtès
@ 2022-08-01  9:15 ` Ludovic Courtès
  2022-08-01  9:56 ` Maxime Devos
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Ludovic Courtès @ 2022-08-01  9:15 UTC (permalink / raw)
  To: 56867; +Cc: guile-devel

[-- Attachment #1: Type: text/plain, Size: 521 bytes --]

Ludovic Courtès <ludo@gnu.org> skribis:

> The custom input/output port wrapping the TLS session record port would
> introduce overhead, and it would also prevent its uses in a non-blocking
> context--e.g., with Fibers.  The port close mechanism added in GnuTLS
> 3.7.7 allows us to get rid of that wrapper.

And here’s the GnuTLS 3.7.7 package to test it; you need to make sure to
have 3.7.7 on your load path, for instance by running:

  ./pre-inst-env guix shell -D guix guile gnutls@3.7.7

Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 990 bytes --]

diff --git a/gnu/packages/tls.scm b/gnu/packages/tls.scm
index 1ee5400a9c..33c93b7a5b 100644
--- a/gnu/packages/tls.scm
+++ b/gnu/packages/tls.scm
@@ -329,6 +329,21 @@ (define-public gnutls
     (properties '((ftp-server . "ftp.gnutls.org")
                   (ftp-directory . "/gcrypt/gnutls")))))
 
+(define-public gnutls-latest
+  (package
+    (inherit gnutls)
+    (version "3.7.7")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "mirror://gnupg/gnutls/v"
+                                  (version-major+minor version)
+                                  "/gnutls-" version ".tar.xz"))
+              (patches (search-patches "gnutls-skip-trust-store-test.patch"
+                                       "gnutls-cross.patch"))
+              (sha256
+               (base32
+                "01i1gl15k6qwvxmxx0by1mn9nlmcmym18wdpm7dn9awfsp8474dy"))))))
+
 (define-public gnutls/guile-2.0
   ;; GnuTLS for Guile 2.0.
   (package/inherit gnutls

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7.
  2022-08-01  9:07 [bug#56867] [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7 Ludovic Courtès
  2022-08-01  9:15 ` Ludovic Courtès
@ 2022-08-01  9:56 ` Maxime Devos
  2022-08-02  7:59   ` Ludovic Courtès
  2022-08-03 15:57 ` bug#56867: " Ludovic Courtès
  2022-08-04 14:20 ` [bug#56867] " Ludovic Courtès
  3 siblings, 1 reply; 12+ messages in thread
From: Maxime Devos @ 2022-08-01  9:56 UTC (permalink / raw)
  To: Ludovic Courtès, 56867; +Cc: guile-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 2414 bytes --]

Some objections on error handling (I don't know much about the wrapping)

On 01-08-2022 11:07, Ludovic Courtès wrote:
> [...]
> Hello!
>
> I'll land a similar change in Guile's (web client) module afterwards
> if there are no objections.
>
> Ludo'.
>
> diff --git a/guix/build/download.scm b/guix/build/download.scm
> index 41583e8143..de094890b3 100644
> --- a/guix/build/download.scm
> +++ b/guix/build/download.scm
> @@ -245,6 +245,54 @@ (define (print-tls-certificate-error port key args default-printer)
>   (set-exception-printer! 'tls-certificate-error
>                           print-tls-certificate-error)
>   
> +(define (wrap-record-port-for-gnutls<3.7.7 record port)
> +  "Return a port that wraps RECORD to ensure that closing it also closes PORT,
> +the actual socket port, and its file descriptor.  Make sure it does not
> +introduce extra buffering (custom ports are buffered by default as of Guile
> +3.0.5).
> +
> +This wrapper is unnecessary with GnuTLS >= 3.7.7, which can automatically
> +close SESSION's file descriptor when RECORD is closed."
> +  (define (read! bv start count)
> +    (define read
> +      (catch 'gnutls-error
> +        (lambda ()
> +          (get-bytevector-n! record bv start count))
> +        (lambda (key err proc . rest)
> +          ;; When responding to "Connection: close" requests, some servers
> +          ;; close the connection abruptly after sending the response body,
> +          ;; without doing a proper TLS connection termination.  Treat it as
> +          ;; EOF.  This is fixed in GnuTLS 3.7.7.
> +          (if (eq? err error/premature-termination)
> +              the-eof-object
> +              (apply throw key err proc rest)))))

Objection: 'catch' makes the backtrace part happening inside the 
'get-bytevector-n!' disappear, because it is unwinding, as has been 
noted a few times (in different contexts) by Attila Lendvai and me.  
Maybe use 'guard' with an appropriate condition instead?

> +      (if (module-defined? (resolve-interface '(gnutls))
> +                           'set-session-record-port-close!) ;GnuTLS >= 3.7.7

resolve-module (and presumably also sets #:ensure #t by default, which 
sometimes causes 'module not found' messages to be replaced by 'unbound 
variable', which I don't think is useful behaviour, can #:ensure be set 
to #false?

Greetings,
Maxime

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7.
  2022-08-01  9:56 ` Maxime Devos
@ 2022-08-02  7:59   ` Ludovic Courtès
  2022-08-04 19:37     ` Maxime Devos
  0 siblings, 1 reply; 12+ messages in thread
From: Ludovic Courtès @ 2022-08-02  7:59 UTC (permalink / raw)
  To: Maxime Devos; +Cc: 56867, guile-devel

Hi,

Maxime Devos <maximedevos@telenet.be> skribis:

> On 01-08-2022 11:07, Ludovic Courtès wrote:

[...]

>> +  (define (read! bv start count)
>> +    (define read
>> +      (catch 'gnutls-error
>> +        (lambda ()
>> +          (get-bytevector-n! record bv start count))
>> +        (lambda (key err proc . rest)
>> +          ;; When responding to "Connection: close" requests, some servers
>> +          ;; close the connection abruptly after sending the response body,
>> +          ;; without doing a proper TLS connection termination.  Treat it as
>> +          ;; EOF.  This is fixed in GnuTLS 3.7.7.
>> +          (if (eq? err error/premature-termination)
>> +              the-eof-object
>> +              (apply throw key err proc rest)))))
>
> Objection: 'catch' makes the backtrace part happening inside the
> 'get-bytevector-n!' disappear, because it is unwinding, as has been
> noted a few times (in different contexts) by Attila Lendvai and me. 
> Maybe use 'guard' with an appropriate condition instead?

This code was already there and has just been moved around.  (It’s also
code that will no longer be used going forward.)

>> +      (if (module-defined? (resolve-interface '(gnutls))
>> +                           'set-session-record-port-close!) ;GnuTLS >= 3.7.7
>
> resolve-module (and presumably also sets #:ensure #t by default, which
> sometimes causes 'module not found' messages to be replaced by
> 'unbound variable', which I don't think is useful behaviour, can
> #:ensure be set to #false?

This is unnecessary: see the ‘load-gnutls’ mechanism there.  The idiom
above is already used in a couple of places.

Thanks for your feedback!

Ludo’.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: bug#56867: [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7.
  2022-08-01  9:07 [bug#56867] [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7 Ludovic Courtès
  2022-08-01  9:15 ` Ludovic Courtès
  2022-08-01  9:56 ` Maxime Devos
@ 2022-08-03 15:57 ` Ludovic Courtès
  2022-08-04 14:20 ` [bug#56867] " Ludovic Courtès
  3 siblings, 0 replies; 12+ messages in thread
From: Ludovic Courtès @ 2022-08-03 15:57 UTC (permalink / raw)
  To: 56867-done; +Cc: guile-devel

Ludovic Courtès <ludo@gnu.org> skribis:

> The custom input/output port wrapping the TLS session record port would
> introduce overhead, and it would also prevent its uses in a non-blocking
> context--e.g., with Fibers.  The port close mechanism added in GnuTLS
> 3.7.7 allows us to get rid of that wrapper.
>
> * guix/build/download.scm (wrap-record-port-for-gnutls<3.7.7): New
> procedure, with code formerly in 'tls-wrap'.
> (tls-wrap): Check for 'set-session-record-port-close!' and use it when
> available; otherwise call 'wrap-record-port-for-gnutls<3.7.7'.
> ---
>  guix/build/download.scm | 102 +++++++++++++++++++++-------------------
>  1 file changed, 54 insertions(+), 48 deletions(-)

Pushed as Guix commit dd573ceea73295c7a872088ecd91e5f0fd74bf2b.

Ludo’.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [bug#56867] [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7.
  2022-08-01  9:07 [bug#56867] [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7 Ludovic Courtès
                   ` (2 preceding siblings ...)
  2022-08-03 15:57 ` bug#56867: " Ludovic Courtès
@ 2022-08-04 14:20 ` Ludovic Courtès
  2022-08-04 14:46   ` bug#56005: " Thiago Jung Bauermann via Bug reports for GNU Guix
  2022-08-04 17:31   ` bug#56867: " Chris Vine
  3 siblings, 2 replies; 12+ messages in thread
From: Ludovic Courtès @ 2022-08-04 14:20 UTC (permalink / raw)
  To: 56867; +Cc: guile-devel

[-- Attachment #1: Type: text/plain, Size: 1953 bytes --]

Ludovic Courtès <ludo@gnu.org> skribis:

> The custom input/output port wrapping the TLS session record port would
> introduce overhead, and it would also prevent its uses in a non-blocking
> context--e.g., with Fibers.  The port close mechanism added in GnuTLS
> 3.7.7 allows us to get rid of that wrapper.
>
> * guix/build/download.scm (wrap-record-port-for-gnutls<3.7.7): New
> procedure, with code formerly in 'tls-wrap'.
> (tls-wrap): Check for 'set-session-record-port-close!' and use it when
> available; otherwise call 'wrap-record-port-for-gnutls<3.7.7'.

I synchronized Guile's copy of this code:

  317b06bf8 web: 'tls-wrap' retries handshake upon non-fatal errors.
  c01ca10b3 web: Do not wrap TLS port on GnuTLS >= 3.7.7.

I realized that’s not enough to make it possible to use non-blocking
ports though.

First, I noticed that GnuTLS doesn’t implement ‘write_wait_fd’, only
‘read_wait_fd’ (not sure how problematic that is):

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,use(web client)
scheme@(guile-user)> (define p (open-socket-for-uri "https://guix.gnu.org"))
scheme@(guile-user)> ((@@ (ice-9 suspendable-ports) wait-for-writable) p)
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
In procedure write_wait_fd: unimplemented

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
scheme@(guile-user) [1]> ,q
scheme@(guile-user)> ,use(gnutls)
scheme@(guile-user)> (gnutls-version)
$1 = "3.7.7"
scheme@(guile-user)> ((@@ (ice-9 suspendable-ports) wait-for-readable) p)
$2 = 1
--8<---------------cut here---------------end--------------->8---

Second, ‘open-socket-for-uri’ creates a blocking socket and uses that as
the backing file descriptor of the TLS session.

We’d need a way to pass flags for the ‘socket’ call made by
‘open-socket-for-uri’ so we can pass O_NONBLOCK, maybe as show below:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1488 bytes --]

diff --git a/module/web/client.scm b/module/web/client.scm
index a08c4203c..9273a45ad 100644
--- a/module/web/client.scm
+++ b/module/web/client.scm
@@ -320,7 +320,8 @@ host name without trailing dot."
   (read-response port))
 
 (define* (open-socket-for-uri uri-or-string
-                              #:key (verify-certificate? #t))
+                              #:key (verify-certificate? #t)
+                              (flags 0))
   "Return an open input/output port for a connection to URI-OR-STRING.
 When VERIFY-CERTIFICATE? is true, verify HTTPS server certificates."
   (define uri
@@ -373,10 +374,18 @@ When VERIFY-CERTIFICATE? is true, verify HTTPS server certificates."
     (when (and https? (current-https-proxy))
       (setup-http-tunnel s uri))
 
-    (if https?
-        (tls-wrap s (uri-host uri)
-                  #:verify-certificate? verify-certificate?)
-        s)))
+    (let ((port (if https?
+                    (tls-wrap s (uri-host uri)
+                              #:verify-certificate? verify-certificate?)
+                    s)))
+      (unless (zero? flags)
+        ;; FLAGS might contain O_NONBLOCK.  Thus, set it as a last step
+        ;; because 'handshake' otherwise throws an exception for
+        ;; GNUTLS_E_AGAIN.
+        (let ((initial-flags (fcntl s F_GETFL)))
+          (fcntl s F_SETFL (logior initial-flags flags))))
+
+      port)))
 
 (define (extend-request r k v . additional)
   (let ((r (set-field r (request-headers)

[-- Attachment #3: Type: text/plain, Size: 392 bytes --]


… which lets us do that:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,use(web client)
scheme@(guile-user)> (define p (open-socket-for-uri "https://guix.gnu.org" #:flags O_NONBLOCK))
scheme@(guile-user)> (http-get "https://guix.gnu.org" #:port p)
--8<---------------cut here---------------end--------------->8---

Thoughts?

Ludo’.

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* bug#56005: [bug#56867] [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7.
  2022-08-04 14:20 ` [bug#56867] " Ludovic Courtès
@ 2022-08-04 14:46   ` Thiago Jung Bauermann via Bug reports for GNU Guix
  2022-08-04 16:19     ` Ludovic Courtès
  2022-08-04 17:31   ` bug#56867: " Chris Vine
  1 sibling, 1 reply; 12+ messages in thread
From: Thiago Jung Bauermann via Bug reports for GNU Guix @ 2022-08-04 14:46 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 56005, 56867, guile-devel


Hello Ludo,

I don't have any comment/insight on what you're doing in general, except
about one of your points below:

Ludovic Courtès <ludo@gnu.org> writes:

> First, I noticed that GnuTLS doesn’t implement ‘write_wait_fd’, only
> ‘read_wait_fd’ (not sure how problematic that is):
>
> scheme@(guile-user)> ,use(web client)
> scheme@(guile-user)> (define p (open-socket-for-uri "https://guix.gnu.org"))
> scheme@(guile-user)> ((@@ (ice-9 suspendable-ports) wait-for-writable) p)
> ice-9/boot-9.scm:1685:16: In procedure raise-exception:
> In procedure write_wait_fd: unimplemented
>
> Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
> scheme@(guile-user) [1]> ,q
> scheme@(guile-user)> ,use(gnutls)
> scheme@(guile-user)> (gnutls-version)
> $1 = "3.7.7"
> scheme@(guile-user)> ((@@ (ice-9 suspendable-ports) wait-for-readable) p)
> $2 = 1

This occasionally causes problems when fetching substitutes, as can be
seen in bug #56005 (during substitution: write_wait_fd: unimplemented).

-- 
Thanks
Thiago





^ permalink raw reply	[flat|nested] 12+ messages in thread

* [bug#56867] [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7.
  2022-08-04 14:46   ` bug#56005: " Thiago Jung Bauermann via Bug reports for GNU Guix
@ 2022-08-04 16:19     ` Ludovic Courtès
  0 siblings, 0 replies; 12+ messages in thread
From: Ludovic Courtès @ 2022-08-04 16:19 UTC (permalink / raw)
  To: Thiago Jung Bauermann; +Cc: 56005, 56867, guile-devel

Hi,

Thiago Jung Bauermann <bauermann@kolabnow.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> First, I noticed that GnuTLS doesn’t implement ‘write_wait_fd’, only
>> ‘read_wait_fd’ (not sure how problematic that is):
>>
>> scheme@(guile-user)> ,use(web client)
>> scheme@(guile-user)> (define p (open-socket-for-uri "https://guix.gnu.org"))
>> scheme@(guile-user)> ((@@ (ice-9 suspendable-ports) wait-for-writable) p)
>> ice-9/boot-9.scm:1685:16: In procedure raise-exception:
>> In procedure write_wait_fd: unimplemented
>>
>> Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
>> scheme@(guile-user) [1]> ,q
>> scheme@(guile-user)> ,use(gnutls)
>> scheme@(guile-user)> (gnutls-version)
>> $1 = "3.7.7"
>> scheme@(guile-user)> ((@@ (ice-9 suspendable-ports) wait-for-readable) p)
>> $2 = 1
>
> This occasionally causes problems when fetching substitutes, as can be
> seen in bug #56005 (during substitution: write_wait_fd: unimplemented).

Oh, I have not seen it but it’s weird: (guix scripts substitute) doesn’t
use O_NONBLOCK sockets, so I don’t get how it can hit that.  Needs
investigation…

Thanks,
Ludo’.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: bug#56867: [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7.
  2022-08-04 14:20 ` [bug#56867] " Ludovic Courtès
  2022-08-04 14:46   ` bug#56005: " Thiago Jung Bauermann via Bug reports for GNU Guix
@ 2022-08-04 17:31   ` Chris Vine
  1 sibling, 0 replies; 12+ messages in thread
From: Chris Vine @ 2022-08-04 17:31 UTC (permalink / raw)
  To: guile-devel

On Thu, 04 Aug 2022 16:20:12 +0200
Ludovic Courtès <ludo@gnu.org> wrote:
> Ludovic Courtès <ludo@gnu.org> skribis:
> 
> > The custom input/output port wrapping the TLS session record port would
> > introduce overhead, and it would also prevent its uses in a non-blocking
> > context--e.g., with Fibers.  The port close mechanism added in GnuTLS
> > 3.7.7 allows us to get rid of that wrapper.
> >
> > * guix/build/download.scm (wrap-record-port-for-gnutls<3.7.7): New
> > procedure, with code formerly in 'tls-wrap'.
> > (tls-wrap): Check for 'set-session-record-port-close!' and use it when
> > available; otherwise call 'wrap-record-port-for-gnutls<3.7.7'.
> 
> I synchronized Guile's copy of this code:
> 
>   317b06bf8 web: 'tls-wrap' retries handshake upon non-fatal errors.
>   c01ca10b3 web: Do not wrap TLS port on GnuTLS >= 3.7.7.
> 
> I realized that’s not enough to make it possible to use non-blocking
> ports though.

[snip revised patch]

> Thoughts?

I have not been following these patches and this may be completely
irrelevant, but if the context of these patches is the use of
gnutls-guile, then when I last looked (which was some time ago)
gnutls-guile ports were not suspendable.  So a non-blocking
gnutls-guile port will still block when used with guile's suspendable
ports.

If I have missed the point please ignore this.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7.
  2022-08-02  7:59   ` Ludovic Courtès
@ 2022-08-04 19:37     ` Maxime Devos
  2022-08-05  8:31       ` Ludovic Courtès
  0 siblings, 1 reply; 12+ messages in thread
From: Maxime Devos @ 2022-08-04 19:37 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 56867, guile-devel


[-- Attachment #1.1.1.1: Type: text/plain, Size: 1793 bytes --]


On 02-08-2022 09:59, Ludovic Courtès wrote:
>>> +      (if (module-defined? (resolve-interface '(gnutls))
>>> +                           'set-session-record-port-close!) ;GnuTLS >= 3.7.7
>> resolve-module (and presumably also sets #:ensure #t by default, which
>> sometimes causes 'module not found' messages to be replaced by
>> 'unbound variable', which I don't think is useful behaviour, can
>> #:ensure be set to #false?
> This is unnecessary: see the ‘load-gnutls’ mechanism there.  The idiom
> above is already used in a couple of places.

I have looked at the 'load-gnutls' procedure, but I do not see how it 
avoids the issue I mentioned (*).

I have also seen this idiom (resolve-interface and friends with #:ensure 
#t) before, in other places, but that doesn't make the idiom correct -- 
in fact, _because_ I've seen the idiom elsewhere causing problems, I 
recommend avoiding the same mistake here (and preferably also 
eliminating it elsewhere).

More generally, the second sentence is a logical fallacy, a variant of 
"ad populum" -- the prevalency of a mistake does not make it correct and 
does not invalidate evidence of it being a mistake.

To be clear, I am not referring to the existence/absence of compilation 
errors when compiling the Guix package without gnutls in the build 
environment, but to the confusing _contents_ of the error message and 
the odd semantics of #:ensure #t, and not only at compilation time but 
also at runtime.

(*) The autoloading of gnutls in load-gnutls avoids compilation errors 
when gnutls is absent, but by the way it does it, it causes the module 
to be registered as 'it exists' even when it doesn't, so the information 
in the module system of Guix becomes incorrect.

Greetings,
Maxime.


[-- Attachment #1.1.1.2: Type: text/html, Size: 2552 bytes --]

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7.
  2022-08-04 19:37     ` Maxime Devos
@ 2022-08-05  8:31       ` Ludovic Courtès
  2022-08-05 10:17         ` Maxime Devos
  0 siblings, 1 reply; 12+ messages in thread
From: Ludovic Courtès @ 2022-08-05  8:31 UTC (permalink / raw)
  To: Maxime Devos; +Cc: 56867, guile-devel

Hi,

Maxime Devos <maximedevos@telenet.be> skribis:

> On 02-08-2022 09:59, Ludovic Courtès wrote:
>>>> +      (if (module-defined? (resolve-interface '(gnutls))
>>>> +                           'set-session-record-port-close!) ;GnuTLS >= 3.7.7
>>> resolve-module (and presumably also sets #:ensure #t by default, which
>>> sometimes causes 'module not found' messages to be replaced by
>>> 'unbound variable', which I don't think is useful behaviour, can
>>> #:ensure be set to #false?
>> This is unnecessary: see the ‘load-gnutls’ mechanism there.  The idiom
>> above is already used in a couple of places.
>
> I have looked at the 'load-gnutls' procedure, but I do not see how it
> avoids the issue I mentioned (*).

[...]

> (*) The autoloading of gnutls in load-gnutls avoids compilation errors
> when gnutls is absent, but by the way it does it, it causes the module
> to be registered as 'it exists' even when it doesn't, so the
> information in the module system of Guix becomes incorrect.

I understand what you’re saying (I’m quite familiar with Guile’s module
system :-) and I do agree that #:ensure #t can lead to bad surprises),
but I don’t think this is correct:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> (resolve-interface '(xxx))
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
no code for module (xxx)

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
scheme@(guile-user) [1]> ,q
scheme@(guile-user)> (resolve-module '(xxx) #f #:ensure #f)
$1 = #f
--8<---------------cut here---------------end--------------->8---

This is because ‘resolve-interface’ does (resolve-module … #:ensure #f).

Does that make sense?

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7.
  2022-08-05  8:31       ` Ludovic Courtès
@ 2022-08-05 10:17         ` Maxime Devos
  0 siblings, 0 replies; 12+ messages in thread
From: Maxime Devos @ 2022-08-05 10:17 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 56867, guile-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 1129 bytes --]


On 05-08-2022 10:31, Ludovic Courtès wrote:
> I understand what you’re saying (I’m quite familiar with Guile’s module
> system :-) and I do agree that #:ensure #t can lead to bad surprises),
> but I don’t think this is correct:
>
> --8<---------------cut here---------------start------------->8---
> scheme@(guile-user)> (resolve-interface '(xxx))
> ice-9/boot-9.scm:1685:16: In procedure raise-exception:
> no code for module (xxx)
>
> Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
> scheme@(guile-user) [1]> ,q
> scheme@(guile-user)> (resolve-module '(xxx) #f #:ensure #f)
> $1 = #f
> --8<---------------cut here---------------end--------------->8---
>
> This is because ‘resolve-interface’ does (resolve-module … #:ensure #f).
>
> Does that make sense?

Oops, I thought the #:ensure #f was universal to all the resolve-... 
interfaces, but apparently not for resole-interface! In that case, no 
problem, though I'd like to eventually make some changes to the Guile 
docs for clarity (and maybe change the default #:ensure #t -> #f)

Greetings,
Maxime.



[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-08-05 10:17 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-01  9:07 [bug#56867] [PATCH] download: Do not wrap TLS port on GnuTLS >= 3.7.7 Ludovic Courtès
2022-08-01  9:15 ` Ludovic Courtès
2022-08-01  9:56 ` Maxime Devos
2022-08-02  7:59   ` Ludovic Courtès
2022-08-04 19:37     ` Maxime Devos
2022-08-05  8:31       ` Ludovic Courtès
2022-08-05 10:17         ` Maxime Devos
2022-08-03 15:57 ` bug#56867: " Ludovic Courtès
2022-08-04 14:20 ` [bug#56867] " Ludovic Courtès
2022-08-04 14:46   ` bug#56005: " Thiago Jung Bauermann via Bug reports for GNU Guix
2022-08-04 16:19     ` Ludovic Courtès
2022-08-04 17:31   ` bug#56867: " Chris Vine

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).