On 28.06.2016 16:57, Florence Blanc-Renaud wrote:
On 06/28/2016 11:05 AM, Martin Basti wrote:

On 28.06.2016 10:51, Florence Blanc-Renaud wrote:
On 06/27/2016 10:18 PM, Rob Crittenden wrote:
Florence Blanc-Renaud wrote:
Hi all,

thanks for your suggestions. Updated patch attached.

The invocation in ipactl should say server, not client.

Otherwise LGTM (untested).


Hi all,

thanks to Rob for catching the typo.
Patch with updated message is attached,

Thank you for the patch I have two comments:

+    except Exception:
+        # Consider that the host is not fips-enabled if the file does
not exist
+        pass

exceptions should be as much specific as possible, otherwise it may mask
real issues
please use 'except IOError' if you want catch the case that file does
not exist

in replicainstall.py and install.py please raise exception
(RuntimeError) instead of sys.exit() to keep proper logging, cleanup, etc.

Sys.exit() should not be used in modules, it is hard to debug etc. It
can be used only in scripts (ipa-client-install, ipa-replica-manage, etc..)



hopefully converging with this updated patch :)
Thanks for all the comments, I'm learning tips with each iteration.


I propose following changes (in attached patch). If you agree I can squash patches and push it.

From a3e91642a83877f45708094f391104fbcb894fd4 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Wed, 29 Jun 2016 13:02:59 +0200
Subject: [PATCH] FIPS: reviewer proposed changes

 ipaplatform/base/paths.py | 1 +
 ipapython/ipautil.py      | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index dddefea0b558010ac24334d041201a80a05587be..d6fbe32f6839a5db40148777132ba1454cbc3382 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -134,6 +134,7 @@ class BasePathNamespace(object):
     SYSTEMD_PKI_TOMCAT_SERVICE = "/etc/systemd/system/pki-tomcatd.target.wants/pki-tomcatd@pki-tomcat.service"
     DNSSEC_TRUSTED_KEY = "/etc/trusted-key.key"
     HOME_DIR = "/home"
+    PROC_FIPS_ENABLED = "/proc/sys/crypto/fips_enabled"
     ROOT_IPA_CACHE = "/root/.ipa_cache"
     ROOT_PKI = "/root/.pki"
     DOGTAG_ADMIN_P12 = "/root/ca-agent.p12"
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 4ef9770e92c3ba86ffa5c6523268475a026705d0..c7e20c5102efaa006c10d4c3af849bc259da43e7 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1440,7 +1440,7 @@ def is_fips_enabled():
     the function returns False.
-        with open('/proc/sys/crypto/fips_enabled', 'r') as f:
+        with open(paths.PROC_FIPS_ENABLED, 'r') as f:
             if f.read().strip() != '0':
                 return True
     except IOError:

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

Reply via email to