unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
       [not found] ` <20170714150707.5E9B322DF8@vcs0.savannah.gnu.org>
@ 2017-07-17  0:05   ` Glenn Morris
  2017-07-17 14:18     ` Ted Zlatanov
  0 siblings, 1 reply; 23+ messages in thread
From: Glenn Morris @ 2017-07-17  0:05 UTC (permalink / raw)
  To: emacs-devel; +Cc: Ted Zlatanov

Teodor Zlatanov wrote:

> branch: master
> commit 583995c62dd424775dda33d5134ce04bee2ae685

Hi, this change apparently causes gnutls tests to segfault on hydra.
I can't give you any more information than is accessible from

http://hydra.nixos.org/build/56430842



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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-17  0:05   ` master 583995c: GnuTLS HMAC and symmetric cipher support Glenn Morris
@ 2017-07-17 14:18     ` Ted Zlatanov
  2017-07-17 14:30       ` Michael Albinus
  2017-07-17 17:12       ` Glenn Morris
  0 siblings, 2 replies; 23+ messages in thread
From: Ted Zlatanov @ 2017-07-17 14:18 UTC (permalink / raw)
  To: emacs-devel

On Sun, 16 Jul 2017 20:05:15 -0400 Glenn Morris <rgm@gnu.org> wrote: 

GM> Teodor Zlatanov wrote:
>> branch: master
>> commit 583995c62dd424775dda33d5134ce04bee2ae685

GM> Hi, this change apparently causes gnutls tests to segfault on hydra.
GM> I can't give you any more information than is accessible from

GM> http://hydra.nixos.org/build/56430842

Thanks, Glenn. The logs have nothing useful unfortunately[1].

I'm not looking to install Nix on my personal machine to debug this,
which seems to be necessary in order to replicate the issue. I also
don't have a login to any machines that can run Nix. Can you or anyone
else recommend a way to replicate the issue using a Docker or LXC
container or other means? Or give me a login to the Hydra build farm?

Thanks
Ted

[1] The log view link in Hydra doesn't work with eww. That's not a big
deal but I thought I'd mention it.




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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-17 14:18     ` Ted Zlatanov
@ 2017-07-17 14:30       ` Michael Albinus
  2017-07-17 14:50         ` Ted Zlatanov
  2017-07-17 17:12       ` Glenn Morris
  1 sibling, 1 reply; 23+ messages in thread
From: Michael Albinus @ 2017-07-17 14:30 UTC (permalink / raw)
  To: emacs-devel

Ted Zlatanov <tzz@lifelogs.com> writes:

Hi Ted,

> Thanks, Glenn. The logs have nothing useful unfortunately[1].

For Tramp, I have modified the Makefile to get continuous print of the
log to stdout. This helps in cases Emacs crashes (your case) or blocks
(Tramp case).

We could extend this for GnuTLS tests. Ping me if you want me to do
this (or check my changes in test/Makefile.in).

> Thanks
> Ted

Best regards, Michael.



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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-17 14:30       ` Michael Albinus
@ 2017-07-17 14:50         ` Ted Zlatanov
  2017-07-17 18:22           ` Michael Albinus
  0 siblings, 1 reply; 23+ messages in thread
From: Ted Zlatanov @ 2017-07-17 14:50 UTC (permalink / raw)
  To: emacs-devel

On Mon, 17 Jul 2017 16:30:38 +0200 Michael Albinus <michael.albinus@gmx.de> wrote: 

MA> For Tramp, I have modified the Makefile to get continuous print of the
MA> log to stdout. This helps in cases Emacs crashes (your case) or blocks
MA> (Tramp case).

MA> We could extend this for GnuTLS tests. Ping me if you want me to do
MA> this (or check my changes in test/Makefile.in).

I'd love this. Maybe there could be a way of triggering this mode from
Hydra's interface, so it's not always on. If not, then yes, please make
the change. I assume it will be related to

    WRITE_LOG = $(if $(and ${NIX_STORE}, $(findstring tramp, $@)), |& tee $@, > $@ 2>&1) \

but a $(or) call just for GnuTLS would duplicate quite a bit of code,
and $(findstring) doesn't do regexes. Maybe you can decide :)

Thanks again
Ted




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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-17 14:18     ` Ted Zlatanov
  2017-07-17 14:30       ` Michael Albinus
@ 2017-07-17 17:12       ` Glenn Morris
  2017-07-17 17:19         ` Noam Postavsky
  2017-07-23 19:18         ` Ted Zlatanov
  1 sibling, 2 replies; 23+ messages in thread
From: Glenn Morris @ 2017-07-17 17:12 UTC (permalink / raw)
  To: emacs-devel

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


Please stop dropping me from the cc. :(

Ted Zlatanov wrote:

>>> branch: master
>>> commit 583995c62dd424775dda33d5134ce04bee2ae685
>
> GM> Hi, this change apparently causes gnutls tests to segfault on hydra.
> GM> I can't give you any more information than is accessible from
>
> GM> http://hydra.nixos.org/build/56430842
>
> Thanks, Glenn. The logs have nothing useful unfortunately[1].

The logs say that gnutls 3.2.21 was used.
I installed that version on rhel 7, and trivially reproduced the crash
when loading the gnutls tests. Backtrace attached. Maybe if you install
that version, it will crash for you too...


[-- Attachment #2: gdb.txt.xz --]
[-- Type: application/octet-stream, Size: 5428 bytes --]

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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-17 17:12       ` Glenn Morris
@ 2017-07-17 17:19         ` Noam Postavsky
  2017-07-17 18:20           ` Glenn Morris
  2017-07-23 19:18         ` Ted Zlatanov
  1 sibling, 1 reply; 23+ messages in thread
From: Noam Postavsky @ 2017-07-17 17:19 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Emacs developers

On Mon, Jul 17, 2017 at 1:12 PM, Glenn Morris <rgm@gnu.org> wrote:
>
> The logs say that gnutls 3.2.21 was used.

How do you find this info from the logs? I searched for "gnutls" and
found only stuff like:

checking for LIBGNUTLS... yes
checking for LIBGNUTLS3... yes



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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-17 17:19         ` Noam Postavsky
@ 2017-07-17 18:20           ` Glenn Morris
  2017-07-17 18:45             ` Noam Postavsky
  0 siblings, 1 reply; 23+ messages in thread
From: Glenn Morris @ 2017-07-17 18:20 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Emacs developers

Noam Postavsky wrote:

> On Mon, Jul 17, 2017 at 1:12 PM, Glenn Morris <rgm@gnu.org> wrote:
>>
>> The logs say that gnutls 3.2.21 was used.
>
> How do you find this info from the logs?

Sorry, the "build dependencies" tab, not the actual build log.

The crash occurs when gca = GNUTLS_CIPHER_UNKNOWN and
gnutls_cipher_get_name = NULL.



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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-17 14:50         ` Ted Zlatanov
@ 2017-07-17 18:22           ` Michael Albinus
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Albinus @ 2017-07-17 18:22 UTC (permalink / raw)
  To: emacs-devel

Ted Zlatanov <tzz@lifelogs.com> writes:

> I'd love this. Maybe there could be a way of triggering this mode from
> Hydra's interface, so it's not always on. If not, then yes, please make
> the change.

I'll wait a little bit, whether you could reproduce the problem with
Glenn's hint.

> Thanks again
> Ted

Best regards, Michael.



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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-17 18:20           ` Glenn Morris
@ 2017-07-17 18:45             ` Noam Postavsky
  2017-07-17 19:27               ` Glenn Morris
  0 siblings, 1 reply; 23+ messages in thread
From: Noam Postavsky @ 2017-07-17 18:45 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Emacs developers

On Mon, Jul 17, 2017 at 2:20 PM, Glenn Morris <rgm@gnu.org> wrote:
> Noam Postavsky wrote:
>
>> On Mon, Jul 17, 2017 at 1:12 PM, Glenn Morris <rgm@gnu.org> wrote:
>>>
>>> The logs say that gnutls 3.2.21 was used.
>>
>> How do you find this info from the logs?
>
> Sorry, the "build dependencies" tab, not the actual build log.

Oh, this is confusing; the build that you linked to,
http://hydra.nixos.org/build/56430842, doesn't have a "build
dependencies" tab.

Although http://hydra.nixos.org/build/56475235 for example, does.



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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-17 18:45             ` Noam Postavsky
@ 2017-07-17 19:27               ` Glenn Morris
  2017-07-17 19:44                 ` Paul Eggert
  0 siblings, 1 reply; 23+ messages in thread
From: Glenn Morris @ 2017-07-17 19:27 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Emacs developers

Noam Postavsky wrote:

> http://hydra.nixos.org/build/56430842, doesn't have a "build
> dependencies" tab.
>
> Although http://hydra.nixos.org/build/56475235 for example, does.

Oh, weird. Maybe this information gets garbage-collected fairly
rapidly to save space. Just guessing though.


After fixing the crash issue, test-gnutls-004-symmetric-ciphers fails with:

  (error "GnuTLS cipher IDEA-PGP-CFB/encrypt initialization failed: The
  request is invalid.")


BTW I note that gnutls_mac_get_name and gnutls_digest_get_name can
also apparently return NULL, and are currently passed to "intern"
without checking for this. I don't know if this can actually crash in
practice though.



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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-17 19:27               ` Glenn Morris
@ 2017-07-17 19:44                 ` Paul Eggert
  2017-07-17 21:37                   ` Glenn Morris
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Eggert @ 2017-07-17 19:44 UTC (permalink / raw)
  To: Glenn Morris, Noam Postavsky; +Cc: Emacs developers

On 07/17/2017 12:27 PM, Glenn Morris wrote:
> gnutls_mac_get_name and gnutls_digest_get_name can
> also apparently return NULL

As I recall, they can't return NULL in the contexts where they're used, 
as a previous call already established the validities of their arguments.




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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-17 19:44                 ` Paul Eggert
@ 2017-07-17 21:37                   ` Glenn Morris
  2017-07-17 21:58                     ` Paul Eggert
  0 siblings, 1 reply; 23+ messages in thread
From: Glenn Morris @ 2017-07-17 21:37 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Emacs developers, Noam Postavsky

Paul Eggert wrote:

>> gnutls_mac_get_name and gnutls_digest_get_name can
>> also apparently return NULL
>
> As I recall, they can't return NULL in the contexts where they're
> used, as a previous call already established the validities of their
> arguments.

It's not obvious to me that 2 and 3 below are any different to 1, save
for the fact that GNUTLS_CIPHER_NULL == 1 (and GNUTLS_CIPHER_UNKNOWN ==
0, which is maybe what happens to stop 2 and 3 crashing in the same way
1 does. Seems a bit like a happy accident?):


1. Fgnutls_ciphers. Can intern NULL and crash.
const gnutls_cipher_algorithm_t *gciphers = gnutls_cipher_list ();
for (ptrdiff_t pos = 0; gciphers[pos] != GNUTLS_CIPHER_NULL; pos++)
{
  gnutls_cipher_algorithm_t gca = gciphers[pos];
  Lisp_Object cipher_symbol = intern (gnutls_cipher_get_name (gca));
  ...
}


2. Fgnutls_macs.
const gnutls_mac_algorithm_t *macs = gnutls_mac_list ();
for (ptrdiff_t pos = 0; macs[pos] != 0; pos++)
{
  const gnutls_mac_algorithm_t gma = macs[pos];
  Lisp_Object gma_symbol = intern (gnutls_mac_get_name (gma));
  ...
}


3. Fgnutls_digests.
const gnutls_digest_algorithm_t *digests = gnutls_digest_list ();
for (ptrdiff_t pos = 0; digests[pos] != 0; pos++)
{
  const gnutls_digest_algorithm_t gda = digests[pos];
  Lisp_Object gda_symbol = intern (gnutls_digest_get_name (gda));
  ...
}



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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-17 21:37                   ` Glenn Morris
@ 2017-07-17 21:58                     ` Paul Eggert
  2017-07-17 22:14                       ` Glenn Morris
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Eggert @ 2017-07-17 21:58 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Emacs developers, Noam Postavsky

Glenn Morris wrote:

> Seems a bit like a happy accident?):

It's not an accident, as every valid cipher etc. has a name.

> 1. Fgnutls_ciphers. Can intern NULL and crash.
> const gnutls_cipher_algorithm_t *gciphers = gnutls_cipher_list ();
> for (ptrdiff_t pos = 0; gciphers[pos] != GNUTLS_CIPHER_NULL; pos++)
> {
>    gnutls_cipher_algorithm_t gca = gciphers[pos];
>    Lisp_Object cipher_symbol = intern (gnutls_cipher_get_name (gca));
>    ...
> }

It's OK, as every cipher other than GNUTLS_CIPHER_NULL has a non-NULL name.

> 2. Fgnutls_macs.
> const gnutls_mac_algorithm_t *macs = gnutls_mac_list ();
> for (ptrdiff_t pos = 0; macs[pos] != 0; pos++)
> {
>    const gnutls_mac_algorithm_t gma = macs[pos];
>    Lisp_Object gma_symbol = intern (gnutls_mac_get_name (gma));
>    ...
> }

Also OK, as every mac algorithm has a non-NULL name.

> 3. Fgnutls_digests.
> const gnutls_digest_algorithm_t *digests = gnutls_digest_list ();
> for (ptrdiff_t pos = 0; digests[pos] != 0; pos++)
> {
>    const gnutls_digest_algorithm_t gda = digests[pos];
>    Lisp_Object gda_symbol = intern (gnutls_digest_get_name (gda));
>    ...
> }

Likewise.



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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-17 21:58                     ` Paul Eggert
@ 2017-07-17 22:14                       ` Glenn Morris
  2017-07-18  0:59                         ` Paul Eggert
  0 siblings, 1 reply; 23+ messages in thread
From: Glenn Morris @ 2017-07-17 22:14 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Emacs developers, Noam Postavsky

Paul Eggert wrote:

> Glenn Morris wrote:
>
>> Seems a bit like a happy accident?):
>
> It's not an accident, as every valid cipher etc. has a name.
>
>> 1. Fgnutls_ciphers. Can intern NULL and crash.
>> const gnutls_cipher_algorithm_t *gciphers = gnutls_cipher_list ();
>> for (ptrdiff_t pos = 0; gciphers[pos] != GNUTLS_CIPHER_NULL; pos++)
>> {
>>    gnutls_cipher_algorithm_t gca = gciphers[pos];
>>    Lisp_Object cipher_symbol = intern (gnutls_cipher_get_name (gca));
>>    ...
>> }
>
> It's OK, as every cipher other than GNUTLS_CIPHER_NULL has a non-NULL name.

But it isn't OK. That's what this thread is about.
GNUTLS_CIPHER_UNKNOWN apparently has a NULL name.



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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-17 22:14                       ` Glenn Morris
@ 2017-07-18  0:59                         ` Paul Eggert
  2017-07-18  2:33                           ` Glenn Morris
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Eggert @ 2017-07-18  0:59 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Emacs developers, Noam Postavsky

Glenn Morris wrote:
>> It's OK, as every cipher other than GNUTLS_CIPHER_NULL has a non-NULL name.
> But it isn't OK. That's what this thread is about.
> GNUTLS_CIPHER_UNKNOWN apparently has a NULL name.

Sorry, I still don't see the problem here. From what I can see, 
GNUTLS_CIPHER_NULL is not a cipher (or to be more precise, it is not a valid 
GnuTLS cipher ID). The code in question:

> const gnutls_cipher_algorithm_t *gciphers = gnutls_cipher_list ();
> for (ptrdiff_t pos = 0; gciphers[pos] != GNUTLS_CIPHER_NULL; pos++)
> {
>   gnutls_cipher_algorithm_t gca = gciphers[pos];
>   Lisp_Object cipher_symbol = intern (gnutls_cipher_get_name (gca));

looks only at GnuTLS cipher IDs returned by gnutls_cipher_list, and 
GNUTLS_CIPHER_UNKNOWN is not one of those cipher IDs.



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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-18  0:59                         ` Paul Eggert
@ 2017-07-18  2:33                           ` Glenn Morris
  2017-07-18  7:45                             ` Paul Eggert
  0 siblings, 1 reply; 23+ messages in thread
From: Glenn Morris @ 2017-07-18  2:33 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Emacs developers, Noam Postavsky

Paul Eggert wrote:

> looks only at GnuTLS cipher IDs returned by gnutls_cipher_list, and
> GNUTLS_CIPHER_UNKNOWN is not one of those cipher IDs.

I feel like you've missed the first half of the discussion, so I'll
summarize it:

Since 583995c, make check on hydra crashes when loading the gnutls tests.
It uses gnutls 3.2.21. I installed that on my rhel7 system (with
--disable-non-suiteb-curves) and reproduced the crash. The backtrace is
at http://lists.gnu.org/archive/html/emacs-devel/2017-07/msg00716.html .

Specifically, I find it crashes because gnutls_cipher_list returns a list
containing GNUTLS_CIPHER_UNKNOWN.

The following patch fixes it for me:

*** /tmp/uOhC3f_gnutls.c	2017-07-17 19:32:32.851361998 -0700
--- src/gnutls.c	2017-07-17 12:03:27.514906186 -0700
***************
*** 1857,1870 ****
    for (ptrdiff_t pos = 0; gciphers[pos] != GNUTLS_CIPHER_NULL; pos++)
      {
        gnutls_cipher_algorithm_t gca = gciphers[pos];
  
        /* A symbol representing the GnuTLS cipher.  */
!       Lisp_Object cipher_symbol = intern (gnutls_cipher_get_name (gca));
  
!       ptrdiff_t cipher_tag_size = gnutls_cipher_get_tag_size (gca);
  
!       Lisp_Object cp
! 	= listn (CONSTYPE_HEAP, 15, cipher_symbol,
  		 QCcipher_id, make_number (gca),
  		 QCtype, Qgnutls_type_cipher,
  		 QCcipher_aead_capable, cipher_tag_size == 0 ? Qnil : Qt,
--- 1857,1877 ----
    for (ptrdiff_t pos = 0; gciphers[pos] != GNUTLS_CIPHER_NULL; pos++)
      {
        gnutls_cipher_algorithm_t gca = gciphers[pos];
+       const char *cipher_name;
+       Lisp_Object cipher_symbol, cp;
+       ptrdiff_t cipher_tag_size;
+ 
+       cipher_name = gnutls_cipher_get_name (gca);
+ 
+       if (! cipher_name)
+         continue;
  
        /* A symbol representing the GnuTLS cipher.  */
!       cipher_symbol = intern (cipher_name);
  
!       cipher_tag_size = gnutls_cipher_get_tag_size (gca);
  
!       cp = listn (CONSTYPE_HEAP, 15, cipher_symbol,
                    QCcipher_id, make_number (gca),
                    QCtype, Qgnutls_type_cipher,
                    QCcipher_aead_capable, cipher_tag_size == 0 ? Qnil : Qt,



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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-18  2:33                           ` Glenn Morris
@ 2017-07-18  7:45                             ` Paul Eggert
  2017-07-18 18:44                               ` Glenn Morris
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Eggert @ 2017-07-18  7:45 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Emacs developers, Noam Postavsky

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

Glenn Morris wrote:
> Since 583995c, make check on hydra crashes when loading the gnutls tests.
> It uses gnutls 3.2.21. I installed that on my rhel7 system (with
> --disable-non-suiteb-curves) and reproduced the crash. The

I don't see how the crash can occur with vanilla GnuTLS 3.2.21, as its 
gnutls_cipher_list returns a list of IDs that does not contain 
GNUTLS_CIPHER_UNKNOWN. Perhaps you were using a modified GnuTLS 3.2.21. Or 
possibly I'm misreading the GnuTLS source code, though I don't see how.

As you're observing the problem I installed the attached, which is similar to 
the patch that worked for you.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Port-gnutls.c-to-older-buggier-GnuTLS.patch --]
[-- Type: text/x-patch; name="0001-Port-gnutls.c-to-older-buggier-GnuTLS.patch", Size: 1693 bytes --]

From 376151481b2172dbb08d25bb5946f0f627f7453d Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 18 Jul 2017 00:37:03 -0700
Subject: [PATCH] Port gnutls.c to older (buggier?) GnuTLS

Problem reported for GnuTLS 3.2.1 by Glenn Morris in:
http://lists.gnu.org/archive/html/emacs-devel/2017-07/msg00716.html
http://lists.gnu.org/archive/html/emacs-devel/2017-07/msg00742.html
Although I don't see how this bug can occur with vanilla GnuTLS 3.2.1,
perhaps hydra was using a modified GnuTLS.
* src/gnutls.c (Fgnutls_ciphers): Don't assume GNUTLS_CIPHER_NULL
is at the end of the list returned by gnutls_cipher_list,
or that the earlier ciphers all have non-null names.
---
 src/gnutls.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/gnutls.c b/src/gnutls.c
index 9fbaea2..e406d66 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -1854,12 +1854,17 @@ The alist key is the cipher name. */)
 
 #ifdef HAVE_GNUTLS3_CIPHER
   const gnutls_cipher_algorithm_t *gciphers = gnutls_cipher_list ();
-  for (ptrdiff_t pos = 0; gciphers[pos] != GNUTLS_CIPHER_NULL; pos++)
+  for (ptrdiff_t pos = 0; gciphers[pos] != 0; pos++)
     {
       gnutls_cipher_algorithm_t gca = gciphers[pos];
+      if (gca == GNUTLS_CIPHER_NULL)
+	continue;
+      char const *cipher_name = gnutls_cipher_get_name (gca);
+      if (!cipher_name)
+	continue;
 
       /* A symbol representing the GnuTLS cipher.  */
-      Lisp_Object cipher_symbol = intern (gnutls_cipher_get_name (gca));
+      Lisp_Object cipher_symbol = intern (cipher_name);
 
       ptrdiff_t cipher_tag_size = gnutls_cipher_get_tag_size (gca);
 
-- 
2.7.4


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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-18  7:45                             ` Paul Eggert
@ 2017-07-18 18:44                               ` Glenn Morris
  2017-07-18 20:44                                 ` Paul Eggert
  0 siblings, 1 reply; 23+ messages in thread
From: Glenn Morris @ 2017-07-18 18:44 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Emacs developers, Noam Postavsky

Paul Eggert wrote:

> I don't see how the crash can occur with vanilla GnuTLS 3.2.21, as its
> gnutls_cipher_list returns a list of IDs that does not contain
> GNUTLS_CIPHER_UNKNOWN. Perhaps you were using a modified GnuTLS
> 3.2.21. Or possibly I'm misreading the GnuTLS source code, though I
> don't see how.

I was using ftp://ftp.gnutls.org/gcrypt/gnutls/v3.2/gnutls-3.2.21.tar.xz



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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-18 18:44                               ` Glenn Morris
@ 2017-07-18 20:44                                 ` Paul Eggert
  2017-07-18 21:02                                   ` Glenn Morris
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Eggert @ 2017-07-18 20:44 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Emacs developers, Noam Postavsky

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

Glenn Morris wrote:
> I was using ftp://ftp.gnutls.org/gcrypt/gnutls/v3.2/gnutls-3.2.21.tar.xz

Weird, because I was looking at the same source code. It does not build under 
recent Ubuntu or Fedora, so I did not compile it: I just looked at the source 
code. Its implementation of gnutls_cipher_list appears to be straightforward: it 
adds only values found as the 'id' component of an entry in the 'algorithms' 
array in lib/algorithms/ciphers.c. None of these values are 
GNUTLS_CIPHER_UNKNOWN, so gnutls_cipher_list cannot possibly return an array 
containing GNUTLS_CIPHER_UNKNOWN.

For example, on my platform, if I compile and run the attached program via:

gcc gs.c -lgnutls
./a.out

the program silently succeeds. What happens on your platform?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gs.c --]
[-- Type: text/x-csrc; name="gs.c", Size: 387 bytes --]

#include <gnutls/gnutls.h>
#include <stdio.h>

int
main (void)
{
  const gnutls_cipher_algorithm_t *gciphers = gnutls_cipher_list ();
  for (int pos = 0; gciphers[pos] != 0; pos++)
    {
      gnutls_cipher_algorithm_t gca = gciphers[pos];
      if (gca == GNUTLS_CIPHER_UNKNOWN)
	return printf ("gciphers[%d] == GNUTLS_CIPHER_UNKNOWN ?!?\n", pos), 1;
    }
  return 0;
}

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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-18 20:44                                 ` Paul Eggert
@ 2017-07-18 21:02                                   ` Glenn Morris
  2017-07-18 21:16                                     ` Paul Eggert
  0 siblings, 1 reply; 23+ messages in thread
From: Glenn Morris @ 2017-07-18 21:02 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Emacs developers, Noam Postavsky

Paul Eggert wrote:

> For example, on my platform, if I compile and run the attached program via:
>
> gcc gs.c -lgnutls
> ./a.out
>
> the program silently succeeds. What happens on your platform?

Your example doesn't correspond to how the code in Emacs used to be.
Change it to:

  for (int pos = 0; gciphers[pos] != GNUTLS_CIPHER_NULL; pos++)

and it fails for me, just as Emacs used to:

  gciphers[17] == GNUTLS_CIPHER_UNKNOWN ?!?

As I said, it seems to be a happy accident that GNUTLS_CIPHER_UNKNOWN == 0.
So your example as written happens to skip it.

I hope we have now established I'm not mad or lying... :)



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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-18 21:02                                   ` Glenn Morris
@ 2017-07-18 21:16                                     ` Paul Eggert
  2017-07-18 21:29                                       ` Glenn Morris
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Eggert @ 2017-07-18 21:16 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Emacs developers, Noam Postavsky

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

Glenn Morris wrote:
> Your example doesn't correspond to how the code in Emacs used to be.
> Change it to:
> 
>    for (int pos = 0; gciphers[pos] != GNUTLS_CIPHER_NULL; pos++)
> 
> and it fails for me, just as Emacs used to:

Weirder and weirder. First, with that change the code still works for me (I 
tried it with GnuTLS 3.4.10). Second, if I read the GnuTLS 3.2.21 source code, 
the changed code should still work for you too. This is because the test 
gciphers[pos] != GNUTLS_CIPHER_NULL should succeed one iteration before the test 
gciphers[pos] != 0 succeeds.

> 
>    gciphers[17] == GNUTLS_CIPHER_UNKNOWN ?!?
> 
> As I said, it seems to be a happy accident that GNUTLS_CIPHER_UNKNOWN == 0.
> So your example as written happens to skip it.
> 
> I hope we have now established I'm not mad or lying... :)

Sorry, not established yet. :-) Quite possibly I'm the crazy one, of course. 
Still, there's something very odd going on here.

To make sure we're on the same page I am attaching the changed code, which still 
works for me.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gs1.c --]
[-- Type: text/x-csrc; name="gs1.c", Size: 404 bytes --]

#include <gnutls/gnutls.h>
#include <stdio.h>

int
main (void)
{
  const gnutls_cipher_algorithm_t *gciphers = gnutls_cipher_list ();
  for (int pos = 0; gciphers[pos] != GNUTLS_CIPHER_NULL; pos++)
    {
      gnutls_cipher_algorithm_t gca = gciphers[pos];
      if (gca == GNUTLS_CIPHER_UNKNOWN)
	return printf ("gciphers[%d] == GNUTLS_CIPHER_UNKNOWN ?!?\n", pos), 1;
    }
  return 0;
}

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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-18 21:16                                     ` Paul Eggert
@ 2017-07-18 21:29                                       ` Glenn Morris
  0 siblings, 0 replies; 23+ messages in thread
From: Glenn Morris @ 2017-07-18 21:29 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Emacs developers, Noam Postavsky

Paul Eggert wrote:

> To make sure we're on the same page I am attaching the changed code,
> which still works for me.

For the Nth time:

When compiled and run using vanilla gnutls 3.2.21, this code prints

  gciphers[17] == GNUTLS_CIPHER_UNKNOWN ?!?



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

* Re: master 583995c: GnuTLS HMAC and symmetric cipher support
  2017-07-17 17:12       ` Glenn Morris
  2017-07-17 17:19         ` Noam Postavsky
@ 2017-07-23 19:18         ` Ted Zlatanov
  1 sibling, 0 replies; 23+ messages in thread
From: Ted Zlatanov @ 2017-07-23 19:18 UTC (permalink / raw)
  To: Glenn Morris; +Cc: emacs-devel

On Mon, 17 Jul 2017 13:12:27 -0400 Glenn Morris <rgm@gnu.org> wrote: 

GM> The logs say that gnutls 3.2.21 was used.
GM> I installed that version on rhel 7, and trivially reproduced the crash
GM> when loading the gnutls tests. Backtrace attached. Maybe if you install
GM> that version, it will crash for you too...

Hi Glenn,

the build appears fixed now, thanks to the fixes that Paul kindly
applied. Am I correct or am I misinterpreting the Hydra build report?

Would it be possible to have Hydra also test the latest release and
latest Git checkout of the GnuTLS library?

GM> Please stop dropping me from the cc. :(

It really is easier for me to use GMane (as a followup to the list
only), but I understand it's an inconvenience for you and will try to
adjust accordingly.

Ted

P.S. Sorry I couldn't reply sooner, I am traveling abroad for family
reasons.



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

end of thread, other threads:[~2017-07-23 19:18 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20170714150706.13106.18905@vcs0.savannah.gnu.org>
     [not found] ` <20170714150707.5E9B322DF8@vcs0.savannah.gnu.org>
2017-07-17  0:05   ` master 583995c: GnuTLS HMAC and symmetric cipher support Glenn Morris
2017-07-17 14:18     ` Ted Zlatanov
2017-07-17 14:30       ` Michael Albinus
2017-07-17 14:50         ` Ted Zlatanov
2017-07-17 18:22           ` Michael Albinus
2017-07-17 17:12       ` Glenn Morris
2017-07-17 17:19         ` Noam Postavsky
2017-07-17 18:20           ` Glenn Morris
2017-07-17 18:45             ` Noam Postavsky
2017-07-17 19:27               ` Glenn Morris
2017-07-17 19:44                 ` Paul Eggert
2017-07-17 21:37                   ` Glenn Morris
2017-07-17 21:58                     ` Paul Eggert
2017-07-17 22:14                       ` Glenn Morris
2017-07-18  0:59                         ` Paul Eggert
2017-07-18  2:33                           ` Glenn Morris
2017-07-18  7:45                             ` Paul Eggert
2017-07-18 18:44                               ` Glenn Morris
2017-07-18 20:44                                 ` Paul Eggert
2017-07-18 21:02                                   ` Glenn Morris
2017-07-18 21:16                                     ` Paul Eggert
2017-07-18 21:29                                       ` Glenn Morris
2017-07-23 19:18         ` Ted Zlatanov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.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).