Hi Mark,
Mark H Weaver <m...@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:
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);
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:
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 '()))
WDYT? I’ll commit it if that’s fine with you.
Thanks for the report!
Ludo’.
