Re: [Freeipa-devel] [PATCH] 0001 Fix ipa-client-install --uninstall crash

2014-07-08 Thread David Kupka


On 07/07/2014 09:15 PM, Petr Viktorin wrote:

On 07/04/2014 08:47 AM, David Kupka wrote:

https://fedorahosted.org/freeipa/ticket/4273

David Kupka


Hi,
This works fine. Just two nitpicks in the log message:
- %s means convert to string, so the str() is redundant
- the logger methods take items to interpolate as arguments, instead 
of one string built with %.


So you'll want:
root_logger.error('Failed to start chronyd: %s', e)



Hi,
thanks for comments. I've fixed the patch accordingly.

--
David Kupka

From 8f51cc43f11485568814ea6937c941a07d5a9e37 Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Tue, 8 Jul 2014 07:36:48 +0200
Subject: [PATCH] Fix ipa-client-install --uninstall crash

Fix ipa-client-install crash when chronyd service fails to start.

https://fedorahosted.org/freeipa/ticket/4273
---
 ipa-client/ipa-install/ipa-client-install | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index bfa43b1468887dcd408cd8f2941f9fd961f372ce..617db26f499106fa10665af3ca9d9f2736ba9b00 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -692,7 +692,10 @@ def uninstall(options, env):
if restored:
services.knownservices.ntpd.restart()
 
-ipaclient.ntpconf.restore_forced_ntpd(statestore)
+try:
+ipaclient.ntpconf.restore_forced_ntpd(statestore)
+except CalledProcessError, e:
+root_logger.error('Failed to start chronyd: %s', e)
 
 if was_sshd_configured and services.knownservices.sshd.is_running():
 services.knownservices.sshd.restart()
-- 
1.9.3

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0001 Fix ipa-client-install --uninstall crash

2014-07-08 Thread Petr Viktorin

On 07/08/2014 07:59 AM, David Kupka wrote:


On 07/07/2014 09:15 PM, Petr Viktorin wrote:

On 07/04/2014 08:47 AM, David Kupka wrote:

https://fedorahosted.org/freeipa/ticket/4273

David Kupka


Hi,
This works fine. Just two nitpicks in the log message:
- %s means convert to string, so the str() is redundant
- the logger methods take items to interpolate as arguments, instead
of one string built with %.

So you'll want:
root_logger.error('Failed to start chronyd: %s', e)


Sorry for not replying to list.


Hi,
thanks for comments. I've fixed the patch accordingly.


Thanks!
ACK, pushed to:
master: 2ff14607b170c86ccd93cff60f5a34d64cd1e419
ipa-4-1: 2ff14607b170c86ccd93cff60f5a34d64cd1e419
ipa-4-0: 2ff14607b170c86ccd93cff60f5a34d64cd1e419


--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 692 webui: capitalize labels of undo and undo all buttons

2014-07-08 Thread Petr Viktorin

On 07/02/2014 09:43 AM, Petr Viktorin wrote:

On 07/02/2014 08:52 AM, Martin Kosek wrote:

On 07/02/2014 07:11 AM, Fraser Tweedale wrote:

[...]

Translations are lost as a result of this change, due to case
sensitive translation lookup by msgid.  I guess our translation
workflow takes care of this - in which case, ACK.


Pushed to:
master: 03c25bd98e7fb16574a8bc0c4a9c9de851ae9bed
ipa-4-0: 03c25bd98e7fb16574a8bc0c4a9c9de851ae9bed
ipa-4-1: 03c25bd98e7fb16574a8bc0c4a9c9de851ae9bed

--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [PATCH] 0620 ldap2 indirect membership processing: Use global limits if greater than per-query ones

2014-07-08 Thread Petr Viktorin

Fix for https://fedorahosted.org/freeipa/ticket/4398

--
Petr³
From 065026028a66107bba0c0265cde3465823db233c Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Wed, 25 Jun 2014 13:55:58 +0200
Subject: [PATCH] ldap2 indirect membership processing: Use global limits if
 greater than per-query ones

Calling an ipa *-find command with --sizelimit=1 on an entry with more
members would result in a LimitsExceeded error as the search for members
was limited to 1 entry.

For the memberof searches, only apply the global limit if it's larger than
the requested one, so decreasing limits on the individual query only
affects the query itself.

https://fedorahosted.org/freeipa/ticket/4398
---
 ipaserver/plugins/ldap2.py | 35 +--
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 9ecd0b87c455fd8fe3c80afcba89ef72fbc3b1fa..ec491e9e805589fad3bfd5e07ad40ef26581f703 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -178,15 +178,24 @@ def destroy_connection(self):
 # ignore when trying to unbind multiple times
 pass
 
+
 def find_entries(self, filter=None, attrs_list=None, base_dn=None,
  scope=_ldap.SCOPE_SUBTREE, time_limit=None,
  size_limit=None, search_refs=False, paged_search=False):
-if time_limit is None or size_limit is None:
-config = self.get_ipa_config()
-if time_limit is None:
-time_limit = config.get('ipasearchtimelimit', [None])[0]
-if size_limit is None:
-size_limit = config.get('ipasearchrecordslimit', [None])[0]
+
+def _get_limits():
+Get configured global limits, caching them for more calls
+if not _lims:
+config = self.get_ipa_config()
+_lims['time'] = config.get('ipasearchtimelimit', [None])[0]
+_lims['size'] = config.get('ipasearchrecordslimit', [None])[0]
+return _lims
+_lims = {}
+
+if time_limit is None:
+time_limit = _get_limits()['time']
+if size_limit is None:
+size_limit = _get_limits()['size']
 
 has_memberindirect = False
 has_memberofindirect = False
@@ -207,6 +216,20 @@ def find_entries(self, filter=None, attrs_list=None, base_dn=None,
 search_refs=search_refs, paged_search=paged_search)
 
 if has_memberindirect or has_memberofindirect:
+
+# For the memberof searches, we want to apply the global limit
+# if it's larger than the requested one, so decreasing limits on
+# the individual query only affects the query itself.
+# See https://fedorahosted.org/freeipa/ticket/4398
+def _max_with_none(a, b):
+Maximum of a and b, treating None as infinity
+if a is None or b is None:
+return None
+else:
+return max(a, b)
+time_limit = _max_with_none(time_limit, _get_limits()['time'])
+size_limit = _max_with_none(size_limit, _get_limits()['size'])
+
 for entry in res:
 if has_memberindirect:
 self._process_memberindirect(
-- 
1.9.3

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0621 test_xmlrpc: Update tests

2014-07-08 Thread Petr Viktorin

Two test fixes for recent permission changes

--
Petr³
From f170d1ac95fc3446b1c4a8d3addd40990dbec72a Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 8 Jul 2014 13:28:24 +0200
Subject: [PATCH] test_xmlrpc: Update tests

- The number of permissions in $SUFFIX changed.
- A new ACI was added to realmdomains

Update the tests.
---
 ipatests/test_xmlrpc/test_permission_plugin.py   |  4 ++--
 ipatests/test_xmlrpc/test_realmdomains_plugin.py | 12 +++-
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
index bf902c367a4a38e4a4a6ea6b20d6140d61e3fbf5..e5c8286708efe60e61e205e6df54fdcd1245a7a6 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -2851,9 +2851,9 @@ class test_permission_legacy(Declarative):
 command=('permission_find', [],
  {'ipapermlocation': api.env.basedn}),
 expected=dict(
-count=15,
+count=16,
 truncated=False,
-summary=u'15 permissions matched',
+summary=u'16 permissions matched',
 result=lambda s: True,
 ),
 ),
diff --git a/ipatests/test_xmlrpc/test_realmdomains_plugin.py b/ipatests/test_xmlrpc/test_realmdomains_plugin.py
index ee8057d1d9aff004e9ed144577e4aed34dc7b8dc..1b59e790a1d91ef0abd59b18c84885851ba10098 100644
--- a/ipatests/test_xmlrpc/test_realmdomains_plugin.py
+++ b/ipatests/test_xmlrpc/test_realmdomains_plugin.py
@@ -71,7 +71,17 @@ class test_realmdomains(Declarative):
 u'(version 3.0;acl '
 u'permission:System: Read Realm Domains;'
 u'allow (compare,read,search) '
-u'userdn = ldap:///all;;)'
+u'userdn = ldap:///all;;)',
+
+u'(targetattr = associateddomain)'
+u'(targetfilter = (objectclass=domainrelatedobject))'
+u'(version 3.0;acl '
+u'permission:System: Modify Realm Domains;'
+u'allow (write) groupdn = ldap:///%s;;)' % 
+DN('cn=System: Modify Realm Domains',
+   api.env.container_permission,
+   api.env.basedn),
+
 ],
 ),
 ),
-- 
1.9.3

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0622 baseldap: Return empty string when no effective rights are found

2014-07-08 Thread Petr Viktorin

A fix for https://fedorahosted.org/freeipa/ticket/4359

--
Petr³
From 9463adb3ea459aa0d0a8c47d0c6d87989426d064 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 8 Jul 2014 15:22:23 +0200
Subject: [PATCH] baseldap: Return empty string when no effective rights are
 found

DS returns the string none when no rights were found. All clients
would need to special-case this value when checking the rights.
Return empty string instead.

https://fedorahosted.org/freeipa/ticket/4359
---
 ipalib/plugins/baseldap.py | 4 
 1 file changed, 4 insertions(+)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 44302c9a7eb3eefeb20fe741938498b99fd70fed..e5a23b99f48f47558937d821c6bbaba94a5707dc 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -210,6 +210,10 @@ def get_effective_rights(ldap, dn, attrs=None):
 rights = rights[0].split(', ')
 for r in rights:
 (k,v) = r.split(':')
+if v == 'none':
+# the string none means no rights found
+# see https://fedorahosted.org/freeipa/ticket/4359
+v = ''
 rdict[k.strip().lower()] = v
 
 return rdict
-- 
1.9.3

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0622 baseldap: Return empty string when no effective rights are found

2014-07-08 Thread Jan Cholasta

On 8.7.2014 16:08, Petr Viktorin wrote:

A fix for https://fedorahosted.org/freeipa/ticket/4359


ACK.

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [SECURITY] Analysis of OTP Attack Vectors

2014-07-08 Thread Nathaniel McCallum
Dmitri, Ludwig and I had a meeting today to discuss the details of OTP
attack vectors. This is the results of that meeting. We would love
review as far and as wide as possible.

=== Rewind Attack ===

This attack is entirely theoretical. It works based on the premise that
simultaneous writes to multiple servers in a replication set might, via
delayed replication, cause a counter/watermark to go backwards. This
attack is limited by the fact that the attack requires two successful
authentications of the same token to occur on different replicas before
the state from the first authentication is replicated to the second. As
the attacker cannot control replication ordering or speed, the ability
to perform this attack is also non-deterministic.

This attack can be entirely eliminated by ensuring that
counters/watermarks never move backwards via custom replication
strategies. Implementing these would be rather difficult.

Lacking custom replication strategies, the ratio of attack success is
essentially the maximum replication time divided by the total
authentication timing variance. Hence, mitigation of the attack can be
achieved by either ensuring timely replication or by introducing random
delays of sufficient duration to the login process.

This attack can take two forms. In both forms, the attacker has managed
to compromise a block of tokens of size N which, without a rewind
attack, permits at most N authentications. Hence, the purpose of this
attack is to expand an existing compromise into a worse compromise.

In the first form of the attack, X simultaneous logins are executed
against distinct replicas in order to obtain more than N
authentications. If every attack was successful, the maximum number of
authentications would be XN-1. If attacking Kerberos, there is no
advantage to obtaining multiple simultaneous authentications. Hence, the
effective attack rate of a Kerberos attack is (XN-1) / X, or, after
rounding, N. In the case of logging into discrete, non-ticketed
services, such as an HTTP authentication via LDAP, there may be some
value to simultaneous logins, placing the expansion at XN-1. However, in
this case, the authentication timing variance will generally increase,
limiting the effectiveness of the attack.

In the second form of the attack, the attacker will attempt to time his
authentication with the user's authentication. Where the first form of
the attack is offensive in attempting to expand the usable size of the
compromised block, the second form is defensive in attempting to
prevent the user's legitimate login from shrinking the usable size of
the block. This attack is extremely difficult to perform. Not only is it
extremely difficult to time an authentication with another user on top
of infrastructure timing variances, the usable state of the block is
also generally unknown. This creates a situation where there is an
inverse relationship between the success rate of the used code and the
value of of the resulting success. In other words, the more likely the
attack is to succeed, the less valuable the successful attack is.

In short, we concluded that a rewind attack while theoretically possible
is both extremely difficult to implement and of limited value to an
attacker. Hence, aside from an earnest attempt to ensure timely
replication, we do not plan to implement specific countermeasures.

=== Replay Attack ===

Currently, this attack is somewhat practical and a known limitation of
the FreeIPA TOTP implementation due to a lack of high watermark support.
This attack has similar characteristics to the rewind attack in that it
permits the expansion of a single OTP code into X authentications where
X is the number of nodes in the replication set.

Four options exist for mitigation.

1. Move the counter/watermark to an external solution. This could
implement local storage at the cost of a single point of
failure/bottleneck. It could also implement a more complex locking or
replication solution.

2. Restrict reads/writes of the counter/watermark to a single 389DS
master. In this case, replay would be limited to the case where failover
has occurred before the writes could complete. The main cost of this
option is a performance bottleneck.

3. Add a mechanism for priority replication to 389DS. While this
wouldn't eliminate the replay attack, it could greatly reduce its
probability. This option trades absolute certainty against the replay
attack for authentication throughput.

4. Add a mechanism for synchronous replication to 389DS. This option
would guarantee that any modification involving the counter would be
replicated to all nodes before succeeding. This provides absolute
certainty at an extreme performance penalty.

It is also possible that some research into academic articles may reveal
some better strategies for counter replication.

In order to adequately assess the current situation, some replication
performance testing is required. This would ideally measure the
replication throughput