Re: [Freeipa-devel] [PATCHES] 0016 Fixes for{add, set, del}attr with managed attributes
On 02/29/2012 04:34 PM, Petr Viktorin wrote: On 02/29/2012 03:50 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 02/27/2012 11:03 PM, Rob Crittenden wrote: Petr Viktorin wrote: Patch 16 defers validation & conversion until after {add,del,set}attr is processed, so that we don't search for an integer in a list of strings (this caused ticket #2405), and so that the end result of these operations is validated (#2407). Patch 17 makes these options honor params marked no_create and no_update. https://fedorahosted.org/freeipa/ticket/2405 https://fedorahosted.org/freeipa/ticket/2407 https://fedorahosted.org/freeipa/ticket/2408 NACK on Patch 17 which breaks patch 16. How is patch 16 broken? It works for me. My point is they rely on one another, IMHO, so without 17 the reported problem still exists. *attr is specifically made to be powerful. We don't want to arbitrarily block updating certain values. Noted Not having patch 17 means that the problem reported in 2408 still occurs. It should probably check both the schema and the param to determine if something can have multiple values and reject that way. I see the problem now: the certificate subject base is defined as a multi-value attribute in the LDAP schema. If it's changed to single-value the existing validation should catch it. Also, most of the config attributes should probably be required in the schema. Am I right? I'm a newbie here; what do I need to do when changing the schema? Is there a patch that does something similar I could use as an example? The framework should be able to impose its own single-value will as well. If a Param is designated as single-value the *attr should honor it. Here is the updated patch. Since *attr is powerful enough to modify 'no_update' Params, which CRUDUpdate forgets about, I need to check the params of the LDAPObject itself. Updating schema is a bit of a nasty business right now. See 10-selinuxusermap.update for an example. I'll leave schema changes for after the release, then. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Attached patch includes tests. Note that the test depends on my patches 12-13, which make ipasearchrecordslimit required. -- PetrĀ³ From 590e37a1982966de8a600918cabdddb17adba2e8 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 24 Feb 2012 12:26:28 -0500 Subject: [PATCH] Defer conversion and validation until after --{add,del,set}attr are handled --addattr & friends that modified attributes known to Python sometimes used converted and validated Python values instead of LDAP strings. This caused a problem for --delattr, which searched for a converted integer in a list of raw strings (ticket 2407). With this patch we work on raw strings, converting only when done. Deferring validation ensures the end result is valid, so proper errors are raised instead of failing later (ticket 2405). Tests included. https://fedorahosted.org/freeipa/ticket/2405 https://fedorahosted.org/freeipa/ticket/2407 https://fedorahosted.org/freeipa/ticket/2408 --- ipalib/plugins/baseldap.py | 37 ++--- tests/test_xmlrpc/test_attr.py | 50 2 files changed, 73 insertions(+), 14 deletions(-) diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py index 725704ee0e2d784a1f5ad8691d90888366f8efec..cdfe0fe2b127b886e1889a68fa263208e055db4b 100644 --- a/ipalib/plugins/baseldap.py +++ b/ipalib/plugins/baseldap.py @@ -759,8 +759,6 @@ last, after all sets and adds."""), Convert a string in the form of name/value pairs into a dictionary. The incoming attribute may be a string or a list. -Any attribute found that is also a param is validated. - :param attrs: A list of name/value pairs :param append: controls whether this returns a list of values or a single @@ -776,12 +774,6 @@ last, after all sets and adds."""), if len(value) == 0: # None means "delete this attribute" value = None -if attr in self.params: -try: - value = self.params[attr](value) -except errors.ValidationError, err: -(name, error) = str(err.strerror).split(':') -raise errors.ValidationError(name=attr, error=error) if append and attr in newdict: if type(value) in (tuple,): newdict[attr] += list(value) @@ -885,12 +877,29 @@ last, after all sets and adds."""), # normalize all values changedattrs = setattrs | addattrs | delattrs for attr in changedattrs: -# remove duplicite and invalid values -entry_attrs[attr] = list(set([val for val in entry_attrs[attr] if val])) -if not entry_attrs[attr]: -entry_attrs[attr] = None -
[Freeipa-devel] [PATCH] 226 Improve hostname verification in install tools
Our install tools like ipa-server-install, ipa-replica-{prepare, install} may allow hostnames that do not match the requirements in ipalib. This creates a disconnect and may cause issues when user cannot delete hostnames created by install tools. This patch makes sure that ipalib requirements are applied to install tools hostnames as well. https://fedorahosted.org/freeipa/ticket/2089 >From 3f7c9478f0c2575aaf7078fe133a15aa7ea3a349 Mon Sep 17 00:00:00 2001 From: Martin Kosek Date: Thu, 1 Mar 2012 11:01:45 +0100 Subject: [PATCH] Improve hostname verification in install tools Our install tools like ipa-server-install, ipa-replica-{prepare, install} may allow hostnames that do not match the requirements in ipalib. This creates a disconnect and may cause issues when user cannot delete hostnames created by install tools. This patch makes sure that ipalib requirements are applied to install tools hostnames as well. https://fedorahosted.org/freeipa/ticket/2089 --- ipaserver/install/installutils.py |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py index a9a3ec4318b1381bfe09fda085c330e40ce13c87..3e7ae41b5fdbc11353e43a63424f19fbc331435a 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -34,6 +34,7 @@ from ConfigParser import SafeConfigParser from ipapython import ipautil, dnsclient, sysrestore from ipapython.ipa_log_manager import * +from ipalib.util import validate_hostname # Used to determine install status IPA_MODULES = ['httpd', 'kadmin', 'dirsrv', 'pki-cad', 'pkids', 'install', 'krb5kdc', 'ntpd', 'named', 'ipa_memcached'] @@ -159,6 +160,12 @@ def verify_fqdn(host_name, no_host_dns=False, local_hostname=True): if ipautil.valid_ip(host_name): raise BadHostError("IP address not allowed as a hostname") +try: +# make sure that the host name meets the requirements in ipalib +validate_hostname(host_name) +except ValueError, e: +raise BadHostError("Invalid hostname '%s', %s" % (host_name, unicode(e))) + if local_hostname: try: ex_name = socket.gethostbyaddr(host_name) -- 1.7.7.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers
On 02/29/2012 07:46 PM, Rob Crittenden wrote: Martin Kosek wrote: On Mon, 2012-02-27 at 17:51 +0100, Petr Viktorin wrote: On 02/22/2012 10:41 AM, Petr Viktorin wrote: This fixes https://fedorahosted.org/freeipa/ticket/2071 (Add final debug message in installers). The try/except blocks at the end of installers/management scripts are replaced by a call to a common function, which includes the final message. Obviously the installers still need some more love. This is as far as I got before Martin stopped me, saying I shouldn't change too much before a release :) If it's still too many changes to test, I could just wrap the blocks in some `with add_final_message` block for now, and resubmit this patch after the release. Yeah, this is exactly the kind of changes that can have yet-unseen consequences and I don't like pushing this close to the release. The original ticket just asks for a debug message when the install script ends. If possible, I would really prefer to have some low-risk patch adding just those and leave install script refactoring for next big release, like 3.x. Rob, what's your opinion on this? Martin Yes, I agree. Simpler is better at this point. rob This patch simply wraps the try blocks in a context that logs the final result. Most of the changes are indentation; diff with -w to see the additions. Not sure if this would count as an update or a new patch... -- PetrĀ³ From 2c05094a2b7e272bb08dc2ab24f4b9b97b00b1b7 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 1 Mar 2012 05:29:52 -0500 Subject: [PATCH] Add final debug message in installers Invocation of install tools is wrapped in a context manager that logs the final success or failure. https://fedorahosted.org/freeipa/ticket/2071 --- install/tools/ipa-adtrust-install| 47 +++--- install/tools/ipa-ca-install | 71 +- install/tools/ipa-compat-manage | 43 ++-- install/tools/ipa-compliance | 17 install/tools/ipa-csreplica-manage | 33 install/tools/ipa-dns-install| 47 +++--- install/tools/ipa-ldap-updater | 27 +++-- install/tools/ipa-managed-entries| 35 + install/tools/ipa-nis-manage | 43 ++-- install/tools/ipa-replica-conncheck | 33 install/tools/ipa-replica-install| 71 +- install/tools/ipa-replica-manage | 45 +++-- install/tools/ipa-replica-prepare| 33 install/tools/ipa-server-certinstall | 17 install/tools/ipa-server-install | 66 --- install/tools/ipa-upgradeconfig | 17 +--- ipaserver/install/installutils.py| 16 17 files changed, 348 insertions(+), 313 deletions(-) diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install index 248ea35eaa86dd59ebbc871b86df780cfd71ccf6..c3558fce0cceae879f83b61e0057d14fe42811de 100755 --- a/install/tools/ipa-adtrust-install +++ b/install/tools/ipa-adtrust-install @@ -227,26 +227,27 @@ def main(): return 0 -try: -sys.exit(main()) -except SystemExit, e: -sys.exit(e) -except KeyboardInterrupt: -print "Installation cancelled." -except RuntimeError, e: -print str(e) -except HostnameLocalhost: -print "The hostname resolves to the localhost address (127.0.0.1/::1)" -print "Please change your /etc/hosts file so that the hostname" -print "resolves to the ip address of your network interface." -print "The KDC service does not listen on localhost" -print "" -print "Please fix your /etc/hosts file and restart the setup program" -except Exception, e: -message = "Unexpected error - see ipaserver-install.log for details:\n %s" % str(e) -print message -message = str(e) -for str in traceback.format_tb(sys.exc_info()[2]): -message = message + "\n" + str -root_logger.debug(message) -sys.exit(1) +with installutils.script_context('ipa-adtrust-install'): +try: +sys.exit(main()) +except SystemExit, e: +sys.exit(e) +except KeyboardInterrupt: +print "Installation cancelled." +except RuntimeError, e: +print str(e) +except HostnameLocalhost: +print "The hostname resolves to the localhost address (127.0.0.1/::1)" +print "Please change your /etc/hosts file so that the hostname" +print "resolves to the ip address of your network interface." +print "The KDC service does not listen on localhost" +print "" +print "Please fix your /etc/hosts file and restart the setup program" +except Exception, e: +message = "Unexpected error - see ipaserver-install.log for details:\n %s" % str(e) +print message +message = str(e) +for str in traceback.format_tb(sys.exc_info()[2]): +
Re: [Freeipa-devel] [PATCH] 940 apply some validation to some classes only
On 29.2.2012 19:45, Rob Crittenden wrote: Jan Cholasta wrote: On 20.2.2012 22:56, Rob Crittenden wrote: Rob Crittenden wrote: The variable name rdnattr can be misleading. It is only used to give the name of hte RDN in something that can be renamed. Compare this to something like netgroups where the DN has no visible relationship to the content of the object (ipaUniqueId). Only those objects that define rdnattr get a rename option (look at netgroups, for example). rdnattr is always the primary key but not always defined. It should probably be a boolean instead, rdn_is_primary_key or something a bit more obvious. I can make that change here. rob Updated patch. It seems I broke query a few months ago trying to enforce no white spaces in some names. I did the rdnattr variable rename. Looking back at the changelog this was meant to always match the primary key so lets remove the possibility that it doesn't. By doing it this way we get the pattern for free. rob This is certainly better, but I still have a few concerns: 1. --rename is tied to the RDN change operation. I think this is wrong. --rename should be available for all objects, not only when the object's RDN attribute and primary key attribute are the same (so that we can change the primary key of any object). Similarly, modrdn should be triggered for all objects every time the RDN attribute changes, not only when the object's RDN attribute and primary key attribute are the same (so the DN is properly updated for all objects). I don't know if this is in the scope of this patch, though. Right, not in this scope. An entry where RDN != primary key doesn't need --rename to do a rename, just a mod on whatever its "key" is. We can fake this to have a consistent interface though. Please open a ticket. Well, you have to do it using --setattr, which is not very user-friendly IMO. https://fedorahosted.org/freeipa/ticket/2466 2. In crud.Create/crud.Update, query is set for params which have the ask_create/ask_update flag set. This is an error, as we are obviously not querying LDAP with these params (it seems someone forgot to remove the query=True keyword argument after copy-pasting from crud.Search). Honza Updated ACK. rob Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 227-228 Add last missing bits in new bind-dyndb-ldap
These 2 patches changes the DNS API to support the last missing bits in new bind-dyndb-ldap: 1) Both global and per-zone forwarders now support a conditional custom port (with format "IP_ADDRESS PORT") 2) Missing global configuration options have been added: * idnsforwardpolicy: Default policy for conditional forwarding * idnsallowsyncptr: Allow globaly PTR synchronization for dynamic updates * idnszonerefresh: Default interval between regular polls of the name server for new DNS zones Before these patches are pushed, I will just have to update the minimal bind-dyndb-ldap version (it has not been built yet) which have a full support for these. Martin >From 01a440ac9498cb8597234267410dca8de1edde97 Mon Sep 17 00:00:00 2001 From: Martin Kosek Date: Thu, 1 Mar 2012 11:35:00 +0100 Subject: [PATCH 1/2] Allow port numbers for idnsForwarders Let user enter custom ports for zone conditional forwarders or global forwarders in dnsconfig. Ports can be specified in a standard BIND format: IP_ADDRESS [PORT] https://fedorahosted.org/freeipa/ticket/2462 --- ipalib/plugins/dns.py | 26 ++ 1 files changed, 22 insertions(+), 4 deletions(-) diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index b7f86e20164d88d6d4d2ab0086d2f0cf6baee8c2..04a1f92de95d64f2486e9e22f3590cfa6a9fe70a 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -299,6 +299,24 @@ def _normalize_bind_aci(bind_acis): acis += u';' return acis +def _validate_bind_forwarder(ugettext, forwarder): +ip_address, space, port = forwarder.partition(u' ') + +ip_address_validation = _validate_ipaddr(ugettext, ip_address) + +if ip_address_validation is not None: +return ip_address_validation + +if port: +try: +port = int(port) +if port < 0 or port > 65535: +raise ValueError() +except ValueError: +return _('%(port)s is not a valid port' % dict(port=port)) + +return None + def _domain_name_validator(ugettext, value): try: # Allow domain name which is not fully qualified. These are supported @@ -1540,10 +1558,10 @@ class dnszone(LDAPObject): autofill=True, ), Str('idnsforwarders*', -_validate_ipaddr, +_validate_bind_forwarder, cli_name='forwarder', label=_('Zone forwarders'), -doc=_('A list of zone forwarders'), +doc=_('A list of zone forwarders. A custom port can be specified for each forwarder using a format "IP_ADDRESS PORT"'), csv=True, ), StrEnum('idnsforwardpolicy?', @@ -2477,10 +2495,10 @@ class dnsconfig(LDAPObject): takes_params = ( Str('idnsforwarders*', -_validate_ipaddr, +_validate_bind_forwarder, cli_name='forwarder', label=_('Global forwarders'), -doc=_('A list of global forwarders'), +doc=_('A list of global forwarders. A custom port can be specified for each forwarder using a format "IP_ADDRESS PORT"'), csv=True, ), ) -- 1.7.7.6 >From 1b3ea1be1ecf4f17c6951ba11552d896a7b3d263 Mon Sep 17 00:00:00 2001 From: Martin Kosek Date: Thu, 1 Mar 2012 13:09:20 +0100 Subject: [PATCH 2/2] Add missing global options in dnsconfig Add a support for new global options in bind-dyndb-ldap, that is: * idnsforwardpolicy: Default policy for conditional forwarding * idnsallowsyncptr: Allow globaly PTR synchronization for dynamic updates * idnszonerefresh: Default interval between regular polls of the name server for new DNS zones https://fedorahosted.org/freeipa/ticket/2439 --- API.txt |5 - ipalib/plugins/dns.py| 21 - tests/test_xmlrpc/test_dns_plugin.py |6 +- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/API.txt b/API.txt index 73d115c0548230fa06fb6142474715606568e1f5..e344215bc19c0ed87f01ac3617459cccb4967051 100644 --- a/API.txt +++ b/API.txt @@ -611,8 +611,11 @@ output: Output('summary', (, ), None) output: Output('result', , None) output: Output('value', , None) command: dnsconfig_mod -args: 0,8,3 +args: 0,11,3 option: Str('idnsforwarders', attribute=True, autofill=False, cli_name='forwarder', csv=True, multivalue=True, required=False) +option: StrEnum('idnsforwardpolicy', attribute=True, autofill=False, cli_name='forward_policy', multivalue=False, required=False, values=(u'only', u'first')) +option: Bool('idnsallowsyncptr', attribute=True, autofill=False, cli_name='allow_sync_ptr', multivalue=False, required=False) +option: Int('idnszonerefresh', attribute=True, autofill=False, cli_name='zone_refresh', minvalue=0, multivalue=False, required=False) option: Str('setattr*', cli_name='setattr', exclude='webui') option: Str('addattr*', cli_name='addattr', exclude='webui') option: Str('delattr*', cli_name='delattr', exclude='webu
Re: [Freeipa-devel] [PATCH] 12 When migrating warn user if compat is enabled
On 02/29/2012 05:07 PM, Rob Crittenden wrote: Ondrej Hamada wrote: On 02/28/2012 10:52 PM, Rob Crittenden wrote: Ondrej Hamada wrote: On 02/27/2012 09:47 PM, Rob Crittenden wrote: Ondrej Hamada wrote: On 02/21/2012 02:32 PM, Ondrej Hamada wrote: On 02/20/2012 06:53 PM, Rob Crittenden wrote: Ondrej Hamada wrote: https://fedorahosted.org/freeipa/ticket/2274 Added check into migration plugin to warn user when compat is enabled. If compat is enabled, the migration fails and user is warned that he must turn the compat off or run the script with (the newly introduced) option '--compat'. '--compat' is just a flag, by default set to false. If it is set, the compat check is skipped. Interesting approach. I think this is probably good, preventing migration when the compat plugin is enabled unless you specifically decide to. I think the option may need another name, maybe --with-compat or something. I think in the message we should use "enabled" instead of "on". That is the language of ipa-compat-manage. The migration help should have a discussion of why this is a problem too, and what compat really is (provides a different view of the data to be compatible with non RFC2703bis systems). rob corrected Ondra ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel I forget to update the commit message about the change of flag name. Corrected patch attached. This works ok it just seems to be making an assumption on the client when to print this. I think a similar value like enabled needs to be created to explicitly say why we are returning. rob sorry for that, value created Ondra I think you need to define beter what compat means in the output, it coudl be very confusing. You can return a value for it without testing whether it is actually a problem or not. I think what compat is supposed to mean is "Am I failing because of compat" and not an indication of whether compat is enabled or not. Some documentation at a minimum should be added. It otherwise seems to work ok. rob You could return a value for compat here without I've updated the description of 'compat' value in output and also changed the condition when this value is set to False. Now it is set to False only when the migration fails because of compatibility plugin. Code looks good. I think the error language needs some tweaking. I think the help text should read: The schema compat feature allows IPA to reformat data for systems that do not support RFC2307bis. It is recommended that this be disabled during migration to reduce system overhead. It can be re-enabled after migration. To migrate with it enabled use the "--with-compat" option. I think the client-side error should read: The compat plug-in is enabled. This can increase the memory requirements during migration. Disable the compat plug-in with \'ipa-compat-manage disable\' or re-run this script with \'--with-compat\' option." rob patch attached Ondra -- Regards, Ondrej Hamada FreeIPA team jabber: oh...@jabbim.cz IRC: ohamada From 0e4eba7a7934bd85cbc400637d95d887e34b557f Mon Sep 17 00:00:00 2001 From: Ondrej Hamada Date: Thu, 1 Mar 2012 11:41:53 +0100 Subject: [PATCH] Migration warning when compat enabled Added check into migration plugin to warn user when compat is enabled. If compat is enabled, the migration fails and user is warned that he must turn the compat off or run the script with (the newly introduced) option '--with-compat'. '--with-compat' is new flag. If it is set, the compat status is ignored. https://fedorahosted.org/freeipa/ticket/2274 --- API.txt |4 +++- VERSION |2 +- ipalib/plugins/migration.py | 30 -- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/API.txt b/API.txt index b5f5774b58449dd5496d85117fa6f970ea34662d..d1f7ca6cdd5a2e814c3715f7c2120f782c7dcc46 100644 --- a/API.txt +++ b/API.txt @@ -1893,7 +1893,7 @@ output: Output('summary', (, ), None) output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) output: Output('value', , None) command: migrate_ds -args: 2,15,3 +args: 2,16,4 arg: Str('ldapuri', cli_name='ldap_uri') arg: Password('bindpw', cli_name='password', confirm=False) option: Str('binddn?', autofill=True, cli_name='bind_dn', default=u'cn=directory manager') @@ -1909,11 +1909,13 @@ option: Flag('groupoverwritegid', autofill=True, cli_name='group_overwrite_gid', option: StrEnum('schema?', autofill=True, cli_name='schema', default=u'RFC2307bis', values=(u'RFC2307bis', u'RFC2307')) option: Flag('continue?', autofill=True, default=False) option: Str('basedn?', cli_name='base_dn') +option: Flag('compat?', autofill=True, cli_name='with_compat', default=False) option: Str('exclude_groups*', autofill=True, cli_name='exclude_groups', csv=True, default=()) option: St
[Freeipa-devel] [PATCH] 229 Add help for new structured DNS framework
DNS Test Day shown that the new RR specific DNS options and the concepts behind them may not be easily understood. This patch adds an explanation of the new DNS framework for structured options to make it easier for the user to understand and use the new options. https://fedorahosted.org/freeipa/ticket/2382 >From a2015ea9c8d5ea773604e4ab1801a9725f3c0856 Mon Sep 17 00:00:00 2001 From: Martin Kosek Date: Thu, 1 Mar 2012 14:05:50 +0100 Subject: [PATCH] Add help for new structured DNS framework DNS Test Day shown that the new RR specific DNS options and the concepts behind them may not be easily understood. This patch adds an explanation of the new DNS framework for structured options to make it easier for the user to understand and use the new options. https://fedorahosted.org/freeipa/ticket/2382 --- ipalib/plugins/dns.py | 35 +++ 1 files changed, 31 insertions(+), 4 deletions(-) diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index 2b054c67ea6c1bf74c42aac7a59c9d8daf6365ac..27bf1091f2cd8ad2ea468cb26f4402d9be88b7cf 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -39,6 +39,33 @@ Domain Name System (DNS) Manage DNS zone and resource records. + +USING STRUCTURED PER-TYPE OPTIONS + +There are many structured DNS RR types where DNS data stored in LDAP server +is not just a scalar value, for example an IP address or a domain name, but +a data structure which may be often complex. A good example is a LOC record +[RFC1876] which consists of many mandatory and optional parts (degrees, +minutes, seconds of latitude and longitude, altitude or precision). + +It may be difficult to manipulate such DNS records without making a mistake +and entering an invalid value. DNS module provides an abstraction over these +raw records and allows to manipulate each RR type with specific options. For +each supported RR type, DNS module provides a standard option to manipulate +a raw records with format ---rec, e.g. --mx-rec, and special options +for every part of the RR structure with format ---, e.g. +--mx-preference and --mx-exchanger. + +When adding a record, either RR specific options or standard option for a raw +value can be used, they just should not be combined in one add operation. When +modifying an existing entry, new RR specific options can be used to change +one part of a DNS record, where the standard option for raw value is used +to specify the modified value. The following example demonstrates +a modification of MX record preference form 0 to 1 in a record without +modifying the exchanger: +ipa dnsrecord-mod --mx-rec="0 mx.example.com." --mx-preference=1 + + EXAMPLES: Add new zone: @@ -960,23 +987,23 @@ class LOCRecord(DNSRecord): values=(u'N', u'S',), ), Int('lon_deg', -label=_('Degrees Longtitude'), +label=_('Degrees Longitude'), minvalue=0, maxvalue=180, ), Int('lon_min?', -label=_('Minutes Longtitude'), +label=_('Minutes Longitude'), minvalue=0, maxvalue=59, ), Decimal('lon_sec?', -label=_('Seconds Longtitude'), +label=_('Seconds Longitude'), minvalue='0.0', maxvalue='59.999', precision=3, ), StrEnum('lon_dir', -label=_('Direction Longtitude'), +label=_('Direction Longitude'), values=(u'E', u'W',), ), Decimal('altitude', -- 1.7.7.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0100 Improved usability of login dialog
Usability was improved in Unauthorized/Login dialog. When the dialog is opened a link which switches to login form is focus so user can do following: 1) press enter (login form is displayed and username field is focused ) 2) type username 3) press tab 4) type password 5) press enter this sequence will execute login request. When filling form user can also press 'escape' to go back to previous form state. It's the same as if he would click on the 'back' button. https://fedorahosted.org/freeipa/ticket/2450 -- Petr Vobornik From d05c016da3ba51dbe3362ceeb87336e7f37031b3 Mon Sep 17 00:00:00 2001 From: Petr Vobornik Date: Thu, 1 Mar 2012 13:58:21 +0100 Subject: [PATCH] Improved usability of login dialog Usability was imporved in Unauthorized/Login dialog. When the dialog is opened a link which switches to login form is focus so user can do following: 1) press enter (login form is displayed and username field is focused ) 2) type username 3) press tab 4) type password 5) press enter this sequence will execute login request. When filling form user can also press 'escape' to go back to previous form state. It's the same as if he would click on the 'back' button. https://fedorahosted.org/freeipa/ticket/2450 --- install/ui/dialog.js |3 +++ install/ui/ipa.js| 48 ++-- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/install/ui/dialog.js b/install/ui/dialog.js index 28c71ad540de21f4579651e5428e70e0a3ca6e66..324eb9b93e8557af44a4dbcfe39609a6d149b44e 100644 --- a/install/ui/dialog.js +++ b/install/ui/dialog.js @@ -66,6 +66,8 @@ IPA.dialog = function(spec) { that.title = spec.title; that.width = spec.width || 500; that.height = spec.height; +that.close_on_escape = spec.close_on_escape !== undefined ? +spec.close_on_escape : true; that.widgets = IPA.widget_container(); that.fields = IPA.field_container({ container: that }); @@ -156,6 +158,7 @@ IPA.dialog = function(spec) { that.container.dialog({ title: that.title, modal: true, +closeOnEscape: that.close_on_escape, width: that.width, minWidth: that.width, height: that.height, diff --git a/install/ui/ipa.js b/install/ui/ipa.js index b5a7486d594b867b3a4a8e26b9fda0e4c81705b3..34174c81a9b3fd142301c3ea80b86fb4e98a949f 100644 --- a/install/ui/ipa.js +++ b/install/ui/ipa.js @@ -489,6 +489,7 @@ IPA.command = function(spec) { xhr: xhr, text_status: text_status, error_thrown: error_thrown, +close_on_escape: false, command: that }); @@ -1353,10 +1354,19 @@ IPA.unauthorized_dialog = function(spec) { }).appendTo(that.krb_message_contatiner); text = IPA.get_message('login.form_auth', "form-based authentication"); -$('', { +that.form_auth_link = $('', { text: text, -style: 'cursor:pointer;', -click: that.show_form +href: '#', +click: function() { +that.show_form(); +return false; +}, +keydown: function(event) { +if (event.keyCode === 13) { //enter +that.show_form(); +return false; +} +} }).appendTo(fb_title); fb_title.append('.'); @@ -1368,7 +1378,8 @@ IPA.unauthorized_dialog = function(spec) { that.form = $('', { 'class': 'auth-dialog', -style: 'display: none;' +style: 'display: none;', +keyup: that.on_form_keyup }).appendTo(that.container); var text = IPA.get_message('login.login', "Login"); @@ -1421,20 +1432,45 @@ IPA.unauthorized_dialog = function(spec) { }); }; +that.open = function() { +that.dialog_open(); +that.form_auth_link.focus(); +}; + +that.on_form_keyup = function(event) { + +if (that.switching) { +that.switching = false; +return; +} + +if (event.keyCode === 13) { // enter +that.on_login(); +event.preventDefault(); +} else if (event.keyCode === 27) { // escape +that.on_back(); +event.preventDefault(); +} +}; + that.show_form = function() { +that.switching = true; + that.krb_message_contatiner.css('display', 'none'); that.form.css('display', 'block'); - that.display_buttons(['login', 'back']); + +var user_field = that.fields.get_field('username'); +user_field.widget.focus_input(); }; that.on_back = function() { that.krb_message_contatiner.css('display', 'block'); that.form.css('display', 'none'); - that.display_buttons(['retry']); +that.form_auth_link.focus(
Re: [Freeipa-devel] More types of replica in FreeIPA
On 02/29/2012 04:36 PM, Simo Sorce wrote: On Wed, 2012-02-29 at 16:19 +0100, Ondrej Hamada wrote: Hi everyone, I'm currently working on my thesis. It's objective is $SUBJ and we already have ticket for that: #194. The task is to create two more replica types - the HUB and Consumer. In 389-DS both the HUB and Consumer are read-only. Additionally the HUB can push the data to the Consumers. In case of FreeIPA the server is not only providing data, but also services like CA, NTP, DNS, Kerberos. Therefore I'm kindly asking you for advices and opinions on that: 1. What should be the position of HUB? I mean should it be used as an interconnection between Masters and Consumers only, so that it will be 'hidden' in the topology and only forwards the updates, or should the HUB be also used as a regular Consumer which has additional ability to push the updates further to Consumers/HUBS? I would think that having shadow HUBs would make things more reliable. 2. Which services should be available on HUB and Consumer? I think, the priority of these replicas would be to answer to data request by ipa whatever-(find|show) commands or to provide some LDAP data for email addressing etc. Also it shouldn't cause much trouble to run NTP on Consumer, but what about Kerberos or CA? Is it a good solution to let users authenticate against these replicas? Is it correct to leave classified data like passwords on these replicas? CA's clearly have no place in a read-only replica in my mind. Kerberos stuff is different. The problem with a read-only replica and Kerberos is that krb needs to write data for local user lockouts. Password changes can be avoided by allowing kadmind only on full masters. There is also another angle to consider and that is the Rad-Only Domain Controller (RODC) available in the Microsoft world. This kind of server is even more limited than a read-only replica as it does not contain the full data. To do that it requires quite some tweaking on the KDC component too, so maybe it is out of scope, but it may make sense reading up on what they do to have a sense of the kind of services they enable/disable on read only servers. I've read some articles about RODC and according to them, RODC is supposed to be used in branch offices where could be problem providing physical security of the machine - therefore RODC doesn't contain any passwords or confident data. The authentication requests must be forwarded to regular Domain Controller, but it is also possible to cache some credentials - usually the credentials of users that uses the RODC frequently, so that possible crack of RODC affects only this small group of users. If someone's credentials are not cached and the connection is broken between RODC and DC, he can't log in. RODC also supports read-only DNS. If the consumer should really be read-only, then the RODC way seems to be exactly what we are looking for. Simo. -- Regards, Ondrej Hamada FreeIPA team jabber:oh...@jabbim.cz IRC: ohamada ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0009 Support for IPv6 elements in idnsForwarders attribute
Hello, here is reworked patch for https://fedorahosted.org/bind-dyndb-ldap/ticket/49 . Changes after yesterday's discussion on IRC with Simo and Mkosek: It follows BIND9 syntax for optional specification of port & adds documentation for this new syntax. Petr^2 Spacek On 02/29/2012 05:33 PM, Martin Kosek wrote: I agree that we should keep the BIND syntax and separate port and IP address with a space. We will at least avoid possible issues with IP address decoding in the future. Since this is a new attribute we have a good chance to do changes now so that it is used correctly. I created an upstream ticket to change the behavior and validators in FreeIPA: https://fedorahosted.org/freeipa/ticket/2462 Martin On Wed, 2012-02-29 at 16:44 +0100, Petr Spacek wrote: On 02/29/2012 04:30 PM, Simo Sorce wrote: Either way looks ok to me. I agree that using a space may be less confusing if this syntax never allows to specify multiple addresses. If multiple address can be specified than it may be less ideal to use spaces. Simo. idnsForwarders is multi-value attribute, so each value contain single forwarder address. Petr^2 Spacek On Wed, 2012-02-29 at 15:14 +0100, Petr Spacek wrote: And there is the patch, sorry. Petr^2 On 02/29/2012 03:10 PM, Petr Spacek wrote: Hello, this patch fixes https://fedorahosted.org/bind-dyndb-ldap/ticket/49 , but I want to discuss one (unimplemented) change: I propose a change in (currently very strange) forwarders syntax. Current syntax: [.port] examples: 1.2.3.4 (without optional port) 1.2.3.4.5553 (optional port 5553) A::B (IPv6, without optional port) A::B.5553 :::1.2.3.4 (6to4, without optional port) :::1.2.3.4.5553 (6to4, with optional port 5553) I find this syntax confusing, non-standard and not-typo-proof. IMHO better choice is to follow BIND forwarders syntax: [port ip_port] (port is string delimited with spaces) (From: http://www.zytrax.com/books/dns/ch7/queries.html#forwarders) *Current syntax is not documented*, so probably is not used anywhere. (And DNS server on non-standard port is probably useful only for testing purposes, but it's another story.) What is you opinion? >From 5e4a9b52a6c42b43829e3275ebf6e7572fd00c48 Mon Sep 17 00:00:00 2001 From: Petr Spacek Date: Thu, 1 Mar 2012 15:08:10 +0100 Subject: [PATCH] Add support for IPv6 elements in idnsForwarders attribute and make syntax compatible with BIND9 forwarders. Signed-off-by: Petr Spacek --- NEWS |3 + README|8 ++- src/ldap_helper.c | 148 3 files changed, 100 insertions(+), 59 deletions(-) diff --git a/NEWS b/NEWS index ced6250..a97a5f3 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +[1] Add support for IPv6 elements in idnsForwarders attribute +and make syntax compatible with BIND9 forwarders. + 1.1.0a2 == diff --git a/README b/README index 914abdc..aedb88c 100644 --- a/README +++ b/README @@ -89,8 +89,12 @@ example zone ldif is available in the doc directory. with a valid idnsForwarders attribute. * idnsForwarders - Defines multiple IP addresses (TODO: optional port numbers) to which - queries will be forwarded. + Defines multiple IP addresses to which queries will be forwarded. + It is multi-value attribute: Each IP address (and optional port) has to + be in own value. BIND9 syntax for "forwarders" is required. + Optional port can be specified by adding " port " after IP + address. IPv4 and IPv6 addresses are supported. + Examples: "1.2.3.4" or "1.2.3.4 port 553" or "A::B" or "A::B port 553" 5. Configuration diff --git a/src/ldap_helper.c b/src/ldap_helper.c index d9e8629..69880d0 100644 --- a/src/ldap_helper.c +++ b/src/ldap_helper.c @@ -777,22 +777,24 @@ cleanup: /** - * @brief + * @brief Convert IP address from string to sockaddr. * - * @param nameserver - * @param sa + * Only AF_INET and AF_INET6 are supported. * - * @return + * @param[in] nameserver IP address as string + * @param[out] ss sockaddr with obtained IP address + * + * @return ISC_R_SUCCESS or ISC_R_FAMILYNOSUPPORT, ISC_R_BADADDRESSFORM (ss untouched) */ static isc_result_t -sockaddr_fromchar(char *nameserver, struct sockaddr *sa) +sockaddr_fromchar(const char *nameserver, struct sockaddr_storage *ss) { isc_result_t result = ISC_R_FAILURE; struct addrinfo *ai; struct addrinfo hints; int res; - REQUIRE(sa != NULL); + REQUIRE(ss != NULL); memset(&hints, 0, sizeof(hints)); hints.ai_flags = AI_NUMERICHOST; @@ -800,83 +802,104 @@ sockaddr_fromchar(char *nameserver, struct sockaddr *sa) res = getaddrinfo(nameserver, NULL, &hints, &ai); if (res == 0) { if ((ai->ai_family == AF_INET) || (ai->ai_family == AF_INET6)) { - memcpy(sa, ai->ai_addr, ai->ai_addrlen); + memcpy(ss, ai->ai_addr, ai->ai_addrlen); result = ISC_R_SUCCESS; + } else { + log_bug("Unsupported ai_family type"); + return ISC_R_FAMILYNOSUPPORT; } freeaddrinfo(ai); +
Re: [Freeipa-devel] [PATCH] 971 detect binary LDAP data
On 29.2.2012 15:45, Rob Crittenden wrote: Jan Cholasta wrote: On 28.2.2012 18:58, Rob Crittenden wrote: Jan Cholasta wrote: On 28.2.2012 18:02, Petr Viktorin wrote: On 02/28/2012 04:45 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 02/28/2012 04:02 AM, Rob Crittenden wrote: Petr Viktorin wrote: On 02/27/2012 05:10 PM, Rob Crittenden wrote: Rob Crittenden wrote: Simo Sorce wrote: On Mon, 2012-02-27 at 09:44 -0500, Rob Crittenden wrote: We are pretty trusting that the data coming out of LDAP matches its schema but it is possible to stuff non-printable characters into most attributes. I've added a sanity checker to keep a value as a python str type (treated as binary internally). This will result in a base64 encoded blob be returned to the client. I don't like the idea of having arbitrary binary data where unicode strings are expected. It might cause some unexpected errors (I have a feeling that --addattr and/or --delattr and possibly some plugins might not handle this very well). Wouldn't it be better to just throw away the value if it's invalid and warn the user? This isn't for user input, it is for data stored in LDAP. User's are going to have no way to provide binary data to us unless they use the API themselves in which case they have to follow our rules. Well my point was that --addattr and --delattr cause an LDAP search for the given attribute and plugins might get the result of a LDAP search in their post_callback and I'm not sure if they can cope with binary data. It wouldn't be any different than if we had the value as a unicode. Let's see what happens if the mail attribute of a user contains invalid UTF-8 (ff 62 30 72 6b 65 64): $ ipa user-find jdoe -- 1 user matched -- User login: jdoe First name: John Last name: Doe Home directory: /home/jdoe Login shell: /bin/sh Email address: /2IwcmtlZA== UID: 526 GID: 526 Account disabled: False Password: False Kerberos keys available: False Number of entries returned 1 $ ipa user-mod jdoe --addattr mail=j...@example.com ipa: ERROR: an internal error has occurred The internal error is: Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 302, in wsgi_execute result = self.Command[name](*args, **options) File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 438, in __call__ ret = self.run(*args, **options) File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 696, in run return self.execute(*args, **options) File "/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.py", line 1217, in execute ldap, dn, entry_attrs, attrs_list, *keys, **options File "/usr/lib/python2.7/site-packages/ipalib/plugins/user.py", line 532, in pre_callback entry_attrs['mail'] = self.obj._normalize_email(entry_attrs['mail']) File "/usr/lib/python2.7/site-packages/ipalib/plugins/user.py", line 338, in _normalize_email norm_email.append(m + u'@' + config['ipadefaultemaildomain'][0]) UnicodeDecodeError: 'utf8' codec can't decode byte 0xff in position 0: invalid start byte $ ipa user-mod jdoe --delattr mail=/2IwcmtlZA== ipa: ERROR: mail does not contain '/2IwcmtlZA==' $ ipa user-mod jdoe --delattr mail=`echo 'ff 62 30 72 6b 65 64' | xxd -p -r` ipa: ERROR: UnicodeDecodeError: 'utf8' codec can't decode byte 0xff in position 5: invalid start byte Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 1242, in run sys.exit(api.Backend.cli.run(argv)) File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 1024, in run kw = self.parse(cmd, argv) File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 1049, in parse return dict(self.parse_iter(cmd, kw)) File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 1058, in parse_iter yield (key, self.Backend.textui.decode(value)) File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 136, in decode return value.decode(encoding) File "/usr/lib64/python2.7/encodings/utf_8.py", line 16, in decode return codecs.utf_8_decode(input, errors, True) UnicodeDecodeError: 'utf8' codec can't decode byte 0xff in position 5: invalid start byte ipa: ERROR: an internal error has occurred I'm sure there is a lot more places in the code where things will break when you feed them arbitrary data. We treat the python type str as binary data. Anything that is a str gets based64 encoded before json or xml-rpc transmission. The type unicode is considered a "string" and goes in the clear. We determine what this type should be not from the data but from the schema. This is a big assumption. Hopefully this answer's Petr's point as well. We decided long ago that str means Binary and unicode means String. It is a bit clumsy perhaps python handles it well. It will be more clear when we switch to Python 3.0 and we
Re: [Freeipa-devel] [PATCH] 69 Configure SSH features of SSSD in ipa-client-install
On 29.2.2012 15:00, Martin Kosek wrote: On Wed, 2012-02-29 at 14:44 +0100, Jan Cholasta wrote: On 29.2.2012 14:24, Martin Kosek wrote: On Wed, 2012-02-29 at 10:52 +0100, Jan Cholasta wrote: On 28.2.2012 23:42, Rob Crittenden wrote: Jan Cholasta wrote: Hi, this patch configures the new SSH features of SSSD in ipa-client-install. To test it, you need to have SSSD 1.8.0 installed. Honza Is there a better name for 'GlobalKnownHostsFile2'? What do you mean? The option name or the file name? Either way, I don't think there is a better name. When is PubKeyAgent used?I tried in RHEL 6.2, F-11 and F15-17 and it was an unknown option in all. It's in openssh in RHEL 6.0. Should you test for the existence of /usr/bin/sss_ssh_knownhostsproxy and /usr/bin/sss_ssh_authorizedkeys before setting it in a config file? It depends. Do we want to support clients with SSSD< 1.8.0? How would you recommend testing this? Enroll a client and try to log into the IPA server? To test host authentication, you need an IPA host with SSH public keys set (which is done automatically in ipa-client-install, so any IPA host should work) and try to ssh into that host from other (actually, it can be the same) IPA host. You should not see "The authenticity of host ... can't be estabilished" ssh message. To test user authentication, you need an IPA user with SSH public keys set. To do that, you need to set the public keys using ipa user-mod. You should then be able to authenticate using your private key on any IPA host. rob Honza I get this exception when running ipa-client-install with your patch. # ipa-client-install --enable-dns-updates Discovery was successful! Hostname: vm-138.idm.lab.bos.redhat.com Realm: IDM.LAB.BOS.REDHAT.COM DNS Domain: idm.lab.bos.redhat.com IPA Server: vm-068.idm.lab.bos.redhat.com BaseDN: dc=idm,dc=lab,dc=bos,dc=redhat,dc=com Continue to configure the system with these values? [no]: y User authorized to enroll computers: admin Synchronizing time with KDC... Unable to sync time with IPA NTP server, assuming the time is in sync. Password for ad...@idm.lab.bos.redhat.com: Enrolled in IPA realm IDM.LAB.BOS.REDHAT.COM Created /etc/ipa/default.conf Traceback (most recent call last): File "/usr/sbin/ipa-client-install", line 1514, in sys.exit(main()) File "/usr/sbin/ipa-client-install", line 1501, in main rval = install(options, env, fstore, statestore) File "/usr/sbin/ipa-client-install", line 1326, in install if configure_sssd_conf(fstore, cli_realm, cli_domain, cli_server, options): File "/usr/sbin/ipa-client-install", line 711, in configure_sssd_conf sssdconfig.activate_service('ssh') File "/usr/lib/python2.7/site-packages/SSSDConfig.py", line 1516, in activate_service raise NoServiceError SSSDConfig.NoServiceError SSSD version: sssd-1.8.1-0.20120228T2018Zgit751b121.fc16.x86_64 Martin Does your /etc/sssd/sssd.conf and /usr/share/sssd/sssd.api.conf contain [ssh] section? sssd.api.conf did contain the ssh section: # grep -C 3 ssh /usr/share/sssd/sssd.api.conf # autofs service autofs_negative_timeout = int, None, false [ssh] # ssh service [provider] #Available provider types sssd.conf did not. Either case, we should not crash but handle the issue in some more friendly way. Martin Patch updated with more defensive code. Honza -- Jan Cholasta >From 70358db5818496b8ae77acb7b68d61fa9d237192 Mon Sep 17 00:00:00 2001 From: Jan Cholasta Date: Thu, 16 Feb 2012 04:21:56 -0500 Subject: [PATCH] Configure SSH features of SSSD in ipa-client-install. This requires SSSD 1.8.0. --- freeipa.spec.in |5 +++- ipa-client/ipa-install/ipa-client-install | 36 - 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/freeipa.spec.in b/freeipa.spec.in index 90c8e9f..0889765 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -210,7 +210,7 @@ Requires: libcurl Requires: xmlrpc-c %endif %endif -Requires: sssd >= 1.5.1 +Requires: sssd >= 1.8.0 Requires: certmonger >= 0.26 Requires: nss-tools Requires: bind-utils @@ -675,6 +675,9 @@ fi %ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/ca.crt %changelog +* Thu Mar 1 2012 Jan Cholasta - 2.99.0-21 +- Set min nvr of sssd to 1.8.0 for SSH support + * Wed Feb 29 2012 Petr Vobornik - 2.99.0-20 - Add Web UI logout page diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install index f5c1efe..f4d65b8 100755 --- a/ipa-client/ipa-install/ipa-client-install +++ b/ipa-client/ipa-install/ipa-client-install @@ -709,6 +709,20 @@ def configure_sssd_conf(fstore, cli_realm, cli_domain, cli_server, options): sssdconfig.new_config() try: +sssdconfig.activate_service('ssh') +except SSSDConfig.NoServiceError: +if options.preserve_sssd: +print "Unable to activate the SSH service in existing SSSD config, please activ
Re: [Freeipa-devel] [PATCH] 940 apply some validation to some classes only
Jan Cholasta wrote: On 29.2.2012 19:45, Rob Crittenden wrote: Jan Cholasta wrote: On 20.2.2012 22:56, Rob Crittenden wrote: Rob Crittenden wrote: The variable name rdnattr can be misleading. It is only used to give the name of hte RDN in something that can be renamed. Compare this to something like netgroups where the DN has no visible relationship to the content of the object (ipaUniqueId). Only those objects that define rdnattr get a rename option (look at netgroups, for example). rdnattr is always the primary key but not always defined. It should probably be a boolean instead, rdn_is_primary_key or something a bit more obvious. I can make that change here. rob Updated patch. It seems I broke query a few months ago trying to enforce no white spaces in some names. I did the rdnattr variable rename. Looking back at the changelog this was meant to always match the primary key so lets remove the possibility that it doesn't. By doing it this way we get the pattern for free. rob This is certainly better, but I still have a few concerns: 1. --rename is tied to the RDN change operation. I think this is wrong. --rename should be available for all objects, not only when the object's RDN attribute and primary key attribute are the same (so that we can change the primary key of any object). Similarly, modrdn should be triggered for all objects every time the RDN attribute changes, not only when the object's RDN attribute and primary key attribute are the same (so the DN is properly updated for all objects). I don't know if this is in the scope of this patch, though. Right, not in this scope. An entry where RDN != primary key doesn't need --rename to do a rename, just a mod on whatever its "key" is. We can fake this to have a consistent interface though. Please open a ticket. Well, you have to do it using --setattr, which is not very user-friendly IMO. https://fedorahosted.org/freeipa/ticket/2466 2. In crud.Create/crud.Update, query is set for params which have the ask_create/ask_update flag set. This is an error, as we are obviously not querying LDAP with these params (it seems someone forgot to remove the query=True keyword argument after copy-pasting from crud.Search). Honza Updated ACK. rob Honza Thanks, pushed to master and ipa-2-2 rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 226 Improve hostname verification in install tools
Martin Kosek wrote: Our install tools like ipa-server-install, ipa-replica-{prepare, install} may allow hostnames that do not match the requirements in ipalib. This creates a disconnect and may cause issues when user cannot delete hostnames created by install tools. This patch makes sure that ipalib requirements are applied to install tools hostnames as well. https://fedorahosted.org/freeipa/ticket/2089 ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 12 When migrating warn user if compat is enabled
Ondrej Hamada wrote: On 02/29/2012 05:07 PM, Rob Crittenden wrote: Ondrej Hamada wrote: On 02/28/2012 10:52 PM, Rob Crittenden wrote: Ondrej Hamada wrote: On 02/27/2012 09:47 PM, Rob Crittenden wrote: Ondrej Hamada wrote: On 02/21/2012 02:32 PM, Ondrej Hamada wrote: On 02/20/2012 06:53 PM, Rob Crittenden wrote: Ondrej Hamada wrote: https://fedorahosted.org/freeipa/ticket/2274 Added check into migration plugin to warn user when compat is enabled. If compat is enabled, the migration fails and user is warned that he must turn the compat off or run the script with (the newly introduced) option '--compat'. '--compat' is just a flag, by default set to false. If it is set, the compat check is skipped. Interesting approach. I think this is probably good, preventing migration when the compat plugin is enabled unless you specifically decide to. I think the option may need another name, maybe --with-compat or something. I think in the message we should use "enabled" instead of "on". That is the language of ipa-compat-manage. The migration help should have a discussion of why this is a problem too, and what compat really is (provides a different view of the data to be compatible with non RFC2703bis systems). rob corrected Ondra ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel I forget to update the commit message about the change of flag name. Corrected patch attached. This works ok it just seems to be making an assumption on the client when to print this. I think a similar value like enabled needs to be created to explicitly say why we are returning. rob sorry for that, value created Ondra I think you need to define beter what compat means in the output, it coudl be very confusing. You can return a value for it without testing whether it is actually a problem or not. I think what compat is supposed to mean is "Am I failing because of compat" and not an indication of whether compat is enabled or not. Some documentation at a minimum should be added. It otherwise seems to work ok. rob You could return a value for compat here without I've updated the description of 'compat' value in output and also changed the condition when this value is set to False. Now it is set to False only when the migration fails because of compatibility plugin. Code looks good. I think the error language needs some tweaking. I think the help text should read: The schema compat feature allows IPA to reformat data for systems that do not support RFC2307bis. It is recommended that this be disabled during migration to reduce system overhead. It can be re-enabled after migration. To migrate with it enabled use the "--with-compat" option. I think the client-side error should read: The compat plug-in is enabled. This can increase the memory requirements during migration. Disable the compat plug-in with \'ipa-compat-manage disable\' or re-run this script with \'--with-compat\' option." rob patch attached Ondra ACK, pushed to master and ipa-2-2 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 976 add tests for HTTP_Status
On 29.2.2012 22:22, Rob Crittenden wrote: The tests for not_found were broken, this fixes it and adds tests for the other statuses. I changed the parent class of HTTP_Status because it calls self.info which is provided by Plugable. This wasn't a problem at runtime because Backend provides self.log. rob ACK. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0009 Support for IPv6 elements in idnsForwarders attribute
Hello, here is (again) reworked patch for https://fedorahosted.org/bind-dyndb-ldap/ticket/49 . Adam pointed me to existing BIND parser, which I missed. Now is all parsing & socket magic done inside BIND libraries. Our code is a bit shorter and syntax is 100% BIND-compatible. (But it means same as discussed yesterday :-) Adam, please review it. Thanks. Petr^2 Spacek On 03/01/2012 03:22 PM, Petr Spacek wrote: Hello, here is reworked patch for https://fedorahosted.org/bind-dyndb-ldap/ticket/49 . Changes after yesterday's discussion on IRC with Simo and Mkosek: It follows BIND9 syntax for optional specification of port & adds documentation for this new syntax. Petr^2 Spacek On 02/29/2012 05:33 PM, Martin Kosek wrote: I agree that we should keep the BIND syntax and separate port and IP address with a space. We will at least avoid possible issues with IP address decoding in the future. Since this is a new attribute we have a good chance to do changes now so that it is used correctly. I created an upstream ticket to change the behavior and validators in FreeIPA: https://fedorahosted.org/freeipa/ticket/2462 Martin On Wed, 2012-02-29 at 16:44 +0100, Petr Spacek wrote: On 02/29/2012 04:30 PM, Simo Sorce wrote: Either way looks ok to me. I agree that using a space may be less confusing if this syntax never allows to specify multiple addresses. If multiple address can be specified than it may be less ideal to use spaces. Simo. idnsForwarders is multi-value attribute, so each value contain single forwarder address. Petr^2 Spacek On Wed, 2012-02-29 at 15:14 +0100, Petr Spacek wrote: And there is the patch, sorry. Petr^2 On 02/29/2012 03:10 PM, Petr Spacek wrote: Hello, this patch fixes https://fedorahosted.org/bind-dyndb-ldap/ticket/49 , but I want to discuss one (unimplemented) change: I propose a change in (currently very strange) forwarders syntax. Current syntax: [.port] examples: 1.2.3.4 (without optional port) 1.2.3.4.5553 (optional port 5553) A::B (IPv6, without optional port) A::B.5553 :::1.2.3.4 (6to4, without optional port) :::1.2.3.4.5553 (6to4, with optional port 5553) I find this syntax confusing, non-standard and not-typo-proof. IMHO better choice is to follow BIND forwarders syntax: [port ip_port] (port is string delimited with spaces) (From: http://www.zytrax.com/books/dns/ch7/queries.html#forwarders) *Current syntax is not documented*, so probably is not used anywhere. (And DNS server on non-standard port is probably useful only for testing purposes, but it's another story.) What is you opinion? >From 14056200915a90c2a957e8a2219819bd3b7f9365 Mon Sep 17 00:00:00 2001 From: Petr Spacek Date: Thu, 1 Mar 2012 15:08:10 +0100 Subject: [PATCH] Add support for IPv6 elements in idnsForwarders attribute and make syntax compatible with BIND9 forwarders. Signed-off-by: Petr Spacek --- NEWS |3 + README|8 ++- src/acl.c | 89 ++ src/acl.h |3 + src/ldap_helper.c | 136 ++--- 5 files changed, 116 insertions(+), 123 deletions(-) diff --git a/NEWS b/NEWS index ced6250..a97a5f3 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +[1] Add support for IPv6 elements in idnsForwarders attribute +and make syntax compatible with BIND9 forwarders. + 1.1.0a2 == diff --git a/README b/README index 914abdc..aedb88c 100644 --- a/README +++ b/README @@ -89,8 +89,12 @@ example zone ldif is available in the doc directory. with a valid idnsForwarders attribute. * idnsForwarders - Defines multiple IP addresses (TODO: optional port numbers) to which - queries will be forwarded. + Defines multiple IP addresses to which queries will be forwarded. + It is multi-value attribute: Each IP address (and optional port) has to + be in own value. BIND9 syntax for "forwarders" is required. + Optional port can be specified by adding " port " after IP + address. IPv4 and IPv6 addresses are supported. + Examples: "1.2.3.4" or "1.2.3.4 port 553" or "A::B" or "A::B port 553" 5. Configuration diff --git a/src/acl.c b/src/acl.c index 32ca702..b888aea 100644 --- a/src/acl.c +++ b/src/acl.c @@ -69,6 +69,7 @@ static isc_once_t once = ISC_ONCE_INIT; static cfg_type_t *update_policy; static cfg_type_t *allow_query; static cfg_type_t *allow_transfer; +static cfg_type_t *forwarders; static cfg_type_t * get_type_from_tuplefield(const cfg_type_t *cfg_type, const char *name) @@ -139,6 +140,7 @@ init_cfgtypes(void) update_policy = get_type_from_clause_array(zoneopts, "update-policy"); allow_query = get_type_from_clause_array(zoneopts, "allow-query"); allow_transfer = get_type_from_clause_array(zoneopts, "allow-transfer"); + forwarders = get_type_from_clause_array(zoneopts, "forwarders"); } static isc_result_t @@ -351,6 +353,24 @@ cleanup: return result; } +static isc_result_t +semicolon_bracket_str(isc_mem_t *mctx, const
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
Jan Cholasta wrote: On 17.1.2012 04:55, Rob Crittenden wrote: Jan Cholasta wrote: Dne 13.1.2012 17:39, Rob Crittenden napsal(a): Jan Cholasta wrote: Dne 14.12.2011 16:21, Rob Crittenden napsal(a): Jan Cholasta wrote: Dne 14.12.2011 15:23, Rob Crittenden napsal(a): Jan Cholasta wrote: Dne 14.12.2011 05:20, Rob Crittenden napsal(a): The sudo schema now defines sudoOrder, sudoNotBefore and sudoNotAfter but these weren't available in the sudorule plugin. I've added support for these. sudoOrder enforces uniqueness because duplicates are undefined. I also added support for a GeneralizedTime parameter type. This is similar to the existing AccessTime parameter but it only handles a single time value. You should parse the date/time part of the value with time.strptime(timestr, '%Y%m%d%H%M%S') instead of doing it manually, that way you'll get most of the validation for free. Yes but it gives a crappy error message, just saying that some data is left over not what is wrong. IMHO having a separate error message for every field in the time string (like you do in the patch) is an overkill, simple "invalid time" and/or "unknown time format" should suffice (we don't have errors like "invalid 3rd octet" for IP adresses either). Well, the work is done, hard to go back on a better error message. Also, it would be nice to be able to enter the value in more user-friendly format (e.g. "2011-12-14 13:01:25 +0100") and normalize that to LDAP generalized time. When dealing with time there are so many ways to input and display the same values this becomes difficult. I'd expect that the times for these two attributes will be relatively simple and I somehow doubt users are going to want seconds, leap seconds or fractions, but we'll need to consider how to do it for future consistency (otherwise we could have a case where time is entered in one format for some attributes and another for others). If we input in a nice way we need to output in the same way. We could make the preferred input/output time format user-configurable, defaulting to current locale time format. This format would be used for output. For input, we could go over a list of formats (first the user-configured format, then current locale format, then a handful of "standard" formats like -MM-DD HH:MM:SS) and use the first format that can be successfully used to parse the time string. See how far you get into the rabbit hole with even this simple format? I don't mind, as long as it is the right thing to do (IMHO) :) Anyway, I think this could be done on the client side, so we might use your patch without changes. However, I would prefer if the parameter class was more generic, so we could use it (hypothetically) to store time in some other way than LDAP generalized time attribute (at least name it DateTime please). Ok, I'm fine with that. Thanks. The LDAP GeneralizedTime needs to be either in GMT or include a differential. This gets us into the territory where the client could be in a different timezone than the server which leads us to why we dropped AccessTime in the first place. Speaking of time zones, the differential alone is not a sufficient time zone description, as it doesn't account for DST. Is there a way to store time in LDAP with full time zone name (just in case it's needed sometime in future)? There is no way to store DST in LDAP (probably for good reason). Oddly enough the older LDAP v3 RFC (2252) strongly recommends using only GMT but the RFC that obsoletes it (4517) does not include this. Thanks for the info. So I'd like the user to supply the timezone themselves so I don't have to guess (wrongly) and let them worry about differing timezones. We don't have to guess, IIRC there is a way to get the local timezone differential in both Python and JavaScript, so the client could supply it automatically if necessary. I was thinking more about non-IPA clients (like sudo and notBefore). I think this can still be done at least in CLI, but it could be done in a separate patch. Updated patches attached. rob Patch 919 doesn't cleanly apply on current master (neither does 916 BTW). Honza Rebased patch (and 916 too, separately). rob Patch 918: 1. LDAP generalized time allows you to omit minutes from time zone differential, your code treats such values as invalid 2. IMO a better pattern could be used, such as u'^([0-9]{2}){5,7}([.,][0-9]+)?([-+]([0-9]{2}){1,2}|Z)$' 3. 20120229000Z has malformed minutes, but the error message says "Malformed seconds" 4. 20120229000+ has malformed minutes, but the error message says "Missing operator for differential or malformed time string" 5. 20120229+ is valid generalized time, but it causes "Missing operator for differential or malformed time string" error 6. Invalid month/day combinations (such as 20120231Z) are treated as valid 7. When + or - is missing, the error message says "Missing operator ..." - IMO a better term than "ope
Re: [Freeipa-devel] [PATCH] 976 add tests for HTTP_Status
Jan Cholasta wrote: On 29.2.2012 22:22, Rob Crittenden wrote: The tests for not_found were broken, this fixes it and adds tests for the other statuses. I changed the parent class of HTTP_Status because it calls self.info which is provided by Plugable. This wasn't a problem at runtime because Backend provides self.log. rob ACK. Honza Pushed to master and ipa-2-2 rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 226 Improve hostname verification in install tools
Rob Crittenden wrote: Martin Kosek wrote: Our install tools like ipa-server-install, ipa-replica-{prepare, install} may allow hostnames that do not match the requirements in ipalib. This creates a disconnect and may cause issues when user cannot delete hostnames created by install tools. This patch makes sure that ipalib requirements are applied to install tools hostnames as well. https://fedorahosted.org/freeipa/ticket/2089 ACK pushed to master and ipa-2-2 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
Rob Crittenden wrote: Jan Cholasta wrote: On 17.1.2012 04:55, Rob Crittenden wrote: Jan Cholasta wrote: Dne 13.1.2012 17:39, Rob Crittenden napsal(a): Jan Cholasta wrote: Dne 14.12.2011 16:21, Rob Crittenden napsal(a): Jan Cholasta wrote: Dne 14.12.2011 15:23, Rob Crittenden napsal(a): Jan Cholasta wrote: Dne 14.12.2011 05:20, Rob Crittenden napsal(a): The sudo schema now defines sudoOrder, sudoNotBefore and sudoNotAfter but these weren't available in the sudorule plugin. I've added support for these. sudoOrder enforces uniqueness because duplicates are undefined. I also added support for a GeneralizedTime parameter type. This is similar to the existing AccessTime parameter but it only handles a single time value. You should parse the date/time part of the value with time.strptime(timestr, '%Y%m%d%H%M%S') instead of doing it manually, that way you'll get most of the validation for free. Yes but it gives a crappy error message, just saying that some data is left over not what is wrong. IMHO having a separate error message for every field in the time string (like you do in the patch) is an overkill, simple "invalid time" and/or "unknown time format" should suffice (we don't have errors like "invalid 3rd octet" for IP adresses either). Well, the work is done, hard to go back on a better error message. Also, it would be nice to be able to enter the value in more user-friendly format (e.g. "2011-12-14 13:01:25 +0100") and normalize that to LDAP generalized time. When dealing with time there are so many ways to input and display the same values this becomes difficult. I'd expect that the times for these two attributes will be relatively simple and I somehow doubt users are going to want seconds, leap seconds or fractions, but we'll need to consider how to do it for future consistency (otherwise we could have a case where time is entered in one format for some attributes and another for others). If we input in a nice way we need to output in the same way. We could make the preferred input/output time format user-configurable, defaulting to current locale time format. This format would be used for output. For input, we could go over a list of formats (first the user-configured format, then current locale format, then a handful of "standard" formats like -MM-DD HH:MM:SS) and use the first format that can be successfully used to parse the time string. See how far you get into the rabbit hole with even this simple format? I don't mind, as long as it is the right thing to do (IMHO) :) Anyway, I think this could be done on the client side, so we might use your patch without changes. However, I would prefer if the parameter class was more generic, so we could use it (hypothetically) to store time in some other way than LDAP generalized time attribute (at least name it DateTime please). Ok, I'm fine with that. Thanks. The LDAP GeneralizedTime needs to be either in GMT or include a differential. This gets us into the territory where the client could be in a different timezone than the server which leads us to why we dropped AccessTime in the first place. Speaking of time zones, the differential alone is not a sufficient time zone description, as it doesn't account for DST. Is there a way to store time in LDAP with full time zone name (just in case it's needed sometime in future)? There is no way to store DST in LDAP (probably for good reason). Oddly enough the older LDAP v3 RFC (2252) strongly recommends using only GMT but the RFC that obsoletes it (4517) does not include this. Thanks for the info. So I'd like the user to supply the timezone themselves so I don't have to guess (wrongly) and let them worry about differing timezones. We don't have to guess, IIRC there is a way to get the local timezone differential in both Python and JavaScript, so the client could supply it automatically if necessary. I was thinking more about non-IPA clients (like sudo and notBefore). I think this can still be done at least in CLI, but it could be done in a separate patch. Updated patches attached. rob Patch 919 doesn't cleanly apply on current master (neither does 916 BTW). Honza Rebased patch (and 916 too, separately). rob Patch 918: 1. LDAP generalized time allows you to omit minutes from time zone differential, your code treats such values as invalid 2. IMO a better pattern could be used, such as u'^([0-9]{2}){5,7}([.,][0-9]+)?([-+]([0-9]{2}){1,2}|Z)$' 3. 20120229000Z has malformed minutes, but the error message says "Malformed seconds" 4. 20120229000+ has malformed minutes, but the error message says "Missing operator for differential or malformed time string" 5. 20120229+ is valid generalized time, but it causes "Missing operator for differential or malformed time string" error 6. Invalid month/day combinations (such as 20120231Z) are treated as valid 7. When + or - is missing, the error message says "Missing operator ..." - IMO
Re: [Freeipa-devel] [PATCH] 956 user lockout status
Martin Kosek wrote: On Wed, 2012-02-29 at 11:20 +0100, Petr Viktorin wrote: On 02/27/2012 06:31 PM, Martin Kosek wrote: 4) Minor change: -except Exception: +except: Don't do that. It would for example disable Ctrl+C by trapping KeyboardInterrupt. PEP8 has a paragraph on this, search for 'except Exception:' Good to know, thanks. Rob, in that case please ignore issue #4. Martin Updated patch attached. rob >From 8acf1185967e6039cdcb25321dd079ec34ed4970 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Thu, 1 Mar 2012 16:24:41 -0500 Subject: [PATCH] Add status command to retrieve user lockout status This information is not replicated so pull from all IPA masters and display the status across all servers. https://fedorahosted.org/freeipa/ticket/2162 --- ipalib/plugins/user.py | 11 --- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py index 54e32f2358f035a6fd4944c38a64c4776b12fdea..ca11315bd939b545d1ea96e22afe29fdd6f9cb1d 100644 --- a/ipalib/plugins/user.py +++ b/ipalib/plugins/user.py @@ -747,12 +747,15 @@ class user_status(LDAPQuery): try: entry = other_ldap.get_entry(dn, attr_list) newresult = dict() -for attr in attr_list: -newresult[attr] = entry[1].get(attr, '') +for attr in ['krblastsuccessfulauth', 'krblastfailedauth']: +newresult[attr] = entry[1].get(attr, [u'N/A']) +newresult['krbloginfailedcount'] = entry[1].get('krbloginfailedcount', u'0') if not options.get('raw', False): for attr in ['krblastsuccessfulauth', 'krblastfailedauth']: try: -newtime = time.strptime(newresult[attr][0], '%Y%m%d%H%M%S%Z') +if newresult[attr][0] == u'N/A': +continue +newtime = time.strptime(newresult[attr][0], '%Y%m%d%H%M%SZ') newresult[attr][0] = unicode(time.strftime('%Y-%m-%dT%H:%M:%SZ', newtime)) except Exception, e: self.debug("time conversion failed with %s" % str(e)) @@ -761,6 +764,8 @@ class user_status(LDAPQuery): newresult['server'] = host entries.append(newresult) count += 1 +except errors.NotFound: +self.obj.handle_not_found(*keys) except Exception, e: raise e -- 1.7.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 484 Fix credentials checks with s4u2proxy delegation
The commit message says it all I think. This is critical for 2.2 and master. Simo. -- Simo Sorce * Red Hat, Inc * New York >From 7508e59ce8dc72f9d93ae9a707ee4888f7fa5f29 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Thu, 1 Mar 2012 17:22:10 -0500 Subject: [PATCH] Fix ticket checks when using either s4u2proxy or a delegated krbtgt When using s4u2proxy the only ticket we can access via direct krb5 calls is the HTTP? ticket which was saved in the ccache as eveidence ticket. This tiket is later used by gssapi as evidence to obtain an ldap ticket. This works by chance, we shouldn't use calls to get_crednetials just to verify ticket expiration dates, but I realize this is a limitation of the current krbV bindings and we have no other way around at the moment. Checking the HTTP/ ticket will fail in case a krbtgt is fully delegated to us, in that case the ccache will contain only a krbtgt, so as a fallback we check that. Checking the ldap/ ticket is never really useful. When s4u2proxy is usedl, trying to check the ldap/ ticket will fail because we do not have it yet on the first authentication before a session is estalished, and doing it later is not useful. When we have a krbtgt we could go and grap n ldap/ ticket directl, but again that makes little sense. In general all tickets will have the same expiration date (which deopends on the original krbtgt) so checking one is sufficient. Fixes: http://fedorahosted.org/freeipa/ticket/2472 --- ipalib/krb_utils.py |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ipalib/krb_utils.py b/ipalib/krb_utils.py index a8f7751dd8e44ac35aad70fb39e14e34912f4979..b0010e9e59ab07aacc1307b5f73607e1b6670d11 100644 --- a/ipalib/krb_utils.py +++ b/ipalib/krb_utils.py @@ -343,7 +343,7 @@ class KRB5_CCache(object): ''' try: -principal = krb5_format_service_principal_name('ldap', host, realm) +principal = krb5_format_service_principal_name('HTTP', host, realm) valid = self.credential_is_valid(principal) if valid: return True @@ -372,7 +372,7 @@ class KRB5_CCache(object): result = 0 try: -principal = krb5_format_service_principal_name('ldap', host, realm) +principal = krb5_format_service_principal_name('HTTP', host, realm) authtime, starttime, endtime, renew_till = self.get_credential_times(principal) if result: result = min(result, endtime) -- 1.7.7.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 484 Fix credentials checks with s4u2proxy delegation
Simo Sorce wrote: The commit message says it all I think. This is critical for 2.2 and master. Simo. ACK. Tested with patch krb 1.9 on F-16. Pushed to master and ipa-2-2 rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 098 Forms based authentication UI
Petr Vobornik wrote: Support for forms based authentication was added to UI. It consist of: 1) new login page Page url is [ipa server]/ipa/ui/login.html Page contains a login form. For authentication it sends ajax request at [ipa server]/session/json/login_password. If authentication is successfull page is redirected to [ipa server]/ipa/ui if it fails from whatever reason a message is shown. 2) new enhanced error dialog - authorization_dialog. This dialog is displayed when user is not authorized to perform action - usually when ticket and session expires. It is a standard error dialog which shows kerberos ticket related error message and newly offers (as a link) to use form based authentication. If user click on the link, the dialog content and buttons switch to login dialog which has same functionality as 'new login page'. User is able to return back to the error message by clicking on a back button. login.html uses same css styles as migration page -> ipa-migration.css was merged into ipa.css. https://fedorahosted.org/freeipa/ticket/2450 Theoretically the login.html is not needed. Sometime later we should come up with a method how to i18n static pages and main page prior to authentication. ACK. It looks like ipa.js in master and ipa-2-2 have diverged slightly, I'll let you push this so you can make sure everything is ok. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0100 Improved usability of login dialog
Petr Vobornik wrote: Usability was improved in Unauthorized/Login dialog. When the dialog is opened a link which switches to login form is focus so user can do following: 1) press enter (login form is displayed and username field is focused ) 2) type username 3) press tab 4) type password 5) press enter this sequence will execute login request. When filling form user can also press 'escape' to go back to previous form state. It's the same as if he would click on the 'back' button. https://fedorahosted.org/freeipa/ticket/2450 ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 977 fix WSGI exceptions
Trying to raise some exceptions in the WSGI code just raised other exceptiosn and were generally confusing. To test this do various combinations (with and without a ccache) of: curl -kv https://ipa.example.com/ipa/json --negotiate -u : -H 'Referer: https://ipa.example.com/ipa/json' curl -kv https://ipa.example.com/ipa/json --negotiate -u : curl -kv https://ipa.example.com/ipa/xml --negotiate -u : -H 'Referer: https://ipa.example.com/ipa/xml' curl -kv https://ipa.example.com/ipa/json --negotiate -u : If you want to get really clever set krbConstrainedDelegation to off in ipa.conf and restart and try them all again. rob >From 17cdceea0c0956db398a6a4e27f860ca76d9d66d Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Thu, 1 Mar 2012 21:54:06 -0500 Subject: [PATCH] Fix WSGI error handling A number of different errors could occur when trying to handle an error which just confused matters. If no CCache was received then trying to retrieve context.principal in the error message caused yet another exception to be raised. Trying to get Command[name] if name wasn't defined in command would raise an exception. Trying to raise errors.CCache was failing because the response hadn't been started. https://fedorahosted.org/freeipa/ticket/2371 --- ipaserver/rpcserver.py | 18 -- 1 files changed, 12 insertions(+), 6 deletions(-) diff --git a/ipaserver/rpcserver.py b/ipaserver/rpcserver.py index 147707b..2fbd79f 100644 --- a/ipaserver/rpcserver.py +++ b/ipaserver/rpcserver.py @@ -322,7 +322,7 @@ class WSGIExecutioner(Executioner): error = InternalError() finally: os.environ['LANG'] = lang -if name: +if name and name in self.Command: try: params = self.Command[name].args_options_2_params(*args, **options) except Exception, e: @@ -331,10 +331,11 @@ class WSGIExecutioner(Executioner): ) # get at least some context of what is going on params = options +principal = getattr(context, 'principal', 'UNKNOWN') if error: -self.info('%s: %s(%s): %s', context.principal, name, ', '.join(self.Command[name]._repr_iter(**params)), e.__class__.__name__) +self.info('%s: %s(%s): %s', principal, name, ', '.join(self.Command[name]._repr_iter(**params)), e.__class__.__name__) else: -self.info('%s: %s(%s): SUCCESS', context.principal, name, ', '.join(self.Command[name]._repr_iter(**params))) +self.info('%s: %s(%s): SUCCESS', principal, name, ', '.join(self.Command[name]._repr_iter(**params))) else: self.info('%s: %s', context.principal, e.__class__.__name__) return self.marshal(result, error, _id) @@ -377,7 +378,7 @@ class WSGIExecutioner(Executioner): raise NotImplementedError('%s.marshal()' % self.fullname) -class xmlserver(WSGIExecutioner): +class xmlserver(WSGIExecutioner, HTTP_Status): """ Execution backend plugin for XML-RPC server. @@ -402,6 +403,8 @@ class xmlserver(WSGIExecutioner): self.debug('WSGI xmlserver.__call__:') user_ccache=environ.get('KRB5CCNAME') if user_ccache is None: +self.internal_error(environ, start_response, +'xmlserver.__call__: KRB5CCNAME not defined in HTTP request environment') return self.marshal(None, CCacheError()) try: self.create_context(ccache=user_ccache) @@ -548,7 +551,7 @@ def json_decode_binary(val): else: return val -class jsonserver(WSGIExecutioner): +class jsonserver(WSGIExecutioner, HTTP_Status): """ JSON RPC server. @@ -576,11 +579,12 @@ class jsonserver(WSGIExecutioner): message=error.strerror, name=error.__class__.__name__, ) +principal = getattr(context, 'principal', 'UNKNOWN') response = dict( result=result, error=error, id=_id, -principal=unicode(context.principal), +principal=unicode(principal), version=unicode(VERSION), ) response = json_encode_binary(response) @@ -844,6 +848,8 @@ class jsonserver_kerb(jsonserver): user_ccache=environ.get('KRB5CCNAME') if user_ccache is None: +self.internal_error(environ, start_response, +'jsonserver_kerb.__call__: KRB5CCNAME not defined in HTTP request environment') return self.marshal(None, CCacheError()) self.create_context(ccache=user_ccache) -- 1.7.7.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 978 fix hostnames in hbac tests
I pushed this under the 1-liner rule. Martin drastically improved hostname validation and this broke several HBAC tests that were using invalid domain names. Pushed to master and ipa-2-2. Incidentally there are some broken sudo tests too but I addressed those in patch 919. rob >From 1aa8c8866ff0f6c8bbf0162ef040a41cb0bf738b Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Thu, 1 Mar 2012 22:21:32 -0500 Subject: [PATCH] Make hostnames adhere to new standards in HBAC tests --- tests/test_xmlrpc/test_hbac_plugin.py |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_xmlrpc/test_hbac_plugin.py b/tests/test_xmlrpc/test_hbac_plugin.py index ebb5d17..ef61d68 100644 --- a/tests/test_xmlrpc/test_hbac_plugin.py +++ b/tests/test_xmlrpc/test_hbac_plugin.py @@ -41,9 +41,9 @@ class test_hbac(XMLRPC_test): test_user = u'hbacrule_test_user' test_group = u'hbacrule_test_group' -test_host = u'hbacrule.test-netgroup' +test_host = u'hbacrule.testnetgroup' test_hostgroup = u'hbacrule_test_hostgroup' -test_sourcehost = u'hbacrule.test-src-host' +test_sourcehost = u'hbacrule.testsrchost' test_sourcehostgroup = u'hbacrule_test_src_hostgroup' test_service = u'sshd' test_host_external = u'notfound.example.com' -- 1.7.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 69 Configure SSH features of SSSD in ipa-client-install
Jan Cholasta wrote: On 29.2.2012 15:00, Martin Kosek wrote: On Wed, 2012-02-29 at 14:44 +0100, Jan Cholasta wrote: On 29.2.2012 14:24, Martin Kosek wrote: On Wed, 2012-02-29 at 10:52 +0100, Jan Cholasta wrote: On 28.2.2012 23:42, Rob Crittenden wrote: Jan Cholasta wrote: Hi, this patch configures the new SSH features of SSSD in ipa-client-install. To test it, you need to have SSSD 1.8.0 installed. Honza Is there a better name for 'GlobalKnownHostsFile2'? What do you mean? The option name or the file name? Either way, I don't think there is a better name. When is PubKeyAgent used?I tried in RHEL 6.2, F-11 and F15-17 and it was an unknown option in all. It's in openssh in RHEL 6.0. Should you test for the existence of /usr/bin/sss_ssh_knownhostsproxy and /usr/bin/sss_ssh_authorizedkeys before setting it in a config file? It depends. Do we want to support clients with SSSD< 1.8.0? How would you recommend testing this? Enroll a client and try to log into the IPA server? To test host authentication, you need an IPA host with SSH public keys set (which is done automatically in ipa-client-install, so any IPA host should work) and try to ssh into that host from other (actually, it can be the same) IPA host. You should not see "The authenticity of host ... can't be estabilished" ssh message. To test user authentication, you need an IPA user with SSH public keys set. To do that, you need to set the public keys using ipa user-mod. You should then be able to authenticate using your private key on any IPA host. rob Honza I get this exception when running ipa-client-install with your patch. # ipa-client-install --enable-dns-updates Discovery was successful! Hostname: vm-138.idm.lab.bos.redhat.com Realm: IDM.LAB.BOS.REDHAT.COM DNS Domain: idm.lab.bos.redhat.com IPA Server: vm-068.idm.lab.bos.redhat.com BaseDN: dc=idm,dc=lab,dc=bos,dc=redhat,dc=com Continue to configure the system with these values? [no]: y User authorized to enroll computers: admin Synchronizing time with KDC... Unable to sync time with IPA NTP server, assuming the time is in sync. Password for ad...@idm.lab.bos.redhat.com: Enrolled in IPA realm IDM.LAB.BOS.REDHAT.COM Created /etc/ipa/default.conf Traceback (most recent call last): File "/usr/sbin/ipa-client-install", line 1514, in sys.exit(main()) File "/usr/sbin/ipa-client-install", line 1501, in main rval = install(options, env, fstore, statestore) File "/usr/sbin/ipa-client-install", line 1326, in install if configure_sssd_conf(fstore, cli_realm, cli_domain, cli_server, options): File "/usr/sbin/ipa-client-install", line 711, in configure_sssd_conf sssdconfig.activate_service('ssh') File "/usr/lib/python2.7/site-packages/SSSDConfig.py", line 1516, in activate_service raise NoServiceError SSSDConfig.NoServiceError SSSD version: sssd-1.8.1-0.20120228T2018Zgit751b121.fc16.x86_64 Martin Does your /etc/sssd/sssd.conf and /usr/share/sssd/sssd.api.conf contain [ssh] section? sssd.api.conf did contain the ssh section: # grep -C 3 ssh /usr/share/sssd/sssd.api.conf # autofs service autofs_negative_timeout = int, None, false [ssh] # ssh service [provider] #Available provider types sssd.conf did not. Either case, we should not crash but handle the issue in some more friendly way. Martin Patch updated with more defensive code. Honza Needs a BuildRequires of sssd 1.8 or you get some pylint errors: ipa-client/ipa-install/ipa-client-install:712: [E1101, configure_sssd_conf] Instance of 'SSSDConfig' has no 'activate_service' member ipa-client/ipa-install/ipa-client-install:723: [E1101, configure_sssd_conf] Instance of 'SSSDConfig' has no 'activate_service' member ipa-client/ipa-install/ipa-client-install:734: [E1101, configure_sssd_conf] Instance of 'SSSDConfig' has no 'activate_service' member Host keys work fine. I wasn't able to get user ssh keys working but my server is still on F-15. I had a daily build of sssd (1.8.1) but it was missing /usr/libexec/sssd/sssd_ssh!? Too tired to work out why right now. Two more things: 1. You will need explicit test cases for QE to test positive and negative login cases (it would have sped me along too). 2. You need to beef up the commit message to describe what this does (e.g. configure for knownhost support). commit message space is cheap, be verbose. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel