Re: [Freeipa-devel] [PATCH 0032] Secure permission and cleanup Custodia server.keys

2016-08-02 Thread Christian Heimes 
On 2016-07-19 17:03, Martin Basti wrote:
> 
> 
> On 12.07.2016 16:45, Christian Heimes wrote:
>> Custodia's server.keys file contain the private RSA keys for encrypting
>> and signing Custodia messages. The file was created with permission 644
>> and is only secured by permission 700 of the directory
>> /etc/ipa/custodia. The installer and upgrader ensure that the file
>> has 600.
>>
>> The server.keys file and all keys are now removed when during
>> uninstallation of a server, too.
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1353936
>> https://fedorahosted.org/freeipa/ticket/6015
>> https://fedorahosted.org/freeipa/ticket/6056
>>
>>
> NACK
> 
> ipa-server-install --uninstall doesn't work

I fixed it by splitting up uninstallation into two parts:

1) the server_del plugin takes care of the LDAP entries
2) CustodiaInstance.uninstall() removes the local key file

From 77541f4d70e3fa02a058c0812d4062a34e0bebf4 Mon Sep 17 00:00:00 2001
From: Christian Heimes 
Date: Fri, 8 Jul 2016 20:06:57 +0200
Subject: [PATCH] Secure permission and cleanup Custodia server.keys

Custodia's server.keys file contain the private RSA keys for encrypting
and signing Custodia messages. The file was created with permission 644
and is only secured by permission 700 of the directory
/etc/ipa/custodia. The installer and upgrader ensure that the file
has 600.

The server.keys file and all keys are now removed when during
uninstallation of a server, too.

https://bugzilla.redhat.com/show_bug.cgi?id=1353936
https://fedorahosted.org/freeipa/ticket/6015
https://fedorahosted.org/freeipa/ticket/6056
---
 ipalib/constants.py   |  1 +
 ipapython/secrets/kem.py  | 60 +++
 ipaserver/install/custodiainstance.py | 25 +++
 ipaserver/plugins/server.py   | 31 ++
 4 files changed, 104 insertions(+), 13 deletions(-)

diff --git a/ipalib/constants.py b/ipalib/constants.py
index 0574bb3aa457dd79a6d64f6b8a6b57161d32da92..9b351e260f15211330521453b3ffcd41433a04bb 100644
--- a/ipalib/constants.py
+++ b/ipalib/constants.py
@@ -124,6 +124,7 @@ DEFAULT_CONFIG = (
 ('container_locations', DN(('cn', 'locations'), ('cn', 'etc'))),
 ('container_ca', DN(('cn', 'cas'), ('cn', 'ca'))),
 ('container_dnsservers', DN(('cn', 'servers'), ('cn', 'dns'))),
+('container_custodia', DN(('cn', 'custodia'), ('cn', 'ipa'), ('cn', 'etc'))),
 
 # Ports, hosts, and URIs:
 ('xmlrpc_uri', 'http://localhost:/ipa/xml'),
diff --git a/ipapython/secrets/kem.py b/ipapython/secrets/kem.py
index d45efe8cc4fb63ae9d8c0b2c920fd1f9e5331a9d..863234e01bf6226ee5090c76450b0fe25c430ed6 100644
--- a/ipapython/secrets/kem.py
+++ b/ipapython/secrets/kem.py
@@ -15,6 +15,8 @@ from jwcrypto.jwk import JWK
 from ipapython.secrets.common import iSecLdap
 from binascii import unhexlify
 import ldap
+import errno
+import os
 
 
 IPA_REL_BASE_DN = 'cn=custodia,cn=ipa,cn=etc'
@@ -66,7 +68,7 @@ class KEMLdap(iSecLdap):
  'princ': principal})
 r = conn.search_s(self.keysbase, scope, ldap_filter)
 if len(r) != 1:
-raise ValueError("Incorrect number of results (%d) searching for"
+raise ValueError("Incorrect number of results (%d) searching for "
  "public key for %s" % (len(r), principal))
 ipa_public_key = r[0][1]['ipaPublicKey'][0]
 jwk = self._parse_public_key(ipa_public_key)
@@ -139,11 +141,29 @@ class KEMLdap(iSecLdap):
 mods = [(ldap.MOD_REPLACE, 'ipaPublicKey', public_key)]
 conn.modify_s(dn, mods)
 
+def remove_key(self, usage, principal):
+conn = self.connect()
+scope = ldap.SCOPE_SUBTREE
+
+ldap_filter = self.build_filter(IPA_KEYS_QUERY,
+{'usage': RFC5280_USAGE_MAP[usage],
+ 'princ': principal})
+
+r = conn.search_s(self.keysbase, scope, ldap_filter)
+if not r:
+return False
+for entry in r:
+dn = r[0][0]
+conn.delete_s(dn)
+return True
+
 
 def newServerKeys(path, keyid):
 skey = JWK(generate='RSA', use='sig', kid=keyid)
 ekey = JWK(generate='RSA', use='enc', kid=keyid)
-with open(path, 'w+') as f:
+with open(path, 'w') as f:
+os.fchmod(f.fileno(), 0o600)
+os.fchown(f.fileno(), 0, 0)
 f.write('[%s,%s]' % (skey.export(), ekey.export()))
 return [skey.get_op_key('verify'), ekey.get_op_key('encrypt')]
 
@@ -177,6 +197,9 @@ class IPAKEMKeys(KEMKeysStore):
 self.ldap_uri = conf.get('global', 'ldap_uri', None)
 self._server_keys = None
 
+def get_principal(self, servicename):
+return '%s/%s@%s' % (servicename, self.host, self.realm)
+
 def find_key(self, kid, usage):
 if kid is None:
 raise TypeError('Key ID is None, should be a SPN')
@@ -187,7

Re: [Freeipa-devel] [PATCH]: 0098-99 : Split make lint to more targets and add jslint

2016-08-02 Thread Pavel Vomacka 


On 08/02/2016 05:31 PM, Pavel Vomacka wrote:



On 08/02/2016 05:27 PM, Martin Basti wrote:



On 02.08.2016 17:12, Rob Crittenden wrote:

Pavel Vomacka wrote:

Hello,

please review attached patches which Split make lint to more 
targets and

add jslint


What's the driver to split the checks out into separate targets?


It is called several times during build (makes build slower), and you 
cannot run `make clean` in case you have wrong API.txt, because it 
will explode

Yes, definitely.

So I removed moving the aci and api checks and just add jslint.


You are moving the makeapi and makeaci from version-update to lint. 
They were in version-update for a reason: downstream builds do not 
call lint. Downstream may patch code. API cannot break.

Can we update downstream spec then?



No ticket?

Pavel please file tickets.


Yes, I will file tickets for these changes.

Also ticket is now filed:

https://fedorahosted.org/freeipa/ticket/6161


rob


Martin^2





--
Pavel^3 Vomacka

From 5d993beeafb2283160dae4d86ed296e882bed920 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Tue, 2 Aug 2016 17:57:24 +0200
Subject: [PATCH] Add jslint into Makefile

Also put jsl into dependencies.

The patch also split lint target into more smaller targets.
The purpose of this change is to add possibility to run only
fast jslint by using make jslint and don't waste time with pylint,
which can take a lot of time.

https://fedorahosted.org/freeipa/ticket/6161
---
 Makefile| 8 +++-
 freeipa.spec.in | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index d84927838cec9b8cf56cf2404c59f68808bce65f..6324308602a4bb7a3a79a8f09b0994482e0f8eef 100644
--- a/Makefile
+++ b/Makefile
@@ -133,7 +133,7 @@ client-dirs:
 		echo "Without those directories ipa-client-install will fail" ; \
 	fi
 
-lint: bootstrap-autogen
+pylint: bootstrap-autogen
 	# find all python modules and executable python files outside modules for pylint check
 	FILES=`find . \
 		-type d -exec test -e '{}/__init__.py' \; -print -prune -o \
@@ -146,8 +146,14 @@ lint: bootstrap-autogen
 		-type f -exec grep -qsm1 '^#!.*\bpython' '{}' \; -print`; \
 	echo "Pylint is running, please wait ..."; \
 	PYTHONPATH=. pylint --rcfile=pylintrc $(PYLINTFLAGS) $$FILES || $(LINT_IGNORE_FAIL)
+
+po-validate:
 	$(MAKE) -C install/po validate-src-strings || $(LINT_IGNORE_FAIL)
 
+jslint:
+	cd install/ui; jsl -nologo -nosummary -nofilelisting -conf jsl.conf || $(LINT_IGNORE_FAIL)
+
+lint: pylint po-validate jslint
 
 test:
 	./make-test
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 135e9c980011c6c2730c6c29a3c22098e48270d5..7fdea0d3133e99d581b05b6692637b3ec8ad1caf 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -87,6 +87,7 @@ BuildRequires:  python-dns >= 1.11.1
 BuildRequires:  libsss_idmap-devel
 BuildRequires:  libsss_nss_idmap-devel >= 1.14.0
 BuildRequires:  java-headless
+BuildRequires:  jsl
 BuildRequires:  rhino
 BuildRequires:  libverto-devel
 BuildRequires:  systemd
-- 
2.5.5

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

Re: [Freeipa-devel] [PATCH]: 0098-99 : Split make lint to more targets and add jslint

2016-08-02 Thread Pavel Vomacka 



On 08/02/2016 05:27 PM, Martin Basti wrote:



On 02.08.2016 17:12, Rob Crittenden wrote:

Pavel Vomacka wrote:

Hello,

please review attached patches which Split make lint to more targets 
and

add jslint


What's the driver to split the checks out into separate targets?


It is called several times during build (makes build slower), and you 
cannot run `make clean` in case you have wrong API.txt, because it 
will explode

Yes, definitely.


You are moving the makeapi and makeaci from version-update to lint. 
They were in version-update for a reason: downstream builds do not 
call lint. Downstream may patch code. API cannot break.

Can we update downstream spec then?



No ticket?

Pavel please file tickets.


Yes, I will file tickets for these changes.


rob


Martin^2



--
Pavel^3 Vomacka

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

Re: [Freeipa-devel] [PATCH]: 0098-99 : Split make lint to more targets and add jslint

2016-08-02 Thread Petr Spacek 
On 2.8.2016 17:12, Rob Crittenden wrote:
> Pavel Vomacka wrote:
>> Hello,
>>
>> please review attached patches which Split make lint to more targets and
>> add jslint
> 
> What's the driver to split the checks out into separate targets?
> 
> You are moving the makeapi and makeaci from version-update to lint. They were
> in version-update for a reason: downstream builds do not call lint. Downstream
> may patch code. API cannot break.
> 
> No ticket?

Jan, can you comment on this?

-- 
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH]: 0098-99 : Split make lint to more targets and add jslint

2016-08-02 Thread Petr Spacek 
On 2.8.2016 17:12, Rob Crittenden wrote:
> Pavel Vomacka wrote:
>> Hello,
>>
>> please review attached patches which Split make lint to more targets and
>> add jslint
> 
> What's the driver to split the checks out into separate targets?

Most importantly, makeapi and makeaci do not need to be called 10 times during
single build (and can be called in parallel, once our build system can manage
that).

> You are moving the makeapi and makeaci from version-update to lint. They were
> in version-update for a reason: downstream builds do not call lint. Downstream
> may patch code. API cannot break.

This is very true, I definitely agree. Even better, downstream should call as
much checks as possible.

This split allows the downstream spec to call all tools which are available in
downstream as part of %check section of the spec, which was not possible
before the split.

> No ticket?

-- 
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH]: 0098-99 : Split make lint to more targets and add jslint

2016-08-02 Thread Martin Basti 



On 02.08.2016 17:12, Rob Crittenden wrote:

Pavel Vomacka wrote:

Hello,

please review attached patches which Split make lint to more targets and
add jslint


What's the driver to split the checks out into separate targets?


It is called several times during build (makes build slower), and you 
cannot run `make clean` in case you have wrong API.txt, because it will 
explode


You are moving the makeapi and makeaci from version-update to lint. 
They were in version-update for a reason: downstream builds do not 
call lint. Downstream may patch code. API cannot break.

Can we update downstream spec then?



No ticket?

Pavel please file tickets.



rob


Martin^2

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

Re: [Freeipa-devel] [PATCH 33] Correct path to HTTPD's systemd service directory

2016-08-02 Thread Martin Basti 



On 02.08.2016 17:12, Abhijeet Kasurde wrote:




On 08/02/2016 08:32 PM, Christian Heimes wrote:

Ticket #5681 and commit 586fee293f42388510fa5436af19460bbe1fdec5 changed
the location of the ipa.conf for Apache HTTPD. The variables
SYSTEMD_SYSTEM_HTTPD_D_DIR and SYSTEMD_SYSTEM_HTTPD_IPA_CONF point to
the wrong directory /etc/systemd/system/httpd.d/. The path is corrected
to  /etc/systemd/system/httpd.service.d/.

https://fedorahosted.org/freeipa/ticket/6158
https://bugzilla.redhat.com/show_bug.cgi?id=1362537



ACK

It is working for me.
--
Thanks,
Abhijeet Kasurde

IRC: akasurde
http://akasurde.github.io




Pushed to master: 64db0592490493a060c7983acdfdf9100d9ea813

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

Re: [Freeipa-devel] [PATCH 33] Correct path to HTTPD's systemd service directory

2016-08-02 Thread Abhijeet Kasurde 



On 08/02/2016 08:32 PM, Christian Heimes wrote:

Ticket #5681 and commit 586fee293f42388510fa5436af19460bbe1fdec5 changed
the location of the ipa.conf for Apache HTTPD. The variables
SYSTEMD_SYSTEM_HTTPD_D_DIR and SYSTEMD_SYSTEM_HTTPD_IPA_CONF point to
the wrong directory /etc/systemd/system/httpd.d/. The path is corrected
to  /etc/systemd/system/httpd.service.d/.

https://fedorahosted.org/freeipa/ticket/6158
https://bugzilla.redhat.com/show_bug.cgi?id=1362537



ACK

It is working for me.

--
Thanks,
Abhijeet Kasurde

IRC: akasurde
http://akasurde.github.io

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

[Freeipa-devel] [PATCH 33] Correct path to HTTPD's systemd service directory

2016-08-02 Thread Christian Heimes 
Ticket #5681 and commit 586fee293f42388510fa5436af19460bbe1fdec5 changed
the location of the ipa.conf for Apache HTTPD. The variables
SYSTEMD_SYSTEM_HTTPD_D_DIR and SYSTEMD_SYSTEM_HTTPD_IPA_CONF point to
the wrong directory /etc/systemd/system/httpd.d/. The path is corrected
to  /etc/systemd/system/httpd.service.d/.

https://fedorahosted.org/freeipa/ticket/6158
https://bugzilla.redhat.com/show_bug.cgi?id=1362537
From c6ab5d9323c1cc389ab221e0fc1c5290cc0075d4 Mon Sep 17 00:00:00 2001
From: Christian Heimes 
Date: Tue, 2 Aug 2016 16:58:07 +0200
Subject: [PATCH] Correct path to HTTPD's systemd service directory

Ticket #5681 and commit 586fee293f42388510fa5436af19460bbe1fdec5 changed
the location of the ipa.conf for Apache HTTPD. The variables
SYSTEMD_SYSTEM_HTTPD_D_DIR and SYSTEMD_SYSTEM_HTTPD_IPA_CONF point to
the wrong directory /etc/systemd/system/httpd.d/. The path is corrected
to  /etc/systemd/system/httpd.service.d/.

https://fedorahosted.org/freeipa/ticket/6158
https://bugzilla.redhat.com/show_bug.cgi?id=1362537
Signed-off-by: Christian Heimes 
---
 ipaplatform/base/paths.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index 579ea70d9f0795443aebb5f120d63c8e5289886c..bb0071a8e1a951e8df661d65058584c595c7a23a 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -126,8 +126,8 @@ class BasePathNamespace(object):
 SYSCONFIG_PKI_TOMCAT = "/etc/sysconfig/pki-tomcat"
 SYSCONFIG_PKI_TOMCAT_PKI_TOMCAT_DIR = "/etc/sysconfig/pki/tomcat/pki-tomcat"
 ETC_SYSTEMD_SYSTEM_DIR = "/etc/systemd/system/"
-SYSTEMD_SYSTEM_HTTPD_D_DIR = "/etc/systemd/system/httpd.d/"
-SYSTEMD_SYSTEM_HTTPD_IPA_CONF = "/etc/systemd/system/httpd.d/ipa.conf"
+SYSTEMD_SYSTEM_HTTPD_D_DIR = "/etc/systemd/system/httpd.service.d/"
+SYSTEMD_SYSTEM_HTTPD_IPA_CONF = "/etc/systemd/system/httpd.service.d/ipa.conf"
 SYSTEMD_CERTMONGER_SERVICE = "/etc/systemd/system/multi-user.target.wants/certmonger.service"
 SYSTEMD_IPA_SERVICE = "/etc/systemd/system/multi-user.target.wants/ipa.service"
 SYSTEMD_SSSD_SERVICE = "/etc/systemd/system/multi-user.target.wants/sssd.service"
-- 
2.7.4

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

Re: [Freeipa-devel] [PATCH] 0001 Added new authentication method

2016-08-02 Thread Alexander Bokovoy 

On Mon, 01 Aug 2016, Rob Crittenden wrote:

Tibor Dudlak wrote:

Hi,

I have added few lines to code to make optional login with personal
certificate (or with smartcard) possible. Some ui changes has to be
made. It is not cosher but it definitely work.

Thank you, Tibor



What about the Apache changes to require a certificate in 
/ipa/session/login_x509?


Does/will this only support a specially crafted certificate subject?

How/where does the UI get a Kerberos ticket for the user?

That's indeed a problem -- even with the PKINIT support in KDC that Simo
is polishing up now, we don't have a way to obtain a ticket on behalf of
the user because Apache would terminate the SSL negotiation and we
wouldn't be able to use user's certificate to do PKINIT negotiation to
obtain a ticket as a user and then continue running on its behalf.
Neither we would get any Kerberos ticket from the client side.
--
/ Alexander Bokovoy

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

[Freeipa-devel] [PATCH]: 0098-99 : Split make lint to more targets and add jslint

2016-08-02 Thread Pavel Vomacka 

Hello,

please review attached patches which Split make lint to more targets and 
add jslint


--
Pavel^3 Vomacka

From f32919a910dc98301b373a47cc72f68b1310c0ee Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Tue, 2 Aug 2016 16:44:10 +0200
Subject: [PATCH 1/2] Split targets in Makefile

Separate aci and api check to new targets. Also separate lint check to pylint
and po-validation target. Prepares for easier adding of new lints.
---
 Makefile | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index d84927838cec9b8cf56cf2404c59f68808bce65f..16d7491cc939d53176893a05b268ea661c32e375 100644
--- a/Makefile
+++ b/Makefile
@@ -133,7 +133,17 @@ client-dirs:
 		echo "Without those directories ipa-client-install will fail" ; \
 	fi
 
-lint: bootstrap-autogen
+aci-check:
+	if [ "$(SKIP_API_VERSION_CHECK)" != "yes" ]; then \
+		./makeaci --validate; \
+	fi
+
+api-check:
+	if [ "$(SKIP_API_VERSION_CHECK)" != "yes" ]; then \
+		./makeapi --validate; \
+	fi
+
+pylint: bootstrap-autogen
 	# find all python modules and executable python files outside modules for pylint check
 	FILES=`find . \
 		-type d -exec test -e '{}/__init__.py' \; -print -prune -o \
@@ -146,8 +156,11 @@ lint: bootstrap-autogen
 		-type f -exec grep -qsm1 '^#!.*\bpython' '{}' \; -print`; \
 	echo "Pylint is running, please wait ..."; \
 	PYTHONPATH=. pylint --rcfile=pylintrc $(PYLINTFLAGS) $$FILES || $(LINT_IGNORE_FAIL)
+
+po-validate:
 	$(MAKE) -C install/po validate-src-strings || $(LINT_IGNORE_FAIL)
 
+lint: aci-check api-check pylint po-validate 
 
 test:
 	./make-test
@@ -198,11 +211,6 @@ version-update: release-update
 		ln -s $(SUPPORTED_PLATFORM)/constants.py ipaplatform/constants.py; \
 	fi
 
-	if [ "$(SKIP_API_VERSION_CHECK)" != "yes" ]; then \
-		./makeapi --validate && \
-		./makeaci --validate; \
-	fi
-
 server: version-update
 	$(PYTHON) setup.py build
 	cd ipaplatform && $(PYTHON) setup.py build
-- 
2.5.5

From 911c4f5c59b3e381aeff97a08bcba2b3a85e60d8 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Tue, 2 Aug 2016 16:46:24 +0200
Subject: [PATCH 2/2] Add jslint to make lint target

Checks all JavaScript files
---
 Makefile| 5 -
 freeipa.spec.in | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 16d7491cc939d53176893a05b268ea661c32e375..71f693b60c1f59f47cddc0d356de95491dbffb10 100644
--- a/Makefile
+++ b/Makefile
@@ -160,7 +160,10 @@ pylint: bootstrap-autogen
 po-validate:
 	$(MAKE) -C install/po validate-src-strings || $(LINT_IGNORE_FAIL)
 
-lint: aci-check api-check pylint po-validate 
+jslint:
+	cd install/ui; jsl -nologo -nosummary -nofilelisting -conf jsl.conf || $(LINT_IGNORE_FAIL)
+
+lint: aci-check api-check pylint po-validate jslint
 
 test:
 	./make-test
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 135e9c980011c6c2730c6c29a3c22098e48270d5..7fdea0d3133e99d581b05b6692637b3ec8ad1caf 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -87,6 +87,7 @@ BuildRequires:  python-dns >= 1.11.1
 BuildRequires:  libsss_idmap-devel
 BuildRequires:  libsss_nss_idmap-devel >= 1.14.0
 BuildRequires:  java-headless
+BuildRequires:  jsl
 BuildRequires:  rhino
 BuildRequires:  libverto-devel
 BuildRequires:  systemd
-- 
2.5.5

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

Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate

2016-08-02 Thread Lenka Doudova 



On 07/29/2016 11:43 AM, Lenka Doudova wrote:




On 07/29/2016 11:41 AM, Lenka Doudova wrote:


On 07/28/2016 01:35 PM, Peter Lacko wrote:

Hops, fixed.

Peter


- Original Message -
From: "Lenka Doudova"
To:freeipa-devel@redhat.com
Sent: Thursday, July 28, 2016 1:32:25 PM
Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in  
certificate

Hi,

I cannot find any attached patch :)

Lenka


On 07/28/2016 01:30 PM, Peter Lacko wrote:

Attached you can find a patch adding test for URIs in generated certificate
ipatests/test_xmlrpc/test_cert_plugin.py
Since I'm leaving Red Hat in end of July, I won't be able to modify this patch 
anymore.

Regards,

Peter





Hi,

NACK. Code looks fine and works well on master branch, but patch does 
not apply on 4-3 and 4-2 branches.
Peter left the company but claimed he can fix the patch if necessary, 
I'll communicate it with him or fix it myself.


Lenka



Oh, and forgot this one - PEP8 error:
./ipatests/test_xmlrpc/test_cert_plugin.py:191:80: E501 line too long 
(105 > 79 characters)


Lenka



Hi,

since Peter has quit already, I took it upon myself to do minor fix and 
rebase to the patch.
1) i removed pylint disable comments from the patch, as they were 
unnecessary (this also solved PEP8 error)

2) I rebased the patch to be applicable for ipa-4-3 branch.
Original functionality of the patch remains unchanged.

Both fixed patches attached.

Lenka
From 63f0efeaf16cff8cb30e6e8e7903722330ae883c Mon Sep 17 00:00:00 2001
From: Peter Lacko 
Date: Fri, 15 Jul 2016 16:55:51 +0200
Subject: [PATCH] Test URIs in certificate.

Test that CRL URI and OCSP URI are present and correct in generated certificate.

https://fedorahosted.org/freeipa/ticket/5881
---
 ipatests/test_xmlrpc/test_cert_plugin.py | 52 
 1 file changed, 39 insertions(+), 13 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py
index a3839d0f79af7208bc2e9ce54183dec288f79ff1..b106c091edde68097c35907ea834b37b64407735 100644
--- a/ipatests/test_xmlrpc/test_cert_plugin.py
+++ b/ipatests/test_xmlrpc/test_cert_plugin.py
@@ -19,24 +19,25 @@
 """
 Test the `ipalib/plugins/cert.py` module against a RA.
 """
+from __future__ import print_function
 
 import sys
+import base64
+import nose
 import os
+import pytest
 import shutil
-from nose.tools import raises, assert_raises  # pylint: disable=E0611
-
-from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, assert_attr_equal
+import six
+import tempfile
 from ipalib import api
 from ipalib import errors
 from ipalib import x509
-import tempfile
-from ipapython import ipautil
-import six
-import nose
-import base64
 from ipaplatform.paths import paths
+from ipapython import ipautil
 from ipapython.dn import DN
-import pytest
+from ipapython.ipautil import run
+from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, assert_attr_equal
+from nose.tools import raises, assert_raises  # pylint: disable=E0611
 
 if six.PY3:
 unicode = str
@@ -44,6 +45,11 @@ if six.PY3:
 # So we can save the cert from issuance and compare it later
 cert = None
 newcert = None
+sn = None
+
+_DOMAIN = api.env.domain
+_EXP_CRL_URI = ''.join(['http://ipa-ca.', _DOMAIN, '/ipa/crl/MasterCRL.bin'])
+_EXP_OCSP_URI = ''.join(['http://ipa-ca.', _DOMAIN, '/ca/ocsp'])
 
 def is_db_configured():
 """
@@ -82,6 +88,8 @@ class test_cert(XMLRPC_test):
 
 if 'cert_request' not in api.Command:
 raise nose.SkipTest('cert_request not registered')
+if 'cert_show' not in api.Command:
+raise nose.SkipTest('cert_show not registered')
 
 is_db_configured()
 
@@ -94,6 +102,7 @@ class test_cert(XMLRPC_test):
 self.reqdir = tempfile.mkdtemp(prefix = "tmp-")
 self.reqfile = self.reqdir + "/test.csr"
 self.pwname = self.reqdir + "/pwd"
+self.certfile = self.reqdir + "/cert.crt"
 
 # Create an empty password file
 fp = open(self.pwname, "w")
@@ -144,13 +153,15 @@ class test_cert(XMLRPC_test):
 Test the `xmlrpc.cert_request` method with --add.
 """
 # Our host should exist from previous test
-global cert
+global cert, sn
 
 csr = unicode(self.generateCSR(str(self.subject)))
 res = api.Command['cert_request'](csr, principal=self.service_princ, add=True)['result']
 assert DN(res['subject']) == self.subject
 # save the cert for the service_show/find tests
 cert = res['certificate'].encode('ascii')
+# save cert's SN for URI test
+sn = res['serial_number']
 
 def test_0003_service_show(self):
 """
@@ -171,7 +182,22 @@ class test_cert(XMLRPC_test):
 res = api.Command['service_find'](self.service_princ)['result']
 assert base64.b64encode(res[0]['usercertificate'][0]) == cert
 
-def test_0005_cert_renew(self):
+def test_0005_cert_uris(self):
+

Re: [Freeipa-devel] [PATCH 0058] Make get_entries not ignore its size_limit argument

2016-08-02 Thread Stanislav Laznicka 

On 07/19/2016 09:20 AM, Jan Cholasta wrote:

Hi,

On 14.7.2016 14:36, Stanislav Laznicka wrote:

Hello,

This patch fixes https://fedorahosted.org/freeipa/ticket/5640.

With not so much experience with the framework, it raises question in my
head whether ipaldap.get_entries is used properly throughout the system
- does it always assume that it gets ALL the requested entries or just a
few of those as configured by the 'ipaSearchRecordsLimit' attribute of
ipaConfig.etc which it actually gets?


That depends. If you call get_entries() on the ldap2 plugin (which is 
usually the case in the framework), then ipaSearchRecordsLimit is 
used. If you call it on some arbitrary LDAPClient instance, the 
hardcoded default (= unlimited) is used.




One spot that I know the get_entries method was definitely not used
properly before this patch is in the
baseldap.LDAPObject.get_memberindirect() method:

 692 result = self.backend.get_entries(
 693 self.api.env.basedn,
 694 filter=filter,
 695 attrs_list=['member'],
 696 size_limit=-1, # paged search will get everything
anyway
 697 paged_search=True)

which to me seems kind of important if the environment size_limit is not
set properly :) The patch does not fix the non-propagation of the
paged_search, though.


Why do you think size_limit is not used properly here?
AFAIU it is desired that the search is unlimited. However, due to the 
fact that neither size_limit nor paged_search are passed from 
ldap2.get_entries() to ldap2.find_entries() (methods inherited from 
LDAPClient), only the number of records specified by 
ipaSearchRecordsLimit is returned. That could eventually cause problems 
should ipaSearchRecordsLimit be set to a low value as in the ticket.


Anyway, this ticket is not really easily fixable without more profound 
changes. Often, multiple LDAP searches are done during command 
execution. What do you do with the size limit then? Do you pass the 
same size limit to all the searches? Do you subtract the result size 
from the size limit after each search? Do you do something else with 
it? ... The answer is that it depends on the purpose of each 
individual LDAP search (like in get_memberindirect() above, we have to 
do unlimited search, otherwise the resulting entry would be 
incomplete), and fixing this accross the whole framework is a 
non-trivial task.


I do realize that the proposed fix for the permission plugin is not 
perfect, it would probably be better to subtract the number of currently 
loaded records from the sizelimit, although in the end the number of 
returned values will not be higher than the given size_limit.  However, 
it seems reasonable that if get_entries is passed a size limit, it 
should apply it over current ipaSearchRecordsLimit rather than ignoring 
it. Then, any use of get_entries could be fixed accordingly if someone 
sees fit.


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

[Freeipa-devel] [PATCH] 0013 Fix ipa hbactest output

2016-08-02 Thread Florence Blanc-Renaud 

Hi,

please find attached a patch related to 'ipa hbactest' producing a 
Traceback.


https://fedorahosted.org/freeipa/ticket/6157

Flo.
>From 38a855bfc1279ae38ac3f8083903af29e9e00a8b Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud 
Date: Tue, 2 Aug 2016 10:40:54 +0200
Subject: [PATCH] Fix ipa hbactest output

ipa hbactest command produces a Traceback (TypeError: cannot concatenate
'str' and 'bool' objects)
This happens because hbactest overrides output_for_cli but does not
properly handle the output for 'value' field. 'value' contains a boolean
but it should not be displayed (refer to ipalib/frontend.py,
Command.output_for_cli()).

Note that the issue did not appear before because the 'value' field
had a flag no_display.

https://fedorahosted.org/freeipa/ticket/6157
---
 ipaclient/plugins/hbactest.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ipaclient/plugins/hbactest.py b/ipaclient/plugins/hbactest.py
index 2518719522c4eddff2e6bc341ee9a7c34b431938..36dfbdc85de9513cee12510768b5ff682c941b18 100644
--- a/ipaclient/plugins/hbactest.py
+++ b/ipaclient/plugins/hbactest.py
@@ -43,6 +43,8 @@ class hbactest(CommandOverride):
 if 'no_display' in outp.flags:
 continue
 result = output[o]
+if o == 'value':
+continue
 if isinstance(result, (list, tuple)):
 textui.print_attribute(unicode(outp.doc), result, '%s: %s', 1, True)
 elif isinstance(result, (unicode, bool)):
-- 
2.7.4

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

Re: [Freeipa-devel] [Test][Patch-0049, 0050] Certs in ID overrides test

2016-08-02 Thread Oleg Fayans 

Hi Martin,

I did! Thank you!

On 08/02/2016 12:31 PM, Martin Basti wrote:



On 01.08.2016 22:46, Oleg Fayans wrote:

The test was redesigned so that it actually tests against an AD user.
cleanly applies, passes lint and passes

https://paste.fedoraproject.org/399504/00843641/


Okay

Did you forget to send patches?

Martin^2



On 06/28/2016 01:40 PM, Oleg Fayans wrote:

Patch-0050 rebased against latest upstream branch

On 06/28/2016 10:45 AM, Oleg Fayans wrote:

Passing test output:

https://paste.fedoraproject.org/385774/71035231/













--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From e8944743236af1fbcf56cbaecb6a4203b4086be9 Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Mon, 1 Aug 2016 22:18:44 +0200
Subject: [PATCH] Added interface to certutil

---
 ipatests/test_integration/tasks.py | 5 +
 1 file changed, 5 insertions(+)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 8cd9ec71bc5ee22b8aba5d5c6324d1e7bf8b28a6..7f6c79e65cda31bdba3d882a72bb5e2dcdb1f355 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -1179,6 +1179,11 @@ def run_server_del(host, server_to_delete, force=False,
 return host.run_command(args, raiseonerr=False)
 
 
+def run_certutil(host, args, reqdir, stdin=None):
+new_args = [paths.CERTUTIL, "-d", reqdir]
+new_args = " ".join(new_args + args)
+return host.run_command(new_args, raiseonerr=False,
+stdin_text=stdin)
 def assert_error(result, stderr_text, returncode=None):
 "Assert that `result` command failed and its stderr contains `stderr_text`"
 assert stderr_text in result.stderr_text, result.stderr_text
-- 
1.8.3.1

From cc88677030efe05044a79486b87533d416b6bcc3 Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Mon, 1 Aug 2016 22:40:00 +0200
Subject: [PATCH] Automated test for certs in idoverrides feature

https://fedorahosted.org/freeipa/ticket/6005
---
 .../test_integration/test_certs_in_idoverrides.py  | 118 +
 1 file changed, 118 insertions(+)
 create mode 100644 ipatests/test_integration/test_certs_in_idoverrides.py

diff --git a/ipatests/test_integration/test_certs_in_idoverrides.py b/ipatests/test_integration/test_certs_in_idoverrides.py
new file mode 100644
index ..9114c4f91cd6378acc53caa068b852ae15670d7a
--- /dev/null
+++ b/ipatests/test_integration/test_certs_in_idoverrides.py
@@ -0,0 +1,118 @@
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+
+import os
+import re
+import string
+from ipatests.test_integration import tasks
+from ipatests.test_integration.base import IntegrationTest
+from ipatests.test_integration.tasks import assert_error
+
+
+class TestCertsInIDOverrides(IntegrationTest):
+topology = "line"
+service_certprofile = 'caIPAserviceCert'
+num_ad_domains = 1
+user_certprofile = 'caIPAuserCert'
+adview = 'Default Trust View'
+cert_re = re.compile('Certificate: (?P.*?)\\s+.*')
+
+@classmethod
+def uninstall(cls, mh):
+cls.master.run_command(['rm', '-rf', cls.reqdir], raiseonerr=False)
+
+@classmethod
+def install(cls, mh):
+super(TestCertsInIDOverrides, cls).install(mh)
+master = cls.master
+
+# AD-related stuff
+cls.ad = cls.ad_domains[0].ads[0]
+cls.ad_domain = cls.ad.domain.name
+cls.aduser = "testuser@%s" % cls.ad_domain
+cls.adcert1 = 'MyCert1'
+cls.adcert2 = 'MyCert2'
+cls.adcert1_file = cls.adcert1 + '.crt'
+cls.adcert2_file = cls.adcert2 + '.crt'
+tasks.install_adtrust(master)
+tasks.sync_time(master, cls.ad)
+tasks.establish_trust_with_ad(cls.master, cls.ad_domain,
+  extra_args=['--range-type',
+  'ipa-ad-trust'])
+
+tasks.sync_time(cls.master, cls.ad)
+master.run_command(['ipa', 'certprofile-show', cls.service_certprofile,
+"--out=%s.txt" % cls.user_certprofile])
+master.run_command("sed -i \"s/profileId=%s/profileId=%s/\" %s.txt" % (
+cls.service_certprofile, cls.user_certprofile,
+cls.user_certprofile)
+)
+master.run_command(['ipa', 'certprofile-import', cls.user_certprofile,
+"--file=%s.txt" % cls.user_certprofile,
+'--store=true', '--desc="User Certs"'])
+
+cls.reqdir = os.path.join(master.config.test_dir, "certs")
+cls.reqfile1 = os.path.join(cls.reqdir, "test1.csr")
+cls.reqfile2 = os.path.join(cls.reqdir, "test2.csr")
+cls.pwname = os.path.join(cls.reqdir, "pwd")
+
+# Create a NSS database folder
+master.run_command(['mkdir', cls.reqdir], raiseonerr=False)
+# Create an empty password file
+master.run_command(["touch",

Re: [Freeipa-devel] [PATCH 0048] Remove sys.exit() from installer modules

2016-08-02 Thread Stanislav Laznicka 

On 07/28/2016 10:57 AM, Martin Basti wrote:

On 17.06.2016 13:56, Stanislav Laznicka wrote:

On 06/17/2016 01:01 PM, Petr Vobornik wrote:

On 17.6.2016 12:12, Stanislav Laznicka wrote:

On 06/17/2016 09:51 AM, Petr Vobornik wrote:

On 17.6.2016 09:24, Stanislav Laznicka wrote:

On 06/17/2016 08:48 AM, Petr Spacek wrote:

On 17.6.2016 08:43, Stanislav Laznicka wrote:

On 06/17/2016 07:45 AM, Petr Spacek wrote:

On 16.6.2016 17:33, Stanislav Laznicka wrote:

Hello,

This patch removes most sys.exits() from installer modules and
scripts and
replaces them with ScriptError. I only left sys.exits at places
where the user
decides yes/no on continuation of the script.
I wonder if yes/no should be replaced with KeyboardInterrupt 
or some

other
exception, too...


I'm not sure, it seems more clear to just really exit if the user
desires it
and it's what we say we'll do (with possible cleanup 
beforehand). Do

you think
we could benefit somehow by raising an exception here?

I'm just thinking out loud.

It seemed to me that it is easier to share cleanup on one except 
block

instead
of having if (interrupt): cleanup; if (interrupt2): same_cleanup;

etc.

Again, just wondering out loud.

If the cleanup is the same, or similar it might be more 
beneficial to
have it in a function where you could pass what was set up 
already and
therefore needs cleanup. But that's just an opinion coming from 
thinking
out loud as well. I went through the code to see if there's much 
cleanup
after these user actions and it seems that usually there's 
nothing much
if anything. However, thinking in advance may save us much 
trouble in

the future, of course.

Btw the original scope of the ticket is to replace sys.exit calls 
ONLY
in installer modules. Please don't waste time with debugging other 
use

cases before 4.4 is out.


I might have gotten carried away a bit. Would you suggest keeping the
sys.exits replaced only in ipaserver/install/server/replicainstall.py,
ipaserver/install/server/install.py modules which are the installer
modules managed by AdminTool? I considered the modules in
ipaserver/install/ to also be installer modules as they are heavily 
used

during installation and the sys.exits there mainly occur in functions
called from install()/install_check() methods. The *-install scripts
were perhaps really obviously over the scope.

Yes, modules:
  ipaserver/install/bindinstance.py  |  2 +-
  ipaserver/install/ca.py| 19 +++---
  ipaserver/install/cainstance.py|  5 +-
  ipaserver/install/dns.py   |  5 +-
  ipaserver/install/dsinstance.py|  3 +-
  ipaserver/install/installutils.py  | 16 +++---
  ipaserver/install/ipa_ldap_updater.py  |  2 +-
  ipaserver/install/krainstance.py   |  3 +-
  ipaserver/install/replication.py   | 10 ++--
  ipaserver/install/server/install.py| 64 +++--
  ipaserver/install/server/replicainstall.py | 92

not modules:
install/tools/ipa-adtrust-install | 17 +++---
install/tools/ipa-ca-install | 23 
install/tools/ipa-compat-manage | 11 ++--
install/tools/ipa-dns-install | 3 +-

I'll keep the sys.exit replaces that won't make it here on the 
side, we

may want to do them later.

I'm a bit worried that the patch might change some behavior. Maybe I'm
wrong.


Attached is the patch with correct modules with sys.exits replaced.

I double-checked the changes and I believe the behavior shouldn't 
really change.






Hello,

suprisingly, patch needs rebase :)

1)
Is the script error the right Exception?
I chose ScriptError because it's able to change the return value of the 
script, which is necessary sometimes. RuntimeError, which may seem more 
suitable, would not be able to do that. However, I am open for ideas on 
which exception type to use.


2)
Can you use rather raise Exception(), instead of raise Exception


Sure, please see the modified attached patch.

3)
I really hate to print errors to STDOUT from modules and then just 
call exit(1) (duplicated evil), could you replace print('xxx') with 
raise AnException('xxx')
Did that, only kept those prints printing directly to stderr. Not sure 
if those should be changed as well.
From d4e820b4e59da1f2ecc6810cbb3353d6f9d53053 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Fri, 17 Jun 2016 13:14:49 +0200
Subject: [PATCH] Remove sys.exit from install modules and scripts

sys.exit() calls sometimes make it hard to find bugs and mask code that
does not always work properly.

https://fedorahosted.org/freeipa/ticket/5750
---
 ipaserver/install/bindinstance.py  |   2 +-
 ipaserver/install/ca.py|  40 -
 ipaserver/install/cainstance.py|   5 +-
 ipaserver/install/dns.py   |   5 +-
 ipaserver/install/dsinstance.py|   3 +-
 ipaserver/install/installutils.py  |  20 ++---
 ipaserver/install/ipa_ldap_updater.py  |   3 +-

Re: [Freeipa-devel] [Test][Patch-0049, 0050] Certs in ID overrides test

2016-08-02 Thread Martin Basti 



On 01.08.2016 22:46, Oleg Fayans wrote:
The test was redesigned so that it actually tests against an AD user. 
cleanly applies, passes lint and passes


https://paste.fedoraproject.org/399504/00843641/


Okay

Did you forget to send patches?

Martin^2



On 06/28/2016 01:40 PM, Oleg Fayans wrote:

Patch-0050 rebased against latest upstream branch

On 06/28/2016 10:45 AM, Oleg Fayans wrote:

Passing test output:

https://paste.fedoraproject.org/385774/71035231/











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

Re: [Freeipa-devel] [PATCH 0059] Fix to ipa-cacert-manage man and help differences

2016-08-02 Thread Stanislav Laznicka 

On 07/19/2016 10:25 AM, Florence Blanc-Renaud wrote:

On 07/15/2016 02:09 PM, Stanislav Laznicka wrote:

https://fedorahosted.org/freeipa/ticket/6013




Hi Stanislav,

thanks for your patch. As CERTFILE is added in the arguments for 
install, I would suggest to mention it in the command description. For 
instance:


install
  - Install a CA certificate
  This command can be used to install the certificate contained in 
CERTFILE as a new CA certificate to IPA.


Flo.


Hi,

Thanks for the notice, I agree that it'd be better to be more verbose 
about the CERTFILE argument. Please see the modified patch.


From 6cfe281647a489909085875b3011486ca276f044 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Fri, 15 Jul 2016 14:04:59 +0200
Subject: [PATCH] Improvements for the ipa-cacert-manage man and help

The man page for ipa-cacert-manage didn't mention that some
options are only applicable to the install some to the renew
subcommand.

Also fixed a few missing articles.

https://fedorahosted.org/freeipa/ticket/6013
---
 install/tools/man/ipa-cacert-manage.1  | 38 ++
 ipaserver/install/ipa_cacert_manage.py |  2 +-
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/install/tools/man/ipa-cacert-manage.1 b/install/tools/man/ipa-cacert-manage.1
index 1f37788336048e412eee71757f236c9944860514..f0a1033ab372c2f923a883b385c0e3304b98f56f 100644
--- a/install/tools/man/ipa-cacert-manage.1
+++ b/install/tools/man/ipa-cacert-manage.1
@@ -20,7 +20,9 @@
 .SH "NAME"
 ipa\-cacert\-manage \- Manage CA certificates in IPA
 .SH "SYNOPSIS"
-\fBipa\-cacert\-manage\fR [\fIOPTIONS\fR...] \fICOMMAND\fR
+\fBipa\-cacert\-manage\fR [\fIOPTIONS\fR...] renew
+.RE
+\fBipa\-cacert\-manage\fR [\fIOPTIONS\fR...] install \fICERTFILE\fR
 .SH "DESCRIPTION"
 \fBipa\-cacert\-manage\fR can be used to manage CA certificates in IPA.
 .SH "COMMANDS"
@@ -29,7 +31,7 @@ ipa\-cacert\-manage \- Manage CA certificates in IPA
 \- Renew the IPA CA certificate
 .sp
 .RS
-This command can be used to manually renew CA certificate of the IPA CA.
+This command can be used to manually renew the CA certificate of the IPA CA.
 .sp
 When the IPA CA is the root CA (the default), it is not usually necessary to manually renew the CA certificate, as it will be renewed automatically when it is about to expire, but you can do so if you wish.
 .sp
@@ -42,13 +44,30 @@ When the IPA CA is not configured, this command is not available.
 \- Install a CA certificate
 .sp
 .RS
-This command can be used to install new CA certificate to IPA.
+This command can be used to install the certificate contained in \fICERTFILE\fR as a new CA certificate to IPA.
 .RE
-.SH "OPTIONS"
+.SH "COMMON OPTIONS"
+.TP
+\fB\-\-version\fR
+Show the program's version and exit.
+.TP
+\fB\-h\fR, \fB\-\-help\fR
+Show the help for this program.
 .TP
 \fB\-p\fR \fIDM_PASSWORD\fR, \fB\-\-password\fR=\fIDM_PASSWORD\fR
 The Directory Manager password to use for authentication.
 .TP
+\fB\-v\fR, \fB\-\-verbose\fR
+Print debugging information.
+.TP
+\fB\-q\fR, \fB\-\-quiet\fR
+Output only errors.
+.TP
+\fB\-\-log\-file\fR=\fIFILE\fR
+Log to the given file.
+.RE
+.SH "RENEW OPTIONS"
+.TP
 \fB\-\-self\-signed\fR
 Sign the renewed certificate by itself.
 .TP
@@ -57,6 +76,8 @@ Sign the renewed certificate by external CA.
 .TP
 \fB\-\-external\-cert\-file\fR=\fIFILE\fR
 File containing the IPA CA certificate and the external CA certificate chain. The file is accepted in PEM and DER certificate and PKCS#7 certificate chain formats. This option may be used multiple times.
+.RE
+.SH "INSTALL OPTIONS"
 .TP
 \fB\-n\fR \fINICKNAME\fR, \fB\-\-nickname\fR=\fINICKNAME\fR
 Nickname for the certificate.
@@ -73,15 +94,6 @@ T \- CA trusted to issue client certificates
 .IP
 p \- not trusted
 .RE
-.TP
-\fB\-v\fR, \fB\-\-verbose\fR
-Print debugging information.
-.TP
-\fB\-q\fR, \fB\-\-quiet\fR
-Output only errors.
-.TP
-\fB\-\-log\-file\fR=\fIFILE\fR
-Log to the given file.
 .SH "EXIT STATUS"
 0 if the command was successful
 
diff --git a/ipaserver/install/ipa_cacert_manage.py b/ipaserver/install/ipa_cacert_manage.py
index de13ad39397ae5e9b924b0621521e5fc6016c8e6..32ef25c7aac3e57d27955b6a2608adb6a1626019 100644
--- a/ipaserver/install/ipa_cacert_manage.py
+++ b/ipaserver/install/ipa_cacert_manage.py
@@ -35,7 +35,7 @@ from ipaserver.install import certs, cainstance, installutils
 class CACertManage(admintool.AdminTool):
 command_name = 'ipa-cacert-manage'
 
-usage = "%prog {renew|install} [options]"
+usage = "%prog renew [options]\n%prog install [options] CERTFILE"
 
 description = "Manage CA certificates."
 
-- 
2.7.4

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