Re: [Freeipa-devel] [PATCH] Workaround for trac N 5348

2015-10-09 Thread Tomas Babej


On 10/09/2015 09:15 AM, Jan Pazdziora wrote:
> On Fri, Oct 09, 2015 at 09:01:46AM +0200, Milan Kubík wrote:
>>
>> Perhaps we could use pytest's expected fail (xfail) or skip marker. [1] It
>> would prevent test from failing in the report and once the underlying issue
>> is fixed, it will raise as an unexpected pass.
>>
>> It could be used as a temporary solution, once the issue is fixed, we would
>> remove the mark from the test. This would probably need some workflow to be
>> defined for these cases.
> 
> That works but please note that this is not about test passing or
> failing, this is about some extra steps needed in the test body to
> achieve deterministic situation in which running that final check
> makes sense.
> 
> I can imagine that simple
> 
>   # workaround 5348
>   time.sleep(20)
> 
> and then some script which would find all these comments and compare
> them to resolved tickets might be enough.
> 

I like this idea the most. Keeping this information in Trac is not much
practical. Having a note in the comment annotating the particular
workaround, however, is quite neat.

Imho we can start such convention. Keeping a keyword in a comment is not
a heavy process. Also, I wouldn't be strict about it, as we already have
a couple of workarounds, and not every time a workaround has a exact
mapping to a particular ticket.

Tomas

-- 
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] Workaround for trac N 5348

2015-10-09 Thread Jan Pazdziora
On Fri, Oct 09, 2015 at 10:31:32AM +0200, Tomas Babej wrote:
> 
> a heavy process. Also, I wouldn't be strict about it, as we already have
> a couple of workarounds, and not every time a workaround has a exact
> mapping to a particular ticket.

Frankly, this worries me much more than not having ticket for some
trivial change to the code base.

If there's workaround in tests, it's some action that we do but
users/admins don't in their real setups. And that can cause failures
for our users that we don't see or even no longer know about because
in our tests, we've cleverly worked around them.

Either that workaround step is needed and needs to be documented, or
that step should not be needed and there should be a ticket describing
the issue.

-- 
Jan Pazdziora
Senior Principal Software Engineer, Identity Management Engineering, Red Hat

-- 
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] Workaround for trac N 5348

2015-10-09 Thread Petr Spacek
On 9.10.2015 11:03, Jan Pazdziora wrote:
> On Fri, Oct 09, 2015 at 10:31:32AM +0200, Tomas Babej wrote:
>>
>> a heavy process. Also, I wouldn't be strict about it, as we already have
>> a couple of workarounds, and not every time a workaround has a exact
>> mapping to a particular ticket.
> 
> Frankly, this worries me much more than not having ticket for some
> trivial change to the code base.
> 
> If there's workaround in tests, it's some action that we do but
> users/admins don't in their real setups. And that can cause failures
> for our users that we don't see or even no longer know about because
> in our tests, we've cleverly worked around them.
> 
> Either that workaround step is needed and needs to be documented, or
> that step should not be needed and there should be a ticket describing
> the issue.

+1

-- 
Petr^2 Spacek

-- 
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 0078-0081] ipa-client-install autodiscovery code improvements

2015-10-09 Thread Petr Spacek
On 9.10.2015 09:14, Petr Spacek wrote:
> On 8.10.2015 19:09, Martin Babinsky wrote:
>> These patches fix https://fedorahosted.org/freeipa/ticket/4305
>>
>> Actually only the last patch does the work itself (suppress autodiscovery 
>> when
>> installing client on master), but when I saw the state of autodiscovery code 
>> I
>> have taken the liberty to clean it up a bit.
>>
>> Patch #78 has separate versions for master and 4-2 branch, other patches
>> should apply on top of it in both branches.
> 
> Uh, I have to say that I'm not big fan of this patch. This will simply hide
> fact that your DNS is terribly misconfigured and other things will fail later 
> on.

To underline this point, this exercise will not be necessary when
https://fedorahosted.org/freeipa/ticket/5087 is solved (some incomplete
patches are on the list already) because the problem should be detected before
installation even starts.

Personally I would rather finish #5087 and do not spend more time on #4305.

Petr^2 Spacek


> Also, even if we decide that it is what we want, I'm not sure what are
> implications for the master. Will it configure to use SSSD for auto-discovery
> as a fallback (when services on local master are not running)?
> 
> If not, what will happen when IPA is not running on that particular master?
> Will the admin be able to log-in with IPA credentials?
> 
> 
> Nitpicks:
>> +if options.on_master:
>> +set_ipa_domain_params(ds, options.server, options.domain,
>> +  options.realm_name, CACERT)
> 
> It seems that set_ipa_domain_params should be class method of "ds" because it
> touches only instance variables and nothing else:-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek
-- 
Petr^2 Spacek

-- 
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] Workaround for trac N 5348

2015-10-09 Thread Jan Pazdziora
On Fri, Oct 09, 2015 at 09:01:46AM +0200, Milan Kubík wrote:
>
> Perhaps we could use pytest's expected fail (xfail) or skip marker. [1] It
> would prevent test from failing in the report and once the underlying issue
> is fixed, it will raise as an unexpected pass.
> 
> It could be used as a temporary solution, once the issue is fixed, we would
> remove the mark from the test. This would probably need some workflow to be
> defined for these cases.

That works but please note that this is not about test passing or
failing, this is about some extra steps needed in the test body to
achieve deterministic situation in which running that final check
makes sense.

I can imagine that simple

# workaround 5348
time.sleep(20)

and then some script which would find all these comments and compare
them to resolved tickets might be enough.

-- 
Jan Pazdziora
Senior Principal Software Engineer, Identity Management Engineering, Red Hat

-- 
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] Workaround for trac N 5348

2015-10-09 Thread Milan Kubík

On 10/08/2015 02:50 PM, Martin Basti wrote:



On 10/08/2015 02:39 PM, Martin Kosek wrote:

On 10/08/2015 02:08 PM, Oleg Fayans wrote:

Hi,

On 10/08/2015 11:18 AM, Jan Pazdziora wrote:

On Thu, Oct 08, 2015 at 11:12:37AM +0200, Oleg Fayans wrote:

When the ticket is addressed and these workarounds are no longer
needed -- what is our process for finding these workarounds and
reverting them, so that the tests test the real, expected behaviour?
As per discussion with Martin Basti, it was decided that this 
workaround
will only be applied to the current 4-2 branch, not to the 
upstream. In

That sounds like a reasonable plan for this issue.


upstream the issue itself will (supposedly) be solved

Except currently it's not, so (IIUIC) you will keep having
nondeterministic failures in master.

I was mostly interested in the general approach that we have to
workarounds -- how do we track them, how do we make sure they don't
stick in tests forever, even after the issue was already properly
addressed.

That's actually a great point. I personally would like tickets to 
have one more
field: "workaround" containing the address of a workaround in the 
format
"path_to_the_file:line_number" or better even - a commit id of this 
workaround,
so that once the ticket is resolved, we could easily find what to 
reverse.


Please don't add any more trac fields, there is too many of them 
already :-)

Keyword may serve better for now...


+1

new trac field for a few workarounds per year is not worth it.

Perhaps we could use pytest's expected fail (xfail) or skip marker. [1] 
It would prevent test from failing in the report and once the underlying 
issue is fixed, it will raise as an unexpected pass.


It could be used as a temporary solution, once the issue is fixed, we 
would remove the mark from the test. This would probably need some 
workflow to be defined for these cases.


[1]: https://pytest.org/latest/skipping.html

--
Milan Kubik

--
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] Workaround for trac N 5348

2015-10-09 Thread Milan Kubík

On 10/09/2015 09:01 AM, Milan Kubík wrote:

On 10/08/2015 02:50 PM, Martin Basti wrote:



On 10/08/2015 02:39 PM, Martin Kosek wrote:

On 10/08/2015 02:08 PM, Oleg Fayans wrote:

Hi,

On 10/08/2015 11:18 AM, Jan Pazdziora wrote:

On Thu, Oct 08, 2015 at 11:12:37AM +0200, Oleg Fayans wrote:

When the ticket is addressed and these workarounds are no longer
needed -- what is our process for finding these workarounds and
reverting them, so that the tests test the real, expected 
behaviour?
As per discussion with Martin Basti, it was decided that this 
workaround
will only be applied to the current 4-2 branch, not to the 
upstream. In

That sounds like a reasonable plan for this issue.


upstream the issue itself will (supposedly) be solved

Except currently it's not, so (IIUIC) you will keep having
nondeterministic failures in master.

I was mostly interested in the general approach that we have to
workarounds -- how do we track them, how do we make sure they don't
stick in tests forever, even after the issue was already properly
addressed.

That's actually a great point. I personally would like tickets to 
have one more
field: "workaround" containing the address of a workaround in the 
format
"path_to_the_file:line_number" or better even - a commit id of this 
workaround,
so that once the ticket is resolved, we could easily find what to 
reverse.


Please don't add any more trac fields, there is too many of them 
already :-)

Keyword may serve better for now...


+1

new trac field for a few workarounds per year is not worth it.

Perhaps we could use pytest's expected fail (xfail) or skip marker. 
[1] It would prevent test from failing in the report and once the 
underlying issue is fixed, it will raise as an unexpected pass.


It could be used as a temporary solution, once the issue is fixed, we 
would remove the mark from the test. This would probably need some 
workflow to be defined for these cases.


[1]: https://pytest.org/latest/skipping.html

I write faster than I read. The issue at hand here is diffeerent use 
case. :)


--
Milan Kubik

--
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 0078-0081] ipa-client-install autodiscovery code improvements

2015-10-09 Thread Petr Spacek
On 8.10.2015 19:09, Martin Babinsky wrote:
> These patches fix https://fedorahosted.org/freeipa/ticket/4305
> 
> Actually only the last patch does the work itself (suppress autodiscovery when
> installing client on master), but when I saw the state of autodiscovery code I
> have taken the liberty to clean it up a bit.
> 
> Patch #78 has separate versions for master and 4-2 branch, other patches
> should apply on top of it in both branches.

Uh, I have to say that I'm not big fan of this patch. This will simply hide
fact that your DNS is terribly misconfigured and other things will fail later 
on.

Also, even if we decide that it is what we want, I'm not sure what are
implications for the master. Will it configure to use SSSD for auto-discovery
as a fallback (when services on local master are not running)?

If not, what will happen when IPA is not running on that particular master?
Will the admin be able to log-in with IPA credentials?


Nitpicks:
> +if options.on_master:
> +set_ipa_domain_params(ds, options.server, options.domain,
> +  options.realm_name, CACERT)

It seems that set_ipa_domain_params should be class method of "ds" because it
touches only instance variables and nothing else:


-- 
Petr^2 Spacek

-- 
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] Workaround for trac N 5348

2015-10-09 Thread Martin Basti



On 09.10.2015 22:04, Oleg Fayans wrote:



On 10/09/2015 11:03 AM, Jan Pazdziora wrote:

On Fri, Oct 09, 2015 at 10:31:32AM +0200, Tomas Babej wrote:


a heavy process. Also, I wouldn't be strict about it, as we already 
have

a couple of workarounds, and not every time a workaround has a exact
mapping to a particular ticket.


Frankly, this worries me much more than not having ticket for some
trivial change to the code base.

If there's workaround in tests, it's some action that we do but
users/admins don't in their real setups. And that can cause failures
for our users that we don't see or even no longer know about because
in our tests, we've cleverly worked around them.


I guess, the global question of whether to do workarounds in tests to 
make them pass or not is older than the very oldest profession on earth.
I must admit, I am on Jan's side here. I would prefer to implement the 
approach proposed by Milan: mark the test scenario as expected failure 
(if it is crucial to make the whole run pass), or better even to leave 
it as it is: just so that each red CI run would remind of the 
necessity to fix the original issue.


This all is a theory, however. What do we do with this particular 
case? It's an edge case (does anyone except root zone admins sign a 
root zone?). Should we probably disable the whole scenario? Or just 
leave it failing as it is?




This bug does not happen just for root zone. Other zones are affected too.
I would leave it failing, we have to fix it.



Either that workaround step is needed and needs to be documented, or
that step should not be needed and there should be a ticket describing
the issue.





--
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


[Freeipa-devel] HOWTO: Troubleshooting SUDO

2015-10-09 Thread Pavel Březina

Hi,
I just submitted a sudo troubleshooting guide [1]. If you find anything 
missing, please, let me know.


[1] https://fedorahosted.org/sssd/wiki/HOWTO_Troubleshoot_SUDO

--
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] 0001 cert-show: Remove check if hostname != CN

2015-10-09 Thread Rob Crittenden
Jan Orel wrote:
> Hello,
> 
> this patch removes (IMHO) redundat check in cert_show, which fails when
> host tries to re-submit certificate of different host/service which he
> can manage. 
> 
> I also reported the bug here:
> https://bugzilla.redhat.com/show_bug.cgi?id=1269089
> 
> I tired to run the tests as well and it doesn't seem to break anything.
> Any feedpack appriciated.

This works around the "Retrieve Certificates from the CA" ACL when done
as a host.

I guess if the hostname isn't the subject then the host for the subject
needs to be read and then look to see if hostname is in the managed_by list.

rob

-- 
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


[Freeipa-devel] [PATCH 0009] WebUI: Disappearing automember rule expressions

2015-10-09 Thread Stanislav Laznicka

Hi,
please see the patch attached.

Standa L.
From 8bb771ad0f2f015ea1ebbdf7291ed5c7ae2a0a9b Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Fri, 9 Oct 2015 13:32:33 +0200
Subject: [PATCH] Fixes disappearing automember expressions

https://fedorahosted.org/freeipa/ticket/5353
---
 install/ui/src/freeipa/automember.js | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/install/ui/src/freeipa/automember.js b/install/ui/src/freeipa/automember.js
index 5eff3e69bdb014a3057bf9532e420cede416e8bf..0a73967290e742bf8ebc4920f6c2ee3cd52bdd2f 100644
--- a/install/ui/src/freeipa/automember.js
+++ b/install/ui/src/freeipa/automember.js
@@ -516,7 +516,8 @@ IPA.automember.condition_widget = function(spec) {
 var i = results.length - 1;
 while (i >= 0) {
 if (results[i].completed === 1){
-that.reload_facet({ result: results[i] });
+that.reload_facet({ error: results[i].error, id: null,
+result: results[i] });
 return;
 }
 i--;
@@ -776,4 +777,4 @@ exp.register = function() {
 phases.on('registration', exp.register);
 
 return exp;
-});
\ No newline at end of file
+});
-- 
2.4.3

-- 
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] 0197 client referral support for trusted domain principal

2015-10-09 Thread Sumit Bose
On Thu, Oct 08, 2015 at 01:36:23PM +0300, Alexander Bokovoy wrote:
> On Mon, 05 Oct 2015, Sumit Bose wrote:
> >On Thu, Sep 03, 2015 at 06:22:05PM +0300, Alexander Bokovoy wrote:
> >>On Thu, 03 Sep 2015, Alexander Bokovoy wrote:
> >>>Hi,
> >>>
> >>>attached patch adds support for issuing client referrals when FreeIPA
> >>>KDC is asked to give a TGT for a principal from a trusted forest.
> >>>
> >>>We return a matching forest name as a realm and KDC then returns an
> >>>error pointing a client to a direction of that realm. You can see how it
> >>>looks with http://fpaste.org/263064/14412849/ -- it shows behavior for
> >>>both 'kinit -E -C' and 'kinit -E'.
> >>>
> >>>Note that current MIT Kerberos KDC has a bug that prevents us from
> >>>responding with a correct client referral. A patched version for Fedora
> >>>22 is available in COPR abbra/krb5-test, a fix to upstream krb5 is
> >>>https://github.com/krb5/krb5/pull/323/ and I'm working on filing bugs to
> >>>Fedora and RHEL versions.
> >>>
> >>>With the version in my abbra/krb5-test COPR you can test the patch with
> >>>the help of kinit like fpaste URL above shows.
> >>After discussing with Simo and Sumit, here is updated patch that
> >>operates directly on 'search_for' krb5_principal and avoids
> >>strchr()/strrchr() and additional memory allocations -- it uses
> >>memrchr() to find '@' in the last component of the search_for principal
> >>and considers the part of the component after '@' as an enterprise realm
> >>to check.
> >
> >The patch looks good and works as advertised. I've tested in a IPA
> >domain which trusts two different forests. All requests to the forest
> >roots and child domains where properly redirected. I tested with your
> >krb5 test build and with MIT Kerberos 1.14 which contains the needed
> >fix.
> >
> >Nevertheless there are a view points I want to discuss:
> >
> >- missing support for AD's Alternative Domain Suffixes, this is
> > important to allow AD users to login in with their "Email-Address"
> > (which is the typical reference for a user name with an alternative
> > domain suffix). I think this is not strictly related to the given
> > ticket, so it can be solved in the context of a new ticket, do you
> > agree?
> Yes, please add a separate ticket. We need to do a bit more here:
> - extend schema to allow adding the attribute for alternative domain
>   suffixes
> - switch to use different DCE RPC call to retrieve forest trust
>   information. We can do it now that we have a call-out mechanism and
>   can isolate access to TDO credentials (this is long standing issue
>   first identified by Metze as part of cross-forest trust support for
>   Samba 4.3)
> - Make possible to associate alternative domain suffixes with IPA
>   realm. We have support for realm domains already but we don't allow
>   to use them yet for the same call as in the above item.

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

> 
> >- referrals from outside. If I call 'kinit -E admin@IPA.DOMAIN' from a
> > client in a trusted AD forest I get a 'Client not found in database'
> > error because AD tends to use lower case domain names in the referal
> > response. The request is still properly send to the IPA KDC because
> > DNS does not care about the case. The IPA KDC processes the request
> > with the principal 'user\@IPA.DOMAIN@ipa.domain' until
> > ipadb_is_princ_from_trusted_realm() returns KRB5_KDB_NOENTRY becasue
> > it detects that the principal is from the local realm. I think it
> > would be good to enhance your patch to handle this case.
> This is a separate bug too. Please file a ticket.

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

> 
> 
> >- S4U2Self. MIT Kerberos 1.14 can now properly handle S4U2Self across
> > domain and forest boundaries (I tested this in a setup with 2 AD
> > forests with request going from a child domain to a child domain in
> > the other forest. Unfortunately it is currently not working with IPA
> > in neither direction (I guess the case issue from above might be the
> > reason for the incoming request to fail). Here I think a new ticket
> > would to good as well because some research might be needed and the
> > issue might even be in the MIT code. (If you want to run some tests I
> > can give you access to my test environment.)
> I think we want to have this working, thus a ticket is due here. This is
> something we'll most likely require for some advanced 2FA operations for
> AD users.

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

bye,
Sumit

> 
> >Let me know if you prefer to handle the issues with other tickets, then
> >I would ACK the patch as it is.
> Please file separate tickets.
> 
> -- 
> / 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] Workaround for trac N 5348

2015-10-09 Thread Martin Basti



On 10/08/2015 11:18 AM, Oleg Fayans wrote:

Done
On 10/08/2015 10:37 AM, Martin Basti wrote:



On 10/08/2015 09:13 AM, Oleg Fayans wrote:

Hi Martin

On 10/07/2015 04:30 PM, Martin Basti wrote:



On 10/07/2015 04:13 PM, Oleg Fayans wrote:

subj




Workaround looks good, but I prefer not to push it in upstream tests,
because it is not test failure.

I agree, we should rather fix the original issue. But as a temporary
solution, to satisfy downstream, it could do.


Why is there this sleep, this might be useful in upstream tests 
too, but

what is the reason to add sleep there?


Without it I kept getting this error:
E   CalledProcessError: Command '['drill', '@localhost', '-k',
'/etc/trusted-key.key', '-S', 'example.test.', 'SOA']' returned
non-zero exit status 29

with --pdb option, though, my attempts to re-run the command
succeeded, so I assumed it was a timing issue, and indeed, this 1
second sleep helped.



  # verify signatures
+time.sleep(1)
  args = [




Attached is an updated version of the patch with Martin's remarks
taken into account

Can you please send this as separate patch? I would like to push this 
one.



ACK

Pushed to:
master: 2b4354f37e7e775dae833d5e2e8824b43800855f
ipa-4-2: f076da99946c0405162c88174e771a5cecfe9664

--
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] [PATCHES 362-366] Realmdomains handling improvements

2015-10-09 Thread Tomas Babej


On 09/23/2015 02:40 PM, Martin Basti wrote:
> 
> 
> On 09/22/2015 02:23 PM, Tomas Babej wrote:
>> On 09/03/2015 04:34 PM, Alexander Bokovoy wrote:
>>> On Thu, 03 Sep 2015, Tomas Babej wrote:
 Hi,

 this couple of patches fix https://fedorahosted.org/freeipa/ticket/5278
 and improve our handling of realmdomains in general.
>>> The code looks good to me. I haven't tested it yet, though.
>>>
>> Rebased on top of current master.
> 
> Please fix tests too.
> 

Updated patchset attached. Also fixed a minor spelling and syntax issues
in the original patches.

Tomas
From e02e5cd1d084f7faef76f3995e9236b7ea0bb3f7 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Thu, 3 Sep 2015 12:13:32 +0200
Subject: [PATCH] util: Add detect_dns_zone_realm_type helper

https://fedorahosted.org/freeipa/ticket/5278
---
 ipalib/util.py | 55 +++
 1 file changed, 55 insertions(+)

diff --git a/ipalib/util.py b/ipalib/util.py
index a37f67342259c1ef8bd31af1d9c40e453c3bf1cf..29b4ca160f1e63dfc2c233547028b5982242a3af 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -801,3 +801,58 @@ def get_topology_connection_errors(graph):
 if not_visited:
 connect_errors.append((m, list(visited), list(not_visited)))
 return connect_errors
+
+def detect_dns_zone_realm_type(api, domain):
+"""
+Detects the type of the realm that the given DNS zone belongs to.
+Note: This method is heuristic. Possible values:
+  - 'current': For IPA domains belonging in the current realm.
+  - 'foreign': For domains belonging in a foreing kerberos realm.
+  - 'unknown': For domains whose allegiance could not be detected.
+"""
+
+# First, try to detect _kerberos TXT record in the domain
+# This would indicate that the domain belongs to IPA realm
+
+kerberos_prefix = DNSName('_kerberos')
+domain_suffix = DNSName(domain)
+kerberos_record_name = kerberos_prefix + domain_suffix
+
+response = None
+
+try:
+result = resolver.query(kerberos_record_name, rdatatype.TXT)
+answer = result.response.answer
+
+# IPA domain will have only one _kerberos TXT record
+if (len(answer) == 1 and
+len(answer[0]) == 1 and
+answer[0].rdtype == rdatatype.TXT):
+
+record = answer[0][0]
+
+# If the record contains our current realm, it is 'ipa-current'
+if record.to_text() == '"{0}"'.format(api.env.realm):
+return 'current'
+else:
+return 'foreign'
+
+except DNSException as e:
+pass
+
+# Try to detect AD specific record in the zone.
+# This would indicate that the domain belongs to foreign (AD) realm
+
+gc_prefix = DNSName('_ldap._tcp.gc._msdcs')
+ad_specific_record_name = gc_prefix + domain_suffix
+
+try:
+# The presence of this record is enough, return foreign in such case
+result = resolver.query(ad_specific_record_name, rdatatype.SRV)
+return 'foreign'
+
+except DNSException as e:
+pass
+
+# If we could not detect type with certainity, return unknown
+return 'unknown'
-- 
2.1.0

From c1f93910a6f5cfaa0f46252b0c6f165e9257a5ae Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Thu, 3 Sep 2015 12:40:17 +0200
Subject: [PATCH] realmdomains: Minor style and wording improvements

https://fedorahosted.org/freeipa/ticket/5278
---
 ipalib/plugins/realmdomains.py | 75 +-
 1 file changed, 60 insertions(+), 15 deletions(-)

diff --git a/ipalib/plugins/realmdomains.py b/ipalib/plugins/realmdomains.py
index f8f838d0ede85ee747a4b2f19129dc757fe837eb..27c4fa228b455e8de5e40dafb8be0e4a4e1d0d65 100644
--- a/ipalib/plugins/realmdomains.py
+++ b/ipalib/plugins/realmdomains.py
@@ -137,16 +137,46 @@ class realmdomains_mod(LDAPUpdate):
 del_domain = entry_attrs.get('del_domain')
 force = options.get('force')
 
+current_domain = get_domain_name()
+
+missing_soa_ns_record_error = _(
+"DNS zone for each realmdomain must contain "
+"SOA or NS records. No records found for: %s"
+)
+
+# User specified the list of domains explicitly
 if associateddomain:
 if add_domain or del_domain:
-raise errors.MutuallyExclusiveError(reason=_("you cannot specify the --domain option together with --add-domain or --del-domain"))
-if get_domain_name() not in associateddomain:
-raise errors.ValidationError(name='domain', error=_("cannot delete domain of IPA server"))
+raise errors.MutuallyExclusiveError(
+reason=_(
+"The --domain option cannot be used together "
+"with --add-domain or --del-domain. Use --domain "
+"to specify the whole realm domain list explicitly, "
+  

[Freeipa-devel] [PATCH 0058] Remove bind configuration detected question

2015-10-09 Thread Gabe Alford
Hello,

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

Thanks,

Gabe
From 509ea0b496fd3d2361df58b23ce6ec8fb0ac9b64 Mon Sep 17 00:00:00 2001
From: Gabe 
Date: Fri, 9 Oct 2015 11:02:06 -0600
Subject: [PATCH] Remove bind configuration detected question

https://fedorahosted.org/freeipa/ticket/5351
---
 ipaserver/install/bindinstance.py | 7 ---
 ipaserver/install/dns.py  | 4 
 2 files changed, 11 deletions(-)

diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 4c4677590b7120b7f12cb014519f61673dd1d68a..1cbda7c6931c55247bb0207ae91fbbf5363ad867 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -63,13 +63,6 @@ named_conf_include_re = re.compile(r'\s*include\s+"(?P)"\s*;')
 named_conf_include_template = "include \"%(path)s\";\n"
 
 
-def check_inst(unattended):
-if not unattended and os.path.exists(NAMED_CONF):
-msg = "Existing BIND configuration detected, overwrite?"
-return ipautil.user_input(msg, False)
-
-return True
-
 def create_reverse():
 return ipautil.user_input("Do you want to configure the reverse zone?", True)
 
diff --git a/ipaserver/install/dns.py b/ipaserver/install/dns.py
index 099e35dc331722607c8ca02cdbc7a0e66f8c4754..eb09af30b0f78f38ab1948d4dd01264f45dadf7c 100644
--- a/ipaserver/install/dns.py
+++ b/ipaserver/install/dns.py
@@ -144,10 +144,6 @@ def install_check(standalone, replica, options, hostname):
 False)):
 sys.exit("Aborted")
 
-# Check bind packages are installed
-if not bindinstance.check_inst(options.unattended):
-sys.exit("Aborting installation.")
-
 if options.disable_dnssec_master:
 _is_master()
 
-- 
1.8.3.1

-- 
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

[Freeipa-devel] [PATCH 0057] Warn in no installation found when running ipa-server-install --uninstall

2015-10-09 Thread Gabe Alford
Hello,

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

Thanks,

Gabe
From 0400bf88987b56d1d3b7a0e665bec525fa81ed02 Mon Sep 17 00:00:00 2001
From: Gabe 
Date: Fri, 9 Oct 2015 10:48:17 -0600
Subject: [PATCH] Warn if no installation found when running ipa-server-install
 --uninstall

https://fedorahosted.org/freeipa/ticket/5341
---
 ipaserver/install/server/install.py | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index 13a59a0e6149dc22ded4a895db02516e9360e02b..ca93e7a6fd7276d9c0d82eb6f94575730759d858 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -954,6 +954,12 @@ def uninstall_check(installer):
 
 installer._installation_cleanup = False
 
+if not is_ipa_configured():
+print("IPA server is not configured on this system.\n" +
+  "If you want to install the IPA server, please install " +
+  "it using 'ipa-server-install'.")
+sys.exit(1)
+
 fstore = sysrestore.FileStore(SYSRESTORE_DIR_PATH)
 sstore = sysrestore.StateFile(SYSRESTORE_DIR_PATH)
 
-- 
1.8.3.1

-- 
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

[Freeipa-devel] [PATCH 0056] Enable nsaccountlock in user.py cli

2015-10-09 Thread Gabe Alford
Hello,

This patch enables nsaccountlock in user.py cli. It is very handy to be
able to search and find users with disabled/enabled accounts, etc. That
said, I couldn't find why it was no_option in the first place, so I am not
100% sure if it breaks something or the reasoning behind no_option.

Thanks,

Gabe
From 985f765d2e25d2ce454884cd4a9f66f9005824a7 Mon Sep 17 00:00:00 2001
From: Gabe 
Date: Fri, 9 Oct 2015 07:22:07 -0600
Subject: [PATCH] Enable nsaccountlock in user.py for cli usage

---
 API.txt| 6 +++---
 VERSION| 2 +-
 ipalib/plugins/user.py | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/API.txt b/API.txt
index 4d36a9885157de13529573b3a386b4ef39eba176..b4df75bb66dab43bc9b7c249851f61efcc284e0f 100644
--- a/API.txt
+++ b/API.txt
@@ -5176,7 +5176,7 @@ option: Str('manager', attribute=True, cli_name='manager', multivalue=False, req
 option: Str('mobile', attribute=True, cli_name='mobile', multivalue=True, required=False)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('noprivate', autofill=True, cli_name='noprivate', default=False)
-option: Bool('nsaccountlock', attribute=True, cli_name='nsaccountlock', multivalue=False, required=False)
+option: Bool('nsaccountlock', attribute=True, cli_name='disabled', multivalue=False, required=False)
 option: Str('ou', attribute=True, cli_name='orgunit', multivalue=False, required=False)
 option: Str('pager', attribute=True, cli_name='pager', multivalue=True, required=False)
 option: Str('postalcode', attribute=True, cli_name='postalcode', multivalue=False, required=False)
@@ -5269,7 +5269,7 @@ option: Str('not_in_hbacrule*', cli_name='not_in_hbacrules', csv=True)
 option: Str('not_in_netgroup*', cli_name='not_in_netgroups', csv=True)
 option: Str('not_in_role*', cli_name='not_in_roles', csv=True)
 option: Str('not_in_sudorule*', cli_name='not_in_sudorules', csv=True)
-option: Bool('nsaccountlock', attribute=True, autofill=False, cli_name='nsaccountlock', multivalue=False, query=True, required=False)
+option: Bool('nsaccountlock', attribute=True, autofill=False, cli_name='disabled', multivalue=False, query=True, required=False)
 option: Str('ou', attribute=True, autofill=False, cli_name='orgunit', multivalue=False, query=True, required=False)
 option: Str('pager', attribute=True, autofill=False, cli_name='pager', multivalue=True, query=True, required=False)
 option: Flag('pkey_only?', autofill=True, default=False)
@@ -5324,7 +5324,7 @@ option: Str('mail', attribute=True, autofill=False, cli_name='email', multivalue
 option: Str('manager', attribute=True, autofill=False, cli_name='manager', multivalue=False, required=False)
 option: Str('mobile', attribute=True, autofill=False, cli_name='mobile', multivalue=True, required=False)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Bool('nsaccountlock', attribute=True, autofill=False, cli_name='nsaccountlock', multivalue=False, required=False)
+option: Bool('nsaccountlock', attribute=True, autofill=False, cli_name='disabled', multivalue=False, required=False)
 option: Str('ou', attribute=True, autofill=False, cli_name='orgunit', multivalue=False, required=False)
 option: Str('pager', attribute=True, autofill=False, cli_name='pager', multivalue=True, required=False)
 option: Str('postalcode', attribute=True, autofill=False, cli_name='postalcode', multivalue=False, required=False)
diff --git a/VERSION b/VERSION
index e1df4694f678b1fb27da7785b94dc827f0f8f207..98b64017f320d1cb5e3015476f894d1ece1d2012 100644
--- a/VERSION
+++ b/VERSION
@@ -91,4 +91,4 @@ IPA_DATA_VERSION=2010061412
 
 IPA_API_VERSION_MAJOR=2
 IPA_API_VERSION_MINOR=156
-# Last change: pvoborni - add vault container commands
+# Last change: galford - enable nssacountlock option in cli
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index cb47cbb4869cb978f87603817033580647cc2d17..802dc35f4321c69460fd13bc1103346ab1e30a50 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -340,8 +340,8 @@ class user(baseuser):
 
 takes_params = baseuser.takes_params + (
 Bool('nsaccountlock?',
+cli_name='disabled',
 label=_('Account disabled'),
-flags=['no_option'],
 ),
 Bool('preserved?',
 label=_('Preserved user'),
-- 
1.8.3.1

-- 
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

[Freeipa-devel] [PATCH 0059] ipa-adtrust-install: Print complete SRV record

2015-10-09 Thread Petr Spacek
Hello,

I found this when reviewing DNS parts of IdM and AD integration guides.

ipa-adtrust-install: Print complete SRV records.
https://fedorahosted.org/freeipa/ticket/5358

-- 
Petr^2 Spacek
From b10bd3b49c24c43a8540830303ab81250de6dd42 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Fri, 9 Oct 2015 14:58:14 +0200
Subject: [PATCH] ipa-adtrust-install: Print complete SRV records

https://fedorahosted.org/freeipa/ticket/5358
---
 ipaserver/install/adtrustinstance.py | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index acc54ab83383b0b514fcd4f14d7480e0b38cb5b1..f7a7899906ca47b4901881fb6f4099ace1780741 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -593,9 +593,10 @@ class ADTRUSTInstance(service.Service):
 self.print_msg(err_msg)
 self.print_msg("Add the following service records to your DNS " \
"server for DNS zone %s: " % zone)
-for srv in ipa_srv_rec:
-for suff in win_srv_suffix:
-self.print_msg(" - %s%s"  % (srv[0], suff))
+for suff in win_srv_suffix:
+for srv in ipa_srv_rec:
+self.print_msg("%s%s IN SRV %s"  % (srv[0], suff, " ".join(srv[1])))
+self.print_msg("")
 return
 
 for (srv, rdata, port) in ipa_srv_rec:
-- 
2.4.3

-- 
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] 0001 cert-show: Remove check if hostname != CN

2015-10-09 Thread Rob Crittenden
Christian Heimes wrote:
> On 2015-10-09 13:21, Jan Orel wrote:
>> Hello,
>>
>> this patch removes (IMHO) redundat check in cert_show, which fails when
>> host tries to re-submit certificate of different host/service which he
>> can manage. 
>>
>> I also reported the bug here:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1269089
>>
>> I tired to run the tests as well and it doesn't seem to break anything.
>> Any feedpack appriciated.
> 
> Jan Cholasta, you implemented the check in 2011. What purpose does it have?
> 
> hostname == CN has been deprecated by RFC 2818 for some time, see
> https://tools.ietf.org/html/rfc2818#section-3.1  The current check is
> also not sufficient to prevent forgery. Browsers and modern TLS
> libraries completely ignore CN when a dNSName SAN extension is present.

He just tweaked a pylint error, I did this code.

The check isn't perfect (by far) but I don't think forgery is an issue.
We're talking about retrieving a public cert, not doing a handshake.

I think checking just the common name is ok because of the way IPA
issues server certs. I'm not sure if SAN would even come into play in
the case of checking this ACL.

The only way I see this as being a problem is if a new profile is
created for issuing server certs where the CN of the target host isn't
in the subject.

rob

-- 
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


[Freeipa-devel] LXC NTP Problem

2015-10-09 Thread Martin Jørgensen
Hi
I really wont to deploy my freeipa server on LXC, but i cannot complete the
installation because
of NTP dont run on LXC Containers

With --no-ntp wont either run, installation just stop when trying to start
ntpd

Help


Mvh Martin Jørgensen
TLF: 28738893
-- 
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] 0001 cert-show: Remove check if hostname != CN

2015-10-09 Thread Jan Cholasta

On 9.10.2015 15:00, Christian Heimes wrote:

On 2015-10-09 13:21, Jan Orel wrote:

Hello,

this patch removes (IMHO) redundat check in cert_show, which fails when
host tries to re-submit certificate of different host/service which he
can manage.

I also reported the bug here:
https://bugzilla.redhat.com/show_bug.cgi?id=1269089

I tired to run the tests as well and it doesn't seem to break anything.
Any feedpack appriciated.


Jan Cholasta, you implemented the check in 2011. What purpose does it have?


I did not, it was added in commit 2e8bae59 by Rob.



hostname == CN has been deprecated by RFC 2818 for some time, see
https://tools.ietf.org/html/rfc2818#section-3.1  The current check is
also not sufficient to prevent forgery. Browsers and modern TLS
libraries completely ignore CN when a dNSName SAN extension is present.

Christian




--
Jan Cholasta

--
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] 0001 cert-show: Remove check if hostname != CN

2015-10-09 Thread Christian Heimes
On 2015-10-09 15:11, Jan Cholasta wrote:
> On 9.10.2015 15:00, Christian Heimes wrote:
>> On 2015-10-09 13:21, Jan Orel wrote:
>>> Hello,
>>>
>>> this patch removes (IMHO) redundat check in cert_show, which fails when
>>> host tries to re-submit certificate of different host/service which he
>>> can manage.
>>>
>>> I also reported the bug here:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1269089
>>>
>>> I tired to run the tests as well and it doesn't seem to break anything.
>>> Any feedpack appriciated.
>>
>> Jan Cholasta, you implemented the check in 2011. What purpose does it
>> have?
> 
> I did not, it was added in commit 2e8bae59 by Rob.

Sorry, I didn't check the context, just the output of

$ git annotate  ipalib/plugins/cert.py | grep common_name



signature.asc
Description: OpenPGP digital signature
-- 
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] Workaround for trac N 5348

2015-10-09 Thread Oleg Fayans



On 10/09/2015 11:03 AM, Jan Pazdziora wrote:

On Fri, Oct 09, 2015 at 10:31:32AM +0200, Tomas Babej wrote:


a heavy process. Also, I wouldn't be strict about it, as we already have
a couple of workarounds, and not every time a workaround has a exact
mapping to a particular ticket.


Frankly, this worries me much more than not having ticket for some
trivial change to the code base.

If there's workaround in tests, it's some action that we do but
users/admins don't in their real setups. And that can cause failures
for our users that we don't see or even no longer know about because
in our tests, we've cleverly worked around them.


I guess, the global question of whether to do workarounds in tests to 
make them pass or not is older than the very oldest profession on earth.
I must admit, I am on Jan's side here. I would prefer to implement the 
approach proposed by Milan: mark the test scenario as expected failure 
(if it is crucial to make the whole run pass), or better even to leave 
it as it is: just so that each red CI run would remind of the necessity 
to fix the original issue.


This all is a theory, however. What do we do with this particular case? 
It's an edge case (does anyone except root zone admins sign a root 
zone?). Should we probably disable the whole scenario? Or just leave it 
failing as it is?




Either that workaround step is needed and needs to be documented, or
that step should not be needed and there should be a ticket describing
the issue.



--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

--
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] LXC NTP Problem

2015-10-09 Thread Martin Basti



On 10/09/2015 03:42 PM, Martin Jørgensen wrote:

Hi
I really wont to deploy my freeipa server on LXC, but i cannot 
complete the installation because

of NTP dont run on LXC Containers

With --no-ntp wont either run, installation just stop when trying to 
start ntpd


Help


Mvh Martin Jørgensen
TLF: 28738893



Hello,

can you share log of installation /var/log/ipaserver-install.log

I'm not a container guru, but get IPA into containers may be tricky.

Here are some docker related materials, which might be useful:
http://www.adelton.com/docs/docker/complex-application-in-container
https://github.com/adelton/docker-freeipa
https://www.freeipa.org/page/Docker

Martin
-- 
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