Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1483972 - curl: utilize system wide crypto policies
Summary: curl: utilize system wide crypto policies
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: curl
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Kamil Dudka
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: fedora-crypto-policies
TreeView+ depends on / blocked
 
Reported: 2017-08-22 12:05 UTC by Nikos Mavrogiannopoulos
Modified: 2017-09-05 10:30 UTC (History)
3 users (show)

Fixed In Version: curl-7.55.1-3.fc28 curl-7.55.1-5.fc27
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-09-05 10:30:54 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Nikos Mavrogiannopoulos 2017-08-22 12:05:49 UTC
[note that curl with NSS in Fedora already handles crypto policies, this is to ensure that the openssl version will handle it too]

Please convert to use the system's crypto policy for SSL and TLS:
https://fedoraproject.org/wiki/Packaging:CryptoPolicies

If this program is compiled against gnutls, change the default priority string to be "@SYSTEM" or to use gnutls_set_default_priority().

If this program is compiled against openssl, and there is no default cipher list specified, you don't need to modify it. Otherwise replace the default cipher list with "PROFILE=SYSTEM".

In both cases please verify that the application uses the system's crypto policies.

If the package is already using the system-wide crypto policies, or it does not use SSL or TLS, no action is required, the bug can simply be closed.

Comment 1 Kamil Dudka 2017-08-22 15:45:42 UTC
I have pushed the following commit to implement this:

https://src.fedoraproject.org/rpms/curl/c/0480ac07

... and verified with gdb that the following command:

    curl -svo/dev/null https://...

... really calls:

    SSL_CTX_set_cipher_list (..., "PROFILE=SYSTEM")

In any case, a review would be appreciated!

Comment 2 Nikos Mavrogiannopoulos 2017-08-24 11:15:51 UTC
Thank you. The patch looks good to me. A comment on the upstream code is that at least with newer openssl versions DEFAULT may be a better option to use.

Comment 3 Kamil Dudka 2017-08-24 14:08:36 UTC
Thanks for review!

Could you please refer to some official statement about the OpenSSL's DEFAULT, preferably with precise info about the minimal version of OpenSSL that supports it?

I can prepare an upstream PR to get it fixed.  But upstream needs to support older versions of OpenSSL, too.  So it will most likely end up with some code conditioned by the version (and supported features?) of OpenSSL...

Comment 4 Nikos Mavrogiannopoulos 2017-08-24 15:36:16 UTC
I'm not sure if there so official statement or so, its just that I see that default is now being maintained more agressively than before. For example:

commit c84f7f4a7405d69be4227d4766290b0950122b3c
Author: Matt Caswell <matt>
Date:   Tue Sep 29 11:14:35 2015 +0100

    Change the DEFAULT ciphersuites to exclude DES, RC4 and RC2
    
    This patch updates the "DEFAULT" cipherstring to be
    "ALL:!COMPLEMENTOFDEFAULT:!eNULL". COMPLEMENTOFDEFAULT is now defined
    internally by a flag on each ciphersuite indicating whether it should be
    excluded from DEFAULT or not. This gives us control at an individual
    ciphersuite level as to exactly what is in DEFAULT and what is not.
    
    Finally all DES, RC4 and RC2 ciphersuites are added to COMPLEMENTOFDEFAULT
    and hence removed from DEFAULT.


Adding Tomas in case I'm wrong in that.

Comment 5 Tomas Mraz 2017-08-25 07:02:00 UTC
Yes, the OpenSSL upstream currently maintains the default as reasonably secure without breaking compatibility. So at least with current versions of openssl it should not be necessary to call the SSL_CTX_set_cipher_list() at all unless the user explicitly sets it. Note that the Fedora OpenSSL is patched in a way that when the SSL_CTX_set_cipher_list() is not called at all the system cipher list is used. With upstream openssl build not calling the SSL_CTX_set_cipher_list() is equivalent to calling it with "DEFAULT" as the cipher string.

Comment 6 Kamil Dudka 2017-08-25 11:40:15 UTC
Tomas, thanks for the info!  But you answered my questions only partially.  curl upstream currently supports OpenSSL 0.9.7 and newer.  How can one reliably check whether skipping SSL_CTX_set_cipher_list() is the correct thing to do for a given build of OpenSSL?

Which build-time/run-time check do you recommend for this purpose?

Comment 7 Tomas Mraz 2017-08-25 12:01:40 UTC
I would simply skip it during build time for OPENSSL_VERSION_NUMBER >= 0x10002000 that means any currently upstream-maintained OpenSSL version.

Comment 8 Kamil Dudka 2017-08-30 14:13:56 UTC
(In reply to Tomas Mraz from comment #7)
> I would simply skip it during build time for OPENSSL_VERSION_NUMBER >=
> 0x10002000 that means any currently upstream-maintained OpenSSL version.

Upstream pull request:

https://github.com/curl/curl/pull/1846

Comment 9 Kamil Dudka 2017-08-31 07:06:30 UTC
(In reply to Tomas Mraz from comment #7)
> I would simply skip it during build time for OPENSSL_VERSION_NUMBER >=
> 0x10002000 that means any currently upstream-maintained OpenSSL version.

Upstream says that, under certain circumstances, OpenSSL 1.0.2 can negotiate ciphers that cannot be used for HTTP/2, which would cause a regression in libcurl:

https://github.com/curl/curl/pull/1846#discussion_r136252573

Tomas, could you please review the above statement?

Will it help if we use OpenSSL 1.1 in the condition instead?

Comment 10 Tomas Mraz 2017-08-31 10:02:35 UTC
I added a comment to the github. Imho using the @STRENGTH is incorrect anyway and the cipher list for HTTP/2 should be properly reduced at the server side and not at the client side.

Comment 11 Tomas Mraz 2017-08-31 10:03:35 UTC
But yes, using the library/system default only on OpenSSL 1.1 and newer is probably reasonable.

Comment 12 Kamil Dudka 2017-09-05 10:30:54 UTC
upstream commit:

https://github.com/curl/curl/commit/curl-7_55_1-151-gea142a8


Note You need to log in before you can comment on or make changes to this bug.