Re: [Freeipa-devel] fixing Kerberos principal aliases handling in IPA

[thierry bordaz]

On 09/07/2015 09:47 PM, Simo Sorce wrote:

On Mon, 2015-09-07 at 09:20 +0200, David Kupka wrote:

On 04/09/15 12:49, thierry bordaz wrote:

On 09/03/2015 04:03 PM, David Kupka wrote:

On 02/09/15 14:27, Simo Sorce wrote:

On Wed, 2015-09-02 at 08:11 +0200, David Kupka wrote:

On 01/09/15 16:53, Simo Sorce wrote:

On Tue, 2015-09-01 at 16:39 +0200, Martin Babinsky wrote:

Hi list,

I own the following ticket
https://fedorahosted.org/freeipa/ticket/3864
and I would like to clarify what needs to be done in order to make
IPA
to fully support multiple aliases per entry.

So far I have identified these task based on the ticket comments and
discussion with Simo way back in the past:

1.) mark 'ipaKrbPrincipalAlias' attribute as deprecated so that it is
not used in the new code.

2.) fix ACIs that do not permit setting multiple values of
'krbPrincipalName' attribute per entry (see
https://fedorahosted.org/freeipa/ticket/3961)

3.) Modify KDB backend (namely 'ipadb_fetch_principal' and
'ipadb_find_principal' functions) to correctly perform lookup of
krbprincipalname/krbcanonicalname, i.e. search krbprincipalname
case-insensitively and krbcanonicalname case-sensitively, return
krbcanonicalname when canonicalization is requested.

4.) Modify KDB backend and IPA framework to handle creation of both
krbprincipalname and krbcanonicalname. I am not quite sure what cases
should be covered here (I remember that we should create
krbcanonicalname when we add another aliases to krbprincipalname),
so it
would be nice if you could comment on this.

5.) write tests which cover all this stuff so that we don't shoot
ourselves in the foot.

I am not very well versed in Kerberos so I might get some of this
stuff
wrong. If that's the case please point me to the right direction.
Also
please write me some additional stuff which I have fogot and needs
to be
done.


I think the summary is correct, the only thing we need to be
careful is
to keep handling entries that have only a single valued
krbprincipalname
correctly as that will happen in upgrade paths and potentially if
someone uses external tools.

The tricky part for point 3 is to implement it *without* changing the
schema. KrbPrincipalName is case-sensitive, however I think we can
solve
the issue of "searching case-insensitively" by always lower-casing the
principal name components and always upper casing the realm part on
storage. If we always store a krbCanonicalName we get the "correct"
case
there anyway so out mucking with the krbPrincipalName case will not
be a
problem for any new entry.

Or as Honza pointed out we can use case-insensitive search like this:
(krbPrincipalName:caseIgnoreMatch:=ad...@example.com). This will return
all variants of casing and reduce the need for fallback code.

The principal name is not case insensitive, a search like that would
probably cause DS to do a full search through the krbPrincipalName
index, it might quickly become a performance issue. Before choosing a
method please create an install with a 1 principals, and then
compare the speed of a few thousand search with exact matching case and
a few with specifying caseIgnoreMatch and see the difference.

Simo.

We will add index for caseIgnoreCaseIA5Match matching rule on
krbPrincipalName and then the case insensitive match should be as
quick as the case sensitive.

Without the index it is indeed far slower. I've generated 10k users
and compared 100 ldapsearches: The indexed ones took ~ 4 seconds and
the nonindexed one ~2 minutes. That's by two orders of magnitude worse.

When we tried to add the index into DS we uncovered a bug in the way
DS handles nsMatchingRule attributes. Thierry investigated it and is
filling the ticket for DS right now. Thierry can you please send link?

The ticket is https://fedorahosted.org/389/ticket/48270.
I think understand where the problem is but I have no fix yet.
David, when you said the unindexed search was slow, did you index
'krbPrincipalName' but added a matching rule to your filter ?
I would like to also verify the fix against that perf hit.

thanks
thierry

I've tried these 3 searches:

1) ldapsearch -h localhost -D "cn=Directory Manager" -b
cn=users,cn=accounts,dc=example,dc=com -w freeipa8
"(krbprincipalname:caseIgnoreIA5Match:=user1005...@example.com)"

2) ldapsearch -h localhost -D "cn=Directory Manager" -b
cn=users,cn=accounts,dc=example,dc=com -w freeipa8
"(krbprincipalname:caseExactIA5Match:=user1005...@example.com)"'

3) ldapsearch -h localhost -D "cn=Directory Manager" -b
cn=users,cn=accounts,dc=example,dc=com -w freeipa8
"(krbprincipalname=user1005...@example.com)"

The first two was slow and only the last one was quick.

Sounds like DS do a full search instead of indexed searches when you
specify a matching rule ?
We should either make sure DS can use indexed searches in this case or
go with the plan I proposed earlier.

Simo.

Yes that is correct, those searches with matching rule are unindexed. I 
think the reason is #48270 that reject the 

[Freeipa-devel] Slow email responses this week from FreeIPA/SSSD teams at Red Hat

[Alexander Bokovoy]

Hi everyone!

We have a gathering of Red Hat members of FreeIPA and SSSD teams in
Brno, Czech Republic this week with a lot of design and discussion
meetings. Naturally, we try to lock ourselves down in dungeons without
wifi access and without laptops (not!) to avoid distractions and great
weather of early autumn in Southern Moravia. This has unfortunate effect
of reducing our availability on the mailing lists and IRC channels.

We are apologizing in case you have something urgent to help with and
hope that someone will be able to help as time permits.

Once we re-emerge from the dungeons of Red Hat Brno offices, there
will be wiki updates and blog posts about what is discussed and
reflected on. At least, I have plans to do so on a number of topics.

On a brighter note, FreeIPA 4.2.1 is on its way to Fedora 23
repositories. It is currently pending the acceptance to updates-testing
repository so we most likely miss Fedora 23 beta release but it gives us
chances to test FreeIPA 4.1 to 4.2.1 upgrade path before final Fedora 23
release later this autumn.

https://bodhi.fedoraproject.org/updates/FEDORA-2015-15284

Once packages are in the repositories, we'll send a proper announcement
of FreeIPA 4.2.1 release.
--
/ Alexander Bokovoy

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

[David Kupka]

On 28/08/15 13:36, Martin Basti wrote:



On 08/28/2015 10:03 AM, Petr Spacek wrote:

On 27.8.2015 14:22, David Kupka wrote:

@@ -2101,11 +2101,25 @@ class DNSZoneBase(LDAPObject):
  class DNSZoneBase_add(LDAPCreate):
+takes_options = LDAPCreate.takes_options + (
+Flag('force',
+ label=_('Force'),
+ doc=_('Force DNS zone creation.')
+),
+Flag('skip_overlap_check',
+ doc=_('Force DNS zone creation even if it will overlap
with '
+   'existing zone.')
+),
+)
+
  has_output_params = LDAPCreate.has_output_params +
dnszone_output_params
  def pre_callback(self, ldap, dn, entry_attrs, attrs_list,
*keys, **options):
  assert isinstance(dn, DN)
+if options['force']:
+options['skip_overlap_check'] = True
+
  try:
  entry = ldap.get_entry(dn)
  except errors.NotFound:
@@ -2120,6 +2134,12 @@ class DNSZoneBase_add(LDAPCreate):
  entry_attrs['idnszoneactive'] = 'TRUE'
+if not options['skip_overlap_check']:
+try:
+check_zone_overlap(keys[-1])
+except RuntimeError as e:
+raise errors.InvocationError(e.message)
+
  return dn
@@ -2673,9 +2693,9 @@ class dnszone_add(DNSZoneBase_add):
  __doc__ = _('Create new DNS zone (SOA record).')
  takes_options = DNSZoneBase_add.takes_options + (
-Flag('force',
- label=_('Force'),
- doc=_('Force DNS zone creation even if nameserver is
not resolvable.'),
+Flag('skip_nameserver_check',
+ doc=_('Force DNS zone creation even if nameserver is not '
+   'resolvable.')
  ),
  # Deprecated
@@ -2699,6 +2719,9 @@ class dnszone_add(DNSZoneBase_add):
  def pre_callback(self, ldap, dn, entry_attrs, attrs_list,
*keys, **options):
  assert isinstance(dn, DN)
+if options['force']:
+options['skip_nameserver_check'] = True

Why is it in DNSZoneBase_add.pre_callback?

Shouldn't the equation force = (skip_nameserver_check +
skip_nameserver_check)
be handled in parameter parsing & validation? (Again, I do not know
the IPA
framework :-))


+
  dn = super(dnszone_add, self).pre_callback(
  ldap, dn, entry_attrs, attrs_list, *keys, **options)
@@ -2713,7 +2736,7 @@ class dnszone_add(DNSZoneBase_add):
  error=_("Nameserver for reverse zone cannot be
a relative DNS name"))
  # verify if user specified server is resolvable
-if not options['force']:
+if not options['skip_nameserver_check']:
  check_ns_rec_resolvable(keys[0],
entry_attrs['idnssoamname'])
  # show warning about --name-server option
  context.show_warning_nameserver_option = True
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index
d959bb369d946217acd080e78483cc9013dda4c7..18f477d4fb6620090b7073689c8df51b65a8307a
100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -924,6 +924,20 @@ def host_exists(host):
  else:
  return True
+def check_zone_overlap(zone):
+if resolver.zone_for_name(zone) == zone:
+try:
+ns = [ans.to_text() for ans in resolver.query(zone, 'NS')]
+except DNSException as e:
+root_logger.debug("Failed to resolve nameserver(s) for
domain"
+" {0}: {1}".format(zone, e))
+ns = []
+
+msg = u"DNS zone {0} already exists".format(zone)

Nitpick: I would say "already exists in DNS" to make it absolutely
clear. Just
'already exists' might be confusing because ipa dnszone-show will say
that the
zone does not exist ...


+if ns:
+msg += u" and is handled by server(s): {0}".format(',
'.join(ns))
+raise RuntimeError(msg)
+
  def get_ipa_basedn(conn):
  """
  Get base DN of IPA suffix in given LDAP server.

0064
NACK

ipa-replica-install should have the --skip-overlap-check too, because
any replica can be the first DNS server.


Thanks for the catch, added.



0064+0058
Can be the options --allow-zone-overlap and --skip-overlap-check merged
into an one name, to have just one name for same thing?



Each option has bit different behavior:
The '--skip-overlap-check' option in API call prevent the check to be 
performed and thus no error or warning is raised. This is the way 
'--force' option was originally working.


The '--allow-zone-overlap' options in installers do not skip the check 
but change the error to warning instead and let the installation continue.


If you think that this can confuse users we need to change the names  or 
even the logic.


Updated patches attached.

--
David Kupka
From 816ee3102ee0603450f897f8f6bfed4d5460636c Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Fri, 21 Aug 2015 13:25:34 +0200
Subject: [PATCH 2/2] dns: Check if domain already exists.

Raise an error when the domain already 

[Freeipa-devel] [PATCH 0311] tests: fix vault tests

[Martin Basti]

Attached patch fixes vault tests.
From dd83cf44e1529be53990978e77d47a0d22c85a8b Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Fri, 4 Sep 2015 13:32:56 +0200
Subject: [PATCH] FIX vault tests

---
 ipatests/test_xmlrpc/test_vault_plugin.py | 39 ++-
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_vault_plugin.py b/ipatests/test_xmlrpc/test_vault_plugin.py
index 758d864ce6d100dfdafa89f45c875e8beba447bd..6992005b8ce982866d62de15be7348ba417645ce 100644
--- a/ipatests/test_xmlrpc/test_vault_plugin.py
+++ b/ipatests/test_xmlrpc/test_vault_plugin.py
@@ -170,6 +170,7 @@ class test_vault_plugin(Declarative):
 'cn': [vault_name],
 'ipavaulttype': [u'standard'],
 'owner_user': [u'admin'],
+'username': u'admin',
 },
 },
 },
@@ -191,6 +192,7 @@ class test_vault_plugin(Declarative):
   % (vault_name, api.env.basedn),
 'cn': [vault_name],
 'ipavaulttype': [u'standard'],
+'username': u'admin',
 },
 ],
 },
@@ -212,6 +214,7 @@ class test_vault_plugin(Declarative):
 'cn': [vault_name],
 'ipavaulttype': [u'standard'],
 'owner_user': [u'admin'],
+'username': u'admin',
 },
 },
 },
@@ -233,6 +236,7 @@ class test_vault_plugin(Declarative):
 'description': [u'Test vault'],
 'ipavaulttype': [u'standard'],
 'owner_user': [u'admin'],
+'username': u'admin',
 },
 },
 },
@@ -267,12 +271,14 @@ class test_vault_plugin(Declarative):
 'value': vault_name,
 'summary': u'Added vault "%s"' % vault_name,
 'result': {
-'dn': u'cn=%s,cn=%s,cn=services,cn=vaults,cn=kra,%s'
-  % (vault_name, service_name, api.env.basedn),
+'dn': u'cn=%s,cn=%s@%s,cn=services,cn=vaults,cn=kra,%s'
+  % (vault_name, service_name, api.env.realm,
+ api.env.basedn),
 'objectclass': [u'top', u'ipaVault'],
 'cn': [vault_name],
 'ipavaulttype': [u'standard'],
 'owner_user': [u'admin'],
+'service': "%s@%s" % (service_name, api.env.realm),
 },
 },
 },
@@ -292,10 +298,12 @@ class test_vault_plugin(Declarative):
 'summary': u'1 vault matched',
 'result': [
 {
-'dn': u'cn=%s,cn=%s,cn=services,cn=vaults,cn=kra,%s'
-  % (vault_name, service_name, api.env.basedn),
+'dn': u'cn=%s,cn=%s@%s,cn=services,cn=vaults,cn=kra,%s'
+  % (vault_name, service_name, api.env.realm,
+ api.env.basedn),
 'cn': [vault_name],
 'ipavaulttype': [u'standard'],
+'service': '%s@%s' % (service_name, api.env.realm),
 },
 ],
 },
@@ -314,11 +322,13 @@ class test_vault_plugin(Declarative):
 'value': vault_name,
 'summary': None,
 'result': {
-'dn': u'cn=%s,cn=%s,cn=services,cn=vaults,cn=kra,%s'
-  % (vault_name, service_name, api.env.basedn),
+'dn': u'cn=%s,cn=%s@%s,cn=services,cn=vaults,cn=kra,%s'
+  % (vault_name, service_name, api.env.realm,
+ api.env.basedn),
 'cn': [vault_name],
 'ipavaulttype': [u'standard'],
 'owner_user': [u'admin'],
+'service': "%s@%s" % (service_name, api.env.realm),
 },
 },
 },
@@ -341,6 +351,7 @@ class test_vault_plugin(Declarative):
 'description': [u'Test vault'],
 'ipavaulttype': [u'standard'],
 'owner_user': [u'admin'],
+'service': "%s@%s" % (service_name, api.env.realm),
 },
 },
 },
@@ -383,6 +394,7 @@ class test_vault_plugin(Declarative):
 'cn': [vault_name],
 'ipavaulttype': [u'standard'],
 'owner_user': [u'admin'],
+'shared': True,
 },
 },
 },
@@ -406,6 +418,7 @@ class test_vault_plugin(Declarative):
   % (vault_name, api.env.basedn),
 'cn': 

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

[David Kupka]

On 28/08/15 10:03, Petr Spacek wrote:

On 27.8.2015 14:22, David Kupka wrote:

@@ -2101,11 +2101,25 @@ class DNSZoneBase(LDAPObject):

  class DNSZoneBase_add(LDAPCreate):

+takes_options = LDAPCreate.takes_options + (
+Flag('force',
+ label=_('Force'),
+ doc=_('Force DNS zone creation.')
+),
+Flag('skip_overlap_check',
+ doc=_('Force DNS zone creation even if it will overlap with '
+   'existing zone.')
+),
+)
+
  has_output_params = LDAPCreate.has_output_params + dnszone_output_params

  def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, 
**options):
  assert isinstance(dn, DN)

+if options['force']:
+options['skip_overlap_check'] = True
+
  try:
  entry = ldap.get_entry(dn)
  except errors.NotFound:
@@ -2120,6 +2134,12 @@ class DNSZoneBase_add(LDAPCreate):

  entry_attrs['idnszoneactive'] = 'TRUE'

+if not options['skip_overlap_check']:
+try:
+check_zone_overlap(keys[-1])
+except RuntimeError as e:
+raise errors.InvocationError(e.message)
+
  return dn


@@ -2673,9 +2693,9 @@ class dnszone_add(DNSZoneBase_add):
  __doc__ = _('Create new DNS zone (SOA record).')

  takes_options = DNSZoneBase_add.takes_options + (
-Flag('force',
- label=_('Force'),
- doc=_('Force DNS zone creation even if nameserver is not 
resolvable.'),
+Flag('skip_nameserver_check',
+ doc=_('Force DNS zone creation even if nameserver is not '
+   'resolvable.')
  ),

  # Deprecated
@@ -2699,6 +2719,9 @@ class dnszone_add(DNSZoneBase_add):
  def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, 
**options):
  assert isinstance(dn, DN)

+if options['force']:
+options['skip_nameserver_check'] = True


Why is it in DNSZoneBase_add.pre_callback?

Shouldn't the equation force = (skip_nameserver_check + skip_nameserver_check)
be handled in parameter parsing & validation? (Again, I do not know the IPA
framework :-))



IIUC it is usually handled in pre_callback because framework does not 
provide any other mechanism preprocess and validate options.



+
  dn = super(dnszone_add, self).pre_callback(
  ldap, dn, entry_attrs, attrs_list, *keys, **options)

@@ -2713,7 +2736,7 @@ class dnszone_add(DNSZoneBase_add):
  error=_("Nameserver for reverse zone cannot be a relative DNS 
name"))

  # verify if user specified server is resolvable
-if not options['force']:
+if not options['skip_nameserver_check']:
  check_ns_rec_resolvable(keys[0], entry_attrs['idnssoamname'])
  # show warning about --name-server option
  context.show_warning_nameserver_option = True
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 
d959bb369d946217acd080e78483cc9013dda4c7..18f477d4fb6620090b7073689c8df51b65a8307a
 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -924,6 +924,20 @@ def host_exists(host):
  else:
  return True

+def check_zone_overlap(zone):
+if resolver.zone_for_name(zone) == zone:
+try:
+ns = [ans.to_text() for ans in resolver.query(zone, 'NS')]
+except DNSException as e:
+root_logger.debug("Failed to resolve nameserver(s) for domain"
+" {0}: {1}".format(zone, e))
+ns = []
+
+msg = u"DNS zone {0} already exists".format(zone)


Nitpick: I would say "already exists in DNS" to make it absolutely clear. Just
'already exists' might be confusing because ipa dnszone-show will say that the
zone does not exist ...


Ok, will update this.




+if ns:
+msg += u" and is handled by server(s): {0}".format(', '.join(ns))
+raise RuntimeError(msg)
+
  def get_ipa_basedn(conn):
  """
  Get base DN of IPA suffix in given LDAP server.





--
David Kupka

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code