[dev.icinga.com #11063] Implement SSL cipher configuration support for the API feature

[icinga-migration]

This issue has been migrated from Redmine: https://dev.icinga.com/issues/11063

Created by kobmaki on 2016-01-31 13:46:17 +00:00

Assignee: gbeutner
Status: Resolved (closed on 2016-07-18 11:50:05 +00:00)
Target Version: 2.5.0
Last Update: 2016-07-18 11:50:05 +00:00 (in Redmine)

Backport?: Not yet backported
Include in Changelog: 1

---

Feature: Make the cipher suit defineable

The ApiListner should be allowed to define the allowed ciphers:

Object 'api' of type 'ApiListener':
  * ciphers = "Here:A:List:Of:Allowed:Ciphers"

This feature will also fix the problem with different OpenSSL protocolls and hardening a system.

Problem: OpenSSL Cipher suit is not configureable

There is no possibility to define the cipher suit that is allowed for use in the ApiListner.

icinga2  object list --type=ApiListener | grep -v modified

gives you an output like:

Object 'api' of type 'ApiListener':
  % declared in '/etc/icinga2/features-enabled/api.conf', lines 5:1-5:24
  * __name = "api"
  * accept_commands = false
  * accept_config = false
  * bind_host = ""
  * bind_port = "5665"
  * ca_path = "/etc/icinga2/pki/ca.crt"
  * cert_path = "/etc/icinga2/pki/deb-icinga-test.koboldmaki.crt"
  * crl_path = ""
  * key_path = "/etc/icinga2/pki/deb-icinga-test.koboldmaki.key"
  * name = "api"
  * package = "_etc"
  * templates = [ "api" ]
  * ticket_salt = ""
  * type = "ApiListener"
  * zone = ""
root@deb-icinga-test:~# 

Security checks

When you run some security scanner against the ApiListener it shows you that weak ciphers are available. The checks were runned with the icinga2 snapshot 2.4.1+snapshot2016.01.29+1~jessie on a debian machine.

Security check with ssl-cipher-check.pl

From http://www.unspecific.com/ssl/ you can download the script.

If you run ithe s against the ApiListner on port 5665 with the following command line:

ssl-cipher-check.pl -w localhost 5665

it shows you the following output:

Testing localhost:5665


** TLSv1.1:DES-CBC-SHA - ENABLED - WEAK 56 bits **

** TLSv1.2:DES-CBC-SHA - ENABLED - WEAK 56 bits **

** TLSv1.0:DES-CBC-SHA - ENABLED - WEAK 56 bits **

*WARNING* 3 WEAK Ciphers Enabled.
Total Ciphers Enabled: 31

Security check with sslscan

The programm "sslscan" is available on most distribution. On debian the package is called "sslscan".

Run the sslscan with:

sslscan --no-failed  localhost:5665

and the shorten output is:

                   _
           ___ ___| |___  ___ __ _ _ __
          / __/ __| / __|/ __/ _` | '_ \
          \__ \__ \ \__ \ (_| (_| | | | |
          |___/___/_|___/\___\__,_|_| |_|

                  Version 1.8.2
             http://www.titania.co.uk
        Copyright Ian Ventura-Whiting 2009

Testing SSL server localhost on port 5665

  Supported Server Cipher(s):
    Accepted  TLSv1  256 bits  AES256-SHA
    Accepted  TLSv1  256 bits  CAMELLIA256-SHA
    Accepted  TLSv1  128 bits  AES128-SHA
    Accepted  TLSv1  128 bits  SEED-SHA
    Accepted  TLSv1  128 bits  CAMELLIA128-SHA
    Accepted  TLSv1  128 bits  RC4-SHA
    Accepted  TLSv1  128 bits  RC4-MD5
    Accepted  TLSv1  112 bits  DES-CBC3-SHA
    Accepted  TLSv1  56 bits   DES-CBC-SHA

  Prefered Server Cipher(s):
    TLSv1  256 bits  AES256-SHA

  SSL Certificate:
    Version: 2
    Serial Number: 2
    Signature Algorithm: sha256WithRSAEncryption
    Issuer: /CN=Icinga CA
...
...

Security hints

The prefered cipher suit against the ApiListener are:

Preferred TLSv1.2  256 bits  AES256-GCM-SHA384            
Preferred TLSv1.1  256 bits  AES256-SHA                   
Preferred TLSv1.0  256 bits  AES256-SHA    

The underlying ca keys are self signed and the strength is state of the art:

Signature Algorithm: sha256WithRSAEncryption
Issuer: /CN=Icinga CA
Public Key Algorithm: rsaEncryption
RSA Public Key: (4096 bit)

Implementation

Let me know when:

1. The feature request is accepted
2. Final design is ready for implementation. This requires more discussions.
3. A implementation is required.

Then I could implement this feature and send the required patch including the required documentation.

Changesets

2016-07-18 11:40:00 +00:00 by kobmaki 1ca8b29

Make the cipher list configurable for TLS streams

fixes #11063

Signed-off-by: Gunnar Beutner <gunnar.beutner@netways.de>

2016-07-18 11:47:53 +00:00 by gbeutner 73d0e75

Update AUTHORS

refs #11063

2016-07-19 18:13:34 +00:00 by mfriedrich e712d6f

Fix error message for specified ciphers

refs #11063

---

Relations:

• relates #11290
• relates #11292
• relates #11063
• duplicates #9745
• duplicates #11063

[icinga-migration]

Updated by tobiasvdk on 2016-02-01 17:49:23 +00:00

I would welcome this feature.

[icinga-migration]

Updated by jflach on 2016-02-09 16:49:58 +00:00

• Status changed from New to Assigned
• Assigned to set to kobmaki

That's definitely a useful feature!

A patch would be very welcome too, and I'd be happy to review and merge it.

Cheers,
Jean

[icinga-migration]

Updated by kobmaki on 2016-02-12 22:18:54 +00:00

For implementation the request is accepted

For position number I would like to make 2. Final design is ready for implementation. This requires more discussions.

"Discussions"

When you look at the icinga2 code lib/remote/apilistener.ti most of the definition looks like 'a_b', e.g. key_path or ca_path. A good thing would be to name the "cipers" in the configuration api as

ciper_list

And it is easy to see the corresponding openssl method

SSL_CTX_set_cipher_list

that is used for setting the ciper list.

The default behaviour should be as secure as possible. Should we define a default secure cipher list definition?

If no, then there will be no real stronger security and the default definition will be empty "". This could be attacked by "a man in the middle" by breaking the connection with a weak "56Bit" cipher.

If yes, it would be good starting point with the default define ciphers that will be used if no cipher suit is defined (further discussion are welcome):

:WEAK:MEDIUM:SHA1:SSLv3:!TLSv1.0:ALL

Here we drop also the ciphers from protocol in the time area of sslv2/sslv3. This will also drop some "secure" protocols that could be used in tlsv1 and newer like "SRP-RSA-AES-128-CBC-SHA" with tls 1.2. We could also add "RC4" to make it clear that these cipher suites are not usable, but it is knocked out by "!MEDIUM" and finally by "SSLv2".

Another way of cipher would be to use the recommended ciphers from RFC7525 https://tools.ietf.org/html/rfc7525

TLS_DHE_RSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
TLS_DHE_RSA_WITH_AES_256_GCM_SHA384
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384

To define this, the cipher list "rule" would look this:

ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES128-GCM-SHA256

This is not easy to "read" and limits to TLS1.2 and newer ciphers from TLS1.3 - which is only available as a draft (januar 2016) - will not be supported by the cipher list.

Another way could be by define a

cipher_drop

in the source code that is always used in combination with the ciper_list. This will give some more secure behavior. This would be independent if a cipher_list is defined or empty or contains insecure options.

The ciper_drop will contains only the definitly unsecure parts and could look like

cipher_drop = :WEAK:MEDIUM:RC2:SSLv2:!SSLv3

A proof of concept with the cipher_list show me the following supported strong ciphers (shorten output):

sslscan.static localhost:5665
Version: 1.11.1-rbsec-3-gc9b411c-static
OpenSSL 1.0.2f-dev xx XXX xxxx

Testing SSL server localhost on port 5665

  TLS renegotiation:
Session renegotiation not supported

  TLS Compression:
Compression disabled

  Heartbleed:
TLS 1.2 not vulnerable to heartbleed
TLS 1.1 not vulnerable to heartbleed
TLS 1.0 not vulnerable to heartbleed

  Supported Server Cipher(s):
Preferred TLSv1.2  256 bits  AES256-GCM-SHA384            
Accepted  TLSv1.2  256 bits  AES256-SHA256                
Accepted  TLSv1.2  128 bits  AES128-GCM-SHA256            
Accepted  TLSv1.2  128 bits  AES128-SHA256                

  SSL Certificate:
Signature Algorithm: sha256WithRSAEncryption
RSA Key Strength:    4096

Waiting for comments.

[icinga-migration]

Updated by tobiasvdk on 2016-02-15 20:59:52 +00:00

kobmaki wrote:

For implementation the request is accepted

For position number I would like to make 2. Final design is ready for implementation. This requires more discussions.

h2. "Discussions"

When you look at the icinga2 code lib/remote/apilistener.ti most of the definition looks like 'a_b', e.g. key_path or ca_path. A good thing would be to name the "cipers" in the configuration api as

ciper_list
I'm fine with that. Another proposal "cipher_suite".

And it is easy to see the corresponding openssl method

SSL_CTX_set_cipher_list

that is used for setting the ciper list.

The default behaviour should be as secure as possible. Should we define a default secure cipher list definition?
Yes, we should.

If no, then there will be no real stronger security and the default definition will be empty "". This could be attacked by "a man in the middle" by breaking the connection with a weak "56Bit" cipher.

If yes, it would be good starting point with the default define ciphers that will be used if no cipher suit is defined (further discussion are welcome):

:WEAK:MEDIUM:SHA1:SSLv3:!TLSv1.0:ALL

Here we drop also the ciphers from protocol in the time area of sslv2/sslv3. This will also drop some "secure" protocols that could be used in tlsv1 and newer like "SRP-RSA-AES-128-CBC-SHA" with tls 1.2. We could also add "RC4" to make it clear that these cipher suites are not usable, but it is knocked out by "!MEDIUM" and finally by "SSLv2".

Another way of cipher would be to use the recommended ciphers from RFC7525 https://tools.ietf.org/html/rfc7525

TLS_DHE_RSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
TLS_DHE_RSA_WITH_AES_256_GCM_SHA384
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384

To define this, the cipher list "rule" would look this:

ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES128-GCM-SHA256

This is not easy to "read" and limits to TLS1.2 and newer ciphers from TLS1.3 - which is only available as a draft (januar 2016) - will not be supported by the cipher list.
I would go this way. If no TLS 1.2 (or newer) is available we should document the best cipher suite.

Another way could be by define a

cipher_drop

in the source code that is always used in combination with the ciper_list. This will give some more secure behavior. This would be independent if a cipher_list is defined or empty or contains insecure options.

The ciper_drop will contains only the definitly unsecure parts and could look like

cipher_drop = :WEAK:MEDIUM:RC2:SSLv2:!SSLv3

A proof of concept with the cipher_list show me the following supported strong ciphers (shorten output):

[...]

Waiting for comments.

Thanks for your detailed proposal!

[icinga-migration]

Updated by kobmaki on 2016-02-18 05:55:20 +00:00

As no comments were given for cipher_drop I "droped" the internal parameter "ciper_drop" setting.

Current state ist:

• Parameter cipher_list in the ApiListener will set the allowed ciphers
• If no cipher_list is defined, the upper defined ciphers from rfc7525 will be used
• If cipher_list results in an no shared cipher list it will throw an error and breaks the start of the daemon.
• The thrown error output you will see the definied cipher list from cipher_list

A error output from with non shared cipher from a remote client against the ApiListener will looks like:

[2016-02-18 05:40:59 +0100] warning/TlsStream: OpenSSL error: error:1408A0C1:SSL routines:SSL3_GET_CLIENT_HELLO:no shared cipher

Example from "config" if no cipher_list is defined

 icinga2 object list --type=ApiListener | grep -e ApiLis -e cipher_list
Object 'api' of type 'ApiListener':
  * cipher_list = "ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES128-GCM-SHA256"
  * type = "ApiListener"

Example of an empty result cipher_list = "WEAK:"

critical/SSL: Error with cipher list '!LOW:!WEAK:' results in no available ciphers: 336646329, "error:1410D0B9:SSL routines:SSL_CTX_set_cipher_list:no cipher match"
critical/config: Error: Cannot set cipher list to SSL context for cipher list: '!LOW:!WEAK:'.

If no more comments were added, I will create the pull-request next weekend.

I would like to add an initial documentation in an separated file. This file will contains all the required changes and behaviours.

Where to put the unit-checks?

[icinga-migration]

Updated by kobmaki on 2016-02-21 22:10:55 +00:00

Create a patch, but it is not final and requires some work.

You can see the difference:

master...kobmaki:feature/feature-api-tls-ciphers-define-11063

• The default cipher list lib/remote/apilistener.ti doesn't work as I changed the default ciphers by features.
  This has to work, as it is used if no cipher_list is defined.
  The cipher_list is now ALL:WEAK:EXP:!NULL and gives a good backwards compatibility without the weak ciphers (56Bit).

• When use the defined ciper_list in ApiListener
  cipher_list = "WEAK:EXP:MD5:SSLv2:!SSLv3:ALL:"
  It is not possible to connect with firefox. Some strange behaviour, like no overlap cipher and downgrade protocol. The same as sslscan reports.

• You can define the cipher_list with the help of openssl like

  openssl ciphers "ECDH+SHA256+AES128:ECDH+SHA384+AES256"
  

  ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA256:ECDH-RSA-AES128-SHA256:ECDH-ECDSA-AES128-SHA256:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA384:ECDH-RSA-AES256-SHA384:ECDH-ECDSA-AES256-SHA384

• Another example with but without the "!SSLv3"

   cipher_list = "!LOW:!WEAK:!MEDIUM:!EXP:!NULL:!MD5:!eNULL:!SSLv2:ALL:"

still do not works with firefox (44.0.2) but do works with Safari!

• This strange behaviour require more investigations
• To many exceptions in my firefox, and mix-up with CAs (Issuer: Icinga CA) in my environment gives my strange phenomena.

tobisavdk can you give it a try? jflach can you try to compile it?

[icinga-migration]

Updated by mfriedrich on 2016-02-24 23:49:57 +00:00

• Duplicates set to 9745

[icinga-migration]

Updated by mfriedrich on 2016-02-26 10:19:39 +00:00

• Subject changed from feature-api-tls-ciphers-define to Implement SSL cipher configuration support for the API feature

[icinga-migration]

Updated by tobiasvdk on 2016-02-26 16:16:33 +00:00

@kobmaki: looks ok so far, the coding style needs to changed, see https://wiki.icinga.org/display/Dev/Icinga+2+Coding+Style. Two other points mentioned in #9745 are SSLHonorCipherOrder and SSLProtocol. Maybe we can also have seperate settings for inter-cluster communication and for the HTTP server to avoid problems with REST API clients?

[icinga-migration]

Updated by tgelf on 2016-02-26 20:02:19 +00:00

tobiasvdk wrote:

Maybe we can also have seperate settings for inter-cluster communication and for the HTTP server to avoid problems with REST API clients?

I guess this would not work, it's the same socket, protocol is multiplexed / determined AFTER the SSL handshake took place. One could of course use different ApiListener instances for different tasks (cluster VS REST), but we should still find a safe default for both of them together.

Best,
Thomas

NB: I'm absolutely in favour of this feature.

[icinga-migration]

Updated by kobmaki on 2016-02-27 17:12:04 +00:00

Ok, style guide will be applied.

The used SSLProtocol on communication level can't be changed with ciphers. Sorry for my wrong comment "This feature will also fix the problem with different OpenSSL protocols" It could only restricted to newer ciphers when a special protocol was available.

When the ciphers are shown with the "protocol", it means that when the cipher was introduced the protocol X was available. E.g. ECDHE-RSA-AES256-SHA SSLv3 is still secure and w

The SSLHonorCipherOrder can / should be applied by the option "@strength" in openssl.
Openssl do order the cipher by strength.

For the protocols, it require an additional option. A good starting point could be:

ssl_protocolmin

and could be used with the default value

ssl_protocolmin tls1_0

That means that tls1.0 and newer are supported like tls1.1 and tls1.2.

Should start with tls1.0. Some discussion are welcome.

https://www.openssl.org/docs/manmaster/ssl/SSL\_CTX\_set\_min\_proto\_version.html

For implementation, I had to take care that these function are implement as macros and available from opensssl 1.1.0.

I do not have a cluster environment make a full test. Also, it is difficult for me to make compile and interoperability with "legacy" OS versions like Centos 5.x or Debian wheezy.

@tobiasvdk: What do you mean by inter-cluster communication and for the HTTP server to avoid problems with REST API clients

Still waiting for information on how to create the unit checks.

[icinga-migration]

Updated by tobiasvdk on 2016-02-29 09:59:03 +00:00

kobmaki wrote:

Ok, style guide will be applied.

The used SSLProtocol on communication level can't be changed with ciphers. Sorry for my wrong comment "This feature will also fix the problem with different OpenSSL protocols" It could only restricted to newer ciphers when a special protocol was available.

When the ciphers are shown with the "protocol", it means that when the cipher was introduced the protocol X was available. E.g. ECDHE-RSA-AES256-SHA SSLv3 is still secure and w

The SSLHonorCipherOrder can / should be applied by the option "STRENGTH" in openssl. Openssl do order the cipher by strength. I would use "STRENGTH" by default as this makes more sense in combination with "SSL_OP_CIPHER_SERVER_PREFERENCE" (see right down at the bottom).

For the protocols, it require an additional option. A good starting point could be:

ssl_protocolmin

and could be used with the default value

ssl_protocolmin tls1_0

That means that tls1.0 and newer are supported like tls1.1 and tls1.2.

Should start with tls1.0. Some discussion are welcome.

https://www.openssl.org/docs/manmaster/ssl/SSL\_CTX\_set\_min\_proto\_version.html
Can you implement this as a configurable option, please.

For implementation, I had to take care that these function are implement as macros and available from opensssl 1.1.0.

I do not have a cluster environment make a full test. Also, it is difficult for me to make compile and interoperability with "legacy" OS versions like Centos 5.x or Debian wheezy.

@tobiasvdk: What do you mean by inter-cluster communication and for the HTTP server to avoid problems with REST API clients
The API functionality is used for the cluster stack of icinga and for the Icinga 2 API (which is a RESTful API). As tgelf wrote you could force a stronger cipher list for the cluster communication (the icinga nodes/instances) and a weaker for the Icinga 2 (REST) API when haveing issues with (browser) client requests.

Still waiting for information on how to create the unit checks.

We should also consider using SSL_OP_CIPHER_SERVER_PREFERENCE to use the strongest cipher suite the client offers:

When choosing a cipher, use the server's preferences instead of the client preferences.
When not set, the SSL server will always follow the clients preferences.
When set, the SSL/TLS server will choose following its own preferences.

[icinga-migration]

Updated by tobiasvdk on 2016-03-03 12:55:43 +00:00

• Relates set to 11290

[icinga-migration]

Updated by tobiasvdk on 2016-03-03 13:37:54 +00:00

• Copied To set to 11292

[icinga-migration]

Updated by tobiasvdk on 2016-03-03 13:38:26 +00:00

• Copied To deleted 11292

[icinga-migration]

Updated by tobiasvdk on 2016-03-03 13:38:32 +00:00

• Relates set to 11292

[icinga-migration]

Updated by tobiasvdk on 2016-03-03 13:41:20 +00:00

I created two new tickets (#11290, #11292) to separate the ideas collected here and focus on implementing "make cipher suite configurable".

[icinga-migration]

Updated by kobmaki on 2016-03-04 22:09:58 +00:00

Feature is implemented.
The TLS minimum protocol can be restricted in the ApiListener with the option

tls_protocolmin

as SSLv2/v3 is never be used and excluded by default.

The protocol can be restrict to minimum TLSv1, TLSv1.1 and TLSv1.2.

The allowed values comes directly from openssl that defines the allowed values:

grep SSL_TXT_TLSV1 /usr/include/openssl/ssl.h
#define SSL_TXT_TLSV1       "TLSv1"
#define SSL_TXT_TLSV1_1     "TLSv1.1"
#define SSL_TXT_TLSV1_2     "TLSv1.2"

For example for TLSv1.1 you have to define

tls_protocolmin = TLSv1.1

And the correct behaviour could be verified with the help of openssl:

for i in ssl3 tls1 tls1_1 tls1_2; do echo -n $i', '; echo | openssl s_client -connect localhost:5665  -$i 2>/dev/null | grep -i "^Secure"; done;
ssl3, Secure Renegotiation IS NOT supported
tls1, Secure Renegotiation IS NOT supported
tls1_1, Secure Renegotiation IS supported
tls1_2, Secure Renegotiation IS supported

Documentation is pending and will follow.

[icinga-migration]

Updated by tobiasvdk on 2016-03-06 20:53:31 +00:00

kobmaki wrote:

Feature is implemented.
The TLS minimum protocol can be restricted in the ApiListener with the option
[...]
Can you use #11292 for the minimum TLS version feature, please.

[icinga-migration]

Updated by kobmaki on 2016-04-16 18:17:54 +00:00

I sepearate the TLS feature from this feature. The PR #74 for is pending.

The TLS feature will be implemente in the #11292.

[icinga-migration]

Updated by tobiasvdk on 2016-07-15 15:27:32 +00:00

• Relates set to 12154

[icinga-migration]

Updated by kobmaki on 2016-07-15 20:32:10 +00:00

Still waiting for integration of my PR.
What is missing? Is something wrong? Waiting since 3 months.

[icinga-migration]

Updated by gbeutner on 2016-07-18 11:39:45 +00:00

• Assigned to changed from kobmaki to gbeutner
• Target Version set to 2.5.0

I'm going to clean up the patch and merge it into the master branch.

[icinga-migration]

Updated by kobmaki on 2016-07-18 11:50:05 +00:00

• Status changed from Assigned to Resolved
• Done % changed from 0 to 100

Applied in changeset 1ca8b29.

[icinga-migration]

Updated by gbeutner on 2016-08-12 05:19:38 +00:00

• Duplicated set to 12406