Re: [Freeipa-devel] [PATCH] 0106 Make host/service cert revocation aware of lightweight CAs
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)
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
[Freeipa-devel] [freeipa PR#63] fix for 6238 "Unable to view certificates issued by Sub CA in Web UI" separated from pr31 (comment)
pvoborni commented on a pull request """ These 2 commits were already ACKed in PR #31 so we can merge them. """ See the full comment at https://github.com/freeipa/freeipa/pull/63#issuecomment-245112325 -- 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 (+ack)
pvoborni's pull request #63: "fix for 6238 "Unable to view certificates issued by Sub CA in Web UI" separated from pr31" label *ack* 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 (opened)
pvoborni's pull request #63: "fix for 6238 "Unable to view certificates issued
by Sub CA in Web UI" separated from pr31" was opened
PR body:
"""
Pavel's patch separated from pr #31. Pavel is on vacation so he cannot split it.
"""
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
From 75b7e9a0f491b148844a8aa49453cd8f542e7fa0 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka
Date: Fri, 26 Aug 2016 12:50:00 +0200
Subject: [PATCH 1/2] 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/2] 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',
{
name: 'status',
width: '120px'
@@ -1645,6 +1647,7 @@ return {
fields: [
'serial_number',
'serial_number_hex',
+'cacn',
'subject',
{
name: 'issuer',
@@ -1772,6 +1775,10 @@ IPA.cert.details_facet = function(spec, no_init) {
var command = that.details_facet_create_refresh_command();
delete command.options.all;
delete command.options.rights;
+
+command.options = command.options || {};
+$.extend(command.options, { cacn: that.state.cacn });
+
return command;
};
--
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 (comment)
LiptonB commented on a pull request """ I've added a commit (Use data_sources option to define which fields are rendered) that simplifies the way we avoid rendering rules whose source data are missing, as discussed here: https://www.redhat.com/archives/freeipa-devel/2016-September/msg00051.html. I prefer this approach to the macros in the original implementation, but I'm leaving it as a separate commit in case you would like to compare them. """ See the full comment at https://github.com/freeipa/freeipa/pull/10#issuecomment-245096157 -- 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)
pvoborni commented on a pull request
"""
nack for f72ae94
it needs this update
```diff
diff --git a/install/ui/src/freeipa/certificate.js
b/install/ui/src/freeipa/certificate.js
index ad7fd87..9ab4002 100755
--- a/install/ui/src/freeipa/certificate.js
+++ b/install/ui/src/freeipa/certificate.js
@@ -268,6 +268,7 @@ IPA.cert.revoke_dialog = function(spec, no_init) {
spec = spec || {};
spec.width = spec.width || 500;
+spec.ok_label = spec.ok_label || '@i18n:buttons.revoke';
spec.sections = [
{
name: 'note',
@@ -308,12 +309,13 @@ IPA.cert.revoke_dialog = function(spec, no_init) {
}
];
-var that = IPA.dialog(spec);
+var that = IPA.confirm_dialog(spec);
that.open = function() {
+
+that.confirmed = false;
that.dialog_open();
that.set_cacn(that.facet.state.cacn);
-
};
that.get_reason = function() {
@@ -336,23 +338,6 @@ IPA.cert.revoke_dialog = function(spec, no_init) {
that.init = function() {
var note = text.get('@i18n:objects.cert.revoke_confirmation');
that.widgets.get_widget('note.note').html = note;
-
-that.create_button({
-name: 'revoke',
-label: '@i18n:buttons.revoke',
-click: function() {
-that.on_ok();
-that.close();
-}
-});
-
-that.create_button({
-name: 'cancel',
-label: '@i18n:buttons.cancel',
-click: function() {
-that.close();
-}
-});
};
```
ACK for 8622b9f and e874ac9 (#6238)
I'll create new separate pull requests for both #6238 and #6216. So that
#6238 can be pushed and aforementioned changes for #6216 reviewed.
"""
See the full comment at
https://github.com/freeipa/freeipa/pull/31#issuecomment-245094506
--
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)
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/6] 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
+++ b/install/share/csr/templates/ipa_macros.tmpl
@@ -0,0 +1,42 @
[Freeipa-devel] [freeipa PR#62] Configure Anonymous PKINIT on server install (synchronize)
simo5's pull request #62: "Configure Anonymous PKINIT on server install" was
synchronize
See the full pull-request at https://github.com/freeipa/freeipa/pull/62
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/62/head:pr62
git checkout pr62
From 0fdf1369c9402e9df76cd74ca32238eb480a1e4c Mon Sep 17 00:00:00 2001
From: Simo Sorce
Date: Tue, 26 Jul 2016 11:19:01 -0400
Subject: [PATCH] Configure Anonymous PKINIT on server install
Allow anonymous pkinit to be used so that unenrolled hosts can perform FAST
authentication (necessary for 2FA for example) using an anonymous krbtgt
obtained via Pkinit.
https://fedorahosted.org/freeipa/ticket/5678
Signed-off-by: Simo Sorce
---
client/ipa-client-install| 2 +-
install/share/kdc.conf.template | 2 +-
install/share/profiles/KDCs_PKINIT_Certs.cfg | 109 +++
install/share/profiles/Makefile.am | 1 +
ipaplatform/base/paths.py| 3 +-
ipapython/certmonger.py | 32 +---
ipapython/dogtag.py | 4 +
ipaserver/install/certs.py | 10 ++-
ipaserver/install/krbinstance.py | 49
ipaserver/install/server/common.py | 5 +-
ipaserver/install/server/install.py | 26 ---
ipaserver/install/server/replicainstall.py | 4 +-
ipaserver/plugins/cert.py| 65 +---
ipaserver/plugins/dogtag.py | 1 +
14 files changed, 260 insertions(+), 53 deletions(-)
create mode 100644 install/share/profiles/KDCs_PKINIT_Certs.cfg
diff --git a/client/ipa-client-install b/client/ipa-client-install
index 4a263b3..590f598 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -1175,7 +1175,7 @@ def configure_certmonger(fstore, subject_base, cli_realm, hostname, options,
subject = str(DN(('CN', hostname), subject_base))
passwd_fname = os.path.join(paths.IPA_NSSDB_DIR, 'pwdfile.txt')
try:
-certmonger.request_cert(nssdb=paths.IPA_NSSDB_DIR,
+certmonger.request_cert(certpath=paths.IPA_NSSDB_DIR,
nickname='Local IPA host',
subject=subject, dns=[hostname],
principal=principal,
diff --git a/install/share/kdc.conf.template b/install/share/kdc.conf.template
index 296b75b..ec53a1f 100644
--- a/install/share/kdc.conf.template
+++ b/install/share/kdc.conf.template
@@ -12,6 +12,6 @@
dict_file = $DICT_WORDS
default_principal_flags = +preauth
; admin_keytab = $KRB5KDC_KADM5_KEYTAB
- pkinit_identity = FILE:$KDC_PEM
+ pkinit_identity = FILE:$KDC_CERT,$KDC_KEY
pkinit_anchors = FILE:$CACERT_PEM
}
diff --git a/install/share/profiles/KDCs_PKINIT_Certs.cfg b/install/share/profiles/KDCs_PKINIT_Certs.cfg
new file mode 100644
index 000..c5e412b
--- /dev/null
+++ b/install/share/profiles/KDCs_PKINIT_Certs.cfg
@@ -0,0 +1,109 @@
+profileId=KDCs_PKINIT_Certs
+classId=caEnrollImpl
+desc=This certificate profile is for enrolling server certificates with IPA-RA agent authentication.
+visible=false
+enable=true
+enableBy=admin
+auth.instance_id=raCertAuth
+name=IPA-RA Agent-Authenticated Server Certificate Enrollment
+input.list=i1,i2
+input.i1.class_id=certReqInputImpl
+input.i2.class_id=submitterInfoInputImpl
+output.list=o1
+output.o1.class_id=certOutputImpl
+policyset.list=serverCertSet
+policyset.serverCertSet.list=1,2,3,4,5,6,7,8,9,10,11
+policyset.serverCertSet.1.constraint.class_id=subjectNameConstraintImpl
+policyset.serverCertSet.1.constraint.name=Subject Name Constraint
+policyset.serverCertSet.1.constraint.params.pattern=CN=[^,]+,.+
+policyset.serverCertSet.1.constraint.params.accept=true
+policyset.serverCertSet.1.default.class_id=subjectNameDefaultImpl
+policyset.serverCertSet.1.default.name=Subject Name Default
+policyset.serverCertSet.1.default.params.name=CN=$$request.req_subject_name.cn$$, $SUBJECT_DN_O
+policyset.serverCertSet.2.constraint.class_id=validityConstraintImpl
+policyset.serverCertSet.2.constraint.name=Validity Constraint
+policyset.serverCertSet.2.constraint.params.range=740
+policyset.serverCertSet.2.constraint.params.notBeforeCheck=false
+policyset.serverCertSet.2.constraint.params.notAfterCheck=false
+policyset.serverCertSet.2.default.class_id=validityDefaultImpl
+policyset.serverCertSet.2.default.name=Validity Default
+policyset.serverCertSet.2.default.params.range=731
+policyset.serverCertSet.2.default.params.startTime=0
+policyset.serverCertSet.3.constraint.class_id=keyConstraintImpl
+policyset.serverCertSet.3.constraint.name=Key Constraint
+policyset.serverCertSet.3.constraint.params.keyType=RSA
+policyset.serverCertSet.3.constraint.params.keyParameters=2048,3072,4096
+policyset.serverCertSet.3.default.class_id=userKeyDefaultImpl
+policyset.serverCertSet.3.default.name=Key Default
+policyset.serverCe
[Freeipa-devel] [freeipa PR#62] Configure Anonymous PKINIT on server install (synchronize)
simo5's pull request #62: "Configure Anonymous PKINIT on server install" was
synchronize
See the full pull-request at https://github.com/freeipa/freeipa/pull/62
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/62/head:pr62
git checkout pr62
From 255f171fcaa443bac586e38a2f7f30aff676739d Mon Sep 17 00:00:00 2001
From: Simo Sorce
Date: Tue, 26 Jul 2016 11:19:01 -0400
Subject: [PATCH] Configure Anonymous PKINIT on server install
Allow anonymous pkinit to be used so that unenrolled hosts can perform FAST
authentication (necessary for 2FA for example) using an anonymous krbtgt
obtained via Pkinit.
https://fedorahosted.org/freeipa/ticket/5678
Signed-off-by: Simo Sorce
---
client/ipa-client-install| 2 +-
install/share/kdc.conf.template | 2 +-
install/share/profiles/KDCs_PKINIT_Certs.cfg | 109 +++
install/share/profiles/Makefile.am | 1 +
ipaplatform/base/paths.py| 3 +-
ipapython/certmonger.py | 32 +---
ipapython/dogtag.py | 4 +
ipaserver/install/certs.py | 10 ++-
ipaserver/install/krbinstance.py | 49
ipaserver/install/server/common.py | 5 +-
ipaserver/install/server/install.py | 26 ---
ipaserver/install/server/replicainstall.py | 4 +-
ipaserver/plugins/cert.py| 65 +---
ipaserver/plugins/dogtag.py | 1 +
14 files changed, 260 insertions(+), 53 deletions(-)
create mode 100644 install/share/profiles/KDCs_PKINIT_Certs.cfg
diff --git a/client/ipa-client-install b/client/ipa-client-install
index 4a263b3..590f598 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -1175,7 +1175,7 @@ def configure_certmonger(fstore, subject_base, cli_realm, hostname, options,
subject = str(DN(('CN', hostname), subject_base))
passwd_fname = os.path.join(paths.IPA_NSSDB_DIR, 'pwdfile.txt')
try:
-certmonger.request_cert(nssdb=paths.IPA_NSSDB_DIR,
+certmonger.request_cert(certpath=paths.IPA_NSSDB_DIR,
nickname='Local IPA host',
subject=subject, dns=[hostname],
principal=principal,
diff --git a/install/share/kdc.conf.template b/install/share/kdc.conf.template
index 296b75b..ec53a1f 100644
--- a/install/share/kdc.conf.template
+++ b/install/share/kdc.conf.template
@@ -12,6 +12,6 @@
dict_file = $DICT_WORDS
default_principal_flags = +preauth
; admin_keytab = $KRB5KDC_KADM5_KEYTAB
- pkinit_identity = FILE:$KDC_PEM
+ pkinit_identity = FILE:$KDC_CERT,$KDC_KEY
pkinit_anchors = FILE:$CACERT_PEM
}
diff --git a/install/share/profiles/KDCs_PKINIT_Certs.cfg b/install/share/profiles/KDCs_PKINIT_Certs.cfg
new file mode 100644
index 000..c5e412b
--- /dev/null
+++ b/install/share/profiles/KDCs_PKINIT_Certs.cfg
@@ -0,0 +1,109 @@
+profileId=KDCs_PKINIT_Certs
+classId=caEnrollImpl
+desc=This certificate profile is for enrolling server certificates with IPA-RA agent authentication.
+visible=false
+enable=true
+enableBy=admin
+auth.instance_id=raCertAuth
+name=IPA-RA Agent-Authenticated Server Certificate Enrollment
+input.list=i1,i2
+input.i1.class_id=certReqInputImpl
+input.i2.class_id=submitterInfoInputImpl
+output.list=o1
+output.o1.class_id=certOutputImpl
+policyset.list=serverCertSet
+policyset.serverCertSet.list=1,2,3,4,5,6,7,8,9,10,11
+policyset.serverCertSet.1.constraint.class_id=subjectNameConstraintImpl
+policyset.serverCertSet.1.constraint.name=Subject Name Constraint
+policyset.serverCertSet.1.constraint.params.pattern=CN=[^,]+,.+
+policyset.serverCertSet.1.constraint.params.accept=true
+policyset.serverCertSet.1.default.class_id=subjectNameDefaultImpl
+policyset.serverCertSet.1.default.name=Subject Name Default
+policyset.serverCertSet.1.default.params.name=CN=$$request.req_subject_name.cn$$, $SUBJECT_DN_O
+policyset.serverCertSet.2.constraint.class_id=validityConstraintImpl
+policyset.serverCertSet.2.constraint.name=Validity Constraint
+policyset.serverCertSet.2.constraint.params.range=740
+policyset.serverCertSet.2.constraint.params.notBeforeCheck=false
+policyset.serverCertSet.2.constraint.params.notAfterCheck=false
+policyset.serverCertSet.2.default.class_id=validityDefaultImpl
+policyset.serverCertSet.2.default.name=Validity Default
+policyset.serverCertSet.2.default.params.range=731
+policyset.serverCertSet.2.default.params.startTime=0
+policyset.serverCertSet.3.constraint.class_id=keyConstraintImpl
+policyset.serverCertSet.3.constraint.name=Key Constraint
+policyset.serverCertSet.3.constraint.params.keyType=RSA
+policyset.serverCertSet.3.constraint.params.keyParameters=2048,3072,4096
+policyset.serverCertSet.3.default.class_id=userKeyDefaultImpl
+policyset.serverCertSet.3.default.name=Key Default
+policyset.serverCe
[Freeipa-devel] [freeipa PR#62] Configure Anonymous PKINIT on server install (synchronize)
simo5's pull request #62: "Configure Anonymous PKINIT on server install" was
synchronize
See the full pull-request at https://github.com/freeipa/freeipa/pull/62
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/62/head:pr62
git checkout pr62
From b8525fc326bfc6ef57bdfc308fe37bfbe175ca7c Mon Sep 17 00:00:00 2001
From: Simo Sorce
Date: Tue, 26 Jul 2016 11:19:01 -0400
Subject: [PATCH] Configure Anonymous PKINIT on server install
Allow anonymous pkinit to be used so that unenrolled hosts can perform FAST
authentication (necessary for 2FA for example) using an anonymous krbtgt
obtained via Pkinit.
Signed-off-by: Simo Sorce
---
client/ipa-client-install| 2 +-
install/share/kdc.conf.template | 2 +-
install/share/profiles/KDCs_PKINIT_Certs.cfg | 109 +++
install/share/profiles/Makefile.am | 1 +
ipaplatform/base/paths.py| 3 +-
ipapython/certmonger.py | 32 +---
ipapython/dogtag.py | 4 +
ipaserver/install/certs.py | 10 ++-
ipaserver/install/krbinstance.py | 49
ipaserver/install/server/common.py | 5 +-
ipaserver/install/server/install.py | 26 ---
ipaserver/install/server/replicainstall.py | 4 +-
ipaserver/plugins/cert.py| 65 +---
ipaserver/plugins/dogtag.py | 1 +
14 files changed, 260 insertions(+), 53 deletions(-)
create mode 100644 install/share/profiles/KDCs_PKINIT_Certs.cfg
diff --git a/client/ipa-client-install b/client/ipa-client-install
index 4a263b3..590f598 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -1175,7 +1175,7 @@ def configure_certmonger(fstore, subject_base, cli_realm, hostname, options,
subject = str(DN(('CN', hostname), subject_base))
passwd_fname = os.path.join(paths.IPA_NSSDB_DIR, 'pwdfile.txt')
try:
-certmonger.request_cert(nssdb=paths.IPA_NSSDB_DIR,
+certmonger.request_cert(certpath=paths.IPA_NSSDB_DIR,
nickname='Local IPA host',
subject=subject, dns=[hostname],
principal=principal,
diff --git a/install/share/kdc.conf.template b/install/share/kdc.conf.template
index 296b75b..ec53a1f 100644
--- a/install/share/kdc.conf.template
+++ b/install/share/kdc.conf.template
@@ -12,6 +12,6 @@
dict_file = $DICT_WORDS
default_principal_flags = +preauth
; admin_keytab = $KRB5KDC_KADM5_KEYTAB
- pkinit_identity = FILE:$KDC_PEM
+ pkinit_identity = FILE:$KDC_CERT,$KDC_KEY
pkinit_anchors = FILE:$CACERT_PEM
}
diff --git a/install/share/profiles/KDCs_PKINIT_Certs.cfg b/install/share/profiles/KDCs_PKINIT_Certs.cfg
new file mode 100644
index 000..c5e412b
--- /dev/null
+++ b/install/share/profiles/KDCs_PKINIT_Certs.cfg
@@ -0,0 +1,109 @@
+profileId=KDCs_PKINIT_Certs
+classId=caEnrollImpl
+desc=This certificate profile is for enrolling server certificates with IPA-RA agent authentication.
+visible=false
+enable=true
+enableBy=admin
+auth.instance_id=raCertAuth
+name=IPA-RA Agent-Authenticated Server Certificate Enrollment
+input.list=i1,i2
+input.i1.class_id=certReqInputImpl
+input.i2.class_id=submitterInfoInputImpl
+output.list=o1
+output.o1.class_id=certOutputImpl
+policyset.list=serverCertSet
+policyset.serverCertSet.list=1,2,3,4,5,6,7,8,9,10,11
+policyset.serverCertSet.1.constraint.class_id=subjectNameConstraintImpl
+policyset.serverCertSet.1.constraint.name=Subject Name Constraint
+policyset.serverCertSet.1.constraint.params.pattern=CN=[^,]+,.+
+policyset.serverCertSet.1.constraint.params.accept=true
+policyset.serverCertSet.1.default.class_id=subjectNameDefaultImpl
+policyset.serverCertSet.1.default.name=Subject Name Default
+policyset.serverCertSet.1.default.params.name=CN=$$request.req_subject_name.cn$$, $SUBJECT_DN_O
+policyset.serverCertSet.2.constraint.class_id=validityConstraintImpl
+policyset.serverCertSet.2.constraint.name=Validity Constraint
+policyset.serverCertSet.2.constraint.params.range=740
+policyset.serverCertSet.2.constraint.params.notBeforeCheck=false
+policyset.serverCertSet.2.constraint.params.notAfterCheck=false
+policyset.serverCertSet.2.default.class_id=validityDefaultImpl
+policyset.serverCertSet.2.default.name=Validity Default
+policyset.serverCertSet.2.default.params.range=731
+policyset.serverCertSet.2.default.params.startTime=0
+policyset.serverCertSet.3.constraint.class_id=keyConstraintImpl
+policyset.serverCertSet.3.constraint.name=Key Constraint
+policyset.serverCertSet.3.constraint.params.keyType=RSA
+policyset.serverCertSet.3.constraint.params.keyParameters=2048,3072,4096
+policyset.serverCertSet.3.default.class_id=userKeyDefaultImpl
+policyset.serverCertSet.3.default.name=Key Default
+policyset.serverCertSet.4.constraint.class_id=noConstraintImpl
+
[Freeipa-devel] [freeipa PR#62] Configure Anonymous PKINIT on server install (synchronize)
simo5's pull request #62: "Configure Anonymous PKINIT on server install" was
synchronize
See the full pull-request at https://github.com/freeipa/freeipa/pull/62
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/62/head:pr62
git checkout pr62
From 32ab40ceae858310c4780504ed1696f30270ade4 Mon Sep 17 00:00:00 2001
From: Simo Sorce
Date: Tue, 26 Jul 2016 11:19:01 -0400
Subject: [PATCH] Configure Anonymous PKINIT on server install
Allow anonymous pkinit to be used so that unenrolled hosts can perform FAST
authentication (necessary for 2FA for example) using an anonymous krbtgt
obtained via Pkinit.
Signed-off-by: Simo Sorce
---
client/ipa-client-install| 2 +-
install/share/kdc.conf.template | 2 +-
install/share/profiles/KDCs_PKINIT_Certs.cfg | 109 +++
install/share/profiles/Makefile.am | 1 +
ipaplatform/base/paths.py| 3 +-
ipapython/certmonger.py | 32 +---
ipapython/dogtag.py | 4 +
ipaserver/install/certs.py | 10 ++-
ipaserver/install/krbinstance.py | 49
ipaserver/install/server/common.py | 5 +-
ipaserver/install/server/install.py | 26 ---
ipaserver/install/server/replicainstall.py | 4 +-
ipaserver/plugins/cert.py| 64 +---
ipaserver/plugins/dogtag.py | 1 +
14 files changed, 259 insertions(+), 53 deletions(-)
create mode 100644 install/share/profiles/KDCs_PKINIT_Certs.cfg
diff --git a/client/ipa-client-install b/client/ipa-client-install
index 4a263b3..590f598 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -1175,7 +1175,7 @@ def configure_certmonger(fstore, subject_base, cli_realm, hostname, options,
subject = str(DN(('CN', hostname), subject_base))
passwd_fname = os.path.join(paths.IPA_NSSDB_DIR, 'pwdfile.txt')
try:
-certmonger.request_cert(nssdb=paths.IPA_NSSDB_DIR,
+certmonger.request_cert(certpath=paths.IPA_NSSDB_DIR,
nickname='Local IPA host',
subject=subject, dns=[hostname],
principal=principal,
diff --git a/install/share/kdc.conf.template b/install/share/kdc.conf.template
index 296b75b..ec53a1f 100644
--- a/install/share/kdc.conf.template
+++ b/install/share/kdc.conf.template
@@ -12,6 +12,6 @@
dict_file = $DICT_WORDS
default_principal_flags = +preauth
; admin_keytab = $KRB5KDC_KADM5_KEYTAB
- pkinit_identity = FILE:$KDC_PEM
+ pkinit_identity = FILE:$KDC_CERT,$KDC_KEY
pkinit_anchors = FILE:$CACERT_PEM
}
diff --git a/install/share/profiles/KDCs_PKINIT_Certs.cfg b/install/share/profiles/KDCs_PKINIT_Certs.cfg
new file mode 100644
index 000..c5e412b
--- /dev/null
+++ b/install/share/profiles/KDCs_PKINIT_Certs.cfg
@@ -0,0 +1,109 @@
+profileId=KDCs_PKINIT_Certs
+classId=caEnrollImpl
+desc=This certificate profile is for enrolling server certificates with IPA-RA agent authentication.
+visible=false
+enable=true
+enableBy=admin
+auth.instance_id=raCertAuth
+name=IPA-RA Agent-Authenticated Server Certificate Enrollment
+input.list=i1,i2
+input.i1.class_id=certReqInputImpl
+input.i2.class_id=submitterInfoInputImpl
+output.list=o1
+output.o1.class_id=certOutputImpl
+policyset.list=serverCertSet
+policyset.serverCertSet.list=1,2,3,4,5,6,7,8,9,10,11
+policyset.serverCertSet.1.constraint.class_id=subjectNameConstraintImpl
+policyset.serverCertSet.1.constraint.name=Subject Name Constraint
+policyset.serverCertSet.1.constraint.params.pattern=CN=[^,]+,.+
+policyset.serverCertSet.1.constraint.params.accept=true
+policyset.serverCertSet.1.default.class_id=subjectNameDefaultImpl
+policyset.serverCertSet.1.default.name=Subject Name Default
+policyset.serverCertSet.1.default.params.name=CN=$$request.req_subject_name.cn$$, $SUBJECT_DN_O
+policyset.serverCertSet.2.constraint.class_id=validityConstraintImpl
+policyset.serverCertSet.2.constraint.name=Validity Constraint
+policyset.serverCertSet.2.constraint.params.range=740
+policyset.serverCertSet.2.constraint.params.notBeforeCheck=false
+policyset.serverCertSet.2.constraint.params.notAfterCheck=false
+policyset.serverCertSet.2.default.class_id=validityDefaultImpl
+policyset.serverCertSet.2.default.name=Validity Default
+policyset.serverCertSet.2.default.params.range=731
+policyset.serverCertSet.2.default.params.startTime=0
+policyset.serverCertSet.3.constraint.class_id=keyConstraintImpl
+policyset.serverCertSet.3.constraint.name=Key Constraint
+policyset.serverCertSet.3.constraint.params.keyType=RSA
+policyset.serverCertSet.3.constraint.params.keyParameters=2048,3072,4096
+policyset.serverCertSet.3.default.class_id=userKeyDefaultImpl
+policyset.serverCertSet.3.default.name=Key Default
+policyset.serverCertSet.4.constraint.class_id=noConstraintImpl
+
[Freeipa-devel] [freeipa PR#50] Add cert checks in ipa-server-certinstall (synchronize)
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 64d335ee59209a7a396313df77c17fd26e14c599 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..2bd12f5 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
[Freeipa-devel] [freeipa PR#62] Configure Anonymous PKINIT on server install (comment)
simo5 commented on a pull request """ Note, I haven't looked into the upgrade of an existing server, so just posting it here for an initial review, and also for someone to pick it up if I can't finish the work on the upgrade path. @abbra @frasertweedale please take a look """ See the full comment at https://github.com/freeipa/freeipa/pull/62#issuecomment-245039584 -- 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#62] Configure Anonymous PKINIT on server install (opened)
simo5's pull request #62: "Configure Anonymous PKINIT on server install" was
opened
PR body:
"""
Allow anonymous pkinit to be used so that unenrolled hosts can perform FAST
authentication (necessary for 2FA for example) using an anonymous krbtgt
obtained via Pkinit.
Signed-off-by: Simo Sorce
"""
See the full pull-request at https://github.com/freeipa/freeipa/pull/62
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/62/head:pr62
git checkout pr62
From 724e7e845e574ef7e2091256ff49338e685585e5 Mon Sep 17 00:00:00 2001
From: Simo Sorce
Date: Tue, 26 Jul 2016 11:19:01 -0400
Subject: [PATCH] Configure Anonymous PKINIT on server install
Allow anonymous pkinit to be used so that unenrolled hosts can perform FAST
authentication (necessary for 2FA for example) using an anonymous krbtgt
obtained via Pkinit.
Signed-off-by: Simo Sorce
---
client/ipa-client-install| 2 +-
install/share/kdc.conf.template | 2 +-
install/share/profiles/KDCs_PKINIT_Certs.cfg | 109 +++
install/share/profiles/Makefile.am | 1 +
ipaplatform/base/paths.py| 3 +-
ipapython/certmonger.py | 32 +---
ipapython/dogtag.py | 2 +
ipaserver/install/certs.py | 10 ++-
ipaserver/install/krbinstance.py | 48
ipaserver/install/server/common.py | 5 +-
ipaserver/install/server/install.py | 26 ---
ipaserver/install/server/replicainstall.py | 4 +-
ipaserver/plugins/cert.py| 62 ---
ipaserver/plugins/dogtag.py | 1 +
14 files changed, 254 insertions(+), 53 deletions(-)
create mode 100644 install/share/profiles/KDCs_PKINIT_Certs.cfg
diff --git a/client/ipa-client-install b/client/ipa-client-install
index 4a263b3..590f598 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -1175,7 +1175,7 @@ def configure_certmonger(fstore, subject_base, cli_realm, hostname, options,
subject = str(DN(('CN', hostname), subject_base))
passwd_fname = os.path.join(paths.IPA_NSSDB_DIR, 'pwdfile.txt')
try:
-certmonger.request_cert(nssdb=paths.IPA_NSSDB_DIR,
+certmonger.request_cert(certpath=paths.IPA_NSSDB_DIR,
nickname='Local IPA host',
subject=subject, dns=[hostname],
principal=principal,
diff --git a/install/share/kdc.conf.template b/install/share/kdc.conf.template
index 296b75b..ec53a1f 100644
--- a/install/share/kdc.conf.template
+++ b/install/share/kdc.conf.template
@@ -12,6 +12,6 @@
dict_file = $DICT_WORDS
default_principal_flags = +preauth
; admin_keytab = $KRB5KDC_KADM5_KEYTAB
- pkinit_identity = FILE:$KDC_PEM
+ pkinit_identity = FILE:$KDC_CERT,$KDC_KEY
pkinit_anchors = FILE:$CACERT_PEM
}
diff --git a/install/share/profiles/KDCs_PKINIT_Certs.cfg b/install/share/profiles/KDCs_PKINIT_Certs.cfg
new file mode 100644
index 000..c5e412b
--- /dev/null
+++ b/install/share/profiles/KDCs_PKINIT_Certs.cfg
@@ -0,0 +1,109 @@
+profileId=KDCs_PKINIT_Certs
+classId=caEnrollImpl
+desc=This certificate profile is for enrolling server certificates with IPA-RA agent authentication.
+visible=false
+enable=true
+enableBy=admin
+auth.instance_id=raCertAuth
+name=IPA-RA Agent-Authenticated Server Certificate Enrollment
+input.list=i1,i2
+input.i1.class_id=certReqInputImpl
+input.i2.class_id=submitterInfoInputImpl
+output.list=o1
+output.o1.class_id=certOutputImpl
+policyset.list=serverCertSet
+policyset.serverCertSet.list=1,2,3,4,5,6,7,8,9,10,11
+policyset.serverCertSet.1.constraint.class_id=subjectNameConstraintImpl
+policyset.serverCertSet.1.constraint.name=Subject Name Constraint
+policyset.serverCertSet.1.constraint.params.pattern=CN=[^,]+,.+
+policyset.serverCertSet.1.constraint.params.accept=true
+policyset.serverCertSet.1.default.class_id=subjectNameDefaultImpl
+policyset.serverCertSet.1.default.name=Subject Name Default
+policyset.serverCertSet.1.default.params.name=CN=$$request.req_subject_name.cn$$, $SUBJECT_DN_O
+policyset.serverCertSet.2.constraint.class_id=validityConstraintImpl
+policyset.serverCertSet.2.constraint.name=Validity Constraint
+policyset.serverCertSet.2.constraint.params.range=740
+policyset.serverCertSet.2.constraint.params.notBeforeCheck=false
+policyset.serverCertSet.2.constraint.params.notAfterCheck=false
+policyset.serverCertSet.2.default.class_id=validityDefaultImpl
+policyset.serverCertSet.2.default.name=Validity Default
+policyset.serverCertSet.2.default.params.range=731
+policyset.serverCertSet.2.default.params.startTime=0
+policyset.serverCertSet.3.constraint.class_id=keyConstraintImpl
+policyset.serverCertSet.3.constraint.name=Key Constraint
+policyset.serverCertSet.3.constraint.params.keyType=RSA
+policyset.serverCertSet.3.constraint.params
Re: [Freeipa-devel] [PATCH] 0106 Make host/service cert revocation aware of lightweight CAs
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.
> > >
> > 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
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?
Thanks,
Fraser
From 9c829d7ec8ff67dcf814c468c406772bf311c9f8 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale
Date: Fri, 26 Aug 2016 15:31:13 +1000
Subject: [PATCH] Make host/service cert revocation aware of lightweight CAs
Revocation of host/service certs on host/service deletion or other
operations is broken when cert is issued by a lightweight (sub)CA,
causing the delete operation to be aborted. Look up the issuing CA
and pass it to 'cert_revoke' to fix the issue.
Fixes: https://fedorahosted.org/freeipa/ticket/6221
---
ipaserver/plugins/host.py| 23 +
ipaserver/plugins/service.py | 59 ++--
2 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py
index
03c64c637cbba0aee1b6569f3b5dbe200953bff8..7f63e94849b4a6f2ce871ec77b188c54d640ba94
100644
--- a/ipaserver/plugins/host.py
+++ b/ipaserver/plugins/host.py
@@ -843,12 +843,8 @@ class host_del(LDAPDelete):
)
if self.api.Command.ca_is_enabled()['result']:
-try:
-entry_attrs = ldap.get_entry(dn, ['usercertificate'])
-except errors.NotFound:
-self.obj.handle_not_found(*keys)
-
-revoke_certs(entry_attrs.get('usercertificate', []), self.log)
+certs = self.api.Command.cert_find(host=keys)['result']
+revoke_certs(certs)
return
[Freeipa-devel] [freeipa PR#61] Use Travis-CI for basic sanity checks (+pushed)
martbab's pull request #61: "Use Travis-CI for basic sanity checks" label *pushed* has been added See the full pull-request at https://github.com/freeipa/freeipa/pull/61 -- 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#61] Use Travis-CI for basic sanity checks (closed)
martbab's pull request #61: "Use Travis-CI for basic sanity checks" was closed See the full pull-request at https://github.com/freeipa/freeipa/pull/61 ... or pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/61/head:pr61 git checkout pr61 -- 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#61] Use Travis-CI for basic sanity checks (comment)
mbasti-rh commented on a pull request """ Fixed upstream master: https://fedorahosted.org/freeipa/changeset/37f3ad8867f347289adcadcc473871c54aa9ca9d """ See the full comment at https://github.com/freeipa/freeipa/pull/61#issuecomment-244993903 -- 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#61] Use Travis-CI for basic sanity checks (+ack)
martbab's pull request #61: "Use Travis-CI for basic sanity checks" label *ack* has been added See the full pull-request at https://github.com/freeipa/freeipa/pull/61 -- 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#61] Use Travis-CI for basic sanity checks (synchronize)
martbab's pull request #61: "Use Travis-CI for basic sanity checks" was
synchronize
See the full pull-request at https://github.com/freeipa/freeipa/pull/61
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/61/head:pr61
git checkout pr61
From 08e345b4b93fd982d54b1e47d8390e54ef7a87c9 Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Mon, 5 Sep 2016 10:19:40 +0200
Subject: [PATCH] Use Travis-CI for basic sanity checks
This patch adds the config file for Travis CI. The config file instructs the
CI to:
* check pep8 errors in PR
* build RPMs in pulled in Fedora builder container
(docker.io/martbab/freeipa-fedora-builder)
These basic checks should eliminate basic errors that can break the build
itself (formatting errors, Syntax errors/undeclared variables, missing
BuildRequires, broken API.txt, etc.). It does not run any of our
integration/unit tests.
---
.travis.yml | 16
1 file changed, 16 insertions(+)
create mode 100644 .travis.yml
diff --git a/.travis.yml b/.travis.yml
new file mode 100644
index 000..f221e82
--- /dev/null
+++ b/.travis.yml
@@ -0,0 +1,16 @@
+services:
+- docker
+
+before_install:
+- pip install pep8
+
+script:
+- >
+if [[ "$TRAVIS_EVENT_TYPE" == "pull_request" ]];
+then
+git diff origin/${TRAVIS_BRANCH} -U0 | pep8 --diff;
+fi
+- >
+docker run -v $PWD:/freeipa -w /freeipa
+martbab/freeipa-fedora-builder:${TRAVIS_BRANCH}-latest
+/bin/bash -c 'dnf builddep -y --spec freeipa.spec.in && make rpms'
--
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#61] Use Travis-CI for basic sanity checks (synchronize)
martbab's pull request #61: "Use Travis-CI for basic sanity checks" was synchronize See the full pull-request at https://github.com/freeipa/freeipa/pull/61 ... or pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/61/head:pr61 git checkout pr61 From d9b1bf57d6a03bcbe762bbe9142a3b9eccb4c7b9 Mon Sep 17 00:00:00 2001 From: Martin Babinsky Date: Mon, 5 Sep 2016 10:19:40 +0200 Subject: [PATCH] Use Travis-CI for basic sanity checks This patch adds the config file for Travis CI. The config file instructs the CI to: * check pep8 errors in PR * build RPMs in pulled in Fedora builder container (docker.io/martbab/freeipa-fedora-builder) These basic checks should eliminate basic errors that can break the build itself (formatting errors, Syntax errors/undeclared variables, missing BuildRequires, broken API.txt, etc.). It does not run any of our integration/unit tests. --- .travis.yml | 16 1 file changed, 16 insertions(+) create mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 000..0b9f2e3 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,16 @@ +services: +- docker + +before_install: +- pip install pep8 + +script: +- > +if [[ "$TRAVIS_EVENT_TYPE" == "pull_request" ]]; +then +git diff origin/master -U0 | pep8 --diff; +fi +- > +docker run -v $PWD:/freeipa -w /freeipa +martbab/freeipa-fedora-builder:master-latest +/bin/bash -c 'dnf builddep -y --spec freeipa.spec.in && make rpms' -- 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
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
From a1aa93ed13a24c9ac946e47ecd49606ebad8bd9e Mon Sep 17 00:00:00 2001
From: Fraser Tweedale
Date: Fri, 26 Aug 2016 08:59:10 +1000
Subject: [PATCH 102/105] Allow Dogtag RestClient to perform requests without
logging in
Currently the Dogtag RestClient '_ssldo' method requires a session
cookie unconditionally, however, not all REST methods require a
session: some do not require authentication at all, and some will
authenticate the agent on the fly.
To avoid unnecessary login/logout requests via the context manager,
add the 'use_session' keyword argument to '_ssldo'. It defaults to
'True' to preserve existing behaviour (session required) but a
caller can set to 'False' to avoid the requirement.
Part of: https://fedorahosted.org/freeipa/ticket/6260
Part of: https://fedorahosted.org/freeipa/ticket/3473
---
ipaserver/plugins/dogtag.py | 36
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index
01e5f1383ee135696a8e968793863ce964025094..f3fb2703f4e1ea688e38cecd02c9acc79213eb40
100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -2071,26 +2071,38 @@ class RestClient(Backend):
)
self.cookie = None
-def _ssldo(self, method, path, headers=None, body=None):
+def _ssldo(self, method, path, headers=None, body=None, use_session=True):
"""
-:param url: The URL to post to.
-:param kw: Keyword arguments to encode into POST body.
+Perform an HTTPS request.
+
+:param method: HTTP method to use
+:param path: Path component. This will *extend* the path defined for
+the class (if any).
+:param headers: Additional headers to include in the request.
+:param body: Request body.
+:param use_session: If ``True``, session cookie is added to request
+(client must be logged in).
+
:return: (http_status, http_headers, http_body)
as (integer, dict, str)
-Perform an HTTPS request
-"""
-if self.cookie is None:
-raise errors.RemoteRetrieveError(
-reason=_("REST API is not logged in."))
+
Re: [Freeipa-devel] [PATCH] 0101 Add ca-disable and ca-enable commands
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
From 61adc46ec9a19f1044231d193a0d9cdef0adba64 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale
Date: Thu, 25 Aug 2016 17:00:01 +1000
Subject: [PATCH] Add ca-disable and ca-enable commands
We soon plan to revoke certificates upon lightweight CA deletion.
This makes it important to provide a way to prevent a CA from
issuing certificates whilst not deleting and revoking it, and
continuing to allow management of issued certs.
This commit adds the ca-disable and ca-enable commands.
Fixes: https://fedorahosted.org/freeipa/ticket/6257
---
API.txt | 16 +++
VERSION | 4 +--
ipaserver/plugins/ca.py | 66 +++--
ipaserver/plugins/dogtag.py | 6 +
4 files changed, 88 insertions(+), 4 deletions(-)
diff --git a/API.txt b/API.txt
index
5b83bfbd0b457b77e0522ab7d83abfae4df3ebe9..27b64ee143fa4f5f55c1b8a32446f004a8e3bb22
100644
--- a/API.txt
+++ b/API.txt
@@ -465,6 +465,20 @@ option: Str('version?')
output: Output('result', type=[])
output: Output('summary', type=[, ])
output: ListOfPrimaryKeys('value')
+command: ca_disable/1
+args: 1,1,3
+arg: Str('cn', cli_name='name')
+option: Str('version?')
+output: Output('result', type=[])
+output: Output('summary', type=[, ])
+output: PrimaryKey('value')
+command: ca_enable/1
+args: 1,1,3
+arg: Str('cn', cli_name='name')
+option: Str('version?')
+output: Output('result', type=[])
+output: Output('summary', type=[, ])
+output: PrimaryKey('value')
command: ca_find/1
args: 1,11,4
arg: Str('criteria?')
@@ -6249,6 +6263,8 @@ default: batch/1
default: ca/1
default: ca_add/1
default: ca_del/1
+default: ca_disable/1
+default: ca_enable/1
default: ca_find/1
default: ca_is_enabled/1
default: ca_mod/1
diff --git a/VERSION b/VERSION
index
bf8160a5deb1f7a5148ef6833cec318af144b5d7..c6fb1cba2757d919a88093ca3f060f80b2d30621
100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=212
-# Last change: ab: service: add flag to allow S4U2Self
+IPA_API_VERSION_MINOR=213
+# Last change: ftweedal: add ca-disable and ca-enable commands
diff --git a/ipaserver/plugins/ca.py b/ipaserver/plugins/ca.py
index
966ae2b1bdb4bb0207dfa58f0e9c951bc930f766..4d83fe81c951b01d06d3c85d74fe94e24bce0b1f
100644
--- a/ipaserver/plugins/ca.py
+++ b/ipaserver/plugins/ca.py
@@ -2,12 +2,12 @@
# Copyright (C) 2016 FreeIPA Contributors see COPYING for license
#
-from ipalib import api, errors, DNParam, Str
+from ipalib import api, errors, output, DNParam, Str
from ipalib.constants import IPA_CA_CN
from ipalib.plugable import Registry
from ipaserver.plugins.baseldap import (
LDAPObject, LDAPSearch, LDAPCreate, LDAPDelete,
-LDAPUpdate, LDAPRetrieve)
+LDAPUpdate, LDAPRetrieve, LDAPQuery, pkey_to_value)
from ipaserver.plugins.cert import ca_enabled_check
from ipalib import _, ngettext
@@ -18,6 +18,14 @@ Manage Certificate Authorities
Subordinate Certificate Authorities (Sub-CAs) can be added for scoped issuance
of X.509 certificates.
+CAs are enabled on creation, but their use is subject to CA ACLs unl
Re: [Freeipa-devel] [PATCH] ca-less tests updated
Hi Oleg!
0013 - It looks like there are two unrelated changes, addition of CRL
distribution extension and creating certificate signed by no longer
existing CA. Please create separate patch for each of the changes, and
describe the change and reason for it in commit messages.
0014 - Could you please split the patch to "numerous" commit each fixing
one error? Please also describe each fix so everyone has at least vague
idea about the patch without reading its code. Also why do you introduce
global variable config, I don't see its used anywhere.
0039 - It looks like multiple different changes and commit message says
nothing again. Please split and describe what did you change and why.
0041 - Looks like weird workaround to me. It would be better to
investigate the root cause and fix it. Or at least describe the cause in
commit message and code comment if it can't be fixed. Also "-h is
deprecated in favor of -H" says man 1 ldapmodify.
On 05/09/16 14:32, Oleg Fayans wrote:
Hi guys,
Finally the ca-less tests are stable. Here in the attachment is the full
set of necessary patches.
On 08/09/2016 10:57 AM, Oleg Fayans wrote:
Hi all,
Bump for the review of the 0013 patch. The script it addresses can be
reused in some WebUI tests - one more reason to have it reviewed/merged
The rest patches should be re-tested, since they were prepared a good
while ago
On 05/10/2016 05:08 PM, Oleg Fayans wrote:
Hi David,
After quite a while and some more struggles here comes the updated
version of the patch together with other patches fixing things in
ipatests/test_integration/tasks.py
Server and replica installation was refactored in a way to utilize the
code from tasks.py as much as it is possible
The full set of necessary patches is attached
On 04/20/2016 10:35 AM, David Kupka wrote:
On 19/04/16 11:13, Oleg Fayans wrote:
OK, that one, though passing lint, did not actually work. I gave up my
attempts to define method decorators inside the class. Now it passes
lint AND works:)
Hi Oleg!
1) Current commit message is useless. Please use it to describe what is
the point of the patch.
2) $ git show -U0 | pep8 --diff
./ipatests/test_integration/test_caless.py:66:1: E302 expected 2 blank
lines, found 1
./ipatests/test_integration/test_caless.py:74:1: E302 expected 2 blank
lines, found 1
./ipatests/test_integration/test_caless.py:820:5: E303 too many blank
lines (2)
./ipatests/test_integration/test_caless.py:825:80: E501 line too long
(80 > 79 characters)
./ipatests/test_integration/test_caless.py:1035:44: E225 missing
whitespace around operator
3) Isn't there a way to do this with pytest's fixtures?
+def server_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+args[0].uninstall_server()
+return wrapped
+
+def replica_install_teardown(func):
+def wrapped(*args):
+try:
+func(*args)
+finally:
+# Uninstall replica
+replica = args[0].replicas[0]
+tasks.kinit_admin(args[0].master)
+args[0].uninstall_server(replica)
+args[0].master.run_command(['ipa-replica-manage', 'del',
+replica.hostname, '--force'],
+ raiseonerr=False)
+args[0].master.run_command(['ipa', 'host-del',
+replica.hostname],
+ raiseonerr=False)
+return wrapped
+
There is a standard pytest method called 'method_teardown', that is
indent to be executed after each test method, but with our setup it does
not work.
4) Is it necessary to create the $TEST_DIR in the test? Isn't it
created
by the framework?
+host.transport.mkdir_recursive(host.config.test_dir)
Removed.
5) I don't think the comment match the code.
+# Remove CA cert in /etc/pki/nssdb, in case of failed
(un)install
+for host in cls.get_all_hosts():
+cls.uninstall_server(host)
+
super(CALessBase, cls).uninstall(mh)
Not actual anymore
6) No! Create list with one element, iterate that list and append every
item to the other list. Maybe there's better way (Hint: append).
I've seen this on multiple places.
if unattended:
args.extend(['-U'])
Agreed
7) Why don't you (extend and) use
ipatests.test_integaration.tasks.(un)install_{master,replica}?
This could be done pretty much all over the code.
host.run_command(['ipa-server-install', '--uninstall',
'-U'])
8) Use ipaplatform.paths for certutil and other binaries. If the binary
is not there feel free to add it.
I've seen this on multiple places.
+host.run_command(['certutil', '-d', paths.NSS_DB_DIR, '-D',
+ '-n', 'External CA cert'],
+ raiseonerr=False)
+# A workaround
forhttps://fedorahosted.org/freeipa/ticket/4639
+
[Freeipa-devel] [freeipa PR#55] Fix parse errors with link-local addresses (comment)
mbasti-rh commented on a pull request """ Fixed upstream master: https://fedorahosted.org/freeipa/changeset/db55bde15dd83a0ed29205a127cefabe691e81b1 ipa-4-4: https://fedorahosted.org/freeipa/changeset/d900c229f484c99a65ff5398de25057c50a6eef1 """ See the full comment at https://github.com/freeipa/freeipa/pull/55#issuecomment-244971711 -- 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#55] Fix parse errors with link-local addresses (+pushed)
mbasti-rh's pull request #55: "Fix parse errors with link-local addresses" label *pushed* has been added See the full pull-request at https://github.com/freeipa/freeipa/pull/55 -- 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#55] Fix parse errors with link-local addresses (closed)
mbasti-rh's pull request #55: "Fix parse errors with link-local addresses" was closed See the full pull-request at https://github.com/freeipa/freeipa/pull/55 ... or pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/55/head:pr55 git checkout pr55 -- 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 (comment)
jcholast commented on a pull request """ More comments inline. """ See the full comment at https://github.com/freeipa/freeipa/pull/50#issuecomment-244967015 -- 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#47] schema cache: Store and check info for pre-schema servers (comment)
jcholast commented on a pull request """ Fixed upstream master: https://fedorahosted.org/freeipa/changeset/ec2401917456d6f643532c0d0218c9e75172c2d8 ipa-4-4: https://fedorahosted.org/freeipa/changeset/2be232f67074ef052debb91962dbc8acd09d45bd """ See the full comment at https://github.com/freeipa/freeipa/pull/47#issuecomment-244965424 -- 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#47] schema cache: Store and check info for pre-schema servers (closed)
dkupka's pull request #47: "schema cache: Store and check info for pre-schema servers" was closed See the full pull-request at https://github.com/freeipa/freeipa/pull/47 ... or pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/47/head:pr47 git checkout pr47 -- 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#47] schema cache: Store and check info for pre-schema servers (+pushed)
dkupka's pull request #47: "schema cache: Store and check info for pre-schema servers" label *pushed* has been added See the full pull-request at https://github.com/freeipa/freeipa/pull/47 -- 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#47] schema cache: Store and check info for pre-schema servers (+ack)
dkupka's pull request #47: "schema cache: Store and check info for pre-schema servers" label *ack* has been added See the full pull-request at https://github.com/freeipa/freeipa/pull/47 -- 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#61] Use Travis-CI for basic sanity checks (opened)
martbab's pull request #61: "Use Travis-CI for basic sanity checks" was opened PR body: """ This patch adds the config file for Travis CI. The config file instructs the CI to: * check pep8 errors in PR * pull in a freeipa builder container image from docker.io/martbab/freeipa-fedora-builder * build RPMs in pulled container These basic checks should eliminate basic errors that can break the build itself, it does not run any of our integration/unit tests. """ See the full pull-request at https://github.com/freeipa/freeipa/pull/61 ... or pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/61/head:pr61 git checkout pr61 From 807925dc8c276eefb3b57d96ad548575ce746a03 Mon Sep 17 00:00:00 2001 From: Martin Babinsky Date: Mon, 5 Sep 2016 10:19:40 +0200 Subject: [PATCH] Use Travis-CI for basic sanity checks This patch adds the config file for Travis CI. The config file instructs the CI to: * check pep8 errors in PR * pull in a freeipa builder container image from docker.io/martbab/freeipa-fedora-builder * build RPMs in pulled container These basic checks should eliminate basic errors that can break the build itself, it does not run any of our integration/unit tests. --- .travis.yml | 16 1 file changed, 16 insertions(+) create mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 000..0b9f2e3 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,16 @@ +services: +- docker + +before_install: +- pip install pep8 + +script: +- > +if [[ "$TRAVIS_EVENT_TYPE" == "pull_request" ]]; +then +git diff origin/master -U0 | pep8 --diff; +fi +- > +docker run -v $PWD:/freeipa -w /freeipa +martbab/freeipa-fedora-builder:master-latest +/bin/bash -c 'dnf builddep -y --spec freeipa.spec.in && make rpms' -- 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)
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 087c96fd57666aebe96863284ebfdb4861bf779a 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 | 43 +++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/ipaserver/install/ipa_server_certinstall.py b/ipaserver/install/ipa_server_certinstall.py index 0a8fb21..705d666 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,41 @@ 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 +try: +tempnssdb.import_pkcs12(pkcs12_filename, db_pwdfile.name, +pkcs12_pin) +for nickname, flags in tempnssdb.list_certs(): +if 'u' not in flags: +tempnssdb.delete_cert(nickname) + +except RuntimeError as e: +raise admintool.ScriptError(str(e)) + +# 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( +"Peer's certificate issuer is not trusted. " +"Please run ipa-cacert-manage install and ipa-certupdate " +"to install the CA certificate.") + 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 +202,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] [Test][Patch-0049, 0050] Certs in ID overrides test
The test is updated to clean up after itself
On 09/06/2016 12:57 PM, Oleg Fayans wrote:
Hi Martin,
Thanks for the review. The updated patches are attached. Please, see my
comments below
On 08/30/2016 01:58 PM, Martin Basti wrote:
On 22.08.2016 13:18, Oleg Fayans wrote:
ping for review
On 08/02/2016 01:11 PM, Oleg Fayans wrote:
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/
NACK for 0049.1
1)
PEP8: you must use 2 empty lines between functions
Fixed
2)
+new_args = " ".join(new_args + args)
you don't need this, run_command takes list as argument too
new_args.extend(args)
The list-based approach does not work with shell redirects which are
heavily used in the certs_id_idoverrides test. Thus, this trick is
really needed
3)
To make it more usable you should add raiseonerr as kwarg to
run_certutil (True as default)
Done
NACK for 0050.2
1)
+tasks.run_certutil(master, ['-L', '-n', cls.adcert1, '-a', '>',
+cls.adcert1_file], cls.reqdir)
+tasks.run_certutil(master, ['-L', '-n', cls.adcert2, '-a', '>',
+cls.adcert2_file], cls.reqdir)
IMO thus should raise an error if failed, but previously you set
raiseonerr=False (multiple times)
Agreed. Done
2)
+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'
New definitions of variables/constants should be directly in class not
in install method, adding new class variables in classmethod is the same
evil as adding instance variables outside __init__
Fair point. Fixed
3)
I have question, why do you need AD for this test? AFAIK you can use ID
overrides without AD
Correct. You can, but the workflow would be slightly different. For
example, you can not issue and sign cert requests for AD-users the way
you would do it for local users. We want to have tests that can be taken
by end-users as example how to use our software, that's why it is better
to be as close to real-world use-cases as it is possible.
Martin^3
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 1a0039b64023b0bb3c9289128413b4ccef489ec4 Mon Sep 17 00:00:00 2001
From: Oleg Fayans
Date: Tue, 6 Sep 2016 13:55:16 +0200
Subject: [PATCH] Automated test for certs in idoverrides feature
https://fedorahosted.org/freeipa/ticket/6005
---
.../test_integration/test_certs_in_idoverrides.py | 121 +
1 file changed, 121 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 ..762ce71a5ed8883b2a2d5bc4185b5ffcb52a4edb
--- /dev/null
+++ b/ipatests/test_integration/test_certs_in_idoverrides.py
@@ -0,0 +1,121 @@
+#
+# 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
+from ipatests.test_integration.env_config import get_global_config
+config = get_global_config()
+
+
+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+.*')
+ad = config.ad_domains[0].ads[0]
+ad_domain = ad.domain.name
+aduser = "testuser@%s" % ad_domain
+adcert1 = 'MyCert1'
+adcert2 = 'MyCert2'
+adcert1_file = adcert1 + '.crt'
+adcert2_file = adcert2 + '.crt'
+
+@classmethod
+def uninstall(cls, mh):
+super(TestCertsInIDOverrides, cls).uninstall(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
+tasks.install_adtrust(master)
+tasks.sync_time(master, cls.ad)
+tasks.establish_trust_with_ad(cls.master, cls.ad_domain,
+
[Freeipa-devel] [freeipa PR#47] schema cache: Store and check info for pre-schema servers (synchronize)
dkupka's pull request #47: "schema cache: Store and check info for pre-schema
servers" was synchronize
See the full pull-request at https://github.com/freeipa/freeipa/pull/47
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/47/head:pr47
git checkout pr47
From a95023175631e8be3531a7bfb52eb8e26ce80afe Mon Sep 17 00:00:00 2001
From: David Kupka
Date: Mon, 22 Aug 2016 13:34:30 +0200
Subject: [PATCH] schema cache: Store and check info for pre-schema servers
Cache CommandError answer to schema command to avoid sending the command
to pre-schema servers every time. This information expires after some
time (1 hour) in order to start using schema as soon as the server is
upgraded.
https://fedorahosted.org/freeipa/ticket/6095
Signed-off-by: Jan Cholasta
Signed-off-by: David Kupka
---
ipaclient/remote_plugins/__init__.py | 80 +
ipaclient/remote_plugins/compat.py | 9 ++-
ipaclient/remote_plugins/schema.py | 130 ++-
3 files changed, 128 insertions(+), 91 deletions(-)
diff --git a/ipaclient/remote_plugins/__init__.py b/ipaclient/remote_plugins/__init__.py
index 2be9222..b783c32 100644
--- a/ipaclient/remote_plugins/__init__.py
+++ b/ipaclient/remote_plugins/__init__.py
@@ -5,7 +5,9 @@
import collections
import errno
import json
+import locale
import os
+import time
from . import compat
from . import schema
@@ -23,20 +25,18 @@ class ServerInfo(collections.MutableMapping):
def __init__(self, api):
hostname = DNSName(api.env.server).ToASCII()
self._path = os.path.join(self._DIR, hostname)
+self._force_check = api.env.force_schema_check
self._dict = {}
-self._dirty = False
-self._read()
-
-def __enter__(self):
-return self
-
-def __exit__(self, *_exc_info):
-self.flush()
+# copy-paste from ipalib/rpc.py
+try:
+self._language = (
+ locale.setlocale(locale.LC_ALL, '').split('.')[0].lower()
+)
+except locale.Error:
+self._language = 'en_us'
-def flush(self):
-if self._dirty:
-self._write()
+self._read()
def _read(self):
try:
@@ -62,13 +62,10 @@ def __getitem__(self, key):
return self._dict[key]
def __setitem__(self, key, value):
-if key not in self._dict or self._dict[key] != value:
-self._dirty = True
self._dict[key] = value
def __delitem__(self, key):
del self._dict[key]
-self._dirty = True
def __iter__(self):
return iter(self._dict)
@@ -76,26 +73,55 @@ def __iter__(self):
def __len__(self):
return len(self._dict)
+def update_validity(self, ttl=None):
+if ttl is None:
+ttl = 3600
+self['expiration'] = time.time() + ttl
+self['language'] = self._language
+self._write()
+
+def is_valid(self):
+if self._force_check:
+return False
+
+try:
+expiration = self._dict['expiration']
+language = self._dict['language']
+except KeyError:
+# if any of these is missing consider the entry expired
+return False
+
+if expiration < time.time():
+# validity passed
+return False
+
+if language != self._language:
+# language changed since last check
+return False
+
+return True
+
def get_package(api):
if api.env.in_tree:
from ipaserver import plugins
else:
-client = rpcclient(api)
-client.finalize()
-
try:
-server_info = api._server_info
+plugins = api._remote_plugins
except AttributeError:
-server_info = api._server_info = ServerInfo(api)
+server_info = ServerInfo(api)
-try:
-plugins = schema.get_package(api, server_info, client)
-except schema.NotAvailable:
-plugins = compat.get_package(api, server_info, client)
-finally:
-server_info.flush()
-if client.isconnected():
-client.disconnect()
+client = rpcclient(api)
+client.finalize()
+
+try:
+plugins = schema.get_package(server_info, client)
+except schema.NotAvailable:
+plugins = compat.get_package(server_info, client)
+finally:
+if client.isconnected():
+client.disconnect()
+
+object.__setattr__(api, '_remote_plugins', plugins)
return plugins
diff --git a/ipaclient/remote_plugins/compat.py b/ipaclient/remote_plugins/compat.py
index 5e08cb0..984eecd 100644
--- a/ipaclient/remote_plugins/compat.py
+++ b/ipaclient/remote_plugins/compat.py
@@ -31,10 +31,15 @@ class CompatObject(Object):
pass
-def get_p
[Freeipa-devel] [freeipa PR#52] Removed incorrect check for returncode (comment)
ofayans commented on a pull request """ Done """ See the full comment at https://github.com/freeipa/freeipa/pull/52#issuecomment-244925053 -- 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#52] Removed incorrect check for returncode (synchronize)
ofayans's pull request #52: "Removed incorrect check for returncode" was
synchronize
See the full pull-request at https://github.com/freeipa/freeipa/pull/52
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/52/head:pr52
git checkout pr52
From 805e3765f10c12faa34db89904b24e89ccf181ad Mon Sep 17 00:00:00 2001
From: Oleg Fayans
Date: Fri, 2 Sep 2016 15:24:40 +0200
Subject: [PATCH 1/4] Removed incorrect check for returncode
The server installation in most cases returns response code 0 no matter what
happens except for really severe errors. In this case when we try to uninstall
the middle replica of a line topology, it fails, notifies us that we should use
'--ignore-topology-disconnect', but returns 0
https://fedorahosted.org/freeipa/ticket/6300
---
ipatests/test_integration/tasks.py | 2 +-
ipatests/test_integration/test_replica_promotion.py | 10 ++
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index c60d436..db99bbb 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -1190,7 +1190,7 @@ def run_server_del(host, server_to_delete, force=False,
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
-if returncode:
+if returncode is not None:
assert result.returncode == returncode
else:
assert result.returncode > 0
diff --git a/ipatests/test_integration/test_replica_promotion.py b/ipatests/test_integration/test_replica_promotion.py
index 3e62f92..e06cafd 100644
--- a/ipatests/test_integration/test_replica_promotion.py
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -348,10 +348,12 @@ def test_replica_uninstallation_prohibited(self):
result = self.replicas[0].run_command(['ipa-server-install',
'--uninstall', '-U'],
raiseonerr=False)
-assert(result.returncode > 0), ("The replica was removed without "
- "'--ignore-topology-disconnect' option")
-assert("Uninstallation leads to disconnected topology"
- in result.stdout_text), ("Expected error message was not found")
+# Due to ticket 3230 server installation/uninstallation always returns
+# 0 unless an uncaught exception occurs. Once this issue is properly
+# addressed, please care to change expected return code in the
+# following assert from 0 to something else.
+assert_error(result, "Removal of '%s' leads to disconnected"
+ " topology" % self.replicas[0].hostname, 0)
self.replicas[0].run_command(['ipa-server-install', '--uninstall',
'-U', '--ignore-topology-disconnect'])
From 524b2d66bbfe96a955a0a7008ec5c6eff134363b Mon Sep 17 00:00:00 2001
From: Oleg Fayans
Date: Mon, 5 Sep 2016 09:05:06 +0200
Subject: [PATCH 2/4] Several fixes in replica_promotion tests
In test_one_command_installation the ipa-replica-install was missing '--server'
and '-U' options which resulted in false negative result. In
test_client_enrollment_by_unprivileged_user '--server' option was messing.
test_replica_promotion_after_adding_to_admin_group lacked '-U' option. It
leaded to 3 failed cases.
https://fedorahosted.org/freeipa/ticket/6301
---
ipatests/test_integration/test_replica_promotion.py | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/ipatests/test_integration/test_replica_promotion.py b/ipatests/test_integration/test_replica_promotion.py
index e06cafd..34af116 100644
--- a/ipatests/test_integration/test_replica_promotion.py
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -187,7 +187,9 @@ def test_one_command_installation(self):
self.replicas[0].run_command(['ipa-replica-install', '-w',
self.master.config.admin_password,
'-n', self.master.domain.name,
- '-r', self.master.domain.realm])
+ '-r', self.master.domain.realm,
+ '--server', self.master.hostname,
+ '-U'])
class TestReplicaManageCommands(IntegrationTest):
@@ -307,7 +309,8 @@ def test_client_enrollment_by_unprivileged_user(self):
'-p', self.username,
'-w', self.new_password,
'--domain', replica.domain.name,
- '--realm', replica.domain.realm, '-U'],
+ '--realm', r
[Freeipa-devel] [freeipa PR#60] Tests: extend DNS cmdline tests with lowercased record type (opened)
mbasti-rh's pull request #60: "Tests: extend DNS cmdline tests with lowercased
record type" was opened
PR body:
"""
Test for https://fedorahosted.org/freeipa/ticket/6203
"""
See the full pull-request at https://github.com/freeipa/freeipa/pull/60
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/60/head:pr60
git checkout pr60
From e43fcefac1c8e908466603d76cd64e62a64d3444 Mon Sep 17 00:00:00 2001
From: Martin Basti
Date: Mon, 5 Sep 2016 18:46:07 +0200
Subject: [PATCH] Tests: extend DNS cmdline tests with lowercased record type
https://fedorahosted.org/freeipa/ticket/6203
---
ipatests/test_cmdline/test_cli.py | 12
1 file changed, 12 insertions(+)
diff --git a/ipatests/test_cmdline/test_cli.py b/ipatests/test_cmdline/test_cli.py
index a6e6363..07bab23 100644
--- a/ipatests/test_cmdline/test_cli.py
+++ b/ipatests/test_cmdline/test_cli.py
@@ -174,6 +174,18 @@ def test_dnsrecord_add_ask_for_missing_fields(self):
sshfp_part_fingerprint=sshfp_parts[2],
)
+# test with lowercase record type
+with self.fake_stdin('sshfp\n%d\n%d\n%s' % sshfp_parts):
+self.check_command(
+'dnsrecord-add %s sshfp' % TEST_ZONE,
+'dnsrecord_add',
+dnszoneidnsname=TEST_ZONE,
+idnsname=u'sshfp',
+sshfp_part_fp_type=sshfp_parts[0],
+sshfp_part_algorithm=sshfp_parts[1],
+sshfp_part_fingerprint=sshfp_parts[2],
+)
+
# NOTE: when a DNS record part is passed via command line, it is not
# converted to its base type when transfered via wire
with self.fake_stdin('%d\n%s' % (sshfp_parts[1], sshfp_parts[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
[Freeipa-devel] [freeipa PR#59] Fix BadSyntax exception in ldapupdate.py (comment)
mbasti-rh commented on a pull request """ Fixed upstream master: https://fedorahosted.org/freeipa/changeset/415600fe451fe806cce7dbe39ad1e9f3d676f2f9 ipa-4-4: https://fedorahosted.org/freeipa/changeset/f3ad90679773b2fd377ffac0a6eda1f674fc94a3 """ See the full comment at https://github.com/freeipa/freeipa/pull/59#issuecomment-244918830 -- 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#59] Fix BadSyntax exception in ldapupdate.py (closed)
martbab's pull request #59: "Fix BadSyntax exception in ldapupdate.py" was closed See the full pull-request at https://github.com/freeipa/freeipa/pull/59 ... or pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/59/head:pr59 git checkout pr59 -- 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#59] Fix BadSyntax exception in ldapupdate.py (+pushed)
martbab's pull request #59: "Fix BadSyntax exception in ldapupdate.py" label *pushed* has been added See the full pull-request at https://github.com/freeipa/freeipa/pull/59 -- 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#59] Fix BadSyntax exception in ldapupdate.py (+ack)
martbab's pull request #59: "Fix BadSyntax exception in ldapupdate.py" label *ack* has been added See the full pull-request at https://github.com/freeipa/freeipa/pull/59 -- 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
Forgot to attach the test run output: -bash-4.3$ ipa-run-tests test_integration/test_certs_in_idoverrides.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 1 items test_integration/test_certs_in_idoverrides.py . = 1 passed in 681.90 seconds = On 09/06/2016 12:57 PM, Oleg Fayans wrote: Hi Martin, Thanks for the review. The updated patches are attached. Please, see my comments below On 08/30/2016 01:58 PM, Martin Basti wrote: On 22.08.2016 13:18, Oleg Fayans wrote: ping for review On 08/02/2016 01:11 PM, Oleg Fayans wrote: 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/ NACK for 0049.1 1) PEP8: you must use 2 empty lines between functions Fixed 2) +new_args = " ".join(new_args + args) you don't need this, run_command takes list as argument too new_args.extend(args) The list-based approach does not work with shell redirects which are heavily used in the certs_id_idoverrides test. Thus, this trick is really needed 3) To make it more usable you should add raiseonerr as kwarg to run_certutil (True as default) Done NACK for 0050.2 1) +tasks.run_certutil(master, ['-L', '-n', cls.adcert1, '-a', '>', +cls.adcert1_file], cls.reqdir) +tasks.run_certutil(master, ['-L', '-n', cls.adcert2, '-a', '>', +cls.adcert2_file], cls.reqdir) IMO thus should raise an error if failed, but previously you set raiseonerr=False (multiple times) Agreed. Done 2) +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' New definitions of variables/constants should be directly in class not in install method, adding new class variables in classmethod is the same evil as adding instance variables outside __init__ Fair point. Fixed 3) I have question, why do you need AD for this test? AFAIK you can use ID overrides without AD Correct. You can, but the workflow would be slightly different. For example, you can not issue and sign cert requests for AD-users the way you would do it for local users. We want to have tests that can be taken by end-users as example how to use our software, that's why it is better to be as close to real-world use-cases as it is possible. Martin^3 -- 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
Re: [Freeipa-devel] [Test][Patch-0049, 0050] Certs in ID overrides test
Hi Martin, Thanks for the review. The updated patches are attached. Please, see my comments below On 08/30/2016 01:58 PM, Martin Basti wrote: On 22.08.2016 13:18, Oleg Fayans wrote: ping for review On 08/02/2016 01:11 PM, Oleg Fayans wrote: 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/ NACK for 0049.1 1) PEP8: you must use 2 empty lines between functions Fixed 2) +new_args = " ".join(new_args + args) you don't need this, run_command takes list as argument too new_args.extend(args) The list-based approach does not work with shell redirects which are heavily used in the certs_id_idoverrides test. Thus, this trick is really needed 3) To make it more usable you should add raiseonerr as kwarg to run_certutil (True as default) Done NACK for 0050.2 1) +tasks.run_certutil(master, ['-L', '-n', cls.adcert1, '-a', '>', +cls.adcert1_file], cls.reqdir) +tasks.run_certutil(master, ['-L', '-n', cls.adcert2, '-a', '>', +cls.adcert2_file], cls.reqdir) IMO thus should raise an error if failed, but previously you set raiseonerr=False (multiple times) Agreed. Done 2) +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' New definitions of variables/constants should be directly in class not in install method, adding new class variables in classmethod is the same evil as adding instance variables outside __init__ Fair point. Fixed 3) I have question, why do you need AD for this test? AFAIK you can use ID overrides without AD Correct. You can, but the workflow would be slightly different. For example, you can not issue and sign cert requests for AD-users the way you would do it for local users. We want to have tests that can be taken by end-users as example how to use our software, that's why it is better to be as close to real-world use-cases as it is possible. Martin^3 -- Oleg Fayans Quality Engineer FreeIPA team RedHat. From 867c603183d792b0056c0f8895f52577bc67d7b0 Mon Sep 17 00:00:00 2001 From: Oleg Fayans Date: Tue, 6 Sep 2016 12:39:45 +0200 Subject: [PATCH] Added interface to certutil --- ipatests/test_integration/tasks.py | 7 +++ 1 file changed, 7 insertions(+) diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py index c60d43699d6577abe930ac8d6ab696feea837331..0e329f4ad5d754fd61a9ca911488230677daad77 100644 --- a/ipatests/test_integration/tasks.py +++ b/ipatests/test_integration/tasks.py @@ -1187,6 +1187,13 @@ 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, raiseonerr=True): +new_args = [paths.CERTUTIL, "-d", reqdir] +new_args = " ".join(new_args + args) +return host.run_command(new_args, raiseonerr=raiseonerr, +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 fb0591407a64dcf84eda1a28a06d1ead2fa7ab0d Mon Sep 17 00:00:00 2001 From: Oleg Fayans Date: Tue, 6 Sep 2016 12:41:06 +0200 Subject: [PATCH] Automated test for certs in idoverrides feature https://fedorahosted.org/freeipa/ticket/6005 --- .../test_integration/test_certs_in_idoverrides.py | 120 + 1 file changed, 120 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 ..d72fc1e898f0574015c6b7dd5f601cec8e4350d6 --- /dev/null +++ b/ipatests/test_integration/test_certs_in_idoverrides.py @@ -0,0 +1,120 @@ +# +# 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 +from ipatests.test_integration.env_config import get_global_config +con
[Freeipa-devel] [freeipa PR#34] dns: prompt for missing record parts in CLI (+pushed)
jcholast's pull request #34: " dns: prompt for missing record parts in CLI" label *pushed* has been added See the full pull-request at https://github.com/freeipa/freeipa/pull/34 -- 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#34] dns: prompt for missing record parts in CLI (closed)
jcholast's pull request #34: " dns: prompt for missing record parts in CLI" was closed See the full pull-request at https://github.com/freeipa/freeipa/pull/34 ... or pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/34/head:pr34 git checkout pr34 -- 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#34] dns: prompt for missing record parts in CLI (comment)
mbasti-rh commented on a pull request """ Fixed upstream master: https://fedorahosted.org/freeipa/changeset/afea9616318fbbffc2c296b7c41890db8595e3cc https://fedorahosted.org/freeipa/changeset/dce95a14595a37ce83bcf3e28f41feab715d0c81 https://fedorahosted.org/freeipa/changeset/38a51fa984a6aa92f383d7f8176057de8e057d52 ipa-4-4: https://fedorahosted.org/freeipa/changeset/fa8a5c33b73398c731cf2a472c79bd9a51404fe2 https://fedorahosted.org/freeipa/changeset/b4c104ee9038d8d87a7e78137826e655ebb5d39b https://fedorahosted.org/freeipa/changeset/47d6f49e53d27e1df1377a91789c072b11ccea31 """ See the full comment at https://github.com/freeipa/freeipa/pull/34#issuecomment-244916455 -- 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#34] dns: prompt for missing record parts in CLI (+ack)
jcholast's pull request #34: " dns: prompt for missing record parts in CLI" label *ack* has been added See the full pull-request at https://github.com/freeipa/freeipa/pull/34 -- 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#52] Removed incorrect check for returncode (comment)
mbasti-rh commented on a pull request """ Ad 2. yes """ See the full comment at https://github.com/freeipa/freeipa/pull/52#issuecomment-244914033 -- 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#52] Removed incorrect check for returncode (comment)
ofayans commented on a pull request """ @mbasti-rh, 1. Fixed 2. It's OK, but we won't have working tests in 4.3 branch. Should I create a ticket? 3. Gotya :) """ See the full comment at https://github.com/freeipa/freeipa/pull/52#issuecomment-244913086 -- 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#55] Fix parse errors with link-local addresses (synchronize)
mbasti-rh's pull request #55: "Fix parse errors with link-local addresses" was
synchronize
See the full pull-request at https://github.com/freeipa/freeipa/pull/55
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/55/head:pr55
git checkout pr55
From 4ca1d867624e81366df6495250ddf410ce724b24 Mon Sep 17 00:00:00 2001
From: Martin Basti
Date: Mon, 5 Sep 2016 14:33:58 +0200
Subject: [PATCH] Fix parse errors with link-local addresses
Link-local addresses received from netifaces contains '%suffix' that
causes parse error in IPNetwork class. We must remove %suffix before
it us used in IPNetwork objects.
https://fedorahosted.org/freeipa/ticket/6296
---
ipapython/ipautil.py | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 9536543..8de9acf 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -173,8 +173,13 @@ def __init__(self, addr, match_local=False, parse_netmask=True,
iface = None
for interface in netifaces.interfaces():
for ifdata in netifaces.ifaddresses(interface).get(family, []):
+
+# link-local addresses contain '%suffix' that causes parse
+# errors in IPNetwork
+ifaddr = ifdata['addr'].split(u'%', 1)[0]
+
ifnet = netaddr.IPNetwork('{addr}/{netmask}'.format(
-addr=ifdata['addr'],
+addr=ifaddr,
netmask=ifdata['netmask']
))
if ifnet == self._net or (
--
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#52] Removed incorrect check for returncode (synchronize)
ofayans's pull request #52: "Removed incorrect check for returncode" was
synchronize
See the full pull-request at https://github.com/freeipa/freeipa/pull/52
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/52/head:pr52
git checkout pr52
From 805e3765f10c12faa34db89904b24e89ccf181ad Mon Sep 17 00:00:00 2001
From: Oleg Fayans
Date: Fri, 2 Sep 2016 15:24:40 +0200
Subject: [PATCH 1/4] Removed incorrect check for returncode
The server installation in most cases returns response code 0 no matter what
happens except for really severe errors. In this case when we try to uninstall
the middle replica of a line topology, it fails, notifies us that we should use
'--ignore-topology-disconnect', but returns 0
https://fedorahosted.org/freeipa/ticket/6300
---
ipatests/test_integration/tasks.py | 2 +-
ipatests/test_integration/test_replica_promotion.py | 10 ++
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index c60d436..db99bbb 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -1190,7 +1190,7 @@ def run_server_del(host, server_to_delete, force=False,
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
-if returncode:
+if returncode is not None:
assert result.returncode == returncode
else:
assert result.returncode > 0
diff --git a/ipatests/test_integration/test_replica_promotion.py b/ipatests/test_integration/test_replica_promotion.py
index 3e62f92..e06cafd 100644
--- a/ipatests/test_integration/test_replica_promotion.py
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -348,10 +348,12 @@ def test_replica_uninstallation_prohibited(self):
result = self.replicas[0].run_command(['ipa-server-install',
'--uninstall', '-U'],
raiseonerr=False)
-assert(result.returncode > 0), ("The replica was removed without "
- "'--ignore-topology-disconnect' option")
-assert("Uninstallation leads to disconnected topology"
- in result.stdout_text), ("Expected error message was not found")
+# Due to ticket 3230 server installation/uninstallation always returns
+# 0 unless an uncaught exception occurs. Once this issue is properly
+# addressed, please care to change expected return code in the
+# following assert from 0 to something else.
+assert_error(result, "Removal of '%s' leads to disconnected"
+ " topology" % self.replicas[0].hostname, 0)
self.replicas[0].run_command(['ipa-server-install', '--uninstall',
'-U', '--ignore-topology-disconnect'])
From a791998ef32085c936d758554a69a4b5b91485bb Mon Sep 17 00:00:00 2001
From: Oleg Fayans
Date: Mon, 5 Sep 2016 09:05:06 +0200
Subject: [PATCH 2/4] Several fixes in replica_promotion tests
In test_one_command_installation the ipa-replica-install was missing '--server'
and '-U' options which resulted in false negative result. In
test_client_enrollment_by_unprivileged_user '--server' option was messing.
test_replica_promotion_after_adding_to_admin_group lacked '-U' option. It
leaded to 3 failed cases.
---
ipatests/test_integration/test_replica_promotion.py | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/ipatests/test_integration/test_replica_promotion.py b/ipatests/test_integration/test_replica_promotion.py
index e06cafd..34af116 100644
--- a/ipatests/test_integration/test_replica_promotion.py
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -187,7 +187,9 @@ def test_one_command_installation(self):
self.replicas[0].run_command(['ipa-replica-install', '-w',
self.master.config.admin_password,
'-n', self.master.domain.name,
- '-r', self.master.domain.realm])
+ '-r', self.master.domain.realm,
+ '--server', self.master.hostname,
+ '-U'])
class TestReplicaManageCommands(IntegrationTest):
@@ -307,7 +309,8 @@ def test_client_enrollment_by_unprivileged_user(self):
'-p', self.username,
'-w', self.new_password,
'--domain', replica.domain.name,
- '--realm', replica.domain.realm, '-U'],
+ '--realm', replica.domain.realm, '-U',
+
[Freeipa-devel] FleetCommander integration
Hi, Now that FreeIPA 4.4.1 is out, I've pushed to github my prototype for FleetCommander integration: https://github.com/abbra/freeipa-desktop-profile/ You can read the design page: https://github.com/abbra/freeipa-desktop-profile/blob/master/plugin/Feature.mediawiki The design was mostly figured out in discussions with Alberto, Fabiano, Nathaniel, and Jakub, so we are more or less on the common ground here between SSSD and FleetCommander. You can send pull requests to me on github to update the design. ;) You can cut a tarball using git archive --format=tar.gz --prefix=freeipa-desktop-profile-0.0.1/ \ --output ~/rpmbuild/SOURCES/freeipa-desktop-profile-0.0.1.tar.gz \ freeipa-desktop-profile-0.0.1 And then build the package with rpmbuild -ta freeipa-desktop-profile-0.0.1.tar.gz When installed, the package does not run ipa-server-upgrade by itself, yet. So you need to run ipa-server-upgrade manually. Once ran, deskprofile/deskprofilerule topics would become available and can be used for testing purposes. For Fedora 24 one can use FreeIPA 4.4.1 from COPR, for Fedora 25 we have FreeIPA 4.4.1 in updates stable as of today. UI plugin is not ready yet and is disabled in the spec file as it breaks loading the whole UI. -- / 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
Re: [Freeipa-devel] [PATCH] 0100 Track lightweight CAs on replica installation
On 09/05/2016 07:46 PM, Fraser Tweedale wrote: On Mon, Aug 29, 2016 at 06:39:58PM +0200, Martin Babinsky wrote: On 08/23/2016 08:40 AM, Fraser Tweedale wrote: Hi folks, Please review attached patch which fixes https://fedorahosted.org/freeipa/ticket/6019. Thanks, Fraser Hi Fraser, I have couple of comments: Thanks for your review, Martin. Updated patch attached. Comments inline. 1.) -for entry in lwcas: -self.server_track_lightweight_ca(entry) +try: +from ipaserver.install import cainstance + cainstance.add_lightweight_ca_tracking_requests(self.log, lwcas) +except Exception as e: +self.log.exception( +"Failed to add lightweight CA tracking requests") You are importing a server-side module in a basically client-side command which I don't like very much. Isn't there a possibility to use shared client-server module for this? It's ugly. It is an effect of my desire to keep LWCA tracking code where IMO it belongs: in cainstance module. If you know a nicer way to conditionally get at contents of cainstance module I'm happy to do it differently. Otherwise I don't think it's a showstopper. 2.) +def __add_lightweight_ca_tracking_requests(self): +server_id = installutils.realm_to_serverid(api.env.realm) +dogtag_uri = 'ldapi://%%2fvar%%2frun%%2fslapd-%s.socket' % server_id +conn = ldap2.ldap2(api, ldap_uri=dogtag_uri) +is_already_connected = conn.isconnected() Why use these connection setup shenanigans when you can use either api.Backend.ldap2 (IIRC this runs in server context so LDAPI should be a given) or even the service's own 'admin_conn' member. I changed it to use admin_conn. + +if not is_already_connected: +try: +conn.connect(autobind=True) +except errors.PublicError as e: +self.log.error( +"Cannot connect to LDAP to add " +"lightweight CA tracking requests: %s", +e +) PEP8 error here, the second line of the message is misformatted. Thanks, fixed. +return + +try: +lwcas = conn.get_entries( +base_dn=ipautil.realm_to_suffix(api.env.realm), +filter='(objectclass=ipaca)', +attrs_list=['cn', 'ipacaid'], +) I would rather use the result of api.Command.ca_find to fetch sub-CAs. Also, ipautil.realm_to_suffix is superseded by api.env.basedn to fetch search base. Updated to use api.env.basedn. I hit problems using ca_find and connecting the ldap2 backend so I'm sticking with admin_conn, which is working. +add_lightweight_ca_tracking_requests(self.log, lwcas) +except errors.NotFound: +pass # shouldn't happen, but don't fail if it does I would add at least some debug message here. OK. +finally: +if not is_already_connected: +conn.disconnect() + -- Martin^3 Babinsky Thanks, ACK. Rebased and pushed to: master: 08b768313020c45bfa82d67cd214afabf605f4b3 ipa-4-4: 99b0db0ebf090c9f60078e9ca9bf2aba665635f5 -- 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#52] Removed incorrect check for returncode (comment)
mbasti-rh commented on a pull request """ I went through commit messages again and I changed my mind, NACK: 1) commit: Removed incorrect check for returncode This commit contains incorrect ticket, issue has not been fixed by this commit 2) commit: Several fixes in replica_promotion tests This does not have ticket, is okay to push it just to master? 3) All commits have different tickets, so I have to manually split hashes and put it to proper tickets. Leave it as is for this PR to avoid mess, but in future create one PR for one ticket """ See the full comment at https://github.com/freeipa/freeipa/pull/52#issuecomment-244906832 -- 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#52] Removed incorrect check for returncode (-ack)
ofayans's pull request #52: "Removed incorrect check for returncode" label *ack* has been removed See the full pull-request at https://github.com/freeipa/freeipa/pull/52 -- 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#59] Fix BadSyntax exception in ldapupdate.py (opened)
martbab's pull request #59: "Fix BadSyntax exception in ldapupdate.py" was opened PR body: """ This complements commit 00d43095da211f542189c95c88fc2e2c32e75565 and fixes two failing testcases in `ipatests/test_install/test_updates.py` https://fedorahosted.org/freeipa/ticket/6294 """ See the full pull-request at https://github.com/freeipa/freeipa/pull/59 ... or pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/59/head:pr59 git checkout pr59 From 3fabb5b64d4df9ef1a7a00312e5244b92af67eb0 Mon Sep 17 00:00:00 2001 From: Martin Babinsky Date: Tue, 6 Sep 2016 11:46:58 +0200 Subject: [PATCH] ldapupdate: Use proper inheritance in BadSyntax exception https://fedorahosted.org/freeipa/ticket/6294 --- ipaserver/install/ldapupdate.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py index 32fa4e2..1b39745 100644 --- a/ipaserver/install/ldapupdate.py +++ b/ipaserver/install/ldapupdate.py @@ -83,8 +83,8 @@ def connect(ldapi=False, realm=None, fqdn=None, dm_password=None, pw_name=None): class BadSyntax(installutils.ScriptError): def __init__(self, value): self.value = value -self.msg = "LDAPUpdate: syntax error: \n %s" % value -self.rval = 1 +super(BadSyntax, self).__init__( +msg="LDAPUpdate: syntax error: \n %s" % value, rval=1) def __str__(self): return repr(self.value) -- 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
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?
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.
Updated patch for option (A) is attached.
1) Instead of
if result['status'] in {'REVOKED', 'REVOKED_EXPIRED'}:
use:
if result['revoked']:
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.
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
--
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
