Re: [Freeipa-devel] [PATCHES] 152-158 ipa-server-certinstall fixes
On 19.8.2013 17:53, Petr Viktorin wrote:
On 08/19/2013 03:50 PM, Jan Cholasta wrote:
On 19.8.2013 14:02, Petr Viktorin wrote:
Thanks!
I've read the patches and have some initial comments; I'll get to
functional testing (and writing related CA-less tests) right away.
The patches need a small rebase (attached since I did it anyway).
Patch 152: OK (I saw some issues but they're fixed later on)
Patch 153: You can use log_file_name = '/var/log/ipa/default.log' on the
ServerCertInstall class to keep the default log file.
What is the benefit in doing this? All ipa-server-certinstall did when
using this file was complain about /var/log/ipa being non-existent.
Ah, okay. If it was a deliberate change, please mention it in the commit
message.
OK.
Patch 154: OK
Patch 155: All this is removed by patch 157, please squash them
together.
Done.
Patch 156: OK
Patch 157: Please add the delete_cert method to the NSSDatabase class,
and have CertDB call it (see e.g. run_certutil, find_server_certs,
import_pkcs12). The CertDB is only meant for IPA-specific functionality.
Done.
Patch 158: OK
The usage looks a bit strange to me.
Yes, it definitely is strange.
Having the --dirsrv_pin and
--http_pin options doesn't make sense if there's only one certificate.
Should we add a --pin option, and make these deprecated aliases of it?
I think we should. Added (patch 162).
Or make the -d and -w options take individual arguments (which would be
backwards incompatible)?
I would rather not introduce backward incompatibility.
Also, it should be possible to enter the pin(s) and DM password
interactively.
Added (patch 163).
Updated patches attached.
Honza
--
Jan Cholasta
From 45e568c98791fa74fb8f798f8ef62c4e08083bf6 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 4 Jul 2013 14:41:07 +
Subject: [PATCH 1/8] Make PKCS#12 handling in ipa-server-certinstall closer to
what other tools do.
In particular, PKCS#12 validation and server certificate selection is now done
the same way as in ipa-server-install and ipa-replica-prepare.
https://fedorahosted.org/freeipa/ticket/3641
---
install/tools/ipa-server-certinstall | 52 +---
1 file changed, 18 insertions(+), 34 deletions(-)
diff --git a/install/tools/ipa-server-certinstall b/install/tools/ipa-server-certinstall
index 5b498b1..01a7ac0 100755
--- a/install/tools/ipa-server-certinstall
+++ b/install/tools/ipa-server-certinstall
@@ -31,10 +31,13 @@ from ipapython.ipautil import user_input
from ipaserver.install import certs, dsinstance, httpinstance, installutils
from ipalib import api
+from ipapython import admintool
from ipapython.ipa_log_manager import *
from ipapython.dn import DN
from ipaserver.plugins.ldap2 import ldap2
+CACERT = /etc/ipa/ca.crt
+
def get_realm_name():
c = krbV.default_context()
return c.default_realm
@@ -72,53 +75,34 @@ def set_ds_cert_name(cert_name, dm_password):
conn.update_entry(DN(('cn', 'RSA'), ('cn', 'encryption'), ('cn', 'config')), mod)
conn.disconnect()
-def choose_server_cert(server_certs):
-print Please select the certificate to use:
-num = 1
-for cert in server_certs:
-print %d. %s % (num, cert[0])
-num += 1
-
-while 1:
-num = user_input(Certificate number, 1)
-print
-if num 1 or num len(server_certs):
-print number out of range
-else:
-break
+def import_cert(dirname, pkcs12_fname, pkcs12_passwd, db_password):
+[pw_fd, pw_name] = tempfile.mkstemp()
+os.write(pw_fd, pkcs12_passwd)
+os.close(pw_fd)
-return server_certs[num - 1]
+try:
+server_cert = installutils.check_pkcs12(
+pkcs12_info=(pkcs12_fname, pw_name),
+ca_file=CACERT,
+hostname=api.env.host)
+except admintool.ScriptError, e:
+print str(e)
+sys.exit(1)
-def import_cert(dirname, pkcs12_fname, pkcs12_passwd, db_password):
cdb = certs.CertDB(api.env.realm, nssdir=dirname)
cdb.create_passwd_file(db_password)
cdb.create_certdbs()
-[pw_fd, pw_name] = tempfile.mkstemp()
-os.write(pw_fd, pkcs12_passwd)
-os.close(pw_fd)
try:
try:
+cdb.nssdb.import_pem_cert('CA', 'CT,CT,', CACERT)
cdb.import_pkcs12(pkcs12_fname, pw_name)
-ca_names = cdb.find_root_cert_from_pkcs12(pkcs12_fname, pw_name)
except RuntimeError, e:
print str(e)
sys.exit(1)
finally:
os.remove(pw_name)
-server_certs = cdb.find_server_certs()
-if len(server_certs) == 0:
-print could not find a suitable server cert in import
-sys.exit(1)
-elif len(server_certs) == 1:
-server_cert = server_certs[0]
-else:
-server_cert = choose_server_cert(server_certs)
-
-for ca in ca_names:
-cdb.trust_root_cert(ca)
-
return server_cert
def main():
@@ -149,12 +133,12 @@
Re: [Freeipa-devel] [PATCH] 0056 Fix broken replication
On 08/19/2013 06:16 PM, Ana Krivokapic wrote: On 08/19/2013 06:01 PM, Petr Viktorin wrote: On 08/19/2013 05:50 PM, Ana Krivokapic wrote: Hello, This patch addresses tickethttps://fedorahosted.org/freeipa/ticket/3868. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. freeipa-akrivoka-0056-Fix-broken-replication.patch From cdcb28b9b3b8e45db1b7a61f0df6f41e7a61450a Mon Sep 17 00:00:00 2001 From: Ana Krivokapicakriv...@redhat.com Date: Mon, 19 Aug 2013 17:45:31 +0200 Subject: [PATCH] Fix broken replication Make sure the subject base parameter is correctly passed and used during the creation of the DS instance on a replica. https://fedorahosted.org/freeipa/ticket/3868 --- [...] --- a/ipaserver/install/dsinstance.py +++ b/ipaserver/install/dsinstance.py @@ -275,7 +275,7 @@ def create_instance(self, realm_name, fqdn, domain_name, def create_replica(self, realm_name, master_fqdn, fqdn, domain_name, dm_password, pkcs12_info=None, - ca_file=None): + ca_file=None, subject_base=None): Does it ever make sense to have subject_base=None here? I don't think so. Fixed. Also changed the commit message and ticket summary, as suggested by Rob. Updated patch is attached. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. freeipa-akrivoka-0056-02-Fix-broken-replica-installation.patch From 0730de02f665da080956175e78c263a011416dc2 Mon Sep 17 00:00:00 2001 From: Ana Krivokapicakriv...@redhat.com Date: Mon, 19 Aug 2013 17:45:31 +0200 Subject: [PATCH] Fix broken replica installation Make sure the subject base parameter is correctly passed and used during the creation of the DS instance on a replica. https://fedorahosted.org/freeipa/ticket/3868 --- install/tools/ipa-replica-install | 14 ++ ipaserver/install/dsinstance.py | 6 +++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install index 79f8a7ab48f75ac2d9cd5149df6eda4784b3854a..8be57bf7d6f5ed956f3d666b6518ea18055d9df6 100755 --- a/install/tools/ipa-replica-install +++ b/install/tools/ipa-replica-install @@ -162,10 +162,16 @@ def install_replica_ds(config): config.dir + /dirsrv_pin.txt) ds = dsinstance.DsInstance() -ds.create_replica(config.realm_name, - config.master_host_name, config.host_name, - config.domain_name, config.dirman_password, - pkcs12_info, ca_file = config.dir + /ca.crt) +ds.create_replica( +config.realm_name, +config.master_host_name, +config.host_name, +config.domain_name, +config.dirman_password, +config.subject_base, +pkcs12_info, +ca_file=config.dir + /ca.crt, +) Here's small a nitpick; if you don't get to it by the time I test the patch I'll aim to ack the current version. Since there's lots of arguments that apparently tend to get rearranged, I think it's good style to name each one when calling the method (e.g. `fqdn=config.master_host_name,`). That way if you for some reason end up with a mismatched version of create_replica (common during development), it would fail loudly rather than doing the wrong thing. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0056 Fix broken replication
On 08/20/2013 01:17 PM, Petr Viktorin wrote: On 08/19/2013 06:16 PM, Ana Krivokapic wrote: On 08/19/2013 06:01 PM, Petr Viktorin wrote: On 08/19/2013 05:50 PM, Ana Krivokapic wrote: Hello, This patch addresses tickethttps://fedorahosted.org/freeipa/ticket/3868. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. freeipa-akrivoka-0056-Fix-broken-replication.patch From cdcb28b9b3b8e45db1b7a61f0df6f41e7a61450a Mon Sep 17 00:00:00 2001 From: Ana Krivokapicakriv...@redhat.com Date: Mon, 19 Aug 2013 17:45:31 +0200 Subject: [PATCH] Fix broken replication Make sure the subject base parameter is correctly passed and used during the creation of the DS instance on a replica. https://fedorahosted.org/freeipa/ticket/3868 --- [...] --- a/ipaserver/install/dsinstance.py +++ b/ipaserver/install/dsinstance.py @@ -275,7 +275,7 @@ def create_instance(self, realm_name, fqdn, domain_name, def create_replica(self, realm_name, master_fqdn, fqdn, domain_name, dm_password, pkcs12_info=None, - ca_file=None): + ca_file=None, subject_base=None): Does it ever make sense to have subject_base=None here? I don't think so. Fixed. Also changed the commit message and ticket summary, as suggested by Rob. Updated patch is attached. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. freeipa-akrivoka-0056-02-Fix-broken-replica-installation.patch From 0730de02f665da080956175e78c263a011416dc2 Mon Sep 17 00:00:00 2001 From: Ana Krivokapicakriv...@redhat.com Date: Mon, 19 Aug 2013 17:45:31 +0200 Subject: [PATCH] Fix broken replica installation Make sure the subject base parameter is correctly passed and used during the creation of the DS instance on a replica. https://fedorahosted.org/freeipa/ticket/3868 --- install/tools/ipa-replica-install | 14 ++ ipaserver/install/dsinstance.py | 6 +++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install index 79f8a7ab48f75ac2d9cd5149df6eda4784b3854a..8be57bf7d6f5ed956f3d666b6518ea18055d9df6 100755 --- a/install/tools/ipa-replica-install +++ b/install/tools/ipa-replica-install @@ -162,10 +162,16 @@ def install_replica_ds(config): config.dir + /dirsrv_pin.txt) ds = dsinstance.DsInstance() -ds.create_replica(config.realm_name, - config.master_host_name, config.host_name, - config.domain_name, config.dirman_password, - pkcs12_info, ca_file = config.dir + /ca.crt) +ds.create_replica( +config.realm_name, +config.master_host_name, +config.host_name, +config.domain_name, +config.dirman_password, +config.subject_base, +pkcs12_info, +ca_file=config.dir + /ca.crt, +) Here's small a nitpick; if you don't get to it by the time I test the patch I'll aim to ack the current version. Since there's lots of arguments that apparently tend to get rearranged, I think it's good style to name each one when calling the method (e.g. `fqdn=config.master_host_name,`). That way if you for some reason end up with a mismatched version of create_replica (common during development), it would fail loudly rather than doing the wrong thing. Agreed, updated patch attached. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. From fbaaa4e6d7da256df429ee86f97ce0038355eed1 Mon Sep 17 00:00:00 2001 From: Ana Krivokapic akriv...@redhat.com Date: Mon, 19 Aug 2013 17:45:31 +0200 Subject: [PATCH] Fix broken replica installation Make sure the subject base parameter is correctly passed and used during the creation of the DS instance on a replica. https://fedorahosted.org/freeipa/ticket/3868 --- install/tools/ipa-replica-install | 14 ++ ipaserver/install/dsinstance.py | 16 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install index 79f8a7ab48f75ac2d9cd5149df6eda4784b3854a..c6d69fca6959fb3e082475b1e5323efe1375c7ce 100755 --- a/install/tools/ipa-replica-install +++ b/install/tools/ipa-replica-install @@ -162,10 +162,16 @@ def install_replica_ds(config): config.dir + /dirsrv_pin.txt) ds = dsinstance.DsInstance() -ds.create_replica(config.realm_name, - config.master_host_name, config.host_name, - config.domain_name, config.dirman_password, - pkcs12_info, ca_file = config.dir + /ca.crt) +ds.create_replica( +realm_name=config.realm_name, +master_fqdn=config.master_host_name, +fqdn=config.host_name, +domain_name=config.domain_name, +
Re: [Freeipa-devel] [PATCH] 445 Web UI integration tests: ID range types
On 08/19/2013 05:33 PM, Petr Vobornik wrote:
https://fedorahosted.org/freeipa/ticket/3834
New version. Fixes usage of secondary rid for non-trust installation.
--
Petr Vobornik
From 4f8358e9aa8a4ec2e28d47acd351b556cd10c272 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 16 Aug 2013 18:18:53 +0200
Subject: [PATCH] Web UI integration tests: ID range types
https://fedorahosted.org/freeipa/ticket/3834
---
.../test_webui/{test_range.py = task_range.py}| 54 +
ipatests/test_webui/test_range.py | 122 +++--
ipatests/test_webui/test_trust.py | 93 +---
ipatests/test_webui/ui_driver.py | 36 +-
4 files changed, 212 insertions(+), 93 deletions(-)
copy ipatests/test_webui/{test_range.py = task_range.py} (66%)
diff --git a/ipatests/test_webui/test_range.py b/ipatests/test_webui/task_range.py
similarity index 66%
copy from ipatests/test_webui/test_range.py
copy to ipatests/test_webui/task_range.py
index 0a7da7e4f3a804c1b4469cac225488b29dcd3138..4775078e7388078ccf4d6a59288388c3dd363ff5 100644
--- a/ipatests/test_webui/test_range.py
+++ b/ipatests/test_webui/task_range.py
@@ -18,16 +18,13 @@
# along with this program. If not, see http://www.gnu.org/licenses/.
-User tests
+Range tasks
from ipatests.test_webui.ui_driver import UI_driver
-ENTITY = 'idrange'
-PKEY = 'itest-range'
-
-class test_range(UI_driver):
+class range_tasks(UI_driver):
def get_shifts(self, idranges=None):
@@ -64,27 +61,42 @@ class test_range(UI_driver):
self.sec_rid_shift = rid_shift + 1000
self.shift = 0
-def get_data(self, pkey, size=50, shift=100):
-self.shift += shift
+def get_sid(self):
+result = self.execute_api_from_ui('trust_find', [], {})
+trusts = result['result']['result']
+sid = None
+if trusts:
+sid = trusts[0]['ipanttrusteddomainsid']
+return sid
+
+def get_data(self, pkey, size=50, add_data=None):
+
+if not add_data:
+add_data = self.get_add_data(pkey, size=size)
+
data = {
'pkey': pkey,
-'add': [
-('textbox', 'cn', pkey),
-('textbox', 'ipabaseid', str(self.id_shift + self.shift)),
-('textbox', 'ipaidrangesize', str(size)),
-('textbox', 'ipabaserid', str(self.rid_shift + self.shift)),
-('textbox', 'ipasecondarybaserid', str(self.sec_rid_shift + self.shift)),
-],
+'add': add_data,
'mod': [
('textbox', 'ipaidrangesize', str(size + 1)),
],
}
return data
-def test_crud(self):
-
-Basic CRUD: range
-
-self.init_app()
-self.get_shifts()
-self.basic_crud(ENTITY, self.get_data(PKEY))
+def get_add_data(self, pkey, range_type='ipa-local', size=50, shift=100, sid=None):
+
+self.shift += shift
+add = [
+('textbox', 'cn', pkey),
+('textbox', 'ipabaseid', str(self.id_shift + self.shift)),
+('textbox', 'ipaidrangesize', str(size)),
+('textbox', 'ipabaserid', str(self.rid_shift + self.shift)),
+('radio', 'iparangetype', range_type),
+]
+
+if not sid:
+add.append(('textbox', 'ipasecondarybaserid', str(self.sec_rid_shift + self.shift)))
+if sid:
+add.append(('textbox', 'ipanttrusteddomainsid', sid))
+
+return add
diff --git a/ipatests/test_webui/test_range.py b/ipatests/test_webui/test_range.py
index 0a7da7e4f3a804c1b4469cac225488b29dcd3138..98c4098a6dba51d9b5d17c36727e3609f5c7ba36 100644
--- a/ipatests/test_webui/test_range.py
+++ b/ipatests/test_webui/test_range.py
@@ -18,68 +18,17 @@
# along with this program. If not, see http://www.gnu.org/licenses/.
-User tests
+Range tests
-from ipatests.test_webui.ui_driver import UI_driver
+import ipatests.test_webui.test_trust as trust_mod
+from ipatests.test_webui.task_range import range_tasks
ENTITY = 'idrange'
PKEY = 'itest-range'
-class test_range(UI_driver):
-
-def get_shifts(self, idranges=None):
-
-if not idranges:
-result = self.execute_api_from_ui('idrange_find', [], {})
-idranges = result['result']['result']
-
-id_shift = 0
-rid_shift = 0
-
-for idrange in idranges:
-size = int(idrange['ipaidrangesize'][0])
-base_id = int(idrange['ipabaseid'][0])
-
-id_end = base_id + size
-rid_end = 0
-
-if 'ipabaserid' in idrange:
-base_rid = int(idrange['ipabaserid'][0])
-rid_end = base_rid + size
-
-if 'ipasecondarybaserid' in idrange:
-secondary_base_rid = int(idrange['ipasecondarybaserid'][0])
-rid_end = max(base_rid, secondary_base_rid) +
Re: [Freeipa-devel] [PATCHES] 152-158 ipa-server-certinstall fixes
On 08/20/2013 09:10 AM, Jan Cholasta wrote: On 19.8.2013 17:53, Petr Viktorin wrote: On 08/19/2013 03:50 PM, Jan Cholasta wrote: On 19.8.2013 14:02, Petr Viktorin wrote: Thanks! I've read the patches and have some initial comments; I'll get to functional testing (and writing related CA-less tests) right away. The patches need a small rebase (attached since I did it anyway). Patch 152: OK (I saw some issues but they're fixed later on) Patch 153: You can use log_file_name = '/var/log/ipa/default.log' on the ServerCertInstall class to keep the default log file. What is the benefit in doing this? All ipa-server-certinstall did when using this file was complain about /var/log/ipa being non-existent. Ah, okay. If it was a deliberate change, please mention it in the commit message. OK. Patch 154: OK Patch 155: All this is removed by patch 157, please squash them together. Done. Patch 156: OK Patch 157: Please add the delete_cert method to the NSSDatabase class, and have CertDB call it (see e.g. run_certutil, find_server_certs, import_pkcs12). The CertDB is only meant for IPA-specific functionality. Done. Patch 158: OK The usage looks a bit strange to me. Yes, it definitely is strange. Having the --dirsrv_pin and --http_pin options doesn't make sense if there's only one certificate. Should we add a --pin option, and make these deprecated aliases of it? I think we should. Added (patch 162). Or make the -d and -w options take individual arguments (which would be backwards incompatible)? I would rather not introduce backward incompatibility. Also, it should be possible to enter the pin(s) and DM password interactively. Added (patch 163). Updated patches attached. Honza ACK, passes my tests. Thanks! I've filed https://fedorahosted.org/freeipa/ticket/3869 for this and other CLI changes to ipa-server-certinstall. I've written a design doc for it: http://www.freeipa.org/page/V3/ipa-server-certinstall_CLI_cleanup I've changed 162's commit message to point to the new ticket, and pushed to ipa-3-3: 43a6af1414eae86d90be2a5b292a37670256cb99 master: 78cf94a52cba0b3205a8aacaeacfe1415a68a2c9 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0091] Perform dirsrv tuning at platform level
On Mon, 2013-08-19 at 14:48 +0200, Tomas Babej wrote:
Hi,
When configuring the 389 Directory Server instance, we tune it
so that number of file descriptors available to the DS is increased
from the default 1024 to 8192.
There are platform specific steps that need to be conducted
differently on systemd compatible platforms and sysV compatible
platforms.
systemd: set LimitNOFILE to 8192 in /etc/sysconfig/dirsrv.systemd
sysV: set ulimit -n 8192 in /etc/sysconfig/dirsrv
set ulimit - nofile 8192 in /etc/security/limits.conf
https://fedorahosted.org/freeipa/ticket/3823
I'd prefer the use of 'with' in the RedHatDirectoryService:
# check limits.conf
need_limits = True
with open(/etc/security/limits.conf) as f:
for line in f:
sline = line.strip()
if not sline.startswith(DS_USER):
continue
if sline.find('nofile') == -1:
continue
# ok we already have an explicit entry for user/nofile
need_limits = False
... and ...
with open(/etc/sysconfig/dirsrv, a+) as f:
f.write('ulimit -n %s\n' % str(num))
Nathaniel
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 443 Web UI integration tests: CA-less
On 08/15/2013 04:03 PM, Petr Vobornik wrote:
+if is_visible:
+is_enabled = 'disabled' not in
link.get_attribute(class).split(' ')
Nitpick: it would be better to use .split() without arguments here. Here's an
example that illustrates the difference in behavior between .split() and split('
') (e.g. if there are multiple spaces between class names):
'class1 class2 class3 class4'.split()
['class1', 'class2', 'class3', 'class4']
'class1 class2 class3 class4'.split(' ')
['class1', '', 'class2', 'class3', '', '', '', '', 'class4']
ACK with this change.
--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
