From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark H Weaver Subject: bug#26948: gnutls errors on multiple guix commands Date: Mon, 29 May 2017 17:26:43 -0400 Message-ID: <87a85v1hik.fsf@netris.org> References: <8737c51e6r.fsf@gmail.com> <87shk3y74g.fsf@gnu.org> <8737btieie.fsf@gmail.com> <87vaoovvvz.fsf@gnu.org> <87poes25dw.fsf@netris.org> <87a85wc8li.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:32827) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dFSCs-0004Hg-VA for bug-guix@gnu.org; Mon, 29 May 2017 17:28:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dFSCo-0007LR-VN for bug-guix@gnu.org; Mon, 29 May 2017 17:28:06 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:41204) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dFSCo-0007LJ-Rv for bug-guix@gnu.org; Mon, 29 May 2017 17:28:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1dFSCo-0003T5-AT for bug-guix@gnu.org; Mon, 29 May 2017 17:28:02 -0400 Sender: "Debbugs-submit" Resent-Message-ID: In-Reply-To: <87a85wc8li.fsf@gnu.org> ("Ludovic \=\?utf-8\?Q\?Court\=C3\=A8s\=22'\?\= \=\?utf-8\?Q\?s\?\= message of "Mon, 29 May 2017 11:31:37 +0200") List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: "bug-Guix" To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 26948@debbugs.gnu.org, Maxim Cournoyer Hi Ludovic, ludo@gnu.org (Ludovic Court=C3=A8s) writes: > Mark H Weaver 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; >>=20=20=20 >> gnutls_certificate_credentials_t c_cred; >> gnutls_x509_crt_fmt_t c_format; >>=20=20=20 >> c_cred =3D scm_to_gnutls_certificate_credentials (cred, 1, FUNC_NAME= ); >> SCM_VALIDATE_STRING (2, file); >> c_format =3D scm_to_gnutls_x509_certificate_format (format, 3, FUNC_= NAME); >>=20=20=20 >> c_file_len =3D scm_c_string_length (file); >> c_file =3D alloca (c_file_len + 1); >>=20=20=20 >> (void) scm_to_locale_stringbuf (file, c_file, c_file_len + 1); >> c_file[c_file_len] =3D '\0'; >>=20=20=20 >> err =3D set_file (c_cred, c_file, c_format); >> if (EXPECT_FALSE (err < 0)) >> scm_gnutls_error (err, FUNC_NAME); >>=20=20=20 >> /* 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: > > 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. >=20=20 > 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_functi= on_t set_file, > { > int err; > char *c_file; > - size_t c_file_len; >=20=20 > gnutls_certificate_credentials_t c_cred; > gnutls_x509_crt_fmt_t c_format; >=20=20 > c_cred =3D scm_to_gnutls_certificate_credentials (cred, 1, FUNC_NAME); > SCM_VALIDATE_STRING (2, file); > - c_format =3D scm_to_gnutls_x509_certificate_format (format, 3, FUNC_NA= ME); > - > - c_file_len =3D scm_c_string_length (file); > - c_file =3D alloca (c_file_len + 1); >=20=20 > - (void) scm_to_locale_stringbuf (file, c_file, c_file_len + 1); > - c_file[c_file_len] =3D '\0'; > + c_format =3D scm_to_gnutls_x509_certificate_format (format, 3, FUNC_NA= ME); > + c_file =3D scm_to_locale_string (file); >=20=20 > err =3D set_file (c_cred, c_file, c_format); > + free (c_file); > + > if (EXPECT_FALSE (err < 0)) > scm_gnutls_error (err, FUNC_NAME); >=20=20 Looks good to me. In the case when a UTF-8 locale is active, and where Guile 2.0.12 or later is available, we could use 'scm_c_string_utf8_length' to find the length in bytes, but optimizing that case is probably not worth the extra code complexity. > Unfortunately there=E2=80=99s a dozen of places in core.c that use this i= diom > and would need to be fixed (it=E2=80=99s pre-2.0 code I think). Bummer. > In the meantime we can work around it this way: > > 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 com= munication port." > (make-parameter (or (getenv "GUIX_TLS_CERTIFICATE_DIRECTORY") > (getenv "SSL_CERT_DIR")))) ;like OpenSSL >=20=20 > +(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 > +." > + (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 '())) > > > WDYT? I=E2=80=99ll commit it if that=E2=80=99s fine with you. I'm not sufficiently familiar with GnuTLS to properly review this, but I trust your judgement. Thanks! Mark