Re: [Freeipa-devel] [PATCH] 381, 387 Preserve order of servers in ipa-client-install

2013-03-14 Thread Martin Kosek
On 03/13/2013 06:43 PM, Rob Crittenden wrote:
> Martin Kosek wrote:
>> On 03/13/2013 02:58 PM, Martin Kosek wrote:
>>> On 03/11/2013 12:40 PM, Martin Kosek wrote:
 On 03/07/2013 03:07 PM, Petr Viktorin wrote:
> On 03/07/2013 02:00 PM, Martin Kosek wrote:
>> When multiple servers are passed via --server option, ipadiscovery
>> module changed its order. Make sure that we preserve it.
>>
>> Also make sure that user is always warned when a tested server is
>> not available as then the server will be excluded from the fixed
>> server list.
>
> The message doesn't actually say that the server will be removed. It 
> would be
> nice if it did.
>
> Otherwise, ACK.

 Sending a patch with improved logging. User should now be more clear what
 server is failing to verify (and why).

>
>> --
>>
>> When working on this ticket I was thinking - do we make the right thing 
>> we
>> deliberately remove a server from user-provided server list just because 
>> we
>> cannot connect to it at the moment if discovery? It may just be 
>> temporarily
>> down or something.
>>
>> Maybe we should preserve the original --server list in this case and use
>> this
>> list when writing krb5.conf or sssd.conf. Of course, for ipa-join or 
>> other
>> active configuration commands we would have to use only the valid 
>> servers so
>> that the we do not hit the server that is currently down.
>>
>> Martin
>
> Good point, this deserves a ticket.
>

 Rob, do you think this deserves to be changed? Or is this behavior indeed
 intended?

 Martin

>>>
>>> Sending a rebased patch 381, logging was also improved. Client discovery
>>> logging now looks like that:
>>>
>>> # ipa-client-install
>>> Skip vm-024.idm.lab.bos.redhat.com: not an IPA server
>>> Skip doesnotexist.test: cannot verify if this is an IPA server
>>> Discovery was successful!
>>> Hostname: vm-148.idm.lab.bos.redhat.com
>>> Realm: IDM.LAB.BOS.REDHAT.COM
>>> DNS Domain: idm.lab.bos.redhat.com
>>> IPA Server: vm-037.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]:
>>> ...
>>>
>>> I also attached a related fix for redundant discoveries with fixed server 
>>> list
>>> (found that when testing logging), details are in the patch description.
>>>
>>> Martin
>>>
>>
>> I just creating a conflict in my patch numbering, renaming 386 to 387...
>>
>> Martin
>>
> 
> ACK.
> 
> I think we probably want this in the 3-1 branch too but some merging is 
> needed,
> so I'll leave that up to you Martin :-)
> 
> rob
> 

Thanks. I rebased that for ipa-3-1 (and did some smoke-testing), it works OK
there too.

Pushed to master, ipa-3-1.

Martin

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


Re: [Freeipa-devel] [PATCH] 376-377, 385 Use tkey-gssapi-keytab in named.conf

2013-03-14 Thread Martin Kosek
On 03/13/2013 06:06 PM, Rob Crittenden wrote:
> Martin Kosek wrote:
>> On 03/11/2013 09:39 AM, Petr Spacek wrote:
>>> On 11.3.2013 09:09, Martin Kosek wrote:
 On 03/08/2013 09:49 AM, Petr Spacek wrote:
> On 8.3.2013 00:14, Rob Crittenden wrote:
>> Martin Kosek wrote:
>>> Remove obsolete BIND GSSAPI configuration options tkey-gssapi-credential
>>> and tkey-domain and replace them with tkey-gssapi-keytab which avoids
>>> unnecessary Kerberos checks on BIND startup and can cause issues when
>>> KDC is not available.
>>>
>>> Both new and current IPA installations are updated.
>>>
>>> https://fedorahosted.org/freeipa/ticket/3429
>>>
>>
>> Still reviewing this but I noticed that after upgrading my 3.1.99 server
>> pre-patch to with with-patch version the connections argument in 
>> named.conf
>> got set to 4 (courtesy of ipa-upgradeconfig). Should we be setting that 
>> to 4
>> during the initial install too?
>
> For 3.2 it doesn't matter. Anything >= 2 should be okay, but more 
> connections
> should not harm.
>
> Higher value should allow higher level of parallelism, it is one of tuning
> parameters. Value 4 was necessary to prevent deadlocks in some previous
> versions of bind-dyndb-ldap.
>

 Previously, when I implemented the upgrade script, I set connections arg 
 only
 if it was present in named.conf and thus bind-dyndb-ldap could not use a
 reasonable default on its own decision.

 This was changed in e578183ea25a40aedf6dcc3e1ee4bcb19b73e70f and 
 connections
 is set always. Rob is correct, that in that case we might want to add it to
 named.conf by default to make it consistent... or we could also fix upgrade
 script to change connections only if it is present in named.conf.

 Petr, what does make more sense bind-dyndb-ldap wise?
>>>
>>> Default values should work. Personally I would include only "override" 
>>> values
>>> in named.conf, but technically it doesn't matter.
>>>
>>> Note: Latest bind-dyndb-ldap versions refuse to start if configuration is
>>> insane. Fatal error will point admin to errors in configuration and should
>>> prevent surprises from auto-magically changed values.
>>>
>>> Invalid configurations - examples:
>>> connections < 1
>>> connections < 2 && psearch is enabled
>>> serial_autoincrement enabled && psearch disabled
>>>
>>
>> Ok, lets set the connections argument only if it is in named.conf _and_ it is
>> lower than the required minimum. All patches attached.
>>
>> Martin
>>
> 
> This works ok if the format of named.conf is stable.
> 
> If, for example, there are extra spaces between options and { then the values
> are not updated. This is nothing new with this patch, our previous code was
> also space dependent (augeas will address this eventually)
> 
> I just wonder: Should we log if a warning if a change has not been applied?
> 
> rob

There is already a warning if we could not match tkey-gssapi-credential,
tkey-domain or tkey-gssapi-keytab. It would look like that:

# ipa-upgradeconfig --quiet
Either tkey-gssapi-credential or tkey-domain is missing in /etc/named.conf.
Skip update.

At least I made our crappy named.conf parser more resilient to spaces in
section start (i.e. it should now work with "options  {" and made the
regular expression object naming more consistent. But you are right, this will
be eventually improved by augueas.

Important thing for now is, that our updater works fine with our template
named.conf we ship with freeipa including user changes, if user does not go too
wild into breaking what's already there...

Martin
From 16063578811053f5a6cd6ed281c868847cb7d492 Mon Sep 17 00:00:00 2001
From: Martin Kosek 
Date: Tue, 5 Mar 2013 12:02:58 +0100
Subject: [PATCH 1/3] Update named.conf parser

Refactor the named.conf parsing and editing functions in bindinstance
so that both "dynamic-db" and "options" sections of named.conf can
be read and updated

https://fedorahosted.org/freeipa/ticket/3429
---
 ipaserver/install/bindinstance.py | 69 +++
 1 file changed, 48 insertions(+), 21 deletions(-)

diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index c14f2423ebecbe5bfb7ae9a719754077f826fcf0..20d3349558e6e916ab0c26d3fa8ba0baf12994ad 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -42,8 +42,13 @@ from ipalib.util import (validate_zonemgr, normalize_zonemgr,
 NAMED_CONF = '/etc/named.conf'
 RESOLV_CONF = '/etc/resolv.conf'
 
-named_conf_ipa_re = re.compile(r'(?P\s*)arg\s+"(?P\S+)\s(?P[^"]+)";')
-named_conf_ipa_template = "%(indent)sarg \"%(name)s %(value)s\";\n"
+named_conf_section_ipa_start_re = re.compile('\s*dynamic-db\s+"ipa"\s+{')
+named_conf_section_options_start_re = re.compile('\s*options\s+{')
+named_conf_section_end_re = re.compile('};')
+named_conf_arg_ipa_re = re.compile(r'(?P\s*)arg

Re: [Freeipa-devel] [PATCH 0039] Enforce exact SID match when adding or modifying a ID range

2013-03-14 Thread Tomas Babej

On 03/13/2013 05:23 PM, Martin Kosek wrote:

On 03/13/2013 09:50 AM, Tomas Babej wrote:

On Wed 13 Mar 2013 09:47:09 AM CET, Tomas Babej wrote:

Hi,

SID validation in idrange.py now enforces exact match on SIDs, thus
one can no longer use SID of an object in a trusted domain as a
trusted domain SID.

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

Tomas


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

Just renamed the patch filename to follow the convention.

Tomas


I do not think that the debug message is needed:

+root_logger.error('No trusted domain with given SID found, '
+  'listing SIDS for all the trusted domains:')
+for domain in self._domains:
+root_logger.error('SID: %s' % self._domains[domain][1])

User will not see it anyway and he can easily get list of SIDs/domains with
"ipa trust-find".

Otherwise the patch looks and works fine. I would just consider renaming the
method from is_trusted_sid_valid_domain to is_trusted_domain_sid_valid. Sounds
better to me, but I have no strong feelings about that.

Martin

Both fixed.

Tomas

>From ee475ccbf0a7849bd8fbd9dba4f79a1b2880f097 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Wed, 6 Mar 2013 12:17:28 +0100
Subject: [PATCH] Enforce exact SID match when adding or modifying a ID range

SID validation in idrange.py now enforces exact match on SIDs, thus
one can no longer use SID of an object in a trusted domain as a
trusted domain SID.

https://fedorahosted.org/freeipa/ticket/3432
---
 ipalib/plugins/idrange.py |  2 +-
 ipaserver/dcerpc.py   | 50 +++
 2 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index d8989327a24af2d4aa278a215d934b9ca0fab87b..54f6fbb3e19b9aa01dfde2a8d0c5da4498632386 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -289,7 +289,7 @@ class idrange(LDAPObject):
 
 domain_validator = self.get_domain_validator()
 
-if not domain_validator.is_trusted_sid_valid(sid):
+if not domain_validator.is_trusted_domain_sid_valid(sid):
 raise errors.ValidationError(name='domain SID',
   error=_('SID is not recognized as a valid SID for a '
   'trusted domain'))
diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py
index b8f83e9a479d2d68cac46000500ddb1051251a22..6253c44fec786572e56883c42b9de7de02b1c352 100644
--- a/ipaserver/dcerpc.py
+++ b/ipaserver/dcerpc.py
@@ -183,37 +183,53 @@ class DomainValidator(object):
 except errors.NotFound, e:
 return []
 
-def get_domain_by_sid(self, sid):
+def get_domain_by_sid(self, sid, exact_match=False):
 if not self.domain:
 # our domain is not configured or self.is_configured() never run
 # reject SIDs as we can't check correctness of them
 raise errors.ValidationError(name='sid',
   error=_('domain is not configured'))
+
 # Parse sid string to see if it is really in a SID format
 try:
 test_sid = security.dom_sid(sid)
-except TypeError, e:
+except TypeError:
 raise errors.ValidationError(name='sid',
   error=_('SID is not valid'))
+
 # At this point we have SID_NT_AUTHORITY family SID and really need to
 # check it against prefixes of domain SIDs we trust to
 if not self._domains:
 self._domains = self.get_trusted_domains()
 if len(self._domains) == 0:
 # Our domain is configured but no trusted domains are configured
-# This means we can't check the correctness of a trusted domain SIDs
+# This means we can't check the correctness of a trusted
+# domain SIDs
 raise errors.ValidationError(name='sid',
   error=_('no trusted domain is configured'))
-# We have non-zero list of trusted domains and have to go through them
-# one by one and check their sids as prefixes
-test_sid_subauths = test_sid.sub_auths
-for domain in self._domains:
-domsid = self._domains[domain][1]
-sub_auths = domsid.sub_auths
-num_auths = min(test_sid.num_auths, domsid.num_auths)
-if test_sid_subauths[:num_auths] == sub_auths[:num_auths]:
-return domain
-raise errors.NotFound(reason=_('SID does not match any trusted domain'))
+
+# We have non-zero list of trusted domains and have to go through
+# them one by one and check their sids as prefixes / exact match
+# depending on the value of exact_match flag
+if exact_match:
+# check exact match of sids
+for domain in self._domains:
+if sid == str(self._domains[do

[Freeipa-devel] [PATCH] 388-389 Improve client install LDAP cert retrieval fallback

2013-03-14 Thread Martin Kosek
[freeipa-mkosek-388-use-temporary-ccache-in-ipa-client-install.patch]:

ipa-client-install failed if user had set his own KRB5CCNAME in his
environment. Use a temporary CCACHE for the installer to avoid these
kind of errors.

[freeipa-mkosek-389-improve-client-install-ldap-cert-retrieval-fallback.patch]:

CA certificate retrieval function did not fallback from LDAP to
HTTP based retrieval in case of an LDAP error, when for example
GSSAPI authentication failed.

-

Sending Fedora 18 client installation fixes  as per
https://bugzilla.redhat.com/show_bug.cgi?id=920716#c10

Martin
From d837418d9424938823a9793ce72de742967bbfd5 Mon Sep 17 00:00:00 2001
From: Martin Kosek 
Date: Thu, 14 Mar 2013 14:33:56 +0100
Subject: [PATCH 1/2] Use temporary CCACHE in ipa-client-install

ipa-client-install failed if user had set his own KRB5CCNAME in his
environment. Use a temporary CCACHE for the installer to avoid these
kind of errors.

https://fedorahosted.org/freeipa/ticket/3512
---
 ipa-client/ipa-install/ipa-client-install | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index d9e1b7e786466ba11fb8fd1d00a72904dfcc0005..fc8b6c85598a6d5b8d7ff3d53dd4db6d9b001b51 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -1979,6 +1979,9 @@ def install(options, env, fstore, statestore):
 root_logger.error("Test kerberos configuration failed")
 return CLIENT_INSTALL_ERROR
 env['KRB5_CONFIG'] = krb_name
+(ccache_fd, ccache_name) = tempfile.mkstemp()
+os.close(ccache_fd)
+env['KRB5CCNAME'] = os.environ['KRB5CCNAME'] = ccache_name
 join_args = ["/usr/sbin/ipa-join", "-s", cli_server[0], "-b", str(realm_to_suffix(cli_realm))]
 if options.debug:
 join_args.append("-d")
@@ -2114,6 +2117,10 @@ def install(options, env, fstore, statestore):
 except OSError:
 root_logger.error("Could not remove %s", krb_name)
 try:
+os.remove(ccache_name)
+except OSError:
+pass
+try:
 os.remove(krb_name + ".ipabkp")
 except OSError:
 root_logger.error("Could not remove %s.ipabkp", krb_name)
-- 
1.8.1.4

From 429b5390e1e75be400ccb7aaa3e2ed4b72b359e2 Mon Sep 17 00:00:00 2001
From: Martin Kosek 
Date: Thu, 14 Mar 2013 14:36:39 +0100
Subject: [PATCH 2/2] Improve client install LDAP cert retrieval fallback

CA certificate retrieval function did not fallback from LDAP to
HTTP based retrieval in case of an LDAP error, when for example
GSSAPI authentication failed.

https://fedorahosted.org/freeipa/ticket/3512
---
 ipa-client/ipa-install/ipa-client-install | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index fc8b6c85598a6d5b8d7ff3d53dd4db6d9b001b51..f1b2c1887a1f393c4ac6ca004deee80ff52b2ca7 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -1624,7 +1624,7 @@ def get_ca_cert(fstore, options, server, basedn):
 except Exception, e:
 os.unlink(ca_file)
 raise
-except errors.NoCertificateError, e:
+except (errors.NoCertificateError, errors.LDAPError), e:
 root_logger.debug(str(e))
 url = http_url()
 if existing_ca_cert:
-- 
1.8.1.4

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

Re: [Freeipa-devel] [PATCH] 0006 Remove check for alphabetic only characters from domain name validation

2013-03-14 Thread Martin Kosek
On 03/11/2013 01:02 PM, Ana Krivokapic wrote:
> On 02/27/2013 10:58 AM, Martin Kosek wrote:
>> On 02/22/2013 04:02 PM, Ana Krivokapic wrote:
>>> On 02/22/2013 10:19 AM, Petr Spacek wrote:
 On 20.2.2013 11:03, Ana Krivokapic wrote:
> On 02/18/2013 01:08 PM, Martin Kosek wrote:
>> On 02/18/2013 12:47 PM, Sumit Bose wrote:
>>> On Mon, Feb 18, 2013 at 12:27:35PM +0100, Petr Spacek wrote:
 On 15.2.2013 15:22, Ana Krivokapic wrote:
> Hello,
>
> The .isalpha() check in validate_domain_name() was too strict,
> causing some commands like ipa dnsrecord-add to fail.
>
> https://fedorahosted.org/freeipa/ticket/3385
 I would add --force option rather than removing whole check, if
 it's possible.

 Would it be possible to mention RFC in the error message? Something
 like _('top level domain label must be alphabetic (RFC 1123 section
 2.1)')
 ?

 IMHO it is handy, because it educates users.
>>> The problem is that this check is always done on the last component of
>>> the domain_name even if it is just a sub-domain of the FreeIPA domain,
>>> where e.g. numbers are valid characters.
>>>
>>> At the beginning of validate_domain_name() a trailing '.' is stripped
>>> away. iirc the trailing '.' is an indication for a complete, fully
>>> qualified name. Would it work if the presence of the trailing '.' is
>>> saved and the check is only done if there was a '.'?
>>>
>>> bye,
>>> Sumit
>>>
>> Sure. Though I am now not 100% sure that some IPA functions do not
>> use this
>> validator with a fqdn hostname without trailing dot. If not, I am
>> for fixing
>> this function as Sumit and Petr suggested.
>>
>> Martin
>>
>> ___
>> Freeipa-devel mailing list
>> Freeipa-devel@redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
> After some thought, I decided to change the approach.
>
> As pointed out by Sumit, the problem was that the validate_domain_name()
> function did not distinguish between fqdn and non-fqdn domains
> (subdomains of the IPA domain). The trailing dot is not a clear
> indication either, because some IPA functions use this validator with an
> fqdn without the trailing dot.
>
> To fix this, I introduced an additional parameter to this function - a
> flag which indicates whether the domain name is an fqdn or not. The is
> .isalpha() check is then performed only in the case of an fqdn.
>
> I also improved the error message to mention the relevant RFC, as
> suggested by Petr.
 Please don't forget to add --force switch. It could be handy.

>>> I added the --force switch to ipa dnsrecord-add and opened a new ticket
>>> to handle the rest of the ipa commands that use domain name validation:
>>> https://fedorahosted.org/freeipa/ticket/3455
>>>
>>> Updated patch is attached.
>>>
>> This patch fixed validation only partially. The --force flag you made 
>> available
>> will not allow admin to for example add a zone "example.zone1" which
>> technically will be resolvable, it is just not a good practice:
>>
>> # ipa dnszone-add example.zone1 --name-server `hostname`. --force
>> ipa: ERROR: invalid 'name': top level domain label must be alphabetic (RFC 
>> 1123
>> section 2.1)
>>
>> To enable this, I think you would need to not postpone the validation to DNS
>> zone pre_callback as you could not check --force flag presence right in the
>> idnsname parameter validator.
>>
>> We may also want to change --force flag label, it now talks only about NS
>> record validation, but we now expanded it a bit, so the label would need to 
>> be
>> more general.
>>
>> Martin
> 
> I added the fix for dnszone-add and edited the label of the --force flag
> to make it more general.
> 

The zone with this name can be created even without the --force flag.

# ipa dnszone-add example.zone1 --name-server `hostname`.
Administrator e-mail address [hostmaster.example.zone1.]:
>>> Administrator e-mail address: top level domain label must be alphabetic
(RFC 1123 section 2.1)
Administrator e-mail address [hostmaster.example.zone1.]: 
hostmaster.example.zone.
  Zone name: example.zone1
  Authoritative nameserver: vm-037.idm.lab.bos.redhat.com.
  Administrator e-mail address: hostmaster.example.zone.
  SOA serial: 1363268183
  SOA refresh: 3600
  SOA retry: 900
  SOA expire: 1209600
  SOA minimum: 3600
  BIND update policy: grant IDM.LAB.BOS.REDHAT.COM krb5-self * A; grant
IDM.LAB.BOS.REDHAT.COM krb5-self
  * ; grant IDM.LAB.BOS.REDHAT.COM krb5-self * SSHFP;
  Active zone: TRUE
  Dynamic update: FALSE
  Allow query: any;
  Allow transfer: none;

I thought that the check would only be surpassed with the --force flag.

All this struggle with this patch makes me thinking if we are not ma

Re: [Freeipa-devel] [PATCH 0039] Enforce exact SID match when adding or modifying a ID range

2013-03-14 Thread Martin Kosek
On 03/14/2013 10:48 AM, Tomas Babej wrote:
> On 03/13/2013 05:23 PM, Martin Kosek wrote:
>> On 03/13/2013 09:50 AM, Tomas Babej wrote:
>>> On Wed 13 Mar 2013 09:47:09 AM CET, Tomas Babej wrote:
 Hi,

 SID validation in idrange.py now enforces exact match on SIDs, thus
 one can no longer use SID of an object in a trusted domain as a
 trusted domain SID.

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

 Tomas


 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel
>>> Just renamed the patch filename to follow the convention.
>>>
>>> Tomas
>>>
>> I do not think that the debug message is needed:
>>
>> +root_logger.error('No trusted domain with given SID found, '
>> +  'listing SIDS for all the trusted domains:')
>> +for domain in self._domains:
>> +root_logger.error('SID: %s' % self._domains[domain][1])
>>
>> User will not see it anyway and he can easily get list of SIDs/domains with
>> "ipa trust-find".
>>
>> Otherwise the patch looks and works fine. I would just consider renaming the
>> method from is_trusted_sid_valid_domain to is_trusted_domain_sid_valid. 
>> Sounds
>> better to me, but I have no strong feelings about that.
>>
>> Martin
> Both fixed.
> 
> Tomas
> 

ACK. Pushed to master, ipa-3-1.

Martin

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


Re: [Freeipa-devel] [PATCH] 0002 Add missing error message when adding duplicate external member to group

2013-03-14 Thread Rob Crittenden

Ana Krivokapic wrote:

On 02/19/2013 09:46 PM, Rob Crittenden wrote:

Ana Krivokapic wrote:

When adding a duplicate member to a group, an error message is issued,
informing the user that the entry is already a member of the group. This
message was missing in case of an external member.

Ticket: https://fedorahosted.org/freeipa/ticket/3254


This works ok but the sister command, group-remove-member, has the
same problem. Can you add a fix there as well?

I don't know if there is a way to add a unit test for this since the
external member is validated meaning we'd need to set up trusts as
well. It might be nice to have an optional test that can be run when a
trust is configured to avoid regressions.

rob


I fixed the group-remove-member command and added unit tests which can
be run when the trust is established (they will be skipped when the
trust is not established).

I also noticed that, in contrast to group-add-member,
group-remove-member did not allow the format 'AD\name' or
'n...@ad.domain.com' for the --external option. I included this fix in
the patch, so the two user friendly formats are now supported.

Updated patch is attached.



Remove member is still not working for me:

$ ipa group-remove-member ad_admins_external --external 'AD\Domain Admins'
[member user]:
[member group]:
  Group name: ad_admins_external
  Description: ad.greyoak.com admins external map
  External member: S-1-5-21-2065961537-1042332738-1594543940-512

$ ipa group-remove-member ad_admins_external --external 
'S-1-5-21-2065961537-1042332738-1594543940-512'

[member user]:
[member group]:
  Group name: ad_admins_external
  Description: ad.greyoak.com admins external map
  External member:
---
Number of members removed 1
---

Removing it again doesn't return an error:

$ ipa group-remove-member ad_admins_external --external 
'S-1-5-21-2065961537-1042332738-1594543940-512'

[member user]:
[member group]:

  Group name: ad_admins_external
  Description: ad.greyoak.com admins external map
---
Number of members removed 0
---

rob

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


Re: [Freeipa-devel] [PATCH] 376-377, 385 Use tkey-gssapi-keytab in named.conf

2013-03-14 Thread Rob Crittenden

Martin Kosek wrote:

On 03/13/2013 06:06 PM, Rob Crittenden wrote:

Martin Kosek wrote:

On 03/11/2013 09:39 AM, Petr Spacek wrote:

On 11.3.2013 09:09, Martin Kosek wrote:

On 03/08/2013 09:49 AM, Petr Spacek wrote:

On 8.3.2013 00:14, Rob Crittenden wrote:

Martin Kosek wrote:

Remove obsolete BIND GSSAPI configuration options tkey-gssapi-credential
and tkey-domain and replace them with tkey-gssapi-keytab which avoids
unnecessary Kerberos checks on BIND startup and can cause issues when
KDC is not available.

Both new and current IPA installations are updated.

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



Still reviewing this but I noticed that after upgrading my 3.1.99 server
pre-patch to with with-patch version the connections argument in named.conf
got set to 4 (courtesy of ipa-upgradeconfig). Should we be setting that to 4
during the initial install too?


For 3.2 it doesn't matter. Anything >= 2 should be okay, but more connections
should not harm.

Higher value should allow higher level of parallelism, it is one of tuning
parameters. Value 4 was necessary to prevent deadlocks in some previous
versions of bind-dyndb-ldap.



Previously, when I implemented the upgrade script, I set connections arg only
if it was present in named.conf and thus bind-dyndb-ldap could not use a
reasonable default on its own decision.

This was changed in e578183ea25a40aedf6dcc3e1ee4bcb19b73e70f and connections
is set always. Rob is correct, that in that case we might want to add it to
named.conf by default to make it consistent... or we could also fix upgrade
script to change connections only if it is present in named.conf.

Petr, what does make more sense bind-dyndb-ldap wise?


Default values should work. Personally I would include only "override" values
in named.conf, but technically it doesn't matter.

Note: Latest bind-dyndb-ldap versions refuse to start if configuration is
insane. Fatal error will point admin to errors in configuration and should
prevent surprises from auto-magically changed values.

Invalid configurations - examples:
connections < 1
connections < 2 && psearch is enabled
serial_autoincrement enabled && psearch disabled



Ok, lets set the connections argument only if it is in named.conf _and_ it is
lower than the required minimum. All patches attached.

Martin



This works ok if the format of named.conf is stable.

If, for example, there are extra spaces between options and { then the values
are not updated. This is nothing new with this patch, our previous code was
also space dependent (augeas will address this eventually)

I just wonder: Should we log if a warning if a change has not been applied?

rob


There is already a warning if we could not match tkey-gssapi-credential,
tkey-domain or tkey-gssapi-keytab. It would look like that:

# ipa-upgradeconfig --quiet
Either tkey-gssapi-credential or tkey-domain is missing in /etc/named.conf.
Skip update.

At least I made our crappy named.conf parser more resilient to spaces in
section start (i.e. it should now work with "options  {" and made the
regular expression object naming more consistent. But you are right, this will
be eventually improved by augueas.

Important thing for now is, that our updater works fine with our template
named.conf we ship with freeipa including user changes, if user does not go too
wild into breaking what's already there...

Martin



ACK, pushed to master x 3.

rob

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


Re: [Freeipa-devel] [WIP] Web UI Refactoring & plugins effort - current state

2013-03-14 Thread Petr Vobornik

Update of the effort:

Current state in git://fedorapeople.org/~pvoborni/freeipa.git branch menu.

Patches will be squashed later.

Fixed regressions:
 * facet refreshed only when pkeys and args are changed
 * menu has 'memory'. One will return to previously selected facet of 
entity.

 * fixed displaying of 3rd level of navigation in dns, automember
 * singleton objects (config, dnsconfig) update doesn't raise error

+ Some internal changes -mostly removal of get_primary_key calls.
+ Fixed error message while adding group external member - this might 
not be regression caused by these patches (didn't check).


Known problems:
 * dirty dialog is displayed twice

On 03/05/2013 06:34 PM, Petr Vobornik wrote:

Hello,

Sending current state of $subj. It's main purpose is to get rough review
and design comments.

Attaching patches of work done.

The effort is documented at: http://pvoborni.fedorapeople.org/doc

Navigation refactoring
--
* http://pvoborni.fedorapeople.org/doc/navigation.html
* almost implemented

Plugin design
-
* http://pvoborni.fedorapeople.org/doc/plugins.html
* nothing implemented

Known problems
--
* http://pvoborni.fedorapeople.org/doc/known_problems.html

Others
--
As a part of the effort I change some Web UI internals. Some of them are
documented on pages:
* http://pvoborni.fedorapeople.org/doc/phases.html
* http://pvoborni.fedorapeople.org/doc/facet_public_state.html
* http://pvoborni.fedorapeople.org/doc/registers.html

NOTE: all doc pages are written in asciidoc, change extension from .html
to .txt to get the source. I use it because our wiki doesn't handle
source codes well. I plan to gradually create complete documentation of
Web UI.

I will create design page in our wiki later - should be less verbose.

Update testing server with:
$ util/sync.sh --host r...@host.test  -cC --dojo --misc --strings --restart
$ util/sync.sh --host r...@host.test -fc



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




--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 386 Fix client discovery crash

2013-03-14 Thread Rob Crittenden

Martin Kosek wrote:

Client discovery LDAP search assumes that the remote LDAP server will
send an entry with lowercase attribute names. When it discovers for
example on openldap which sends it in CamelCase, the discovery
crashes.

Convert retrieved entry to CIDict to avoid this error. Also add
a fallback to ipa-client-install server discovery process so that
it rather skips the faulty server instead of crashing.

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

---

NOTE: this is just a hotfix for IPA 3.1.x (ipa-3-1 branch). Proper fix was done
by Petr Viktorin (patches 0191-0195).

Martin


ACK. I'll let you push this, it didn't apply cleanly to my 3-1 branch.

rob

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


Re: [Freeipa-devel] [PATCH] 1089 fix some ipa-replica-manage error handling

2013-03-14 Thread Martin Kosek
On 03/01/2013 11:58 PM, Rob Crittenden wrote:
> I found a couple of problems in error handling in ipa-replica-manage when 
> doing
> some oddball testing. See the ticket for full details.
> 
> This may apply by itself but in my tree it exists after patch 1088.
> 
> rob
> 

Looks and works OK, ACK!

I think it would make sense to also rebase it for ipa-3-1 and thus for future
Fedora 18 release. In your first diff chunk, you would just need to catch for
NO_SUCH_OBJECT instead of errors.NotFound...

Martin

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


Re: [Freeipa-devel] [PATCH] 0002 Add missing error message when adding duplicate external member to group

2013-03-14 Thread Rob Crittenden

Rob Crittenden wrote:

Ana Krivokapic wrote:

On 02/19/2013 09:46 PM, Rob Crittenden wrote:

Ana Krivokapic wrote:

When adding a duplicate member to a group, an error message is issued,
informing the user that the entry is already a member of the group.
This
message was missing in case of an external member.

Ticket: https://fedorahosted.org/freeipa/ticket/3254


This works ok but the sister command, group-remove-member, has the
same problem. Can you add a fix there as well?

I don't know if there is a way to add a unit test for this since the
external member is validated meaning we'd need to set up trusts as
well. It might be nice to have an optional test that can be run when a
trust is configured to avoid regressions.

rob


I fixed the group-remove-member command and added unit tests which can
be run when the trust is established (they will be skipped when the
trust is not established).

I also noticed that, in contrast to group-add-member,
group-remove-member did not allow the format 'AD\name' or
'n...@ad.domain.com' for the --external option. I included this fix in
the patch, so the two user friendly formats are now supported.

Updated patch is attached.



Actually applying the patch makes testing it a lot easier. Ignore my 
previous e-mail.


Things are working fine, including tests (both with and without trust).

ACK

pushed to master

rob

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


Re: [Freeipa-devel] [PATCH] 1089 fix some ipa-replica-manage error handling

2013-03-14 Thread Rob Crittenden

Martin Kosek wrote:

On 03/01/2013 11:58 PM, Rob Crittenden wrote:

I found a couple of problems in error handling in ipa-replica-manage when doing
some oddball testing. See the ticket for full details.

This may apply by itself but in my tree it exists after patch 1088.

rob



Looks and works OK, ACK!

I think it would make sense to also rebase it for ipa-3-1 and thus for future
Fedora 18 release. In your first diff chunk, you would just need to catch for
NO_SUCH_OBJECT instead of errors.NotFound...


I ended up catching both, to be consistent with similar excepts. In my 
testing though getList returned errors.NotFound on failure.


Pushed to master and ipa-3-1

rob

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


Re: [Freeipa-devel] [PATCH] 0007 Web UI: Realm Domains page

2013-03-14 Thread Rob Crittenden

Petr Vobornik wrote:

On 03/07/2013 05:32 PM, Petr Vobornik wrote:

On 03/07/2013 02:19 PM, Ana Krivokapic wrote:

On 03/07/2013 12:41 PM, Petr Vobornik wrote:

On 03/06/2013 08:26 PM, Ana Krivokapic wrote:

On 03/06/2013 10:40 AM, Petr Vobornik wrote:

On 03/05/2013 05:52 PM, Ana Krivokapic wrote:

On 02/27/2013 05:10 PM, Petr Vobornik wrote:

On 02/27/2013 04:20 PM, Ana Krivokapic wrote:

Add support for Realm Domains to web UI.

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




8><---



Almost there, as discussed in person:

1. following strings should be add to and obtained from internal.py
plugin:
 title: 'Check DNS',
 message: 'Do you also want to perform DNS check?',
 ok_label: 'Check DNS',


2. the server plugin should report all dns resolution failures, not
just the first one.


Fixed, updated patch is attached.


Works fine, but you forgot to update all related tests
(s/domain/domains/):


==
FAIL: test_realmdomains[8]: realmdomains_mod: Try to replace list of
realm domains with a list with an invalid domain "doesnotexist.test"
--
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/nose/case.py", line 197, in
runTest
self.test(*self.arg)
  File "/home/pvoborni/dev/freeipa/tests/test_xmlrpc/xmlrpc_test.py",
line 264, in 
func = lambda: self.check(nice, **test)
  File "/home/pvoborni/dev/freeipa/tests/test_xmlrpc/xmlrpc_test.py",
line 278, in check
self.check_exception(nice, cmd, args, options, expected)
  File "/home/pvoborni/dev/freeipa/tests/test_xmlrpc/xmlrpc_test.py",
line 304, in check_exception
assert_deepequal(expected.strerror, e.strerror)
  File "/home/pvoborni/dev/freeipa/tests/util.py", line 343, in
assert_deepequal
VALUE % (doc, expected, got, stack)
AssertionError: assert_deepequal: expected != got.

  expected = u"invalid 'domain': no SOA or NS records found for
domains: doesnotexist.test"
  got = u"invalid 'domain': no SOA or NS records found for domain
doesnotexist.test"
  path = ()

--




False alarm. It was an error on my side.

ACK


Is this ready to be pushed? Do we need an ACK from Kyle too?

rob

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