[Freeipa-devel] [PATCH] 0108 cert-request: raise error when request fails

2016-09-07 Thread Fraser Tweedale
The attached patch fixes regression in cert-request:
https://fedorahosted.org/freeipa/ticket/6309

Thanks,
Fraser
From b27eef53ee36b7cae70206c37dea6aaa3bcfc940 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Thu, 8 Sep 2016 11:56:16 +1000
Subject: [PATCH] cert-request: raise error when request fails

Fix a regression in recent change to request cert via Dogtag REST
API.  'ra.request_certificate' was no longer raising
CertificateOperationError when the cert request failed.  Inspect the
request result to determine if the request completed, and raise if
it did not.

Fixes: https://fedorahosted.org/freeipa/ticket/6309
---
 ipaserver/plugins/dogtag.py | 4 
 1 file changed, 4 insertions(+)

diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index 
77d24731bbc102ace3123a6fe41a631ea7c24f3b..644b41e90f2d377ae9b70cf4719ab8789fdfc649
 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1678,6 +1678,10 @@ class ra(rabase.rabase, RestClient):
 return cmd_result
 certinfo = entries[0]
 
+if certinfo['requestStatus'] != 'complete':
+raise errors.CertificateOperationError(
+error=certinfo.get('errorMessage'))
+
 if 'certId' in certinfo:
 cmd_result = self.get_certificate(certinfo['certId'])
 cert = ''.join(cmd_result['certificate'].splitlines())
-- 
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

[Freeipa-devel] Karma Requests for pki-core-10.3.5-5

2016-09-07 Thread Matthew Harmsen
*The following updated candidate builds of pki-core 10.3.5 on Fedora 24, 
25, and 26 (rawhide) consist of the following:

*

 * *Fedora 24*
 o *pki-core-10.3.5-5.fc24
   
   *
 * *Fedora 25*
 o *pki-core-10.3.5-5.fc25
   
   *
 * *Fedora 26*
 o *pki-core-10.3.5-5.fc26
   *

*Additionally, the CentOS 7 COPR EPEL Builds of Dogtag 10.3.3 were also 
updated:*


 * 
*https://copr.fedorainfracloud.org/coprs/g/pki/10.3.3/repo/epel-7/group_pki-10.3.3-epel-7.repo*
   



   [group_pki-10.3.3]
   name=Copr repo for 10.3.3 owned by @pki
   
baseurl=https://copr-be.cloud.fedoraproject.org/results/@pki/10.3.3/epel-7-$basearch/
   skip_if_unavailable=True
   gpgcheck=1
   gpgkey=https://copr-be.cloud.fedoraproject.org/results/@pki/10.3.3/pubkey.gpg
   enabled=1
   enabled_metadata=1

*These builds address the following PKI tickets:
*

 * PKI TRAC Ticket #1638 - Lightweight CAs: revoke certificate on CA
   deletion 
 * PKI TRAC Ticket #2346 - Dogtag 10.3.6: Miscellaneous Enhancements
   
 * PKI TRAC Ticket #2443 - Prevent deletion of host CA's keys if LWCA
   entry deleted 
 * PKI TRAC Ticket #2444 - Authority entry without entryUSN is skipped
   even if USN plugin enabled 
 * PKI TRAC Ticket #2446 - pkispawn: make subject_dn defaults unique
   per instance name (for shared HSM)
   
 * PKI TRAC Ticket #2447 - CertRequestInfo has incorrect URLs
   
 * PKI TRAC Ticket #2449 - Unable to create system certificates in
   different tokens 

*Please provide Karma for the following builds:
*

 * *Fedora 24*
 o *https://bodhi.fedoraproject.org/updates/FEDORA-2016-994f943797
   pki-core-10.3.5-5.fc24*
 * *Fedora 25*
 o *https://bodhi.fedoraproject.org/updates/FEDORA-2016-d363d36e22
   pki-core-10.3.5-5.fc25
   *

-- 
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] [freeipa PR#10] Client-side CSR autogeneration (synchronize)

2016-09-07 Thread LiptonB
LiptonB's pull request #10: "Client-side CSR autogeneration" was synchronize

See the full pull-request at https://github.com/freeipa/freeipa/pull/10
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/10/head:pr10
git checkout pr10
From eeeb57fa9ff1642dbd1e32fbfe435052de2541ee Mon Sep 17 00:00:00 2001
From: Ben Lipton 
Date: Tue, 5 Jul 2016 14:19:35 -0400
Subject: [PATCH 1/8] Add code to generate scripts that generate CSRs

Adds a library that uses jinja2 to format a script that, when run, will
build a CSR. Also adds a CLI command, 'cert-get-requestdata', that uses
this library and builds the script for a given principal. The rules are
read from json files in /usr/share/ipa/csr, but the rule provider is a
separate class so that it can be replaced easily.

https://fedorahosted.org/freeipa/ticket/4899
---
 freeipa.spec.in |   8 +
 install/configure.ac|   1 +
 install/share/Makefile.am   |   1 +
 install/share/csr/Makefile.am   |  27 +++
 install/share/csr/templates/certutil_base.tmpl  |  14 ++
 install/share/csr/templates/ipa_macros.tmpl |  42 
 install/share/csr/templates/openssl_base.tmpl   |  35 +++
 install/share/csr/templates/openssl_macros.tmpl |  29 +++
 ipaclient/plugins/certmapping.py| 105 +
 ipalib/certmapping.py   | 285 
 ipalib/errors.py|   9 +
 ipapython/templating.py |  31 +++
 12 files changed, 587 insertions(+)
 create mode 100644 install/share/csr/Makefile.am
 create mode 100644 install/share/csr/templates/certutil_base.tmpl
 create mode 100644 install/share/csr/templates/ipa_macros.tmpl
 create mode 100644 install/share/csr/templates/openssl_base.tmpl
 create mode 100644 install/share/csr/templates/openssl_macros.tmpl
 create mode 100644 ipaclient/plugins/certmapping.py
 create mode 100644 ipalib/certmapping.py
 create mode 100644 ipapython/templating.py

diff --git a/freeipa.spec.in b/freeipa.spec.in
index e3ad5b6..ab8e8e6 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -507,6 +507,7 @@ Requires: python-custodia
 Requires: python-dns >= 1.11.1
 Requires: python-netifaces >= 0.10.4
 Requires: pyusb
+Requires: python-jinja2
 
 Conflicts: %{alt_name}-python < %{version}
 
@@ -1178,6 +1179,13 @@ fi
 %{_usr}/share/ipa/advise/legacy/*.template
 %dir %{_usr}/share/ipa/profiles
 %{_usr}/share/ipa/profiles/*.cfg
+%dir %{_usr}/share/ipa/csr
+%dir %{_usr}/share/ipa/csr/templates
+%{_usr}/share/ipa/csr/templates/*.tmpl
+%dir %{_usr}/share/ipa/csr/profiles
+%{_usr}/share/ipa/csr/profiles/*.json
+%dir %{_usr}/share/ipa/csr/rules
+%{_usr}/share/ipa/csr/rules/*.json
 %dir %{_usr}/share/ipa/ffextension
 %{_usr}/share/ipa/ffextension/bootstrap.js
 %{_usr}/share/ipa/ffextension/install.rdf
diff --git a/install/configure.ac b/install/configure.ac
index 81f17b9..365f0e9 100644
--- a/install/configure.ac
+++ b/install/configure.ac
@@ -87,6 +87,7 @@ AC_CONFIG_FILES([
 share/Makefile
 share/advise/Makefile
 share/advise/legacy/Makefile
+share/csr/Makefile
 share/profiles/Makefile
 share/schema.d/Makefile
 ui/Makefile
diff --git a/install/share/Makefile.am b/install/share/Makefile.am
index d8845ee..0a15635 100644
--- a/install/share/Makefile.am
+++ b/install/share/Makefile.am
@@ -2,6 +2,7 @@ NULL =
 
 SUBDIRS =  \
 	advise\
+	csr\
 	profiles			\
 	schema.d			\
 	$(NULL)
diff --git a/install/share/csr/Makefile.am b/install/share/csr/Makefile.am
new file mode 100644
index 000..5a8ef5c
--- /dev/null
+++ b/install/share/csr/Makefile.am
@@ -0,0 +1,27 @@
+NULL =
+
+profiledir = $(IPA_DATA_DIR)/csr/profiles
+profile_DATA =\
+	$(NULL)
+
+ruledir = $(IPA_DATA_DIR)/csr/rules
+rule_DATA =\
+	$(NULL)
+
+templatedir = $(IPA_DATA_DIR)/csr/templates
+template_DATA =			\
+	templates/certutil_base.tmpl	\
+	templates/openssl_base.tmpl	\
+	templates/openssl_macros.tmpl	\
+	templates/ipa_macros.tmpl	\
+	$(NULL)
+
+EXTRA_DIST =\
+	$(profile_DATA)			\
+	$(rule_DATA)			\
+	$(template_DATA)		\
+	$(NULL)
+
+MAINTAINERCLEANFILES =			\
+	*~\
+	Makefile.in
diff --git a/install/share/csr/templates/certutil_base.tmpl b/install/share/csr/templates/certutil_base.tmpl
new file mode 100644
index 000..6c6425f
--- /dev/null
+++ b/install/share/csr/templates/certutil_base.tmpl
@@ -0,0 +1,14 @@
+{% raw -%}
+{% import "ipa_macros.tmpl" as ipa -%}
+{%- endraw %}
+#!/bin/bash -e
+
+if [[ $# -lt 1 ]]; then
+echo "Usage: $0  [  ]"
+echo "Called as: $0 $@"
+exit 1
+fi
+
+CSR="$1"
+shift
+certutil -R -a -z <(head -c 4096 /dev/urandom) -o "$CSR" {{ options|join(' ') }} "$@"
diff --git a/install/share/csr/templates/ipa_macros.tmpl b/install/share/csr/templates/ipa_macros.tmpl
new file mode 100644
index 000..e790d4e
--- /dev/null
+++ 

[Freeipa-devel] [freeipa PR#67] advise: Use `name` instead of `__name__` to get plugin names (opened)

2016-09-07 Thread martbab
martbab's pull request #67: "advise: Use `name` instead of `__name__` to get 
plugin names" was opened

PR body:
"""
This change will allow ipa-advise to correctly handle advise plugins with
custom names.
"""

See the full pull-request at https://github.com/freeipa/freeipa/pull/67
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/67/head:pr67
git checkout pr67
From c4555af51ccdf9e867436ec5e0349538da512baf Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 18 Jul 2016 10:44:23 +0200
Subject: [PATCH] advise: Use `name` instead of `__name__` to get plugin names

This change will allow ipa-advise to correctly handle advise plugins with
custom names.
---
 ipaserver/advise/base.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipaserver/advise/base.py b/ipaserver/advise/base.py
index a2dc9cc..f7e8ef5 100644
--- a/ipaserver/advise/base.py
+++ b/ipaserver/advise/base.py
@@ -168,11 +168,11 @@ def print_config_list(self):
 self.print_header('List of available advices')
 
 max_keyword_len = max(
-(len(advice.__name__) for advice in advise_api.Advice))
+(len(advice.name) for advice in advise_api.Advice))
 
 for advice in advise_api.Advice:
 description = getattr(advice, 'description', '')
-keyword = advice.__name__.replace('_', '-')
+keyword = advice.name.replace('_', '-')
 
 # Compute the number of spaces needed for the table to be aligned
 offset = max_keyword_len - len(keyword)
-- 
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] [freeipa PR#58] Ip addr validation (closed)

2016-09-07 Thread dkupka
mbasti-rh's pull request #58: "Ip addr validation" was closed

See the full pull-request at https://github.com/freeipa/freeipa/pull/58
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/58/head:pr58
git checkout pr58
-- 
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] [freeipa PR#58] Ip addr validation (comment)

2016-09-07 Thread dkupka
dkupka commented on a pull request

"""
Fixed upstream
master:
https://fedorahosted.org/freeipa/changeset/81d64d530cca148198312d3d993502575b288f63
https://fedorahosted.org/freeipa/changeset/71ad8d4fc982b5349248d50338e1d16ce45c523e
https://fedorahosted.org/freeipa/changeset/f3d379071a9af50e38fcc07491bc68b8d3d172a4
https://fedorahosted.org/freeipa/changeset/b232ad463cf43596cdf397e51469df13a89e83fa
ipa-4-4:
https://fedorahosted.org/freeipa/changeset/00e747226fc011ab7181f5ed54cd4c2bc5470406
https://fedorahosted.org/freeipa/changeset/a6ab515add69058f9a45309a110d3a4553250529
https://fedorahosted.org/freeipa/changeset/435318ef347163e740a70ffe1f8a1246a908e3fe
https://fedorahosted.org/freeipa/changeset/3ffd1dceebdaf7617cabfbb57b3d0ca1f9e87065
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/58#issuecomment-245296345
-- 
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] [freeipa PR#58] Ip addr validation (+pushed)

2016-09-07 Thread dkupka
mbasti-rh's pull request #58: "Ip addr validation" label *pushed* has been added

See the full pull-request at https://github.com/freeipa/freeipa/pull/58
-- 
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] [freeipa PR#58] Ip addr validation (+ack)

2016-09-07 Thread dkupka
mbasti-rh's pull request #58: "Ip addr validation" label *ack* has been added

See the full pull-request at https://github.com/freeipa/freeipa/pull/58
-- 
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 190] expose `--secret` option in radiusproxy-* commands

2016-09-07 Thread Jan Cholasta

On 7.9.2016 16:13, Martin Babinsky wrote:

On 09/07/2016 03:55 PM, Jan Cholasta wrote:

On 21.7.2016 10:50, Jan Cholasta wrote:

On 21.7.2016 10:13, Martin Babinsky wrote:

On 07/20/2016 12:10 PM, Martin Babinsky wrote:

On 07/19/2016 12:32 PM, Jan Cholasta wrote:

Hi,

On 18.7.2016 13:51, Martin Babinsky wrote:

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


I don't think we want the secret searchable. Add a 'no_search'
flag to
the param to fix that.

Honza



'no_search' flag breaks the API backwards compatibility, so I am
sending
another two patches which fix handling of deprecated options in the
framework and deprecate `--secret` in radiusproxy-find command.

I hope this solution is the best.




After discussion with Jan we realized that it is enough to hide the
'--secret' option from CLI, not deprecate it.

Re-sending patch 190 and updated 193.1.


Thanks, ACK.

Pushed to master: 66da08445370f7024a6a529a6659714c33b7525e


Patch 192 will be send in
separate thread since the actual issue it fixes is orthogonal to this
one and requires a separate ticket.


Right. ATM this only affects --srchostcat in hbacrule-find.


Bump so that patch 192 is not forgotten.



Patch 192 was pushed as a fix to
https://fedorahosted.org/freeipa/ticket/6190.


Okay.

--
Jan Cholasta

--
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 190] expose `--secret` option in radiusproxy-* commands

2016-09-07 Thread Martin Babinsky

On 09/07/2016 03:55 PM, Jan Cholasta wrote:

On 21.7.2016 10:50, Jan Cholasta wrote:

On 21.7.2016 10:13, Martin Babinsky wrote:

On 07/20/2016 12:10 PM, Martin Babinsky wrote:

On 07/19/2016 12:32 PM, Jan Cholasta wrote:

Hi,

On 18.7.2016 13:51, Martin Babinsky wrote:

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


I don't think we want the secret searchable. Add a 'no_search' flag to
the param to fix that.

Honza



'no_search' flag breaks the API backwards compatibility, so I am
sending
another two patches which fix handling of deprecated options in the
framework and deprecate `--secret` in radiusproxy-find command.

I hope this solution is the best.




After discussion with Jan we realized that it is enough to hide the
'--secret' option from CLI, not deprecate it.

Re-sending patch 190 and updated 193.1.


Thanks, ACK.

Pushed to master: 66da08445370f7024a6a529a6659714c33b7525e


Patch 192 will be send in
separate thread since the actual issue it fixes is orthogonal to this
one and requires a separate ticket.


Right. ATM this only affects --srchostcat in hbacrule-find.


Bump so that patch 192 is not forgotten.



Patch 192 was pushed as a fix to 
https://fedorahosted.org/freeipa/ticket/6190.


--
Martin^3 Babinsky

--
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 0183] ipa-advise: correct handling of plugin namespace iteration

2016-09-07 Thread Jan Cholasta

On 19.7.2016 09:15, Martin Babinsky wrote:

On 07/18/2016 08:46 AM, Jan Cholasta wrote:

Hi,

On 11.7.2016 14:18, Martin Babinsky wrote:

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


Note that you should use .name rather than .__name__ to get plugin
names, otherwise the code won't work with plugins with non-default names.

There currently aren't any Advice plugins with non-default name, but I
would rather fix this now to avoid surprises later.

Honza



I didn't realize this when doing the patch, here's the fix for that.

I have attached the original closed ticket to the commit message, should
I create a new ticket for such a small change?


Bump. I'm fine with new ticket or no ticket, but don't use 6044.

--
Jan Cholasta

--
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 190] expose `--secret` option in radiusproxy-* commands

2016-09-07 Thread Jan Cholasta

On 21.7.2016 10:50, Jan Cholasta wrote:

On 21.7.2016 10:13, Martin Babinsky wrote:

On 07/20/2016 12:10 PM, Martin Babinsky wrote:

On 07/19/2016 12:32 PM, Jan Cholasta wrote:

Hi,

On 18.7.2016 13:51, Martin Babinsky wrote:

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


I don't think we want the secret searchable. Add a 'no_search' flag to
the param to fix that.

Honza



'no_search' flag breaks the API backwards compatibility, so I am sending
another two patches which fix handling of deprecated options in the
framework and deprecate `--secret` in radiusproxy-find command.

I hope this solution is the best.




After discussion with Jan we realized that it is enough to hide the
'--secret' option from CLI, not deprecate it.

Re-sending patch 190 and updated 193.1.


Thanks, ACK.

Pushed to master: 66da08445370f7024a6a529a6659714c33b7525e


Patch 192 will be send in
separate thread since the actual issue it fixes is orthogonal to this
one and requires a separate ticket.


Right. ATM this only affects --srchostcat in hbacrule-find.


Bump so that patch 192 is not forgotten.

--
Jan Cholasta

--
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-0058] Fixed topology tests failures in CI

2016-09-07 Thread Oleg Fayans

ping for review

On 08/24/2016 01:58 PM, Oleg Fayans wrote:

And here is how the run looks like:

$ ipa-run-tests test_integration/test_topology.py
WARNING: Couldn't write lextab module 'pycparser.lextab'. [Errno 13]
Permission denied: 'lextab.py'
WARNING: yacc table file version is out of date
WARNING: Couldn't create 'pycparser.yacctab'. [Errno 13] Permission
denied: 'yacctab.py'

test session starts
=

platform linux2 -- Python 2.7.11, pytest-2.9.2, py-1.4.31, pluggy-0.3.1
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
plugins: sourceorder-0.5, multihost-1.0
collected 3 items

test_integration/test_topology.py ..x

===
2 passed, 1 xfailed in 1558.66 seconds
===



On 08/12/2016 04:05 PM, Martin Basti wrote:



On 12.08.2016 15:48, Oleg Fayans wrote:

Hi Martin,



On 08/11/2016 10:05 AM, Martin Basti wrote:



On 10.08.2016 20:32, Oleg Fayans wrote:





Hello,

before we jump into fixing tests, my question is: Was this planned
change and not reflected by test, or switched values are unwanted side
effect and thus bug for us?


That's a marvelous question! The test used to pass, which means that
at some point the convention of naming the segments must have changed.
Is it a bug? I do not think so: the feature still works as expected.


Ludwig, do you know details about this change, why positions of server
names are different than used to be in topology name?





Ticket contains almost no info, except a traceback and it says nothing.
Commit message says at least something.

I'm not sure if this patch fixes that ticket, because traceback in test
shows error message that "removal of segment will disconnect topology",
but this patch only swap order of replica names in segment name. I
would
expect that you should get different error, something like segment does
not exist.

Which I do get in jenkins job N 37: "segment not found"

In fact, the error in the issue is unrelated to the fix, you are right.



To tell the truth, I just put a random error from one of the jenkins
topology testruns into the issue.

This is very good way how to report tickets:
* nobody knows what happened
* nobody can search in current tickets,  what is wrong without proper
description
* developers cannot investigate issue, because there is even no name of
exact test in ticket, no steps to reproduce, nothing
* without proper tickets it is hard to backport patches correctly, if
patch fixes different issue than is reported

I'm closing ticket as invalid, please follow
http://www.chiark.greenend.org.uk/~sgtatham/bugs.html and file a new
proper ticket.


This particular error message was caused by a previous replica
installation failure, which resulted in existing only one segment
instead of three:
master <-> replica1
instead of:
master <-> replica1,
master <-> replica2
replica1 <-> replica2

In fact the patch supplied fixes 2 tests at once:
The first test tries to remove the unexisting segment master <->
replica2 and fails, the second test expects the line topology
master <-> replica1 <-> replica2.
It removes the connection between replica1 and replica2, expects the
operation to fail but it does not because the connection between
master and replica2 exists

the output from the testrun with the patch applied:


-bash-4.3$ ipa-run-tests test_integration/test_topology.py --pdb
WARNING: Couldn't write lextab module 'pycparser.lextab'. [Errno 13]
Permission denied: 'lextab.py'
WARNING: yacc table file version is out of date
WARNING: Couldn't create 'pycparser.yacctab'. [Errno 13] Permission
denied: 'yacctab.py'


test session starts
=


platform linux2 -- Python 2.7.11, pytest-2.9.2, py-1.4.31, pluggy-0.3.1
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
plugins: sourceorder-0.5, multihost-1.0
collected 3 items

test_integration/test_topology.py ...



3 passed in 2156.82 seconds
=





I don't care about test output until there is no valid description of
problem, fixing test may just cover real issue.
Martin^2


Martin^2










--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

--
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] [freeipa PR#58] Ip addr validation (synchronize)

2016-09-07 Thread mbasti-rh
mbasti-rh's pull request #58: "Ip addr validation" was synchronize

See the full pull-request at https://github.com/freeipa/freeipa/pull/58
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/58/head:pr58
git checkout pr58
From 7fc0b28b05acca51ffbdfbb04a7e1dc4212ae9a0 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Fri, 2 Sep 2016 13:25:19 +0200
Subject: [PATCH 1/4] Allow network ip addresses

Currently cloud environments uses heavily prefix /32 (/128) what makes
IPA validators to fail. IPA should not care if IP address is network or not.
This commit allows usage of network addresses in:
* host plugin
* dns plugin
* server-installer
* client-installer

https://fedorahosted.org/freeipa/ticket/5814
---
 ipapython/ipautil.py| 9 +
 ipaserver/plugins/dns.py| 5 ++---
 ipatests/test_ipapython/test_ipautil.py | 6 --
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 8de9acf..8a9aa0e 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -132,8 +132,8 @@ class CheckedIPAddress(UnsafeIPAddress):
 Reserved or link-local addresses are never accepted.
 """
 def __init__(self, addr, match_local=False, parse_netmask=True,
- allow_network=False, allow_loopback=False,
- allow_broadcast=False, allow_multicast=False):
+ allow_loopback=False, allow_broadcast=False,
+ allow_multicast=False):
 
 super(CheckedIPAddress, self).__init__(addr)
 if isinstance(addr, CheckedIPAddress):
@@ -199,14 +199,15 @@ def __init__(self, addr, match_local=False, parse_netmask=True,
 elif self.version == 6:
 self._net = netaddr.IPNetwork(str(self) + '/64')
 
-if not allow_network and self == self._net.network:
-raise ValueError("cannot use IP network address {}".format(addr))
 if not allow_broadcast and (self.version == 4 and
 self == self._net.broadcast):
 raise ValueError("cannot use broadcast IP address {}".format(addr))
 
 self.prefixlen = self._net.prefixlen
 
+def is_network_addr(self):
+return self == self._net.network
+
 
 def valid_ip(addr):
 return netaddr.valid_ipv4(addr) or netaddr.valid_ipv6(addr)
diff --git a/ipaserver/plugins/dns.py b/ipaserver/plugins/dns.py
index f048351..a5f11a4 100644
--- a/ipaserver/plugins/dns.py
+++ b/ipaserver/plugins/dns.py
@@ -413,8 +413,7 @@ def _validate_bind_aci(ugettext, bind_acis):
 bind_aci = bind_aci[1:]
 
 try:
-ip = CheckedIPAddress(bind_aci, parse_netmask=True,
-  allow_network=True, allow_loopback=True)
+CheckedIPAddress(bind_aci, parse_netmask=True, allow_loopback=True)
 except (netaddr.AddrFormatError, ValueError) as e:
 return unicode(e)
 except UnboundLocalError:
@@ -439,7 +438,7 @@ def _normalize_bind_aci(bind_acis):
 
 try:
 ip = CheckedIPAddress(bind_aci, parse_netmask=True,
-  allow_network=True, allow_loopback=True)
+  allow_loopback=True)
 if '/' in bind_aci:# addr with netmask
 netmask = "/%s" % ip.prefixlen
 else:
diff --git a/ipatests/test_ipapython/test_ipautil.py b/ipatests/test_ipapython/test_ipautil.py
index 8c0b9c4..ea9251b 100644
--- a/ipatests/test_ipapython/test_ipautil.py
+++ b/ipatests/test_ipapython/test_ipautil.py
@@ -44,6 +44,7 @@ def check_ipaddress():
 
 def test_ip_address():
 addrs = [
+('0.0.0.0/0',),
 ('10.11.12.13', (10, 11, 12, 13),   8),
 ('10.11.12.13/14',  (10, 11, 12, 13),   14),
 ('10.11.12.13%zoneid',),
@@ -53,10 +54,11 @@ def test_ip_address():
 ('127.0.0.1',),
 ('241.1.2.3',),
 ('169.254.1.2',),
-('10.11.12.0/24',),
+('10.11.12.0/24',   (10, 11, 12, 0),   24),
 ('224.5.6.7',),
 ('10.11.12.255/24',),
 
+('::/0',),
 ('2001::1', (0x2001, 0, 0, 0, 0, 0, 0, 1), 64),
 ('2001::1/72',  (0x2001, 0, 0, 0, 0, 0, 0, 1), 72),
 ('2001::1%zoneid',  (0x2001, 0, 0, 0, 0, 0, 0, 1), 64),
@@ -66,7 +68,7 @@ def test_ip_address():
 ('::1',),
 ('6789::1',),
 ('fe89::1',),
-('2001::/64',),
+('2001::/64',   (0x2001, 0, 0, 0, 0, 0, 0, 0), 64),
 ('ff01::1',),
 
 ('junk',)

From e4167ec9df06a0508602968ea9d9b69b370a56c5 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Fri, 2 Sep 2016 17:07:03 +0200
Subject: [PATCH 2/4] Allow broadcast ip addresses

Currently environments may use prefix /31 on point-to-point connections what
makes IPA validators to fail. IPA should not care if IP address is broadcast
or not. In some 

[Freeipa-devel] [freeipa PR#58] Ip addr validation (comment)

2016-09-07 Thread martbab
martbab commented on a pull request

"""
@mbasti-rh you forgot to copy-paste the code to promote_check function.

@pvoborni  ^^ and that's why we need to refactor installer code
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/58#issuecomment-245277233
-- 
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] [freeipa PR#43] Tests: Fix regex errors in integration trust tests (synchronize)

2016-09-07 Thread mirielka
mirielka's pull request #43: "Tests: Fix regex errors in integration trust 
tests" was synchronize

See the full pull-request at https://github.com/freeipa/freeipa/pull/43
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/43/head:pr43
git checkout pr43
From 313aa2b2ec7af4e3ee232de75ed99bc0795f834a Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Wed, 31 Aug 2016 16:57:12 +0200
Subject: [PATCH] Tests: Fix regex errors in integration trust tests

In integration trust tests some values are checked using regular expressions.
Some of these expressions from recently added coverage have minor mistakes
which causes the comparisons to fail. Providing fix for these regular
expressions.

https://fedorahosted.org/freeipa/ticket/6285
---
 ipatests/test_integration/test_trust.py | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/ipatests/test_integration/test_trust.py b/ipatests/test_integration/test_trust.py
index d0b8e58..69418dc 100644
--- a/ipatests/test_integration/test_trust.py
+++ b/ipatests/test_integration/test_trust.py
@@ -298,8 +298,8 @@ def test_user_gid_uid_resolution_in_nonposix_trust(self):
 testuser = 'subdomaintestuser@{0}'.format(self.ad_subdomain)
 result = self.master.run_command(['getent', 'passwd', testuser])
 
-testuser_regex = ("^subdomaintestuser@{0}:\*:(?!10042)(\d+):"
-  "(?!)10047(\d+):Subdomain TestUser:"
+testuser_regex = ("^subdomaintestuser@{0}:\*:(?!10142)(\d+):"
+  "(?!10147)(\d+):Subdomaintest User:"
   "/home/{1}/subdomaintestuser:/bin/sh$".format(
   re.escape(self.ad_subdomain),
   re.escape(self.ad_subdomain)))
@@ -393,8 +393,9 @@ def test_upn_user_resolution_in_nonposix_trust(self):
   self.upn_principal])
 
 # result will contain AD domain, not UPN
-upnuser_regex = "^{}@{}:\*:(\d+):(\d+):{}:/:$".format(
-self.upn_username, self.ad_domain, self.upn_name)
+upnuser_regex = "^{}@{}:\*:(\d+):(\d+):{}:/home/{}/{}:/bin/sh$".format(
+self.upn_username, self.ad_domain, self.upn_name,
+self.ad_domain, self.upn_username)
 assert re.search(upnuser_regex, result.stdout_text)
 
 def test_upn_user_authentication(self):
-- 
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] [freeipa PR#58] Ip addr validation (synchronize)

2016-09-07 Thread mbasti-rh
mbasti-rh's pull request #58: "Ip addr validation" was synchronize

See the full pull-request at https://github.com/freeipa/freeipa/pull/58
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/58/head:pr58
git checkout pr58
From 7fc0b28b05acca51ffbdfbb04a7e1dc4212ae9a0 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Fri, 2 Sep 2016 13:25:19 +0200
Subject: [PATCH 1/4] Allow network ip addresses

Currently cloud environments uses heavily prefix /32 (/128) what makes
IPA validators to fail. IPA should not care if IP address is network or not.
This commit allows usage of network addresses in:
* host plugin
* dns plugin
* server-installer
* client-installer

https://fedorahosted.org/freeipa/ticket/5814
---
 ipapython/ipautil.py| 9 +
 ipaserver/plugins/dns.py| 5 ++---
 ipatests/test_ipapython/test_ipautil.py | 6 --
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 8de9acf..8a9aa0e 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -132,8 +132,8 @@ class CheckedIPAddress(UnsafeIPAddress):
 Reserved or link-local addresses are never accepted.
 """
 def __init__(self, addr, match_local=False, parse_netmask=True,
- allow_network=False, allow_loopback=False,
- allow_broadcast=False, allow_multicast=False):
+ allow_loopback=False, allow_broadcast=False,
+ allow_multicast=False):
 
 super(CheckedIPAddress, self).__init__(addr)
 if isinstance(addr, CheckedIPAddress):
@@ -199,14 +199,15 @@ def __init__(self, addr, match_local=False, parse_netmask=True,
 elif self.version == 6:
 self._net = netaddr.IPNetwork(str(self) + '/64')
 
-if not allow_network and self == self._net.network:
-raise ValueError("cannot use IP network address {}".format(addr))
 if not allow_broadcast and (self.version == 4 and
 self == self._net.broadcast):
 raise ValueError("cannot use broadcast IP address {}".format(addr))
 
 self.prefixlen = self._net.prefixlen
 
+def is_network_addr(self):
+return self == self._net.network
+
 
 def valid_ip(addr):
 return netaddr.valid_ipv4(addr) or netaddr.valid_ipv6(addr)
diff --git a/ipaserver/plugins/dns.py b/ipaserver/plugins/dns.py
index f048351..a5f11a4 100644
--- a/ipaserver/plugins/dns.py
+++ b/ipaserver/plugins/dns.py
@@ -413,8 +413,7 @@ def _validate_bind_aci(ugettext, bind_acis):
 bind_aci = bind_aci[1:]
 
 try:
-ip = CheckedIPAddress(bind_aci, parse_netmask=True,
-  allow_network=True, allow_loopback=True)
+CheckedIPAddress(bind_aci, parse_netmask=True, allow_loopback=True)
 except (netaddr.AddrFormatError, ValueError) as e:
 return unicode(e)
 except UnboundLocalError:
@@ -439,7 +438,7 @@ def _normalize_bind_aci(bind_acis):
 
 try:
 ip = CheckedIPAddress(bind_aci, parse_netmask=True,
-  allow_network=True, allow_loopback=True)
+  allow_loopback=True)
 if '/' in bind_aci:# addr with netmask
 netmask = "/%s" % ip.prefixlen
 else:
diff --git a/ipatests/test_ipapython/test_ipautil.py b/ipatests/test_ipapython/test_ipautil.py
index 8c0b9c4..ea9251b 100644
--- a/ipatests/test_ipapython/test_ipautil.py
+++ b/ipatests/test_ipapython/test_ipautil.py
@@ -44,6 +44,7 @@ def check_ipaddress():
 
 def test_ip_address():
 addrs = [
+('0.0.0.0/0',),
 ('10.11.12.13', (10, 11, 12, 13),   8),
 ('10.11.12.13/14',  (10, 11, 12, 13),   14),
 ('10.11.12.13%zoneid',),
@@ -53,10 +54,11 @@ def test_ip_address():
 ('127.0.0.1',),
 ('241.1.2.3',),
 ('169.254.1.2',),
-('10.11.12.0/24',),
+('10.11.12.0/24',   (10, 11, 12, 0),   24),
 ('224.5.6.7',),
 ('10.11.12.255/24',),
 
+('::/0',),
 ('2001::1', (0x2001, 0, 0, 0, 0, 0, 0, 1), 64),
 ('2001::1/72',  (0x2001, 0, 0, 0, 0, 0, 0, 1), 72),
 ('2001::1%zoneid',  (0x2001, 0, 0, 0, 0, 0, 0, 1), 64),
@@ -66,7 +68,7 @@ def test_ip_address():
 ('::1',),
 ('6789::1',),
 ('fe89::1',),
-('2001::/64',),
+('2001::/64',   (0x2001, 0, 0, 0, 0, 0, 0, 0), 64),
 ('ff01::1',),
 
 ('junk',)

From e4167ec9df06a0508602968ea9d9b69b370a56c5 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Fri, 2 Sep 2016 17:07:03 +0200
Subject: [PATCH 2/4] Allow broadcast ip addresses

Currently environments may use prefix /31 on point-to-point connections what
makes IPA validators to fail. IPA should not care if IP address is broadcast
or not. In some 

[Freeipa-devel] [freeipa PR#31] WebUI: add support for sub-CAs while revoking certificates and removing certificate hold (+rejected)

2016-09-07 Thread dkupka
pvomacka's pull request #31: "WebUI: add support for sub-CAs while revoking 
certificates and removing certificate hold" label *rejected* has been added

See the full pull-request at https://github.com/freeipa/freeipa/pull/31
-- 
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] 0102..0105 Better handling for cert-request to disabled CA

2016-09-07 Thread Martin Babinsky

On 09/06/2016 04:51 PM, Fraser Tweedale wrote:

On Tue, Aug 30, 2016 at 10:54:32AM +0200, Martin Babinsky wrote:

On 08/26/2016 04:19 AM, Fraser Tweedale wrote:

The attached patches add better handling of cert-request failure due
to target CA being disabled (#6260).  To do this, rather than go and
do extra work in Dogtag that we would depend on, instead I bite the
bullet and refactor ra.request_certificate to use the Dogtag REST
API, which correctly responds with status 409 in this case.

Switching RA to Dogtag REST API is an old ticket (#3437) so these
patches address it in part, and show the way forward for the rest of
it.

These patches don't technically depend on patch 0101 which adds the
ca-enable and ca-disable commands, but 0101 may help for testing :)

Thanks,
Fraser





Hi Fraser,

PATCH 102:

LGTM, but please use the standard ":param " annotations in the docstring for
`_ssldo` method. It will make out life easier if we decide to use Sphinx or
similar tool to auto-generate documentation from sources.

You can also add ":raises:" section describing that RemoteRetrieveError is
raised when use_session is True but the session cookie wasn't acquired. It
is kind of obvious but it may trip the uninitiated.

PATCH 103:

Due to magical behavior of our public errors, the exception body should look
like this:

--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1413,10 +1413,7 @@ class HTTPRequestError(RemoteRetrieveError):
 """

 errno = 4035
-
-def __init__(self, status=None, **kw):
-assert status is not None
-super(HTTPRequestError, self).__init__(status=status, **kw)
+format = _('Request failed with status %(status)s: %(reason)')

The format string will be then automatically be supplied with status and
reason if you pass them to the constructor ass you already do. The errors
will be also handled magically (such as status which is None etc.)

PATCH 104:

1.) please don't use bare except here:

"""
+try:
+resp_obj = json.loads(http_body)
+except:
+raise errors.RemoteRetrieveError(reason=_("Response from CA was
not valid JSON"))
"""

use 'except Exception' at least.

PATCH 105:

+if e.status == 409:  # pylint: disable=E1101
+raise errors.CertificateOperationError(
+error=_("CA '%s' is disabled") % ca)
+else:
+raise e
+

please use named errors instead of error codes in pylint annotations:
# pylint: disable=no-member


Thanks for your review, Martin.  Updated patches attached; they
address all mentioned issues.

Cheers,
Fraser



Thanks, ACK.

Pushed to:
ipa-4-4: b8491490c2dbb3b2db3ce64cd154b499142bc250
master: 520ad7d865ff147d3ff8819d3e384d7cbd69bfb7

--
Martin^3 Babinsky

--
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] [freeipa PR#64] cert: fix cert-find --certificate when the cert is not in LDAP (closed)

2016-09-07 Thread dkupka
jcholast's pull request #64: "cert: fix cert-find --certificate when the cert 
is not in LDAP" was closed

See the full pull-request at https://github.com/freeipa/freeipa/pull/64
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/64/head:pr64
git checkout pr64
-- 
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] [freeipa PR#64] cert: fix cert-find --certificate when the cert is not in LDAP (comment)

2016-09-07 Thread dkupka
dkupka commented on a pull request

"""
Fixed upstream
master:
https://fedorahosted.org/freeipa/changeset/b7b6faf14aaa8ac677ab9ebc2bcbf87e6b2a1146
ipa-4-4:
https://fedorahosted.org/freeipa/changeset/5d4f7b78bc4d179544810419f73ec4d48b0a2a76
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/64#issuecomment-245243898
-- 
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] [freeipa PR#64] cert: fix cert-find --certificate when the cert is not in LDAP (+pushed)

2016-09-07 Thread dkupka
jcholast's pull request #64: "cert: fix cert-find --certificate when the cert 
is not in LDAP" label *pushed* has been added

See the full pull-request at https://github.com/freeipa/freeipa/pull/64
-- 
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] [freeipa PR#64] cert: fix cert-find --certificate when the cert is not in LDAP (+ack)

2016-09-07 Thread dkupka
jcholast's pull request #64: "cert: fix cert-find --certificate when the cert 
is not in LDAP" label *ack* has been added

See the full pull-request at https://github.com/freeipa/freeipa/pull/64
-- 
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] 0101 Add ca-disable and ca-enable commands

2016-09-07 Thread Martin Babinsky

On 09/06/2016 04:49 PM, Fraser Tweedale wrote:

On Tue, Aug 30, 2016 at 10:23:10AM +0200, Martin Babinsky wrote:

On 08/30/2016 10:09 AM, Jan Cholasta wrote:

Hi,

On 30.8.2016 09:56, Martin Babinsky wrote:

On 08/25/2016 10:25 AM, Fraser Tweedale wrote:

Hi team,

The attached patch fixes
https://fedorahosted.org/freeipa/ticket/6257.

The behaviour of cert-request when the CA is disabled is not very
nice (it reports a server error from Dogtag).  The Dogtag REST
interface gives much better errors so I plan to move to it in a
later change (which will also address
https://fedorahosted.org/freeipa/ticket/3473, in part).

Thanks,
Fraser





HI Fraser,

I have a couple of comments below:

1.)
@@ -25,6 +33,10 @@ EXAMPLES:
 ipa ca-add puppet --desc "Puppet" \\
 --subject "CN=Puppet CA,O=EXAMPLE.COM"

+  Disable a CA.
+
+ipa ca-disable puppet
+
 """)

You missed an example of `ca-enable` command in the doc string.

2.)

Regarding implementation of ca_enable/disable, I think you can reduce
the amount of code duplication by employing a base class which will look
up the required sub-CA and call the RA backend method required by the
subclass. See the attached untested diff (passes lint) for details.


Looks like I forgot how to OOP while on PTO :) Honza is right, of course,
see the example code in the attached diff (again not tested, just a quick
example).


Updated patch attached, implemented inheritance suggestion and
expanding plugin help.

Thanks,
Fraser



Thanks, ACK.

Pushed to:
master: c7e0dbc4e174d0bb7577de18cdb2f414f4199c57
ipa-4-4: b037e54e457d731cd16144df7573f4c85d79368a

--
Martin^3 Babinsky

--
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] [freeipa PR#65] #6216 - webui: cert_revoke should use --cacn to set correct CA when revoking certificate (closed)

2016-09-07 Thread pvoborni
pvoborni's pull request #65: "#6216 - webui: cert_revoke should use --cacn to 
set correct CA when revoking certificate" was closed

See the full pull-request at https://github.com/freeipa/freeipa/pull/65
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/65/head:pr65
git checkout pr65
-- 
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] [freeipa PR#65] #6216 - webui: cert_revoke should use --cacn to set correct CA when revoking certificate (+pushed)

2016-09-07 Thread pvoborni
pvoborni's pull request #65: "#6216 - webui: cert_revoke should use --cacn to 
set correct CA when revoking certificate" label *pushed* has been added

See the full pull-request at https://github.com/freeipa/freeipa/pull/65
-- 
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] [freeipa PR#65] #6216 - webui: cert_revoke should use --cacn to set correct CA when revoking certificate (comment)

2016-09-07 Thread pvoborni
pvoborni commented on a pull request

"""
Fixed upstream
master:
https://fedorahosted.org/freeipa/changeset/7fea3914fbfc0748f26dfe41445b5f0d12f406e6
ipa-4-4:
https://fedorahosted.org/freeipa/changeset/a68da14654243821274848b9af57fec3dc2fdb39
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/65#issuecomment-245241413
-- 
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] 0107 Fix cert revocation when removing all certs via host/service-mod

2016-09-07 Thread Fraser Tweedale
Attached patch fixes https://fedorahosted.org/freeipa/ticket/6305

Thanks,
Fraser
From d4d7e77795f96a4970058e61d99c70522689b22d Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Wed, 7 Sep 2016 19:00:18 +1000
Subject: [PATCH] Fix cert revocation when removing all certs via
 host/service-mod

When removing all host/service certificates via host/service-mod
--certificate=, the removed certificates should be revoked, but they
are not.  Examine whether the --certificate option was provided to
determine whether certs should be revoked, instead of looking for a
cert list in the options (which in this case is empty).

Fixes: https://fedorahosted.org/freeipa/ticket/6305
---
 ipaserver/plugins/host.py| 3 ++-
 ipaserver/plugins/service.py | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py
index 
2362b6247af87b4ce63c21083e6bc8ac39db0804..7f63e94849b4a6f2ce871ec77b188c54d640ba94
 100644
--- a/ipaserver/plugins/host.py
+++ b/ipaserver/plugins/host.py
@@ -898,7 +898,8 @@ class host_mod(LDAPUpdate):
 certs_der = [x509.normalize_certificate(c) for c in certs]
 
 # revoke removed certificates
-if certs and self.api.Command.ca_is_enabled()['result']:
+ca_is_enabled = self.api.Command.ca_is_enabled()['result']
+if 'usercertificate' in options and ca_is_enabled:
 try:
 entry_attrs_old = ldap.get_entry(dn, ['usercertificate'])
 except errors.NotFound:
diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py
index 
093525f2e7cb84b18f0658dcb5d7c786e45c6ab6..c0590732470ac1200d4dd4ea1f089e4384a509b3
 100644
--- a/ipaserver/plugins/service.py
+++ b/ipaserver/plugins/service.py
@@ -701,7 +701,8 @@ class service_mod(LDAPUpdate):
 certs = entry_attrs.get('usercertificate') or []
 certs_der = [x509.normalize_certificate(c) for c in certs]
 # revoke removed certificates
-if certs and self.api.Command.ca_is_enabled()['result']:
+ca_is_enabled = self.api.Command.ca_is_enabled()['result']
+if 'usercertificate' in options and ca_is_enabled:
 try:
 entry_attrs_old = ldap.get_entry(dn, ['usercertificate'])
 except errors.NotFound:
-- 
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] 0106 Make host/service cert revocation aware of lightweight CAs

2016-09-07 Thread Fraser Tweedale
On Wed, Sep 07, 2016 at 10:39:59AM +0200, Jan Cholasta wrote:
> On 7.9.2016 10:28, Fraser Tweedale wrote:
> > On Wed, Sep 07, 2016 at 08:32:42AM +0200, Jan Cholasta wrote:
> > > On 6.9.2016 19:36, Fraser Tweedale wrote:
> > > > On Tue, Sep 06, 2016 at 10:19:14AM +0200, Jan Cholasta wrote:
> > > > > On 5.9.2016 17:30, Fraser Tweedale wrote:
> > > > > > On Mon, Sep 05, 2016 at 11:59:11PM +1000, Fraser Tweedale wrote:
> > > > > > > On Tue, Aug 30, 2016 at 10:39:16AM +0200, Jan Cholasta wrote:
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > On 26.8.2016 07:42, Fraser Tweedale wrote:
> > > > > > > > > On Fri, Aug 26, 2016 at 03:37:17PM +1000, Fraser Tweedale 
> > > > > > > > > wrote:
> > > > > > > > > > Hi all,
> > > > > > > > > > 
> > > > > > > > > > Attached patch fixes 
> > > > > > > > > > https://fedorahosted.org/freeipa/ticket/6221.
> > > > > > > > > > It depends on Honza's PR #20
> > > > > > > > > > https://github.com/freeipa/freeipa/pull/20.
> > > > > > > > > > 
> > > > > > > > > > Thanks,
> > > > > > > > > > Fraser
> > > > > > > > > > 
> > > > > > > > > It does help to attach the patch :)
> > > > > > > > 
> > > > > > > > I think it would be better to call cert-find once per 
> > > > > > > > host-del/service-del
> > > > > > > > with the --host/--service option specified. That way you'll get 
> > > > > > > > all
> > > > > > > > certificates for the given host/service at once.
> > > > > > > > 
> > > > > > > > Honza
> > > > > > > > 
> > > > > > > I agree that is a nicer approach.
> > > > > > > 
> > > > > > > 'revoke_certs' is called from several other places besides just
> > > > > > > host/service_del.  If we want to land this fix Real Soon I'd 
> > > > > > > suggest
> > > > > > > we either:
> > > > > > > 
> > > > > > > A) Define function 'revoke_certs_from_cert_find', call it from
> > > > > > > host/service_del, and leave 'revoke_certs' alone; or
> > > > > > > 
> > > > > > > B) Land the patch as-is and do a bigger refactor at a later time.
> > > > > > > 
> > > > > > > What do you think?
> > > > > 
> > > > Updated patch attached; comments inline.
> > > > 
> > > > > C) Use cert-find-based revoke_certs() everywhere; use the 
> > > > > --certificate
> > > > > option of cert-find in the other places to get information about 
> > > > > specific
> > > > > certificates.
> > > > > 
> > > > As discussed on IRC, I have implemented this option.  The caveat is
> > > > that for host/service-mod, we incur call to cert_find for each
> > > > removed certificate.
> > > 
> > > It's worth noting that A) and B) suffer from the same caveat.
> > > 
> > > > 
> > > > > > > 
> > > > > > Updated patch for option (A) is attached.
> > > > > 
> > > > > 1) Instead of
> > > > > 
> > > > > if result['status'] in {'REVOKED', 'REVOKED_EXPIRED'}:
> > > > > 
> > > > > use:
> > > > > 
> > > > > if result['revoked']:
> > > > > 
> > > > Done.
> > > > 
> > > > > 
> > > > > 2)
> > > > > 
> > > > > +if 'cacn' not in cert:
> > > > > +# cert is known to Dogtag, but CA appears to have been
> > > > > +# deleted.  We cannot revoke this cert via IPA anymore.
> > > > > +# We could go directly to Dogtag to revoke it, but the
> > > > > +# issuer's cert should have been revoked so never mind.
> > > > > +continue
> > > > > 
> > > > > Or, it could be a cert issued by a 3rd party CA.
> > > > > 
> > > > I updated to comment to include this.
> > > > 
> > > > > 
> > > > > 3) host-mod/service-mod do not revoke certs:
> > > > > 
> > > > > $ ipa cert-request test.csr --principal host/test.example.com
> > > > >   Serial number: 13
> > > > > 
> > > > > $ ipa cert-show 13
> > > > >   Revoked: False
> > > > >   Owner host: test.example.com
> > > > > 
> > > > > $ ipa host-mod test.example.com --certificate=
> > > > > 
> > > > > $ ipa cert-show 13
> > > > >   Revoked: False
> > > > > 
> > > > Nice find.  This was a pre-existing bug: nothing gets revoked when
> > > > all certs are removed.  Here is the fix:
> > > > 
> > > > -if certs and self.api.Command.ca_is_enabled()['result']:
> > > > +ca_is_enabled = self.api.Command.ca_is_enabled()['result']
> > > > +if 'usercertificate' in options and ca_is_enabled:
> > > >  ... revocation code
> > > 
> > > OK. Since it is a different bug, it should be fixed in a separate patch 
> > > and
> > > have a separate ticket.
> > > 
> > > > 
> > > > Finally, host/service-remove-cert does not revoke the cert because
> > > > of (I think) a bug in cert-find.  If the cert does not exist on a
> > > > host/service the cert-find cannot find it with --certificate option.
> > > > Because host/service-remove-cert uses a post_callback to revoke the
> > > > cert, cert-find doesn't find it thus no revocation occurs.
> > > > 
> > > > Honza could you check whether this is indeed a bug/limitation of
> > > > cert-find or is it the smog in Saigon affecting me?
> > > 
> > > It's a bug - FTFY, 

Re: [Freeipa-devel] [PATCH] 0106 Make host/service cert revocation aware of lightweight CAs

2016-09-07 Thread Jan Cholasta

On 7.9.2016 10:28, Fraser Tweedale wrote:

On Wed, Sep 07, 2016 at 08:32:42AM +0200, Jan Cholasta wrote:

On 6.9.2016 19:36, Fraser Tweedale wrote:

On Tue, Sep 06, 2016 at 10:19:14AM +0200, Jan Cholasta wrote:

On 5.9.2016 17:30, Fraser Tweedale wrote:

On Mon, Sep 05, 2016 at 11:59:11PM +1000, Fraser Tweedale wrote:

On Tue, Aug 30, 2016 at 10:39:16AM +0200, Jan Cholasta wrote:

Hi,

On 26.8.2016 07:42, Fraser Tweedale wrote:

On Fri, Aug 26, 2016 at 03:37:17PM +1000, Fraser Tweedale wrote:

Hi all,

Attached patch fixes https://fedorahosted.org/freeipa/ticket/6221.
It depends on Honza's PR #20
https://github.com/freeipa/freeipa/pull/20.

Thanks,
Fraser


It does help to attach the patch :)


I think it would be better to call cert-find once per host-del/service-del
with the --host/--service option specified. That way you'll get all
certificates for the given host/service at once.

Honza


I agree that is a nicer approach.

'revoke_certs' is called from several other places besides just
host/service_del.  If we want to land this fix Real Soon I'd suggest
we either:

A) Define function 'revoke_certs_from_cert_find', call it from
host/service_del, and leave 'revoke_certs' alone; or

B) Land the patch as-is and do a bigger refactor at a later time.

What do you think?



Updated patch attached; comments inline.


C) Use cert-find-based revoke_certs() everywhere; use the --certificate
option of cert-find in the other places to get information about specific
certificates.


As discussed on IRC, I have implemented this option.  The caveat is
that for host/service-mod, we incur call to cert_find for each
removed certificate.


It's worth noting that A) and B) suffer from the same caveat.






Updated patch for option (A) is attached.


1) Instead of

if result['status'] in {'REVOKED', 'REVOKED_EXPIRED'}:

use:

if result['revoked']:


Done.



2)

+if 'cacn' not in cert:
+# cert is known to Dogtag, but CA appears to have been
+# deleted.  We cannot revoke this cert via IPA anymore.
+# We could go directly to Dogtag to revoke it, but the
+# issuer's cert should have been revoked so never mind.
+continue

Or, it could be a cert issued by a 3rd party CA.


I updated to comment to include this.



3) host-mod/service-mod do not revoke certs:

$ ipa cert-request test.csr --principal host/test.example.com
  Serial number: 13

$ ipa cert-show 13
  Revoked: False
  Owner host: test.example.com

$ ipa host-mod test.example.com --certificate=

$ ipa cert-show 13
  Revoked: False


Nice find.  This was a pre-existing bug: nothing gets revoked when
all certs are removed.  Here is the fix:

-if certs and self.api.Command.ca_is_enabled()['result']:
+ca_is_enabled = self.api.Command.ca_is_enabled()['result']
+if 'usercertificate' in options and ca_is_enabled:
 ... revocation code


OK. Since it is a different bug, it should be fixed in a separate patch and
have a separate ticket.



Finally, host/service-remove-cert does not revoke the cert because
of (I think) a bug in cert-find.  If the cert does not exist on a
host/service the cert-find cannot find it with --certificate option.
Because host/service-remove-cert uses a post_callback to revoke the
cert, cert-find doesn't find it thus no revocation occurs.

Honza could you check whether this is indeed a bug/limitation of
cert-find or is it the smog in Saigon affecting me?


It's a bug - FTFY, .

Functional ACK. Full ACK once my fix is merged and the host/service-mod is
split off into a separate patch.


To clarify - you want only the fix discussed above in the separate
patch?


I want the fix for sub-CA revocation in one patch and the fix for 
host/service-mod with empty --certificate in other patch.


--
Jan Cholasta

--
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] 0106 Make host/service cert revocation aware of lightweight CAs

2016-09-07 Thread Fraser Tweedale
On Wed, Sep 07, 2016 at 08:32:42AM +0200, Jan Cholasta wrote:
> On 6.9.2016 19:36, Fraser Tweedale wrote:
> > On Tue, Sep 06, 2016 at 10:19:14AM +0200, Jan Cholasta wrote:
> > > On 5.9.2016 17:30, Fraser Tweedale wrote:
> > > > On Mon, Sep 05, 2016 at 11:59:11PM +1000, Fraser Tweedale wrote:
> > > > > On Tue, Aug 30, 2016 at 10:39:16AM +0200, Jan Cholasta wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > On 26.8.2016 07:42, Fraser Tweedale wrote:
> > > > > > > On Fri, Aug 26, 2016 at 03:37:17PM +1000, Fraser Tweedale wrote:
> > > > > > > > Hi all,
> > > > > > > > 
> > > > > > > > Attached patch fixes 
> > > > > > > > https://fedorahosted.org/freeipa/ticket/6221.
> > > > > > > > It depends on Honza's PR #20
> > > > > > > > https://github.com/freeipa/freeipa/pull/20.
> > > > > > > > 
> > > > > > > > Thanks,
> > > > > > > > Fraser
> > > > > > > > 
> > > > > > > It does help to attach the patch :)
> > > > > > 
> > > > > > I think it would be better to call cert-find once per 
> > > > > > host-del/service-del
> > > > > > with the --host/--service option specified. That way you'll get all
> > > > > > certificates for the given host/service at once.
> > > > > > 
> > > > > > Honza
> > > > > > 
> > > > > I agree that is a nicer approach.
> > > > > 
> > > > > 'revoke_certs' is called from several other places besides just
> > > > > host/service_del.  If we want to land this fix Real Soon I'd suggest
> > > > > we either:
> > > > > 
> > > > > A) Define function 'revoke_certs_from_cert_find', call it from
> > > > > host/service_del, and leave 'revoke_certs' alone; or
> > > > > 
> > > > > B) Land the patch as-is and do a bigger refactor at a later time.
> > > > > 
> > > > > What do you think?
> > > 
> > Updated patch attached; comments inline.
> > 
> > > C) Use cert-find-based revoke_certs() everywhere; use the --certificate
> > > option of cert-find in the other places to get information about specific
> > > certificates.
> > > 
> > As discussed on IRC, I have implemented this option.  The caveat is
> > that for host/service-mod, we incur call to cert_find for each
> > removed certificate.
> 
> It's worth noting that A) and B) suffer from the same caveat.
> 
> > 
> > > > > 
> > > > Updated patch for option (A) is attached.
> > > 
> > > 1) Instead of
> > > 
> > > if result['status'] in {'REVOKED', 'REVOKED_EXPIRED'}:
> > > 
> > > use:
> > > 
> > > if result['revoked']:
> > > 
> > Done.
> > 
> > > 
> > > 2)
> > > 
> > > +if 'cacn' not in cert:
> > > +# cert is known to Dogtag, but CA appears to have been
> > > +# deleted.  We cannot revoke this cert via IPA anymore.
> > > +# We could go directly to Dogtag to revoke it, but the
> > > +# issuer's cert should have been revoked so never mind.
> > > +continue
> > > 
> > > Or, it could be a cert issued by a 3rd party CA.
> > > 
> > I updated to comment to include this.
> > 
> > > 
> > > 3) host-mod/service-mod do not revoke certs:
> > > 
> > > $ ipa cert-request test.csr --principal host/test.example.com
> > >   Serial number: 13
> > > 
> > > $ ipa cert-show 13
> > >   Revoked: False
> > >   Owner host: test.example.com
> > > 
> > > $ ipa host-mod test.example.com --certificate=
> > > 
> > > $ ipa cert-show 13
> > >   Revoked: False
> > > 
> > Nice find.  This was a pre-existing bug: nothing gets revoked when
> > all certs are removed.  Here is the fix:
> > 
> > -if certs and self.api.Command.ca_is_enabled()['result']:
> > +ca_is_enabled = self.api.Command.ca_is_enabled()['result']
> > +if 'usercertificate' in options and ca_is_enabled:
> >  ... revocation code
> 
> OK. Since it is a different bug, it should be fixed in a separate patch and
> have a separate ticket.
> 
> > 
> > Finally, host/service-remove-cert does not revoke the cert because
> > of (I think) a bug in cert-find.  If the cert does not exist on a
> > host/service the cert-find cannot find it with --certificate option.
> > Because host/service-remove-cert uses a post_callback to revoke the
> > cert, cert-find doesn't find it thus no revocation occurs.
> > 
> > Honza could you check whether this is indeed a bug/limitation of
> > cert-find or is it the smog in Saigon affecting me?
> 
> It's a bug - FTFY, .
> 
> Functional ACK. Full ACK once my fix is merged and the host/service-mod is
> split off into a separate patch.
> 
To clarify - you want only the fix discussed above in the separate
patch?

Thanks,
Fraser

-- 
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] [freeipa PR#65] #6216 - webui: cert_revoke should use --cacn to set correct CA when revoking certificate (+ack)

2016-09-07 Thread stlaz
pvoborni's pull request #65: "#6216 - webui: cert_revoke should use --cacn to 
set correct CA when revoking certificate" label *ack* has been added

See the full pull-request at https://github.com/freeipa/freeipa/pull/65
-- 
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] [freeipa PR#65] #6216 - webui: cert_revoke should use --cacn to set correct CA when revoking certificate (comment)

2016-09-07 Thread stlaz
stlaz commented on a pull request

"""
Seems to be working as expected.
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/65#issuecomment-245210985
-- 
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] [freeipa PR#58] Ip addr validation (comment)

2016-09-07 Thread dkupka
dkupka commented on a pull request

"""
@mbasti-rh In last patch, please copy-paste the warnings also into 
replicainstall.py
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/58#issuecomment-245205422
-- 
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] [freeipa PR#31] WebUI: add support for sub-CAs while revoking certificates and removing certificate hold (closed)

2016-09-07 Thread pvoborni
pvomacka's pull request #31: "WebUI: add support for sub-CAs while revoking 
certificates and removing certificate hold" was closed

See the full pull-request at https://github.com/freeipa/freeipa/pull/31
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/31/head:pr31
git checkout pr31
-- 
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] [freeipa PR#31] WebUI: add support for sub-CAs while revoking certificates and removing certificate hold (comment)

2016-09-07 Thread pvoborni
pvoborni commented on a pull request

"""
Obsoleted by pull request #65
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/31#issuecomment-245200659
-- 
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] [freeipa PR#65] #6216 - webui: cert_revoke should use --cacn to set correct CA when revoking certificate (synchronize)

2016-09-07 Thread pvoborni
pvoborni's pull request #65: "#6216 - webui: cert_revoke should use --cacn to 
set correct CA when revoking certificate" was synchronize

See the full pull-request at https://github.com/freeipa/freeipa/pull/65
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/65/head:pr65
git checkout pr65
From aaa01ab7b38c00998c5da53c8f40096e248f86f0 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Fri, 26 Aug 2016 13:11:22 +0200
Subject: [PATCH] WebUI add support for sub-CAs while revoking certificates

Also the same for removing certificate hold.

https://fedorahosted.org/freeipa/ticket/6216
---
 install/ui/src/freeipa/certificate.js | 129 ++
 install/ui/src/freeipa/widget.js  |   1 +
 2 files changed, 100 insertions(+), 30 deletions(-)

diff --git a/install/ui/src/freeipa/certificate.js b/install/ui/src/freeipa/certificate.js
index e67c348..9ab4002 100755
--- a/install/ui/src/freeipa/certificate.js
+++ b/install/ui/src/freeipa/certificate.js
@@ -244,44 +244,104 @@ IPA.cert.download_dialog = function(spec) {
 return that;
 };
 
-IPA.cert.revoke_dialog = function(spec) {
+IPA.cert.revocation_reason_select_widget = function(spec) {
+spec = spec || {};
+
+var that = IPA.select_widget(spec);
+
+that.create_options = function() {
+for (var i=0; i

[Freeipa-devel] [freeipa PR#65] #6216 - webui: cert_revoke should use --cacn to set correct CA when revoking certificate (opened)

2016-09-07 Thread pvoborni
pvoborni's pull request #65: "#6216 - webui: cert_revoke should use --cacn to 
set correct CA when revoking certificate" was opened

PR body:
"""
This is Pavel's patch with changes mentioned in pull request #31 comment 1
"""

See the full pull-request at https://github.com/freeipa/freeipa/pull/65
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/65/head:pr65
git checkout pr65
From 75b7e9a0f491b148844a8aa49453cd8f542e7fa0 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Fri, 26 Aug 2016 12:50:00 +0200
Subject: [PATCH 1/3] Add support for additional options taken from table facet

Sometimes the entity_show command must be called with options which are gathered
from result of entity_find command. These options needs to be passed as
arguments in URL which points to details page.

This functionality is implemented to table facet. There is new property
'additional_navigation_arguments' which is prepared for array of attributes
which will be passed to URL.

Part of: https://fedorahosted.org/freeipa/ticket/6238
---
 install/ui/src/freeipa/facet.js | 49 -
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/install/ui/src/freeipa/facet.js b/install/ui/src/freeipa/facet.js
index 4553c5c..06eca18 100644
--- a/install/ui/src/freeipa/facet.js
+++ b/install/ui/src/freeipa/facet.js
@@ -1819,6 +1819,15 @@ exp.table_facet = IPA.table_facet = function(spec, no_init) {
 var that = IPA.facet(spec, no_init);
 
 /**
+ * Names of additional row attributes which will be send to another facet
+ * during navigation as URL parameters.
+ *
+ * @property {Array}
+ */
+that.additional_navigation_arguments = spec.additional_navigation_arguments;
+
+
+/**
  * Entity of data displayed in the table
  * @property {entity.entity}
  */
@@ -2268,6 +2277,38 @@ exp.table_facet = IPA.table_facet = function(spec, no_init) {
 
 
 /**
+ * Extract data from command response and return them.
+ *
+ * @param pkey {string} primary key of row which is chosen
+ * @param attrs {Array} names of attributes which will be extracted
+ */
+that.get_row_attribute_values = function(key, attrs) {
+var result = that.data.result.result;
+var options = {};
+var row;
+
+if (result) {
+for (var i=0, l=result.length; i
Date: Fri, 26 Aug 2016 13:03:58 +0200
Subject: [PATCH 2/3] WebUI: Fix showing certificates issued by sub-CA

The cert-show command needs to be called with cacn option. Cacn option is
passed using URL attribute.

https://fedorahosted.org/freeipa/ticket/6238
---
 install/ui/src/freeipa/certificate.js | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/install/ui/src/freeipa/certificate.js b/install/ui/src/freeipa/certificate.js
index 232bdbf..e67c348 100755
--- a/install/ui/src/freeipa/certificate.js
+++ b/install/ui/src/freeipa/certificate.js
@@ -1543,6 +1543,7 @@ return {
 row_enabled_attribute: 'status',
 facet_groups: [exp.facet_group],
 facet_group: 'certificates',
+additional_navigation_arguments: [ 'cacn' ],
 pagination: false,
 no_update: true,
 columns: [
@@ -1552,6 +1553,7 @@ return {
 width: '90px'
 },
 'subject',
+'cacn',
 

[Freeipa-devel] [freeipa PR#63] fix for 6238 "Unable to view certificates issued by Sub CA in Web UI" separated from pr31 (closed)

2016-09-07 Thread martbab
pvoborni's pull request #63: "fix for 6238 "Unable to view certificates issued 
by Sub CA in Web UI" separated from pr31" was closed

See the full pull-request at https://github.com/freeipa/freeipa/pull/63
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/63/head:pr63
git checkout pr63
-- 
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] [freeipa PR#63] fix for 6238 "Unable to view certificates issued by Sub CA in Web UI" separated from pr31 (+pushed)

2016-09-07 Thread martbab
pvoborni's pull request #63: "fix for 6238 "Unable to view certificates issued 
by Sub CA in Web UI" separated from pr31" label *pushed* has been added

See the full pull-request at https://github.com/freeipa/freeipa/pull/63
-- 
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] [freeipa PR#63] fix for 6238 "Unable to view certificates issued by Sub CA in Web UI" separated from pr31 (comment)

2016-09-07 Thread martbab
martbab commented on a pull request

"""
Fixed upstream
master:
https://fedorahosted.org/freeipa/changeset/40f923f56b4777e3e18c9f76ba1a745ed69ef0a6
https://fedorahosted.org/freeipa/changeset/64ac981dddcecf1176585b6e7b729cf38b24bcea
ipa-4-4:
https://fedorahosted.org/freeipa/changeset/0b76ba8723d7ba6f7657d0f7c17f2fc2a7356752
https://fedorahosted.org/freeipa/changeset/29af03aa4283883612bdc8cbd299f5caa6adee2b
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/63#issuecomment-245196074
-- 
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] [freeipa PR#50] Add cert checks in ipa-server-certinstall (synchronize)

2016-09-07 Thread flo-renaud
flo-renaud's pull request #50: "Add cert checks in ipa-server-certinstall" was 
synchronize

See the full pull-request at https://github.com/freeipa/freeipa/pull/50
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/50/head:pr50
git checkout pr50
From 835eb957473317be495753cd8988084cd6566caa Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud 
Date: Thu, 1 Sep 2016 13:56:24 +0200
Subject: [PATCH] Add cert checks in ipa-server-certinstall

When ipa-server-certinstall is called to install a new server certificate,
the prerequisite is that the certificate issuer must be already known by IPA.
This fix adds new checks to make sure that the tool exits before
modifying the target NSS database if it is not the case.
The fix consists in creating a temp NSS database with the CA certs from the
target NSS database + the new server cert and checking the new server cert
validity.

https://fedorahosted.org/freeipa/ticket/6263
---
 ipaserver/install/ipa_server_certinstall.py | 40 +++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/ipaserver/install/ipa_server_certinstall.py b/ipaserver/install/ipa_server_certinstall.py
index 0a8fb21..ecfeca1 100644
--- a/ipaserver/install/ipa_server_certinstall.py
+++ b/ipaserver/install/ipa_server_certinstall.py
@@ -25,8 +25,8 @@
 
 from ipaplatform.constants import constants
 from ipaplatform.paths import paths
-from ipapython import admintool
-from ipapython.certdb import get_ca_nickname
+from ipapython import admintool, ipautil
+from ipapython.certdb import get_ca_nickname, NSSDatabase
 from ipapython.dn import DN
 from ipalib import api, errors
 from ipalib.constants import CACERT
@@ -157,6 +157,38 @@ def install_http_cert(self):
 os.chown(os.path.join(dirname, 'key3.db'), 0, pent.pw_gid)
 os.chown(os.path.join(dirname, 'secmod.db'), 0, pent.pw_gid)
 
+def check_chain(self, pkcs12_filename, pkcs12_pin, nssdb):
+# create a temp nssdb
+with NSSDatabase() as tempnssdb:
+db_password = ipautil.ipa_generate_password()
+db_pwdfile = ipautil.write_tmp_file(db_password)
+tempnssdb.create_db(db_pwdfile.name)
+
+# import the PKCS12 file, then delete all CA certificates
+# this leaves only the server certs in the temp db
+tempnssdb.import_pkcs12(
+pkcs12_filename, db_pwdfile.name, pkcs12_pin)
+for nickname, flags in tempnssdb.list_certs():
+if 'u' not in flags:
+while tempnssdb.has_nickname(nickname):
+tempnssdb.delete_cert(nickname)
+
+# import all the CA certs from nssdb into the temp db
+for nickname, flags in nssdb.list_certs():
+if 'u' not in flags:
+cert = nssdb.get_cert_from_db(nickname)
+tempnssdb.add_cert(cert, nickname, flags)
+
+# now get the server certs from tempnssdb and check their validity
+try:
+for nick, flags in tempnssdb.find_server_certs():
+tempnssdb.verify_server_cert_validity(nick, api.env.host)
+except ValueError as e:
+raise admintool.ScriptError(
+"Error: %s\n"
+"Please run ipa-cacert-manage install and ipa-certupdate "
+"to install the CA certificate." % str(e))
+
 def import_cert(self, dirname, pkcs12_passwd, old_cert, principal, command):
 pkcs12_file, pin, ca_cert = installutils.load_pkcs12(
 cert_files=self.args,
@@ -167,6 +199,10 @@ def import_cert(self, dirname, pkcs12_passwd, old_cert, principal, command):
 
 dirname = os.path.normpath(dirname)
 cdb = certs.CertDB(api.env.realm, nssdir=dirname)
+
+# Check that the ca_cert is known and trusted
+self.check_chain(pkcs12_file.name, pin, cdb)
+
 try:
 ca_enabled = api.Command.ca_is_enabled()['result']
 if ca_enabled:
-- 
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] 0106 Make host/service cert revocation aware of lightweight CAs

2016-09-07 Thread Jan Cholasta

On 6.9.2016 19:36, Fraser Tweedale wrote:

On Tue, Sep 06, 2016 at 10:19:14AM +0200, Jan Cholasta wrote:

On 5.9.2016 17:30, Fraser Tweedale wrote:

On Mon, Sep 05, 2016 at 11:59:11PM +1000, Fraser Tweedale wrote:

On Tue, Aug 30, 2016 at 10:39:16AM +0200, Jan Cholasta wrote:

Hi,

On 26.8.2016 07:42, Fraser Tweedale wrote:

On Fri, Aug 26, 2016 at 03:37:17PM +1000, Fraser Tweedale wrote:

Hi all,

Attached patch fixes https://fedorahosted.org/freeipa/ticket/6221.
It depends on Honza's PR #20
https://github.com/freeipa/freeipa/pull/20.

Thanks,
Fraser


It does help to attach the patch :)


I think it would be better to call cert-find once per host-del/service-del
with the --host/--service option specified. That way you'll get all
certificates for the given host/service at once.

Honza


I agree that is a nicer approach.

'revoke_certs' is called from several other places besides just
host/service_del.  If we want to land this fix Real Soon I'd suggest
we either:

A) Define function 'revoke_certs_from_cert_find', call it from
host/service_del, and leave 'revoke_certs' alone; or

B) Land the patch as-is and do a bigger refactor at a later time.

What do you think?



Updated patch attached; comments inline.


C) Use cert-find-based revoke_certs() everywhere; use the --certificate
option of cert-find in the other places to get information about specific
certificates.


As discussed on IRC, I have implemented this option.  The caveat is
that for host/service-mod, we incur call to cert_find for each
removed certificate.


It's worth noting that A) and B) suffer from the same caveat.






Updated patch for option (A) is attached.


1) Instead of

if result['status'] in {'REVOKED', 'REVOKED_EXPIRED'}:

use:

if result['revoked']:


Done.



2)

+if 'cacn' not in cert:
+# cert is known to Dogtag, but CA appears to have been
+# deleted.  We cannot revoke this cert via IPA anymore.
+# We could go directly to Dogtag to revoke it, but the
+# issuer's cert should have been revoked so never mind.
+continue

Or, it could be a cert issued by a 3rd party CA.


I updated to comment to include this.



3) host-mod/service-mod do not revoke certs:

$ ipa cert-request test.csr --principal host/test.example.com
  Serial number: 13

$ ipa cert-show 13
  Revoked: False
  Owner host: test.example.com

$ ipa host-mod test.example.com --certificate=

$ ipa cert-show 13
  Revoked: False


Nice find.  This was a pre-existing bug: nothing gets revoked when
all certs are removed.  Here is the fix:

-if certs and self.api.Command.ca_is_enabled()['result']:
+ca_is_enabled = self.api.Command.ca_is_enabled()['result']
+if 'usercertificate' in options and ca_is_enabled:
 ... revocation code


OK. Since it is a different bug, it should be fixed in a separate patch 
and have a separate ticket.




Finally, host/service-remove-cert does not revoke the cert because
of (I think) a bug in cert-find.  If the cert does not exist on a
host/service the cert-find cannot find it with --certificate option.
Because host/service-remove-cert uses a post_callback to revoke the
cert, cert-find doesn't find it thus no revocation occurs.

Honza could you check whether this is indeed a bug/limitation of
cert-find or is it the smog in Saigon affecting me?


It's a bug - FTFY, .

Functional ACK. Full ACK once my fix is merged and the host/service-mod 
is split off into a separate patch.


--
Jan Cholasta

--
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] [freeipa PR#64] cert: fix cert-find --certificate when the cert is not in LDAP (opened)

2016-09-07 Thread jcholast
jcholast's pull request #64: "cert: fix cert-find --certificate when the cert 
is not in LDAP" was opened

PR body:
"""
Always return the cert specified in --certificate in cert-find result, even
when the cert is not found in LDAP.

https://fedorahosted.org/freeipa/ticket/6304
"""

See the full pull-request at https://github.com/freeipa/freeipa/pull/64
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/64/head:pr64
git checkout pr64
From ecab8d6ed81150ebf651270aa52116924c6c01ba Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 7 Sep 2016 08:06:10 +0200
Subject: [PATCH] cert: fix cert-find --certificate when the cert is not in
 LDAP

Always return the cert specified in --certificate in cert-find result, even
when the cert is not found in LDAP.

https://fedorahosted.org/freeipa/ticket/6304
---
 ipaserver/plugins/cert.py | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 6195a6b..8da1869 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -1266,17 +1266,15 @@ def _ldap_search(self, all, raw, pkey_only, no_members, timelimit,
 rule)
 filters.append(filter)
 
-cert = options.get('certificate')
-if cert is not None:
-filter = ldap.make_filter_from_attr('usercertificate', cert)
-filters.append(filter)
-
 result = collections.OrderedDict()
 complete = bool(filters)
 
-if cert is None:
+cert = options.get('certificate')
+if cert is not None:
+filter = ldap.make_filter_from_attr('usercertificate', cert)
+else:
 filter = '(usercertificate=*)'
-filters.append(filter)
+filters.append(filter)
 
 filter = ldap.combine_filters(filters, ldap.MATCH_ALL)
 try:
-- 
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