[Freeipa-devel] [PATCH 0195] Fix memory leaks in ipapkcs11helper module
Ticket: https://fedorahosted.org/freeipa/ticket/4657 Patch attached. -- Martin Basti From 15509e09b658e9b2460ce42081baa1bb77a4d1e8 Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Tue, 24 Feb 2015 19:25:31 +0100 Subject: [PATCH] Fix memory leaks in ipap11helper Ticket: https://fedorahosted.org/freeipa/ticket/4657 --- ipapython/ipap11helper/p11helper.c | 295 ++--- 1 file changed, 212 insertions(+), 83 deletions(-) diff --git a/ipapython/ipap11helper/p11helper.c b/ipapython/ipap11helper/p11helper.c index c638bbe849f1bbddc8004bd1c41128b1c9e7..e3a7a9399c12759c537d13a291dcdf6ec1a1efa4 100644 --- a/ipapython/ipap11helper/p11helper.c +++ b/ipapython/ipap11helper/p11helper.c @@ -197,9 +197,11 @@ unsigned char* unicode_to_char_array(PyObject *unicode, Py_ssize_t *l) { /* Copy string first, then DECREF * https://docs.python.org/2/c-api/string.html#c.PyString_AS_STRING */ -result = (unsigned char *) malloc((size_t) *l); +result = (unsigned char *) PyMem_Malloc((size_t) *l); if (result == NULL){ -PyErr_SetString(ipap11helperError, Memory allocation error); +Py_DECREF(utf8_str); +PyErr_NoMemory(); +return NULL; } else { memcpy(result, bytes, *l); } @@ -650,7 +652,9 @@ P11_Helper_generate_master_key(P11_Helper* self, PyObject *args, PyObject *kwds) PyObject *label_unicode = NULL; Py_ssize_t label_length = 0; +CK_BYTE *label = NULL; int r; +int error = 0; static char *kwlist[] = { subject, id, key_length, cka_copyable, cka_decrypt, cka_derive, cka_encrypt, cka_extractable, cka_modifiable, cka_private, cka_sensitive, cka_sign, @@ -672,26 +676,28 @@ P11_Helper_generate_master_key(P11_Helper* self, PyObject *args, PyObject *kwds) return NULL; } -CK_BYTE *label = (unsigned char*) unicode_to_char_array(label_unicode, +label = (unsigned char*) unicode_to_char_array(label_unicode, label_length); CK_MECHANISM mechanism = { //TODO param? CKM_AES_KEY_GEN, NULL_PTR, 0 }; +if (label == NULL) return NULL; if ((key_length != 16) (key_length != 24) (key_length != 32)) { PyErr_SetString(ipap11helperError, generate_master_key: key length allowed values are: 16, 24 and 32); -return NULL; +error = 1; +goto final; } -//TODO free label if check failed -//TODO is label freed inside dont we use freed value later r = _id_exists(self, id, id_length, CKO_SECRET_KEY); if (r == 1) { PyErr_SetString(ipap11helperDuplicationError, Master key with same ID already exists); -return NULL; +error = 1; +goto final; } else if (r == -1) { -return NULL; +error = 1; +goto final; } /* Process keyword boolean arguments */ @@ -719,9 +725,15 @@ P11_Helper_generate_master_key(P11_Helper* self, PyObject *args, PyObject *kwds) rv = self-p11-C_GenerateKey(self-session, mechanism, symKeyTemplate, sizeof(symKeyTemplate) / sizeof(CK_ATTRIBUTE), master_key); -if (!check_return_value(rv, generate master key)) +if (!check_return_value(rv, generate master key)){ +error = 1; +goto final; +} +final: +if (label != NULL) PyMem_Free(label); +if (error){ return NULL; - +} return Py_BuildValue(k, master_key); } @@ -740,6 +752,8 @@ P11_Helper_generate_replica_key_pair(P11_Helper* self, PyObject *args, int id_length = 0; PyObject* label_unicode = NULL; Py_ssize_t label_length = 0; +CK_BYTE *label = NULL; +int error = 0; PyObj2Bool_mapping_t attrs_pub[] = { { NULL, true }, //pub_en_cka_copyable { NULL, false }, //pub_en_cka_derive @@ -806,30 +820,33 @@ P11_Helper_generate_replica_key_pair(P11_Helper* self, PyObject *args, return NULL; } -CK_BYTE *label = unicode_to_char_array(label_unicode, label_length); +label = unicode_to_char_array(label_unicode, label_length); +if (label == NULL) return NULL; CK_OBJECT_HANDLE public_key, private_key; CK_MECHANISM mechanism = { CKM_RSA_PKCS_KEY_PAIR_GEN, NULL_PTR, 0 }; -//TODO free variables - r = _id_exists(self, id, id_length, CKO_PRIVATE_KEY); if (r == 1) { PyErr_SetString(ipap11helperDuplicationError, Private key with same ID already exists); -return NULL; +error = 1; +goto final; } else if (r == -1) { -return NULL; +error = 1; +goto final; } r = _id_exists(self, id, id_length, CKO_PUBLIC_KEY); if (r == 1) { PyErr_SetString(ipap11helperDuplicationError, Public key with same ID already exists); -return NULL; +error = 1; +goto final; }
[Freeipa-devel] [PATCH 0194] Remove unused method to export secret key from ipapkcs11helper module
The method never been used, and never will be, because we do not want to export secrets. Ticket: https://fedorahosted.org/freeipa/ticket/4657 Patch attached (may require mbasti-0195, mbasti-0190) -- Martin Basti From b812b72924aff118ed5c83d40040dc93f623c22b Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Wed, 25 Feb 2015 12:37:57 +0100 Subject: [PATCH] Remove unused method from ipap11pkcs helper module Ticket: https://fedorahosted.org/freeipa/ticket/4657 --- ipapython/ipap11helper/p11helper.c | 51 -- 1 file changed, 51 deletions(-) diff --git a/ipapython/ipap11helper/p11helper.c b/ipapython/ipap11helper/p11helper.c index e3a7a9399c12759c537d13a291dcdf6ec1a1efa4..c0089b6825097c24fd8b1404aa79dedcf6a03936 100644 --- a/ipapython/ipap11helper/p11helper.c +++ b/ipapython/ipap11helper/p11helper.c @@ -1034,54 +1034,6 @@ P11_Helper_delete_key(P11_Helper* self, PyObject *args, PyObject *kwds) { } /** - * export secret key - */ -//TODO remove, we don't want to export secret key -static PyObject * -P11_Helper_export_secret_key(P11_Helper* self, PyObject *args, PyObject *kwds) { -CK_RV rv; -CK_UTF8CHAR_PTR value = NULL; -CK_OBJECT_HANDLE key_handle = 0; -PyObject *ret = NULL; -static char *kwlist[] = { key_handle, NULL }; -//TODO check long overflow -if (!PyArg_ParseTupleAndKeywords(args, kwds, k|, kwlist, key_handle)) { -return NULL; -} - -//TODO which attributes should be returned -CK_ATTRIBUTE obj_template[] = { { CKA_VALUE, NULL_PTR, 0 } }; - -rv = self-p11-C_GetAttributeValue(self-session, key_handle, obj_template, -1); -if (!check_return_value(rv, get attribute value - prepare)) { -return NULL; -} - -/* Set proper size for attributes*/ -value = (CK_UTF8CHAR_PTR) malloc( -obj_template[0].ulValueLen * sizeof(CK_BYTE)); -obj_template[0].pValue = value; - -rv = self-p11-C_GetAttributeValue(self-session, key_handle, obj_template, -1); -if (!check_return_value(rv, get attribute value)) { -free(value); -return NULL; -} - -if (obj_template[0].ulValueLen = 0) { -PyErr_SetString(ipap11helperNotFound, Value not found); -free(value); -return NULL; -} -ret = Py_BuildValue({s:s#}, value, obj_template[0].pValue, -obj_template[0].ulValueLen); -free(value); -return ret; -} - -/** * export RSA public key */ static PyObject * @@ -2007,9 +1959,6 @@ static PyMethodDef P11_Helper_methods[] = { { finalize, (PyCFunction) P11_Helper_find_keys, METH_VARARGS | METH_KEYWORDS, Find keys }, { delete_key, (PyCFunction) P11_Helper_delete_key, METH_VARARGS | METH_KEYWORDS, Delete key }, { -export_secret_key, //TODO deprecated, delete it -(PyCFunction) P11_Helper_export_secret_key, -METH_VARARGS | METH_KEYWORDS, Export secret key }, { export_public_key, (PyCFunction) P11_Helper_export_public_key, METH_VARARGS | METH_KEYWORDS, Export public key }, { import_public_key, (PyCFunction) P11_Helper_import_public_key, -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0197-0198] Fix uniqueness plugins upgrade
On Wed, 25 Feb 2015, Martin Basti wrote: Modifications: * All plugins are migrated into new configuration style. * I left attribute uniqueness plugin disabled, cn=uid uniqueness,cn=plugins,cn=config is checking the same attribute. * POST_UPDATE plugin for uid removed, I moved it to update file. Is it okay Alexander? I haven't found reason why we need to do it in update plugin. So I looked up the original thread and since there are three different ways of defining uniqueness plugin's configuration, update plugin was to me the only way to handle all different configuration types. In general we cannot rely on the fact that FreeIPA deployment only contains FreeIPA-defined plugin configurations. -- / Alexander Bokovoy pgp0rYQ5YZQgQ.pgp Description: PGP signature ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 133] ipa-range-check: do not treat missing objects as error
On 02/24/2015 06:47 PM, Sumit Bose wrote: Hi, this patch changes a return code and should fix https://fedorahosted.org/freeipa/ticket/4924 . bye, Sumit I have a related question. Do I read the plugin right, that whenever any object is changed, this plugins loads the whole entry and tests some of it's attribute to see if it is ID views and then does the actual check. Is this good approach performance wise? Wouldn't it be better to decide even before that, based on DN and whether it is in the ID View Sub-Tree? CCing Thierry. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCHES 0197-0198] Fix uniqueness plugins upgrade
Modifications: * All plugins are migrated into new configuration style. * I left attribute uniqueness plugin disabled, cn=uid uniqueness,cn=plugins,cn=config is checking the same attribute. * POST_UPDATE plugin for uid removed, I moved it to update file. Is it okay Alexander? I haven't found reason why we need to do it in update plugin. Thierry, I touched configuration of plugins, which user lifecycle requires, can you take look if I it does not break anything? Patches attached. -- Martin Basti From a8e1c7874acaa3b0fe9bd3ae316379ca3ddb95dc Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Mon, 23 Feb 2015 16:09:25 +0100 Subject: [PATCH 1/2] Migrate uniquess plugins configuration to new style New configuration style contains options required for user lifecycle management. --- install/share/unique-attributes.ldif | 24 +-- install/updates/10-uniqueness.update | 36 ++--- ipaserver/install/plugins/update_uniqueness.py | 203 - 3 files changed, 232 insertions(+), 31 deletions(-) diff --git a/install/share/unique-attributes.ldif b/install/share/unique-attributes.ldif index 0e680a0e45b455469f9be9555aed1e63f1d97faf..ea38ac75310403d9deef8ea0b8ccb6e70b9e57fe 100644 --- a/install/share/unique-attributes.ldif +++ b/install/share/unique-attributes.ldif @@ -8,8 +8,8 @@ nsslapd-pluginPath: libattr-unique-plugin nsslapd-pluginInitfunc: NSUniqueAttr_Init nsslapd-pluginType: preoperation nsslapd-pluginEnabled: on -nsslapd-pluginarg0: krbPrincipalName -nsslapd-pluginarg1: $SUFFIX +uniqueness-attribute-name: krbPrincipalName +uniqueness-subtrees: $SUFFIX nsslapd-plugin-depends-on-type: database nsslapd-pluginId: NSUniqueAttr nsslapd-pluginVersion: 1.1.0 @@ -26,8 +26,8 @@ nsslapd-pluginPath: libattr-unique-plugin nsslapd-pluginInitfunc: NSUniqueAttr_Init nsslapd-pluginType: preoperation nsslapd-pluginEnabled: on -nsslapd-pluginarg0: krbCanonicalName -nsslapd-pluginarg1: $SUFFIX +uniqueness-attribute-name: krbCanonicalName +uniqueness-subtrees: $SUFFIX nsslapd-plugin-depends-on-type: database nsslapd-pluginId: NSUniqueAttr nsslapd-pluginVersion: 1.1.0 @@ -44,8 +44,8 @@ nsslapd-pluginPath: libattr-unique-plugin nsslapd-pluginInitfunc: NSUniqueAttr_Init nsslapd-pluginType: preoperation nsslapd-pluginEnabled: on -nsslapd-pluginarg0: cn -nsslapd-pluginarg1: cn=ng,cn=alt,$SUFFIX +uniqueness-attribute-name: cn +uniqueness-subtrees: cn=ng,cn=alt,$SUFFIX nsslapd-plugin-depends-on-type: database nsslapd-pluginId: NSUniqueAttr nsslapd-pluginVersion: 1.1.0 @@ -62,8 +62,8 @@ nsslapd-pluginPath: libattr-unique-plugin nsslapd-pluginInitfunc: NSUniqueAttr_Init nsslapd-pluginType: preoperation nsslapd-pluginEnabled: on -nsslapd-pluginarg0: ipaUniqueID -nsslapd-pluginarg1: $SUFFIX +uniqueness-attribute-name: ipaUniqueID +uniqueness-subtrees: $SUFFIX nsslapd-plugin-depends-on-type: database nsslapd-pluginId: NSUniqueAttr nsslapd-pluginVersion: 1.1.0 @@ -81,8 +81,8 @@ nsslapd-pluginPath: libattr-unique-plugin nsslapd-pluginInitfunc: NSUniqueAttr_Init nsslapd-pluginType: preoperation nsslapd-pluginEnabled: on -nsslapd-pluginarg0: cn -nsslapd-pluginarg1: cn=sudorules,cn=sudo,$SUFFIX +uniqueness-attribute-name: cn +uniqueness-subtrees: cn=sudorules,cn=sudo,$SUFFIX nsslapd-plugin-depends-on-type: database nsslapd-pluginId: NSUniqueAttr nsslapd-pluginVersion: 1.1.0 @@ -97,8 +97,8 @@ nsslapd-pluginVendor: Fedora Project #nsslapd-pluginInitfunc: NSUniqueAttr_Init #nsslapd-pluginType: preoperation #nsslapd-pluginEnabled: on -#nsslapd-pluginarg0: uid -#nsslapd-pluginarg1: cn=accounts,$SUFFIX +#uniqueness-attribute-name: uid +#uniqueness-subtrees: cn=accounts,$SUFFIX #nsslapd-plugin-depends-on-type: database #nsslapd-pluginId: NSUniqueAttr #nsslapd-pluginVersion: 1.1.0 diff --git a/install/updates/10-uniqueness.update b/install/updates/10-uniqueness.update index c9641c47fabdffdc278216b38abd606745781d41..b6e2fff6db8c2da9b6303e183fa92e807eab929a 100644 --- a/install/updates/10-uniqueness.update +++ b/install/updates/10-uniqueness.update @@ -8,8 +8,8 @@ default:nsslapd-pluginPath: libattr-unique-plugin default:nsslapd-pluginInitfunc: NSUniqueAttr_Init default:nsslapd-pluginType: preoperation default:nsslapd-pluginEnabled: on -default:nsslapd-pluginarg0: cn -default:nsslapd-pluginarg1: cn=sudorules,cn=sudo,$SUFFIX +default:uniqueness-attribute-name: cn +default:uniqueness-subtrees: cn=sudorules,cn=sudo,$SUFFIX default:nsslapd-plugin-depends-on-type: database default:nsslapd-pluginId: NSUniqueAttr default:nsslapd-pluginVersion: 1.1.0 @@ -25,8 +25,8 @@ default:nsslapd-pluginPath: libattr-unique-plugin default:nsslapd-pluginInitfunc: NSUniqueAttr_Init default:nsslapd-pluginType: preoperation default:nsslapd-pluginEnabled: on -default:nsslapd-pluginarg0: ipaCertSubject -default:nsslapd-pluginarg1: cn=certificates,cn=ipa,cn=etc,$SUFFIX +default:uniqueness-attribute-name: ipaCertSubject +default:uniqueness-subtrees:
[Freeipa-devel] [PATCH 0199] Remove unused disable-betxn.ldif file
Hello, the file 'disable-betxn.ldif' is not used in code in IPA master branch. There is 10-enable-betxn.update which is used. If I'm right we can remove it. Patch attached. Please correct me if the file is needed. Martin^2 -- Martin Basti From 9d6f9e4099cdc699ec146a647829c2b626468547 Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Mon, 23 Feb 2015 20:09:51 +0100 Subject: [PATCH] Remove unused disable-betxn.ldif file --- install/share/Makefile.am| 1 - install/share/disable-betxn.ldif | 61 2 files changed, 62 deletions(-) delete mode 100644 install/share/disable-betxn.ldif diff --git a/install/share/Makefile.am b/install/share/Makefile.am index 878d88684f774d378b1d2e886841e2b4b7e4..ca6128e2911ab5c0a773dd553f8e67eab944f120 100644 --- a/install/share/Makefile.am +++ b/install/share/Makefile.am @@ -29,7 +29,6 @@ app_DATA =\ default-smb-group.ldif \ default-trust-view.ldif \ delegation.ldif \ - disable-betxn.ldif \ replica-acis.ldif \ ds-nfiles.ldif \ dns.ldif \ diff --git a/install/share/disable-betxn.ldif b/install/share/disable-betxn.ldif deleted file mode 100644 index 57a6858ddbb161dc5153d46caf3653998a60ce9f.. --- a/install/share/disable-betxn.ldif +++ /dev/null @@ -1,61 +0,0 @@ -# Disable transactions in 389-ds-base - -dn: cn=7-bit check,cn=plugins,cn=config -changetype: modify -replace: nsslapd-pluginType -nsslapd-pluginType: preoperation - -dn: cn=attribute uniqueness,cn=plugins,cn=config -changetype: modify -replace: nsslapd-pluginType -nsslapd-pluginType: preoperation - -dn: cn=Auto Membership Plugin,cn=plugins,cn=config -changetype: modify -replace: nsslapd-pluginType -nsslapd-pluginType: preoperation - -dn: cn=Linked Attributes,cn=plugins,cn=config -changetype: modify -replace: nsslapd-pluginType -nsslapd-pluginType: preoperation - -dn: cn=Managed Entries,cn=plugins,cn=config -changetype: modify -replace: nsslapd-pluginType -nsslapd-pluginType: preoperation - -dn: cn=MemberOf Plugin,cn=plugins,cn=config -changetype: modify -replace: nsslapd-pluginType -nsslapd-pluginType: postoperation - -dn: cn=Multimaster Replication Plugin,cn=plugins,cn=config -changetype: modify -replace: nsslapd-pluginbetxn -nsslapd-pluginbetxn: off - -dn: cn=PAM Pass Through Auth,cn=plugins,cn=config -changetype: modify -replace: nsslapd-pluginType -nsslapd-pluginType: preoperation - -dn: cn=referential integrity postoperation,cn=plugins,cn=config -changetype: modify -replace: nsslapd-pluginType -nsslapd-pluginType: postoperation - -dn: cn=Roles Plugin,cn=plugins,cn=config -changetype: modify -replace: nsslapd-pluginbetxn -nsslapd-pluginbetxn: off - -dn: cn=State Change Plugin,cn=plugins,cn=config -changetype: modify -replace: nsslapd-pluginType -nsslapd-pluginType: postoperation - -dn: cn=USN,cn=plugins,cn=config -changetype: modify -replace: nsslapd-pluginbetxn -nsslapd-pluginbetxn: off -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] Announcing FreeIPA 4.1.3
The FreeIPA team would like to announce FreeIPA v4.1.3 bug fix release! It can be downloaded from http://www.freeipa.org/page/Downloads . Fedora 21 builds are already on their way to updates-testing repository. Builds for Fedora 20 are available in the official COPR repository [https://copr.fedoraproject.org/coprs/mkosek/freeipa/]. == Highlights in 4.1.3 == === Enhancements === * ID Views support user SSH public keys * ID Views support IPA user overrides * OTP token authentication and synchronization windows are configurable * RADIUS server proxy fields added to user page in Web UI === Bug fixes === * Issues fixed in ipa-restore: ** doesn't crash if replica is unreachable ** checks if it isn't a restore on non matching host ** improved validation of input options to disallow invalid combinations ** doesn't fail if run on a system without IPA installed ** creates correct log directories * certificate renewal process is synchronized * migrate-ds: warns user if compat plugin is enabled * PassSync plugin could not update synchronized users due to too strict access control * replication agreements by Replication Administrators could not be removed due to strict access control * anonymous read of a DUA profile was not possible due to strict access control * various upgrade fixes related to DNSSEC == Upgrading == Upgrade instructions are available on upgrade page [http://www.freeipa.org/page/Upgrade]. == Feedback == Please provide comments, bugs and other feedback via the freeipa-users mailing list (http://www.redhat.com/mailman/listinfo/freeipa-users) or #freeipa channel on Freenode. == Detailed Changelog since 4.1.2 == === Alexander Bokovoy (4) === * Support Samba PASSDB 0.2.0 aka interface version 24 * ipa-cldap: support NETLOGON_NT_VERSION_5EX_WITH_IP properly * ipa-kdb: when processing transitions, hand over unknown ones to KDC * ipa-kdb: reject principals from disabled domains as a KDC policy === David Kupka (5) === * Use singular in help metavars + update man pages. * Always add /etc/hosts record when DNS is being configured. * Remove ipanttrustauthincoming/ipanttrustauthoutgoing from ipa trust-add output. * Abort backup restoration on not matching host. * idviews: Allow setting ssh public key on ipauseroverride-add === Gabe Alford (3) === * Remove dependency on subscription-manager * Typos in ipa-rmkeytab options help and man page * permission-add does not prompt for ipapermright in interactive mode === Jan Cholasta (18) === * Fix automatic CA cert renewal endless loop in dogtag-ipa-ca-renew-agent * Do not renew the IPA CA cert by serial number in dogtag-ipa-ca-renew-agent * Improve validation of --instance and --backend options in ipa-restore * Check subject name encoding in ipa-cacert-manage renew * Refer the user to freeipa.org when something goes wrong in ipa-cacert-manage * Fix ipa-restore on systems without IPA installed * Remove RUV from LDIF files before using them in ipa-restore * Fix CA certificate renewal syslog alert * Do not crash on unknown services in installutils.stopped_service * Restart dogtag when its server certificate is renewed * Make certificate renewal process synchronized * Fix validation of ipa-restore options * Do not assume certmonger is running in httpinstance * Put LDIF files to their original location in ipa-restore * Revert Make all ipatokenTOTP attributes mandatory * Create correct log directories during full restore in ipa-restore * Do not crash when replica is unreachable in ipa-restore * Bump 389-ds-base and pki-ca dependencies for POODLE fixes === Jan Pazdziora (1) === * No explicit zone specification. === Martin Babinsky (11) === * Moved dbus-python dependence to freeipa-python package * ipa-kdb: unexpected error code in 'ipa_kdb_audit_as_req' triggers a message * always get PAC for client principal if AS_REQ is true * ipa-kdb: more robust handling of principal addition/editing * OTP: failed search for the user of last token emits an error message * ipa-pwd-extop: added an informational comment about intentional fallthrough * ipa-uuid: emit a message when unexpected mod type is encountered * OTP: emit a log message when LDAP entry for config record is not found * ipa-client-install: put eol character after the last line of altered config file(s) * migrate-ds: exit with error message if no users/groups to migrate are found * Changing the token owner changes also the manager === Martin Bašti (19) === * Fix zonemgr option encoding detection * Throw zonemgr error message before installation proceeds * Upgrade fix: masking named should be executed only once * Using wget to get status of CA * Show SSHFP record containing space in fingerprint * Fix don't check certificate during getting CA status * Fix: Upgrade forwardzones zones after adding newer replica * Fix zone find during forwardzone upgrade * Fix traceback if zonemgr error contains unicode * DNS tests: separate current forward zone tests * New test cases for Forward_zones *
Re: [Freeipa-devel] [PATCH 0195] Fix memory leaks in ipapkcs11helper module
On 25/02/15 14:21, Martin Basti wrote: Ticket: https://fedorahosted.org/freeipa/ticket/4657 Patch attached. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel I forgot to mention, requires patch mbasti-0190 -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 133] ipa-range-check: do not treat missing objects as error
On Wed, Feb 25, 2015 at 02:43:05PM +0100, Martin Kosek wrote: On 02/24/2015 06:47 PM, Sumit Bose wrote: Hi, this patch changes a return code and should fix https://fedorahosted.org/freeipa/ticket/4924 . bye, Sumit I have a related question. Do I read the plugin right, that whenever any object is changed, this plugins loads the whole entry and tests some of it's attribute to see if it is ID views and then does the actual check. Is this good approach performance wise? Wouldn't it be better to decide even before that, based on DN and whether it is in the ID View Sub-Tree? CCing Thierry. I assume you meant id-ranges instead of views, but yes, as long as we add range objects only in a given sub-tree we can check the DN first. bye, Sumit Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0197-0198] Fix uniqueness plugins upgrade
On 02/25/2015 04:00 PM, Alexander Bokovoy wrote: On Wed, 25 Feb 2015, Martin Basti wrote: On 25/02/15 15:37, Alexander Bokovoy wrote: On Wed, 25 Feb 2015, Martin Basti wrote: Modifications: * All plugins are migrated into new configuration style. * I left attribute uniqueness plugin disabled, cn=uid uniqueness,cn=plugins,cn=config is checking the same attribute. * POST_UPDATE plugin for uid removed, I moved it to update file. Is it okay Alexander? I haven't found reason why we need to do it in update plugin. So I looked up the original thread and since there are three different ways of defining uniqueness plugin's configuration, update plugin was to me the only way to handle all different configuration types. In general we cannot rely on the fact that FreeIPA deployment only contains FreeIPA-defined plugin configurations. IMO, we should care only about IPA configured plugins, we cant handle everything, what users added there. If user adds an own plugin configuration there, the one should keep responsibility to test, if the plugin configuration still works after the IPA upgrade. We can't keep what user want, and what IPA needs in all cases, we would break IPA or users expectations, or both. In this case we can add detection of conflicts and print errors during upgrade, but we cant fix plugins which user created. If we want to handle user custom configuration, we will need to add detection for lot of things during upgrade not just uid uniqueness plugin. I tend to agree with Martin. I know that we created the special uniqueness plugin handling custom user plugins in the last release, but I am not convinced it is a good idea, it adds complexity to the upgrade, making it more difficult to debug. So if we can create the cn=uid uniqueness,cn=plugins,cn=config plugin just with simple update, I am fine with it. Uhm, right. Where my brain was today? :) Is that an agreement with Martin's approach? I am not sure :-) ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0039] Add test case for unsupported arg for ipa-advise
No worries about the delay. Thanks for taking the time! Updated patch attached. Thanks, Gabe On Tue, Feb 24, 2015 at 11:03 AM, Tomas Babej tba...@redhat.com wrote: Hi Gabe, sorry for the delay. Here comes the review! 1.) All the tests fail, since the IPA master is not installed at all: def test_advice(self): # Obtain the advice from the server tasks.kinit_admin(self.master) test_integration/test_advise.py:37: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ test_integration/tasks.py:484: in kinit_admin stdin_text=host.config.admin_password) ../pytest_multihost/host.py:222: in run_command command.wait(raiseonerr=raiseonerr) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = pytest_multihost.transport.SSHCommand object at 0x7f09c0530c90 raiseonerr = True def wait(self, raiseonerr=True): Wait for the remote process to exit Raises an excption if the exit code is not 0, unless raiseonerr is true. if self._done: return self.returncode self._end_process() self._done = True if raiseonerr and self.returncode: self.log.error('Exit code: %s', self.returncode) raise subprocess.CalledProcessError(self.returncode, self.argv) E CalledProcessError: Command '['kinit', 'admin']' returned non-zero exit status 1 Similiarly for other tests. This is caused by the fact that you did not set topology in the BaseTestAdvise class, like this: --- a/ipatests/test_integration/test_advise.py +++ b/ipatests/test_integration/test_advise.py @@ -31,6 +31,7 @@ class BaseTestAdvise(IntegrationTest, object): advice_id = None raiseerr = None advice_regex = '' +topology = 'line' 2.) BaseTestAdvise inherits from IntegrationTest and from object. Explicitly specifying object as superclass is not needed, IntegrationTest already inherits from it. 3.) I think there is no good incentive to separate the test cases into mutliple classes. Each test class adds overhead of installing and uninstalling IPA server, to guarantee a clean and sane environment. However, it seems to be an overkill for testing ipa-advise command, which should be read-only anyway. By squashing the tests into one test class, we will decrease the run time of this test more than 8-fold. 4.) The patch adds a whitespace error. The test cases themselves are looking fine, and when I fixed the missing topology, they all passed. So this is a question of fixing the above issues, and we should be ready to push. Tomas freeipa-rga-0039-2-ipatests-Add-tests-for-valid-and-invalid-ipa-advise.patch Description: Binary data ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0197-0198] Fix uniqueness plugins upgrade
On 25/02/15 15:37, Alexander Bokovoy wrote: On Wed, 25 Feb 2015, Martin Basti wrote: Modifications: * All plugins are migrated into new configuration style. * I left attribute uniqueness plugin disabled, cn=uid uniqueness,cn=plugins,cn=config is checking the same attribute. * POST_UPDATE plugin for uid removed, I moved it to update file. Is it okay Alexander? I haven't found reason why we need to do it in update plugin. So I looked up the original thread and since there are three different ways of defining uniqueness plugin's configuration, update plugin was to me the only way to handle all different configuration types. In general we cannot rely on the fact that FreeIPA deployment only contains FreeIPA-defined plugin configurations. IMO, we should care only about IPA configured plugins, we cant handle everything, what users added there. If user adds an own plugin configuration there, the one should keep responsibility to test, if the plugin configuration still works after the IPA upgrade. We can't keep what user want, and what IPA needs in all cases, we would break IPA or users expectations, or both. In this case we can add detection of conflicts and print errors during upgrade, but we cant fix plugins which user created. If we want to handle user custom configuration, we will need to add detection for lot of things during upgrade not just uid uniqueness plugin. Martin^2 -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0197-0198] Fix uniqueness plugins upgrade
On Wed, 25 Feb 2015, Martin Basti wrote: On 25/02/15 15:37, Alexander Bokovoy wrote: On Wed, 25 Feb 2015, Martin Basti wrote: Modifications: * All plugins are migrated into new configuration style. * I left attribute uniqueness plugin disabled, cn=uid uniqueness,cn=plugins,cn=config is checking the same attribute. * POST_UPDATE plugin for uid removed, I moved it to update file. Is it okay Alexander? I haven't found reason why we need to do it in update plugin. So I looked up the original thread and since there are three different ways of defining uniqueness plugin's configuration, update plugin was to me the only way to handle all different configuration types. In general we cannot rely on the fact that FreeIPA deployment only contains FreeIPA-defined plugin configurations. IMO, we should care only about IPA configured plugins, we cant handle everything, what users added there. If user adds an own plugin configuration there, the one should keep responsibility to test, if the plugin configuration still works after the IPA upgrade. We can't keep what user want, and what IPA needs in all cases, we would break IPA or users expectations, or both. In this case we can add detection of conflicts and print errors during upgrade, but we cant fix plugins which user created. If we want to handle user custom configuration, we will need to add detection for lot of things during upgrade not just uid uniqueness plugin. Uhm, right. Where my brain was today? :) -- / Alexander Bokovoy pgpwWp_beEuk_.pgp Description: PGP signature ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 133] ipa-range-check: do not treat missing objects as error
On 02/25/2015 02:43 PM, Martin Kosek wrote: On 02/24/2015 06:47 PM, Sumit Bose wrote: Hi, this patch changes a return code and should fix https://fedorahosted.org/freeipa/ticket/4924 . bye, Sumit I have a related question. Do I read the plugin right, that whenever any object is changed, this plugins loads the whole entry and tests some of it's attribute to see if it is ID views and then does the actual check. Is this good approach performance wise? Wouldn't it be better to decide even before that, based on DN and whether it is in the ID View Sub-Tree? CCing Thierry. Martin Hello, My understanding of this preop plugins is that it checks the defined range (RangeID, RangeSize, RangeRid...) of a new/modified 'ipaBaseID' entry. It checks the range by comparing the new range against all ranges defined in objectclass=ipaIDRange entries. As far as I understand that plugin, I tend to agree with your point Martin. The DN of the new entry/modified entry is not taken into consideration. Only the fact that it contains ipaBaseID attribute. If we know that we can ignore ADD/MOD under ID View Sub-tree, then it could be tested early in the plugin and would accelerate the plugin. That would require a plugin config attibute to specify which subtrees should be ignore. thanks thierry ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] IPA Server upgrade 4.2 design
On 24.2.2015 19:10, Martin Basti wrote: Hello all, please read the design page, any objections/suggestions appreciated http://www.freeipa.org/page/V4/Server_Upgrade_Refactoring Thank you for the design, I have only few nitpicks. Increase update files numbers range Update files number will be extended into 4 digits values. IMHO the dependency on particular number format should be removed altogether. It should be perfectly enough to say that updates are executed in ASCII lexicographic order and be done with it. To resolve issues mentioned above only one command should do upgrade: ipa-server-upgrade. I very much agree with this. ipa-server-upgrade characteristics ... 4. LDAP data update (+ update plugins) 5. upgrade configuration At this point I would appreciate explanatory text what is 'LDAP data update' and what is 'upgrade configuration'. Maybe some examples could be enough. ipactl checks if installed version and version stored in LDAP are the same: ... ipactl start|restart option --force overrides this check. I would like to see a separate option. --force currently skips rollback if some services could not start but this is fundamentally different from version/upgrade checks. ipa-server-upgrade --version show program's version number and exit Maybe it could print code version + data version (if available). It could be handy debugging tool. ipa-server-upgrade --testNote: for developing only Is it really worth the effort to keep the option and invest more time in it? -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 808 webui: service: add ipakrbrequirespreauth checkbox
Allow to configure missing krb ticket flag - ipakrbrequirespreauth from Web UI. -- Petr Vobornik From 23def2a08bf3aec43bc341bafeebaf4fee302ad2 Mon Sep 17 00:00:00 2001 From: Petr Vobornik pvobo...@redhat.com Date: Wed, 25 Feb 2015 12:56:19 +0100 Subject: [PATCH] webui: service: add ipakrbrequirespreauth checkbox Allow to configure missing krb ticket flag - ipakrbrequirespreauth from Web UI. --- install/ui/src/freeipa/service.js | 5 + 1 file changed, 5 insertions(+) diff --git a/install/ui/src/freeipa/service.js b/install/ui/src/freeipa/service.js index 94842a912c77a55acad9d2f0881f3ad23915f700..5c28e939dedf0986d969afeeb6cff00f856bea8e 100644 --- a/install/ui/src/freeipa/service.js +++ b/install/ui/src/freeipa/service.js @@ -112,6 +112,11 @@ return { name: 'ipakrbokasdelegate', $type: 'checkbox', acl_param: 'krbticketflags' +}, +{ +name: 'ipakrbrequirespreauth', +$type: 'checkbox', +acl_param: 'krbticketflags' } ] }, -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0197-0198] Fix uniqueness plugins upgrade
On 02/25/2015 02:34 PM, Martin Basti wrote: Modifications: * All plugins are migrated into new configuration style. * I left attribute uniqueness plugin disabled, cn=uid uniqueness,cn=plugins,cn=config is checking the same attribute. * POST_UPDATE plugin for uid removed, I moved it to update file. Is it okay Alexander? I haven't found reason why we need to do it in update plugin. Thierry, I touched configuration of plugins, which user lifecycle requires, can you take look if I it does not break anything? Patches attached. Hello Martin, The fix looks good. I have just one question regarding install/updates/10-uniqueness.update. For example : # uid uniqueness scopes Active/Delete containers dn: cn=attribute uniqueness,cn=plugins,cn=config -remove:nsslapd-pluginarg1:'$SUFFIX' -add:nsslapd-pluginarg1:'cn=accounts,$SUFFIX' -add:nsslapd-pluginarg2:'cn=deleted users,cn=accounts,cn=provisioning,$SUFFIX' +remove:uniqueness-subtrees:'$SUFFIX' +add:uniqueness-subtrees:'cn=accounts,$SUFFIX' +add:uniqueness-subtrees:'cn=deleted users,cn=accounts,cn=provisioning,$SUFFIX' remove:nsslapd-pluginenabled:off add:nsslapd-pluginenabled:on If we update the rpm from a version where 'nsslapd-pluginarg1' was used. It will not remove it and we will have 'nsslapd-pluginarg1' along with 'uniqueness-subtrees'. Should not we keep 'remove:nsslapd-pluginarg1:'$SUFFIX'' ? thanks thierry ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0197-0198] Fix uniqueness plugins upgrade
On 02/25/2015 05:28 PM, Martin Basti wrote: On 25/02/15 17:23, thierry bordaz wrote: On 02/25/2015 02:34 PM, Martin Basti wrote: Modifications: * All plugins are migrated into new configuration style. * I left attribute uniqueness plugin disabled, cn=uid uniqueness,cn=plugins,cn=config is checking the same attribute. * POST_UPDATE plugin for uid removed, I moved it to update file. Is it okay Alexander? I haven't found reason why we need to do it in update plugin. Thierry, I touched configuration of plugins, which user lifecycle requires, can you take look if I it does not break anything? Patches attached. Hello Martin, The fix looks good. I have just one question regarding install/updates/10-uniqueness.update. For example : # uid uniqueness scopes Active/Delete containers dn: cn=attribute uniqueness,cn=plugins,cn=config -remove:nsslapd-pluginarg1:'$SUFFIX' -add:nsslapd-pluginarg1:'cn=accounts,$SUFFIX' -add:nsslapd-pluginarg2:'cn=deleted users,cn=accounts,cn=provisioning,$SUFFIX' +remove:uniqueness-subtrees:'$SUFFIX' +add:uniqueness-subtrees:'cn=accounts,$SUFFIX' +add:uniqueness-subtrees:'cn=deleted users,cn=accounts,cn=provisioning,$SUFFIX' remove:nsslapd-pluginenabled:off add:nsslapd-pluginenabled:on If we update the rpm from a version where 'nsslapd-pluginarg1' was used. It will not remove it and we will have 'nsslapd-pluginarg1' along with 'uniqueness-subtrees'. Should not we keep 'remove:nsslapd-pluginarg1:'$SUFFIX'' ? thanks thierry Hello Thierry, in patch 0197 is pre-upgrade plugin, which migrate all uniqueness plugins into new syntax (this happens before the update file is applied). So no nsslapd-pluginarg* attrs will be there. and in patch 0198 I removed the cn=attribute uniquenes plugin, we already have cn=uid uniqueness that do the same thing. Martin^2 -- Martin Basti Ok. I understand. Thanks for the explanation. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0197-0198] Fix uniqueness plugins upgrade
On 25/02/15 17:23, thierry bordaz wrote: On 02/25/2015 02:34 PM, Martin Basti wrote: Modifications: * All plugins are migrated into new configuration style. * I left attribute uniqueness plugin disabled, cn=uid uniqueness,cn=plugins,cn=config is checking the same attribute. * POST_UPDATE plugin for uid removed, I moved it to update file. Is it okay Alexander? I haven't found reason why we need to do it in update plugin. Thierry, I touched configuration of plugins, which user lifecycle requires, can you take look if I it does not break anything? Patches attached. Hello Martin, The fix looks good. I have just one question regarding install/updates/10-uniqueness.update. For example : # uid uniqueness scopes Active/Delete containers dn: cn=attribute uniqueness,cn=plugins,cn=config -remove:nsslapd-pluginarg1:'$SUFFIX' -add:nsslapd-pluginarg1:'cn=accounts,$SUFFIX' -add:nsslapd-pluginarg2:'cn=deleted users,cn=accounts,cn=provisioning,$SUFFIX' +remove:uniqueness-subtrees:'$SUFFIX' +add:uniqueness-subtrees:'cn=accounts,$SUFFIX' +add:uniqueness-subtrees:'cn=deleted users,cn=accounts,cn=provisioning,$SUFFIX' remove:nsslapd-pluginenabled:off add:nsslapd-pluginenabled:on If we update the rpm from a version where 'nsslapd-pluginarg1' was used. It will not remove it and we will have 'nsslapd-pluginarg1' along with 'uniqueness-subtrees'. Should not we keep 'remove:nsslapd-pluginarg1:'$SUFFIX'' ? thanks thierry Hello Thierry, in patch 0197 is pre-upgrade plugin, which migrate all uniqueness plugins into new syntax (this happens before the update file is applied). So no nsslapd-pluginarg* attrs will be there. and in patch 0198 I removed the cn=attribute uniquenes plugin, we already have cn=uid uniqueness that do the same thing. Martin^2 -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0042] ipa-replica-prepare should document ipv6 options
Hello, Fix for https://fedorahosted.org/freeipa/ticket/4877. I just took what was in the ticket. Thanks, Gabe freeipa-rga-0042-ipa-replica-prepare-should-document-ipv6-options.patch Description: Binary data ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCHES 0001-0002] ipa-client-install NTP fixes
Hi, The two attached patches address some issues that affect ipa-client-install when syncing time from the NTP server. Now that we use ntpd to perform the time sync, the client install can end up hanging forever when the server is not reachable (firewall issues, etc.). These patches address the issues in two different ways: 1 - Don't attempt to sync time when --no-ntp is specified. 2 - Implement a timeout capability that is used when we run ntpd to perform the time sync to prevent indefinite hanging. The one potentially contentious issue is that this introduces a new dependency on python-subprocess32 to allow us to have timeout support when using Python 2.x. This is packaged for Fedora, but I don't see it on RHEL or CentOS currently. It would need to be packaged there. https://fedorahosted.org/freeipa/ticket/4842 Thanks, -NGK From 80514f225f628f7c7993b85e562a851e7ee40644 Mon Sep 17 00:00:00 2001 From: Nathan Kinder nkin...@redhat.com Date: Wed, 25 Feb 2015 14:22:02 -0800 Subject: [PATCH 1/2] Skip time sync during client install when using --no-ntp When --no-ntp is specified during ipa-client-install, we still attempt to perform a time sync before obtaining a TGT from the KDC. We should not be attempting to sync time with the KDC if we are explicitly told to not configure ntp. Ticket: https://fedorahosted.org/freeipa/ticket/4842 --- ipa-client/ipa-install/ipa-client-install | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install index ccaab55..a625fbd 100755 --- a/ipa-client/ipa-install/ipa-client-install +++ b/ipa-client/ipa-install/ipa-client-install @@ -2324,8 +2324,9 @@ def install(options, env, fstore, statestore): # hostname if different from system hostname tasks.backup_and_replace_hostname(fstore, statestore, options.hostname) -if not options.on_master: +if not options.on_master and options.conf_ntp: # Attempt to sync time with IPA server. +# If we're skipping NTP configuration, we also skip the time sync here. # We assume that NTP servers are discoverable through SRV records in the DNS # If that fails, we try to sync directly with IPA server, assuming it runs NTP root_logger.info('Synchronizing time with KDC...') -- 1.9.3 From 7ba0745b10145f516536a2b1b711203b5bb8cb09 Mon Sep 17 00:00:00 2001 From: Nathan Kinder nkin...@redhat.com Date: Wed, 25 Feb 2015 15:19:47 -0800 Subject: [PATCH 2/2] Timeout when performing time sync during client install We use ntpd now to sync time before fetching a TGT during client install. Unfortuantely, ntpd will hang forever if it is unable to reach the NTP server. This patch adds the ability for commands run via ipautil.run() to have an optional timeout. This capability is used by the NTP sync code that is run during ipa-client-install. This new timeout capability requires a new dependency on the subprocess32 module, which is a backport of the Python 3.x subprocess module for Python 2.x. Ticket: https://fedorahosted.org/freeipa/ticket/4842 --- freeipa.spec.in | 1 + ipa-client/ipaclient/ntpconf.py | 4 +++- ipapython/ipautil.py| 21 +++-- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/freeipa.spec.in b/freeipa.spec.in index b186d9f..8379b31 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -285,6 +285,7 @@ Requires: libipa_hbac-python Requires: python-qrcode-core = 5.0.0 Requires: python-pyasn1 Requires: python-dateutil +Requires: python-subprocess32 Requires: python-yubico Requires: wget Requires: dbus-python diff --git a/ipa-client/ipaclient/ntpconf.py b/ipa-client/ipaclient/ntpconf.py index e1ac55a..2cc3332 100644 --- a/ipa-client/ipaclient/ntpconf.py +++ b/ipa-client/ipaclient/ntpconf.py @@ -149,7 +149,9 @@ def synconce_ntp(server_fqdn): tmp_ntp_conf = ipautil.write_tmp_file('server %s' % server_fqdn) try: -ipautil.run([ntpd, '-qgc', tmp_ntp_conf.name]) +# The ntpd command will never exit if it is unable to reach the +# server, so timeout after 15 seconds. +ipautil.run([ntpd, '-qgc', tmp_ntp_conf.name], timeout=15) return True except ipautil.CalledProcessError: return False diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py index 4116d97..41046fc 100644 --- a/ipapython/ipautil.py +++ b/ipapython/ipautil.py @@ -20,6 +20,7 @@ import string import tempfile import subprocess +import subprocess32 import random import os, sys, traceback import copy @@ -38,6 +39,7 @@ import krbV import pwd from dns import resolver, rdatatype from dns.exception import DNSException +from subprocess32 import TimeoutExpired from ipapython.ipa_log_manager import * from ipapython import ipavalidate @@ -249,7 +251,7 @@ def shell_quote(string): def run(args, stdin=None, raiseonerr=True, nolog=(), env=None, capture_output=True,
Re: [Freeipa-devel] [PATCH 0042] ipa-replica-prepare should document ipv6 options
On 02/25/2015 09:26 PM, Gabe Alford wrote: Hello, Fix for https://fedorahosted.org/freeipa/ticket/4877. I just took what was in the ticket. Thanks, Gabe ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK (I did a very minor change to wording). Pushed to: master: c75025df8cd9b634ffe464e07ba23a2ee99b3e2d ipa-4-1: 3ab7f551f86bee75b5260901352ec6538ebda50e ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0039] Add test case for unsupported arg for ipa-advise
Hi Gabe, sorry for not being clear. This approach will not work: +class TestAdvice(BaseTestInvalidAdvice, + BaseTestFedoraAuthconfig, + BaseTestFreeBSDNSSPAM, + BaseTestGenericNSSPAM, + BaseTestGenericSSSDBefore19, + BaseTestRedHatNSS, + BaseTestRedHatNSSPAM, + BaseTestRedHatSSSDBefore19, + BaseTestAdvice): +pass By combining all the base classes into one, you will not get the desired effect (which is to run the test_advice method for each advice_id). Let me explain why: The test runner works in the following way: it inspects any discovered class which name begins with Test, and executes each its method, which names begins with test as a test case. If the test runner inspects the TestAdvice class, the only method beggining with test, which it will see, is the test_advice which was inherited back from BaseTestAdvice class. So we can safely conclude the test runner will only run 1 test case. Which one, you may ask? Well, since the test_advice behaviour is fully determined by the values of advice_id, advice_regex and raiseerr attributes, let's look at their values in TestAdvice class. This class does not define attirbutes with such names, so we move along the inheritance chain (also called MRO) - the first class from which we inherit is BaseTestInvalidAdvice, and this class defines all three mentioned attributes. Hence the only test method will be run the test for invalid advice :) Now, how to fix this? The easiest approach would be to abandon the approach with the separate classes, and map each class to a test method in the TestAdvice class, like this (from the top of my head): +class TestAdvice(IntegrationTest): +topology = 'line' + +def test_invalid_advice(self): +advice_id = 'invalid-advise-param' +advice_regex = invalid[\s]+\'advice\'.* +raiseerr = False +# Obtain the advice from the server +tasks.kinit_admin(self.master) +result = self.master.run_command(['ipa-advise', self.advice_id], + raiseonerr=self.raiseerr) + +if not result.stdout_text: +advice = result.stderr_text +else: +advice = result.stdout_text + +assert re.search(self.advice_regex, advice, re.S) + +def test_advice_fedora_authconfig(self): +advice_id = 'config-fedora-authconfig' +advice_regex = \#\!\/bin\/sh.* \ + authconfig[\s]+\-\-enableldap[\s]+ \ + \-\-ldapserver\=.*[\s]+\-\-enablerfc2307bis[\s]+ \ + \-\-enablekrb5 +raiseonerr = True +# Obtain the advice from the server +tasks.kinit_admin(self.master) +result = self.master.run_command(['ipa-advise', self.advice_id], + raiseonerr=self.raiseerr) + +if not result.stdout_text: +advice = result.stderr_text +else: +advice = result.stdout_text + +assert re.search(self.advice_regex, advice, re.S) ... the same for the remaining 6 cases Now, this pattern has lots of duplicated code which can be extracted to a helper method, I just thought it would help to be more explicit to get the idea across. In the end you can achieve the same level of conciseness than with the separate test classes. Good luck! HTH, Tomas On 02/25/2015 03:52 PM, Gabe Alford wrote: No worries about the delay. Thanks for taking the time! Updated patch attached. Thanks, Gabe On Tue, Feb 24, 2015 at 11:03 AM, Tomas Babej tba...@redhat.com mailto:tba...@redhat.com wrote: Hi Gabe, sorry for the delay. Here comes the review! 1.) All the tests fail, since the IPA master is not installed at all: def test_advice(self): # Obtain the advice from the server tasks.kinit_admin(self.master) test_integration/test_advise.py:37: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ test_integration/tasks.py:484: in kinit_admin stdin_text=host.config.admin_password) ../pytest_multihost/host.py:222: in run_command command.wait(raiseonerr=raiseonerr) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = pytest_multihost.transport.SSHCommand object at 0x7f09c0530c90 raiseonerr = True def wait(self, raiseonerr=True): Wait for the remote process to exit Raises an excption if the exit code is not 0, unless raiseonerr is true. if self._done: return self.returncode self._end_process() self._done = True if raiseonerr and self.returncode: self.log.error('Exit code: %s', self.returncode)
Re: [Freeipa-devel] [PATCH] 808 webui: service: add ipakrbrequirespreauth checkbox
On 02/25/2015 05:22 PM, Petr Vobornik wrote: Allow to configure missing krb ticket flag - ipakrbrequirespreauth from Web UI. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK, works fine. Pushed to master: 55413566caf7ba9b2f5b40d160c3a105c6599ac2 Please push to ipa-4-1 if necessary. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel