bug#25061: consider adding %COMPAT to default gnutls priority string

bug#25061: consider adding %COMPAT to default gnutls priority string

[Andy Wingo]

There have been reports of errors from people using melpa and so on
which manifest themselves as:

    gnutls.c: [0] (Emacs) fatal error: The TLS connection was non-properly terminated.

However I think maybe that's just the symptom and not the cause; see the
previous report:

    https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25060

Ludovic Courtès was seeing a similar issue to the one that people are
reporting for melpa etc in Guix, where we also use GnuTLS though not in
Emacs.  He then found that GNU wget, which also uses GnuTLS, wasn't
exhibiting the same behavior.  He was eventually able to reproduce the
problem with just gnutls-cli.  He tracked down the difference in that if
he adds %COMPAT to the priority list, then he has no problems:

    https://lists.gnu.org/archive/html/bug-guix/2016-04/msg00098.html
    http://bugs.gnu.org/23311

Note that the problem only exhibits itself for some web sites, and only
some of the time.  It manifested itself as a timeout where the server
would get stuck, which could explain that people are unable to fetch
packages then blame the problem on the spurious post-close error message
from bug 25060.

So, as Ludovic suggests in his message, a workaround might be:

    (setq gnutls-algorithm-priority "NORMAL:%COMPAT")

See Ludovic's message for some justification.  Just an idea.  I have
been trying to reproduce the problem that people report locally as some
TLS errors but I have not been able to.

Andy

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Tue, 29 Nov 2016 11:24:53 +0100 Andy Wingo <wingo@igalia.com> wrote: 

AW> There have been reports of errors from people using melpa and so on
AW> which manifest themselves as:

AW>     gnutls.c: [0] (Emacs) fatal error: The TLS connection was non-properly terminated.
...
AW> So, as Ludovic suggests in his message, a workaround might be:

AW>     (setq gnutls-algorithm-priority "NORMAL:%COMPAT")

AW> See Ludovic's message for some justification.  Just an idea.  I have
AW> been trying to reproduce the problem that people report locally as some
AW> TLS errors but I have not been able to.

We could break down %COMPAT to all its components and find which ones
are causing the issue.

I wouldn't make %COMPAT part of the default. For the sake of accessing
some sites, we'd make all our users less secure. But if the connection
fails, maybe we can suggest setting it for just that site. That's not
trivial right now but I plan to add that capability.

Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ludovic Courtès]

Ted Zlatanov <tzz@lifelogs.com> skribis:

> On Tue, 29 Nov 2016 11:24:53 +0100 Andy Wingo <wingo@igalia.com> wrote: 
>
> AW> There have been reports of errors from people using melpa and so on
> AW> which manifest themselves as:
>
> AW>     gnutls.c: [0] (Emacs) fatal error: The TLS connection was non-properly terminated.
> ...
> AW> So, as Ludovic suggests in his message, a workaround might be:
>
> AW>     (setq gnutls-algorithm-priority "NORMAL:%COMPAT")
>
> AW> See Ludovic's message for some justification.  Just an idea.  I have
> AW> been trying to reproduce the problem that people report locally as some
> AW> TLS errors but I have not been able to.
>
> We could break down %COMPAT to all its components and find which ones
> are causing the issue.

%DUMBFW may be that option (info "(gnutls) Priority Strings"):

--8<---------------cut here---------------start------------->8---
%DUMBFW                          will add a private extension
                                 with bogus data that make the
                                 client hello exceed 512 bytes.
                                 This avoids a black hole
                                 behavior in some firewalls.
                                 This is the [_rfc7685_] client
                                 hello padding extension, also
                                 enabled with %COMPAT.
--8<---------------cut here---------------end--------------->8---

(Somehow I don’t recall seeing it back in the day.)

Ludo’.

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Thu, 01 Dec 2016 21:25:29 +0100 ludo@gnu.org (Ludovic Courtès) wrote: 

LC> Ted Zlatanov <tzz@lifelogs.com> skribis:

>> We could break down %COMPAT to all its components and find which ones
>> are causing the issue.

LC> %DUMBFW may be that option (info "(gnutls) Priority Strings"):

LC> will add a private extension with bogus data that make the
LC> client hello exceed 512 bytes. This avoids a black hole
LC> behavior in some firewalls. This is the [_rfc7685_] client
LC> hello padding extension, also enabled with %COMPAT.

Nice. Could you or Andy verify if it resolves the originally reported
problem? I think it's very reasonable to add it to the default, if it does.

Thanks
Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Lars Ingebrigtsen]

Ted Zlatanov <tzz@lifelogs.com> writes:

> On Thu, 01 Dec 2016 21:25:29 +0100 ludo@gnu.org (Ludovic Courtès) wrote: 
>
> LC> Ted Zlatanov <tzz@lifelogs.com> skribis:
>
>>> We could break down %COMPAT to all its components and find which ones
>>> are causing the issue.
>
> LC> %DUMBFW may be that option (info "(gnutls) Priority Strings"):
>
> LC> will add a private extension with bogus data that make the
> LC> client hello exceed 512 bytes. This avoids a black hole
> LC> behavior in some firewalls. This is the [_rfc7685_] client
> LC> hello padding extension, also enabled with %COMPAT.
>
> Nice. Could you or Andy verify if it resolves the originally reported
> problem? I think it's very reasonable to add it to the default, if it does.

Yes, I think so, too.  Did anyone of you check whether it fixed the problems?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ludovic Courtès]

Hi Lars,

Lars Ingebrigtsen <larsi@gnus.org> skribis:

> Ted Zlatanov <tzz@lifelogs.com> writes:
>
>> On Thu, 01 Dec 2016 21:25:29 +0100 ludo@gnu.org (Ludovic Courtès) wrote: 
>>
>> LC> Ted Zlatanov <tzz@lifelogs.com> skribis:
>>
>>>> We could break down %COMPAT to all its components and find which ones
>>>> are causing the issue.
>>
>> LC> %DUMBFW may be that option (info "(gnutls) Priority Strings"):
>>
>> LC> will add a private extension with bogus data that make the
>> LC> client hello exceed 512 bytes. This avoids a black hole
>> LC> behavior in some firewalls. This is the [_rfc7685_] client
>> LC> hello padding extension, also enabled with %COMPAT.
>>
>> Nice. Could you or Andy verify if it resolves the originally reported
>> problem? I think it's very reasonable to add it to the default, if it does.
>
> Yes, I think so, too.  Did anyone of you check whether it fixed the problems?

I’m just a passerby so I haven’t tested.  ;-)

I think Andy has been traveling lately but he might be able to check
later.

Cheers,
Ludo’.

bug#25061: consider adding %COMPAT to default gnutls priority string

[Andy Wingo]

On Mon 30 Jan 2017 09:01, ludo@gnu.org (Ludovic Courtès) writes:

> Lars Ingebrigtsen <larsi@gnus.org> skribis:
>
>> Ted Zlatanov <tzz@lifelogs.com> writes:
>>
>>> On Thu, 01 Dec 2016 21:25:29 +0100 ludo@gnu.org (Ludovic Courtès) wrote: 
>>>
>>> LC> Ted Zlatanov <tzz@lifelogs.com> skribis:
>>>
>>>>> We could break down %COMPAT to all its components and find which ones
>>>>> are causing the issue.
>>>
>>> LC> %DUMBFW may be that option (info "(gnutls) Priority Strings"):
>>>
>>> LC> will add a private extension with bogus data that make the
>>> LC> client hello exceed 512 bytes. This avoids a black hole
>>> LC> behavior in some firewalls. This is the [_rfc7685_] client
>>> LC> hello padding extension, also enabled with %COMPAT.
>>>
>>> Nice. Could you or Andy verify if it resolves the originally reported
>>> problem? I think it's very reasonable to add it to the default, if it does.
>>
>> Yes, I think so, too.  Did anyone of you check whether it fixed the problems?
>
> I’m just a passerby so I haven’t tested.  ;-)
>
> I think Andy has been traveling lately but he might be able to check
> later.

I tried checking (had to remember what I was doing to begin with!) and
was not able to reproduce the original problem, and therefore couldn't
test NORMAL:%COMPAT or NORMAL:%DUMBFW :/  Sorry :/

I was trying to just do this:

  ;; uncomment to test original proposed workaround
  ;; (setq gnutls-algorithm-priority "NORMAL:%COMPAT")
  (setq gnutls-log-level 2)
  (url-retrieve "https://mirror.hydra.gnu.org/"
                #'(lambda (status)
                    (message "success")))

and evaluating that last form a number of times.  Not very scientific :P
I was unable to reproduce the problem though.

Andy

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Fri, 10 Feb 2017 16:51:39 +0100 Andy Wingo <wingo@igalia.com> wrote: 

AW> I tried checking (had to remember what I was doing to begin with!) and
AW> was not able to reproduce the original problem, and therefore couldn't
AW> test NORMAL:%COMPAT or NORMAL:%DUMBFW :/  Sorry :/

AW> I was trying to just do this:

AW>   ;; uncomment to test original proposed workaround
AW>   ;; (setq gnutls-algorithm-priority "NORMAL:%COMPAT")
AW>   (setq gnutls-log-level 2)
AW>   (url-retrieve "https://mirror.hydra.gnu.org/"
AW>                 #'(lambda (status)
AW>                     (message "success")))

AW> and evaluating that last form a number of times.  Not very scientific :P
AW> I was unable to reproduce the problem though.

Thanks, Andy.

We were just talking with Michael about connection-specific settings;
this is a perfect use case. It will be one of the first things we use
for testing. So that will resolve the need for per-connection
adjustments, and we can focus on just the default value.

Does anyone think we should add %COMPAT or %DUMBFW to the default
priority string? Without definitive proof that it will help, I'm not
sure we should, but I'm open to comments. Either way, we'll document it.

Thanks
Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Eli Zaretskii]

> From: Ted Zlatanov <tzz@lifelogs.com>
> Gmane-Reply-To-List: yes
> Date: Mon, 13 Feb 2017 11:04:55 -0500
> Cc: 25061@debbugs.gnu.org, Ludovic Courtès <ludo@gnu.org>,
>  Lars Ingebrigtsen <larsi@gnus.org>
> 
> On Fri, 10 Feb 2017 16:51:39 +0100 Andy Wingo <wingo@igalia.com> wrote: 
> 
> AW> I tried checking (had to remember what I was doing to begin with!) and
> AW> was not able to reproduce the original problem, and therefore couldn't
> AW> test NORMAL:%COMPAT or NORMAL:%DUMBFW :/  Sorry :/
> 
> AW> I was trying to just do this:
> 
> AW>   ;; uncomment to test original proposed workaround
> AW>   ;; (setq gnutls-algorithm-priority "NORMAL:%COMPAT")
> AW>   (setq gnutls-log-level 2)
> AW>   (url-retrieve "https://mirror.hydra.gnu.org/"
> AW>                 #'(lambda (status)
> AW>                     (message "success")))
> 
> AW> and evaluating that last form a number of times.  Not very scientific :P
> AW> I was unable to reproduce the problem though.
> 
> Thanks, Andy.
> 
> We were just talking with Michael about connection-specific settings;
> this is a perfect use case. It will be one of the first things we use
> for testing. So that will resolve the need for per-connection
> adjustments, and we can focus on just the default value.
> 
> Does anyone think we should add %COMPAT or %DUMBFW to the default
> priority string? Without definitive proof that it will help, I'm not
> sure we should, but I'm open to comments. Either way, we'll document it.

Any progress on this one, Ted?  This bug currently blocks the release
of Emacs 26.1, so could we please expedite its resolution, whatever
that is?

Thanks.

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Sat, 02 Sep 2017 16:49:20 +0300 Eli Zaretskii <eliz@gnu.org> wrote: 

>> From: Ted Zlatanov <tzz@lifelogs.com>
>> Date: Mon, 13 Feb 2017 11:04:55 -0500
>> 
>> On Fri, 10 Feb 2017 16:51:39 +0100 Andy Wingo <wingo@igalia.com> wrote: 
>> 
AW> I tried checking (had to remember what I was doing to begin with!) and
AW> was not able to reproduce the original problem, and therefore couldn't
AW> test NORMAL:%COMPAT or NORMAL:%DUMBFW :/  Sorry :/
>> 
AW> I was trying to just do this:
>> 
AW> ;; uncomment to test original proposed workaround
AW> ;; (setq gnutls-algorithm-priority "NORMAL:%COMPAT")
AW> (setq gnutls-log-level 2)
AW> (url-retrieve "https://mirror.hydra.gnu.org/"
AW> #'(lambda (status)
AW> (message "success")))
>> 
AW> and evaluating that last form a number of times.  Not very scientific :P
AW> I was unable to reproduce the problem though.
>> 
>> Thanks, Andy.
>> 
>> We were just talking with Michael about connection-specific settings;
>> this is a perfect use case. It will be one of the first things we use
>> for testing. So that will resolve the need for per-connection
>> adjustments, and we can focus on just the default value.
>> 
>> Does anyone think we should add %COMPAT or %DUMBFW to the default
>> priority string? Without definitive proof that it will help, I'm not
>> sure we should, but I'm open to comments. Either way, we'll document it.

EZ> Any progress on this one, Ted?  This bug currently blocks the release
EZ> of Emacs 26.1, so could we please expedite its resolution, whatever
EZ> that is?

Unfortunately I wasn't able to get to the connection-specific settings,
so right now we have to make these changes globally.

We've had no followup on this from anyone else and it's not easily
reproducible. Using %COMPAT for everyone could open them to old
vulnerabilities.

I'd rather stay with the current defaults and defer the rest of the work
to when connection-specific settings are available. I'm not sure of the
right place to discuss these settings--maybe a new section will be
needed once connection-specific settings exist.

So that's my vote; please add yours.

Thanks
Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Michael Albinus]

Ted Zlatanov <tzz@lifelogs.com> writes:

Hi Ted,

> I'd rather stay with the current defaults and defer the rest of the work
> to when connection-specific settings are available. I'm not sure of the
> right place to discuss these settings--maybe a new section will be
> needed once connection-specific settings exist.

???

Connection-local variables have landed in Emacs last November, reworked
in February. What do you miss?

> Thanks
> Ted

Best regards, Michael.

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Thu, 07 Sep 2017 09:18:44 +0200 Michael Albinus <michael.albinus@gmx.de> wrote: 

MA> Connection-local variables have landed in Emacs last November, reworked
MA> in February. What do you miss?

Sadly, time.

Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Eli Zaretskii]

> From: Ted Zlatanov <tzz@lifelogs.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  wingo@igalia.com,  25061@debbugs.gnu.org,  ludo@gnu.org,  larsi@gnus.org
> Date: Thu, 14 Sep 2017 17:11:53 -0400
> 
> On Thu, 07 Sep 2017 09:18:44 +0200 Michael Albinus <michael.albinus@gmx.de> wrote: 
> 
> MA> Connection-local variables have landed in Emacs last November, reworked
> MA> in February. What do you miss?
> 
> Sadly, time.

No worries, there's still plenty of time before 26.1 will be close to
release.  I hope you will find time until then to take care of this
issue.

Thanks.

bug#25061: consider adding %COMPAT to default gnutls priority string

[Eli Zaretskii]

> Date: Fri, 15 Sep 2017 09:05:14 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: wingo@igalia.com, 25061@debbugs.gnu.org, ludo@gnu.org,
>  michael.albinus@gmx.de, larsi@gnus.org
> 
> > On Thu, 07 Sep 2017 09:18:44 +0200 Michael Albinus <michael.albinus@gmx.de> wrote: 
> > 
> > MA> Connection-local variables have landed in Emacs last November, reworked
> > MA> in February. What do you miss?
> > 
> > Sadly, time.
> 
> No worries, there's still plenty of time before 26.1 will be close to
> release.  I hope you will find time until then to take care of this
> issue.

Ted, any news on this?  Emacs 26.1 is getting closer to the release,
so I'd like to see this issue resolved.

Thanks.

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Sat, 02 Dec 2017 19:36:16 +0200 Eli Zaretskii <eliz@gnu.org> wrote: 

EZ> Ted, any news on this?  Emacs 26.1 is getting closer to the release,
EZ> so I'd like to see this issue resolved.

I've looked at the code and at the work that Michael has kindly done on
connection-local variables and profiles.

Eli, first, I need to know if I can make large changes (introducing
connection profiles to GnuTLS) this close to the release. If so, I'll
work in emacs-26.

If we can't, I'll do this work for 26.2 and work in master.

To help you and others gauge the extent of the work, here's a summary:

* support connection profiles for processes, not just buffers (only
  buffers are supported today AFAICT). Or maybe only support processes
  with associated buffers. I'm not sure what's best, maybe Michael and
  others can make suggestions.

* only apply connection profiles when the connection is created. Users
  and applications will be responsible for closing the connection and
  reopening it if the profile changes.

* using `connection-local-set-profile-variables' in gnutls.el, create a
  'gnutls-default-profile with today's settings for
  `gnutls-min-prime-bits', `gnutls-trustfiles', `gnutls-verify-error',
  and `gnutls-algorithm-priority', installing it like so

          (connection-local-set-profiles
            nil
            'gnutls-default-profile)

* using `connection-local-set-profile-variables' in gnutls.el, create a
  'gnutls-compatible-profile with `gnutls-algorithm-priority' containing
  "%COMPAT" and any other needed changes to resolve this bug. Note that
  the default profile will be applied first, so this profile will be
  fairly small. The user will then need to do

          (connection-local-set-profiles
            '(:machine "system-that-needs-compatibility")
            'gnutls-compatible-profile)

* apply connection profiles in `open-network-stream',
  `open-gnutls-stream', and `gnutls-negotiate' as needed. The parameters
  will be :machine (host parameter) and :protocol (service parameter).
  Any other parameters such as :user and :application will be used if
  the application passes them in (so a new optional search criteria
  parameter will need to be added).

* add logging to make it clear to the user what profiles are getting
  applied, and what the final variable values are. This may deserve
  special UI if we can integrate it with the NSM (optional work, Lars
  and others can recommend what's best).

* support connection profiles for the network-security-level and other
  NSM variables as well (optional work, Lars and others can recommend
  what's best).

I think this covers what's needed. Let me know your thoughts and I hope
to wrap this up quickly either way.

Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Eli Zaretskii]

> From: Ted Zlatanov <tzz@lifelogs.com>
> Cc: wingo@igalia.com,  25061@debbugs.gnu.org,  ludo@gnu.org,  michael.albinus@gmx.de,  larsi@gnus.org
> Date: Sat, 09 Dec 2017 18:50:08 -0500
> 
> I've looked at the code and at the work that Michael has kindly done on
> connection-local variables and profiles.
> 
> Eli, first, I need to know if I can make large changes (introducing
> connection profiles to GnuTLS) this close to the release. If so, I'll
> work in emacs-26.
> 
> If we can't, I'll do this work for 26.2 and work in master.

Is everything you proposed strictly needed to fix the original
problem?  It sounds like a lot of changes just for having the
equivalent of '(setq gnutls-algorithm-priority "NORMAL:%DUMBFW")'
setting in effect.  E.g., could we simply make the above the default
value of gnutls-algorithm-priority for Emacs 26.1, and do all the rest
on master?

Previously, you said:

> I'd rather stay with the current defaults and defer the rest of the work
> to when connection-specific settings are available.

but you didn't explain why you didn't like changing the defaults.  If
that's because you thought it wasn't very clean, then would such a
change be okay just as a temporary measure, to be replaced by a
cleaner solution in Emacs 27?  Or can this default hurt something?

Thanks.

bug#25061: consider adding %COMPAT to default gnutls priority string

[Michael Albinus]

Ted Zlatanov <tzz@lifelogs.com> writes:

Hi Ted,

> * support connection profiles for processes, not just buffers (only
>   buffers are supported today AFAICT). Or maybe only support processes
>   with associated buffers. I'm not sure what's best, maybe Michael and
>   others can make suggestions.

Connection-local variables are implemented as buffer-local variables. I
believe best would be if you apply them to the related process buffer.

> Ted

Best regards, Michael.

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Sun, 10 Dec 2017 09:04:30 +0200 Eli Zaretskii <eliz@gnu.org> wrote: 

EZ> Is everything you proposed strictly needed to fix the original
EZ> problem?  It sounds like a lot of changes just for having the
EZ> equivalent of '(setq gnutls-algorithm-priority "NORMAL:%DUMBFW")'
EZ> setting in effect.

It's a plan to provide a facility that can solve the underlying problem
of heterogeneous network setups.

EZ> E.g., could we simply make the above the default
EZ> value of gnutls-algorithm-priority for Emacs 26.1, and do all the rest
EZ> on master?

Yes, let's.

EZ> Previously, you said:

>> I'd rather stay with the current defaults and defer the rest of the work
>> to when connection-specific settings are available.

EZ> but you didn't explain why you didn't like changing the defaults.  If
EZ> that's because you thought it wasn't very clean, then would such a
EZ> change be okay just as a temporary measure, to be replaced by a
EZ> cleaner solution in Emacs 27?  Or can this default hurt something?

It would change behavior for everyone for the sake of fixing a few
setups. Does %DUMBFW or %COMPAT create a risk that's not justified by
the functionality it provides? These exceptions have a way of living
long past their expiration date.

If we're confident that's the right thing, then let's change it in the
release and add a note in the docs. I'm OK with the change; any other
comments? What should be the actual string?

Regardless, I'll make the connection profile changes in master, which
gives us more time to find problems and provide a better integration for
them. I'll discuss it in emacs-devel. Thank you for your consideration
and help.

Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Eli Zaretskii]

> From: Ted Zlatanov <tzz@lifelogs.com>
> Cc: wingo@igalia.com,  25061@debbugs.gnu.org,  ludo@gnu.org,  michael.albinus@gmx.de,  larsi@gnus.org
> Date: Sun, 10 Dec 2017 08:29:27 -0500
> 
> It would change behavior for everyone for the sake of fixing a few
> setups. Does %DUMBFW or %COMPAT create a risk that's not justified by
> the functionality it provides? These exceptions have a way of living
> long past their expiration date.
> 
> If we're confident that's the right thing, then let's change it in the
> release and add a note in the docs. I'm OK with the change; any other
> comments? What should be the actual string?

You mean, should we use %COMPAT or %DUMBFW?  I think the latter.  But
if no one can reproduce the problem and verify the fix, I think we
should simply describe the problem in PROBLEMS and leave the code
intact.

> Regardless, I'll make the connection profile changes in master, which
> gives us more time to find problems and provide a better integration for
> them.

Sounds good, thanks.

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Sun, 10 Dec 2017 16:12:20 +0200 Eli Zaretskii <eliz@gnu.org> wrote: 

>> From: Ted Zlatanov <tzz@lifelogs.com>
>> Cc: wingo@igalia.com,  25061@debbugs.gnu.org,  ludo@gnu.org,  michael.albinus@gmx.de,  larsi@gnus.org
>> Date: Sun, 10 Dec 2017 08:29:27 -0500
>> 
>> It would change behavior for everyone for the sake of fixing a few
>> setups. Does %DUMBFW or %COMPAT create a risk that's not justified by
>> the functionality it provides? These exceptions have a way of living
>> long past their expiration date.
>> 
>> If we're confident that's the right thing, then let's change it in the
>> release and add a note in the docs. I'm OK with the change; any other
>> comments? What should be the actual string?

EZ> You mean, should we use %COMPAT or %DUMBFW?  I think the latter.  But
EZ> if no one can reproduce the problem and verify the fix, I think we
EZ> should simply describe the problem in PROBLEMS and leave the code
EZ> intact.

The GnuTLS docs say it "will add a private extension with bogus data
that make the client hello exceed 512 bytes. This avoids a black hole
behavior in some firewalls. This is the [RFC7685] client hello padding
extension, also enabled with %COMPAT." https://gnutls.org/manual/html_node/Priority-Strings.html

To me this appears benign and without downsides.

Can anyone knowledgeable comment on any possible downsides to this? I'll
wait 3 days for objections, then make the change in emacs-26.

Thanks
Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Eli Zaretskii]

> From: Ted Zlatanov <tzz@lifelogs.com>
> Cc: wingo@igalia.com,  25061@debbugs.gnu.org,  ludo@gnu.org,  michael.albinus@gmx.de,  larsi@gnus.org
> Date: Mon, 11 Dec 2017 10:03:42 -0500
> 
> The GnuTLS docs say it "will add a private extension with bogus data
> that make the client hello exceed 512 bytes. This avoids a black hole
> behavior in some firewalls. This is the [RFC7685] client hello padding
> extension, also enabled with %COMPAT." https://gnutls.org/manual/html_node/Priority-Strings.html
> 
> To me this appears benign and without downsides.

Me too, but I don't consider myself an expert.  I'm okay with doing
that if you think it's safe and no one else objects.

> Can anyone knowledgeable comment on any possible downsides to this? I'll
> wait 3 days for objections, then make the change in emacs-26.

Sounds like a good plan, thanks.

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Mon, 11 Dec 2017 17:40:05 +0200 Eli Zaretskii <eliz@gnu.org> wrote: 

>> Can anyone knowledgeable comment on any possible downsides to this? I'll
>> wait 3 days for objections, then make the change in emacs-26.

EZ> Sounds like a good plan, thanks.

OK; done.

Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Philipp Stephani]

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

Ted Zlatanov <tzz@lifelogs.com> schrieb am Fr., 15. Dez. 2017 um 05:19 Uhr:

> On Mon, 11 Dec 2017 17:40:05 +0200 Eli Zaretskii <eliz@gnu.org> wrote:
>
> >> Can anyone knowledgeable comment on any possible downsides to this? I'll
> >> wait 3 days for objections, then make the change in emacs-26.
>
> EZ> Sounds like a good plan, thanks.
>
> OK; done.
>
>
>
This appears to break at least Ubuntu 14.04, where apparently GnuTLS
doesn't yet support DUMBFW.

[-- Attachment #2: Type: text/html, Size: 835 bytes --]

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Sat, 16 Dec 2017 23:25:15 +0000 Philipp Stephani <p.stephani2@gmail.com> wrote: 

PS> Ted Zlatanov <tzz@lifelogs.com> schrieb am Fr., 15. Dez. 2017 um 05:19 Uhr:
>> On Mon, 11 Dec 2017 17:40:05 +0200 Eli Zaretskii <eliz@gnu.org> wrote:
>> 
>> >> Can anyone knowledgeable comment on any possible downsides to this? I'll
>> >> wait 3 days for objections, then make the change in emacs-26.
>> 
EZ> Sounds like a good plan, thanks.
>> 
>> OK; done.
>> 
>> 
>> 
PS> This appears to break at least Ubuntu 14.04, where apparently GnuTLS
PS> doesn't yet support DUMBFW.

It seems we've uncovered some downsides. What's the actual user-visible error?

Considering 14.04 is pretty old, and AFAIK we can't check if %DUMBFW is
supported, we don't have good options:

1) undo the change (please don't just revert the commit)
2) document the breakage
3) start setting the default conditionally based on GnuTLS version (ugh)

Any other options?

Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Eli Zaretskii]

> From: Ted Zlatanov <tzz@lifelogs.com>
> Date: Sat, 16 Dec 2017 18:34:51 -0500
> Cc: wingo@igalia.com, 25061@debbugs.gnu.org, ludo@gnu.org,
> 	michael.albinus@gmx.de, larsi@gnus.org
> 
> Considering 14.04 is pretty old, and AFAIK we can't check if %DUMBFW is
> supported, we don't have good options:
> 
> 1) undo the change (please don't just revert the commit)
> 2) document the breakage
> 3) start setting the default conditionally based on GnuTLS version (ugh)
> 
> Any other options?

If no other ideas come up, I don't see 3) as such a terrible choice.

Thanks.

bug#25061: consider adding %COMPAT to default gnutls priority string

[Eli Zaretskii]

> Date: Sun, 17 Dec 2017 05:39:22 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: wingo@igalia.com, 25061@debbugs.gnu.org, ludo@gnu.org,
> 	p.stephani2@gmail.com, michael.albinus@gmx.de, larsi@gnus.org
> 
> > 1) undo the change (please don't just revert the commit)
> > 2) document the breakage
> > 3) start setting the default conditionally based on GnuTLS version (ugh)
> > 
> > Any other options?
> 
> If no other ideas come up, I don't see 3) as such a terrible choice.

Btw, you may wish to consider implementing the removal of %DUMBFW
inside gnutls.c, when the version of GnuTLS doesn't support that, this
could be cleaner on the Lisp level.

bug#25061: consider adding %COMPAT to default gnutls priority string

[Philipp Stephani]

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

Eli Zaretskii <eliz@gnu.org> schrieb am So., 17. Dez. 2017 um 04:40 Uhr:

> > From: Ted Zlatanov <tzz@lifelogs.com>
> > Date: Sat, 16 Dec 2017 18:34:51 -0500
> > Cc: wingo@igalia.com, 25061@debbugs.gnu.org, ludo@gnu.org,
> >       michael.albinus@gmx.de, larsi@gnus.org
> >
> > Considering 14.04 is pretty old, and AFAIK we can't check if %DUMBFW is
> > supported, we don't have good options:
> >
> > 1) undo the change (please don't just revert the commit)
> > 2) document the breakage
> > 3) start setting the default conditionally based on GnuTLS version (ugh)
> >
> > Any other options?
>
> If no other ideas come up, I don't see 3) as such a terrible choice.
>
>
Agreed, that seems like the cleanest solution.

[-- Attachment #2: Type: text/html, Size: 1423 bytes --]

bug#25061: consider adding %COMPAT to default gnutls priority string

[Kaushal Modi]

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

Hello,

I install packages from Org Elpa and Melpa on the fly on Travis CI. Travis
CI is a continuous integration service used by many GitHub projects and it
uses an image (trusty) that's based off Ubuntu 14.04.

They made an update to their Ubuntu 14.04 image last week, and after that
these errors started showing up when emacs tried to retrieve package
archives:

Full log:https://travis-ci.org/jwiegley/use-package/jobs/317613388

Relevant excerpt:
=====
Contacting host: elpa.gnu.org:443

gnutls.el: (err=[-50] The request is invalid.) boot: (:priority
NORMAL:%DUMBFW :hostname elpa.gnu.org :loglevel 0 :min-prime-bits 256
:trustfiles (/etc/ssl/certs/ca-certificates.crt) :crlfiles nil :keylist nil
:verify-flags nil :verify-error nil :callbacks nil)

Failed to download ‘gnu’ archive.

Contacting host: melpa.org:443

gnutls.el: (err=[-50] The request is invalid.) boot: (:priority
NORMAL:%DUMBFW :hostname melpa.org :loglevel 0 :min-prime-bits 256
:trustfiles (/etc/ssl/certs/ca-certificates.crt) :crlfiles nil :keylist nil
:verify-flags nil :verify-error nil :callbacks nil)

Failed to download ‘melpa’ archive.
Package refresh done.
=====

I got the packages from Melpa to install fine only after I switch from
https to http.

Is this gnutls.el error related to this bug? This was on the latest
emacs-26 build as of yesterday.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1903 bytes --]

bug#25061: consider adding %COMPAT to default gnutls priority string

[Philipp Stephani]

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

Kaushal Modi <kaushal.modi@gmail.com> schrieb am Mo., 18. Dez. 2017 um
18:22 Uhr:

> Hello,
>
> I install packages from Org Elpa and Melpa on the fly on Travis CI. Travis
> CI is a continuous integration service used by many GitHub projects and it
> uses an image (trusty) that's based off Ubuntu 14.04.
>
> They made an update to their Ubuntu 14.04 image last week, and after that
> these errors started showing up when emacs tried to retrieve package
> archives:
>
> Full log:https://travis-ci.org/jwiegley/use-package/jobs/317613388
>
> Relevant excerpt:
> =====
> Contacting host: elpa.gnu.org:443
>
> gnutls.el: (err=[-50] The request is invalid.) boot: (:priority
> NORMAL:%DUMBFW :hostname elpa.gnu.org :loglevel 0 :min-prime-bits 256
> :trustfiles (/etc/ssl/certs/ca-certificates.crt) :crlfiles nil :keylist nil
> :verify-flags nil :verify-error nil :callbacks nil)
>
> Failed to download ‘gnu’ archive.
>
> Contacting host: melpa.org:443
>
> gnutls.el: (err=[-50] The request is invalid.) boot: (:priority
> NORMAL:%DUMBFW :hostname melpa.org :loglevel 0 :min-prime-bits 256
> :trustfiles (/etc/ssl/certs/ca-certificates.crt) :crlfiles nil :keylist nil
> :verify-flags nil :verify-error nil :callbacks nil)
>
> Failed to download ‘melpa’ archive.
> Package refresh done.
> =====
>
> I got the packages from Melpa to install fine only after I switch from
> https to http.
>
> Is this gnutls.el error related to this bug? This was on the latest
> emacs-26 build as of yesterday.
>
>
Yes, this is the same thing – at least I got exactly the same error message
when running 'emacs -Q -f report-emacs-bug'.
Considering that a version check takes some time to implement, I'd suggest
reverting the commit that introduced %DUMBFW for now.

[-- Attachment #2: Type: text/html, Size: 2594 bytes --]

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Sun, 17 Dec 2017 17:17:41 +0200 Eli Zaretskii <eliz@gnu.org> wrote: 

EZ> Btw, you may wish to consider implementing the removal of %DUMBFW
EZ> inside gnutls.c, when the version of GnuTLS doesn't support that, this
EZ> could be cleaner on the Lisp level.

Emacs now collects all the GnuTLS extensions into the list returned by
`gnutls-available-p' and that's used by gnutls.el. I hope that's enough
to resolve this problem.

I don't think the C code should be modifying the priority string before
it's passed on. That can cause pretty bad problems.

Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Mon, 18 Dec 2017 17:16:17 +0000 Kaushal Modi <kaushal.modi@gmail.com> wrote: 

KM> gnutls.el: (err=[-50] The request is invalid.) boot: (:priority
KM> NORMAL:%DUMBFW :hostname melpa.org :loglevel 0 :min-prime-bits 256
KM> :trustfiles (/etc/ssl/certs/ca-certificates.crt) :crlfiles nil :keylist nil
KM> :verify-flags nil :verify-error nil :callbacks nil)

Please try again, I added a fix that should remove %DUMBFW if it's not
supported.

Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Kaushal Modi]

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

On Tue, Dec 19, 2017 at 12:47 PM Ted Zlatanov <tzz@lifelogs.com> wrote:

>
> Please try again, I added a fix that should remove %DUMBFW if it's not
> supported.
>

Hi Ted,

I couldn't test the fix on my local machine because I cannot recreate the
problem there.

So I was waiting for the emacs-26 branch build to finish on the Travis CI
set up by Noam.. but the build itself failed:
https://travis-ci.org/npostavs/emacs-travis/jobs/318857869

  CC       lastfile.o
  CCLD     temacs
gnutls.o: In function `Fgnutls_available_p':
gnutls.c:(.text+0x105d): undefined reference to `gnutls_ext_get_name'
collect2: error: ld returned 1 exit status
make[1]: *** [temacs] Error 1
make[1]: Leaving directory `/tmp/emacs-emacs-26/src'
make: *** [src] Error 2
make: Leaving directory `/tmp/emacs-emacs-26'


-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1422 bytes --]

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Tue, 19 Dec 2017 22:00:34 +0000 Kaushal Modi <kaushal.modi@gmail.com> wrote: 

KM> I couldn't test the fix on my local machine because I cannot recreate the
KM> problem there.

KM> So I was waiting for the emacs-26 branch build to finish on the Travis CI
KM> set up by Noam.. but the build itself failed:
KM> https://travis-ci.org/npostavs/emacs-travis/jobs/318857869

KM>   CC       lastfile.o
KM>   CCLD     temacs
KM> gnutls.o: In function `Fgnutls_available_p':
KM> gnutls.c:(.text+0x105d): undefined reference to `gnutls_ext_get_name'
KM> collect2: error: ld returned 1 exit status
KM> make[1]: *** [temacs] Error 1
KM> make[1]: Leaving directory `/tmp/emacs-emacs-26/src'
KM> make: *** [src] Error 2
KM> make: Leaving directory `/tmp/emacs-emacs-26'

I don't understand why. This function has been available since GnuTLS
was created, according to the docs. Do we know what GnuTLS version is used
in Travis CI?

Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Robert Pluim]

Ted Zlatanov <tzz@lifelogs.com> writes:

> On Tue, 19 Dec 2017 22:00:34 +0000 Kaushal Modi <kaushal.modi@gmail.com> wrote: 
>
> KM> I couldn't test the fix on my local machine because I cannot recreate the
> KM> problem there.
>
> KM> So I was waiting for the emacs-26 branch build to finish on the Travis CI
> KM> set up by Noam.. but the build itself failed:
> KM> https://travis-ci.org/npostavs/emacs-travis/jobs/318857869
>
> KM>   CC       lastfile.o
> KM>   CCLD     temacs
> KM> gnutls.o: In function `Fgnutls_available_p':
> KM> gnutls.c:(.text+0x105d): undefined reference to `gnutls_ext_get_name'
> KM> collect2: error: ld returned 1 exit status
> KM> make[1]: *** [temacs] Error 1
> KM> make[1]: Leaving directory `/tmp/emacs-emacs-26/src'
> KM> make: *** [src] Error 2
> KM> make: Leaving directory `/tmp/emacs-emacs-26'
>
> I don't understand why. This function has been available since GnuTLS
> was created, according to the docs. Do we know what GnuTLS version is used
> in Travis CI?

It fails for me here with GnuTLS-3.4.10. According to
https://lists.gnupg.org/pipermail/gnutls-help/2016-June/004129.html
that function was added in 3.5.1. Looking through the git repo it was
previously an internal function.

Robert

bug#25061: consider adding %COMPAT to default gnutls priority string

[Kaushal Modi]

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

On Tue, Dec 19, 2017, 8:09 PM Ted Zlatanov <tzz@lifelogs.com> wrote:

> Do we know what GnuTLS version is used
> in Travis CI?
>

Travis CI uses the trusty image, which is Ubuntu 14.04LTS. From
https://packages.ubuntu.com/trusty/gnutls-bin, it looks like it is
using libgnutls26 (>= 2.12.17-0).

> --

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 875 bytes --]

bug#25061: consider adding %COMPAT to default gnutls priority string

[Andy Moreton]

On Tue 19 Dec 2017, Ted Zlatanov wrote:

> On Tue, 19 Dec 2017 22:00:34 +0000 Kaushal Modi <kaushal.modi@gmail.com> wrote: 
>
> KM> I couldn't test the fix on my local machine because I cannot recreate the
> KM> problem there.
>
> KM> So I was waiting for the emacs-26 branch build to finish on the Travis CI
> KM> set up by Noam.. but the build itself failed:
> KM> https://travis-ci.org/npostavs/emacs-travis/jobs/318857869
>
> KM>   CC       lastfile.o
> KM>   CCLD     temacs
> KM> gnutls.o: In function `Fgnutls_available_p':
> KM> gnutls.c:(.text+0x105d): undefined reference to `gnutls_ext_get_name'
> KM> collect2: error: ld returned 1 exit status
> KM> make[1]: *** [temacs] Error 1
> KM> make[1]: Leaving directory `/tmp/emacs-emacs-26/src'
> KM> make: *** [src] Error 2
> KM> make: Leaving directory `/tmp/emacs-emacs-26'
>
> I don't understand why. This function has been available since GnuTLS
> was created, according to the docs. Do we know what GnuTLS version is used
> in Travis CI?

The Windows builds of emacs use runtime imports of GnuTLS functions, so
that emacs will still run on a system that does not have the required
DLLs installed.

The following patch fixes this:

diff --git a/src/gnutls.c b/src/gnutls.c
index 8db201ae83..f2b078d964 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -238,6 +238,7 @@ DEF_DLL_FN (int, gnutls_hash, (gnutls_hash_hd_t, const void *, size_t));
 DEF_DLL_FN (void, gnutls_hash_deinit, (gnutls_hash_hd_t, void *));
 DEF_DLL_FN (void, gnutls_hash_output, (gnutls_hash_hd_t, void *));
 #  endif        /* HAVE_GNUTLS3 */
+DEF_DLL_FN (const char *, gnutls_ext_get_name, (unsigned int));


 static bool
@@ -357,6 +358,7 @@ init_gnutls_functions (void)
   LOAD_DLL_FN (library, gnutls_hash_deinit);
   LOAD_DLL_FN (library, gnutls_hash_output);
 #  endif        /* HAVE_GNUTLS3 */
+  LOAD_DLL_FN (library, gnutls_ext_get_name);

   max_log_level = global_gnutls_log_level;

@@ -470,6 +472,8 @@ init_gnutls_functions (void)
 #  define gnutls_hash_deinit fn_gnutls_hash_deinit
 #  define gnutls_hash_output fn_gnutls_hash_output
 #  endif        /* HAVE_GNUTLS3 */
+#  define gnutls_ext_get_name fn_gnutls_ext_get_name

 /* This wrapper is called from fns.c, which doesn't know about the
    LOAD_DLL_FN stuff above.  */

bug#25061: consider adding %COMPAT to default gnutls priority string

[Andy Moreton]

On Wed 20 Dec 2017, Andy Moreton wrote:

> On Tue 19 Dec 2017, Ted Zlatanov wrote:
>
>> On Tue, 19 Dec 2017 22:00:34 +0000 Kaushal Modi <kaushal.modi@gmail.com> wrote: 
>>
>> KM> I couldn't test the fix on my local machine because I cannot recreate the
>> KM> problem there.
>>
>> KM> So I was waiting for the emacs-26 branch build to finish on the Travis CI
>> KM> set up by Noam.. but the build itself failed:
>> KM> https://travis-ci.org/npostavs/emacs-travis/jobs/318857869
>>
>> KM>   CC       lastfile.o
>> KM>   CCLD     temacs
>> KM> gnutls.o: In function `Fgnutls_available_p':
>> KM> gnutls.c:(.text+0x105d): undefined reference to `gnutls_ext_get_name'
>> KM> collect2: error: ld returned 1 exit status
>> KM> make[1]: *** [temacs] Error 1
>> KM> make[1]: Leaving directory `/tmp/emacs-emacs-26/src'
>> KM> make: *** [src] Error 2
>> KM> make: Leaving directory `/tmp/emacs-emacs-26'
>>
>> I don't understand why. This function has been available since GnuTLS
>> was created, according to the docs. Do we know what GnuTLS version is used
>> in Travis CI?
>
> The Windows builds of emacs use runtime imports of GnuTLS functions, so
> that emacs will still run on a system that does not have the required
> DLLs installed.

Please ignore the previous patch - this version has been tested:

diff --git a/src/gnutls.c b/src/gnutls.c
index 8db201ae83..acea77ba32 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -238,6 +238,7 @@ DEF_DLL_FN (int, gnutls_hash, (gnutls_hash_hd_t, const void *, size_t));
 DEF_DLL_FN (void, gnutls_hash_deinit, (gnutls_hash_hd_t, void *));
 DEF_DLL_FN (void, gnutls_hash_output, (gnutls_hash_hd_t, void *));
 #  endif	 /* HAVE_GNUTLS3 */
+DEF_DLL_FN (const char *, gnutls_ext_get_name, (unsigned int));
 
 
 static bool
@@ -357,6 +358,7 @@ init_gnutls_functions (void)
   LOAD_DLL_FN (library, gnutls_hash_deinit);
   LOAD_DLL_FN (library, gnutls_hash_output);
 #  endif	 /* HAVE_GNUTLS3 */
+  LOAD_DLL_FN (library, gnutls_ext_get_name);
 
   max_log_level = global_gnutls_log_level;
 
@@ -470,6 +472,8 @@ init_gnutls_functions (void)
 #  define gnutls_hash_deinit fn_gnutls_hash_deinit
 #  define gnutls_hash_output fn_gnutls_hash_output
 #  endif	 /* HAVE_GNUTLS3 */
+#  define gnutls_ext_get_name fn_gnutls_ext_get_name
+
 
 /* This wrapper is called from fns.c, which doesn't know about the
    LOAD_DLL_FN stuff above.  */
@@ -2439,15 +2443,6 @@ Any GnuTLS extension with ID up to 100
   capabilities = Fcons (intern("macs"), capabilities);
 # endif	  /* HAVE_GNUTLS3 */
 
-  for (unsigned int ext=0; ext < 100; ext++)
-    {
-      const char* name = gnutls_ext_get_name(ext);
-      if (name != NULL)
-        {
-          capabilities = Fcons (intern(name), capabilities);
-        }
-    }
-
 # ifdef WINDOWSNT
   Lisp_Object found = Fassq (Qgnutls, Vlibrary_cache);
   if (CONSP (found))
@@ -2462,6 +2457,15 @@ Any GnuTLS extension with ID up to 100
 # endif /* WINDOWSNT */
 #endif	/* HAVE_GNUTLS */
 
+  for (unsigned int ext=0; ext < 100; ext++)
+    {
+      const char* name = gnutls_ext_get_name(ext);
+      if (name != NULL)
+        {
+          capabilities = Fcons (intern(name), capabilities);
+        }
+    }
+
   return capabilities;
 }
 

bug#25061: consider adding %COMPAT to default gnutls priority string

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Date: Wed, 20 Dec 2017 12:41:08 +0100
> Cc: Noam Postavsky <npostavs@users.sourceforge.net>, wingo@igalia.com,
> 	25061@debbugs.gnu.org, ludo@gnu.org, p.stephani2@gmail.com,
> 	michael.albinus@gmx.de, larsi@gnus.org
> 
> > I don't understand why. This function has been available since GnuTLS
> > was created, according to the docs. Do we know what GnuTLS version is used
> > in Travis CI?
> 
> It fails for me here with GnuTLS-3.4.10. According to
> https://lists.gnupg.org/pipermail/gnutls-help/2016-June/004129.html
> that function was added in 3.5.1. Looking through the git repo it was
> previously an internal function.

This means we cannot use this method without some fallback for systems
with older GnuTLS, as 3.5.X is still not old enough to assume it's
available widely enough.  Could we perhaps use a fixed list of
features for older GnuTLS versions?

Ted, could you please fix this ASAP?  It currently prevents the
release branch from building on many systems.

bug#25061: consider adding %COMPAT to default gnutls priority string

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Wed, 20 Dec 2017 12:54:45 +0000
> 
> diff --git a/src/gnutls.c b/src/gnutls.c
> index 8db201ae83..f2b078d964 100644
> --- a/src/gnutls.c
> +++ b/src/gnutls.c
> @@ -238,6 +238,7 @@ DEF_DLL_FN (int, gnutls_hash, (gnutls_hash_hd_t, const void *, size_t));
>  DEF_DLL_FN (void, gnutls_hash_deinit, (gnutls_hash_hd_t, void *));
>  DEF_DLL_FN (void, gnutls_hash_output, (gnutls_hash_hd_t, void *));
>  #  endif        /* HAVE_GNUTLS3 */
> +DEF_DLL_FN (const char *, gnutls_ext_get_name, (unsigned int));
> 
> 
>  static bool
> @@ -357,6 +358,7 @@ init_gnutls_functions (void)
>    LOAD_DLL_FN (library, gnutls_hash_deinit);
>    LOAD_DLL_FN (library, gnutls_hash_output);
>  #  endif        /* HAVE_GNUTLS3 */
> +  LOAD_DLL_FN (library, gnutls_ext_get_name);
> 
>    max_log_level = global_gnutls_log_level;
> 
> @@ -470,6 +472,8 @@ init_gnutls_functions (void)
>  #  define gnutls_hash_deinit fn_gnutls_hash_deinit
>  #  define gnutls_hash_output fn_gnutls_hash_output
>  #  endif        /* HAVE_GNUTLS3 */
> +#  define gnutls_ext_get_name fn_gnutls_ext_get_name

Thanks, but this means Emacs will now refuse to load GnuTLS on systems
that have version 3.4 or older of the library, right?  If so, I think
the loading and the use of the function should be conditioned by an
appropriate preprocessor directive to compile that code only for
3.5.X.

bug#25061: consider adding %COMPAT to default gnutls priority string

[Andy Moreton]

On Wed 20 Dec 2017, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@gmail.com>
>> Date: Wed, 20 Dec 2017 12:54:45 +0000
>> 
>> diff --git a/src/gnutls.c b/src/gnutls.c
>> index 8db201ae83..f2b078d964 100644
>> --- a/src/gnutls.c
>> +++ b/src/gnutls.c
>> @@ -238,6 +238,7 @@ DEF_DLL_FN (int, gnutls_hash, (gnutls_hash_hd_t, const void *, size_t));
>>  DEF_DLL_FN (void, gnutls_hash_deinit, (gnutls_hash_hd_t, void *));
>>  DEF_DLL_FN (void, gnutls_hash_output, (gnutls_hash_hd_t, void *));
>>  #  endif        /* HAVE_GNUTLS3 */
>> +DEF_DLL_FN (const char *, gnutls_ext_get_name, (unsigned int));
>> 
>> 
>>  static bool
>> @@ -357,6 +358,7 @@ init_gnutls_functions (void)
>>    LOAD_DLL_FN (library, gnutls_hash_deinit);
>>    LOAD_DLL_FN (library, gnutls_hash_output);
>>  #  endif        /* HAVE_GNUTLS3 */
>> +  LOAD_DLL_FN (library, gnutls_ext_get_name);
>> 
>>    max_log_level = global_gnutls_log_level;
>> 
>> @@ -470,6 +472,8 @@ init_gnutls_functions (void)
>>  #  define gnutls_hash_deinit fn_gnutls_hash_deinit
>>  #  define gnutls_hash_output fn_gnutls_hash_output
>>  #  endif        /* HAVE_GNUTLS3 */
>> +#  define gnutls_ext_get_name fn_gnutls_ext_get_name
>
> Thanks, but this means Emacs will now refuse to load GnuTLS on systems
> that have version 3.4 or older of the library, right?  If so, I think
> the loading and the use of the function should be conditioned by an
> appropriate preprocessor directive to compile that code only for
> 3.5.X.

Agreed. This version also fixes a bug in the previous version, where
the capabilities stored in Vlibrary_cache were not updated properly.


diff --git a/src/gnutls.c b/src/gnutls.c
index 8db201ae83..48ea25397a 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -46,6 +46,10 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 # define HAVE_GNUTLS_MAC_GET_NONCE_SIZE
 #endif
 
+#if GNUTLS_VERSION_NUMBER >= 0x030501
+# define HAVE_GNUTLS_EXT_GET_NAME
+#endif
+
 #ifdef HAVE_GNUTLS
 
 # ifdef WINDOWSNT
@@ -237,6 +241,9 @@ DEF_DLL_FN (int, gnutls_hash_get_len, (gnutls_digest_algorithm_t));
 DEF_DLL_FN (int, gnutls_hash, (gnutls_hash_hd_t, const void *, size_t));
 DEF_DLL_FN (void, gnutls_hash_deinit, (gnutls_hash_hd_t, void *));
 DEF_DLL_FN (void, gnutls_hash_output, (gnutls_hash_hd_t, void *));
+#   ifdef HAVE_GNUTLS_EXT_GET_NAME
+DEF_DLL_FN (const char *, gnutls_ext_get_name, (unsigned int));
+#   endif
 #  endif	 /* HAVE_GNUTLS3 */
 
 
@@ -356,6 +363,9 @@ init_gnutls_functions (void)
   LOAD_DLL_FN (library, gnutls_hash);
   LOAD_DLL_FN (library, gnutls_hash_deinit);
   LOAD_DLL_FN (library, gnutls_hash_output);
+#   ifdef HAVE_GNUTLS_EXT_GET_NAME
+  LOAD_DLL_FN (library, gnutls_ext_get_name);
+#   endif
 #  endif	 /* HAVE_GNUTLS3 */
 
   max_log_level = global_gnutls_log_level;
@@ -469,8 +479,12 @@ init_gnutls_functions (void)
 #  define gnutls_hash fn_gnutls_hash
 #  define gnutls_hash_deinit fn_gnutls_hash_deinit
 #  define gnutls_hash_output fn_gnutls_hash_output
+#   ifdef HAVE_GNUTLS_EXT_GET_NAME
+#    define gnutls_ext_get_name fn_gnutls_ext_get_name
+#   endif
 #  endif	 /* HAVE_GNUTLS3 */
 
+
 /* This wrapper is called from fns.c, which doesn't know about the
    LOAD_DLL_FN stuff above.  */
 int
@@ -2425,6 +2439,18 @@ Any GnuTLS extension with ID up to 100
 
 #ifdef HAVE_GNUTLS
 
+# ifdef WINDOWSNT
+  Lisp_Object found = Fassq (Qgnutls, Vlibrary_cache);
+  if (CONSP (found))
+    return XCDR (found);
+
+  /* Load the GnuTLS DLL and find exported functions.  The external
+     library cache is updated after the capabilities have been
+     determined.  */
+  if (!init_gnutls_functions ())
+    return Qnil;
+# endif /* WINDOWSNT */
+
   capabilities = Fcons (intern("gnutls"), capabilities);
 
 # ifdef HAVE_GNUTLS3
@@ -2437,8 +2463,8 @@ Any GnuTLS extension with ID up to 100
 #  endif
 
   capabilities = Fcons (intern("macs"), capabilities);
-# endif	  /* HAVE_GNUTLS3 */
 
+#  ifdef HAVE_GNUTLS_EXT_GET_NAME
   for (unsigned int ext=0; ext < 100; ext++)
     {
       const char* name = gnutls_ext_get_name(ext);
@@ -2447,18 +2473,11 @@ Any GnuTLS extension with ID up to 100
           capabilities = Fcons (intern(name), capabilities);
         }
     }
+#  endif
+# endif	  /* HAVE_GNUTLS3 */
 
 # ifdef WINDOWSNT
-  Lisp_Object found = Fassq (Qgnutls, Vlibrary_cache);
-  if (CONSP (found))
-    return XCDR (found);
-  else
-    {
-      Lisp_Object status;
-      status = init_gnutls_functions () ? capabilities : Qnil;
-      Vlibrary_cache = Fcons (Fcons (Qgnutls, status), Vlibrary_cache);
-      return status;
-    }
+  Vlibrary_cache = Fcons (Fcons (Qgnutls, capabilities), Vlibrary_cache);
 # endif /* WINDOWSNT */
 #endif	/* HAVE_GNUTLS */
 

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Wed, 20 Dec 2017 18:19:10 +0200 Eli Zaretskii <eliz@gnu.org> wrote: 

EZ> Thanks, but this means Emacs will now refuse to load GnuTLS on systems
EZ> that have version 3.4 or older of the library, right?  If so, I think
EZ> the loading and the use of the function should be conditioned by an
EZ> appropriate preprocessor directive to compile that code only for
EZ> 3.5.X.

I've applied Andy's updated patch for GnuTLS 3.5.1 and older, and
hard-coded compatibility for %DUMBFW between 3.2.5 when it was
introduced and 3.5.1.

Looking forward to closing this one for good... thanks.

Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Wed, 20 Dec 2017 20:15:48 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> On Wed, 20 Dec 2017 18:19:10 +0200 Eli Zaretskii <eliz@gnu.org> wrote: 
EZ> Thanks, but this means Emacs will now refuse to load GnuTLS on systems
EZ> that have version 3.4 or older of the library, right?  If so, I think
EZ> the loading and the use of the function should be conditioned by an
EZ> appropriate preprocessor directive to compile that code only for
EZ> 3.5.X.

TZ> I've applied Andy's updated patch for GnuTLS 3.5.1 and older, and
TZ> hard-coded compatibility for %DUMBFW between 3.2.5 when it was
TZ> introduced and 3.5.1.

TZ> Looking forward to closing this one for good... thanks.

Noah's Travis CI build passed. I'll wait for Andy, Eli, and Kaushal to
confirm before marking this as done.

Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Robert Pluim]

Ted Zlatanov <tzz@lifelogs.com> writes:

> On Wed, 20 Dec 2017 20:15:48 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 
>
> TZ> On Wed, 20 Dec 2017 18:19:10 +0200 Eli Zaretskii <eliz@gnu.org> wrote: 
> EZ> Thanks, but this means Emacs will now refuse to load GnuTLS on systems
> EZ> that have version 3.4 or older of the library, right?  If so, I think
> EZ> the loading and the use of the function should be conditioned by an
> EZ> appropriate preprocessor directive to compile that code only for
> EZ> 3.5.X.
>
> TZ> I've applied Andy's updated patch for GnuTLS 3.5.1 and older, and
> TZ> hard-coded compatibility for %DUMBFW between 3.2.5 when it was
> TZ> introduced and 3.5.1.
>
> TZ> Looking forward to closing this one for good... thanks.
>
> Noah's Travis CI build passed. I'll wait for Andy, Eli, and Kaushal to
> confirm before marking this as done.

emacs-26 builds for me now. BTW Ted, did you want to put the DUMBFW
compatibility inside the HAVE_GNUTLS3 conditional?
HAVE_GNUTLS_EXT__DUMBFW implies HAVE_GNUTLS3, but it's currently used
on its own.

Regards

Robert

bug#25061: consider adding %COMPAT to default gnutls priority string

[Andy Moreton]

On Wed 20 Dec 2017, Ted Zlatanov wrote:

> On Wed, 20 Dec 2017 18:19:10 +0200 Eli Zaretskii <eliz@gnu.org> wrote: 
>
> EZ> Thanks, but this means Emacs will now refuse to load GnuTLS on systems
> EZ> that have version 3.4 or older of the library, right?  If so, I think
> EZ> the loading and the use of the function should be conditioned by an
> EZ> appropriate preprocessor directive to compile that code only for
> EZ> 3.5.X.
>
> I've applied Andy's updated patch for GnuTLS 3.5.1 and older, and
> hard-coded compatibility for %DUMBFW between 3.2.5 when it was
> introduced and 3.5.1.

Looks good. I have papers on file, so the "tiny change" note was not
needed.

> Looking forward to closing this one for good... thanks.

Let's hope so.

    AndyM

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Thu, 21 Dec 2017 09:30:41 +0100 Robert Pluim <rpluim@gmail.com> wrote: 

RP> emacs-26 builds for me now. BTW Ted, did you want to put the DUMBFW
RP> compatibility inside the HAVE_GNUTLS3 conditional?
RP> HAVE_GNUTLS_EXT__DUMBFW implies HAVE_GNUTLS3, but it's currently used
RP> on its own.

It's pinned to a version over 3 so I think that's OK. Makes no
difference either way, so if someone wants to move it, feel free.

Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Kaushal Modi]

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

Thank you all. This fixed the emacs-26 branch. But the master branch is
still broken (not broken build but broken https fetches):
https://travis-ci.org/kaushalmodi/ox-hugo/jobs/319693934

The emacs-26 branch will be merged into master at some point, but I am just
saying.

On Wed, Dec 20, 2017 at 11:14 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Robert Pluim <rpluim@gmail.com>
> > Date: Wed, 20 Dec 2017 12:41:08 +0100
> > Cc: Noam Postavsky <npostavs@users.sourceforge.net>, wingo@igalia.com,
> >       25061@debbugs.gnu.org, ludo@gnu.org, p.stephani2@gmail.com,
> >       michael.albinus@gmx.de, larsi@gnus.org
> >
> > > I don't understand why. This function has been available since GnuTLS
> > > was created, according to the docs. Do we know what GnuTLS version is
> used
> > > in Travis CI?
> >
> > It fails for me here with GnuTLS-3.4.10. According to
> > https://lists.gnupg.org/pipermail/gnutls-help/2016-June/004129.html
> > that function was added in 3.5.1. Looking through the git repo it was
> > previously an internal function.
>
> This means we cannot use this method without some fallback for systems
> with older GnuTLS, as 3.5.X is still not old enough to assume it's
> available widely enough.  Could we perhaps use a fixed list of
> features for older GnuTLS versions?
>
> Ted, could you please fix this ASAP?  It currently prevents the
> release branch from building on many systems.
>
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 2623 bytes --]

bug#25061: consider adding %COMPAT to default gnutls priority string

[Ted Zlatanov]

On Thu, 21 Dec 2017 13:20:06 +0000 Kaushal Modi <kaushal.modi@gmail.com> wrote: 

KM> Thank you all. This fixed the emacs-26 branch. But the master branch is
KM> still broken (not broken build but broken https fetches):
KM> https://travis-ci.org/kaushalmodi/ox-hugo/jobs/319693934

KM> The emacs-26 branch will be merged into master at some point, but I am just
KM> saying.

Right. Marking this bug as done, since the merge is not part of it. Thank you.

Ted

bug#25061: consider adding %COMPAT to default gnutls priority string

[Eli Zaretskii]

> From: Ted Zlatanov <tzz@lifelogs.com>
> Cc: Andy Moreton <andrewjmoreton@gmail.com>,  Noam Postavsky <npostavs@users.sourceforge.net>,  wingo@igalia.com,  25061@debbugs.gnu.org,  ludo@gnu.org,  p.stephani2@gmail.com,  michael.albinus@gmx.de,  larsi@gnus.org
> Date: Wed, 20 Dec 2017 20:39:58 -0500
> 
> Noah's Travis CI build passed. I'll wait for Andy, Eli, and Kaushal to
> confirm before marking this as done.

The release branch builds OK now, thanks.  I've successfully visited
Savannah via HTTPS.  Should I test anything else to make sure GnuTLS
works as expected?

bug#25061: consider adding %COMPAT to default gnutls priority string

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Thu, 21 Dec 2017 10:54:32 +0000
> 
> I have papers on file, so the "tiny change" note was not
> needed.

Not to mention the fact that "tiny changes" should be marked by
"Copyright-paperwork-exempt: yes" instead (as described in
CONTRIBUTE).

Thanks.