On 29.01.2016 14:42, Christian Heimes wrote:
On 2016-01-28 09:47, Martin Basti wrote:

On 22.01.2016 12:32, Martin Kosek wrote:
On 01/21/2016 04:21 PM, Christian Heimes wrote:
The list of supported TLS cipher suites in /etc/httpd/conf.d/nss.conf
has been modernized. Insecure or less secure algorithms such as RC4,
DES and 3DES are removed. Perfect forward secrecy suites with ephemeral
ECDH key exchange have been added. IE 8 on Windows XP is no longer
supported.

The list of enabled cipher suites has been generated with the script
contrib/nssciphersuite/nssciphersuite.py.

The supported suites are currently:

TLS_RSA_WITH_AES_128_CBC_SHA256
TLS_RSA_WITH_AES_256_CBC_SHA256
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA
TLS_RSA_WITH_AES_128_GCM_SHA256
TLS_RSA_WITH_AES_128_CBC_SHA
TLS_RSA_WITH_AES_256_GCM_SHA384
TLS_RSA_WITH_AES_256_CBC_SHA

https://fedorahosted.org/freeipa/ticket/5589
Thanks for the patch! I updated the ticket to make sure this change is
release notes.

Hello,

I'm not sure if I'm the right person to do review on this, but I will
try :-)

1)
Your patch adds whitespace error

Applying: Modernize mod_nss's cipher suites
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:52: new blank
line at EOF.
+
warning: 1 line adds whitespace errors.


2)
+import urllib.request  # pylint: disable=E0611

Please specify pylint disabled check by name

3)
+def update_mod_nss_cipher_suite(http):

in this upgrade, is there any possibility that ciphers might be upgraded
again in future? (IMO yes).

I think, it can be better to store revision of change instead of boolean

LAST_REVISION =  1

if revision >= LAST_REVISION:
     return

sysupgrade.set_upgrade_state('nss.conf', 'cipher_suite_revision',
LAST_REVISION)
Thanks for the review. I have addressed the problems. Instead of a
revision number I'm using a date string. The sysupgrade module only
stores str and bool. With a date-based revision it's easy to see when
the cipher suite was checked last time.

Christian


Thanks

1) Pylint :-)
+    with urllib.request.urlopen(SOURCE) as r:  # pylint: disable=E1101

2)
+    if revision == httpinstance.NSS_CIPHER_REVISION:

may happen a case where just comparation with '==' can cause a issues (docker world)? Should not be there rather '>='?

3)
+        root_logger.info("Cipher suite already updated")

Sorry that I did not noticed earlier, this should be just debug level, IMO this message is not so important, it will cause only mess on output (we already have plenty of unneeded info messages in upgrade, they will be fixed once)

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to