Re: [Freeipa-devel] [PATCH] 993 disable UPG for migration
On Mon, 2012-04-02 at 15:18 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Fri, 2012-03-30 at 09:05 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2012-03-29 at 11:27 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Wed, 2012-03-28 at 17:28 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2012-03-22 at 15:21 -0400, Rob Crittenden wrote: We don't want to create private groups automatically for migrated users, there could be namespace overlap (and race conditions prevent us from trying to check in advance). Check the sanity of groups in general, warn if the group for the gidnumber doesn't exist at least on the remote server. Fill in the user's shell as well. This will migrate users that are non-POSIX on the remote server. rob This patch fixes the problem of creating UPGs for migrated users, but there are several parts which confused me. 1) Confusing defaults +if 'def_group_gid' in ctx: +entry_attrs.setdefault('gidnumber', ctx['def_group_gid']) This statement seems redundant, because the account either is posix and has both uidnumber and gidnumber or it is non-posix and does not have any. Now, we don't touch gidNumber for posix user, and non-posix users are made posix with this statement: +# migrated user is not already POSIX, make it so +if 'uidnumber' not in entry_attrs: +entry_attrs['uidnumber'] = entry_attrs['gidnumber'] = [999] 2) Missing UPG When UPG is disabled, the following statement will result in a user with a GID pointing to non-existent group. +# migrated user is not already POSIX, make it so +if 'uidnumber' not in entry_attrs: +entry_attrs['uidnumber'] = entry_attrs['gidnumber'] = [999] We may want to run ldap.has_upg() and report a add this user to failed users with appropriate error. 3) Check for GID The patch implements a check if a group with the gidNumber exists on a remote LDAP server and the result is a warning: -(g_dn, g_attrs) = ldap.get_entry(ctx['def_group_dn'], ['gidnumber']) +(remote_dn, remote_entry) = ds_ldap.find_entry_by_attr( +'gidnumber', entry_attrs['gidnumber'][0], 'posixgroup', [''], + search_bases['group'] +) I have few (minor-ish) questions there: a) Is the warning in a apache log enough? Maybe it should be included in migrate-ds output. b) This will be a one more remote LDAP call for every user, we may want to optimize it with something like that: valid_gids = [] if user.gidnumber not in valid_gids: run the check in remote LDAP valid_gids.append(user.gidnumber) That would save us 999 LDAP queries for 1000 migrated with the same primary group. 4) Extraneous Warning: When non-posix user is migrated and thus we make it a posix user, we still produce a warning for non-existent group: [Fri Mar 23 04:21:36 2012] [error] ipa: WARNING: Migrated user's GID number 999 does not point to a known group. 5) Extraneous LDAP call Isn't the following call to LDAP to get a description redundant? We already have the description in entry_attrs. +(dn, desc_attr) = ldap.get_entry(dn, ['description']) +entry_attrs.update(desc_attr) +if 'description' in entry_attrs and NO_UPG_MAGIC in entry_attrs['description']: Martin I think this covers your concerns. I can't do anything but log warnings at this point in order to maintain backwards compatibility. I looked into returning a warning entry and it blew up in validate_output() on older clients. rob This patch is much better and covers my previous concerns. I just find an issue with UPG. It is not created for non-posix users when UPGs are enabled: # echo Secret123 | ipa migrate-ds ldap://ldap.example.com --with-compat --base-dn=dc=greyoak,dc=com --- migrate-ds: --- Migrated: user: darcee_leeson, ayaz_kreiger, mnonposix, mollee_weisenberg group: ipagroup Failed user: Failed group: -- Passwords have been migrated in pre-hashed format. IPA is unable to generate Kerberos keys unless provided with clear text passwords. All migrated users need to login at https://your.domain/ipa/migration/ before they can use their Kerberos accounts. # ipa user-show mnonposix User login: mnonposix First name: Mister Last name: Nonposix Home directory: /home/mnonposix Login shell: /bin/sh UID: 328000195 GID: 328000195 Org. Unit: Product Testing Job Title: Test User Account disabled: False Password: True Member of groups: ipausers Kerberos keys available: False # ipa group-show mnonposix ipa: ERROR: mnonposix: group not found Yes, I was always disabling UPG. I now allow it when migrating a non-POSIX
Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal
On Mon, 2012-04-02 at 15:36 -0400, Rob Crittenden wrote: Rob Crittenden wrote: Martin Kosek wrote: On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote: Certmonger will currently automatically renew server certificates but doesn't restart the services so you can still end up with expired certificates if you services never restart. This patch registers are restart command with certmonger so the IPA services will automatically be restarted to get the updated cert. Easy to test. Install IPA then resubmit the current server certs and watch the services restart: # ipa-getcert list Find the ID for either your dirsrv or httpd instance # ipa-getcert resubmit -iID Watch /var/log/httpd/error_log or /var/log/dirsrv/slapd-INSTANCE/errors to see the service restart. rob What about current instances - can we/do we want to update certmonger tracking so that their instances are restarted as well? Anyway, I found few issues SELinux issues with the patch: 1) # rpm -Uvh freeipa-* Preparing... ### [100%] 1:freeipa-python ### [ 20%] 2:freeipa-client ### [ 40%] 3:freeipa-admintools ### [ 60%] 4:freeipa-server ### [ 80%] /usr/bin/chcon: failed to change context of `/usr/lib64/ipa/certmonger' to `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument /usr/bin/chcon: failed to change context of `/usr/lib64/ipa/certmonger/restart_dirsrv' to `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument /usr/bin/chcon: failed to change context of `/usr/lib64/ipa/certmonger/restart_httpd' to `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64) scriptlet failed, exit status 1 5:freeipa-server-selinux ### [100%] certmonger_unconfined_exec_t type was unknown with my selinux policy: selinux-policy-3.10.0-80.fc16.noarch selinux-policy-targeted-3.10.0-80.fc16.noarch If we need a higher SELinux version, we should bump the required package version spec file. Yeah, waiting on it to be backported. 2) Change of SELinux context with /usr/bin/chcon is temporary until restorecon or system relabel occurs. I think we should make it persistent and enforce this type in our SELinux policy and rather call restorecon instead of chcon That's a good idea, why didn't I think of that :-( Ah, now I remember, it will be handled by selinux-policy. I would have used restorecon here but since the policy isn't there yet this seemed like a good idea. I'm trying to find out the status of this new policy, it may only make it into F-17. rob Ok. But if this policy does not go in F-16 and if we want this fix in F16 release too, I guess we would have to implement both approaches in our spec file: 1) When on F16, include SELinux policy for restart scripts + run restorecon 2) When on F17, do not include the SELinux policy (+ run restorecon) Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 246 Configure SELinux for httpd during upgrades
SELinux configuration for httpd instance was set for new installations only. Upgraded IPA servers (namely 2.1.x - 2.2.x upgrade) missed the configuration. This lead to AVCs when httpd tries to contact ipa_memcached and user not being able to log in. This patch updates ipa-upgradeconfig to configure SELinux in the same way as ipa-server-install does. https://fedorahosted.org/freeipa/ticket/2603 From 846eb1d6153e9a5f97e25dbb75b858eed1a17b77 Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Tue, 3 Apr 2012 10:47:40 +0200 Subject: [PATCH] Configure SELinux for httpd during upgrades SELinux configuration for httpd instance was set for new installations only. Upgraded IPA servers (namely 2.1.x - 2.2.x upgrade) missed the configuration. This lead to AVCs when httpd tries to contact ipa_memcached and user not being able to log in. This patch updates ipa-upgradeconfig to configure SELinux in the same way as ipa-server-install does. https://fedorahosted.org/freeipa/ticket/2603 --- install/tools/ipa-upgradeconfig | 24 ipaserver/install/httpinstance.py |4 ++-- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig index 40a2b68ce89b58b98077428783a98e3060674665..a2a30249923ed127d2d68d312ad7abeb04627678 100644 --- a/install/tools/ipa-upgradeconfig +++ b/install/tools/ipa-upgradeconfig @@ -163,7 +163,7 @@ def check_certs(): print Missing Certification Authority file. print You should place a copy of the CA certificate in /usr/share/ipa/html/ca.crt -def upgrade_pki(): +def upgrade_pki(fstore): Update/add the dogtag proxy configuration. The IPA side of this is handled in ipa-pki-proxy.conf. @@ -173,7 +173,6 @@ def upgrade_pki(): if not os.path.exists('/etc/pki-ca/CS.cfg'): return -fstore = sysrestore.FileStore('/var/lib/ipa/sysrestore') http = httpinstance.HTTPInstance(fstore) http.enable_mod_nss_renegotiate() if not installutils.get_directive('/etc/pki-ca/CS.cfg', @@ -222,13 +221,11 @@ def update_dbmodules(realm, filename=/etc/krb5.conf): fd.write(.join(newfile)) fd.close() -def cleanup_kdc(): +def cleanup_kdc(fstore): Clean up old KDC files if they exist. We need to remove the actual file and any references in the uninstall configuration. -fstore = sysrestore.FileStore('/var/lib/ipa/sysrestore') - for file in ['kpasswd.keytab', 'ldappwd']: filename = '/var/kerberos/krb5kdc/%s' % file installutils.remove_file(filename) @@ -244,6 +241,14 @@ def upgrade_ipa_profile(realm): if ca.enable_subject_key_identifier(): ca.restart() +def upgrade_httpd_selinux(fstore): + +Update SElinux configuration for httpd instance in the same way as the +new server installation does. + +http = httpinstance.HTTPInstance(fstore) +http.configure_selinux_for_httpd() + def main(): Get some basics about the system. If getting those basics fail then @@ -254,6 +259,8 @@ def main(): if not os.geteuid()==0: sys.exit(\nYou must be root to run this script.\n) +fstore = sysrestore.FileStore('/var/lib/ipa/sysrestore') + try: krbctx = krbV.default_context() except krbV.Krb5Error, e: @@ -274,12 +281,13 @@ def main(): upgrade(sub_dict, /etc/httpd/conf.d/ipa.conf, ipautil.SHARE_DIR + ipa.conf) upgrade(sub_dict, /etc/httpd/conf.d/ipa-rewrite.conf, ipautil.SHARE_DIR + ipa-rewrite.conf) upgrade(sub_dict, /etc/httpd/conf.d/ipa-pki-proxy.conf, ipautil.SHARE_DIR + ipa-pki-proxy.conf, add=True) -upgrade_pki() +upgrade_pki(fstore) update_dbmodules(krbctx.default_realm) uninstall_ipa_kpasswd() -http = httpinstance.HTTPInstance() +http = httpinstance.HTTPInstance(fstore) http.remove_httpd_ccache() +http.configure_selinux_for_httpd() memcache = memcacheinstance.MemcacheInstance() memcache.ldapi = True @@ -294,7 +302,7 @@ def main(): except (ldap.ALREADY_EXISTS, ipalib.errors.DuplicateEntry): pass -cleanup_kdc() +cleanup_kdc(fstore) upgrade_ipa_profile(krbctx.default_realm) try: diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py index e46d4ed5a19fb93398acf3c39cdefeafbac3ea9c..0a09c26f2d16af62b66bc5b9c24851f2cfd46158 100644 --- a/ipaserver/install/httpinstance.py +++ b/ipaserver/install/httpinstance.py @@ -86,7 +86,7 @@ class HTTPInstance(service.Service): self.step(publish CA cert, self.__publish_ca_cert) self.step(creating a keytab for httpd, self.__create_http_keytab) self.step(clean up any existing httpd ccache, self.remove_httpd_ccache) -self.step(configuring SELinux for httpd, self.__selinux_config) +self.step(configuring SELinux for httpd, self.configure_selinux_for_httpd) self.step(restarting httpd, self.__start)
[Freeipa-devel] [PATCH] 73 Check whether the default user group is POSIX when adding new user with --noprivate
https://fedorahosted.org/freeipa/ticket/2572 Honza -- Jan Cholasta From 2fbfab66064d045c192d2cc8d747d30bca1ebdc6 Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Thu, 29 Mar 2012 09:12:36 -0400 Subject: [PATCH] Check whether the default user group is POSIX when adding new user with --noprivate. ticket 2572 --- ipalib/plugins/user.py | 30 ++ 1 files changed, 14 insertions(+), 16 deletions(-) diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py index 64424e8..a552960 100644 --- a/ipalib/plugins/user.py +++ b/ipalib/plugins/user.py @@ -455,22 +455,20 @@ class user_add(LDAPCreate): entry_attrs.setdefault('krbpwdpolicyreference', 'cn=global_policy,cn=%s,cn=kerberos,%s' % (api.env.realm, api.env.basedn)) entry_attrs.setdefault('krbprincipalname', '%s@%s' % (entry_attrs['uid'], api.env.realm)) -if 'gidnumber' not in entry_attrs: -# gidNumber wasn't specified explicity, find out what it should be -if not options.get('noprivate', False) and ldap.has_upg(): -# User Private Groups - uidNumber == gidNumber -entry_attrs['gidnumber'] = entry_attrs['uidnumber'] -else: -# we're adding new users to a default group, get its gidNumber -# get default group name from config -def_primary_group = config.get('ipadefaultprimarygroup') -group_dn = self.api.Object['group'].get_dn(def_primary_group) -try: -(group_dn, group_attrs) = ldap.get_entry(group_dn, ['gidnumber']) -except errors.NotFound: -error_msg = 'Default group for new users not found.' -raise errors.NotFound(reason=error_msg) -entry_attrs['gidnumber'] = group_attrs['gidnumber'] +if options.get('noprivate', False) or not ldap.has_upg(): +# we're adding new users to a default group, get its gidNumber +# get default group name from config +def_primary_group = config.get('ipadefaultprimarygroup') +group_dn = self.api.Object['group'].get_dn(def_primary_group) +try: +(group_dn, group_attrs) = ldap.get_entry(group_dn, ['gidnumber']) +except errors.NotFound: +error_msg = 'Default group for new users not found.' +raise errors.NotFound(reason=error_msg) +if 'gidnumber' not in group_attrs: +error_msg = 'Default group for new users is not POSIX.' +raise errors.NotFound(reason=error_msg) +entry_attrs['gidnumber'] = group_attrs['gidnumber'] if 'userpassword' not in entry_attrs and options.get('random'): entry_attrs['userpassword'] = ipa_generate_password(user_pwdchars) -- 1.7.7.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 20 Fix empty external member processing
https://fedorahosted.org/freeipa/ticket/2447 Validation of external member was failing for empty strings because of wrong condition. -- Regards, Ondrej Hamada FreeIPA team jabber: oh...@jabbim.cz IRC: ohamada From 137c676c6c182f839cbcd9332f9d0f6d8d18b3f0 Mon Sep 17 00:00:00 2001 From: Ondrej Hamada oham...@redhat.com Date: Tue, 3 Apr 2012 12:07:04 +0200 Subject: [PATCH] Fix empty external member processing Validation of external member was failing for empty strings because of wrong condition. https://fedorahosted.org/freeipa/ticket/2447 --- ipalib/plugins/baseldap.py |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py index 38f369a779adc53454837994bd2bec5b74d3bbd4..1c893018c6452b5979c2c721e325005cb0d676a9 100644 --- a/ipalib/plugins/baseldap.py +++ b/ipalib/plugins/baseldap.py @@ -326,7 +326,7 @@ def add_external_pre_callback(membertype, ldap, dn, keys, options): def validate_host(hostname): validate_hostname(hostname, check_fqdn=False, allow_underscore=True) -if membertype in options: +if membertype in options and options[membertype]: if membertype == 'host': validator = validate_host else: -- 1.7.6.5 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 73 Check whether the default user group is POSIX when adding new user with --noprivate
On Tue, 2012-04-03 at 13:02 +0200, Martin Kosek wrote: On Tue, 2012-04-03 at 11:58 +0200, Jan Cholasta wrote: https://fedorahosted.org/freeipa/ticket/2572 Honza NACK. This creates a regression: # ipa group-show foogroup Group name: foogroup Description: foo GID: 358800017 # ipa user-add --first=Foo --last=Bar fbar5 --gidnumber=358800017 --noprivate -- Added user fbar5 -- User login: fbar5 First name: Foo Last name: Bar Full name: Foo Bar Display name: Foo Bar Initials: FB Home directory: /home/fbar5 GECOS field: Foo Bar Login shell: /bin/sh Kerberos principal: fb...@idm.lab.bos.redhat.com UID: 358800019 GID: 358800012 Password: False Member of groups: ipausers Kerberos keys available: False # id fbar5 uid=358800019(fbar5) gid=358800012(ipausers) groups=358800012(ipausers) Custom user group (GID) was overwritten. I think we also want a test case for this situation. Martin ... and we also want to have the new error message(s) i18n-able. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0015 Don't try to remove auxiliary nodes from internal RBT
Hello, this patch optimizes code for removing deleted zones from BIND instance little bit. In some cases there are auxiliary zones (= not really served zones) in internal Red-Black tree. Current code tries to remove these auxiliary zones on each zone_refresh attempt. Everything works fine, because auxiliary zones are detected deeper in zone deletion code. Now plugin prints very confusing message Zone '%s' has been removed from database. each 'zone_refresh' seconds, again and again. This patch prevents this. I think it's very very confusing. I spent a lot of time while debugging before I realized where is the problem. Petr^2 Spacek From ce620e1e4bb888d784b8cdfac5ba75182d45b6c3 Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Tue, 3 Apr 2012 14:50:12 +0200 Subject: [PATCH] Don't try to remove auxiliary nodes from internal RBT Signed-off-by: Petr Spacek pspa...@redhat.com --- src/ldap_helper.c | 10 -- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ldap_helper.c b/src/ldap_helper.c index d0cde9d..df8b01e 100644 --- a/src/ldap_helper.c +++ b/src/ldap_helper.c @@ -1192,6 +1192,14 @@ refresh_zones_from_ldap(ldap_instance_t *ldap_inst) goto next; } + /* Do not remove auxilitary (= non-zone) nodes. */ + char buf[DNS_NAME_FORMATSIZE]; + dns_name_format(aname, buf, DNS_NAME_FORMATSIZE); + if (!node-data) { + log_debug(5,auxilitary zone/node '%s' will not be removed, buf); + goto next; + } + DECLARE_BUFFERED_NAME(foundname); INIT_BUFFERED_NAME(foundname); @@ -1201,8 +1209,6 @@ refresh_zones_from_ldap(ldap_instance_t *ldap_inst) goto next; } /* Log zone removing. */ - char buf[255]; - dns_name_format(aname, buf, 255); log_debug(1, Zone '%s' has been removed from database., buf); delete = ISC_TRUE; -- 1.7.7.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal
Martin Kosek wrote: On Mon, 2012-04-02 at 15:36 -0400, Rob Crittenden wrote: Rob Crittenden wrote: Martin Kosek wrote: On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote: Certmonger will currently automatically renew server certificates but doesn't restart the services so you can still end up with expired certificates if you services never restart. This patch registers are restart command with certmonger so the IPA services will automatically be restarted to get the updated cert. Easy to test. Install IPA then resubmit the current server certs and watch the services restart: # ipa-getcert list Find the ID for either your dirsrv or httpd instance # ipa-getcert resubmit -iID Watch /var/log/httpd/error_log or /var/log/dirsrv/slapd-INSTANCE/errors to see the service restart. rob What about current instances - can we/do we want to update certmonger tracking so that their instances are restarted as well? Anyway, I found few issues SELinux issues with the patch: 1) # rpm -Uvh freeipa-* Preparing... ### [100%] 1:freeipa-python ### [ 20%] 2:freeipa-client ### [ 40%] 3:freeipa-admintools ### [ 60%] 4:freeipa-server ### [ 80%] /usr/bin/chcon: failed to change context of `/usr/lib64/ipa/certmonger' to `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument /usr/bin/chcon: failed to change context of `/usr/lib64/ipa/certmonger/restart_dirsrv' to `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument /usr/bin/chcon: failed to change context of `/usr/lib64/ipa/certmonger/restart_httpd' to `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64) scriptlet failed, exit status 1 5:freeipa-server-selinux ### [100%] certmonger_unconfined_exec_t type was unknown with my selinux policy: selinux-policy-3.10.0-80.fc16.noarch selinux-policy-targeted-3.10.0-80.fc16.noarch If we need a higher SELinux version, we should bump the required package version spec file. Yeah, waiting on it to be backported. 2) Change of SELinux context with /usr/bin/chcon is temporary until restorecon or system relabel occurs. I think we should make it persistent and enforce this type in our SELinux policy and rather call restorecon instead of chcon That's a good idea, why didn't I think of that :-( Ah, now I remember, it will be handled by selinux-policy. I would have used restorecon here but since the policy isn't there yet this seemed like a good idea. I'm trying to find out the status of this new policy, it may only make it into F-17. rob Ok. But if this policy does not go in F-16 and if we want this fix in F16 release too, I guess we would have to implement both approaches in our spec file: 1) When on F16, include SELinux policy for restart scripts + run restorecon 2) When on F17, do not include the SELinux policy (+ run restorecon) Martin Won't work without updated selinux-policy. Without the permission for certmonger to execute the commands things will still fail (just in really non-obvious and far in the future ways). It looks like this is fixed in F-17 selinux-policy-3.10.0-107. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 20 Fix empty external member processing
On 04/03/2012 12:22 PM, Ondrej Hamada wrote: https://fedorahosted.org/freeipa/ticket/2447 Validation of external member was failing for empty strings because of wrong condition. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Used clearer solution. Thanks to Rob for advice. -- Regards, Ondrej Hamada FreeIPA team jabber: oh...@jabbim.cz IRC: ohamada From 837734d515b72fd10b5284b13acfdcba94deeec1 Mon Sep 17 00:00:00 2001 From: Ondrej Hamada oham...@redhat.com Date: Tue, 3 Apr 2012 15:16:58 +0200 Subject: [PATCH] Fix empty external member processing Validation of external member was failing for empty strings because of wrong condition. https://fedorahosted.org/freeipa/ticket/2447 --- ipalib/plugins/baseldap.py |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py index 38f369a779adc53454837994bd2bec5b74d3bbd4..11ec16fe10a7ebaf1cd00214f8c6c264952d81d4 100644 --- a/ipalib/plugins/baseldap.py +++ b/ipalib/plugins/baseldap.py @@ -326,7 +326,7 @@ def add_external_pre_callback(membertype, ldap, dn, keys, options): def validate_host(hostname): validate_hostname(hostname, check_fqdn=False, allow_underscore=True) -if membertype in options: +if options.get(membertype,False): if membertype == 'host': validator = validate_host else: -- 1.7.6.5 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 490 Fix s4u2proxy handling when a MS-PAC is available
On Wed, 2012-03-28 at 11:36 +0200, Sumit Bose wrote: On Tue, Mar 27, 2012 at 03:17:06PM -0400, Simo Sorce wrote: This patch fixes #2504, the logic to choose the client principal to use was basically reversed, and we ended up using the wrong principal to verify the PAC owner. This patch fixes it. Tested and s4u2proxy keeps working both with and without a PAC attached. It also keeps working with normal TGS requests of course. ACK, '--delegate' is not neede anymore. 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] [PATCH] 74 Check configured maximum user login length on user rename
https://fedorahosted.org/freeipa/ticket/2587 Honza -- Jan Cholasta From 595e012ae9b6a7f4f6eef7d534dcb9e7c7574144 Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Tue, 3 Apr 2012 09:23:39 -0400 Subject: [PATCH] Check configured maximum user login length on user rename. ticket 2587 --- ipalib/plugins/user.py | 10 ++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py index 64424e8..5e2471e 100644 --- a/ipalib/plugins/user.py +++ b/ipalib/plugins/user.py @@ -547,6 +547,16 @@ class user_mod(LDAPUpdate): has_output_params = LDAPUpdate.has_output_params + user_output_params def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options): +if 'rename' in options: +config = ldap.get_ipa_config()[1] +if 'ipamaxusernamelength' in config: +if len(options['rename']) int(config.get('ipamaxusernamelength')[0]): +raise errors.ValidationError( +name=self.obj.primary_key.cli_name, +error=_('can be at most %(len)d characters') % dict( +len = int(config.get('ipamaxusernamelength')[0]) +) +) if 'mail' in entry_attrs: entry_attrs['mail'] = self.obj._normalize_email(entry_attrs['mail']) if 'manager' in entry_attrs: -- 1.7.7.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0042-0048 AD trusts support (master)
On Tue, Apr 03, 2012 at 01:41:35PM +0300, Alexander Bokovoy wrote: Hi! Attached are the current patches for adding support for Active Directory trusts for FreeIPA v3 (master). These are tested and working with samba4 build available in ipa-devel@ repo. You have to use --delegate until we'll get all the parts of the Heimdal puzzle untangled and solved, and Simo patch 490 (s4u2proxy fix) is committed as well. Sumit asked me to send patches for review and commit to master so that he can proceed with his changes (removal of kadmin.local use, SID population task for 389-ds, etc). Without kadmin.local use fix these patches are not working with SELinux enabled. Patches have [../9] mark because they were generated out of my adwork tree. I have merged two patches together for obvious change reason and have left out Simo's s4u2proxy patch out, thus there are seven patches proposed for commit. I have tested the patches and they worked fine for me. They currently only work in F17, because it relies on the version of python-ldap shipped with F17. So it is an ACK form my side. It would be nice if someone more can have a look at the python parts if they are in agreement with the IPA standards (I expect they are :-). bye, Sumit -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] More types of replica in FreeIPA
On 03/13/2012 01:13 AM, Dmitri Pal wrote: On 03/12/2012 06:10 PM, Simo Sorce wrote: On Mon, 2012-03-12 at 17:40 -0400, Dmitri Pal wrote: On 03/12/2012 04:16 PM, Simo Sorce wrote: On Mon, 2012-03-12 at 20:38 +0100, Ondrej Hamada wrote: USER'S operations when connection is OK: --- read data - local write data - forwarding to master authentication: -credentials cached -- authenticate against credentials in local cache -on failure: log failure locally, update data about failures only on lock-down of account -credentials not cached -- forward request to master, on success cache the credentials This scheme doesn't work with Kerberos. Either you have a copy of the user's keys locally or you don't, there is nothing you can really cache if you don't. Simo. Yes this is what we are talking about here - the cache would have to contain user Kerberos key but there should be some expiration on the cache so that fetched and stored keys periodically cleaned following the policy an admin has defined. We would need a mechanism to transfer Kerberos keys, but that would not be sufficient, you'd have to give read-only servers also the realm krbtgt in order to be able to do anything with those keys. The way MS solves hits (I think) is by giving a special RODC krbtgt to each RODC, and then replicating all RODC krbtgt's with full domain controllers. Full domain controllers have logic to use RODC's krbtgt keys instead of the normal krbtgt to perform operations when user's krbtgt are presented to a different server. This is a lot of work and changes in the KDC, not something we can implement easily. As a first implementation I would restrict read-only replicas to not do Kerberos at all, only LDAP for all the lookup stuff necessary. to add a RO KDC we will need to plan a lot of changes in the KDC. We will also need intelligent partial replication where the rules about which object (and which attributes in the object) need/can be replicated are established based on some grouping+filter mechanism. This also is a pretty important change to 389ds. Simo. I agree. I am just trying to structure the discussion a bit so that all what you are saying can be captured in the design document and then we can pick a subset of what Ondrej will actually implement. So let us capture all the complexity and then do a POC for just LDAP part. Sorry for inactivity, I was struggling with a lot of school stuff. I've summed up the main goals, do you agree on them or should I add/remove any? GOALS === Create Hub and Consumer types of replica with following features: * Hub is read-only * Hub interconnects Masters with Consumers or Masters with Hubs or Hubs with other Hubs * Hub is hidden in the network topology * Consumer is read-only * Consumer interconnects Masters/Hubs with clients * Write operations should be forwarded to Master * Consumer should be able to log users into system without communication with master * Consumer should cache user's credentials * Caching of credentials should be configurable * CA server should not be allowed on Hubs and Consumers -- Regards, Ondrej Hamada FreeIPA team jabber: oh...@jabbim.cz IRC: ohamada ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 20 Fix empty external member processing
On Tue, 2012-04-03 at 15:22 +0200, Ondrej Hamada wrote: On 04/03/2012 12:22 PM, Ondrej Hamada wrote: https://fedorahosted.org/freeipa/ticket/2447 Validation of external member was failing for empty strings because of wrong condition. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Used clearer solution. Thanks to Rob for advice. ACK for this patch fixing empty --hosts, --users, etc. options. We just need to triage the second issue found during testing - an ability to set invalid external* attribute value with --setattr or --addattr options. I see 2 ways to fix that: 1) Ugly fix: Call a similar precallback in all affected *-mod commands where --addattr or --setattr could be used (netgroup-mod, sudorule-mod, etc.) which would specifically validate external* attribute values. 2) Nice fix: - create a param for external hosts, users to the respective LDAPOobjects - netgroup, sudorule, etc. and implement proper validators for them. These params would not be visible for users or cloned for Commands. Most code from Ondra's original patch 16 could be re-used - update Ondra's precallback to use these params for validation - update --setattr and --addattr param processing to consider also these params that exist only in LDAPObject and not in Command I think it would be OK to just create a ticket for the second issue and close ticket #2447 with Ondra's patch 20-2 as is. The new ticket could be targeted for next release as there are more changes needed, including fixes in --setattr and --addattr processing. I don't think this issue has a high impact, setting external* attribute values via --setattr is not really a standard procedure. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal
On Tue, 2012-04-03 at 10:45 -0400, Rob Crittenden wrote: Rob Crittenden wrote: Martin Kosek wrote: On Mon, 2012-04-02 at 15:36 -0400, Rob Crittenden wrote: Rob Crittenden wrote: Martin Kosek wrote: On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote: Certmonger will currently automatically renew server certificates but doesn't restart the services so you can still end up with expired certificates if you services never restart. This patch registers are restart command with certmonger so the IPA services will automatically be restarted to get the updated cert. Easy to test. Install IPA then resubmit the current server certs and watch the services restart: # ipa-getcert list Find the ID for either your dirsrv or httpd instance # ipa-getcert resubmit -iID Watch /var/log/httpd/error_log or /var/log/dirsrv/slapd-INSTANCE/errors to see the service restart. rob What about current instances - can we/do we want to update certmonger tracking so that their instances are restarted as well? Anyway, I found few issues SELinux issues with the patch: 1) # rpm -Uvh freeipa-* Preparing... ### [100%] 1:freeipa-python ### [ 20%] 2:freeipa-client ### [ 40%] 3:freeipa-admintools ### [ 60%] 4:freeipa-server ### [ 80%] /usr/bin/chcon: failed to change context of `/usr/lib64/ipa/certmonger' to `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument /usr/bin/chcon: failed to change context of `/usr/lib64/ipa/certmonger/restart_dirsrv' to `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument /usr/bin/chcon: failed to change context of `/usr/lib64/ipa/certmonger/restart_httpd' to `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64) scriptlet failed, exit status 1 5:freeipa-server-selinux ### [100%] certmonger_unconfined_exec_t type was unknown with my selinux policy: selinux-policy-3.10.0-80.fc16.noarch selinux-policy-targeted-3.10.0-80.fc16.noarch If we need a higher SELinux version, we should bump the required package version spec file. Yeah, waiting on it to be backported. 2) Change of SELinux context with /usr/bin/chcon is temporary until restorecon or system relabel occurs. I think we should make it persistent and enforce this type in our SELinux policy and rather call restorecon instead of chcon That's a good idea, why didn't I think of that :-( Ah, now I remember, it will be handled by selinux-policy. I would have used restorecon here but since the policy isn't there yet this seemed like a good idea. I'm trying to find out the status of this new policy, it may only make it into F-17. rob Ok. But if this policy does not go in F-16 and if we want this fix in F16 release too, I guess we would have to implement both approaches in our spec file: 1) When on F16, include SELinux policy for restart scripts + run restorecon 2) When on F17, do not include the SELinux policy (+ run restorecon) Martin Won't work without updated selinux-policy. Without the permission for certmonger to execute the commands things will still fail (just in really non-obvious and far in the future ways). It looks like this is fixed in F-17 selinux-policy-3.10.0-107. rob Updated patch which works on F-17. rob What about F-16? The restart scripts won't work with enabled enforcing and will raise AVCs. Maybe we really need to deliver our own SELinux policy allowing it on F-16. I also found an issue with the restart scripts: 1) restart_dirsrv: this won't work with systemd: # /sbin/service dirsrv restart Redirecting to /bin/systemctl restart dirsrv.service Failed to issue method call: Unit dirsrv.service failed to load: No such file or directory. See system logs and 'systemctl status dirsrv.service' for details. We would need to pass an instance of IPA dirsrv for this to work. 2) restart_httpd: Is reload enough for httpd to pull a new certificate? Don't we need a full restart? If reload is enough, I think the command should be named reload_httpd Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal
Martin Kosek wrote: On Tue, 2012-04-03 at 10:45 -0400, Rob Crittenden wrote: Rob Crittenden wrote: Martin Kosek wrote: On Mon, 2012-04-02 at 15:36 -0400, Rob Crittenden wrote: Rob Crittenden wrote: Martin Kosek wrote: On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote: Certmonger will currently automatically renew server certificates but doesn't restart the services so you can still end up with expired certificates if you services never restart. This patch registers are restart command with certmonger so the IPA services will automatically be restarted to get the updated cert. Easy to test. Install IPA then resubmit the current server certs and watch the services restart: # ipa-getcert list Find the ID for either your dirsrv or httpd instance # ipa-getcert resubmit -iID Watch /var/log/httpd/error_log or /var/log/dirsrv/slapd-INSTANCE/errors to see the service restart. rob What about current instances - can we/do we want to update certmonger tracking so that their instances are restarted as well? Anyway, I found few issues SELinux issues with the patch: 1) # rpm -Uvh freeipa-* Preparing... ### [100%] 1:freeipa-python ### [ 20%] 2:freeipa-client ### [ 40%] 3:freeipa-admintools ### [ 60%] 4:freeipa-server ### [ 80%] /usr/bin/chcon: failed to change context of `/usr/lib64/ipa/certmonger' to `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument /usr/bin/chcon: failed to change context of `/usr/lib64/ipa/certmonger/restart_dirsrv' to `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument /usr/bin/chcon: failed to change context of `/usr/lib64/ipa/certmonger/restart_httpd' to `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64) scriptlet failed, exit status 1 5:freeipa-server-selinux ### [100%] certmonger_unconfined_exec_t type was unknown with my selinux policy: selinux-policy-3.10.0-80.fc16.noarch selinux-policy-targeted-3.10.0-80.fc16.noarch If we need a higher SELinux version, we should bump the required package version spec file. Yeah, waiting on it to be backported. 2) Change of SELinux context with /usr/bin/chcon is temporary until restorecon or system relabel occurs. I think we should make it persistent and enforce this type in our SELinux policy and rather call restorecon instead of chcon That's a good idea, why didn't I think of that :-( Ah, now I remember, it will be handled by selinux-policy. I would have used restorecon here but since the policy isn't there yet this seemed like a good idea. I'm trying to find out the status of this new policy, it may only make it into F-17. rob Ok. But if this policy does not go in F-16 and if we want this fix in F16 release too, I guess we would have to implement both approaches in our spec file: 1) When on F16, include SELinux policy for restart scripts + run restorecon 2) When on F17, do not include the SELinux policy (+ run restorecon) Martin Won't work without updated selinux-policy. Without the permission for certmonger to execute the commands things will still fail (just in really non-obvious and far in the future ways). It looks like this is fixed in F-17 selinux-policy-3.10.0-107. rob Updated patch which works on F-17. rob What about F-16? The restart scripts won't work with enabled enforcing and will raise AVCs. Maybe we really need to deliver our own SELinux policy allowing it on F-16. Right, I don't see this working on F-16. I don't really want to carry this type of policy. It goes beyond marking a few files as certmonger_t, it is going to let certmonger execute arbitrary scripts. This is best left to the SELinux team who understand the consequences better. I also found an issue with the restart scripts: 1) restart_dirsrv: this won't work with systemd: # /sbin/service dirsrv restart Redirecting to /bin/systemctl restart dirsrv.service Failed to issue method call: Unit dirsrv.service failed to load: No such file or directory. See system logs and 'systemctl status dirsrv.service' for details. Wouldn't work so hot for sysV either as we'd be restarting all instances. I'll take a look. We would need to pass an instance of IPA dirsrv for this to work. 2) restart_httpd: Is reload enough for httpd to pull a new certificate? Don't we need a full restart? If reload is enough, I think the command should be named reload_httpd Yes, it causes the modules to be reloaded which will reload the NSS database, that's all we need. I named it this way for consistency. I can rename it, though I doubt it would cause any confusion either way. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com