-
Notifications
You must be signed in to change notification settings - Fork 574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[dev.icinga.com #11063] Implement SSL cipher configuration support for the API feature #3888
Comments
Updated by tobiasvdk on 2016-02-01 17:49:23 +00:00 I would welcome this feature. |
Updated by jflach on 2016-02-09 16:49:58 +00:00
That's definitely a useful feature! A patch would be very welcome too, and I'd be happy to review and merge it. Cheers, |
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 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):
Waiting for comments. |
Updated by tobiasvdk on 2016-02-15 20:59:52 +00:00 kobmaki wrote:
Thanks for your detailed proposal! |
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
still do not works with firefox (44.0.2) but do works with Safari!
|
Updated by mfriedrich on 2016-02-24 23:49:57 +00:00
|
Updated by mfriedrich on 2016-02-26 10:19:39 +00:00
|
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? |
Updated by tgelf on 2016-02-26 20:02:19 +00:00 tobiasvdk wrote:
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, NB: I'm absolutely in favour of this feature. |
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. 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. |
Updated by tobiasvdk on 2016-02-29 09:59:03 +00:00 kobmaki wrote:
We should also consider using SSL_OP_CIPHER_SERVER_PREFERENCE to use the strongest cipher suite the client offers:
|
Updated by tobiasvdk on 2016-03-03 12:55:43 +00:00
|
Updated by tobiasvdk on 2016-03-03 13:37:54 +00:00
|
Updated by tobiasvdk on 2016-03-03 13:38:26 +00:00
|
Updated by tobiasvdk on 2016-03-03 13:38:32 +00:00
|
Updated by kobmaki on 2016-03-04 22:09:58 +00:00 Feature is implemented. 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:
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:
Documentation is pending and will follow. |
Updated by tobiasvdk on 2016-03-06 20:53:31 +00:00 kobmaki wrote:
|
Updated by tobiasvdk on 2016-07-15 15:27:32 +00:00
|
Updated by kobmaki on 2016-07-15 20:32:10 +00:00 Still waiting for integration of my PR. |
Updated by gbeutner on 2016-07-18 11:39:45 +00:00
I'm going to clean up the patch and merge it into the master branch. |
Updated by kobmaki on 2016-07-18 11:50:05 +00:00
Applied in changeset 1ca8b29. |
Updated by gbeutner on 2016-08-12 05:19:38 +00:00
|
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)
Feature: Make the cipher suit defineable
The ApiListner should be allowed to define the 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.
gives you an output like:
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:
it shows you the following output:
Security check with sslscan
The programm "sslscan" is available on most distribution. On debian the package is called "sslscan".
Run the sslscan with:
and the shorten output is:
Security hints
The prefered cipher suit against the ApiListener are:
The underlying ca keys are self signed and the strength is state of the art:
Implementation
Let me know when:
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
2016-07-18 11:47:53 +00:00 by gbeutner 73d0e75
2016-07-19 18:13:34 +00:00 by mfriedrich e712d6f
Relations:
The text was updated successfully, but these errors were encountered: