On 07/17/2013 06:04 PM, Jan Cholasta wrote:
> On 17.7.2013 17:39, Ana Krivokapic wrote:
>> On 07/17/2013 04:57 PM, Jan Cholasta wrote:
>>> Hi,
>>>
>>> On 17.7.2013 16:38, Ana Krivokapic wrote:
>>>> Hello,
>>>>
>>>> This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3785.
>>>>
>>>
>>> NACK, this results in an unnecessarily ugly error message "[Errno 2] No such
>>> file or directory: 'file'".
>>>
>>> I would suggest something like this instead:
>>>
>>> except IOError as e:
>>>      raise ScriptError("Failed to open %s: %s" % (ca_cert_name, e.strerror))
>>
>> Fixed.
>
> Hmm, seeing how RuntimeError is used for this kind of thing in import_pkcs12,
> I think it would make sense to catch the IOError right in import_pem_cert and
> re-raise it as RuntimeError and then handle that RuntimeError in check_pkcs12
> (sorry for misleading you into doing something else in my previous mail).

I don't see much value in doing that - it just adds complexity. I would rather
leave it as it is.
>
>>>
>>> Can you please also check what happens if you pass non-existent filename to
>>> --dirsrv_pkcs12 and --http_pkcs12?
>>>
>>> Honza
>>>
>>
>> I added a more specific error message to cover these cases.
>
> Can you please also add it to find_root_cert_from_pkcs12?

Done, thanks for catching that.
>
>>
>> Updated patch attached.
>>
>
> Honza
>

Updated patch is attached.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 688675612f2a139eb090301b8dec2b56b1a34df0 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic <akriv...@redhat.com>
Date: Wed, 17 Jul 2013 16:30:15 +0200
Subject: [PATCH] Properly handle non-existent cert files

https://fedorahosted.org/freeipa/ticket/3785
---
 ipaserver/install/certs.py        | 4 ++++
 ipaserver/install/installutils.py | 4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/certs.py b/ipaserver/install/certs.py
index 06925d53b2fa6df6d94d41d758944b6497ce2bcd..54ecf53df68312e6aa835ad2209f905210dfb7b0 100644
--- a/ipaserver/install/certs.py
+++ b/ipaserver/install/certs.py
@@ -188,6 +188,8 @@ def import_pkcs12(self, pkcs12_filename, db_password_filename,
             if e.returncode == 17:
                 raise RuntimeError("incorrect password for pkcs#12 file %s" %
                     pkcs12_filename)
+            elif e.returncode == 10:
+                raise RuntimeError("Failed to open %s" % pkcs12_filename)
             else:
                 raise RuntimeError("unknown error import pkcs#12 file %s" %
                     pkcs12_filename)
@@ -206,6 +208,8 @@ def find_root_cert_from_pkcs12(self, pkcs12_fname, passwd_fname=None):
         except ipautil.CalledProcessError, e:
             if e.returncode == 17:
                 raise RuntimeError("incorrect password for pkcs#12 file")
+            elif e.returncode == 10:
+                raise RuntimeError("Failed to open %s" % pkcs12_fname)
             else:
                 raise RuntimeError("unknown error using pkcs#12 file")
 
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index a716525b3ebc20fe516613d57f19377519212a5a..188971f40f7c15cb473d590c2fcc2001b6419db7 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -721,8 +721,10 @@ def check_pkcs12(pkcs12_info, ca_file, hostname):
         ca_cert_name = 'The Root CA'
         try:
             nssdb.import_pem_cert(ca_cert_name, "CT,C,C", ca_file)
-        except ValueError, e:
+        except ValueError as e:
             raise ScriptError(str(e))
+        except IOError as e:
+            raise ScriptError("Failed to open %s: %s" % (ca_cert_name, e.strerror))
 
         # Import everything in the PKCS#12
         nssdb.import_pkcs12(pkcs12_filename, db_pwd_file.name, pin_filename)
-- 
1.8.1.4

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to