unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
From: ludo@gnu.org (Ludovic Courtès)
To: Mark H Weaver <mhw@netris.org>
Cc: 26948@debbugs.gnu.org, Maxim Cournoyer <maxim.cournoyer@gmail.com>
Subject: bug#26948: gnutls errors on multiple guix commands
Date: Mon, 29 May 2017 11:31:37 +0200	[thread overview]
Message-ID: <87a85wc8li.fsf@gnu.org> (raw)
In-Reply-To: <87poes25dw.fsf@netris.org> (Mark H. Weaver's message of "Sun, 28 May 2017 14:38:51 -0400")

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

Hi Mark,

Mark H Weaver <mhw@netris.org> skribis:

> This reminds me of a bug that I found in the Guile binding in GnuTLS a
> while ago, but forgot to report.  Maybe it's related:
>
> In 'set_certificate_file' in gnutls-3.5.9/guile/src/core.c:
>
>   static unsigned int
>   set_certificate_file (certificate_set_file_function_t set_file,
>                         SCM cred, SCM file, SCM format, const char *func_name)
>   #define FUNC_NAME func_name
>   {
>     int err;
>     char *c_file;
>     size_t c_file_len;
>   
>     gnutls_certificate_credentials_t c_cred;
>     gnutls_x509_crt_fmt_t c_format;
>   
>     c_cred = scm_to_gnutls_certificate_credentials (cred, 1, FUNC_NAME);
>     SCM_VALIDATE_STRING (2, file);
>     c_format = scm_to_gnutls_x509_certificate_format (format, 3, FUNC_NAME);
>   
>     c_file_len = scm_c_string_length (file);
>     c_file = alloca (c_file_len + 1);
>   
>     (void) scm_to_locale_stringbuf (file, c_file, c_file_len + 1);
>     c_file[c_file_len] = '\0';
>   
>     err = set_file (c_cred, c_file, c_format);
>     if (EXPECT_FALSE (err < 0))
>       scm_gnutls_error (err, FUNC_NAME);
>   
>     /* Return the number of certificates processed.  */
>     return ((unsigned int) err);
>   }
>
> 'scm_c_string_length' is inappropriately assumed to return the length
> of the encoded C string in bytes, whereas it actually returns the
> number of characters (code points).

This is terrible!  WDYT of this:


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

diff --git a/guile/src/core.c b/guile/src/core.c
index 605c91f7a..38d573fa9 100644
--- a/guile/src/core.c
+++ b/guile/src/core.c
@@ -1,5 +1,5 @@
 /* GnuTLS --- Guile bindings for GnuTLS.
-   Copyright (C) 2007-2014, 2016 Free Software Foundation, Inc.
+   Copyright (C) 2007-2014, 2016-2017 Free Software Foundation, Inc.
 
    GnuTLS is free software; you can redistribute it and/or
    modify it under the terms of the GNU Lesser General Public
@@ -1428,22 +1428,19 @@ set_certificate_file (certificate_set_file_function_t set_file,
 {
   int err;
   char *c_file;
-  size_t c_file_len;
 
   gnutls_certificate_credentials_t c_cred;
   gnutls_x509_crt_fmt_t c_format;
 
   c_cred = scm_to_gnutls_certificate_credentials (cred, 1, FUNC_NAME);
   SCM_VALIDATE_STRING (2, file);
-  c_format = scm_to_gnutls_x509_certificate_format (format, 3, FUNC_NAME);
-
-  c_file_len = scm_c_string_length (file);
-  c_file = alloca (c_file_len + 1);
 
-  (void) scm_to_locale_stringbuf (file, c_file, c_file_len + 1);
-  c_file[c_file_len] = '\0';
+  c_format = scm_to_gnutls_x509_certificate_format (format, 3, FUNC_NAME);
+  c_file = scm_to_locale_string (file);
 
   err = set_file (c_cred, c_file, c_format);
+  free (c_file);
+
   if (EXPECT_FALSE (err < 0))
     scm_gnutls_error (err, FUNC_NAME);
 

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


Unfortunately there’s a dozen of places in core.c that use this idiom
and would need to be fixed (it’s pre-2.0 code I think).

In the meantime we can work around it this way:


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

diff --git a/guix/build/download.scm b/guix/build/download.scm
index ce4708a87..6ef623334 100644
--- a/guix/build/download.scm
+++ b/guix/build/download.scm
@@ -296,6 +296,13 @@ session record port using PORT as its underlying communication port."
   (make-parameter (or (getenv "GUIX_TLS_CERTIFICATE_DIRECTORY")
                       (getenv "SSL_CERT_DIR"))))  ;like OpenSSL
 
+(define (set-certificate-credentials-x509-trust-file!* cred file format)
+  "Like 'set-certificate-credentials-x509-trust-file!', but without the file
+name decoding bug described at
+<https://debbugs.gnu.org/cgi/bugreport.cgi?bug=26948#17>."
+  (let ((data (call-with-input-file file get-bytevector-all)))
+    (set-certificate-credentials-x509-trust-data! cred data format)))
+
 (define (make-credendials-with-ca-trust-files directory)
   "Return certificate credentials with X.509 authority certificates read from
 DIRECTORY.  Those authority certificates are checked when
@@ -309,7 +316,7 @@ DIRECTORY.  Those authority certificates are checked when
                 (let ((file (string-append directory "/" file)))
                   ;; Protect against dangling symlinks.
                   (when (file-exists? file)
-                    (set-certificate-credentials-x509-trust-file!
+                    (set-certificate-credentials-x509-trust-file!*
                      cred file
                      x509-certificate-format/pem))))
               (or files '()))

[-- Attachment #5: Type: text/plain, Size: 92 bytes --]


WDYT?  I’ll commit it if that’s fine with you.

Thanks for the report!

Ludo’.

  parent reply	other threads:[~2017-05-29  9:32 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-16  5:19 bug#26948: gnutls errors on multiple guix commands Maxim Cournoyer
2017-05-17 12:56 ` Ludovic Courtès
2017-05-25  7:26   ` Maxim Cournoyer
2017-05-26  8:56     ` Ludovic Courtès
2017-05-28 18:38       ` Mark H Weaver
2017-05-29  4:36         ` Maxim Cournoyer
2017-05-29  9:31         ` Ludovic Courtès [this message]
2017-05-29 21:26           ` Mark H Weaver
2017-05-30 11:25             ` Ludovic Courtès
2017-05-28 21:00       ` Maxim Cournoyer
2017-05-29  9:12         ` bug#26948: ‘write-file’ output should not be locale-dependent Ludovic Courtès
2017-05-29 20:15           ` Maxim Cournoyer
2017-05-30 11:57             ` Ludovic Courtès
2017-06-16 15:09               ` Ludovic Courtès
2017-07-27 12:55           ` Ludovic Courtès
2021-01-08 22:04             ` bug#26948: 'guix publish' file name decoding is locale-dependent Maxim Cournoyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a85wc8li.fsf@gnu.org \
    --to=ludo@gnu.org \
    --cc=26948@debbugs.gnu.org \
    --cc=maxim.cournoyer@gmail.com \
    --cc=mhw@netris.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

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).