Bug 172154 - [SOUP] Stop setting G_TLS_GNUTLS_PRIORITY
Summary: [SOUP] Stop setting G_TLS_GNUTLS_PRIORITY
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: Other
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
: 158785 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-05-15 18:14 PDT by Michael Catanzaro
Modified: 2019-09-28 12:35 PDT (History)
5 users (show)

See Also:


Attachments
Patch (3.63 KB, patch)
2017-05-15 18:22 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (4.06 KB, patch)
2019-09-20 13:18 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2017-05-15 18:14:21 PDT
Stop setting G_TLS_GNUTLS_PRIORITY in web process, because the network process is mandatory nowadays and the web process should not be performing any networking.

Currently some media elements bits that perform networking in the web process are probably affected by this, but that's a bug that should be handled separately.
Comment 1 Michael Catanzaro 2017-05-15 18:22:30 PDT
Created attachment 310202 [details]
Patch
Comment 2 Carlos Garcia Campos 2017-05-16 00:41:46 PDT
(In reply to Michael Catanzaro from comment #0)
> Stop setting G_TLS_GNUTLS_PRIORITY in web process, because the network
> process is mandatory nowadays and the web process should not be performing
> any networking.
> 
> Currently some media elements bits that perform networking in the web
> process are probably affected by this, but that's a bug that should be
> handled separately.

And the appcache manifest downloads.
Comment 3 Michael Catanzaro 2017-07-15 20:54:06 PDT
Comment on attachment 310202 [details]
Patch

So we cannot do this yet.
Comment 4 Michael Catanzaro 2019-09-20 13:10:25 PDT
(In reply to Michael Catanzaro from comment #3) 
> So we cannot do this yet.

I wonder why.

This is actually reducing security nowadays by defeating my attempts to change glib-networking's base priority so it needs to go.
Comment 5 Michael Catanzaro 2019-09-20 13:18:23 PDT
Created attachment 379264 [details]
Patch
Comment 6 Michael Catanzaro 2019-09-20 13:20:09 PDT
Downside of this is it will reenable RC4 for LTS distros still using GnuTLS 3.5 or earlier, but we shouldn't be reducing security for modern distros for the benefit of old ones.
Comment 7 Adrian Perez 2019-09-20 13:29:59 PDT
Comment on attachment 379264 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=379264&action=review

Just one question below…

> Source/WebKit/NetworkProcess/EntryPoint/unix/NetworkProcessMain.cpp:-46
> -    setenv("G_TLS_GNUTLS_PRIORITY", "NORMAL:%COMPAT:!VERS-SSL3.0:!ARCFOUR-128", 0);

Instead of weakening security for users of LTS distributions,
I would rather use:

  if (!gnutls_check_version_numeric(3, 6, 0))
      setenv("G_TLS_GNUTLS_PRIORITY", "NORMAL:%COMPAT:!VERS-SSL3.0:!ARCFOUR-128", 0);

Is there some reason to not use this suggestion?
Comment 8 Michael Catanzaro 2019-09-20 17:12:56 PDT
(In reply to Adrian Perez from comment #7)
> Is there some reason to not use this suggestion?

It's nice to remove yucky workarounds from the code.

I don't see many distros with old GnuTLS upgrading to our 2.26 anyway, and they should patch GnuTLS to adjust available ciphersuites regardless.
Comment 9 Adrian Perez 2019-09-22 06:58:39 PDT
(In reply to Michael Catanzaro from comment #8)
> (In reply to Adrian Perez from comment #7)
> > Is there some reason to not use this suggestion?
> 
> It's nice to remove yucky workarounds from the code.

Indeed, it is!

> I don't see many distros with old GnuTLS upgrading to our 2.26 anyway, and
> they should patch GnuTLS to adjust available ciphersuites regardless.

Fair enough, let's go ahead with this cleanup then 👍
Comment 10 Michael Catanzaro 2019-09-22 10:03:18 PDT
Also, stronger reason: WebKit cannot link directly to GnuTLS or know that it is using GnuTLS under the hood.
Comment 11 WebKit Commit Bot 2019-09-23 02:05:22 PDT
Comment on attachment 379264 [details]
Patch

Clearing flags on attachment: 379264

Committed r250217: <https://trac.webkit.org/changeset/250217>
Comment 12 WebKit Commit Bot 2019-09-23 02:05:24 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Adrian Perez 2019-09-28 12:35:47 PDT
*** Bug 158785 has been marked as a duplicate of this bug. ***