Re: [Freeipa-devel] [PATCHES] 0016 Fixes for{add, set, del}attr with managed attributes

2012-03-01 Thread Petr Viktorin

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

2012-03-01 Thread Martin Kosek
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

2012-03-01 Thread Petr Viktorin

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

2012-03-01 Thread Jan Cholasta

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

2012-03-01 Thread Martin Kosek
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

2012-03-01 Thread Ondrej Hamada

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

2012-03-01 Thread Martin Kosek
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

2012-03-01 Thread Petr Vobornik

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

2012-03-01 Thread Ondrej Hamada

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

2012-03-01 Thread Petr Spacek

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

2012-03-01 Thread Jan Cholasta

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

2012-03-01 Thread Jan Cholasta

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

2012-03-01 Thread Rob Crittenden

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

2012-03-01 Thread Rob Crittenden

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

2012-03-01 Thread Rob Crittenden

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

2012-03-01 Thread Jan Cholasta

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

2012-03-01 Thread Petr Spacek

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

2012-03-01 Thread Rob Crittenden

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

2012-03-01 Thread Rob Crittenden

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

2012-03-01 Thread Rob Crittenden

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

2012-03-01 Thread Rob Crittenden

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

2012-03-01 Thread Rob Crittenden

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

2012-03-01 Thread Simo Sorce
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

2012-03-01 Thread Rob Crittenden

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

2012-03-01 Thread Rob Crittenden

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

2012-03-01 Thread Rob Crittenden

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

2012-03-01 Thread Rob Crittenden
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

2012-03-01 Thread Rob Crittenden

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

2012-03-01 Thread Rob Crittenden

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