Re: [Freeipa-devel] [PATCH] 1059 single CRL generator
On 10/04/2012 06:17 PM, Rob Crittenden wrote: > This changes the way IPA generates CRLs for new installs only. > > The first master installed is configured as the CRL generator. An entry is > added to cn=masters that designates it. > > When a replica is installed it queries this entry so it knows where to forward > CRL requests. CRL files are not available on cloned CAs (so /ipa/crl will > return not found). It is possible to get a CRL directly from the clone CA via > http://:9180/ca/ee/ca/getCRL?op=getCRL&crlIssuingPoint=MasterCRL > > rob I tested new IPA server + replica with your patch and the CRL was now generated only on the CRL master. I also tried OCSP (though this may not be relevant) and it worked for me too. 1) I do not understand the following block in set_crl_master(self, suffix): +try: +self.admin_conn.addEntry(entry) +except ldap.ALREADY_EXISTS, e: +root_logger.debug("failed to set CA as CRL generator") +raise e - when we hit ldap.ALREADY_EXISTS, we are actually OK because cn=CRL is set, right? - AFAIK, addEntry should return our errors, i.e. errors.DuplicateEntry - s/raise e/raise/ I think you may have wanted to rather catch for more general LDAP error and then report a real error and not just a debug note. 2) In get_crl_master: +except Exception, e: +root_logger.debug("Could not connect to the Directory Server on %s: %s" % (master_host, str(e))) +raise e s/raise e/raise/ +except errors.NotFound, e: +root_logger.debug("failed to find CA CRL generator") +self.crl_master = None - e is actually not used, "except errors.NotFound" would be enough 3) Majorish issue I hit with the actual CRL publishing on our server (F17). I always get 403 Forbidden error when trying to download CRL from the CRL master: # wget --ca-certificate /etc/ipa/ca.crt https://`hostname`/ipa/crl/MasterCRL.bin --2012-10-05 03:32:58-- https://vm-120.idm.lab.bos.redhat.com/ipa/crl/MasterCRL.bin Resolving vm-120.idm.lab.bos.redhat.com... 10.16.78.120 Connecting to vm-120.idm.lab.bos.redhat.com|10.16.78.120|:443... connected. HTTP request sent, awaiting response... 403 Forbidden 2012-10-05 03:32:58 ERROR 403: Forbidden. I tracked the problem down to too strict permission on /var/lib/pki-ca directory which is being published by httpd which does not have access to it: # ll /var/lib/pki-ca drwxrwx---. 11 pkiuser pkiuser 4096 Oct 5 03:00 pki-ca When I fixed the permission: # chmod o+x /var/lib/pki-ca/ I was able to get pass the Forbidden error and actually retrieved the CRL. Adding Ade on CC list to follow on this permission issue. I was thinking about usability of this new approach, we may want to make user life easier in a perspective of CRL master managing. I have following enhancements in mind: - mark CRL master in ipa-replica-manage list, or ipa-csreplica-manage list: # ipa-csreplica-manage list Directory Manager password: vm-065.idm.lab.bos.redhat.com: master [CRL] vm-120.idm.lab.bos.redhat.com: master - when removing master with CRL by "ipa-replica-manage del" we should warn user and inform him what he should do next to amend the situation. I am thinking about 2 new commands for ipa-csreplica-manage: * ipa-csreplica-manage crl-promote - promote current master as the new CRL master, enable CRL generation in CS.cfg, mark master as the new CRL master in cn=masters * ipa-csreplica-manage crl-update - update CS.cfg of current CA replica and point master.ca.agent.* to current CRL master I can work on those enhancements if we agree on them. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Changes to use a single database for dogtag and IPA
On 10/04/2012 10:04 PM, Ade Lee wrote: Attached is a patch to handle the ipa-replica-conncheck issue. It should be applied on top of your patch. Essentially, the fix is as follows: A. If the DS_PORT = 7389, then we pass --check-ca in the ipa-replica-conncheck to be executed on the master. a1. If the master is ipa 2.x, this will prompt a check for port 7389. a2. If the master is ipa 3, this will default to dogtag_ds_port of 7389. B. Else if DS_PORT = 389, then we pass nothing to the ipa-replica-conncheck to be executed on the master. This is because we will be checking 389 in any case for the IPA DS check. Ade The code works, I got past the connection check, and now I'm getting the same internal server error with a clone from IPA 2.2 as with a clone from "3.0 upgraded from 2.2" (unknown object class "request"). However, since with this patch IPA will depend on Dogtag 10, install_constants.DS_PORT will always be 389, so case A will never happen. I think the two if-blocks that add the --check-ca can be dropped entirely. Assuming that even a clone from an old instance will use the single DB, conncheck's --dogtag-master-ds-port option is redundant: iff the server supports it, it uses port 389 which doesn't need re-checking. 0001-Allow-ipa-replica-conncheck-to-work-with-2.2-instanc.patch From a9cd4cb15e6c230e5690f3fa919fda9c5728ee10 Mon Sep 17 00:00:00 2001 From: Ade Lee Date: Thu, 4 Oct 2012 15:55:29 -0400 Subject: [PATCH] Allow ipa-replica-conncheck to work with 2.2 instances --- install/tools/ipa-replica-conncheck | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/install/tools/ipa-replica-conncheck b/install/tools/ipa-replica-conncheck index c9fb816be43d873a6ca79396e77270fd0d10aa12..498ef49e84e1dc8325b6fc2d850c8bffb9297e69 100755 --- a/install/tools/ipa-replica-conncheck +++ b/install/tools/ipa-replica-conncheck @@ -282,7 +282,11 @@ def main(): required_ports = BASE_PORTS if options.check_ca: -ca_port = CheckedPort(int(options.dogtag_master_ds_port), SOCK_STREAM, +port_val = 7389 +if options.dogtag_master_ds_port: +port_val = int(options.dogtag_master_ds_port) + +ca_port = CheckedPort(port_val, SOCK_STREAM, "PKI-CA: Directory Service port") required_ports.extend([ca_port]) @@ -359,11 +363,8 @@ def main(): raise RuntimeError("Could not get ticket for master server: %s" % stderr) remote_check_opts = ['--replica %s' % options.hostname] -if options.check_ca: +if options.check_ca and dogtag.install_constants.DS_PORT == 7389: remote_check_opts.append('--check-ca') -remote_check_opts.extend(["--dogtag-master-ds-port", -str(dogtag.install_constants.DS_PORT)]) - print_info("Execute check on remote master") @@ -387,10 +388,8 @@ def main(): print_info("Please run the following command on remote master:") remote_check_opts = ['--replica %s' % options.hostname] -if options.check_ca: +if options.check_ca and dogtag.install_constants.DS_PORT == 7389: remote_check_opts.append('--check-ca') -remote_check_opts.extend(["--dogtag-master-ds-port", -str(dogtag.install_constants.DS_PORT)]) print_info("/usr/sbin/ipa-replica-conncheck " + " ".join(remote_check_opts)) time.sleep(3600) -- 1.7.12 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [RFC] Reload trust data in ipadb
Hi, currently the KDC must be restarted if a new trust is added to make the KDC aware of the new domain. With the attached patch the data is reloaded automatically if a request from an unknown domain was received. It works for me, but I'm not completely sure if this is the best approach to avoid the restart of the KDC. Comments are welcome. bye Sumit From 5014831c66fa73bb25715725498c45e7d796 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Fri, 5 Oct 2012 12:06:24 +0200 Subject: [PATCH] ipadb: reload trust information if domain is not know Currently the data about trusted domains is read once at startup. If a new trust is added the KDC must be restarted to know about the new trust. This patch reloads the trust data if there is a request from an unknown domain. To make DOS attacks a bit harder the data can be updated only once in a minute. --- daemons/ipa-kdb/ipa_kdb_mspac.c | 44 - 1 Datei geändert, 35 Zeilen hinzugefügt(+), 9 Zeilen entfernt(-) diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c index 1a03323d137a2b89a08f5b12ca00752e4e190de7..cbf74cf0b2abdf814b4254d89565399b6eeced0f 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac.c +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c @@ -40,6 +40,7 @@ struct ipadb_mspac { int num_trusts; struct ipadb_adtrusts *trusts; +time_t last_update; }; #ifdef DEBUG @@ -967,6 +968,8 @@ static struct ipadb_adtrusts *get_domain_from_realm(krb5_context context, struct ipadb_context *ipactx; struct ipadb_adtrusts *domain; int i; +krb5_error_code kerr; +bool updated = false; ipactx = ipadb_get_context(context); if (!ipactx) { @@ -977,17 +980,28 @@ static struct ipadb_adtrusts *get_domain_from_realm(krb5_context context, return NULL; } -for (i = 0; i < ipactx->mspac->num_trusts; i++) { -domain = &ipactx->mspac->trusts[i]; -if (strlen(domain->domain_name) != realm.length) { -continue; +do { +for (i = 0; i < ipactx->mspac->num_trusts; i++) { +domain = &ipactx->mspac->trusts[i]; +if (strlen(domain->domain_name) != realm.length) { +continue; +} +if (strncasecmp(domain->domain_name, realm.data, +realm.length) == 0) { +return domain; +} } -if (strncasecmp(domain->domain_name, realm.data, realm.length) == 0) { -return domain; -} -} -return NULL; +if (updated) { +return NULL; +} else { +kerr = ipadb_reinit_mspac(ipactx); +if (kerr != 0) { +return NULL; +} +updated = true; +} +} while(1); } static krb5_error_code filter_logon_info(krb5_context context, @@ -1534,6 +1548,16 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx) struct dom_sid gsid; char *resstr; int ret; +time_t now; + +/* Do not update the mspac struct more than once a minute. This would + * avoid heavy load on the directory server if there are lots of requests + * from domains which we do not trust. */ +now = time(NULL); +if (ipactx->mspac != NULL && now > ipactx->mspac->last_update && +(now - ipactx->mspac->last_update) < 60) { +return 0; +} /* clean up in case we had old values around */ ipadb_mspac_struct_free(&ipactx->mspac); @@ -1544,6 +1568,8 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx) goto done; } +ipactx->mspac->last_update = now; + kerr = ipadb_simple_search(ipactx, ipactx->base, LDAP_SCOPE_SUBTREE, "(objectclass=ipaNTDomainAttrs)", dom_attrs, &result); -- 1.7.11.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0082/0083 Handle NotFound exception when establishing trust
On 10/04/2012 05:06 PM, Alexander Bokovoy wrote: Hi, two attached patches attempt to solve https://fedorahosted.org/freeipa/ticket/3103 We cannot make educated guess where trusted domain's DNS server is located as we ended up with NotFound exception precisely because we were unable to discover trusted domain's domain controller location. Thus, all we can do is to suggest ways to fix the issue. Since suggestion becomes relatively long (multi-line, at least), it creates few issues for current exception error message handling: - root_logger does not use textui to format output - textui is only printing to standard output - multi-line error messages thus become non-wrapped - localizing such messages would become a harder context-wise. Web UI is showing error message as a single paragraph (), all multi-line markup would be lost. In the end, I decided to support list-based multi-line error messages in PublicError class. Its constructor will join all list-based arguments into single string per argument (first patch). In Web UI I've added special text processing of error messages that splits message into multiple lines and wraps those which start with a TAB symbol into 'error-message-hinted' span to visually separate it from the rest of error message. Trust code uses this to display suggested CLI command to set up DNS forwarder (second patch). In the end, CLI shows such error message fine (note tabulated CLI command): --- # ipa trust-add --type=ad --admin Administrator@ad.local1 --password ad.local1 Active directory domain administrator's password: ipa: ERROR: Unable to resolve domain controller for 'ad.local1' domain. IPA manages DNS, please configure forwarder to 'ad.local1' domain by using following CLI command. Please replace DNS_SERVER and IP_ADDRESS by name and address of the domain's name server: ipa dnszone-add ad.local1 --name-server=DNS_SERVER --admin-email='hostmaster@ad.local1' --force --forwarder=IP_ADDRESS --forward-policy=only When using Web UI, please create DNS zone for domain 'ad.local1' first and then set forwarder and forward policy --- Web UI looks like this: http://abbra.fedorapeople.org/.paste/ui.png You have undeclared identifier: lines. I recommend to run `jsl -conf jsl.conf` command in install/ui folder to prevent these issues. I'm not convinced that "beautify" call should be in command object. I would rather see it in error_dialog. I'm not sure that all tabbed error text should be red. But I don't recall any other usage so it's probably OK. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0074] Fix zone removal in persistent search update_zone()
Hello, Fix zone removal in persistent search update_zone(). Without this patch any zone removed through ipa dnszone-del will remain active and will return SERVFAILs. -- Petr^2 Spacek From e09eebf3c370ff4106013cdeda10a80782e26611 Mon Sep 17 00:00:00 2001 From: Petr Spacek Date: Fri, 5 Oct 2012 13:02:37 +0200 Subject: [PATCH] Fix zone removal in persistent search update_zone() handler. Signed-off-by: Petr Spacek --- src/ldap_helper.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/ldap_helper.c b/src/ldap_helper.c index 68b635f84b4c9015752667510c0497e5f56bc7ab..199b345bb604c30bfa8a3690afc844ca8b264e89 100644 --- a/src/ldap_helper.c +++ b/src/ldap_helper.c @@ -3056,7 +3056,6 @@ update_zone(isc_task_t *task, isc_event_t *event) ldap_qresult_t *ldap_qresult_record = NULL; ldap_entry_t *entry_zone = NULL; ldap_entry_t *entry_record = NULL; - isc_boolean_t delete = ISC_TRUE; isc_mem_t *mctx; dns_name_t prevname; char *attrs_zone[] = { @@ -3073,14 +3072,16 @@ update_zone(isc_task_t *task, isc_event_t *event) dns_name_init(&prevname, NULL); CHECK(manager_get_ldap_instance(pevent->dbname, &inst)); - CHECK(ldap_query(inst, NULL, &ldap_qresult_zone, pevent->dn, + + result = ldap_query(inst, NULL, &ldap_qresult_zone, pevent->dn, LDAP_SCOPE_BASE, attrs_zone, 0, - "(&(objectClass=idnsZone)(idnsZoneActive=TRUE))")); + "(&(objectClass=idnsZone)(idnsZoneActive=TRUE))"); + if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND) + CLEANUP_WITH(result); - for (entry_zone = HEAD(ldap_qresult_zone->ldap_entries); - entry_zone != NULL; - entry_zone = NEXT(entry_zone, link)) { - delete = ISC_FALSE; + if (result == ISC_R_SUCCESS && + HEAD(ldap_qresult_zone->ldap_entries) != NULL) { + entry_zone = HEAD(ldap_qresult_zone->ldap_entries); CHECK(ldap_parse_zoneentry(entry_zone, inst)); if (PSEARCH_MODDN(pevent->chgtype)) { @@ -3106,10 +3107,9 @@ update_zone(isc_task_t *task, isc_event_t *event) } INSIST(NEXT(entry_zone, link) == NULL); /* no multiple zones with same DN */ - } - - if (delete) + } else { CHECK(ldap_delete_zone(inst, pevent->dn, ISC_TRUE)); + } cleanup: if (result != ISC_R_SUCCESS) -- 1.7.11.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [RFC] Reload trust data in ipadb
On Fri, Oct 05, 2012 at 12:17:02PM +0200, Sumit Bose wrote: > Hi, > > currently the KDC must be restarted if a new trust is added to make the > KDC aware of the new domain. With the attached patch the data is > reloaded automatically if a request from an unknown domain was received. > It works for me, but I'm not completely sure if this is the best > approach to avoid the restart of the KDC. > > Comments are welcome. > > bye > Sumit I'm sorry, but the previous version of the patch depended on some unpushed patches in my tree. This new version should apply fine on master. bye, Sumit From f8726fe1c4a2ab71ada1297003e3dbe6068e4207 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Fri, 5 Oct 2012 12:06:24 +0200 Subject: [PATCH] ipadb: reload trust information if domain is not know Currently the data about trusted domains is read once at startup. If a new trust is added the KDC must be restarted to know about the new trust. This patch reloads the trust data if there is a request from an unknown domain. To make DOS attacks a bit harder the data can be updated only once in a minute. --- daemons/ipa-kdb/ipa_kdb_mspac.c | 44 - 1 Datei geändert, 35 Zeilen hinzugefügt(+), 9 Zeilen entfernt(-) diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c index b5346fed1230d02a88c94ab913507112990a1651..f0bd3bfe913705abd44efb08325f54521533637e 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac.c +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c @@ -40,6 +40,7 @@ struct ipadb_mspac { int num_trusts; struct ipadb_adtrusts *trusts; +time_t last_update; }; @@ -983,6 +984,8 @@ static struct ipadb_adtrusts *get_domain_from_realm(krb5_context context, struct ipadb_context *ipactx; struct ipadb_adtrusts *domain; int i; +krb5_error_code kerr; +bool updated = false; ipactx = ipadb_get_context(context); if (!ipactx) { @@ -993,17 +996,28 @@ static struct ipadb_adtrusts *get_domain_from_realm(krb5_context context, return NULL; } -for (i = 0; i < ipactx->mspac->num_trusts; i++) { -domain = &ipactx->mspac->trusts[i]; -if (strlen(domain->domain_name) != realm.length) { -continue; +do { +for (i = 0; i < ipactx->mspac->num_trusts; i++) { +domain = &ipactx->mspac->trusts[i]; +if (strlen(domain->domain_name) != realm.length) { +continue; +} +if (strncasecmp(domain->domain_name, realm.data, +realm.length) == 0) { +return domain; +} } -if (strncasecmp(domain->domain_name, realm.data, realm.length) == 0) { -return domain; -} -} -return NULL; +if (updated) { +return NULL; +} else { +kerr = ipadb_reinit_mspac(ipactx); +if (kerr != 0) { +return NULL; +} +updated = true; +} +} while(1); } static krb5_error_code filter_logon_info(krb5_context context, @@ -1550,6 +1564,16 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx) struct dom_sid gsid; char *resstr; int ret; +time_t now; + +/* Do not update the mspac struct more than once a minute. This would + * avoid heavy load on the directory server if there are lots of requests + * from domains which we do not trust. */ +now = time(NULL); +if (ipactx->mspac != NULL && now > ipactx->mspac->last_update && +(now - ipactx->mspac->last_update) < 60) { +return 0; +} /* clean up in case we had old values around */ ipadb_mspac_struct_free(&ipactx->mspac); @@ -1560,6 +1584,8 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx) goto done; } +ipactx->mspac->last_update = now; + kerr = ipadb_simple_search(ipactx, ipactx->base, LDAP_SCOPE_SUBTREE, "(objectclass=ipaNTDomainAttrs)", dom_attrs, &result); -- 1.7.11.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0075] Prevent misleading "partial match" error messages for disabled zones
Hello, Prevent misleading "partial match" error messages for disabled zones. Following message was printed if zone "e.test" was disabled and LDAP contained zones "test" and "e.test": update_zone (psearch) failed for 'idnsName=e.test,cn=dns,dc=e,dc=org'. Zones can be outdated, run `rndc reload`: partial match -- Petr^2 Spacek From e7f7a7d6e4fb3c9e47bac22d3291d09aa3d885ab Mon Sep 17 00:00:00 2001 From: Petr Spacek Date: Fri, 5 Oct 2012 13:37:18 +0200 Subject: [PATCH] Prevent misleading "partial match" error messages for disabled zones. Following message was printed if zone "e.test" was disabled and LDAP contained zones "test" and "e.test": update_zone (psearch) failed for 'idnsName=e.test,cn=dns,dc=e,dc=org'. Zones can be outdated, run `rndc reload`: partial match Signed-off-by: Petr Spacek --- src/ldap_helper.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ldap_helper.c b/src/ldap_helper.c index 199b345bb604c30bfa8a3690afc844ca8b264e89..d4bb6db10b0e79f8777fde3c5f344298af87ce56 100644 --- a/src/ldap_helper.c +++ b/src/ldap_helper.c @@ -815,10 +815,9 @@ ldap_delete_zone2(ldap_instance_t *inst, dns_name_t *name, isc_boolean_t lock) } result = zr_get_zone_ptr(inst->zone_register, name, &zone); - if (result == ISC_R_NOTFOUND) { + if (result == ISC_R_NOTFOUND || result == DNS_R_PARTIALMATCH) { log_debug(1, "zone '%s' not found in zone register", zone_name_char); - result = ISC_R_SUCCESS; - goto cleanup; + CLEANUP_WITH(ISC_R_SUCCESS); } else if (result != ISC_R_SUCCESS) goto cleanup; -- 1.7.11.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [RFC] Reload trust data in ipadb
On Fri, 2012-10-05 at 13:32 +0200, Sumit Bose wrote: > From f8726fe1c4a2ab71ada1297003e3dbe6068e4207 Mon Sep 17 00:00:00 2001 > From: Sumit Bose > Date: Fri, 5 Oct 2012 12:06:24 +0200 > Subject: [PATCH] ipadb: reload trust information if domain is not know > > Currently the data about trusted domains is read once at startup. If a > new trust is added the KDC must be restarted to know about the new > trust. This patch reloads the trust data if there is a request from an > unknown domain. To make DOS attacks a bit harder the data can be > updated > only once in a minute. > --- > daemons/ipa-kdb/ipa_kdb_mspac.c | 44 > - > 1 Datei geändert, 35 Zeilen hinzugefügt(+), 9 Zeilen entfernt(-) > > diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c > b/daemons/ipa-kdb/ipa_kdb_mspac.c > index > b5346fed1230d02a88c94ab913507112990a1651..f0bd3bfe913705abd44efb08325f54521533637e > 100644 > --- a/daemons/ipa-kdb/ipa_kdb_mspac.c > +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c > @@ -40,6 +40,7 @@ struct ipadb_mspac { > > int num_trusts; > struct ipadb_adtrusts *trusts; > +time_t last_update; > }; > > > @@ -983,6 +984,8 @@ static struct ipadb_adtrusts > *get_domain_from_realm(krb5_context context, > struct ipadb_context *ipactx; > struct ipadb_adtrusts *domain; > int i; > +krb5_error_code kerr; > +bool updated = false; The name of this variable sounds backwards, should be called need_update or similar. > ipactx = ipadb_get_context(context); > if (!ipactx) { > @@ -993,17 +996,28 @@ static struct ipadb_adtrusts > *get_domain_from_realm(krb5_context context, > return NULL; > } > > -for (i = 0; i < ipactx->mspac->num_trusts; i++) { > -domain = &ipactx->mspac->trusts[i]; > -if (strlen(domain->domain_name) != realm.length) { > -continue; > +do { > +for (i = 0; i < ipactx->mspac->num_trusts; i++) { > +domain = &ipactx->mspac->trusts[i]; > +if (strlen(domain->domain_name) != realm.length) { > +continue; > +} > +if (strncasecmp(domain->domain_name, realm.data, > +realm.length) == 0) { > +return domain; > +} > } > -if (strncasecmp(domain->domain_name, realm.data, > realm.length) == 0) { > -return domain; > -} > -} > > -return NULL; > +if (updated) { > +return NULL; > +} else { > +kerr = ipadb_reinit_mspac(ipactx); > +if (kerr != 0) { > +return NULL; > +} > +updated = true; > +} > +} while(1); > } I do not think I like the do/while loop here, I would rather have another function that wraps the current get_domain_from_realm() like: static struct ipadb_adtrusts *get_domain_from_realm_update() { dom = get_domain_from_realm(); if (dom == NULL) { /* no dom, try to see if a new trust appeared */ kerr = ipadb_reinit_mspac(ipactx); dom = get_domain_from_realm(); } return dom; } This is clearer imo, and allows you to decide if you really want to update or if you want to call the non-updating function later on, should we need it. > static krb5_error_code filter_logon_info(krb5_context context, > @@ -1550,6 +1564,16 @@ krb5_error_code ipadb_reinit_mspac(struct > ipadb_context *ipactx) > struct dom_sid gsid; > char *resstr; > int ret; > +time_t now; > + > +/* Do not update the mspac struct more than once a minute. This > would > + * avoid heavy load on the directory server if there are lots of > requests > + * from domains which we do not trust. */ > +now = time(NULL); > +if (ipactx->mspac != NULL && now > ipactx->mspac->last_update && > +(now - ipactx->mspac->last_update) < 60) { > +return 0; > +} > > /* clean up in case we had old values around */ > ipadb_mspac_struct_free(&ipactx->mspac); > @@ -1560,6 +1584,8 @@ krb5_error_code ipadb_reinit_mspac(struct > ipadb_context *ipactx) > goto done; > } > > +ipactx->mspac->last_update = now; > + > kerr = ipadb_simple_search(ipactx, ipactx->base, > LDAP_SCOPE_SUBTREE, > "(objectclass=ipaNTDomainAttrs)", > dom_attrs, > &result); This part look fine, I wonder if we shouldn't make it even longer than 1 min. 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] 0082/0083 Handle NotFound exception when establishing trust
On Fri, 05 Oct 2012, Petr Vobornik wrote: On 10/04/2012 05:06 PM, Alexander Bokovoy wrote: Hi, two attached patches attempt to solve https://fedorahosted.org/freeipa/ticket/3103 We cannot make educated guess where trusted domain's DNS server is located as we ended up with NotFound exception precisely because we were unable to discover trusted domain's domain controller location. Thus, all we can do is to suggest ways to fix the issue. Since suggestion becomes relatively long (multi-line, at least), it creates few issues for current exception error message handling: - root_logger does not use textui to format output - textui is only printing to standard output - multi-line error messages thus become non-wrapped - localizing such messages would become a harder context-wise. Web UI is showing error message as a single paragraph (), all multi-line markup would be lost. In the end, I decided to support list-based multi-line error messages in PublicError class. Its constructor will join all list-based arguments into single string per argument (first patch). In Web UI I've added special text processing of error messages that splits message into multiple lines and wraps those which start with a TAB symbol into 'error-message-hinted' span to visually separate it from the rest of error message. Trust code uses this to display suggested CLI command to set up DNS forwarder (second patch). In the end, CLI shows such error message fine (note tabulated CLI command): --- # ipa trust-add --type=ad --admin Administrator@ad.local1 --password ad.local1 Active directory domain administrator's password: ipa: ERROR: Unable to resolve domain controller for 'ad.local1' domain. IPA manages DNS, please configure forwarder to 'ad.local1' domain by using following CLI command. Please replace DNS_SERVER and IP_ADDRESS by name and address of the domain's name server: ipa dnszone-add ad.local1 --name-server=DNS_SERVER --admin-email='hostmaster@ad.local1' --force --forwarder=IP_ADDRESS --forward-policy=only When using Web UI, please create DNS zone for domain 'ad.local1' first and then set forwarder and forward policy --- Web UI looks like this: http://abbra.fedorapeople.org/.paste/ui.png You have undeclared identifier: lines. I recommend to run `jsl -conf jsl.conf` command in install/ui folder to prevent these issues. Fixed. I'm not convinced that "beautify" call should be in command object. I would rather see it in error_dialog. I moved everything to error_dialog and used $() selectors instead of directly playing with text. I'm not sure that all tabbed error text should be red. But I don't recall any other usage so it's probably OK. We don't have any, this is first one. -- / Alexander Bokovoy >From d7a413c0f3d57bbe788951de624f2caaf7065941 Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy Date: Thu, 4 Oct 2012 15:05:17 +0300 Subject: [PATCH 01/16] support multi-line error messages in exceptions --- install/ui/ipa.css | 9 - install/ui/ipa.js | 30 -- ipalib/errors.py | 12 +--- 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/install/ui/ipa.css b/install/ui/ipa.css index c8a220e78430fd38a1781be63421babaadafc948..3146e115524a6672b80e44956710a92bba79af0a 100644 --- a/install/ui/ipa.css +++ b/install/ui/ipa.css @@ -1113,6 +1113,13 @@ table.kerberos-key-status { background-color: #daa520; } +.error-message-hinted { +color: red; +padding-top: 0.5em; +padding-bottom: 0.5em; +font-family: monospace; +} + /* Table */ table.scrollable thead { @@ -1755,4 +1762,4 @@ form#login { .choice-header { font-weight: bold; -} \ No newline at end of file +} diff --git a/install/ui/ipa.js b/install/ui/ipa.js index bd25aeae2b4ff53f928b6fbd7429109fa9c55f68..a511d788bd230f33382f0abbd9496f47b027f9a3 100644 --- a/install/ui/ipa.js +++ b/install/ui/ipa.js @@ -1419,6 +1419,24 @@ IPA.error_dialog = function(spec) { that.visible_buttons = spec.visible_buttons || ['retry', 'cancel']; }; +that.beautify_message = function(container, message) { +var lines = message.split(/\n/g); +for(var i=0; i', { +class: 'error-message-hinted', +text: lines[i].substr(1) +}).appendTo(container); +} else { +var line_span = $('', { +text: lines[i] +}).appendTo(container); +} +} +}; + that.create = function() { if (that.error_thrown.url) { $('', { @@ -1426,9 +1444,9 @@ IPA.error_dialog = function(spec) { }).appendTo(that.container); } -$('', { -html: that.error_thrown.message -}).appendTo(that.container); +var error_message = $('', {}); +
Re: [Freeipa-devel] [PATCH] 75-78 Add fallback group
On Tue, 02 Oct 2012, Simo Sorce wrote: On Tue, 2012-10-02 at 21:29 +0200, Sumit Bose wrote: Hi, this patch should fix https://fedorahosted.org/freeipa/ticket/2955 by adding a fallback group as described in comment 2 of the ticket in ipa-adtrust-install. If you prefer to use a different kind of group I can change the patch accordingly. Patch works for me, so ACK except the group name. Yes I think we should use a more natural group name. In my recent testing I have been using the name 'Trust Users' that pairs well with another group we create called 'Trust Admins'. But I am open to suggestions on a better name, 'Domain Users' may be better if we really want to associate the wellknown SID to this group. I'm fine with 'Trust Users'. On the SID side I wonder if using the wellknown 'Domain Users' SID is the right thing to do. I do not see any special reasons why it shouldn't but I also do not have any special reason why we should. Anyone can think of any pros/cons of doing that ? Since it only has special meaning within the same domain and we are not using it for anything, it should be fine. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 83-84 Fix ipaIDobject usage
On Thu, 04 Oct 2012, Sumit Bose wrote: Hi, this is something that felt between the cracks. Some time ago we introduced a new objectclass ipaIDobject to allow objects to have an UID, GID or SID which are basically no users or groups. The DNA plugin should be aware of this new objectclass which is fix by the first patch. The second patch actually use this new objectclass in ipasam. Currenlty ipasam generates a hardcoded SID for the trusted domain user which might lead to confusion. With the second patch the trusted domain user has a proper SID. ACK, works for me. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 82 ipa-adtrust-install: print list of needed SRV records
On Thu, 04 Oct 2012, Sumit Bose wrote: Hi, this patch should fix the reopend https://fedorahosted.org/freeipa/ticket/3019 . ACK. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 75-78 Add fallback group
On 10/05/2012 03:27 PM, Alexander Bokovoy wrote: > On Tue, 02 Oct 2012, Simo Sorce wrote: >> On Tue, 2012-10-02 at 21:29 +0200, Sumit Bose wrote: >>> Hi, >>> >>> this patch should fix https://fedorahosted.org/freeipa/ticket/2955 by >>> adding a fallback group as described in comment 2 of the ticket in >>> ipa-adtrust-install. >>> >>> If you prefer to use a different kind of group I can change the patch >>> accordingly. > Patch works for me, so ACK except the group name. > >> Yes I think we should use a more natural group name. In my recent >> testing I have been using the name 'Trust Users' that pairs well with >> another group we create called 'Trust Admins'. But I am open to >> suggestions on a better name, 'Domain Users' may be better if we really >> want to associate the wellknown SID to this group. > I'm fine with 'Trust Users'. We may also want to add "Trust Users" (or other name we settle on) to PROTECTED_GROUPS list in group.py. This should prevent user accidentally deleting or renaming the group. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 75-78 Add fallback group
On Fri, 2012-10-05 at 16:27 +0300, Alexander Bokovoy wrote: > On Tue, 02 Oct 2012, Simo Sorce wrote: > >On Tue, 2012-10-02 at 21:29 +0200, Sumit Bose wrote: > >> Hi, > >> > >> this patch should fix https://fedorahosted.org/freeipa/ticket/2955 by > >> adding a fallback group as described in comment 2 of the ticket in > >> ipa-adtrust-install. > >> > >> If you prefer to use a different kind of group I can change the patch > >> accordingly. > Patch works for me, so ACK except the group name. > > >Yes I think we should use a more natural group name. In my recent > >testing I have been using the name 'Trust Users' that pairs well with > >another group we create called 'Trust Admins'. But I am open to > >suggestions on a better name, 'Domain Users' may be better if we really > >want to associate the wellknown SID to this group. > I'm fine with 'Trust Users'. > > >On the SID side I wonder if using the wellknown 'Domain Users' SID is > >the right thing to do. I do not see any special reasons why it shouldn't > >but I also do not have any special reason why we should. > >Anyone can think of any pros/cons of doing that ? > Since it only has special meaning within the same domain and we are not > using it for anything, it should be fine. Well it has a special meaning for samba servers ... we may have a few in the IPA domain. I think using that SID is fine but I think then the name 'Trust Users' could be confusing to users looking at permissions. We also need to make the group a posix group btw, because samba uses the Primary Group SID for users and if it can't be resolved to uid/gids it will probably prevent authentication. What about calling it 'Default SMB Group' or similar ? 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] 75-78 Add fallback group
On Fri, 2012-10-05 at 15:35 +0200, Martin Kosek wrote: > On 10/05/2012 03:27 PM, Alexander Bokovoy wrote: > > On Tue, 02 Oct 2012, Simo Sorce wrote: > >> On Tue, 2012-10-02 at 21:29 +0200, Sumit Bose wrote: > >>> Hi, > >>> > >>> this patch should fix https://fedorahosted.org/freeipa/ticket/2955 by > >>> adding a fallback group as described in comment 2 of the ticket in > >>> ipa-adtrust-install. > >>> > >>> If you prefer to use a different kind of group I can change the patch > >>> accordingly. > > Patch works for me, so ACK except the group name. > > > >> Yes I think we should use a more natural group name. In my recent > >> testing I have been using the name 'Trust Users' that pairs well with > >> another group we create called 'Trust Admins'. But I am open to > >> suggestions on a better name, 'Domain Users' may be better if we really > >> want to associate the wellknown SID to this group. > > I'm fine with 'Trust Users'. > > We may also want to add "Trust Users" (or other name we settle on) to > PROTECTED_GROUPS list in group.py. This should prevent user accidentally > deleting or renaming the group. +1 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] 79 Add SIDs for existing users and groups at the end of ipa-adtrust-install
On Wed, 03 Oct 2012, Sumit Bose wrote: Hi, this patch adds a new option to ipa-adtrust-install to generate the SID for users and groups at the end of the run. This fixes https://fedorahosted.org/freeipa/ticket/3104 . ACK, works for me too. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] [WIP] Firefox extension
On Thu, 04 Oct 2012, Petr Vobornik wrote: On 10/03/2012 04:19 PM, Simo Sorce wrote: On Wed, 2012-10-03 at 15:50 +0200, Petr Vobornik wrote: As Alexander proposed in other channel. I will remove the removal of configure.jar and offer the old configuration method if user is using FF < 4 so we don't have to make the extension compatible with this ancient version. It will be done this way: If FF < 4 is detected: * in browserconfig.html steps 2 and 3 will be grayed-out and replaced with step 2a with a link to ssbrowser.html and a description explaining the problem * ssbrowser.html will be enhanced by steps for autoconfiguration of FF < 4. We can also show the steps in browserconfig, but I want to have it somehow available even if user is not using FF<4 to keep general awareness about the problem and also to be usable if version detection fails. Other possible problem with steps in browserconfig is different styles of buttons (to keep the same styles we would have to include same css files and jquery.js to configure.jar, which I don't want to do). Excellent plan, we should try to reduce the amount of work, and letting old browsers use the old method is perfectly fine. If FF15 is the only browser that fails with the old method I would even go as far as testing exclusively with FF15 and have anything < FF15 use the old method. Simo. Updated patches attached. browserconfig.html points to older config method for versions prior to 15. The extension is theoretically compatible with FF3.6 and then FF10 and later. There is a problem for FF4-9 with loading strings from .properties file. FF3.6 is working because it doesn't use bootstraping. I also notice that we have an existing issue when navigating to config pages right away before accepting any certificate. Those pages are using some resorces from /ui/ folder which is redirected to https. These resources are not loaded because certificate isn't imported. If user is going straight for Web UI, he won't encounter this issue, but someone might. I tested this patchset and apart from the non-obvious extension description displayed when installing it, which is based on a certificate, everything is great. ACK. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] [WIP] Firefox extension
On 10/5/2012 8:56 AM, Alexander Bokovoy wrote: On Thu, 04 Oct 2012, Petr Vobornik wrote: On 10/03/2012 04:19 PM, Simo Sorce wrote: On Wed, 2012-10-03 at 15:50 +0200, Petr Vobornik wrote: As Alexander proposed in other channel. I will remove the removal of configure.jar and offer the old configuration method if user is using FF < 4 so we don't have to make the extension compatible with this ancient version. It will be done this way: If FF < 4 is detected: * in browserconfig.html steps 2 and 3 will be grayed-out and replaced with step 2a with a link to ssbrowser.html and a description explaining the problem * ssbrowser.html will be enhanced by steps for autoconfiguration of FF < 4. We can also show the steps in browserconfig, but I want to have it somehow available even if user is not using FF<4 to keep general awareness about the problem and also to be usable if version detection fails. Other possible problem with steps in browserconfig is different styles of buttons (to keep the same styles we would have to include same css files and jquery.js to configure.jar, which I don't want to do). Excellent plan, we should try to reduce the amount of work, and letting old browsers use the old method is perfectly fine. If FF15 is the only browser that fails with the old method I would even go as far as testing exclusively with FF15 and have anything < FF15 use the old method. Simo. Updated patches attached. browserconfig.html points to older config method for versions prior to 15. The extension is theoretically compatible with FF3.6 and then FF10 and later. There is a problem for FF4-9 with loading strings from .properties file. FF3.6 is working because it doesn't use bootstraping. I also notice that we have an existing issue when navigating to config pages right away before accepting any certificate. Those pages are using some resorces from /ui/ folder which is redirected to https. These resources are not loaded because certificate isn't imported. If user is going straight for Web UI, he won't encounter this issue, but someone might. I tested this patchset and apart from the non-obvious extension description displayed when installing it, which is based on a certificate, everything is great. ACK. It works for me too. Just some questions: 1. It looks like the Firefox is limited to version 10 to 15 in install/ffextension/install.rdf. Do we need the upper limit? 10.0 15.0.* 2. In install/html/ssbrowser.html the step 5 is optional. Should we explain what's that for or why we need it? General users could be confused and stuck if they are given choices that they don't understand. It's probably better to make it a required step if it doesn't cause any problem. 5. Optional: Repeat the above procedure for the network.negotiate-auth.delegation-uris entry, using the same domain. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] [WIP] Firefox extension
On Fri, 05 Oct 2012, Endi Sukma Dewata wrote: On 10/5/2012 8:56 AM, Alexander Bokovoy wrote: On Thu, 04 Oct 2012, Petr Vobornik wrote: On 10/03/2012 04:19 PM, Simo Sorce wrote: On Wed, 2012-10-03 at 15:50 +0200, Petr Vobornik wrote: As Alexander proposed in other channel. I will remove the removal of configure.jar and offer the old configuration method if user is using FF < 4 so we don't have to make the extension compatible with this ancient version. It will be done this way: If FF < 4 is detected: * in browserconfig.html steps 2 and 3 will be grayed-out and replaced with step 2a with a link to ssbrowser.html and a description explaining the problem * ssbrowser.html will be enhanced by steps for autoconfiguration of FF < 4. We can also show the steps in browserconfig, but I want to have it somehow available even if user is not using FF<4 to keep general awareness about the problem and also to be usable if version detection fails. Other possible problem with steps in browserconfig is different styles of buttons (to keep the same styles we would have to include same css files and jquery.js to configure.jar, which I don't want to do). Excellent plan, we should try to reduce the amount of work, and letting old browsers use the old method is perfectly fine. If FF15 is the only browser that fails with the old method I would even go as far as testing exclusively with FF15 and have anything < FF15 use the old method. Simo. Updated patches attached. browserconfig.html points to older config method for versions prior to 15. The extension is theoretically compatible with FF3.6 and then FF10 and later. There is a problem for FF4-9 with loading strings from .properties file. FF3.6 is working because it doesn't use bootstraping. I also notice that we have an existing issue when navigating to config pages right away before accepting any certificate. Those pages are using some resorces from /ui/ folder which is redirected to https. These resources are not loaded because certificate isn't imported. If user is going straight for Web UI, he won't encounter this issue, but someone might. I tested this patchset and apart from the non-obvious extension description displayed when installing it, which is based on a certificate, everything is great. ACK. It works for me too. Just some questions: 1. It looks like the Firefox is limited to version 10 to 15 in install/ffextension/install.rdf. Do we need the upper limit? 10.0 15.0.* 2. In install/html/ssbrowser.html the step 5 is optional. Should we explain what's that for or why we need it? General users could be confused and stuck if they are given choices that they don't understand. It's probably better to make it a required step if it doesn't cause any problem. 5. Optional: Repeat the above procedure for the network.negotiate-auth.delegation-uris entry, using the same domain. delegation-uris setting should be removed. It is not needed since we started using s4u2proxy mechanism. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] [WIP] Firefox extension
On Fri, 2012-10-05 at 17:33 +0300, Alexander Bokovoy wrote: > On Fri, 05 Oct 2012, Endi Sukma Dewata wrote: > >On 10/5/2012 8:56 AM, Alexander Bokovoy wrote: > >>On Thu, 04 Oct 2012, Petr Vobornik wrote: > >>>On 10/03/2012 04:19 PM, Simo Sorce wrote: > On Wed, 2012-10-03 at 15:50 +0200, Petr Vobornik wrote: > >As Alexander proposed in other channel. I will remove the removal of > >configure.jar and offer the old configuration method if user is > >using FF > >< 4 so we don't have to make the extension compatible with this ancient > >version. It will be done this way: > > > >If FF < 4 is detected: > > * in browserconfig.html steps 2 and 3 will be grayed-out and > >replaced > >with step 2a with a link to ssbrowser.html and a description explaining > >the problem > > * ssbrowser.html will be enhanced by steps for autoconfiguration > >of FF > >< 4. > > > >We can also show the steps in browserconfig, but I want to have it > >somehow available even if user is not using FF<4 to keep general > >awareness about the problem and also to be usable if version detection > >fails. Other possible problem with steps in browserconfig is different > >styles of buttons (to keep the same styles we would have to include > >same > >css files and jquery.js to configure.jar, which I don't want to do). > > Excellent plan, we should try to reduce the amount of work, and letting > old browsers use the old method is perfectly fine. > If FF15 is the only browser that fails with the old method I would even > go as far as testing exclusively with FF15 and have anything < FF15 use > the old method. > > Simo. > > >>> > >>>Updated patches attached. > >>> > >>>browserconfig.html points to older config method for versions prior to > >>>15. > >>> > >>>The extension is theoretically compatible with FF3.6 and then FF10 and > >>>later. There is a problem for FF4-9 with loading strings from > >>>.properties file. FF3.6 is working because it doesn't use bootstraping. > >>> > >>>I also notice that we have an existing issue when navigating to config > >>>pages right away before accepting any certificate. Those pages are > >>>using some resorces from /ui/ folder which is redirected to https. > >>>These resources are not loaded because certificate isn't imported. If > >>>user is going straight for Web UI, he won't encounter this issue, but > >>>someone might. > >>I tested this patchset and apart from the non-obvious extension > >>description displayed when installing it, which is based on a certificate, > >>everything is great. > >> > >>ACK. > >> > > > >It works for me too. Just some questions: > > > >1. It looks like the Firefox is limited to version 10 to 15 in > >install/ffextension/install.rdf. Do we need the upper limit? > > > > 10.0 > > 15.0.* > > > >2. In install/html/ssbrowser.html the step 5 is optional. Should we > >explain what's that for or why we need it? General users could be > >confused and stuck if they are given choices that they don't > >understand. It's probably better to make it a required step if it > >doesn't cause any problem. > > > > 5. Optional: Repeat the above procedure for the > >network.negotiate-auth.delegation-uris entry, using the same > >domain. > delegation-uris setting should be removed. It is not needed since we > started using s4u2proxy mechanism. Yes and we removed it because it is potentially a dangerous setting. It should be generally discouraged and enabled only for specific fqdn's not wildcard ones in future. 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] 1059 single CRL generator
On 10/05/2012 10:59 AM, Martin Kosek wrote: > On 10/04/2012 06:17 PM, Rob Crittenden wrote: >> This changes the way IPA generates CRLs for new installs only. >> >> The first master installed is configured as the CRL generator. An entry is >> added to cn=masters that designates it. >> >> When a replica is installed it queries this entry so it knows where to >> forward >> CRL requests. CRL files are not available on cloned CAs (so /ipa/crl will >> return not found). It is possible to get a CRL directly from the clone CA via >> http://:9180/ca/ee/ca/getCRL?op=getCRL&crlIssuingPoint=MasterCRL >> >> rob ... > 3) Majorish issue I hit with the actual CRL publishing on our server (F17). I > always get 403 Forbidden error when trying to download CRL from the CRL > master: > > # wget --ca-certificate /etc/ipa/ca.crt > https://`hostname`/ipa/crl/MasterCRL.bin > --2012-10-05 03:32:58-- > https://vm-120.idm.lab.bos.redhat.com/ipa/crl/MasterCRL.bin > Resolving vm-120.idm.lab.bos.redhat.com... 10.16.78.120 > Connecting to vm-120.idm.lab.bos.redhat.com|10.16.78.120|:443... connected. > HTTP request sent, awaiting response... 403 Forbidden > 2012-10-05 03:32:58 ERROR 403: Forbidden. > > I tracked the problem down to too strict permission on /var/lib/pki-ca > directory which is being published by httpd which does not have access to it: > > # ll /var/lib/pki-ca > > drwxrwx---. 11 pkiuser pkiuser 4096 Oct 5 03:00 pki-ca > > When I fixed the permission: > # chmod o+x /var/lib/pki-ca/ > > I was able to get pass the Forbidden error and actually retrieved the CRL. > Adding Ade on CC list to follow on this permission issue. FYI - I filed a ticket for this issue: https://fedorahosted.org/freeipa/ticket/3144 I plan to simply fix permission on /var/lib/pki-ca/ in a similar way we do for /var/lib/pki-ca/publish/ Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0076] Fix crashes on BIND reload caused by improper error handling during new zone addition
Hello, Fix crashes on BIND reload caused by improper error handling during new zone addition. Crash can be triggered by invalid query/transfer/update ACLs or potentially by error in zr_get_zone_ptr(). I found this problem during PATCH 75 testing, so there is a new ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/93 -- Petr^2 Spacek From d0e958cac75035b212f87f00fade080b025d0a23 Mon Sep 17 00:00:00 2001 From: Petr Spacek Date: Fri, 5 Oct 2012 14:41:57 +0200 Subject: [PATCH] Fix crashes on BIND reload caused by improper error handling during new zone addition. Crash can be triggered by invalid query/transfer/update ACLs or potentially by error in zr_get_zone_ptr(). https://fedorahosted.org/bind-dyndb-ldap/ticket/93 Signed-off-by: Petr Spacek --- src/ldap_helper.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/ldap_helper.c b/src/ldap_helper.c index d4bb6db10b0e79f8777fde3c5f344298af87ce56..0e1cf6f7a6986db126aaa5329dbe9abbc98c8bf4 100644 --- a/src/ldap_helper.c +++ b/src/ldap_helper.c @@ -1031,6 +1031,7 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst) isc_result_t result; isc_boolean_t unlock = ISC_FALSE; isc_boolean_t publish = ISC_FALSE; + isc_boolean_t published = ISC_FALSE; isc_task_t *task = inst->task; isc_uint32_t ldap_serial; isc_uint32_t zr_serial; /* SOA serial value from in-memory zone register */ @@ -1074,12 +1075,13 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst) /* Check if we are already serving given zone */ result = zr_get_zone_ptr(inst->zone_register, &name, &zone); - if (result != ISC_R_SUCCESS) { /* TODO: What about other errors? */ + if (result == ISC_R_NOTFOUND || result == DNS_R_PARTIALMATCH) { CHECK(create_zone(inst, &name, &zone)); CHECK(zr_add_zone(inst->zone_register, zone, dn)); publish = ISC_TRUE; log_debug(2, "created zone %p: %s", zone, dn); - } + } else if (result != ISC_R_SUCCESS) + CLEANUP_WITH(result); log_debug(2, "Setting SSU table for %p: %s", zone, dn); /* Get the update policy and update the zone with it. */ @@ -1119,6 +1121,7 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst) if (publish) { /* Everything is set correctly, publish zone */ CHECK(publish_zone(inst, zone)); + published = ISC_TRUE; } /* @@ -1178,6 +1181,13 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst) } cleanup: + if (publish && !published) { /* Failure in ACL parsing or so. */ + log_error_r("zone '%s': publishing failed, rolling back due to", + entry->dn); + result = zr_del_zone(inst->zone_register, &name); + if (result != ISC_R_SUCCESS) + log_error_r("zone '%s': rollback failed", entry->dn); + } if (unlock) isc_task_endexclusive(task); if (dns_name_dynamic(&name)) -- 1.7.11.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0017] Improve error message in ipa-replica-manage
On 10/02/2012 03:55 PM, Rob Crittenden wrote: Tomas Babej wrote: Hi, When executing ipa-replica-manage connect to an unknown or irrelevant master, we now print a sensible error message informing the user about this possiblity as well. https://fedorahosted.org/freeipa/ticket/3105 Tomas I put a whole bunch of code into a try/except and this may be catching errors in unexpected ways. I'm not entirely sure right now what we should do, but looking at the code in the try: repl1.conn.getEntry(master1_dn, ldap.SCOPE_BASE) repl1.conn.getEntry(master2_dn, ldap.SCOPE_BASE) We take in replica1 and replica1 as arguments (the default for replica1 is the current host). If either of these raise a NotFound it means there there is no master of that name. Does that mean that the master was deleted? Well, clearly not. A lot has changed since I did this, I may have been relying on a side-effect, or just hadn't tested well-enough. I wonder if we need that message at all. Is "foo" is not an IPA server good enough? It still might be confusing if someone didn't know that "foo" was deleted and it was still running. We could probably verify that it is at least an IPA server by doing similar checking in the client, it all depends on how far we want to take it. rob I modified the patch. Now if the NotFound error is encountered, we try to investigate whether we're trying to connect to an IPA server at all. Please see if you have any suggestions. Tomas >From 8bcd599a4fb2bb33f0592170df4a0f6b76d8aa2f Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Tue, 2 Oct 2012 09:15:33 -0400 Subject: [PATCH] IPA Server check in ipa-replica-manage When executing ipa-replica-manage connect to an master that raises NotFound error we now check if the master is at least IPA server. If so, we inform the user that it is probably foreign or previously deleted master. If not, we inform the user that the master is not an IPA server at all. https://fedorahosted.org/freeipa/ticket/3105 --- install/tools/ipa-replica-manage | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage index 897d117681d3e1559d5710366101b50540b705c8..19c9fea3092be0da4c9fed2640183a1939141a59 100755 --- a/install/tools/ipa-replica-manage +++ b/install/tools/ipa-replica-manage @@ -33,6 +33,7 @@ from ipalib import api, errors, util from ipapython.ipa_log_manager import * from ipapython.dn import DN from ipapython.config import IPAOptionParser +from ipaclient import ipadiscovery CACERT = "/etc/ipa/ca.crt" @@ -709,7 +710,19 @@ def add_link(realm, replica1, replica2, dirman_passwd, options): repl2.conn.getEntry(master2_dn, ldap.SCOPE_BASE) except errors.NotFound: -sys.exit("You cannot connect to a previously deleted master") +ds = ipadiscovery.IPADiscovery() +ret = ds.search(server=replica2) + +if ret == -4: # not an IPA Server +sys.exit("Connection unsuccessful: %s is not an IPA Server." % +replica2) +elif ret == 0: # success +sys.exit("Connection unsuccessful: %s is an IPA Server, " +"but it might be unknown, foreign or previously deleted " +"one." % replica2) +else: +sys.exit("Connection unsuccessful.") + repl1.setup_gssapi_replication(replica2, DN(('cn', 'Directory Manager')), dirman_passwd) print "Connected '%s' to '%s'" % (replica1, replica2) -- 1.7.11.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] Fix up trust attributes on trust-add
A onliner but better to have it validated by a second pair of eyes. Simo. -- Simo Sorce * Red Hat, Inc * New York >From 7cce4fe928bd56cd8ad8fbb10ee8f40961e0d739 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Fri, 5 Oct 2012 11:25:29 -0400 Subject: [PATCH] Fix trust attributes for ipa trust-add The RC4 flags in the trust attributes makes sense only fro trust type MIT We are using the UPLEVEL trust type. --- ipaserver/dcerpc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py index dd16c9f9e70950ecf679416f0c76ee21f0cc2cd3..c40313a697fa6ff842129944b7ad9c1f5fc14a77 100644 --- a/ipaserver/dcerpc.py +++ b/ipaserver/dcerpc.py @@ -365,7 +365,7 @@ class TrustDomainInstance(object): info.sid = security.dom_sid(another_domain.info['sid']) info.trust_direction = lsa.LSA_TRUST_DIRECTION_INBOUND | lsa.LSA_TRUST_DIRECTION_OUTBOUND info.trust_type = lsa.LSA_TRUST_TYPE_UPLEVEL -info.trust_attributes = lsa.LSA_TRUST_ATTRIBUTE_FOREST_TRANSITIVE | lsa.LSA_TRUST_ATTRIBUTE_USES_RC4_ENCRYPTION +info.trust_attributes = lsa.LSA_TRUST_ATTRIBUTE_FOREST_TRANSITIVE try: dname = lsa.String() -- 1.7.11.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] [WIP] Firefox extension
Simo Sorce wrote: On Fri, 2012-10-05 at 17:33 +0300, Alexander Bokovoy wrote: On Fri, 05 Oct 2012, Endi Sukma Dewata wrote: On 10/5/2012 8:56 AM, Alexander Bokovoy wrote: On Thu, 04 Oct 2012, Petr Vobornik wrote: On 10/03/2012 04:19 PM, Simo Sorce wrote: On Wed, 2012-10-03 at 15:50 +0200, Petr Vobornik wrote: As Alexander proposed in other channel. I will remove the removal of configure.jar and offer the old configuration method if user is using FF < 4 so we don't have to make the extension compatible with this ancient version. It will be done this way: If FF < 4 is detected: * in browserconfig.html steps 2 and 3 will be grayed-out and replaced with step 2a with a link to ssbrowser.html and a description explaining the problem * ssbrowser.html will be enhanced by steps for autoconfiguration of FF < 4. We can also show the steps in browserconfig, but I want to have it somehow available even if user is not using FF<4 to keep general awareness about the problem and also to be usable if version detection fails. Other possible problem with steps in browserconfig is different styles of buttons (to keep the same styles we would have to include same css files and jquery.js to configure.jar, which I don't want to do). Excellent plan, we should try to reduce the amount of work, and letting old browsers use the old method is perfectly fine. If FF15 is the only browser that fails with the old method I would even go as far as testing exclusively with FF15 and have anything < FF15 use the old method. Simo. Updated patches attached. browserconfig.html points to older config method for versions prior to 15. The extension is theoretically compatible with FF3.6 and then FF10 and later. There is a problem for FF4-9 with loading strings from .properties file. FF3.6 is working because it doesn't use bootstraping. I also notice that we have an existing issue when navigating to config pages right away before accepting any certificate. Those pages are using some resorces from /ui/ folder which is redirected to https. These resources are not loaded because certificate isn't imported. If user is going straight for Web UI, he won't encounter this issue, but someone might. I tested this patchset and apart from the non-obvious extension description displayed when installing it, which is based on a certificate, everything is great. ACK. It works for me too. Just some questions: 1. It looks like the Firefox is limited to version 10 to 15 in install/ffextension/install.rdf. Do we need the upper limit? 10.0 15.0.* 2. In install/html/ssbrowser.html the step 5 is optional. Should we explain what's that for or why we need it? General users could be confused and stuck if they are given choices that they don't understand. It's probably better to make it a required step if it doesn't cause any problem. 5. Optional: Repeat the above procedure for the network.negotiate-auth.delegation-uris entry, using the same domain. delegation-uris setting should be removed. It is not needed since we started using s4u2proxy mechanism. Yes and we removed it because it is potentially a dangerous setting. It should be generally discouraged and enabled only for specific fqdn's not wildcard ones in future. Simo. I've pushed these 4 patches to master and ipa-3-0. Petr, please submit a patch to remove/clarify references to delegation-uris setting. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Automatic keytab upgrade
On Mon, 2012-09-17 at 11:04 +0930, William Brown wrote: > Hi, > > In freeipa, we export a keytab for a service. Say we upgrade our freeipa > install, and the newer version of MIT kerberos supports a stronger > encryption type on the KDC. Does freeipa automatically refresh the > keytabs of hosts / services with a new keytab that also contains these > stronger encryption types? Does this matter if it does / doesn't happen? No FreeIPA can't, and it does matter, because FreeIPA doesn't know if the service actually does know how to use new encryption types, but most importantly FreeIPa does not have a mechanism to 'push' this change to the host. Keytabs contains shared (between KDC and service) secrets, so they have to be kept consistent and in sync between the KDC and the service, if the KDC unilaterally changes a keytab the service can't see the changes reflected in its keytab, and when a ticket will come in with the new encryption type it will have no key to decrypt it. Also even if we could somehow push an updated keytab, then if the service doesn't support the new encryption types the outcome would be that authentication would be broken, as the KDC would use the strongest enctype to encrypt tickets sent to clients and the service wouldn't be able to decrypt them even if it has a key. HTH, 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] [WIP] Firefox extension
On 10/05/2012 05:30 PM, Rob Crittenden wrote: Simo Sorce wrote: On Fri, 2012-10-05 at 17:33 +0300, Alexander Bokovoy wrote: On Fri, 05 Oct 2012, Endi Sukma Dewata wrote: On 10/5/2012 8:56 AM, Alexander Bokovoy wrote: On Thu, 04 Oct 2012, Petr Vobornik wrote: On 10/03/2012 04:19 PM, Simo Sorce wrote: On Wed, 2012-10-03 at 15:50 +0200, Petr Vobornik wrote: As Alexander proposed in other channel. I will remove the removal of configure.jar and offer the old configuration method if user is using FF < 4 so we don't have to make the extension compatible with this ancient version. It will be done this way: If FF < 4 is detected: * in browserconfig.html steps 2 and 3 will be grayed-out and replaced with step 2a with a link to ssbrowser.html and a description explaining the problem * ssbrowser.html will be enhanced by steps for autoconfiguration of FF < 4. We can also show the steps in browserconfig, but I want to have it somehow available even if user is not using FF<4 to keep general awareness about the problem and also to be usable if version detection fails. Other possible problem with steps in browserconfig is different styles of buttons (to keep the same styles we would have to include same css files and jquery.js to configure.jar, which I don't want to do). Excellent plan, we should try to reduce the amount of work, and letting old browsers use the old method is perfectly fine. If FF15 is the only browser that fails with the old method I would even go as far as testing exclusively with FF15 and have anything < FF15 use the old method. Simo. Updated patches attached. browserconfig.html points to older config method for versions prior to 15. The extension is theoretically compatible with FF3.6 and then FF10 and later. There is a problem for FF4-9 with loading strings from .properties file. FF3.6 is working because it doesn't use bootstraping. I also notice that we have an existing issue when navigating to config pages right away before accepting any certificate. Those pages are using some resorces from /ui/ folder which is redirected to https. These resources are not loaded because certificate isn't imported. If user is going straight for Web UI, he won't encounter this issue, but someone might. I tested this patchset and apart from the non-obvious extension description displayed when installing it, which is based on a certificate, everything is great. ACK. It works for me too. Just some questions: 1. It looks like the Firefox is limited to version 10 to 15 in install/ffextension/install.rdf. Do we need the upper limit? 10.0 15.0.* My understanding is that maxversion represents maximum tested version. [https://developer.mozilla.org/en-US/docs/Install_Manifests] but the document doesn't say if the extension stops being installable on newer versions. I tried maxversion=14.0.* on FF15 and it worked. 2. In install/html/ssbrowser.html the step 5 is optional. Should we explain what's that for or why we need it? General users could be confused and stuck if they are given choices that they don't understand. It's probably better to make it a required step if it doesn't cause any problem. 5. Optional: Repeat the above procedure for the network.negotiate-auth.delegation-uris entry, using the same domain. delegation-uris setting should be removed. It is not needed since we started using s4u2proxy mechanism. Yes and we removed it because it is potentially a dangerous setting. It should be generally discouraged and enabled only for specific fqdn's not wildcard ones in future. Simo. I've pushed these 4 patches to master and ipa-3-0. Petr, please submit a patch to remove/clarify references to delegation-uris setting. Patch attached. rob -- -- Petr Vobornik >From 89279cf52e12fed1cde7bb5bac248e975ec36e7a Mon Sep 17 00:00:00 2001 From: Petr Vobornik Date: Fri, 5 Oct 2012 17:39:18 +0200 Subject: [PATCH] Removal of delegation-uris instruction from browser config Delegation is not needed since support of s4u2proxy mechanism. https://fedorahosted.org/freeipa/ticket/3094 --- install/html/ssbrowser.html | 1 - 1 file changed, 1 deletion(-) diff --git a/install/html/ssbrowser.html b/install/html/ssbrowser.html index 285b1d1239288badca56ab29c4694c6722fdd534..9e17b5fb04951ad8d05077a62ddb809441c58104 100644 --- a/install/html/ssbrowser.html +++ b/install/html/ssbrowser.html @@ -63,7 +63,6 @@ 2. In the Filter field, type negotiate to restrict the list of options. 3. Double-click the network.negotiate-auth.trusted-uris entry to display the Enter string value dialog box. 4. Enter the name of the domain against which you want to authenticate, for example, .example.com. - 5. Optional: Repeat the above procedure for the network.negotiate-auth.delegation-uris entry, using the same domain. You are all set. -- 1.7.11.4
Re: [Freeipa-devel] [PATCH] 1059 single CRL generator
Martin Kosek wrote: On 10/04/2012 06:17 PM, Rob Crittenden wrote: This changes the way IPA generates CRLs for new installs only. The first master installed is configured as the CRL generator. An entry is added to cn=masters that designates it. When a replica is installed it queries this entry so it knows where to forward CRL requests. CRL files are not available on cloned CAs (so /ipa/crl will return not found). It is possible to get a CRL directly from the clone CA via http://:9180/ca/ee/ca/getCRL?op=getCRL&crlIssuingPoint=MasterCRL rob I tested new IPA server + replica with your patch and the CRL was now generated only on the CRL master. I also tried OCSP (though this may not be relevant) and it worked for me too. 1) I do not understand the following block in set_crl_master(self, suffix): +try: +self.admin_conn.addEntry(entry) +except ldap.ALREADY_EXISTS, e: +root_logger.debug("failed to set CA as CRL generator") +raise e - when we hit ldap.ALREADY_EXISTS, we are actually OK because cn=CRL is set, right? - AFAIK, addEntry should return our errors, i.e. errors.DuplicateEntry - s/raise e/raise/ I think you may have wanted to rather catch for more general LDAP error and then report a real error and not just a debug note. I just wanted to flag this error specifically. It should theoretically never happen because we only do this on new installs. 2) In get_crl_master: +except Exception, e: +root_logger.debug("Could not connect to the Directory Server on %s: %s" % (master_host, str(e))) +raise e s/raise e/raise/ +except errors.NotFound, e: +root_logger.debug("failed to find CA CRL generator") +self.crl_master = None - e is actually not used, "except errors.NotFound" would be enough Yeah, I'll remove that. I initially was raising an exception instead of setting crl_master to None, but that wasn't such a nice experience. 3) Majorish issue I hit with the actual CRL publishing on our server (F17). I always get 403 Forbidden error when trying to download CRL from the CRL master: # wget --ca-certificate /etc/ipa/ca.crt https://`hostname`/ipa/crl/MasterCRL.bin --2012-10-05 03:32:58-- https://vm-120.idm.lab.bos.redhat.com/ipa/crl/MasterCRL.bin Resolving vm-120.idm.lab.bos.redhat.com... 10.16.78.120 Connecting to vm-120.idm.lab.bos.redhat.com|10.16.78.120|:443... connected. HTTP request sent, awaiting response... 403 Forbidden 2012-10-05 03:32:58 ERROR 403: Forbidden. I tracked the problem down to too strict permission on /var/lib/pki-ca directory which is being published by httpd which does not have access to it: # ll /var/lib/pki-ca drwxrwx---. 11 pkiuser pkiuser 4096 Oct 5 03:00 pki-ca When I fixed the permission: # chmod o+x /var/lib/pki-ca/ I was able to get pass the Forbidden error and actually retrieved the CRL. Adding Ade on CC list to follow on this permission issue. I was thinking about usability of this new approach, we may want to make user life easier in a perspective of CRL master managing. I have following enhancements in mind: - mark CRL master in ipa-replica-manage list, or ipa-csreplica-manage list: # ipa-csreplica-manage list Directory Manager password: vm-065.idm.lab.bos.redhat.com: master [CRL] vm-120.idm.lab.bos.redhat.com: master - when removing master with CRL by "ipa-replica-manage del" we should warn user and inform him what he should do next to amend the situation. I am thinking about 2 new commands for ipa-csreplica-manage: * ipa-csreplica-manage crl-promote - promote current master as the new CRL master, enable CRL generation in CS.cfg, mark master as the new CRL master in cn=masters * ipa-csreplica-manage crl-update - update CS.cfg of current CA replica and point master.ca.agent.* to current CRL master I can work on those enhancements if we agree on them. I think the display and warning parts would be quite easy. The promotion a bit more challenging and invasive. I think documentation would be adequate for a first stab a this. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] [WIP] Firefox extension
Petr Vobornik wrote: On 10/05/2012 05:30 PM, Rob Crittenden wrote: Simo Sorce wrote: On Fri, 2012-10-05 at 17:33 +0300, Alexander Bokovoy wrote: On Fri, 05 Oct 2012, Endi Sukma Dewata wrote: On 10/5/2012 8:56 AM, Alexander Bokovoy wrote: On Thu, 04 Oct 2012, Petr Vobornik wrote: On 10/03/2012 04:19 PM, Simo Sorce wrote: On Wed, 2012-10-03 at 15:50 +0200, Petr Vobornik wrote: As Alexander proposed in other channel. I will remove the removal of configure.jar and offer the old configuration method if user is using FF < 4 so we don't have to make the extension compatible with this ancient version. It will be done this way: If FF < 4 is detected: * in browserconfig.html steps 2 and 3 will be grayed-out and replaced with step 2a with a link to ssbrowser.html and a description explaining the problem * ssbrowser.html will be enhanced by steps for autoconfiguration of FF < 4. We can also show the steps in browserconfig, but I want to have it somehow available even if user is not using FF<4 to keep general awareness about the problem and also to be usable if version detection fails. Other possible problem with steps in browserconfig is different styles of buttons (to keep the same styles we would have to include same css files and jquery.js to configure.jar, which I don't want to do). Excellent plan, we should try to reduce the amount of work, and letting old browsers use the old method is perfectly fine. If FF15 is the only browser that fails with the old method I would even go as far as testing exclusively with FF15 and have anything < FF15 use the old method. Simo. Updated patches attached. browserconfig.html points to older config method for versions prior to 15. The extension is theoretically compatible with FF3.6 and then FF10 and later. There is a problem for FF4-9 with loading strings from .properties file. FF3.6 is working because it doesn't use bootstraping. I also notice that we have an existing issue when navigating to config pages right away before accepting any certificate. Those pages are using some resorces from /ui/ folder which is redirected to https. These resources are not loaded because certificate isn't imported. If user is going straight for Web UI, he won't encounter this issue, but someone might. I tested this patchset and apart from the non-obvious extension description displayed when installing it, which is based on a certificate, everything is great. ACK. It works for me too. Just some questions: 1. It looks like the Firefox is limited to version 10 to 15 in install/ffextension/install.rdf. Do we need the upper limit? 10.0 15.0.* My understanding is that maxversion represents maximum tested version. [https://developer.mozilla.org/en-US/docs/Install_Manifests] but the document doesn't say if the extension stops being installable on newer versions. I tried maxversion=14.0.* on FF15 and it worked. 2. In install/html/ssbrowser.html the step 5 is optional. Should we explain what's that for or why we need it? General users could be confused and stuck if they are given choices that they don't understand. It's probably better to make it a required step if it doesn't cause any problem. 5. Optional: Repeat the above procedure for the network.negotiate-auth.delegation-uris entry, using the same domain. delegation-uris setting should be removed. It is not needed since we started using s4u2proxy mechanism. Yes and we removed it because it is potentially a dangerous setting. It should be generally discouraged and enabled only for specific fqdn's not wildcard ones in future. Simo. I've pushed these 4 patches to master and ipa-3-0. Petr, please submit a patch to remove/clarify references to delegation-uris setting. Patch attached. rob ACK, pushed to master and ipa-3-0 rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1059 single CRL generator
On Fri, 2012-10-05 at 16:45 +0200, Martin Kosek wrote: > On 10/05/2012 10:59 AM, Martin Kosek wrote: > > On 10/04/2012 06:17 PM, Rob Crittenden wrote: > >> This changes the way IPA generates CRLs for new installs only. > >> > >> The first master installed is configured as the CRL generator. An entry is > >> added to cn=masters that designates it. > >> > >> When a replica is installed it queries this entry so it knows where to > >> forward > >> CRL requests. CRL files are not available on cloned CAs (so /ipa/crl will > >> return not found). It is possible to get a CRL directly from the clone CA > >> via > >> http://:9180/ca/ee/ca/getCRL?op=getCRL&crlIssuingPoint=MasterCRL > >> > >> rob > > ... > > > 3) Majorish issue I hit with the actual CRL publishing on our server (F17). > > I > > always get 403 Forbidden error when trying to download CRL from the CRL > > master: > > > > # wget --ca-certificate /etc/ipa/ca.crt > > https://`hostname`/ipa/crl/MasterCRL.bin > > --2012-10-05 03:32:58-- > > https://vm-120.idm.lab.bos.redhat.com/ipa/crl/MasterCRL.bin > > Resolving vm-120.idm.lab.bos.redhat.com... 10.16.78.120 > > Connecting to vm-120.idm.lab.bos.redhat.com|10.16.78.120|:443... connected. > > HTTP request sent, awaiting response... 403 Forbidden > > 2012-10-05 03:32:58 ERROR 403: Forbidden. > > > > I tracked the problem down to too strict permission on /var/lib/pki-ca > > directory which is being published by httpd which does not have access to > > it: > > > > # ll /var/lib/pki-ca > > > > drwxrwx---. 11 pkiuser pkiuser 4096 Oct 5 03:00 pki-ca > > > > When I fixed the permission: > > # chmod o+x /var/lib/pki-ca/ > > > > I was able to get pass the Forbidden error and actually retrieved the CRL. > > Adding Ade on CC list to follow on this permission issue. > > FYI - I filed a ticket for this issue: > https://fedorahosted.org/freeipa/ticket/3144 > > I plan to simply fix permission on /var/lib/pki-ca/ in a similar way we do for > /var/lib/pki-ca/publish/ > Sorry, but why do you need permissions on /var/lib/pki-ca? Aren't permissions on /var/lib/pki-ca/publish sufficient? > Martin > > ___ > 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
Re: [Freeipa-devel] [PATCH] 0082/0083 Handle NotFound exception when establishing trust
On 10/05/2012 03:24 PM, Alexander Bokovoy wrote: On Fri, 05 Oct 2012, Petr Vobornik wrote: On 10/04/2012 05:06 PM, Alexander Bokovoy wrote: Hi, two attached patches attempt to solve https://fedorahosted.org/freeipa/ticket/3103 We cannot make educated guess where trusted domain's DNS server is located as we ended up with NotFound exception precisely because we were unable to discover trusted domain's domain controller location. Thus, all we can do is to suggest ways to fix the issue. Since suggestion becomes relatively long (multi-line, at least), it creates few issues for current exception error message handling: - root_logger does not use textui to format output - textui is only printing to standard output - multi-line error messages thus become non-wrapped - localizing such messages would become a harder context-wise. Web UI is showing error message as a single paragraph (), all multi-line markup would be lost. In the end, I decided to support list-based multi-line error messages in PublicError class. Its constructor will join all list-based arguments into single string per argument (first patch). In Web UI I've added special text processing of error messages that splits message into multiple lines and wraps those which start with a TAB symbol into 'error-message-hinted' span to visually separate it from the rest of error message. Trust code uses this to display suggested CLI command to set up DNS forwarder (second patch). In the end, CLI shows such error message fine (note tabulated CLI command): --- # ipa trust-add --type=ad --admin Administrator@ad.local1 --password ad.local1 Active directory domain administrator's password: ipa: ERROR: Unable to resolve domain controller for 'ad.local1' domain. IPA manages DNS, please configure forwarder to 'ad.local1' domain by using following CLI command. Please replace DNS_SERVER and IP_ADDRESS by name and address of the domain's name server: ipa dnszone-add ad.local1 --name-server=DNS_SERVER --admin-email='hostmaster@ad.local1' --force --forwarder=IP_ADDRESS --forward-policy=only When using Web UI, please create DNS zone for domain 'ad.local1' first and then set forwarder and forward policy --- Web UI looks like this: http://abbra.fedorapeople.org/.paste/ui.png You have undeclared identifier: lines. I recommend to run `jsl -conf jsl.conf` command in install/ui folder to prevent these issues. Fixed. I'm not convinced that "beautify" call should be in command object. I would rather see it in error_dialog. I moved everything to error_dialog and used $() selectors instead of directly playing with text. 1) +var error_message = $('', {}); I would rather see a as a container. Span is and should contain only inline elements. 2) var line_span = $('', { class: 'error-message-hinted', text: lines[i].substr(1) }).appendTo(container); Why do you use for hinted message and for other lines? Shouldn't we use for both? 3) var line_span is declared twice. JS use function scope, not block scope. I'm not sure that all tabbed error text should be red. But I don't recall any other usage so it's probably OK. We don't have any, this is first one. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1059 single CRL generator
On Fri, 2012-10-05 at 12:19 -0400, Ade Lee wrote: > On Fri, 2012-10-05 at 16:45 +0200, Martin Kosek wrote: > > On 10/05/2012 10:59 AM, Martin Kosek wrote: > > > On 10/04/2012 06:17 PM, Rob Crittenden wrote: > > >> This changes the way IPA generates CRLs for new installs only. > > >> > > >> The first master installed is configured as the CRL generator. An entry > > >> is > > >> added to cn=masters that designates it. > > >> > > >> When a replica is installed it queries this entry so it knows where to > > >> forward > > >> CRL requests. CRL files are not available on cloned CAs (so /ipa/crl will > > >> return not found). It is possible to get a CRL directly from the clone > > >> CA via > > >> http://:9180/ca/ee/ca/getCRL?op=getCRL&crlIssuingPoint=MasterCRL > > >> > > >> rob > > > > ... > > > > > 3) Majorish issue I hit with the actual CRL publishing on our server > > > (F17). I > > > always get 403 Forbidden error when trying to download CRL from the CRL > > > master: > > > > > > # wget --ca-certificate /etc/ipa/ca.crt > > > https://`hostname`/ipa/crl/MasterCRL.bin > > > --2012-10-05 03:32:58-- > > > https://vm-120.idm.lab.bos.redhat.com/ipa/crl/MasterCRL.bin > > > Resolving vm-120.idm.lab.bos.redhat.com... 10.16.78.120 > > > Connecting to vm-120.idm.lab.bos.redhat.com|10.16.78.120|:443... > > > connected. > > > HTTP request sent, awaiting response... 403 Forbidden > > > 2012-10-05 03:32:58 ERROR 403: Forbidden. > > > > > > I tracked the problem down to too strict permission on /var/lib/pki-ca > > > directory which is being published by httpd which does not have access to > > > it: > > > > > > # ll /var/lib/pki-ca > > > > > > drwxrwx---. 11 pkiuser pkiuser 4096 Oct 5 03:00 pki-ca > > > > > > When I fixed the permission: > > > # chmod o+x /var/lib/pki-ca/ > > > > > > I was able to get pass the Forbidden error and actually retrieved the CRL. > > > Adding Ade on CC list to follow on this permission issue. > > > > FYI - I filed a ticket for this issue: > > https://fedorahosted.org/freeipa/ticket/3144 > > > > I plan to simply fix permission on /var/lib/pki-ca/ in a similar way we do > > for > > /var/lib/pki-ca/publish/ > > > > Sorry, but why do you need permissions on /var/lib/pki-ca? Aren't > permissions on /var/lib/pki-ca/publish sufficient? Ade, on unix filesystems you cannot traverse a directory path if you do not have permission all the way through. If 'others' can't access /var/lib/pki-ca they will never even reach the point where they can see that they have access to a subdirectory. The very minimum permission you need to be able to traverse directories is to have the 'traverse' (x) bit set. (note that you do not need the read (r) bit set just to traverse). 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] 75-78 Add fallback group
On Fri, Oct 05, 2012 at 09:45:58AM -0400, Simo Sorce wrote: > On Fri, 2012-10-05 at 16:27 +0300, Alexander Bokovoy wrote: > > On Tue, 02 Oct 2012, Simo Sorce wrote: > > >On Tue, 2012-10-02 at 21:29 +0200, Sumit Bose wrote: > > >> Hi, > > >> > > >> this patch should fix https://fedorahosted.org/freeipa/ticket/2955 by > > >> adding a fallback group as described in comment 2 of the ticket in > > >> ipa-adtrust-install. > > >> > > >> If you prefer to use a different kind of group I can change the patch > > >> accordingly. > > Patch works for me, so ACK except the group name. > > > > >Yes I think we should use a more natural group name. In my recent > > >testing I have been using the name 'Trust Users' that pairs well with > > >another group we create called 'Trust Admins'. But I am open to > > >suggestions on a better name, 'Domain Users' may be better if we really > > >want to associate the wellknown SID to this group. > > I'm fine with 'Trust Users'. > > > > >On the SID side I wonder if using the wellknown 'Domain Users' SID is > > >the right thing to do. I do not see any special reasons why it shouldn't > > >but I also do not have any special reason why we should. > > >Anyone can think of any pros/cons of doing that ? > > Since it only has special meaning within the same domain and we are not > > using it for anything, it should be fine. > > Well it has a special meaning for samba servers ... we may have a few in > the IPA domain. > I think using that SID is fine but I think then the name 'Trust Users' > could be confusing to users looking at permissions. > > We also need to make the group a posix group btw, because samba uses the > Primary Group SID for users and if it can't be resolved to uid/gids it > will probably prevent authentication. > > What about calling it 'Default SMB Group' or similar ? > > Simo. ok new version attached. The group is now a Posix with an automatically assigned SID called 'default_smb_group' and in the protected list. bye, Sumit > > -- > Simo Sorce * Red Hat, Inc * New York > From f48001acd9b8b6719e118fbc620204a788378922 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Tue, 18 Sep 2012 11:32:10 +0200 Subject: [PATCH] ipa-adtrust-install: Add fallback group --- ipalib/plugins/group.py | 2 +- ipaserver/install/adtrustinstance.py | 104 +++ 2 Dateien geändert, 93 Zeilen hinzugefügt(+), 13 Zeilen entfernt(-) diff --git a/ipalib/plugins/group.py b/ipalib/plugins/group.py index 45758d320bb58f3933938ce579d211a83d56ce09..1611e422bbef3d1ee9c71a819b7f9cc13cd819ab 100644 --- a/ipalib/plugins/group.py +++ b/ipalib/plugins/group.py @@ -107,7 +107,7 @@ Example: ipa group-add-member ad_admins --groups ad_admins_external """) -PROTECTED_GROUPS = (u'admins', u'trust admins') +PROTECTED_GROUPS = (u'admins', u'trust admins', u'default_smb_group') class group(LDAPObject): """ diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py index c44037754dfd74cf6a9ceda4c9f4d2a6a224b1ea..fe44ef36dab1c124c062db84207995adb4649723 100644 --- a/ipaserver/install/adtrustinstance.py +++ b/ipaserver/install/adtrustinstance.py @@ -22,6 +22,7 @@ import errno import ldap import tempfile import uuid +import krbV from ipaserver import ipaldap from ipaserver.install import installutils from ipaserver.install import service @@ -96,9 +97,11 @@ class ADTRUSTInstance(service.Service): ATTR_SID = "ipaNTSecurityIdentifier" ATTR_FLAT_NAME = "ipaNTFlatName" ATTR_GUID = "ipaNTDomainGUID" +ATTR_FALLBACK_GROUP = "ipaNTFallbackPrimaryGroup" OBJC_USER = "ipaNTUserAttrs" OBJC_GROUP = "ipaNTGroupAttrs" OBJC_DOMAIN = "ipaNTDomainAttrs" +FALLBACK_GROUP_NAME = u'Default_SMB_Group' def __init__(self, fstore=None): self.fqdn = None @@ -134,6 +137,18 @@ class ADTRUSTInstance(service.Service): return "S-1-5-21-%d-%d-%d" % (sub_ids[0], sub_ids[1], sub_ids[2]) def __add_admin_sids(self): +""" +The IPA admin and the IPA admins group with get the well knows SIDs +used by AD for the administrator and the administrator group. +Additonally the well know domain users SID will be assinged to a +special fallback group. + +By default new users belong only to a user private group (UPG) and no +other Posix group since ipausers is not a Posix group anymore. To be +able to add a RID to the primary RID attribute in a PAC a fallback +group is added. +""" + admin_dn = DN(('uid', 'admin'), api.env.container_user, self.suffix) admin_group_dn = DN(('cn', 'admins'), api.env.container_group, @@ -163,24 +178,88 @@ class ADTRUSTInstance(service.Service): print "IPA admin group object not found" return -if admin_entry.getValue(self.ATTR_SID) or \ - admin_group_entry.getValue(self.ATTR_SID): -print "Admin S
Re: [Freeipa-devel] [PATCH] [WIP] Firefox extension
On 10/5/2012 10:43 AM, Petr Vobornik wrote: 1. It looks like the Firefox is limited to version 10 to 15 in install/ffextension/install.rdf. Do we need the upper limit? 10.0 15.0.* My understanding is that maxversion represents maximum tested version. [https://developer.mozilla.org/en-US/docs/Install_Manifests] but the document doesn't say if the extension stops being installable on newer versions. I tried maxversion=14.0.* on FF15 and it worked. I see in that page there's a separate parameter for that: strictCompatibility. By default it's false, so we're OK. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 75-78 Add fallback group
On Fri, Oct 05, 2012 at 06:34:25PM +0200, Sumit Bose wrote: > On Fri, Oct 05, 2012 at 09:45:58AM -0400, Simo Sorce wrote: > > On Fri, 2012-10-05 at 16:27 +0300, Alexander Bokovoy wrote: > > > On Tue, 02 Oct 2012, Simo Sorce wrote: > > > >On Tue, 2012-10-02 at 21:29 +0200, Sumit Bose wrote: > > > >> Hi, > > > >> > > > >> this patch should fix https://fedorahosted.org/freeipa/ticket/2955 by > > > >> adding a fallback group as described in comment 2 of the ticket in > > > >> ipa-adtrust-install. > > > >> > > > >> If you prefer to use a different kind of group I can change the patch > > > >> accordingly. > > > Patch works for me, so ACK except the group name. > > > > > > >Yes I think we should use a more natural group name. In my recent > > > >testing I have been using the name 'Trust Users' that pairs well with > > > >another group we create called 'Trust Admins'. But I am open to > > > >suggestions on a better name, 'Domain Users' may be better if we really > > > >want to associate the wellknown SID to this group. > > > I'm fine with 'Trust Users'. > > > > > > >On the SID side I wonder if using the wellknown 'Domain Users' SID is > > > >the right thing to do. I do not see any special reasons why it shouldn't > > > >but I also do not have any special reason why we should. > > > >Anyone can think of any pros/cons of doing that ? > > > Since it only has special meaning within the same domain and we are not > > > using it for anything, it should be fine. > > > > Well it has a special meaning for samba servers ... we may have a few in > > the IPA domain. > > I think using that SID is fine but I think then the name 'Trust Users' > > could be confusing to users looking at permissions. > > > > We also need to make the group a posix group btw, because samba uses the > > Primary Group SID for users and if it can't be resolved to uid/gids it > > will probably prevent authentication. > > > > What about calling it 'Default SMB Group' or similar ? > > > > Simo. > > ok new version attached. The group is now a Posix with an > automatically assigned SID called 'default_smb_group' and in the > protected list. > > bye, > Sumit > > > > -- > > Simo Sorce * Red Hat, Inc * New York > > I found a copy-and-paste issue in the first patch, new version attached. bye, Sumit From a855b06f390791e873cf9ee6b49ec7d7f63733c4 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Tue, 18 Sep 2012 11:32:10 +0200 Subject: [PATCH] ipa-adtrust-install: Add fallback group --- ipalib/plugins/group.py | 2 +- ipaserver/install/adtrustinstance.py | 110 +++ 2 Dateien geändert, 99 Zeilen hinzugefügt(+), 13 Zeilen entfernt(-) diff --git a/ipalib/plugins/group.py b/ipalib/plugins/group.py index 45758d320bb58f3933938ce579d211a83d56ce09..1611e422bbef3d1ee9c71a819b7f9cc13cd819ab 100644 --- a/ipalib/plugins/group.py +++ b/ipalib/plugins/group.py @@ -107,7 +107,7 @@ Example: ipa group-add-member ad_admins --groups ad_admins_external """) -PROTECTED_GROUPS = (u'admins', u'trust admins') +PROTECTED_GROUPS = (u'admins', u'trust admins', u'default_smb_group') class group(LDAPObject): """ diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py index c44037754dfd74cf6a9ceda4c9f4d2a6a224b1ea..3c7aeb578cc7b1062f2b5b78e434a8af69be1aaa 100644 --- a/ipaserver/install/adtrustinstance.py +++ b/ipaserver/install/adtrustinstance.py @@ -22,6 +22,7 @@ import errno import ldap import tempfile import uuid +import krbV from ipaserver import ipaldap from ipaserver.install import installutils from ipaserver.install import service @@ -96,9 +97,11 @@ class ADTRUSTInstance(service.Service): ATTR_SID = "ipaNTSecurityIdentifier" ATTR_FLAT_NAME = "ipaNTFlatName" ATTR_GUID = "ipaNTDomainGUID" +ATTR_FALLBACK_GROUP = "ipaNTFallbackPrimaryGroup" OBJC_USER = "ipaNTUserAttrs" OBJC_GROUP = "ipaNTGroupAttrs" OBJC_DOMAIN = "ipaNTDomainAttrs" +FALLBACK_GROUP_NAME = u'Default_SMB_Group' def __init__(self, fstore=None): self.fqdn = None @@ -134,6 +137,18 @@ class ADTRUSTInstance(service.Service): return "S-1-5-21-%d-%d-%d" % (sub_ids[0], sub_ids[1], sub_ids[2]) def __add_admin_sids(self): +""" +The IPA admin and the IPA admins group with get the well knows SIDs +used by AD for the administrator and the administrator group. +Additonally the well know domain users SID will be assinged to a +special fallback group. + +By default new users belong only to a user private group (UPG) and no +other Posix group since ipausers is not a Posix group anymore. To be +able to add a RID to the primary RID attribute in a PAC a fallback +group is added. +""" + admin_dn = DN(('uid', 'admin'), api.env.container_user, self.suffix) admin_group_dn = DN(('cn', 'admins'), api.env.container_group, @@ -163,24 +178,94 @@ cla
Re: [Freeipa-devel] [PATCH] 75-78 Add fallback group
On Fri, 2012-10-05 at 19:02 +0200, Sumit Bose wrote: > On Fri, Oct 05, 2012 at 06:34:25PM +0200, Sumit Bose wrote: > > On Fri, Oct 05, 2012 at 09:45:58AM -0400, Simo Sorce wrote: > > > On Fri, 2012-10-05 at 16:27 +0300, Alexander Bokovoy wrote: > > > > On Tue, 02 Oct 2012, Simo Sorce wrote: > > > > >On Tue, 2012-10-02 at 21:29 +0200, Sumit Bose wrote: > > > > >> Hi, > > > > >> > > > > >> this patch should fix https://fedorahosted.org/freeipa/ticket/2955 by > > > > >> adding a fallback group as described in comment 2 of the ticket in > > > > >> ipa-adtrust-install. > > > > >> > > > > >> If you prefer to use a different kind of group I can change the patch > > > > >> accordingly. > > > > Patch works for me, so ACK except the group name. > > > > > > > > >Yes I think we should use a more natural group name. In my recent > > > > >testing I have been using the name 'Trust Users' that pairs well with > > > > >another group we create called 'Trust Admins'. But I am open to > > > > >suggestions on a better name, 'Domain Users' may be better if we really > > > > >want to associate the wellknown SID to this group. > > > > I'm fine with 'Trust Users'. > > > > > > > > >On the SID side I wonder if using the wellknown 'Domain Users' SID is > > > > >the right thing to do. I do not see any special reasons why it > > > > >shouldn't > > > > >but I also do not have any special reason why we should. > > > > >Anyone can think of any pros/cons of doing that ? > > > > Since it only has special meaning within the same domain and we are not > > > > using it for anything, it should be fine. > > > > > > Well it has a special meaning for samba servers ... we may have a few in > > > the IPA domain. > > > I think using that SID is fine but I think then the name 'Trust Users' > > > could be confusing to users looking at permissions. > > > > > > We also need to make the group a posix group btw, because samba uses the > > > Primary Group SID for users and if it can't be resolved to uid/gids it > > > will probably prevent authentication. > > > > > > What about calling it 'Default SMB Group' or similar ? > > > > > > Simo. > > > > ok new version attached. The group is now a Posix with an > > automatically assigned SID called 'default_smb_group' and in the > > protected list. > > > > bye, > > Sumit > > > > > > -- > > > Simo Sorce * Red Hat, Inc * New York > > > > > I found a copy-and-paste issue in the first patch, new version attached. > > bye, > Sumit 0075: I think we should default_smb_group -> 'default smb group', Default_SMB_Group -> 'Default SMB Group' etc... not sure why we shouldn't use spaces here ? 'Trust Admins' has spaces we should be consistent and use the same conventions everywhere for builtin groups. The comment in __add_admin_sids() is now wrong, we do not associate the wellknown 'Domain Users' sid to the fallback group. The second phrase of the docstring should simply be dropped. The description attribute of the group_add command in __add_fallback_group() should use 'users' (plural) not 'user' I think. 0076: Ack 0077: I am going to Ack this patch but please open a ticket to fix edtection of UPGs. It's not enough to have uidnumber to match the pgid gidnumber to tell that the group is actually a upg. In cases where admins decide to override the DNA plugin and user their own uid/gid allocation, they may have users that use a real group as primary group (potentially with the same name of the user too). We will need to verify we can use the secondary rid range and to mangle the group name if that happens, but we shouldn't fallback to the primary group fallback I think. However I do not think this is should hold back the patchset, so Ack. Nitpicks faillback -> fallback in get_fallback_group_sid() 0078: Nack, I think you had in mind the fact that the group we were using had no posix IDs, but that should never be the case, samba needs the primary group to be a posix group. So I think we should just drop this patch, we do create the Default_SMB_Group as a posix group. 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] 0082/0083 Handle NotFound exception when establishing trust
On Fri, 05 Oct 2012, Petr Vobornik wrote: On 10/05/2012 03:24 PM, Alexander Bokovoy wrote: On Fri, 05 Oct 2012, Petr Vobornik wrote: On 10/04/2012 05:06 PM, Alexander Bokovoy wrote: Hi, two attached patches attempt to solve https://fedorahosted.org/freeipa/ticket/3103 We cannot make educated guess where trusted domain's DNS server is located as we ended up with NotFound exception precisely because we were unable to discover trusted domain's domain controller location. Thus, all we can do is to suggest ways to fix the issue. Since suggestion becomes relatively long (multi-line, at least), it creates few issues for current exception error message handling: - root_logger does not use textui to format output - textui is only printing to standard output - multi-line error messages thus become non-wrapped - localizing such messages would become a harder context-wise. Web UI is showing error message as a single paragraph (), all multi-line markup would be lost. In the end, I decided to support list-based multi-line error messages in PublicError class. Its constructor will join all list-based arguments into single string per argument (first patch). In Web UI I've added special text processing of error messages that splits message into multiple lines and wraps those which start with a TAB symbol into 'error-message-hinted' span to visually separate it from the rest of error message. Trust code uses this to display suggested CLI command to set up DNS forwarder (second patch). In the end, CLI shows such error message fine (note tabulated CLI command): --- # ipa trust-add --type=ad --admin Administrator@ad.local1 --password ad.local1 Active directory domain administrator's password: ipa: ERROR: Unable to resolve domain controller for 'ad.local1' domain. IPA manages DNS, please configure forwarder to 'ad.local1' domain by using following CLI command. Please replace DNS_SERVER and IP_ADDRESS by name and address of the domain's name server: ipa dnszone-add ad.local1 --name-server=DNS_SERVER --admin-email='hostmaster@ad.local1' --force --forwarder=IP_ADDRESS --forward-policy=only When using Web UI, please create DNS zone for domain 'ad.local1' first and then set forwarder and forward policy --- Web UI looks like this: http://abbra.fedorapeople.org/.paste/ui.png You have undeclared identifier: lines. I recommend to run `jsl -conf jsl.conf` command in install/ui folder to prevent these issues. Fixed. I'm not convinced that "beautify" call should be in command object. I would rather see it in error_dialog. I moved everything to error_dialog and used $() selectors instead of directly playing with text. 1) +var error_message = $('', {}); I would rather see a as a container. Span is and should contain only inline elements. Fixed. 2) var line_span = $('', { class: 'error-message-hinted', text: lines[i].substr(1) }).appendTo(container); Why do you use for hinted message and for other lines? Shouldn't we use for both? Fixed to use everywhere. 3) var line_span is declared twice. JS use function scope, not block scope. Fixed. Thanks for the review. New patch 0082 attached. -- / Alexander Bokovoy >From 5a85259e688aae66d740ba6407e60a0fcc601964 Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy Date: Thu, 4 Oct 2012 15:05:17 +0300 Subject: [PATCH 01/12] support multi-line error messages in exceptions --- install/ui/ipa.css | 9 - install/ui/ipa.js | 31 +-- ipalib/errors.py | 12 +--- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/install/ui/ipa.css b/install/ui/ipa.css index bc971dce4aa3377fc9918ed3c16a89565f505c94..4e51c3051e75846f386910c8998f73db7afbddaa 100644 --- a/install/ui/ipa.css +++ b/install/ui/ipa.css @@ -1112,6 +1112,13 @@ table.kerberos-key-status { background-color: #daa520; } +.error-message-hinted { +color: red; +padding-top: 0.5em; +padding-bottom: 0.5em; +font-family: monospace; +} + /* Table */ table.scrollable thead { @@ -1784,4 +1791,4 @@ form#login { .choice-header { font-weight: bold; -} \ No newline at end of file +} diff --git a/install/ui/ipa.js b/install/ui/ipa.js index 45195bc499f874426020cd400a1ae9a5c1ef5f0f..8f924b9d29262282f3ad771653e64892c2945efc 100644 --- a/install/ui/ipa.js +++ b/install/ui/ipa.js @@ -1419,6 +1419,25 @@ IPA.error_dialog = function(spec) { that.visible_buttons = spec.visible_buttons || ['retry', 'cancel']; }; +that.beautify_message = function(container, message) { +var lines = message.split(/\n/g); +var line_span; +for(var i=0; i', { +class: 'error-message-hinted', +text: lines[i].substr(1) +}).appendTo(container); +}
Re: [Freeipa-devel] [PATCH] Fix up trust attributes on trust-add
On Fri, 05 Oct 2012, Simo Sorce wrote: A onliner but better to have it validated by a second pair of eyes. Yep. Go ahead. The origin of USES_RC4_ENCRYPTION comes from Samba 3 code in net utility that Sumit implemented ~1.5 year ago. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] slow response
On Tue, Sep 11, 2012 at 9:17 PM, Stephen Ingram wrote: > On Tue, Sep 11, 2012 at 7:31 PM, Rob Crittenden wrote: >> Stephen Ingram wrote: >>> >>> On Tue, Aug 7, 2012 at 1:53 PM, Simo Sorce wrote: On Tue, 2012-08-07 at 13:30 -0700, Stephen Ingram wrote: > > On Thu, Aug 2, 2012 at 1:04 PM, Simo Sorce wrote: >> >> Quick heads up in this thread, >> >> apparently we have isolated the issue to libkrb5 and its selinux >> integration. >> I have made the whole delay go away by disabling selinux entirely >> (which >> makes libkrb5 stop trying to do some selinux related operations). >> >> We will be working on a fix to have libkrb5 not cause this delay (F17 >> doesn't have it). >> >> You can follow progress on this bugzilla: >> https://bugzilla.redhat.com/show_bug.cgi?id=845125 As I'm thinking this might also solve my IPA large memory usage issue, I've been following this bug and see there is now a patch for it. I also see it is in QA along with several other IPA-related (and non-IPA-related) Kerberos fixes. I thought at some point an errata release would happen during the RHEL 6.3 time-frame, but as I'm not too familiar with how this works, so I'm not sure. Is this a possibility, or are these being held back for some reason like additional QA time? Steve ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Changes to use a single database for dogtag and IPA
Agreed with your assessment that the --check-ca is not needed, as we will be checking this into 3.1 (and not 3.0). Attached is a patch for the "request object" issue. Basically, the replication was not working because ipa-replica-prepare was passing in the wrong port when creating the replica package. Let me know if you have issues - and remember to add the missing link to the master. That fix to add that link will be checked in today and should be in the dogtag developer nightly build as of tommorow. Ade On Fri, 2012-10-05 at 11:57 +0200, Petr Viktorin wrote: > On 10/04/2012 10:04 PM, Ade Lee wrote: > > Attached is a patch to handle the ipa-replica-conncheck issue. It > > should be applied on top of your patch. > > > > Essentially, the fix is as follows: > > A. If the DS_PORT = 7389, then we pass --check-ca in the > > ipa-replica-conncheck to be executed on the master. > > a1. If the master is ipa 2.x, this will prompt a check for port 7389. > > a2. If the master is ipa 3, this will default to dogtag_ds_port of 7389. > > > > B. Else if DS_PORT = 389, then we pass nothing to the > > ipa-replica-conncheck to be executed on the master. This is because we > > will be checking 389 in any case for the IPA DS check. > > > > Ade > > The code works, I got past the connection check, and now I'm getting the > same internal server error with a clone from IPA 2.2 as with a clone > from "3.0 upgraded from 2.2" (unknown object class "request"). > > However, since with this patch IPA will depend on Dogtag 10, > install_constants.DS_PORT will always be 389, so case A will never > happen. I think the two if-blocks that add the --check-ca can be dropped > entirely. > > Assuming that even a clone from an old instance will use the single DB, > conncheck's --dogtag-master-ds-port option is redundant: iff the server > supports it, it uses port 389 which doesn't need re-checking. > > > > > 0001-Allow-ipa-replica-conncheck-to-work-with-2.2-instanc.patch > > > > > > From a9cd4cb15e6c230e5690f3fa919fda9c5728ee10 Mon Sep 17 00:00:00 2001 > > From: Ade Lee > > Date: Thu, 4 Oct 2012 15:55:29 -0400 > > Subject: [PATCH] Allow ipa-replica-conncheck to work with 2.2 instances > > > > --- > > install/tools/ipa-replica-conncheck | 15 +++ > > 1 file changed, 7 insertions(+), 8 deletions(-) > > > > diff --git a/install/tools/ipa-replica-conncheck > > b/install/tools/ipa-replica-conncheck > > index > > c9fb816be43d873a6ca79396e77270fd0d10aa12..498ef49e84e1dc8325b6fc2d850c8bffb9297e69 > > 100755 > > --- a/install/tools/ipa-replica-conncheck > > +++ b/install/tools/ipa-replica-conncheck > > @@ -282,7 +282,11 @@ def main(): > > > > required_ports = BASE_PORTS > > if options.check_ca: > > -ca_port = CheckedPort(int(options.dogtag_master_ds_port), > > SOCK_STREAM, > > +port_val = 7389 > > +if options.dogtag_master_ds_port: > > +port_val = int(options.dogtag_master_ds_port) > > + > > +ca_port = CheckedPort(port_val, SOCK_STREAM, > > "PKI-CA: Directory Service port") > > required_ports.extend([ca_port]) > > > > @@ -359,11 +363,8 @@ def main(): > > raise RuntimeError("Could not get ticket for master > > server: %s" % stderr) > > > > remote_check_opts = ['--replica %s' % options.hostname] > > -if options.check_ca: > > +if options.check_ca and dogtag.install_constants.DS_PORT == > > 7389: > > remote_check_opts.append('--check-ca') > > -remote_check_opts.extend(["--dogtag-master-ds-port", > > -str(dogtag.install_constants.DS_PORT)]) > > - > > > > print_info("Execute check on remote master") > > > > @@ -387,10 +388,8 @@ def main(): > > print_info("Please run the following command on remote > > master:") > > > > remote_check_opts = ['--replica %s' % options.hostname] > > -if options.check_ca: > > +if options.check_ca and dogtag.install_constants.DS_PORT == > > 7389: > > remote_check_opts.append('--check-ca') > > -remote_check_opts.extend(["--dogtag-master-ds-port", > > -str(dogtag.install_constants.DS_PORT)]) > > > > print_info("/usr/sbin/ipa-replica-conncheck " + " > > ".join(remote_check_opts)) > > time.sleep(3600) > > -- 1.7.12 > > > > >From 424288306c6682fc99ef518b1c11a49880988564 Mon Sep 17 00:00:00 2001 From: Ade Lee Date: Fri, 5 Oct 2012 15:18:50 -0400 Subject: [PATCH] Fix ipa-replica-prepare to include correct port --- install/tools/ipa-replica-prepare | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/install/tools/ipa-replica-prepare b/install/tools/ipa-replica-prepare index 5a566d61df521d3801c41a07753a2af728a7b6e7..80fb8d1de9555ad1ac957560f2d022e7b41adb1d 100755 --- a/install/tools/ipa-replica-prepare +++ b/install/tools/ipa-repli
Re: [Freeipa-devel] slow response
On Fri, Oct 05, 2012 at 12:02:52PM -0700, Stephen Ingram wrote: > As I'm thinking this might also solve my IPA large memory usage issue, > I've been following this bug and see there is now a patch for it. I > also see it is in QA along with several other IPA-related (and > non-IPA-related) Kerberos fixes. That group is currently slated for a later update. > I thought at some point an errata > release would happen during the RHEL 6.3 time-frame, but as I'm not > too familiar with how this works, so I'm not sure. Is this a > possibility, or are these being held back for some reason like > additional QA time? The isolated fix was pushed as RHBA-2012:1294. Cheers, Nalin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1059 single CRL generator
On Fri, 2012-10-05 at 12:26 -0400, Simo Sorce wrote: > On Fri, 2012-10-05 at 12:19 -0400, Ade Lee wrote: > > On Fri, 2012-10-05 at 16:45 +0200, Martin Kosek wrote: > > > On 10/05/2012 10:59 AM, Martin Kosek wrote: > > > > On 10/04/2012 06:17 PM, Rob Crittenden wrote: > > > >> This changes the way IPA generates CRLs for new installs only. > > > >> > > > >> The first master installed is configured as the CRL generator. An > > > >> entry is > > > >> added to cn=masters that designates it. > > > >> > > > >> When a replica is installed it queries this entry so it knows where to > > > >> forward > > > >> CRL requests. CRL files are not available on cloned CAs (so /ipa/crl > > > >> will > > > >> return not found). It is possible to get a CRL directly from the clone > > > >> CA via > > > >> http://:9180/ca/ee/ca/getCRL?op=getCRL&crlIssuingPoint=MasterCRL > > > >> > > > >> rob > > > > > > ... > > > > > > > 3) Majorish issue I hit with the actual CRL publishing on our server > > > > (F17). I > > > > always get 403 Forbidden error when trying to download CRL from the CRL > > > > master: > > > > > > > > # wget --ca-certificate /etc/ipa/ca.crt > > > > https://`hostname`/ipa/crl/MasterCRL.bin > > > > --2012-10-05 03:32:58-- > > > > https://vm-120.idm.lab.bos.redhat.com/ipa/crl/MasterCRL.bin > > > > Resolving vm-120.idm.lab.bos.redhat.com... 10.16.78.120 > > > > Connecting to vm-120.idm.lab.bos.redhat.com|10.16.78.120|:443... > > > > connected. > > > > HTTP request sent, awaiting response... 403 Forbidden > > > > 2012-10-05 03:32:58 ERROR 403: Forbidden. > > > > > > > > I tracked the problem down to too strict permission on /var/lib/pki-ca > > > > directory which is being published by httpd which does not have access > > > > to it: > > > > > > > > # ll /var/lib/pki-ca > > > > > > > > drwxrwx---. 11 pkiuser pkiuser 4096 Oct 5 03:00 pki-ca > > > > > > > > When I fixed the permission: > > > > # chmod o+x /var/lib/pki-ca/ > > > > > > > > I was able to get pass the Forbidden error and actually retrieved the > > > > CRL. > > > > Adding Ade on CC list to follow on this permission issue. > > > > > > FYI - I filed a ticket for this issue: > > > https://fedorahosted.org/freeipa/ticket/3144 > > > > > > I plan to simply fix permission on /var/lib/pki-ca/ in a similar way we > > > do for > > > /var/lib/pki-ca/publish/ > > > > > > > Sorry, but why do you need permissions on /var/lib/pki-ca? Aren't > > permissions on /var/lib/pki-ca/publish sufficient? > > Ade, > on unix filesystems you cannot traverse a directory path if you do not > have permission all the way through. > If 'others' can't access /var/lib/pki-ca they will never even reach the > point where they can see that they have access to a subdirectory. > The very minimum permission you need to be able to traverse directories > is to have the 'traverse' (x) bit set. (note that you do not need the > read (r) bit set just to traverse). > I suspected as much, but wanted to confirm. The decision to publish the CRL to /var/lib/pki-ca/publish is an IPA deployment decision. This is a directory that is created by the IPA install scripts. Other folks who deploy Dogtag may choose to publish elsewhere - like under /var/www/... for instance. A location outside of the pki-ca tree seems like a safer option to me, rather than allowing "other" to step through /var/lib/pki-ca ... In any case, if you do decide to leave the publishing directory in that location, you will need to also set the +x permissions for other on /var/lib/pki-ca in the IPA install scripts. Ade > Simo. > ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 83-84 Fix ipaIDobject usage
Alexander Bokovoy wrote: On Thu, 04 Oct 2012, Sumit Bose wrote: Hi, this is something that felt between the cracks. Some time ago we introduced a new objectclass ipaIDobject to allow objects to have an UID, GID or SID which are basically no users or groups. The DNA plugin should be aware of this new objectclass which is fix by the first patch. The second patch actually use this new objectclass in ipasam. Currenlty ipasam generates a hardcoded SID for the trusted domain user which might lead to confusion. With the second patch the trusted domain user has a proper SID. ACK, works for me. pushed to master and ipa-3-0 rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 75-78 Add fallback group
Simo Sorce wrote: On Fri, 2012-10-05 at 19:02 +0200, Sumit Bose wrote: On Fri, Oct 05, 2012 at 06:34:25PM +0200, Sumit Bose wrote: On Fri, Oct 05, 2012 at 09:45:58AM -0400, Simo Sorce wrote: On Fri, 2012-10-05 at 16:27 +0300, Alexander Bokovoy wrote: On Tue, 02 Oct 2012, Simo Sorce wrote: On Tue, 2012-10-02 at 21:29 +0200, Sumit Bose wrote: Hi, this patch should fix https://fedorahosted.org/freeipa/ticket/2955 by adding a fallback group as described in comment 2 of the ticket in ipa-adtrust-install. If you prefer to use a different kind of group I can change the patch accordingly. Patch works for me, so ACK except the group name. Yes I think we should use a more natural group name. In my recent testing I have been using the name 'Trust Users' that pairs well with another group we create called 'Trust Admins'. But I am open to suggestions on a better name, 'Domain Users' may be better if we really want to associate the wellknown SID to this group. I'm fine with 'Trust Users'. On the SID side I wonder if using the wellknown 'Domain Users' SID is the right thing to do. I do not see any special reasons why it shouldn't but I also do not have any special reason why we should. Anyone can think of any pros/cons of doing that ? Since it only has special meaning within the same domain and we are not using it for anything, it should be fine. Well it has a special meaning for samba servers ... we may have a few in the IPA domain. I think using that SID is fine but I think then the name 'Trust Users' could be confusing to users looking at permissions. We also need to make the group a posix group btw, because samba uses the Primary Group SID for users and if it can't be resolved to uid/gids it will probably prevent authentication. What about calling it 'Default SMB Group' or similar ? Simo. ok new version attached. The group is now a Posix with an automatically assigned SID called 'default_smb_group' and in the protected list. bye, Sumit -- Simo Sorce * Red Hat, Inc * New York I found a copy-and-paste issue in the first patch, new version attached. bye, Sumit 0075: I think we should default_smb_group -> 'default smb group', Default_SMB_Group -> 'Default SMB Group' etc... not sure why we shouldn't use spaces here ? 'Trust Admins' has spaces we should be consistent and use the same conventions everywhere for builtin groups. The comment in __add_admin_sids() is now wrong, we do not associate the wellknown 'Domain Users' sid to the fallback group. The second phrase of the docstring should simply be dropped. The description attribute of the group_add command in __add_fallback_group() should use 'users' (plural) not 'user' I think. 0076: Ack 0077: I am going to Ack this patch but please open a ticket to fix edtection of UPGs. It's not enough to have uidnumber to match the pgid gidnumber to tell that the group is actually a upg. In cases where admins decide to override the DNA plugin and user their own uid/gid allocation, they may have users that use a real group as primary group (potentially with the same name of the user too). We will need to verify we can use the secondary rid range and to mangle the group name if that happens, but we shouldn't fallback to the primary group fallback I think. However I do not think this is should hold back the patchset, so Ack. Nitpicks faillback -> fallback in get_fallback_group_sid() 0078: Nack, I think you had in mind the fact that the group we were using had no posix IDs, but that should never be the case, samba needs the primary group to be a posix group. So I think we should just drop this patch, we do create the Default_SMB_Group as a posix group. Simo. pushed 75-77 to master and ipa-3-0 rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 82 ipa-adtrust-install: print list of needed SRV records
Alexander Bokovoy wrote: On Thu, 04 Oct 2012, Sumit Bose wrote: Hi, this patch should fix the reopend https://fedorahosted.org/freeipa/ticket/3019 . ACK. pushed to master and ipa-3-0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 80 Avoid ldapmodify error messages during ipa-adtrust-install
Alexander Bokovoy wrote: On Thu, 04 Oct 2012, Sumit Bose wrote: On Thu, Oct 04, 2012 at 12:39:07PM +0300, Alexander Bokovoy wrote: On Thu, 04 Oct 2012, Sumit Bose wrote: >On Thu, Oct 04, 2012 at 12:13:57PM +0300, Alexander Bokovoy wrote: >>On Thu, 04 Oct 2012, Sumit Bose wrote: >>>Hi, >>> >>>this patch tries to avoid the ldapmodiy error messages during >>>ipa-adtrust-install by checking if the related object already exists. >>>Fixes https://fedorahosted.org/freeipa/ticket/3012 . >>In general -- ACK for the approach. However, I wonder if you could create >>a helper method that would accept: >> - ldif file name, >> - cn component >> - name of the plugin for the "already configured" message >> >>Then every __add_* method would call simply the helper with appropriate >>arguments. > >yes, I was thinking about the same, but I preferred this solution for >now instead of refactoring the current code. Additionally it would be >nice if the helper method would read the DN from the ldif file to make >the code more robust. Would you mind to open a trac ticket to refactor >this part of the code? I would disagree here. Reading DN from ldif file adds complexity for no real gain. Refactoring this code is really simple -- your patch did already introduced bigger change than that refactoring would make. If you aren't comfort with this suggestion, I can make the change myself. Having these repeated patterns is hurting my eyes :) new version attached. ACK. Thanks! pushed to master and ipa-3-0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel