Re: [Freeipa-devel] [PATCH] 55 Parse comma-separated lists of values in all parameter types
Dne 4.11.2011 22:25, Rob Crittenden napsal(a):
Jan Cholasta wrote:
Dne 24.10.2011 17:42, Rob Crittenden napsal(a):
Jan Cholasta wrote:
Dne 20.10.2011 13:20, Jan Cholasta napsal(a):
Parse comma-separated lists of values in all parameter types. This can
enabled for a specific parameter by setting the csvlist option to
True.
Remove List parameter type and replace all occurences with Str with
csvlist enabled.
https://fedorahosted.org/freeipa/ticket/2007
This change will be useful for
https://fedorahosted.org/freeipa/ticket/1487 and
https://fedorahosted.org/freeipa/ticket/1847
Unit tests show no regressions.
Honza
Self-NACK - I have noticed that the batch command no longer works.
Updated patch attached.
Honza
What is the benefit of this over the List parameter type?
rob
Mainly because the List parameter type is just a hack. This is the right
thing to do if we want to use comma-separated lists of parameters of any
type, with all the validation and other parameter type-specific features.
For example, I've added a new parameter type for IP addresses in my
patch 46
(http://www.redhat.com/archives/freeipa-devel/2011-September/msg00187.html)
and use it for A and DNS records. Without this patch, we can either
use List for the record parameters and lose validation in dnsrecord-find
(because it is based on crud.Search, which strips all the custom
validation rules - like _validate_ipaddr - from the command parameters,
which is one of the causes of #1627) or use IPAddress for the record
parameters and lose the ability to specify them as comma-separated list
of values. With this patch, we can have both comma-separated lists and
validation at the same time.
Besides, the patch is not as big as it looks like, all the interesting
stuff is in ipalib/parameters.py, everything else is just
search-and-replace. Also I need it to fix #1487 and #1847 without doing
ugly hacks.
Honza
I think this would constitute a major version change.
I'm not sure about that, as the patch doesn't break API compatibility -
a string containing a comma-separated list of values is used for list
parameters both with and without the patch.
One downside is you can no longer tell in the help with arguments take a
CSV and which don't.
Why not? A simple look at csvlist value should provide enough information.
I think the CSV-related Parameter options should all begin with csv,
separator and skipspace.
OK.
The batch command may eventually be made into a command, how will that
affect the Any type?
Batch currently uses List for the methods parameter not because of its
CSV capability, but because it doesn't do any type conversion and
validation on the values. This allows it to use values of the form
{u'params': [args_list, kwargs_dict], u'method': method_name}. The Any
parameter type exists so that this can still be done without List - it's
basically a single-valued version of List (i.e. Any(csvlist=True) is
equivalent to List()).
IMO if batch is ever made into a command, it would have to be redesigned
not to use List/Any, so that it can be used from CLI with validation of
the parameter values. Any can then be easily removed.
It otherwise seems to work in my spot-testing.
rob
Honza
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 308 Added current password field.
On 11/04/2011 11:29 PM, Endi Sukma Dewata wrote: The reset password dialog for user has been modified to provide a field to specify the current password when changing the user's own password. Ticket #2065 ACK ipa-2-1, master -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 308 Added current password field.
On Mon, 2011-11-07 at 13:48 +0100, Petr Vobornik wrote: On 11/04/2011 11:29 PM, Endi Sukma Dewata wrote: The reset password dialog for user has been modified to provide a field to specify the current password when changing the user's own password. Ticket #2065 ACK ipa-2-1, master Pushed to master, ipa-2-1. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 029 Page is cleared before it is visible
On 11/05/2011 01:37 AM, Endi Sukma Dewata wrote: On 11/4/2011 11:02 AM, Petr Vobornik wrote: Found another problem, changing page in the association facet didn't work because pkey is still the same. See the attached patch. ACK and pushed to master -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 030 Extending facet's mechanism of gathering changes
On 11/04/2011 06:22 PM, Endi Sukma Dewata wrote: Rebased, ACK, and pushed to master. Some comments below. On 11/4/2011 7:21 AM, Petr Vobornik wrote: Perhaps the save_as_update_info() later can be merged with get_update_info() too because both are essentially generating update_info for dirty fields. Yes, If we ensure that appropriate get_update_info() method will be in each custom widget/section, which isn't now. Attached preview patch for #1515. Also attaching diff patch of reviewed patch. OK, I see how the enable widget creates the update info. How would you handle the removal of users in HBAC rule when the usercategory is changed to ALL? This logic is part of rule_association_table_widget:rule.js:142. The radio buttons are triggering an event which disables/enables the asso. table. During get_update_info if the asso. table is disabled and it has entries in it, it creates a command which deletes the entries. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 160 Hosts file not updated when IP is passed as option
When an IPA server with unresolvable hostname is being installed, a hostname record must be inserted to /etc/hosts or the installation will fail. However, it is not inserted when IP address is passed as an option (--ip-address) and not interactively. This patch fixes this so that /etc/hosts record is inserted in both cases. https://fedorahosted.org/freeipa/ticket/2074 From 52b097218c4c031c4a91557611e31dd1da921911 Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Mon, 7 Nov 2011 18:35:23 +0100 Subject: [PATCH] Hosts file not updated when IP is passed as option When an IPA server with unresolvable hostname is being installed, a hostname record must be inserted to /etc/hosts or the installation will fail. However, it is not inserted when IP address is passed as an option (--ip-address) and not interactively. This patch fixes this so that /etc/hosts record is inserted in both cases. https://fedorahosted.org/freeipa/ticket/2074 --- install/tools/ipa-server-install |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install index 1dbeef59620ff135efd68fb47fef740015b62639..4c56b66172fb41d2c0372022b6608a118083e116 100755 --- a/install/tools/ipa-server-install +++ b/install/tools/ipa-server-install @@ -754,10 +754,14 @@ def main(): # Check we have a public IP that is associated with the hostname hostaddr = resolve_host(host_name) +ip_add_to_hosts = False if hostaddr is not None: ip = CheckedIPAddress(hostaddr, match_local=True) else: +# hostname is not resolvable ip = options.ip_address +ip_add_to_hosts = True + if ip is None: print Unable to resolve IP address for host name if options.unattended: @@ -772,11 +776,9 @@ def main(): ip = options.ip_address -ip_add_to_hosts = False if ip is None: ip = read_ip_address(host_name, fstore) logging.debug(read ip_address: %s\n % str(ip)) -ip_add_to_hosts = True ip_address = str(ip) -- 1.7.6.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] #2036 Fix coverity bugs
On Thu, 03 Nov 2011, Simo Sorce wrote:
Just some unchecked returns, we do not care much, but will keep Coverity
happy.
ACK to all below, they seem to be straight-forward.
As slapi_ch_free_string() accepts a pointer to NULL and does nothing in
that case, you don't need to protect against fetched NULL in
ipa_pwd_extop.c.
https://fedorahosted.org/freeipa/ticket/2036
---
.../ipa-enrollment/ipa_enrollment.c|2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-enrollment/ipa_enrollment.c
b/daemons/ipa-slapi-plugins/ipa-enrollment/ipa_enrollment.c
index
1f5ce9b477a6152e3ef7befeea1230ab75dfba70..9f884bd39233adf90b0f4eff1868885d587d351a
100644
--- a/daemons/ipa-slapi-plugins/ipa-enrollment/ipa_enrollment.c
+++ b/daemons/ipa-slapi-plugins/ipa-enrollment/ipa_enrollment.c
@@ -451,7 +451,7 @@ ipaenrollment_init(Slapi_PBlock *pb)
if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_DESCRIPTION, (void
*)pdesc);
if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_OIDLIST,
ipaenrollment_oid_list);
if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_NAMELIST,
ipaenrollment_name_list);
-if (!ret) slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_FN, (void
*)ipaenrollment_extop);
+if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_FN, (void
*)ipaenrollment_extop);
if (ret) {
LOG(Failed to set plug-in version, function, and OID.\n);
ACK.
https://fedorahosted.org/freeipa/ticket/2036
---
.../ipa-pwd-extop/ipa_pwd_extop.c | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index
95ac68e9cfc8d7024048d8a9d2793044f01dd1aa..a0f9c5e14747b5c38952db27b005874a4afe1c4d
100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -504,9 +504,15 @@ free_and_return:
/* Either this is the same pointer that we allocated and set above,
* or whoever used it should have freed it and allocated a new
* value that we need to free here */
- slapi_pblock_get(pb, SLAPI_ORIGINAL_TARGET, dn);
+ret = slapi_pblock_get(pb, SLAPI_ORIGINAL_TARGET, dn);
+if (ret) {
+LOG_TRACE(Failed to get SLAPI_ORIGINAL_TARGET\n);
+}
slapi_ch_free_string(dn);
- slapi_pblock_set(pb, SLAPI_ORIGINAL_TARGET, NULL);
+ret = slapi_pblock_set(pb, SLAPI_ORIGINAL_TARGET, NULL);
+if (ret) {
+LOG_TRACE(Failed to clear SLAPI_ORIGINAL_TARGET\n);
+}
slapi_ch_free_string(authmethod);
slapi_ch_free_string(principal);
ACK.
https://fedorahosted.org/freeipa/ticket/2036
---
.../ipa-pwd-extop/ipa_pwd_extop.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index
a0f9c5e14747b5c38952db27b005874a4afe1c4d..65c5834595f89aee8502347311f247be058c3416
100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -1295,7 +1295,7 @@ int ipapwd_init( Slapi_PBlock *pb )
if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_DESCRIPTION, (void
*)ipapwd_plugin_desc);
if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_OIDLIST,
ipapwd_oid_list);
if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_NAMELIST,
ipapwd_name_list);
-if (!ret) slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_FN, (void
*)ipapwd_extop);
+if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_FN, (void
*)ipapwd_extop);
if (ret) {
LOG(Failed to set plug-in version, function, and OID.\n );
return -1;
ACK.
--
/ Alexander Bokovoy
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] #2037 Coverity issues
On Thu, 03 Nov 2011, Simo Sorce wrote:
These are actual issues. Most are resource leaks.
And one is a bad sizeof() computation that will cause us later to
overwrite out of bound memory, so potentially a segfault.
Went through all of these. ACK.
https://fedorahosted.org/freeipa/ticket/2037
---
daemons/ipa-kdb/ipa_kdb.c | 13 +++--
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c
index
6a6c2063902f8b2a76d97f3510f09333c5af168d..481b1f392766498c5d7c6333fe73bafefde87dae
100644
--- a/daemons/ipa-kdb/ipa_kdb.c
+++ b/daemons/ipa-kdb/ipa_kdb.c
@@ -263,6 +263,13 @@ int ipadb_get_connection(struct ipadb_context *ipactx)
done:
ldap_msgfree(res);
+
+ldap_value_free_len(vals);
+for (i = 0; i c cvals[i]; i++) {
+free(cvals[i]);
+}
+free(cvals);
+
if (ret) {
if (ipactx-lcontext) {
ldap_unbind_ext_s(ipactx-lcontext, NULL, NULL);
@@ -274,12 +281,6 @@ done:
return EIO;
}
-ldap_value_free_len(vals);
-for (i = 0; i c; i++) {
-free(cvals[i]);
-}
-free(cvals);
-
return 0;
}
ACK.
https://fedorahosted.org/freeipa/ticket/2037
---
daemons/ipa-kdb/ipa_kdb_passwords.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb_passwords.c
b/daemons/ipa-kdb/ipa_kdb_passwords.c
index
93e9e206081af412a472ab0c7624611a628a15b7..0bb7fa72496789241e27ecc852a1c6ede7f8e40a
100644
--- a/daemons/ipa-kdb/ipa_kdb_passwords.c
+++ b/daemons/ipa-kdb/ipa_kdb_passwords.c
@@ -203,6 +203,7 @@ krb5_error_code ipadb_change_pwd(krb5_context context,
ret = asprintf(ied-pw_policy_dn,
cn=global_policy,%s, ipactx-realm_base);
if (ret == -1) {
+free(ied);
return ENOMEM;
}
db_entry-e_data = (krb5_octet *)ied;
ACK.
https://fedorahosted.org/freeipa/ticket/2037
---
util/ipa_pwd.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/util/ipa_pwd.c b/util/ipa_pwd.c
index
c41617533ec25e8e656e9bb7a69d5b8b5dd8b5f7..fda6cb34ef24059362207325db61aedb62d7b665
100644
--- a/util/ipa_pwd.c
+++ b/util/ipa_pwd.c
@@ -560,7 +560,7 @@ int ipapwd_generate_new_history(char *password,
unsigned char *hash = NULL;
unsigned int hash_len;
char *new_element;
-char **ordered;
+char **ordered = NULL;
int c, i, n;
int len;
int ret;
@@ -626,9 +626,11 @@ int ipapwd_generate_new_history(char *password,
*new_pwd_history = ordered;
*new_pwd_hlen = n;
+ordered = NULL;
ret = IPAPWD_POLICY_OK;
done:
+free(ordered);
free(hash);
return ret;
}
ACK
https://fedorahosted.org/freeipa/ticket/2037
---
daemons/ipa-kdb/ipa_kdb_principals.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c
b/daemons/ipa-kdb/ipa_kdb_principals.c
index
fdd834f355fd9e056058fa205b217e9e1f142e51..117eea86952ee4662930b80ba5e54c75aa587faf
100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -1571,6 +1571,7 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context
kcontext,
char **new_history;
int nh_len;
int ret;
+int i;
ied = (struct ipadb_e_data *)entry-e_data;
if (ied-magic != IPA_E_DATA_MAGIC) {
@@ -1619,6 +1620,12 @@ static krb5_error_code
ipadb_entry_to_mods(krb5_context kcontext,
kerr = ipadb_get_ldap_mod_str_list(imods, passwordHistory,
new_history, nh_len, mod_op);
+
+for (i = 0; i nh_len; i++) {
+free(new_history[i]);
+}
+free(new_history);
+
if (kerr) {
goto done;
}
ACK.
https://fedorahosted.org/freeipa/ticket/2037
---
daemons/ipa-kdb/ipa_kdb_principals.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c
b/daemons/ipa-kdb/ipa_kdb_principals.c
index
117eea86952ee4662930b80ba5e54c75aa587faf..bb1356a01a27a639c15439187ffb8d3537c1fcec
100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -334,6 +334,7 @@ done:
free(keys[i].key_data_contents[0]);
free(keys[i].key_data_contents[1]);
}
+free(keys);
*result = NULL;
*num = 0;
}
ACK.
https://fedorahosted.org/freeipa/ticket/2037
---
daemons/ipa-kdb/ipa_kdb_principals.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c
b/daemons/ipa-kdb/ipa_kdb_principals.c
index
Re: [Freeipa-devel] [PATCHES] #2036 Fix coverity bugs
On Mon, 2011-11-07 at 22:42 +0200, Alexander Bokovoy wrote: ACK to all below, they seem to be straight-forward. Thanks, pushed to master. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] #2037 Coverity issues
On Tue, 2011-11-08 at 00:22 +0200, Alexander Bokovoy wrote: Went through all of these. ACK. Thanks again, pushed to master. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] #1791 Tust Effort: Add support for generating MS-PAC
On Fri, 2011-11-04 at 23:31 +0100, Sumit Bose wrote: On Fri, Nov 04, 2011 at 10:49:40AM -0400, Simo Sorce wrote: The attached patches are for master and concern the effort of creating trust relationships between IPA and AD domains. With these patches if you have run ipa-adtrust-install the IPA kdc will be able to create a MS-PAC if the user has the right attributes ipaNTSecurityIdentifier on the user entry and on the primary group entry are required (or a fallback primary group). If the objects are not in place the MS-PAC generation is silently skipped and no MS-PAC will be attached to the tickets. The MS-PAC is always generated if all data is available, in future we may think of making this conditional, but that is not in the scope of this patches. In order to apply these patches you need the coverity fix patches #2036 #2037 I sent yesterday. In order to build this code you need samba 4 experimental packages with the libndr_krb5pac.so librray, header files and pkgconfig configuration files. Please add these dependencies to the BuildRequires in the spec file. Otherwise the patch looks fine. Added BuildRequires: samba-4.0-devel, tested and pushed to master. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] LDAPS for the IPA LDAP server?
I noticed that the PKI Directory server has a secure port set but the IPA DS instance does not: PKI nsslapd-secureport: 7390 Why doesn IPA set up ldapson port 636? ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
