After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 782853 - glib-networking: Fails tests is system gnutls built without SSLv3
glib-networking: Fails tests is system gnutls built without SSLv3
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: network
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2017-05-20 05:14 UTC by Mart Raudsepp
Modified: 2018-01-07 21:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tls test-suite.log (4.50 KB, text/plain)
2017-05-20 05:14 UTC, Mart Raudsepp
  Details
Fallback tests should not expect SSLv3 to be available (3.72 KB, patch)
2018-01-04 19:42 UTC, Michael Catanzaro
committed Details | Review

Description Mart Raudsepp 2017-05-20 05:14:48 UTC
Created attachment 352199 [details]
tls test-suite.log

If system gnutls is built without SSLv3 support (--disable-ssl3-support), glib-networking fails tests. This setting is getting common with POODLE in the past, etc.
I'm not sure it makes sense to test SSLv3 fallback anymore, after bug 738633 and such?
Comment 1 Philip Withnall 2017-05-31 09:48:52 UTC
This is related to bug #782218, where the %LATEST_VERSION_RECORD atom was removed from the priority string.

Dan/Michael, does it make sense to drop some of the unit tests now?
Comment 2 Dan Winship 2017-05-31 13:22:22 UTC
(In reply to Philip Withnall from comment #1)
> This is related to bug #782218, where the %LATEST_VERSION_RECORD atom was
> removed from the priority string.

I don't think it is; the problem is that the test tries to force SSLv3, so it will fail if SSLv3 isn't available.

If something like g_tls_connection_get_connection_requirements() from bug 745637 landed then we could use that to test if SSLv3 was available and then skip the test if not.

I guess I'd be OK with tweaking it for now so that if the SSLv3 test fails, it calls g_test_skip() rather than actually failing it. (But the TLS1.0 fallback test still needs to pass no matter what.)
Comment 3 Philip Withnall 2018-01-04 12:33:03 UTC
Michael, it would be good to get this fixed for 2.56, since presumably more and more systems are going to be built without SSLv3 support as time goes on (since SSLv3 was prohibited in 2015).
Comment 4 Michael Catanzaro 2018-01-04 18:29:42 UTC
(In reply to Dan Winship from comment #2)
> I guess I'd be OK with tweaking it for now so that if the SSLv3 test fails,
> it calls g_test_skip() rather than actually failing it. (But the TLS1.0
> fallback test still needs to pass no matter what.)

Good idea.
Comment 5 Michael Catanzaro 2018-01-04 19:32:15 UTC
Currently we run the test twice, with two different priorities:

#define PRIORITY_SSL_FALLBACK "NORMAL:+VERS-SSL3.0"
#define PRIORITY_TLS_FALLBACK "NORMAL:+VERS-TLS-ALL:-VERS-SSL3.0"

PRIORITY_SSL_FALLBACK is guaranteed to be broken with newer GnuTLS, and PRIORITY_TLS_FALLBACK is effectively the default case, so neither is interesting. I think we should just not set custom priority strings for the test. That will avoid the problem entirely, and allow us to simplify the test by getting rid of the subprocess stuff. The priority strings are not supposed to be API anyway.
Comment 6 Michael Catanzaro 2018-01-04 19:42:37 UTC
Created attachment 366327 [details] [review]
Fallback tests should not expect SSLv3 to be available

Dan, what do you think? This removes a bunch of code that you added to be able to test different priority strings, so I'm not sure if you'll agree with this approach....
Comment 7 Dan Winship 2018-01-05 14:25:21 UTC
Comment on attachment 366327 [details] [review]
Fallback tests should not expect SSLv3 to be available

If we're going to continue to support :use-ssl3, then we should have the code to test it (even if that test can't be run on some machines). But if that's going to go away then obviously the tests can too.
Comment 8 Dan Winship 2018-01-05 14:26:19 UTC
(In reply to Dan Winship from comment #7)
> If we're going to continue to support :use-ssl3, then we should have the
> code to test it (even if that test can't be run on some machines).

Of course, "we can't test that the code still works" is also a good argument for not supporting it any more.
Comment 9 Michael Catanzaro 2018-01-05 17:54:49 UTC
My point is that there's no point in running the test under a special priority string. All we care is that the property performs as documented. It's documented to force the connection to use the lowest-supported version of SSL/TLS. We can't check if that actually works until we implement bug #745637 (which is still on my TODO list, I promise ;) so the best we can do is check that the connection succeeds, same as the current tests do.
Comment 10 Dan Winship 2018-01-07 16:42:31 UTC
ah, I just saw that it was deleting a bunch of code and thought you were getting rid of the whole test. Yeah, this makes sense.
Comment 11 Michael Catanzaro 2018-01-07 21:39:25 UTC
Nah, the test stays, but now it will run once instead of twice, and with the default priority string instead of with custom priority strings.
Comment 12 Michael Catanzaro 2018-01-07 21:41:24 UTC
Attachment 366327 [details] pushed as 806d703 - Fallback tests should not expect SSLv3 to be available