On 03.02.2016 15:35, Christian Heimes wrote:
On 2016-01-29 15:05, Martin Basti wrote:

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
Thanks! It was easier to change the import to get rid of the second
pylint stanza.

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 '>='?
Makes sense, I've changed the comparison operator to >=. This may still
override user settings, though.

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)
Fine with me :)

Christian
ACK

Pushed to:
master: 5ac3a3cee534a16db86c541b9beff4939f03410e
ipa-4-3: c3496a4a4893c75789bdf0c617e46923361fb43b

--
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