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

["Peter Müller" <peter.mueller@link38.eu>]

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

Hello,

> 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.
Okay, development is running... :-)

@Michael: Speaking about OpenSSL 1.1.0: I saw you patching ClamAV
to work with that version some time ago. It looks like the upcoming
release (0.99.3) is linked against it by default
(http://blog.clamav.net/2017/12/clamav-0993-beta2-has-been-released.html).
Just thought you might want to know.

Best regards,
Peter Müller
> 
> > 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