[Freeipa-devel] [PATCHES] Address #413 and Complete UUID related changes
These patches apply on top of the previous ipa_uuid related patches. #1 handles automatic generation of the uuid when the uuid attribute is the RDN (fixes #413). #2 prevents cases of false positives when enforcing is set and we are handling a simple modification of an object that falls into the plugin scope. #3 remove the python uuid plugin and changes all callers to always pass in the special value 'autogenerate' for the ipauniqueid attribute. This way uuids are generated server side. #3 introduces a problem with the baseldap class LDAPCreate, because that calss always tries to reuse the passed in DN to lookup the entry after creation. Unfortunately when ipaUniqueID is part of the DN, the DN is changed on add so the lookup using the special autogenerate value will fail. Pavel is looking into it to provide an alternative way to lookup the entry in these cases. Simo. -- Simo Sorce * Red Hat, Inc * New York From c6aa13c14280cc36fb3ad443b2f584d488d2fe53 Mon Sep 17 00:00:00 2001 From: Simo Sorce sso...@redhat.com Date: Tue, 26 Oct 2010 11:29:53 -0400 Subject: [PATCH 1/3] ipa_uuid: Handle generation of the uuid when it is a RDN --- daemons/ipa-slapi-plugins/ipa-uuid/ipa_uuid.c | 60 1 files changed, 50 insertions(+), 10 deletions(-) diff --git a/daemons/ipa-slapi-plugins/ipa-uuid/ipa_uuid.c b/daemons/ipa-slapi-plugins/ipa-uuid/ipa_uuid.c index eb5b40d..c0fde90 100644 --- a/daemons/ipa-slapi-plugins/ipa-uuid/ipa_uuid.c +++ b/daemons/ipa-slapi-plugins/ipa-uuid/ipa_uuid.c @@ -987,17 +987,13 @@ static int ipauuid_pre_op(Slapi_PBlock *pb, int modtype) bv = slapi_mod_get_first_value(smod); /* If we have a value, see if it's the magic value. */ if (bv) { -int len = strlen(cfgentry-generate); -if (len == bv-bv_len) { -if (!slapi_UTF8NCASECMP(bv-bv_val, -cfgentry-generate, -len)) { -generate = true; +if (!slapi_UTF8CASECMP(bv-bv_val, + cfgentry-generate)) { +generate = true; -/* also remove this mod, as we will add - * it again later */ -slapi_mod_remove_value(next_mod); -} +/* also remove this mod, as we will add + * it again later */ +slapi_mod_remove_value(next_mod); } } else { /* This is a replace with no new values, so we need @@ -1054,8 +1050,52 @@ static int ipauuid_pre_op(Slapi_PBlock *pb, int modtype) /* do the mod */ if (LDAP_CHANGETYPE_ADD == modtype) { +Slapi_DN *sdn; +Slapi_RDN *rdn; +char *attr; +char *nrdn; + /* add - set in entry */ slapi_entry_attr_set_charptr(e, cfgentry-attr, new_value); + +/* check to see if we need to change the RDN too */ +rdn = slapi_rdn_new(); +if (!rdn) { +LOG_OOM(); +ret = LDAP_OPERATIONS_ERROR; +goto done; +} +sdn = slapi_sdn_new_dn_byval(dn); +if (!sdn) { +LOG_OOM(); +ret = LDAP_OPERATIONS_ERROR; +slapi_rdn_free(rdn); +goto done; +} +slapi_rdn_set_sdn(rdn, sdn); +ret = slapi_rdn_contains_attr(rdn, cfgentry-attr, attr); +slapi_rdn_done(rdn); +if (ret == 1) { +/* no need to recheck if it is valid, it will be handled + * later by checking the value in the entry */ +nrdn = slapi_ch_smprintf(%s=%s, + cfgentry-attr, new_value); +if (!nrdn) { +LOG_OOM(); +ret = LDAP_OPERATIONS_ERROR; +slapi_rdn_free(rdn); +slapi_sdn_free(sdn); +goto done; +} + +slapi_rdn_set_dn(rdn, nrdn); +slapi_ch_free_string(nrdn); +slapi_sdn_set_rdn(sdn, rdn); +slapi_entry_set_sdn(e, sdn); +} +slapi_rdn_free(rdn); +slapi_sdn_free(sdn); + } else { /* mod - add to mods */ slapi_mods_add_string(smods, LDAP_MOD_REPLACE, -- 1.7.2.3 From
Re: [Freeipa-devel] Proposed standard for Patches: RFC
On 10/26/2010 04:47 PM, Simo Sorce wrote: On Tue, 26 Oct 2010 16:26:13 -0400 Adam Youngayo...@redhat.com wrote: I'll admit this would be useful, but it would be another process that we don't have now, that I was trying to avoid. We all have git repos on fedorapeople. The trick is to deal with patches that have to get changed prior to commit, hence the numbering of -2 -3 after the seq number. Not sure what's the problem here, if I rebase a patch you have the latest one in my tree, no need to look for -1 -2 -3 as you can't be wrong if you re-fetch from my tree. Yes, and if we go with a Git based appraoch, that would be fine. But the team seems to havea preference and a system in place that works with straight patches. I am not trying to change that. If you are set on a Git based approach, it is a more significant change from our current process, something that I would not advocate this close to a major release. I'm just trying to codify what Rob, Endi and I are already doing, and which seems to work well. Really, the seq number is not needed, but makes a nice ready shorthand for the patch. Pavel, Endi and I often refer to patches by number, like your patch 0019 which makes it handy. The increasing seq approach to detect a missing packet works in TCP, so why not for us? Because I am not a machine :) I disagree. So does schoolhouse rock: http://www.youtube.com/watch?v=Kdn0pPcTvN4 I see we constantly fail at correctly numbering sequentially stuff, from new error numbers to OIDs and other stuff, so I do not see this as a big improvement. I am not saying people shouldn't use this method if so they prefer, but mandating it seems a bit too much. It seems to be easy enough to do. Of course if others strongly feel this is the way to go, I will (try to) comply. Let's give it a go for a few. Simo. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] RFC wrt little snag in LDAPCreate when ipa_uuid manipulates the DN on entry add
On 10/26/2010 11:21 PM, Simo Sorce wrote: So, I have been working on this ipa_uuid plugin as of late and one of the last tasks was to let it modify the RDN if ipaUniqueID is part of the DN of an entry we want to create. Example: dn: ipauniqueid=autogenerate,cn=hbac,dc=... cn: foo rule hbactype: allow ... 'autogenerate' is the magic value that makes the ipa_uuid plugin generate a uuid and set it on the entry. The problem is that LDAPCreate, after adding the entry will try to search it back immediately using the DN we passed in. This search will fail as the DN that is stored in LDAP is different (it has the generated uuid instead of 'autogenerate'). So ideas on how to gracefully handle this are welcome. I discussed of 2 with Rob on IRC but we'd like more inputs (Pavel, that would be directed at you :-). 1. Ignore the error in calls that pass in a DN containing ipauniqueid as the RDN and perform a new search. 2. modify LDAPCreate so that we can pass in a filter. If the caller passes in a filter we use that instead of the DN to search the entry back. Simo. I'm not up to speed on this code. Why do a find right after create? ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] RFC wrt little snag in LDAPCreate when ipa_uuid manipulates the DN on entry add
Adam Young wrote: On 10/26/2010 11:21 PM, Simo Sorce wrote: So, I have been working on this ipa_uuid plugin as of late and one of the last tasks was to let it modify the RDN if ipaUniqueID is part of the DN of an entry we want to create. Example: dn: ipauniqueid=autogenerate,cn=hbac,dc=... cn: foo rule hbactype: allow ... 'autogenerate' is the magic value that makes the ipa_uuid plugin generate a uuid and set it on the entry. The problem is that LDAPCreate, after adding the entry will try to search it back immediately using the DN we passed in. This search will fail as the DN that is stored in LDAP is different (it has the generated uuid instead of 'autogenerate'). So ideas on how to gracefully handle this are welcome. I discussed of 2 with Rob on IRC but we'd like more inputs (Pavel, that would be directed at you :-). 1. Ignore the error in calls that pass in a DN containing ipauniqueid as the RDN and perform a new search. 2. modify LDAPCreate so that we can pass in a filter. If the caller passes in a filter we use that instead of the DN to search the entry back. Simo. I'm not up to speed on this code. Why do a find right after create? Normally an add works like this. * Use the get_dn() class method to create the dn based on the primary key and the container. * call add_entry() * do a get_entry() to retrieve the data we just added to we can show the user what we did. In the case of HBAC and netgroups the dn contains the attribute ipaUniqueId which is something we want to autogenerate. So the dn we pass the add function isn't going to match the dn that gets written to the database. The get_entry() is failing because we are trying to read dn: ipauniqueid=autogenerate, ... and what got written was dn: ipauniqueid=1092a93-9as9d-f... So we need some way of finding the entry we just wrote, whose uniqueid we don't know (but we know other stuff about it). rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Proposed standard for Patches: RFC
Made a change based on a recommendation by Simo: proejct name now leads, followed by user name. Posted on the wiki here: http://fedorahosted.org/freeipa/wiki/PatchFormat ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] RFC wrt little snag in LDAPCreate when ipa_uuid manipulates the DN on entry add
Simo Sorce wrote: On Wed, 27 Oct 2010 09:35:17 -0400 Adam Youngayo...@redhat.com wrote: I'm not up to speed on this code. Why do a find right after create? I guess to pick up all attributes added automatically by DS, not sure why it just is. Simo. Yes, that's exactly it. We have other autogenerated values (uid, gid) so we fetch the entry to be sure we are representing things as they are. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] freeipa-admiyo-freeipa-0069-Field-Errors.patch
Field Errors Uses the pattern field of the metat data to see if the input for a given field is valid. If not, displays a red box with the contents of pattern_msg To test this, I artificially modified the meta data for the Group description From 12a16f24b7a75f4621e077bcd3a61976a8e624c9 Mon Sep 17 00:00:00 2001 From: Adam Young ayo...@redhat.com Date: Wed, 27 Oct 2010 13:41:48 -0400 Subject: [PATCH] Field Errors Uses the pattern field of the metat data to see if the input for a given field is valid. If not, displays a red box with the contents of pattern_msg To test this, I artificially modified the meta data for the Group description field --- install/static/details.js | 29 +- install/static/test/data/json_metadata.json |6 ++-- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/install/static/details.js b/install/static/details.js index 312ad95..d0688f5 100644 --- a/install/static/details.js +++ b/install/static/details.js @@ -123,6 +123,7 @@ function ipa_stanza(spec){ var value = $.trim(input.val()); if (!value) value = ''; + values.push(value); }); @@ -480,20 +481,34 @@ function _ipa_create_text_input(attr, value, param_info) return index; } +function validate_input(text, param_info,error_link){ +if(param_info param_info.pattern){ +var regex = new RegExp( param_info.pattern ); +if (!text.match(regex)) { +error_link.style.display =block; +if ( param_info.pattern_errmsg){ +error_link.innerHTML = param_info.pattern_errmsg; +} +}else{ +error_link.style.display =none; +} +} +} + var input = $(Span /); input.append($(input/,{ type:text, name:attr, value:value.toString(), -keypress: function(){ -var validation_info=param_info; +keyup: function(){ var undo_link=this.nextElementSibling; undo_link.style.display =inline; -if(false){ -var error_link = undo_link.nextElementSibling; -error_link.style.display =block; -} +var error_link = undo_link.nextElementSibling; + +var text = $(this).val(); +validate_input(text, param_info,error_link); } + })); input.append($(a/,{ html:undo, @@ -515,6 +530,8 @@ function _ipa_create_text_input(attr, value, param_info) this.previousElementSibling.value = previous_value; this.style.display = none; +var error_link = this.nextElementSibling; +validate_input(previous_value, param_info,error_link); } })); input.append($(span/,{ diff --git a/install/static/test/data/json_metadata.json b/install/static/test/data/json_metadata.json index 8571ddf..a005d1d 100644 --- a/install/static/test/data/json_metadata.json +++ b/install/static/test/data/json_metadata.json @@ -1098,8 +1098,8 @@ minlength: null, multivalue: false, name: description, -pattern: null, -pattern_errmsg: null, +pattern: ^[a-zA-Z-2-9_. ]{0,30}[a-zA-Z0-9_.$-]?$, +pattern_errmsg: may only include letters, numbers, _, -, . and $, primary_key: false, query: false, required: true, @@ -4474,4 +4474,4 @@ } } } -} \ No newline at end of file +} -- 1.7.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] RFC wrt little snag in LDAPCreate when ipa_uuid manipulates the DN on entry add
Rob Crittenden wrote: Simo Sorce wrote: On Wed, 27 Oct 2010 09:35:17 -0400 Adam Youngayo...@redhat.com wrote: I'm not up to speed on this code. Why do a find right after create? I guess to pick up all attributes added automatically by DS, not sure why it just is. Simo. Yes, that's exactly it. We have other autogenerated values (uid, gid) so we fetch the entry to be sure we are representing things as they are. One enhancement we have discussed adding to 389 is a control sent with update operations - the control response would contain the values of generated attributes, to remove the need to immediately perform a search to get attributes such as uniqueid, uid, gid, createTimestamp, etc. Is this something IPA would be interested in? There has already been some discussion (a long time ago) on the 389 lists. afaik there is no LDAP proposed standard feature for this. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] #412 Make always use of special salt type
By using the special salt type and generating a random salt we can rename user's principal name without invalidating their password. This works only if pre-authentication is required, but that's how we configure our server anyway. This patch does not disallow normal salts, but if used they will prevent renames from working correctly. By default special is used. Simo. -- Simo Sorce * Red Hat, Inc * New York From 2accddf2bb85ea41e73c2ff48f4c39fc4c6b5e90 Mon Sep 17 00:00:00 2001 From: Simo Sorce sso...@redhat.com Date: Wed, 27 Oct 2010 15:05:56 -0400 Subject: [PATCH] pwd-plugin: Always use a special salt by default. This should make renamed users able to keep using old credentials as the salt is not derived from the principal name but is always a random quantity. https://fedorahosted.org/freeipa/ticket/412 --- .../ipa-pwd-extop/ipapwd_encoding.c| 50 +--- install/share/default-keytypes.ldif| 14 +++-- 2 files changed, 30 insertions(+), 34 deletions(-) diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_encoding.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_encoding.c index 462622a518c60574d15399e025d60655ca21c2f0..527e178fe01b9f775fb0522f81eeedfcbde0ee50 100644 --- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_encoding.c +++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_encoding.c @@ -350,35 +350,29 @@ static Slapi_Value **encrypt_encode_key(struct ipapwd_krbcfg *krbcfg, case KRB5_KDB_SALTTYPE_NORMAL: -/* If pre auth is required we can set a random salt, otherwise - * we have to use a more conservative approach and set the salt - * to be REALMprincipal (the concatenation of REALM and principal - * name without any separator) */ -#if 0 -if (krbTicketFlags KTF_REQUIRES_PRE_AUTH) { -salt.length = KRB5P_SALT_SIZE; -salt.data = malloc(KRB5P_SALT_SIZE); -if (!salt.data) { -LOG_OOM(); -goto enc_error; -} -krberr = krb5_c_random_make_octets(krbctx, salt); -if (krberr) { -LOG_FATAL(krb5_c_random_make_octets failed [%s]\n, - krb5_get_error_message(krbctx, krberr)); -goto enc_error; -} -} else { -#endif -krberr = krb5_principal2salt(krbctx, princ, salt); -if (krberr) { -LOG_FATAL(krb5_principal2salt failed [%s]\n, - krb5_get_error_message(krbctx, krberr)); -goto enc_error; -} -#if 0 +krberr = krb5_principal2salt(krbctx, princ, salt); +if (krberr) { +LOG_FATAL(krb5_principal2salt failed [%s]\n, + krb5_get_error_message(krbctx, krberr)); +goto enc_error; +} +break; + +case KRB5_KDB_SALTTYPE_SPECIAL: + +/* make random salt */ +salt.length = KRB5P_SALT_SIZE; +salt.data = malloc(KRB5P_SALT_SIZE); +if (!salt.data) { +LOG_OOM(); +goto enc_error; +} +krberr = krb5_c_random_make_octets(krbctx, salt); +if (krberr) { +LOG_FATAL(krb5_c_random_make_octets failed [%s]\n, + krb5_get_error_message(krbctx, krberr)); +goto enc_error; } -#endif break; case KRB5_KDB_SALTTYPE_V4: diff --git a/install/share/default-keytypes.ldif b/install/share/default-keytypes.ldif index 8561b98dcc26bfc71ee6455dec391ee023b10fe7..8093b6989851ad632e4e5954496f5ae8cde10ddd 100644 --- a/install/share/default-keytypes.ldif +++ b/install/share/default-keytypes.ldif @@ -3,9 +3,13 @@ dn: cn=$REALM,cn=kerberos,$SUFFIX changetype: modify add: krbSupportedEncSaltTypes krbSupportedEncSaltTypes: aes256-cts:normal +krbSupportedEncSaltTypes: aes256-cts:special krbSupportedEncSaltTypes: aes128-cts:normal +krbSupportedEncSaltTypes: aes128-cts:special krbSupportedEncSaltTypes: des3-hmac-sha1:normal +krbSupportedEncSaltTypes: des3-hmac-sha1:special krbSupportedEncSaltTypes: arcfour-hmac:normal +krbSupportedEncSaltTypes: arcfour-hmac:special krbSupportedEncSaltTypes: des-hmac-sha1:normal krbSupportedEncSaltTypes: des-cbc-md5:normal krbSupportedEncSaltTypes: des-cbc-crc:normal @@ -22,10 +26,8 @@ krbMaxRenewableAge: 604800 dn: cn=$REALM,cn=kerberos,$SUFFIX changetype: modify add: krbDefaultEncSaltTypes -krbDefaultEncSaltTypes: aes256-cts:normal -krbDefaultEncSaltTypes: aes128-cts:normal -krbDefaultEncSaltTypes: des3-hmac-sha1:normal -krbDefaultEncSaltTypes: arcfour-hmac:normal -krbDefaultEncSaltTypes: des-hmac-sha1:normal -krbDefaultEncSaltTypes: des-cbc-md5:normal +krbDefaultEncSaltTypes: aes256-cts:special +krbDefaultEncSaltTypes: aes128-cts:special
Re: [Freeipa-devel] RFC wrt little snag in LDAPCreate when ipa_uuid manipulates the DN on entry add
On Wed, 27 Oct 2010 14:52:17 -0600 Rich Megginson rmegg...@redhat.com wrote: Rob Crittenden wrote: Simo Sorce wrote: On Wed, 27 Oct 2010 09:35:17 -0400 Adam Youngayo...@redhat.com wrote: I'm not up to speed on this code. Why do a find right after create? I guess to pick up all attributes added automatically by DS, not sure why it just is. Simo. Yes, that's exactly it. We have other autogenerated values (uid, gid) so we fetch the entry to be sure we are representing things as they are. One enhancement we have discussed adding to 389 is a control sent with update operations - the control response would contain the values of generated attributes, to remove the need to immediately perform a search to get attributes such as uniqueid, uid, gid, createTimestamp, etc. Is this something IPA would be interested in? There has already been some discussion (a long time ago) on the 389 lists. afaik there is no LDAP proposed standard feature for this. Looks like an interesting thing. It would also help esp. in the case we change the DN under users noses. But the patch Pavel sent seem to deal well with the current contingency. Still I would mark it as a nice to have. 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] [PATCH] #333 plugin to change kerberos principal name when user is renamed
Simo Sorce wrote: This plugin intercepts a modrdn change so that when a user is renamed the krbprincipalname is changhed accordingly. The second patch activates the plugin. Simo. ack x2 rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] UUID Plugin: Code fixes and cleanups
Simo Sorce wrote: These are a few minor fixes and cleanups I split in multiple patches for easier review. 1. makes sure we reset the generate flag at every loop, so that we do not risk a false positive if multiple UUIDs are generated on an entry. 2. makes unlocks safer by tracking when we need to unlock and doing so in the cleanup code. This is necessary as later code will be introduced that may error out in the middle of the main loop. 3. tidy up some code and remove one nesting level (hopefully making stuff slightly more readable). This is possible thanks to (2). Simo. ack x3 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] UUID Plugin: add enforce option
Simo Sorce wrote: When the ipaUuidEnforce option is set to TRUE only the Directory Manager is allowed to set arbitrary values. Any attempt to set values != the ipaUuidGenerate value by non DirMgr users will throw an error. This is useful to enforce UUIDs are always set by the server. At this moment normal users are still allowed to modify the value so that the uuid is regenerated (and therefore changed, although not with arbitrary values). If modifications are unwanted I guess we can easily add an ACI that allow someone to add the attribute but mot modify it afterwards. Currently the install code does not yet set the plugin into enforcing mode as that would break all ipa tools, tomorrow I plan to go through the framework code and rip off the uuid stuff and finally change the default to enforcing for the ipaUniqueID attribute once all client code is converted to always set only 0 on creation. Simo. Ack. I think we still have some acis controlling access to ipauniqueid. I think we can remove them and save a few cycles in the DS aci subsystem. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] Address #413 and Complete UUID related changes
Simo Sorce wrote: These patches apply on top of the previous ipa_uuid related patches. #1 handles automatic generation of the uuid when the uuid attribute is the RDN (fixes #413). #2 prevents cases of false positives when enforcing is set and we are handling a simple modification of an object that falls into the plugin scope. #3 remove the python uuid plugin and changes all callers to always pass in the special value 'autogenerate' for the ipauniqueid attribute. This way uuids are generated server side. #3 introduces a problem with the baseldap class LDAPCreate, because that calss always tries to reuse the passed in DN to lookup the entry after creation. Unfortunately when ipaUniqueID is part of the DN, the DN is changed on add so the lookup using the special autogenerate value will fail. Pavel is looking into it to provide an alternative way to lookup the entry in these cases. Simo. There is one minor problem in the 3rd patch. The admin user has the wrong magic value for ipauniqueid. Fix that and you have a pre-ack x3. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel