Re: [PATCH] set OpenSSL DEFAULT cipher list to secure value

[Michael Tremer <michael.tremer@ipfire.org>]

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

Hi,

On Mon, 2018-01-22 at 13:55 +0100, Peter Müller wrote:
> Hello *,
> 
> thanks for the comments.
> 
> To avoid some misunderstandings here: The patch changes the
> content of the DEFAULT cipher list of OpenSSL. This list is
> used by programs such as curl and wget for establishing TLS
> sessions. As far as I know, it does not have effect on VPN
> components such as IPsec.

This does *not* affect OpenVPN and IPsec tunnels because the ciphers are chosen
in each of the tunnel's configurations.

However, wget isn't affected either because we use GnuTLS for wget which won't
compile against OpenSSL 1.1.0. Possibly GnuTLS requires the same configuration
then.

Please provide a second patch for the OpenSSL 1.1.0 because it does not make
sense to get this right in next and then revert everything next month when we
are going to roll out OpenSSL 1.1.0.

> Further, insecure algorithm (3DES, RC4, MD5, ...) are still
> supported by OpenSSL, since we have not disabled them during
> the compilation support, which I aim to do in another patch.

I am happy to remove RC4, but we cannot remove 3DES or MD5.

Please send this for the OpenSSL 1.1.0 branch.

> In case a program uses one of those algorithm, it still works.
> This is because the DEFAULT cipher list does not contain all
> ciphers supported by OpenSSL, but those which are chosen if
> no specific algorithm keywords (HIGH, !aNULL, ...) are used.
> 
> The negative impacts of this patch are:
> (a) It might break TLS connections to _very_ outdated server
> which still require weak and/or broken ciphers. But, as it
> was in similar situations, that is the problem of those who
> operate the servers.
> 
> At the moment, I am not aware of such servers IPFire connects
> to by default. Please drop me a line if I missed one.

I don't think so. The proxy doesn't initiate any SSL connections on its own so I
suppose we don't have anything that we can foresee.

Mirrors which require outdated cryptography won't be eligible for us.

> (b) Looking at the cipher list attached to the patch, you
> probably notice that TLS 1.2 ciphers without PFS (AES256-GCM
> and similar) are listed after TLS 1.0 ciphers with PFS.
> 
> While this does not necessarily have performance impacts, it
> is - with regard to security - a matter of discussion. In my
> point of view, it is better to use a cipher provides PFS
> (the transmitted contents cannot be decrypted afterwards) but
> uses a weak message digest algorithm (SHA1) than using a TLS 1.2
> cipher with secure algos, but an attacker "just" need to
> steal the private key and is able to decrypt the contents.

Agreed.

SHA1 might be broken in theory but it is not *that* broken. PFS is always
preferred.

> Unfortunately, and this is basically why patches like this
> are necessary, OpenSSL has a very bad concept of sorting cipher
> suites: It just looks at the encryption algorithm (AES, CAMELLIA, ...)
> and its strength. For example, if you look at a snippet of the output of
> "openssl ciphers -v 'HIGH:!aNULL:!eNULL:!EXP:!PSK:!SRP:!DSS'":
> 
> [...]
> ECDH-RSA-AES256-SHA384  TLSv1.2 Kx=ECDH/RSA Au=ECDH Enc=AES(256)  Mac=SHA384
> ECDH-ECDSA-AES256-SHA384 TLSv1.2 Kx=ECDH/ECDSA Au=ECDH
> Enc=AES(256)  Mac=SHA384
> ECDH-RSA-AES256-SHA     SSLv3 Kx=ECDH/RSA Au=ECDH Enc=AES(256)  Mac=SHA1
> ECDH-ECDSA-AES256-SHA   SSLv3 Kx=ECDH/ECDSA Au=ECDH Enc=AES(256)  Mac=SHA1
> AES256-GCM-SHA384       TLSv1.2 Kx=RSA      Au=RSA  Enc=AESGCM(256) Mac=AEAD
> AES256-SHA256           TLSv1.2 Kx=RSA      Au=RSA  Enc=AES(256)  Mac=SHA256
> AES256-SHA              SSLv3 Kx=RSA      Au=RSA  Enc=AES(256)  Mac=SHA1
> CAMELLIA256-SHA         SSLv3 Kx=RSA      Au=RSA  Enc=Camellia(256) Mac=SHA1
> ECDHE-RSA-AES128-GCM-SHA256 TLSv1.2 Kx=ECDH     Au=RSA  Enc=AESGCM(128)
> Mac=AEAD
> ECDHE-ECDSA-AES128-GCM-SHA256 TLSv1.2 Kx=ECDH     Au=ECDSA Enc=AESGCM(128)
> Mac=AEAD
> ECDHE-RSA-AES128-SHA256 TLSv1.2 Kx=ECDH     Au=RSA  Enc=AES(128)  Mac=SHA256
> ECDHE-ECDSA-AES128-SHA256 TLSv1.2 Kx=ECDH     Au=ECDSA
> Enc=AES(128)  Mac=SHA256
> ECDHE-RSA-AES128-SHA    SSLv3 Kx=ECDH     Au=RSA  Enc=AES(128)  Mac=SHA1
> ECDHE-ECDSA-AES128-SHA  SSLv3 Kx=ECDH     Au=ECDSA Enc=AES(128)  Mac=SHA1
> [...]
> 
> Do you see it? AES256-* is preferred over the PFS-providing cipher
> ECDHE-RSA-AES128-... because the encryption algorithm is somewhat
> weaker that that one above. OpenSSL does not care weather SHA1 is
> used, no PFS is present, or anything else.

Well it is indeed a very hard decision to make and possibly there is no clear
answer that works for everyone. We here value security more than anything. An
Android device would prefer compatibility over anything else and won't really
care if it has chosen the best algorithms that were available.

But cipher performance highly depends on the device IPFire is running on. I
suppose this absolutely not important either for downloading some files here.

> Because of that, we need to build cipher suites (for web applications,
> client programs, and much more) by hand. <sarcasm>Of couse, we don't
> do security here...</sarcasm>
> 
> In case there are any questions left, or you disagree with something, let me
> know.
> 
> Best regards,
> Peter Müller

Best,
-Michael

> 
> 
> > What would be the negative implications of this patch? Would existing
> > tunnels on production servers stop working?
> > 
> > > On Jan 21, 2018, at 2:55 PM, Paul Simmons <mbatranch(a)gmail.com> wrote:
> > >   
> > > > On Sun, 2018-01-21 at 19:08 +0000, Michael Tremer wrote:
> > > > Hello,
> > > > 
> > > > since there usually is a few people being opinionated about this sort
> > > > of changes, I will wait a little until we get the comments in. Let's
> > > > say a week.
> > > > 
> > > > Best,
> > > > -Michael
> > > >   
> > > > > On Sat, 2018-01-20 at 15:28 +0100, Peter Müller wrote:
> > > > > Only use secure cipher list for the OpenSSL DEFAULT list:
> > > > > * ECDSA is preferred over RSA since it is faster and more scalable
> > > > > * TLS 1.2 suites are preferred over anything older
> > > > > * weak ciphers such as RC4 and 3DES have been eliminated
> > > > > * AES-GCM is preferred over AES-CBC (known as "mac-then-encrypt"
> > > > > problem)
> > > > > * ciphers without PFS are moved to the end of the cipher list
> > > > > 
> > > > > The DEFAULT cipher list is now ("openssl ciphers -v"):
> > > > > 
> > > > > ECDHE-ECDSA-AES256-GCM-SHA384 TLSv1.2 Kx=ECDH     Au=ECDSA
> > > > > Enc=AESGCM(256) Mac=AEAD
> > > > > ECDHE-ECDSA-AES256-SHA384 TLSv1.2 Kx=ECDH     Au=ECDSA
> > > > > Enc=AES(256)  Mac=SHA384
> > > > > ECDHE-ECDSA-AES128-GCM-SHA256 TLSv1.2 Kx=ECDH     Au=ECDSA
> > > > > Enc=AESGCM(128) Mac=AEAD
> > > > > ECDHE-ECDSA-AES128-SHA256 TLSv1.2 Kx=ECDH     Au=ECDSA
> > > > > Enc=AES(128)  Mac=SHA256
> > > > > ECDHE-RSA-AES256-GCM-SHA384 TLSv1.2
> > > > > Kx=ECDH     Au=RSA  Enc=AESGCM(256) Mac=AEAD
> > > > > ECDHE-RSA-AES256-SHA384 TLSv1.2
> > > > > Kx=ECDH     Au=RSA  Enc=AES(256)  Mac=SHA384
> > > > > ECDHE-RSA-AES128-GCM-SHA256 TLSv1.2
> > > > > Kx=ECDH     Au=RSA  Enc=AESGCM(128) Mac=AEAD
> > > > > ECDHE-RSA-AES128-SHA256 TLSv1.2
> > > > > Kx=ECDH     Au=RSA  Enc=AES(128)  Mac=SHA256
> > > > > DHE-RSA-AES256-GCM-SHA384 TLSv1.2
> > > > > Kx=DH       Au=RSA  Enc=AESGCM(256) Mac=AEAD
> > > > > DHE-RSA-AES256-SHA256   TLSv1.2
> > > > > Kx=DH       Au=RSA  Enc=AES(256)  Mac=SHA256
> > > > > DHE-RSA-AES128-GCM-SHA256 TLSv1.2
> > > > > Kx=DH       Au=RSA  Enc=AESGCM(128) Mac=AEAD
> > > > > DHE-RSA-AES128-SHA256   TLSv1.2
> > > > > Kx=DH       Au=RSA  Enc=AES(128)  Mac=SHA256
> > > > > ECDHE-ECDSA-AES256-SHA  SSLv3 Kx=ECDH     Au=ECDSA
> > > > > Enc=AES(256)  Mac=SHA1
> > > > > ECDHE-ECDSA-AES128-SHA  SSLv3 Kx=ECDH     Au=ECDSA
> > > > > Enc=AES(128)  Mac=SHA1
> > > > > ECDHE-RSA-AES256-SHA    SSLv3
> > > > > Kx=ECDH     Au=RSA  Enc=AES(256)  Mac=SHA1
> > > > > ECDHE-RSA-AES128-SHA    SSLv3
> > > > > Kx=ECDH     Au=RSA  Enc=AES(128)  Mac=SHA1
> > > > > DHE-RSA-AES256-SHA      SSLv3
> > > > > Kx=DH       Au=RSA  Enc=AES(256)  Mac=SHA1
> > > > > DHE-RSA-CAMELLIA256-SHA SSLv3 Kx=DH       Au=RSA  Enc=Camellia(256)
> > > > > Mac=SHA1
> > > > > DHE-RSA-AES128-SHA      SSLv3
> > > > > Kx=DH       Au=RSA  Enc=AES(128)  Mac=SHA1
> > > > > DHE-RSA-CAMELLIA128-SHA SSLv3 Kx=DH       Au=RSA  Enc=Camellia(128)
> > > > > Mac=SHA1
> > > > > AES256-GCM-SHA384       TLSv1.2 Kx=RSA      Au=RSA  Enc=AESGCM(256)
> > > > > Mac=AEAD
> > > > > AES256-SHA256           TLSv1.2
> > > > > Kx=RSA      Au=RSA  Enc=AES(256)  Mac=SHA256
> > > > > AES128-GCM-SHA256       TLSv1.2 Kx=RSA      Au=RSA  Enc=AESGCM(128)
> > > > > Mac=AEAD
> > > > > AES128-SHA256           TLSv1.2
> > > > > Kx=RSA      Au=RSA  Enc=AES(128)  Mac=SHA256
> > > > > AES256-SHA              SSLv3
> > > > > Kx=RSA      Au=RSA  Enc=AES(256)  Mac=SHA1
> > > > > CAMELLIA256-SHA         SSLv3 Kx=RSA      Au=RSA  Enc=Camellia(256)
> > > > > Mac=SHA1
> > > > > AES128-SHA              SSLv3
> > > > > Kx=RSA      Au=RSA  Enc=AES(128)  Mac=SHA1
> > > > > CAMELLIA128-SHA         SSLv3 Kx=RSA      Au=RSA  Enc=Camellia(128)
> > > > > Mac=SHA1
> > > > > 
> > > > > This has been discussed at 2017-12-04 (https://wiki.ipfire.org/deve
> > > > > l/telco/2017-12-04).
> > > > > 
> > > > > Signed-off-by: Peter Müller <peter.mueller(a)link38.eu>
> > > > > Cc: Michael Tremer <michael.tremer(a)ipfire.org>
> > > > > ---
> > > > > lfs/openssl                                   |  2 +-
> > > > > src/patches/openssl-1.0.2n-weak-ciphers.patch | 12 ++++++++++++
> > > > > 2 files changed, 13 insertions(+), 1 deletion(-)
> > > > > create mode 100644 src/patches/openssl-1.0.2n-weak-ciphers.patch
> > > > > 
> > > > > diff --git a/lfs/openssl b/lfs/openssl
> > > > > index 6050768ec..65d738d0f 100644
> > > > > --- a/lfs/openssl
> > > > > +++ b/lfs/openssl
> > > > > @@ -126,7 +126,7 @@ $(TARGET) : $(patsubst
> > > > > %,$(DIR_DL)/%,$(objects))
> > > > >    @rm -rf $(DIR_APP) && cd $(DIR_SRC) && tar zxf
> > > > > $(DIR_DL)/$(DL_FILE)
> > > > >    cd $(DIR_APP) && patch -Np1 <
> > > > > $(DIR_SRC)/src/patches/openssl-1.0.0-beta5-enginesdir.patch
> > > > >    cd $(DIR_APP) && patch -Np1 <
> > > > > $(DIR_SRC)/src/patches/openssl-1.0.2a-rpmbuild.patch
> > > > > -    cd $(DIR_APP) && patch -Np1 <
> > > > > $(DIR_SRC)/src/patches/openssl-1.0.2h-weak-ciphers.patch
> > > > > +    cd $(DIR_APP) && patch -Np1 <
> > > > > $(DIR_SRC)/src/patches/openssl-1.0.2n-weak-ciphers.patch
> > > > >    cd $(DIR_APP) && patch -Np1 <
> > > > > $(DIR_SRC)/src/patches/openssl-1.0.2g-disable-sslv2v3.patch
> > > > > 
> > > > >    # i586 specific patches
> > > > > diff --git a/src/patches/openssl-1.0.2n-weak-ciphers.patch
> > > > > b/src/patches/openssl-1.0.2n-weak-ciphers.patch
> > > > > new file mode 100644
> > > > > index 000000000..9fb4051e3
> > > > > --- /dev/null
> > > > > +++ b/src/patches/openssl-1.0.2n-weak-ciphers.patch
> > > > > @@ -0,0 +1,12 @@
> > > > > +diff -Naur openssl-1.0.2n-orig/ssl/ssl.h openssl-1.0.2n/ssl/ssl.h
> > > > > +--- openssl-1.0.2n-orig/ssl/ssl.h    2017-12-07
> > > > > 14:16:42.000000000 +0100
> > > > > ++++ openssl-1.0.2n/ssl/ssl.h    2018-01-20 11:56:02.477927590
> > > > > +0100
> > > > > +@@ -338,7 +338,7 @@
> > > > > +  * The following cipher list is used by default. It also is
> > > > > substituted when
> > > > > +  * an application-defined cipher list string starts with
> > > > > 'DEFAULT'.
> > > > > +  */
> > > > > +-# define SSL_DEFAULT_CIPHER_LIST
> > > > > "ALL:!EXPORT:!LOW:!aNULL:!eNULL:!SSLv2"
> > > > > ++# define SSL_DEFAULT_CIPHER_LIST
> > > > > "kEECDH+ECDSA:kEECDH:kEDH:HIGH:+SHA:+kRSA:!aNULL:!eNULL:!LOW:!3DES:
> > > > > !MD5:!EXP:!PSK:!SRP:!kECDH:!IDEA:!SEED:!RC4:!kDH:!DSS"
> > > > > + /*
> > > > > +  * As of OpenSSL 1.0.0, ssl_create_cipher_list() in
> > > > > ssl/ssl_ciph.c always
> > > > > +  * starts with a reasonable order, and all we have to do for
> > > > > DEFAULT is  
> > > 
> > > Since some IPFire users are ignorant of the latest and greatest security 
> > > discussions, implementing this patch will help many of us to adhere to 
> > > best practices.  Therefore, I support this patch.
> > > 
> > > Best,
> > > Paul Simmons  

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]