Re: [Freeipa-devel] [PATCH 0069] ipa-nis-manage enable: change service name from 'portmap' to 'rpcbind'

2016-05-03 Thread Abhijeet Kasurde

Hi Gabe,

I am wondering, how are we handling "CalledProcessError" exception ?

On 05/04/2016 09:17 AM, Gabe Alford wrote:

Hello,

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

Thanks,

Gabe



Thanks,
Abhijeet Kasurde
-- 
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] 0053..0054 Configure lightweight CA key replication

2016-05-03 Thread Fraser Tweedale
On Tue, May 03, 2016 at 05:05:58PM +1000, Fraser Tweedale wrote:
> On Tue, Apr 26, 2016 at 10:02:45AM +0200, Jan Cholasta wrote:
> > On 21.4.2016 05:30, Fraser Tweedale wrote:
> > >On Thu, Apr 14, 2016 at 04:39:37PM +1000, Fraser Tweedale wrote:
> > >>Hi all,
> > >>
> > >>The attached patches configure lightweight CA key replication on IPA
> > >>CAs, on upgrade and installation.
> > >>
> > >>Patches 0051..0052 from my other mail are also needed for the system
> > >>to work, but this patchset does not depend on them and can be
> > >>reviewed independently.
> > >>
> > >>There is also no hard dependency on the (unreleased) Dogtag 10.3.0b1
> > >>- it just puts the necessary principals/keys/configuration in place.
> > >>
> > >>Cheers,
> > >>Fraser
> > >>
> > >New patches attached;  0054-2 changes the service name from
> > >'dogtag-ipa-custodia' to just 'dogtag', and adds an ACI to allow the
> > >principal to search server Custodia keys.
> > 
> Honza, thanks for review.  Comments inline.
> 
> > Patch 53:
> > 
> > I'm not sure about this approach - the cn of custodia keys in LDAP is a
> > free-form string, I would not tie it to service names, but rather try to
> > keep it short.
> > 
> > In the key replication section of the design page, you mention "ca/$NAME", I
> > think this is a good template for the cn and that we should stick to it.
> > 
> This scheme (or something like it, *without* '/' as the separator)
> is needed to satisfy the ACI that allows host principals to manage
> Custodia keys:
> 
> add:aci: (target = 
> "ldap:///cn=*/($$dn),cn=custodia,cn=ipa,cn=etc,$SUFFIX")
> (version 3.0; acl "IPA server hosts can create own Custodia secrets";
>   allow(add) groupdn = 
> "ldap:///cn=ipaservers,cn=hostgroups,cn=accounts,$SUFFIX;
>  and userdn = 
> "ldap:///fqdn=($$dn),cn=computers,cn=accounts,$SUFFIX";)
> 
> The CN must contain the hostname, and we must also disambiguate on
> key type.  The current scheme is:
> 
> {sig,enc}~dogtag/
> e.g.
> enc~dogtag/f23-2.ipa.local
> 
> The first separator cannot be '/' because the '*' wildcard in the
> ACI is not greedy - the captured text would include the servicename
> and fail to match any userdn.
> 
> If you do not like '~' feel free to suggest a different symbol :)
> The alternative is to add more ACIs.
> 
> > 
> > Patch 54:
> > 
> > 1) This belongs to CAInstance.configure_instance():
> > 
> > +CA = cainstance.CAInstance(
> > +api.env.realm, certs.NSS_DIR, host_name=api.env.host)
> > +CA.setup_lightweight_ca_key_retrieval()
> > 
> See comments for (5).
> 
> > 
> > 2) Any ACI changes should be in a separate patch. (What happened to patch
> > 52?)
> > 
> Patch 52 added an ACI that allowed any authenticated user to see the
> keys.  Simo wanted it limited it to the Dogtag principal so I
> rescinded patch 52 and added the ACI in the same patch where the
> principal was added.
> 
> Separate patch is no problem; I will resurrect number 52.
> 
> > 
> > 3) This is not a platform constant, just a constant:
> > 
> > +PKI_GSSAPI_SERVICE_NAME = 'dogtag'
> > 
> Thanks, will put it in `ipalib.constants'.
> 
> > 
> > 4) CAInstance.setup_lightweight_ca_key_retrieval() does too much. Please
> > split it into a "setup keytab" and "setup custodia" parts.
> > 
> Will extract methods for next patchset.
> 
> > 
> > 5) This also belongs to CAInstance.configure_instance():
> > 
> > +if setup_ca:
> > +# CA was configured before Kerberos;
> > +# add Custodia client princ and keys now
> > +ca_instance.setup_lightweight_ca_key_retrieval()
> > 
> > In order for that to work, you need to move the ca.install_step_1() after
> > krb.create_instance(), but that should be OK, since KrbInstance does not
> > talk to the CA.
> > 
> `setup_lightweight_ca_key_retrieval' calls `kadmin_addprinc', which
> fails if called before `krb.create_instance' due to missing
> krb5.conf::
> 
> 2016-05-03T06:29:23Z DEBUG args=kadmin.local -q addprinc -randkey 
> dogtag/f23-2.ipa.local@IPA.LOCAL -x ipa-setup-override-restrictions
> 2016-05-03T06:29:23Z DEBUG Process finished, return code=1
> 2016-05-03T06:29:23Z DEBUG stdout=
> 2016-05-03T06:29:23Z DEBUG stderr=kadmin.local: unable to get default 
> realm
> 
> Moving `ca.install_step_1()' to after `krb.create_instance()' does
> not help, because `CAInstance.configure_instance' is called from
> `ca.install_step_0()'.
> 
> However, calling `CAInstance.setup_lightweight_ca_key_retrieval()'
> *directly* from `ca.install_step_1' would probably work.  Are you
> happy with putting it there, instead of `configure_instance()'?
> 
> Cheers,
> Fraser
> 
Updated patches attached, include bringing back 0052-2 for the ACI
change.

Cheers,
Fraser
From 3d047e3dc1e7f700751c0f52f26326764b70d94d Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Tue, 3 May 2016 13:22:39 +1000
Subject: [PATCH] Allow Dogtag service principals to read Custodia keys

The 

[Freeipa-devel] [PATCH 0069] ipa-nis-manage enable: change service name from 'portmap' to 'rpcbind'

2016-05-03 Thread Gabe Alford
Hello,

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

Thanks,

Gabe
From 950da9c812a162569379bd9e530977960e9ab7ca Mon Sep 17 00:00:00 2001
From: Gabe 
Date: Tue, 3 May 2016 21:33:33 -0600
Subject: [PATCH] ipa-nis-manage enable: change service name from 'portmap' to
 'rpcbind'

https://fedorahosted.org/freeipa/ticket/5857
---
 install/tools/ipa-nis-manage | 21 ++---
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/install/tools/ipa-nis-manage b/install/tools/ipa-nis-manage
index 3aa1507b205eaa679edebda2a3705b494369abc3..948aa0046b6eeb0f68dd90390eaca6d5b6c8dba3 100755
--- a/install/tools/ipa-nis-manage
+++ b/install/tools/ipa-nis-manage
@@ -144,19 +144,18 @@ def main():
 retval = 1
 
 # Enable either the portmap or rpcbind service
-try:
-portmap = services.knownservices.portmap
+portmap = services.knownservices.portmap
+rpcbind = services.knownservices.rpcbind
+
+if portmap.is_installed():
 portmap.enable()
 servicemsg = portmap.service_name
-except ipautil.CalledProcessError as cpe:
-if cpe.returncode == 1:
-try:
-rpcbind = services.knownservices.rpcbind
-rpcbind.enable()
-servicemsg = rpcbind.service_name
-except ipautil.CalledProcessError as cpe:
-print("Unable to enable either %s or %s" % (portmap.service_name, rpcbind.service_name))
-retval = 3
+elif rpcbind.is_installed():
+rpcbind.enable()
+servicemsg = rpcbind.service_name
+else:
+print("Unable to enable either %s or %s" % (portmap.service_name, rpcbind.service_name))
+retval = 3
 
 # The cn=config entry for the plugin may already exist but it
 # could be turned off, handle both cases.
-- 
2.5.5

-- 
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 provide more information for "ipa cert-revoke -h"

2016-05-03 Thread Gabe Alford
On Tue, May 3, 2016 at 9:35 AM, Patrice Duc-Jacquet <
patrice.duc.jacq...@gmail.com> wrote:

> On 05/03/2016 04:41 PM, Rob Crittenden wrote:
>
> Gabe Alford wrote:
>>
>>> Hello,
>>>
>>> Thank you for your patch as well.
>>>
>>>  >-doc=_('Reason for revoking the certificate (0-10)'),
>>>  >+doc=_('Reason for revoking the certificate (0-10). See
>>> RFC 5280 (paragraph 5.3.1) for reason details'),
>>>
>>> Rather than just specifying the RFC with the paragraph to go look up,
>>> can you either add the revocation options or say something like:
>>>
>>> +doc=_('Reason for revoking the certificate (0-10). See
>>> \'ipa help cert\' for revocation reason details.'),
>>>
>>> IMO, it is a little annoying to go look up revocation reasons when those
>>> reasons can either be added to the help output or exist already in `ipa
>>> help cert`.
>>>
>>
>> FTR I added it to the top level help because the reasons are used in
>> multiple places and didn't want to duplicate them, and adding them to a
>> specific option help would overload it big time IMHO.
>>
>> rob
>>
>> Hi everyone
> thanks for your valuable comments. I fully agree that it is not
> recommended to duplicate this information. So as Rob suggested, I should
> avoid to add this information to cert_revoke option and thus I plan to
> modify the help message as follow:
>
> doc=_('Reason for revoking the certificate (0-10). Type "ipa help cert"
> for reason details'),
>
> Do you agree with that modification? Thanks in advance and regards
>

I think the modification is fine. One nitpick that I would have is to say
"for revocation reason details."  rather than "for reason details."
Also, don't forget a period after the word "details". :)

Gabe



>
> Pat
>
-- 
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] #5836 [RFE] Allow profile to specify default CA

2016-05-03 Thread Fraser Tweedale
Continuing the discussion for #5836[1] as requested from triage
session.

[1] https://fedorahosted.org/freeipa/ticket/5836

IMO it is not important for FreeIPA 4.4.  It is nice to have but I
doubt it will make it.

Honza suggested it should be the other way around, i.e. CA specifies
default profile rather than profile specifies default CA.

The fact (also raised by Christian) is that multiple profiles may be
used with a single CA, and vice-versa.  CA ACLs will govern what
combinations are acceptable.

Thinking from user perspective, there are a couple of things to
consider:

- Currently, to request a particular kind of cert, user must specify
  a profile ID.

- It is more natural to ask for a particular profile and have the
  request dispatched to a profile-specified default CA, than to ask
  for a cert issued by a particular CA, and a CA-specified default
  profile will be used.

Given these points, I am strongly in favour of having the profile
indicate the default CA - not the other way around.

Cheers,
Fraser

-- 
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] Improving bug reporting

2016-05-03 Thread Robbie Harwood
Lukas Slebodnik  writes:

> On (03/05/16 12:29), Robbie Harwood wrote:
>>David Kupka  writes:
>>
>>> --8<- trac-ticket-template-proposal --->8--
>>> Related SW versions:
>>> On server:
>>> {{{
>>> $ rpm -q freeipa-server pki-base 389-ds-base bind samba krb5-server 
>>
>> I think this is a good idea.  However, we are on Debian/family as
>> well now, and I think we want to accept bugs that come from these
>> users as well.
>
> FreeIPA is heavily patched on debian and has quite old version there
> 4.0.5.
>
> The better would be recommend to reproduce with upstream version
> (fedora/CentOS).

(FreeIPA 4.1.4 is available on Debian, but your point still stands.)

In summary: I don't like that upstream is conflated with fedora/CentOS.
Of course I understand that this was done to ease development and not
out of malice.  But longer term I would like Debian/Ubuntu FreeIPA to be
less of an afterthought because I believe we can attract users to our
product.  I believe this to be especially true with working
freeipa-client on those distros, which we now have and I am very happy
about.

Ultimately, it's not a huge issue.  Users who are able to build from
source very likely also know their package manager and how to translate
the invocations.  And if they're not building from source, then the bug
should go downstream first regardless.


signature.asc
Description: PGP 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] Improving bug reporting

2016-05-03 Thread Lukas Slebodnik
On (03/05/16 12:29), Robbie Harwood wrote:
>David Kupka  writes:
>
>> --8<- trac-ticket-template-proposal --->8--
>> Related SW versions:
>> On server:
>> {{{
>> $ rpm -q freeipa-server pki-base 389-ds-base bind samba krb5-server 
>> certmonger
>> }}}
>> On client:
>> {{{
>> $ rpm -q freeipa-client krb5-workstation certmonger
>> }}}
>
>I think this is a good idea.  However, we are on Debian/family as well
>now, and I think we want to accept bugs that come from these users as
>well.  So, in the interest of completeness, I believe the corresponding
>invocations are:
>
>> $ dpkg-query -W freeipa-server pki-base 389-ds-base bind9 samba \
>> krb5-kdc krb5-admin-server krb5-kpropd
>
>Note the split on the krb5 packaging; depending on what piece(s) you're
>checking for in the RPM krb5-server, this list is probably reducible.
>
>> $ dpkg-query -W freeipa-client krb5-user certmonger
>
>To give an example of what output of these looks like, here's a run of
>the server command on a machine that's missing some packages:
>
>> rharwood@thriss:~$ dpkg-query -W freeipa-server pki-base 389-ds-base \
>> bind9 samba krb5-kdc krb5-admin-server krb5-kpropd
>> bind9
>> krb5-admin-server   1.13.2+dfsg-5
>> krb5-kdc1.13.2+dfsg-5
>> samba   2:4.3.8+dfsg-1
>> dpkg-query: no packages found matching freeipa-server
>> dpkg-query: no packages found matching pki-base
>> dpkg-query: no packages found matching 389-ds-base
>> dpkg-query: no packages found matching krb5-kpropd
>> rharwood@thriss:~$ 
>
>i.e., it has bind9, krb5-admin-server, krb5-kdc, and samba, but is
>missing the rest of the requested packages.
>

FreeIPA is heavily patched on debian and has quite old version there
4.0.5.

The better would be recommend to reproduce with upstream version
(fedora/CentOS).

LS

-- 
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] External trust to AD

2016-05-03 Thread Alexander Bokovoy

On Tue, 03 May 2016, Simo Sorce wrote:

On Wed, 2016-03-02 at 21:11 +0200, Alexander Bokovoy wrote:

On Wed, 02 Mar 2016, Petr Vobornik wrote:
>On 03/02/2016 11:13 AM, Alexander Bokovoy wrote:
>>Hi,
>>
>>http://www.freeipa.org/page/V4/External_trust_to_AD documents a design
>>for external trust to AD feature.
>>
>>The text is included below for easier review.
>>---
>>{{Feature|version=TODO|ticket=TODO|author=Ab}}
>>
>>== Overview ==
>>Support for external trust to a domain from Active Directory forest
>>
>>An external trust is a trust relationship between Active Directory
>>domains that are in different Active Directory forests. While forest
>>trust always requires to establish trust between root domains of the
>>Active Directory forests, external trust can be established to any
>>domain within the forest.
>>
>>== Use Cases ==
>>
>>As an Active Directory domain admin, I want to establish trust between
>>IPA and my domain only. The trust between IPA and an external Active
>>Directory domain will be non-transitive as no users or groups from other
>>Active Directory domains will have access to IPA resources.
>>
>>== Design==
>>
>>External trust between Active Directory domains is by definition
>>non-transitive and enforces SID filtering between the domain boundaries.
>>This means only users and groups with SIDs from the trusted domain can
>>use the resources and be visible on IPA systems. None of other users and
>>groups from domains the trusted domain trusts within its own Active
>>Directory forest or other externally trusted domains will be allowed to
>>access IPA resources.
>>
>>== Implementation ==
>>
>>External trust feature re-uses existing forest trust infrastructure.
>>There are several specific changes to allow supporting external trust:
>>* '''Non-transitivity''': since external trust is non-transitive by
>>* definition, any attempt to set transitivity feature of the trust link
>>* with LSA SetInformationTrustedDomain() command will fail. Thus, there
>>* is no need to set transitivity for the external trust.
>
>Sounds very simple :)
>
>Do I get it right that it is possible to do it today? Because now the
>code just do:
>   root_logger.error('unable to set trust to transitive: %s' % (str(e)))
>   pass
I have a patchset to add this support already. I want to clean up some
parts of it, namely, reporting of the resulting trust type, but it all works.

>
>>* '''Trust attributes''': external trust can be detected by looking into
>>* absense of ipaNTTrustAttributes LDAP attribute of the trusted domain
>>* object.
>>
>>== Feature Management ==
>>
>>=== UI ===
>>An option 'external trust' needs to be added to Web UI, corresponding to
>>'--external' flag in 'trust-add' command in CLI.
>>
>>=== CLI ===
>>An external trust creation can be requested by passing additional flag
>>'--external=true' to the 'trust-add' command. The flag defaults to
>>'false', e.g. no external trust would be created.
>>
>>{| class="wikitable"
>>|-
>>! Command
>>! Options
>>|-
>>| trust-add
>>| --external=true/false
>>|}
>
>We should also add 'external' param to output of trust_find and
>trust_show + corresponding change in Web UI and CLI.
It will be part of trust type string, not a separate param.



I reviewed the design and associated tickts, and all checks out for me.
I have only one nitpick that is not spelled out.

What happens if someone has a forest trust and tries to also set up an
external trust with a child domain of the existing forest trust ?

Do we need to add code to prevent it, or should we allow it ?

I think we should prevent it. This is currently a bit unclear because
I'm trying to come to a common ground in understanding trust namespace
conflicts with Samba AD DC and Windows. The way Metze implemented
conflict checking in Samba AD DC is against Windows rules and I plan to
clear it during SambaXP so that we have common logic. Once the logic is
defined, I can run verification using the algorithm implemented in
Samba.


Secondary nitpick that came to mind right now, will one-way external
trust also be allowed/supported in the first implementation ?

Yes, it will -- the implementation I currently have is orthogonal to
one-way/two-way trust types.


I have some ideas on the answers to the above questions, but I think
they should be put in the design as considerations and explained there.

I agree, I'll add clarifications once everything is clear.

--
/ 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] External trust to AD

2016-05-03 Thread Simo Sorce
On Wed, 2016-03-02 at 21:11 +0200, Alexander Bokovoy wrote:
> On Wed, 02 Mar 2016, Petr Vobornik wrote:
> >On 03/02/2016 11:13 AM, Alexander Bokovoy wrote:
> >>Hi,
> >>
> >>http://www.freeipa.org/page/V4/External_trust_to_AD documents a design
> >>for external trust to AD feature.
> >>
> >>The text is included below for easier review.
> >>---
> >>{{Feature|version=TODO|ticket=TODO|author=Ab}}
> >>
> >>== Overview ==
> >>Support for external trust to a domain from Active Directory forest
> >>
> >>An external trust is a trust relationship between Active Directory
> >>domains that are in different Active Directory forests. While forest
> >>trust always requires to establish trust between root domains of the
> >>Active Directory forests, external trust can be established to any
> >>domain within the forest.
> >>
> >>== Use Cases ==
> >>
> >>As an Active Directory domain admin, I want to establish trust between
> >>IPA and my domain only. The trust between IPA and an external Active
> >>Directory domain will be non-transitive as no users or groups from other
> >>Active Directory domains will have access to IPA resources.
> >>
> >>== Design==
> >>
> >>External trust between Active Directory domains is by definition
> >>non-transitive and enforces SID filtering between the domain boundaries.
> >>This means only users and groups with SIDs from the trusted domain can
> >>use the resources and be visible on IPA systems. None of other users and
> >>groups from domains the trusted domain trusts within its own Active
> >>Directory forest or other externally trusted domains will be allowed to
> >>access IPA resources.
> >>
> >>== Implementation ==
> >>
> >>External trust feature re-uses existing forest trust infrastructure.
> >>There are several specific changes to allow supporting external trust:
> >>* '''Non-transitivity''': since external trust is non-transitive by
> >>* definition, any attempt to set transitivity feature of the trust link
> >>* with LSA SetInformationTrustedDomain() command will fail. Thus, there
> >>* is no need to set transitivity for the external trust.
> >
> >Sounds very simple :)
> >
> >Do I get it right that it is possible to do it today? Because now the 
> >code just do:
> >   root_logger.error('unable to set trust to transitive: %s' % (str(e)))
> >   pass
> I have a patchset to add this support already. I want to clean up some
> parts of it, namely, reporting of the resulting trust type, but it all works.
> 
> >
> >>* '''Trust attributes''': external trust can be detected by looking into
> >>* absense of ipaNTTrustAttributes LDAP attribute of the trusted domain
> >>* object.
> >>
> >>== Feature Management ==
> >>
> >>=== UI ===
> >>An option 'external trust' needs to be added to Web UI, corresponding to
> >>'--external' flag in 'trust-add' command in CLI.
> >>
> >>=== CLI ===
> >>An external trust creation can be requested by passing additional flag
> >>'--external=true' to the 'trust-add' command. The flag defaults to
> >>'false', e.g. no external trust would be created.
> >>
> >>{| class="wikitable"
> >>|-
> >>! Command
> >>! Options
> >>|-
> >>| trust-add
> >>| --external=true/false
> >>|}
> >
> >We should also add 'external' param to output of trust_find and 
> >trust_show + corresponding change in Web UI and CLI.
> It will be part of trust type string, not a separate param.


I reviewed the design and associated tickts, and all checks out for me.
I have only one nitpick that is not spelled out.

What happens if someone has a forest trust and tries to also set up an
external trust with a child domain of the existing forest trust ?

Do we need to add code to prevent it, or should we allow it ?

Secondary nitpick that came to mind right now, will one-way external
trust also be allowed/supported in the first implementation ?

I have some ideas on the answers to the above questions, but I think
they should be put in the design as considerations and explained there. 

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

-- 
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] Improving bug reporting

2016-05-03 Thread Petr Vobornik
On 05/03/2016 01:45 PM, David Kupka wrote:
> Hello everyone!
> 
> I often miss proper reproducer and other important info in trac tickets.
> Asking for the missing info or guessing and trying is as ineffective as
> it sounds and costs us a lot of time and effort. I believe we can
> improve that.
> 
> We have guidelines for reporting a bug [1] but it obviously isn't
> enough. I propose to prefill track ticket's description with following
> (or similar) template and be strict on refusing (i.e. closing as
> invalid) tickets that are incomplete.
> 
> Any thoughts, suggestions, agreement or disagreement?
> 
> [1] http://www.freeipa.org/page/Troubleshooting#Reporting_bugs
> 
> --8<- trac-ticket-template-proposal --->8--
> Related SW versions:
> On server:
> {{{
> $ rpm -q freeipa-server pki-base 389-ds-base bind samba krb5-server
> certmonger
> }}}
> On client:
> {{{
> $ rpm -q freeipa-client krb5-workstation certmonger
> }}}
> 
> Reproducible:
> How often the issue occurs or what special condition is required to be met.
> 
> Examples:
> Always / Happened X times of Y tries / Only at noon 29th February when
> it's also Thursday / Only on Raspberry Pi
> 
> Steps to reproduce:
> Precise description of all related steps you have done. List of commands
> to run is the best form.
> 
> Example:
> {{{
> # ipa-server-install -a Secret123 -p Secret123 -r EXAMPLE.TEST -U
> # echo Secret123 | kinit admin
> }}}
> 
> 
> Actual result:
> Description of behavior you have observed (error, unexpected warning, ...).
> 
> Example:
> {{{
> kinit: Client 'ad...@example.test' not found in Kerberos database while
> getting initial credentials
> }}}
> 
> Expected result:
> Description of behavior you have expected.
> 
> Example:
> TGT for admin user is acquired.
> --8<--->8--
> 

+1

I would also consider section "Impact". Sometimes it is not clear if the
reported just saw a error message but it works otherwise or something
doesn't not work at all. Or that an issue blocks half of integration
test suite.

-- 
Petr Vobornik

-- 
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] Improving bug reporting

2016-05-03 Thread Robbie Harwood
David Kupka  writes:

> --8<- trac-ticket-template-proposal --->8--
> Related SW versions:
> On server:
> {{{
> $ rpm -q freeipa-server pki-base 389-ds-base bind samba krb5-server 
> certmonger
> }}}
> On client:
> {{{
> $ rpm -q freeipa-client krb5-workstation certmonger
> }}}

I think this is a good idea.  However, we are on Debian/family as well
now, and I think we want to accept bugs that come from these users as
well.  So, in the interest of completeness, I believe the corresponding
invocations are:

> $ dpkg-query -W freeipa-server pki-base 389-ds-base bind9 samba \
> krb5-kdc krb5-admin-server krb5-kpropd

Note the split on the krb5 packaging; depending on what piece(s) you're
checking for in the RPM krb5-server, this list is probably reducible.

> $ dpkg-query -W freeipa-client krb5-user certmonger

To give an example of what output of these looks like, here's a run of
the server command on a machine that's missing some packages:

> rharwood@thriss:~$ dpkg-query -W freeipa-server pki-base 389-ds-base \
> bind9 samba krb5-kdc krb5-admin-server krb5-kpropd
> bind9
> krb5-admin-server   1.13.2+dfsg-5
> krb5-kdc1.13.2+dfsg-5
> samba   2:4.3.8+dfsg-1
> dpkg-query: no packages found matching freeipa-server
> dpkg-query: no packages found matching pki-base
> dpkg-query: no packages found matching 389-ds-base
> dpkg-query: no packages found matching krb5-kpropd
> rharwood@thriss:~$ 

i.e., it has bind9, krb5-admin-server, krb5-kdc, and samba, but is
missing the rest of the requested packages.

Thanks,
--Robbie


signature.asc
Description: PGP 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] [REVIEW] Intial stab towards Authentication Indicators

2016-05-03 Thread Petr Vobornik
On 05/03/2016 06:11 PM, Nathaniel McCallum wrote:
> On Mon, 2016-05-02 at 18:27 +0200, Petr Vobornik wrote:
>> Hi Matt, Nathaniel and Simo,
>>
>> I'd like to kindly check the status of this effort therefore
>> resurrecting this thread.
>>
>> First, Is the design up to date? Are there still aspects which need
>> to
>> be figured out?
> 
> I do not believe there are any remaining design decisions.
> 
>> Second execution, I see that Matt is about to finish
>> https://fedorahosted.org/freeipa/ticket/5782
> 
> +1
> 
>> Is it planned to handle CLI and Web UI as a part of ticket
>> https://fedorahosted.org/freeipa/ticket/433? If not then I can file
>> tickets for it.
> 
> I plan on doing the CLI work. We need a ticket for UI. This should
> hopefully be a trivial change.
> 
>> Will the rest be handled with variation of
>> https://github.com/npmccallum/freeipa/pull/1 and
>> https://github.com/npmccallum/freeipa/commit/a78191ee5d31e1de39f28eb6
>> 37f66199da7e9225
>> Or is an additional patch/work needed?   
> 
> One additional patch is needed for CLI. UI also needs a patch.
> 
> I will work on the CLI patch and clean up the existing patches. They
> should be on the list shortly.
> 

Thanks Nathaniel. Web UI ticket created:
https://fedorahosted.org/freeipa/ticket/5872 I expect that it will be
implemented by me or Pavel V.

-- 
Petr Vobornik

-- 
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] Another batch of Python 3 patches

2016-05-03 Thread Petr Viktorin
On 05/03/2016 04:31 PM, Martin Basti wrote:
> 
> 
> On 03.05.2016 15:52, Petr Viktorin wrote:
>> On 05/03/2016 03:02 PM, Petr Spacek wrote:
>>> On 2.5.2016 18:02, Martin Basti wrote:

 On 29.04.2016 19:46, Petr Viktorin wrote:
> Hello,
> These patches concentrate on tests, and code that was added/changed
> since I last looked at the FreeIPA project.
>
> With these patches, I'm back to getting the same errors under py2 and
> py3 when in test_xmlrpc.
>
>
>
>
 Patch 777:
 Could you fix all relative imports and enable check in pylint for that?
 (Remove relative-import from pylintrc), IMO there is just one extra
 relative
 import in custodia module.
>> Would it be OK if I do that in a separate patch, in the next batch? This
>> one is fixing the tests.
>> (I have the change in my worktree, so I won't forget when I next sit
>> down to work on IPA.)
> It is okay to send it in a next patch :)
> ACK on this patch then
 Do you plan to use in py2 ?
 from__future__importabsolute_import
>> I think that's unnecessary boilerplate; the errors this catches are
>> easily found by other means.
>> And it doesn't guard against someone forgetting the __future__ import
>> itself in a new file. The pylint check will be much better.
> Ok, just FYI pylint has check that prevents forgetting this import
> (disabled in IPA)
>
 Patch 778:
 LGTM

 Patch 779
 LGTM

 Patch 780
 LGTM

 Patch 781
 LGTM

 Patch 782
 Not sure, I will review it longer

 Patch 783
 LGTM

 Patch 784
 LGTM

 Patch 785
 LGTM

 I will test it with both py2 and py3 to convert LGTM to ACK :)
>>> Functional ACK, I did not find any breakage (when combined with other
>>> Py3
>>> patches).
>>>
>>
> Hold your horses :D, I probably find something in tests
> 
> I run ipa-run-tests with xmlrpc tests under python2 and python3, please
> note the different count of tests and errors in py3
> 
> platform linux2 -- Python 2.7.11, pytest-2.8.7, py-1.4.31, pluggy-0.3.1
> -- /usr/bin/python
> rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
> collecting ... collected 1835 items
> 
> platform linux -- Python 3.5.1, pytest-2.9.1, py-1.4.31, pluggy-0.3.1 --
> /bin/python3
> rootdir: /usr/lib/python3.5/site-packages/ipatests, inifile: pytest.ini
> collecting ... collected 1694 items / 7 errors
> 
> 
> Collecting failed on following import errors:
>test_xmlrpc/test_add_remove_cert_cmd.py:13: in 
> ipatests.test_xmlrpc.testcert import get_testcert
>xmlrpc/testcert.py:34: in 
> ipaserver.plugins import rabase
>ImportError: No module named 'ipaserver'
> 
> And I found more errors, but they may be unrelated I have to investigate
> more
> Martin^2

Right, some of the xmlrpc tests use ipaserver, which isn't fully ported
yet, and the python3-ipaserver RPM isn't even built. The parts of
ipaserver needed for these tests will be my next goal.

-- 
Petr Viktorin

-- 
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] [REVIEW] Intial stab towards Authentication Indicators

2016-05-03 Thread Nathaniel McCallum
On Mon, 2016-05-02 at 18:27 +0200, Petr Vobornik wrote:
> Hi Matt, Nathaniel and Simo,
> 
> I'd like to kindly check the status of this effort therefore
> resurrecting this thread.
> 
> First, Is the design up to date? Are there still aspects which need
> to
> be figured out?

I do not believe there are any remaining design decisions.

> Second execution, I see that Matt is about to finish
> https://fedorahosted.org/freeipa/ticket/5782

+1

> Is it planned to handle CLI and Web UI as a part of ticket
> https://fedorahosted.org/freeipa/ticket/433? If not then I can file
> tickets for it.

I plan on doing the CLI work. We need a ticket for UI. This should
hopefully be a trivial change.

> Will the rest be handled with variation of
> https://github.com/npmccallum/freeipa/pull/1 and
> https://github.com/npmccallum/freeipa/commit/a78191ee5d31e1de39f28eb6
> 37f66199da7e9225
> Or is an additional patch/work needed?

One additional patch is needed for CLI. UI also needs a patch.

I will work on the CLI patch and clean up the existing patches. They
should be on the list shortly.

-- 
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] Updated ipa command man page

2016-05-03 Thread Martin Basti



On 03.05.2016 15:21, Petr Spacek wrote:

On 2.5.2016 09:41, Petr Spacek wrote:

On 29.4.2016 12:57, Abhijeet Kasurde wrote:

Hi All,

Please review this patch.

LGTM

Okay, so I applied it!

ACK ;-)


trac ticket (#5871) added to commit message


Pushed to:
master: 7d46fd15f84ee31b4141ed3fc53b4ddb3fbe4167
ipa-4-3: 2c0c7bece2f90dba568a39db68b835e99ccbc9ba

--
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 provide more information for "ipa cert-revoke -h"

2016-05-03 Thread Patrice Duc-Jacquet

On 05/03/2016 04:41 PM, Rob Crittenden wrote:


Gabe Alford wrote:

Hello,

Thank you for your patch as well.

 >-doc=_('Reason for revoking the certificate (0-10)'),
 >+doc=_('Reason for revoking the certificate (0-10). See
RFC 5280 (paragraph 5.3.1) for reason details'),

Rather than just specifying the RFC with the paragraph to go look up,
can you either add the revocation options or say something like:

+doc=_('Reason for revoking the certificate (0-10). See
\'ipa help cert\' for revocation reason details.'),

IMO, it is a little annoying to go look up revocation reasons when those
reasons can either be added to the help output or exist already in `ipa
help cert`.


FTR I added it to the top level help because the reasons are used in 
multiple places and didn't want to duplicate them, and adding them to 
a specific option help would overload it big time IMHO.


rob


Hi everyone
thanks for your valuable comments. I fully agree that it is not 
recommended to duplicate this information. So as Rob suggested, I should 
avoid to add this information to cert_revoke option and thus I plan to 
modify the help message as follow:


doc=_('Reason for revoking the certificate (0-10). Type "ipa help cert" 
for reason details'),


Do you agree with that modification? Thanks in advance and regards

Pat

--
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] [TESTS][PATCH 0012] Provide cleanup for host certificate

2016-05-03 Thread Martin Basti



On 03.05.2016 14:26, Lenka Doudova wrote:



On 05/03/2016 02:08 PM, Lenka Doudova wrote:



On 05/03/2016 12:15 PM, Martin Basti wrote:



On 03.05.2016 11:18, Lenka Doudova wrote:



On 05/03/2016 10:33 AM, Martin Basti wrote:
Hello I'm quite confused what is happening in that code, can you 
explain it more to me? I see duplicated code there.

Sorry, that was just an unnecessary leftover. Fixed patch attached.
The code is expected to remove any certificates that were added to 
the local host but not to try to remove the host itself.


Lenka



Martin^2


Looks better, please follow proper naming of patches according how 
to format patch guide.
I propose following (Patch attached) changes. It looks weird to me 
to return self object


Martin


Ok, fixed patch attached.

Thanks,
Lenka



And one more small change.
Lenka




ACK

Pushed to:
ipa-4-3: c8330a9b09c1999d2721f3fb2b2170b3c1568b22
master: 847c950408b3c00ce3a4625709cadcddf39af6a5
-- 
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 provide more information for "ipa cert-revoke -h"

2016-05-03 Thread Rob Crittenden

Gabe Alford wrote:

Hello,

Thank you for your patch as well.

 >-doc=_('Reason for revoking the certificate (0-10)'),
 >+doc=_('Reason for revoking the certificate (0-10). See
RFC 5280 (paragraph 5.3.1) for reason details'),

Rather than just specifying the RFC with the paragraph to go look up,
can you either add the revocation options or say something like:

+doc=_('Reason for revoking the certificate (0-10). See
\'ipa help cert\' for revocation reason details.'),

IMO, it is a little annoying to go look up revocation reasons when those
reasons can either be added to the help output or exist already in `ipa
help cert`.


FTR I added it to the top level help because the reasons are used in 
multiple places and didn't want to duplicate them, and adding them to a 
specific option help would overload it big time IMHO.


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


Re: [Freeipa-devel] Another batch of Python 3 patches

2016-05-03 Thread Martin Basti



On 03.05.2016 15:52, Petr Viktorin wrote:

On 05/03/2016 03:02 PM, Petr Spacek wrote:

On 2.5.2016 18:02, Martin Basti wrote:


On 29.04.2016 19:46, Petr Viktorin wrote:

Hello,
These patches concentrate on tests, and code that was added/changed
since I last looked at the FreeIPA project.

With these patches, I'm back to getting the same errors under py2 and
py3 when in test_xmlrpc.





Patch 777:
Could you fix all relative imports and enable check in pylint for that?
(Remove relative-import from pylintrc), IMO there is just one extra relative
import in custodia module.

Would it be OK if I do that in a separate patch, in the next batch? This
one is fixing the tests.
(I have the change in my worktree, so I won't forget when I next sit
down to work on IPA.)

It is okay to send it in a next patch :)
ACK on this patch then

Do you plan to use in py2 ?
from__future__importabsolute_import

I think that's unnecessary boilerplate; the errors this catches are
easily found by other means.
And it doesn't guard against someone forgetting the __future__ import
itself in a new file. The pylint check will be much better.
Ok, just FYI pylint has check that prevents forgetting this import 
(disabled in IPA)

Patch 778:
LGTM

Patch 779
LGTM

Patch 780
LGTM

Patch 781
LGTM

Patch 782
Not sure, I will review it longer

Patch 783
LGTM

Patch 784
LGTM

Patch 785
LGTM

I will test it with both py2 and py3 to convert LGTM to ACK :)

Functional ACK, I did not find any breakage (when combined with other Py3
patches).




Hold your horses :D, I probably find something in tests

I run ipa-run-tests with xmlrpc tests under python2 and python3, please 
note the different count of tests and errors in py3


platform linux2 -- Python 2.7.11, pytest-2.8.7, py-1.4.31, pluggy-0.3.1 
-- /usr/bin/python

rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
collecting ... collected 1835 items

platform linux -- Python 3.5.1, pytest-2.9.1, py-1.4.31, pluggy-0.3.1 -- 
/bin/python3

rootdir: /usr/lib/python3.5/site-packages/ipatests, inifile: pytest.ini
collecting ... collected 1694 items / 7 errors


Collecting failed on following import errors:
   test_xmlrpc/test_add_remove_cert_cmd.py:13: in 
ipatests.test_xmlrpc.testcert import get_testcert
   xmlrpc/testcert.py:34: in 
ipaserver.plugins import rabase
   ImportError: No module named 'ipaserver'

And I found more errors, but they may be unrelated I have to investigate 
more

Martin^2

--
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 provide more information for "ipa cert-revoke -h"

2016-05-03 Thread Gabe Alford
Hello,

Thank you for your patch as well.

>-doc=_('Reason for revoking the certificate (0-10)'),
>+doc=_('Reason for revoking the certificate (0-10). See RFC
5280 (paragraph 5.3.1) for reason details'),

Rather than just specifying the RFC with the paragraph to go look up, can
you either add the revocation options or say something like:

+doc=_('Reason for revoking the certificate (0-10). See \'ipa
help cert\' for revocation reason details.'),

IMO, it is a little annoying to go look up revocation reasons when those
reasons can either be added to the help output or exist already in `ipa
help cert`.

Thanks,

Gabe


On Tue, May 3, 2016 at 8:13 AM, Martin Basti  wrote:

>
>
> On 03.05.2016 16:01, Patrice Duc-Jacquet wrote:
>
> Hi everyone
> this is my first patch. So I may have done thhings nor in  a proper way.
> Please let me know if something is wrong in the proceess I followed. With
> regards
>
> Pat
>
>
> Hello,
>
> thank you for your patch. Please remove changes in .po and .pot files from
> the patch, these files are generated automatically from zanata.
>
> thank you
>
> 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
>
-- 
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 provide more information for "ipa cert-revoke -h"

2016-05-03 Thread Martin Basti



On 03.05.2016 16:01, Patrice Duc-Jacquet wrote:

Hi everyone
this is my first patch. So I may have done thhings nor in  a proper 
way. Please let me know if something is wrong in the proceess I 
followed. With regards


Pat



Hello,

thank you for your patch. Please remove changes in .po and .pot files 
from the patch, these files are generated automatically from zanata.


thank you

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

[Freeipa-devel] [PATCH] 0001 provide more information for "ipa cert-revoke -h"

2016-05-03 Thread Patrice Duc-Jacquet

Hi everyone
this is my first patch. So I may have done thhings nor in  a proper way. 
Please let me know if something is wrong in the proceess I followed. 
With regards


Pat
>From bb06bd55c9f68af1a3aa01b671757abfbb45c822 Mon Sep 17 00:00:00 2001
From: Patrice Duc-Jacquet 
Date: Tue, 3 May 2016 15:17:16 +0200
Subject: [PATCH] Add more information regarding where to find revocation
 reason in "ipa cert_revoke -h" and "ipa cert_find -h"  commands. More
 precisely a pointer to RFC 5280 (paragraph 5.3.1) which contain the
 definition of the revocation code.

So basically "ipa cert_revoke -h command" now returns :

$ ipa cert_revoke -h
Usage: ipa [global-options] cert-revoke SERIAL-NUMBER [options]

Revoke a certificate.
Options:
  -h, --helpshow this help message and exit
  --revocation-reason=INT
Reason for revoking the certificate (0-10). See RFC
5280 (paragraph 5.3.1) for reason details

ipa.pot and correspondig .po files have also been modified to take into account this new string.

https://fedorahosted.org/freeipa/ticket/5819
---
 install/po/de.po   | 2 +-
 install/po/es.po   | 2 +-
 install/po/fr.po   | 2 +-
 install/po/ipa.pot | 2 +-
 install/po/kn.po   | 2 +-
 install/po/pl.po   | 2 +-
 install/po/ru.po   | 2 +-
 install/po/uk.po   | 2 +-
 install/po/zh_CN.po| 2 +-
 ipalib/plugins/cert.py | 4 ++--
 10 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/install/po/de.po b/install/po/de.po
index 35d20932060ea8865aa8cb7b19a57e4e0e3d8469..52509711c3bdb881d28e85f5da1326f4b606d453 100644
--- a/install/po/de.po
+++ b/install/po/de.po
@@ -1343,7 +1343,7 @@ msgstr "Widerrufen"
 msgid "Reason"
 msgstr "Grund"
 
-msgid "Reason for revoking the certificate (0-10)"
+msgid "Reason for revoking the certificate (0-10). See RFC 5280 (paragraph 5.3.1) for reason details"
 msgstr "Grund für den Widerruf des Zertifikats (0-10)"
 
 msgid "7 is not a valid revocation reason"
diff --git a/install/po/es.po b/install/po/es.po
index 48f4284e6d0ba1650dc77a97e7c67532411dd392..3e76f419403381c675dc0b461746488a00d7e84a 100644
--- a/install/po/es.po
+++ b/install/po/es.po
@@ -1402,7 +1402,7 @@ msgstr "Revocado"
 msgid "Reason"
 msgstr "Motivo"
 
-msgid "Reason for revoking the certificate (0-10)"
+msgid "Reason for revoking the certificate (0-10). See RFC 5280 (paragraph 5.3.1) for reason details"
 msgstr "Motivo por el cual el certificado ha sido revocado (0-10)"
 
 msgid "7 is not a valid revocation reason"
diff --git a/install/po/fr.po b/install/po/fr.po
index cefe28797ba0d89e7361980e3f851577738d8b63..f250b228678706f8908bc6a6e29972f975df5786 100644
--- a/install/po/fr.po
+++ b/install/po/fr.po
@@ -2896,7 +2896,7 @@ msgstr "Révoqué"
 msgid "Reason"
 msgstr "Raison"
 
-msgid "Reason for revoking the certificate (0-10)"
+msgid "Reason for revoking the certificate (0-10). See RFC 5280 (paragraph 5.3.1) for reason details"
 msgstr "Raison de révocation du certificat (0-10)"
 
 msgid "7 is not a valid revocation reason"
diff --git a/install/po/ipa.pot b/install/po/ipa.pot
index 8256bb77da282d6c327a761ffd07c31b8fc7bf28..e89f035b20cac28034ba36accf7ffeba4306815d 100644
--- a/install/po/ipa.pot
+++ b/install/po/ipa.pot
@@ -3114,7 +3114,7 @@ msgid "Reason"
 msgstr ""
 
 #: ipalib/plugins/cert.py:669 ipalib/plugins/cert.py:737
-msgid "Reason for revoking the certificate (0-10)"
+msgid "Reason for revoking the certificate (0-10). See RFC 5280 (paragraph 5.3.1) for reason details"
 msgstr ""
 
 #: ipalib/plugins/cert.py:692
diff --git a/install/po/kn.po b/install/po/kn.po
index e3b6f67092e30b4684a3b665966082c79ad26e59..b2551c726ab4fded523830d6b8f6f86a1b756463 100644
--- a/install/po/kn.po
+++ b/install/po/kn.po
@@ -414,7 +414,7 @@ msgstr "ರದ್ದು ಮಾಡಲಾಗಿದೆ"
 msgid "Reason"
 msgstr "ಕಾರಣ"
 
-msgid "Reason for revoking the certificate (0-10)"
+msgid "Reason for revoking the certificate (0-10). See RFC 5280 (paragraph 5.3.1) for reason details"
 msgstr "ಪ್ರಮಾಣಪತ್ರವನ್ನು (0-10) ರದ್ದು ಮಾಡಲು ಕಾರಣ"
 
 msgid "Unrevoked"
diff --git a/install/po/pl.po b/install/po/pl.po
index 2f98114435081cd3b4da676608fb804292c0cff5..cc0b2fc9a1e32bf076387bf2449f5101d302dade 100644
--- a/install/po/pl.po
+++ b/install/po/pl.po
@@ -587,7 +587,7 @@ msgstr "Unieważniono"
 msgid "Reason"
 msgstr "Przyczyna"
 
-msgid "Reason for revoking the certificate (0-10)"
+msgid "Reason for revoking the certificate (0-10). See RFC 5280 (paragraph 5.3.1) for reason details"
 msgstr "Przyczyna unieważnienia certyfikatu (0-10)"
 
 msgid "Unrevoked"
diff --git a/install/po/ru.po b/install/po/ru.po
index b24064f4dc0e56bb8a669dfe5949d0787146835c..a275d72a0a1a5db0c519f83fbf72186f3f747edf 100644
--- a/install/po/ru.po
+++ b/install/po/ru.po
@@ -1268,7 +1268,7 @@ msgstr "Отозван"
 msgid "Reason"
 msgstr "Причина"
 
-msgid "Reason for revoking the certificate 

Re: [Freeipa-devel] Improving bug reporting

2016-05-03 Thread Abhijeet Kasurde



On 05/03/2016 07:05 PM, Jakub Hrozek wrote:

On Tue, May 03, 2016 at 01:45:39PM +0200, David Kupka wrote:

Hello everyone!

I often miss proper reproducer and other important info in trac tickets.
Asking for the missing info or guessing and trying is as ineffective as it
sounds and costs us a lot of time and effort. I believe we can improve that.

We have guidelines for reporting a bug [1] but it obviously isn't enough. I
propose to prefill track ticket's description with following (or similar)
template and be strict on refusing (i.e. closing as invalid) tickets that
are incomplete.

Any thoughts, suggestions, agreement or disagreement?

I'll just throw the sssd page we have on the subject:
 https://fedorahosted.org/sssd/wiki/Reporting_sssd_bugs

I would at least add the note about security-sensitive bugs to the
freeipa page, we really don't want CVEs being reported to trac :-)

Is there any automated script which can do this for us, something like 
sosreport ?
If not then it would be good idea to invest time and efforts to have one 
script to gather all logs and reports.


Thanks,
Abhijeet Kasurde

--
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] Another batch of Python 3 patches

2016-05-03 Thread Petr Viktorin
On 05/03/2016 03:02 PM, Petr Spacek wrote:
> On 2.5.2016 18:02, Martin Basti wrote:
>>
>>
>> On 29.04.2016 19:46, Petr Viktorin wrote:
>>> Hello,
>>> These patches concentrate on tests, and code that was added/changed
>>> since I last looked at the FreeIPA project.
>>>
>>> With these patches, I'm back to getting the same errors under py2 and
>>> py3 when in test_xmlrpc.
>>>
>>>
>>>
>>>
>> Patch 777:
>> Could you fix all relative imports and enable check in pylint for that?
>> (Remove relative-import from pylintrc), IMO there is just one extra relative
>> import in custodia module.

Would it be OK if I do that in a separate patch, in the next batch? This
one is fixing the tests.
(I have the change in my worktree, so I won't forget when I next sit
down to work on IPA.)

>> Do you plan to use in py2 ?
>> from__future__importabsolute_import

I think that's unnecessary boilerplate; the errors this catches are
easily found by other means.
And it doesn't guard against someone forgetting the __future__ import
itself in a new file. The pylint check will be much better.

>>
>> Patch 778:
>> LGTM
>>
>> Patch 779
>> LGTM
>>
>> Patch 780
>> LGTM
>>
>> Patch 781
>> LGTM
>>
>> Patch 782
>> Not sure, I will review it longer
>>
>> Patch 783
>> LGTM
>>
>> Patch 784
>> LGTM
>>
>> Patch 785
>> LGTM
>>
>> I will test it with both py2 and py3 to convert LGTM to ACK :)
> 
> Functional ACK, I did not find any breakage (when combined with other Py3
> patches).
> 


-- 
Petr Viktorin

-- 
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 0068] Use ipareplica-ca-install.log instead of ipaserver-ca-install.log

2016-05-03 Thread Gabe Alford
Hello,

Fix for https://fedorahosted.org/freeipa/ticket/5727. Per comment #7, this
removes ipaserver-ca-install.log and uses ipareplica-ca-install.log.

Thanks,

Gabe
From 9f8cb593c1b207d96693879fbd8717a78421e157 Mon Sep 17 00:00:00 2001
From: Gabe 
Date: Tue, 3 May 2016 07:30:13 -0600
Subject: [PATCH] Use ipareplica-ca-install.log instead of
 ipaserver-ca-install.log

https://fedorahosted.org/freeipa/ticket/5727
---
 install/tools/ipa-ca-install | 2 +-
 ipaplatform/base/paths.py| 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index 1bc5def03bf687a1e4f9fb38a54363b5429c8fc4..2947009f58ba7ef96ec303e7731dc9b3fdfc8ff2 100755
--- a/install/tools/ipa-ca-install
+++ b/install/tools/ipa-ca-install
@@ -280,7 +280,7 @@ def main():
 cainstance.is_ca_installed_locally()):
 sys.exit("CA is already installed on this host.")
 
-standard_logging_setup(paths.IPASERVER_CA_INSTALL_LOG, debug=options.debug)
+standard_logging_setup(paths.IPAREPLICA_CA_INSTALL_LOG, debug=options.debug)
 root_logger.debug("%s was invoked with options: %s,%s",
   sys.argv[0], safe_options, filename)
 root_logger.debug("IPA version %s", version.VENDOR_VERSION)
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index ca7eb6cf47b4442fa538a47c74846e13c25e02e8..6d07621b8c001a6a1bc6baa8e5bcb775136d7a62 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -304,7 +304,6 @@ class BasePathNamespace(object):
 IPAREPLICA_CONNCHECK_LOG = "/var/log/ipareplica-conncheck.log"
 IPAREPLICA_INSTALL_LOG = "/var/log/ipareplica-install.log"
 IPARESTORE_LOG = "/var/log/iparestore.log"
-IPASERVER_CA_INSTALL_LOG = "/var/log/ipaserver-ca-install.log"
 IPASERVER_INSTALL_LOG = "/var/log/ipaserver-install.log"
 IPASERVER_KRA_INSTALL_LOG = "/var/log/ipaserver-kra-install.log"
 IPASERVER_KRA_UNINSTALL_LOG = "/var/log/ipaserver-kra-uninstall.log"
-- 
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

Re: [Freeipa-devel] Improving bug reporting

2016-05-03 Thread Jakub Hrozek
On Tue, May 03, 2016 at 01:45:39PM +0200, David Kupka wrote:
> Hello everyone!
> 
> I often miss proper reproducer and other important info in trac tickets.
> Asking for the missing info or guessing and trying is as ineffective as it
> sounds and costs us a lot of time and effort. I believe we can improve that.
> 
> We have guidelines for reporting a bug [1] but it obviously isn't enough. I
> propose to prefill track ticket's description with following (or similar)
> template and be strict on refusing (i.e. closing as invalid) tickets that
> are incomplete.
> 
> Any thoughts, suggestions, agreement or disagreement?

I'll just throw the sssd page we have on the subject:
https://fedorahosted.org/sssd/wiki/Reporting_sssd_bugs

I would at least add the note about security-sensitive bugs to the
freeipa page, we really don't want CVEs being reported to trac :-)

-- 
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] Updated ipa command man page

2016-05-03 Thread Petr Spacek
On 2.5.2016 09:41, Petr Spacek wrote:
> On 29.4.2016 12:57, Abhijeet Kasurde wrote:
>> Hi All,
>>
>> Please review this patch.
> 
> LGTM

Okay, so I applied it!

ACK ;-)

-- 
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] Fix added to ipa-compat-manage command line help

2016-05-03 Thread Petr Spacek
On 2.5.2016 12:43, Abhijeet Kasurde wrote:
> Hi All,
> 
> Please review this patch.

ACK

-- 
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 0097-0098] Makefile: replace perl with sed

2016-05-03 Thread Petr Spacek
On 25.4.2016 10:12, Lukas Slebodnik wrote:
> On (25/04/16 09:59), Jan Cholasta wrote:
>> On 25.4.2016 09:34, Petr Spacek wrote:
>>> On 25.4.2016 09:29, Lukas Slebodnik wrote:
 On (25/04/16 07:23), Jan Cholasta wrote:
> Hi,
>
> On 22.4.2016 13:29, Petr Spacek wrote:
>> Hello,
>>
>> Makefile: add sed to BuildRequires
>>
>> It was requried since forever but we did not explicitly mention it.
>
> IIRC sed is part of the minimum build environemnt and as such should not 
> be
> explicitly required in the spec file. I personally don't care, but this is
> the likely reason why it wan't there from the beginning.
>
 +1

 It is part of group "@buildsys-build".
 and fedora packaging guidelines does not recommend to list
 packages from this group in BuildRequires.
>>>
>>> I consider this piece of Fedora guidelines brain-dead as "explicit is better
>>> than implicit". Anyway, feel free to NACK it so the status of the patch is
>>> clear and this thread can die. I do not insist on it.
>>
>> I can't find it in the guidelines anymore, so LGTM.
>>
> It seems that it was changed since I read it last time.
> 
> There is vague description of which packages should be there.
> http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires_2
>It is important that your package list all necessary build dependencies
>using the BuildRequires?:
>tag. You may assume that enough of an environment exists for RPM to 
> function
>and execute basic shell scripts, but you should not assume any other 
> packages
>are present as RPM dependencies and anything brought into the buildroot
>by the build system may change over time.
> 
> But utility fedora-review still complains if you list packages from group
> "@buildsys-build"

So, should I drop the patch from my queue or not?

I still think that it is brain-dead not to list dependencies explicitly but
you tell me if I should remove the patch from my waiting-for-review-queue or 
not.

-- 
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] Another batch of Python 3 patches

2016-05-03 Thread Petr Spacek
On 2.5.2016 18:02, Martin Basti wrote:
> 
> 
> On 29.04.2016 19:46, Petr Viktorin wrote:
>> Hello,
>> These patches concentrate on tests, and code that was added/changed
>> since I last looked at the FreeIPA project.
>>
>> With these patches, I'm back to getting the same errors under py2 and
>> py3 when in test_xmlrpc.
>>
>>
>>
>>
> Patch 777:
> Could you fix all relative imports and enable check in pylint for that?
> (Remove relative-import from pylintrc), IMO there is just one extra relative
> import in custodia module.
> 
> Do you plan to use in py2 ?
> from__future__importabsolute_import
> 
> Patch 778:
> LGTM
> 
> Patch 779
> LGTM
> 
> Patch 780
> LGTM
> 
> Patch 781
> LGTM
> 
> Patch 782
> Not sure, I will review it longer
> 
> Patch 783
> LGTM
> 
> Patch 784
> LGTM
> 
> Patch 785
> LGTM
> 
> I will test it with both py2 and py3 to convert LGTM to ACK :)

Functional ACK, I did not find any breakage (when combined with other Py3
patches).

-- 
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] 0770 Switch /usr/bin/ipa to Python 3

2016-05-03 Thread Petr Spacek
On 29.4.2016 19:49, Petr Viktorin wrote:
> On 04/12/2016 12:52 PM, Petr Spacek wrote:
>> On 19.2.2016 13:50, Petr Viktorin wrote:
>>> Is it time yet?
>>>
>>> This patch switches /usr/bin/ipa to Python 3 for
>>> - the in-tree ./ipa command
>>> - RPMs, when built with_python3
>>
>> NACK, the change in 'ipa' command broke ipa dnszone-find:
>>
>> # ipa dnsrecord-find dom-033.abc.idm.lab.eng.brq.redhat.com.
>> ipa: ERROR: b'dom-033.abc.idm.lab.eng.brq.redhat.com.'.: DNS zone not found
>>
>> The same command works when I switch python3->python2 in the 'ipa' command.
> 
> That error is now fixed, could you please re-review?

ACK, I did not find any breakage (when combined with other Py3 patches).

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


[Freeipa-devel] [PATCH 0104-0109] DNS upgrade: change forwarding policy to "only" if private IPs are used

2016-05-03 Thread Petr Spacek
Hello,

DNS upgrade: change forwarding policy to "only" if private IPs are used.

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

This is the upgrade part. I will add one more patch to print a warning in
dnsforwardzone* commands to avoid surprises. Please do not close the ticket yet.

-- 
Petr^2 Spacek
From b1d75807e0f8a117fe4b28f1c83054bb3ff1db6c Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Mon, 25 Apr 2016 14:07:16 +0200
Subject: [PATCH] Add ipaDNSVersion option to dnsconfig* commands and use new
 attribute

Ad-hoc LDAP calls in DNS upgrade code were hard to maintain and
ipaConfigString was bad idea from the very beginning as it was hard to
manipulate the number in it.

To avoid problems in future we are introducing new ipaDNSVersion
attribute which is used on cn=dns instead of ipaConfigString.
Original value of ipaConfigString is kept in the tree for now
so older upgraders see it and do not execute the upgrade procedure again.

The attribute can be changed only by installer/upgrade so it is not
exposed in dnsconfig_mod API.

Command dnsconfig_show displays it only if --all option was used.

https://fedorahosted.org/freeipa/ticket/5710
---
 install/share/60ipadns.ldif|  2 +
 install/share/dns.ldif |  4 +-
 install/updates/40-dns.update  |  1 -
 install/updates/90-post_upgrade_plugins.update |  1 +
 ipalib/plugins/dns.py  | 20 +--
 ipaserver/install/plugins/dns.py   | 73 --
 6 files changed, 77 insertions(+), 24 deletions(-)

diff --git a/install/share/60ipadns.ldif b/install/share/60ipadns.ldif
index e0ed0ab869cea0478d9640bb509c6267abed1a01..71b99d4d03c34591dc83a5706d300727f3f77f30 100644
--- a/install/share/60ipadns.ldif
+++ b/install/share/60ipadns.ldif
@@ -70,9 +70,11 @@ attributeTypes: ( 2.16.840.1.113730.3.8.5.25 NAME 'idnsSecKeyRevoke' DESC 'DNSKE
 attributeTypes: ( 2.16.840.1.113730.3.8.5.26 NAME 'idnsSecKeySep' DESC 'DNSKEY SEP flag (equivalent to bit 15): RFC 4035' EQUALITY booleanMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN 'IPA v4.1' )
 attributeTypes: ( 2.16.840.1.113730.3.8.5.27 NAME 'idnsSecAlgorithm' DESC 'DNSKEY algorithm: string used as mnemonic' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 SINGLE-VALUE X-ORIGIN 'IPA v4.1' )
 attributeTypes: ( 2.16.840.1.113730.3.8.5.28 NAME 'idnsSecKeyRef' DESC 'PKCS#11 URI of the key' EQUALITY caseExactMatch SINGLE-VALUE SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v4.1' )
+attributeTypes: ( 2.16.840.1.113730.3.8.11.74 NAME 'ipaDNSVersion' DESC 'IPA DNS data version' EQUALITY integerMatch ORDERING integerOrderingMatch SINGLE-VALUE SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 X-ORIGIN 'IPA v4.3' )
 objectClasses: ( 2.16.840.1.113730.3.8.6.0 NAME 'idnsRecord' DESC 'dns Record, usually a host' SUP top STRUCTURAL MUST idnsName MAY ( cn $ idnsAllowDynUpdate $ dNSTTL $ dNSClass $ aRecord $ aAAARecord $ a6Record $ nSRecord $ cNAMERecord $ pTRRecord $ sRVRecord $ tXTRecord $ mXRecord $ mDRecord $ hInfoRecord $ mInfoRecord $ aFSDBRecord $ SigRecord $ KeyRecord $ LocRecord $ nXTRecord $ nAPTRRecord $ kXRecord $ certRecord $ dNameRecord $ dSRecord $ sSHFPRecord $ rRSIGRecord $ nSECRecord $ DLVRecord $ TLSARecord $ UnknownRecord $ RPRecord $ APLRecord $ IPSECKEYRecord $ DHCIDRecord $ HIPRecord $ SPFRecord ) )
 objectClasses: ( 2.16.840.1.113730.3.8.6.1 NAME 'idnsZone' DESC 'Zone class' SUP idnsRecord STRUCTURAL MUST ( idnsZoneActive $ idnsSOAmName $ idnsSOArName $ idnsSOAserial $ idnsSOArefresh $ idnsSOAretry $ idnsSOAexpire $ idnsSOAminimum ) MAY ( idnsUpdatePolicy $ idnsAllowQuery $ idnsAllowTransfer $ idnsAllowSyncPTR $ idnsForwardPolicy $ idnsForwarders $ idnsSecInlineSigning $ nSEC3PARAMRecord ) )
 objectClasses: ( 2.16.840.1.113730.3.8.6.2 NAME 'idnsConfigObject' DESC 'DNS global config options' STRUCTURAL MAY ( idnsForwardPolicy $ idnsForwarders $ idnsAllowSyncPTR $ idnsZoneRefresh $ idnsPersistentSearch ) )
 objectClasses: ( 2.16.840.1.113730.3.8.12.18 NAME 'ipaDNSZone' SUP top AUXILIARY MUST idnsName MAY managedBy X-ORIGIN 'IPA v3' )
 objectClasses: ( 2.16.840.1.113730.3.8.6.3 NAME 'idnsForwardZone' DESC 'Forward Zone class' SUP top STRUCTURAL MUST ( idnsName $ idnsZoneActive ) MAY ( idnsForwarders $ idnsForwardPolicy ) )
 objectClasses: ( 2.16.840.1.113730.3.8.6.4 NAME 'idnsSecKey' DESC 'DNSSEC key metadata' STRUCTURAL MUST ( idnsSecKeyRef $ idnsSecKeyCreated $ idnsSecAlgorithm ) MAY ( idnsSecKeyPublish $ idnsSecKeyActivate $ idnsSecKeyInactive $ idnsSecKeyDelete $ idnsSecKeyZone $ idnsSecKeyRevoke $ idnsSecKeySep $ cn ) X-ORIGIN 'IPA v4.1' )
+objectClasses: ( 2.16.840.1.113730.3.8.12.36 NAME 'ipaDNSContainer' DESC 'IPA DNS container' AUXILIARY MUST ( ipaDNSVersion ) X-ORIGIN 'IPA v4.3' )
diff --git a/install/share/dns.ldif b/install/share/dns.ldif
index 42b41a8d706a8a3fd826320aff6c9333264128fc..d71e2ad7d7ca530247198d9db71aebd497d0c121 

[Freeipa-devel] [PATCH 0103] DNS installer: accept --auto-forwarders option in unattended mode

2016-05-03 Thread Petr Spacek
Hello,

DNS installer: accept --auto-forwarders option in unattended mode

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

-- 
Petr^2 Spacek
From f1ad3a19f250d88502f47e3b5541388e14ab5f08 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Tue, 3 May 2016 14:12:44 +0200
Subject: [PATCH] DNS installer: accept --auto-forwarders option in unattended
 mode

https://fedorahosted.org/freeipa/ticket/5869
---
 install/tools/ipa-dns-install | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index d8b2eb0fe2842208c64a3a8bb4e4fc295681fb9b..4138622356e49a02750da3084667604ae7426769 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -104,8 +104,11 @@ def parse_options():
 parser.error("You cannot specify a --auto-reverse option together with --no-reverse")
 
 if options.unattended:
-if not options.forwarders and not options.no_forwarders:
-parser.error("You must specify at least one --forwarder option or --no-forwarders option")
+if (not options.forwarders
+and not options.no_forwarders
+and not options.auto_forwarders):
+parser.error("You must specify at least one option: "
+"--forwarder or --no-forwarders or --auto-forwarders")
 
 if options.kasp_db_file and not ipautil.file_exists(options.kasp_db_file):
 parser.error("File %s does not exist" % options.kasp_db_file)
-- 
2.5.5

-- 
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] [TESTS][PATCH 0012] Provide cleanup for host certificate

2016-05-03 Thread Lenka Doudova



On 05/03/2016 12:15 PM, Martin Basti wrote:



On 03.05.2016 11:18, Lenka Doudova wrote:



On 05/03/2016 10:33 AM, Martin Basti wrote:
Hello I'm quite confused what is happening in that code, can you 
explain it more to me? I see duplicated code there.

Sorry, that was just an unnecessary leftover. Fixed patch attached.
The code is expected to remove any certificates that were added to 
the local host but not to try to remove the host itself.


Lenka



Martin^2


Looks better, please follow proper naming of patches according how to 
format patch guide.
I propose following (Patch attached) changes. It looks weird to me to 
return self object


Martin


Ok, fixed patch attached.

Thanks,
Lenka
From 46a8fdc91918bfbb2bc9f231581fcb767bc635f5 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Mon, 2 May 2016 14:04:24 +0200
Subject: [PATCH] Test fix: Cleanup for host certificate

This fix provides means to remove certificates from host that were added during tests, but not removed.

Ticket: https://fedorahosted.org/freeipa/ticket/5839
---
 ipatests/test_xmlrpc/test_host_plugin.py|  3 ++-
 ipatests/test_xmlrpc/tracker/host_plugin.py | 14 ++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py
index 47f05a403ddb519f201b11251c2acb71faa9133b..85fec423ed530727a20b52534f7568f24e918efa 100644
--- a/ipatests/test_xmlrpc/test_host_plugin.py
+++ b/ipatests/test_xmlrpc/test_host_plugin.py
@@ -130,8 +130,9 @@ def this_host(request):
 """Fixture for the current master"""
 tracker = HostTracker(name=api.env.host.partition('.')[0],
   fqdn=api.env.host)
-# This host is not created/deleted, so don't call make_fixture
 tracker.exists = True
+# Finalizer ensures that any certificates added to this_host are removed
+tracker.add_finalizer_certcleanup(request)
 return tracker
 
 
diff --git a/ipatests/test_xmlrpc/tracker/host_plugin.py b/ipatests/test_xmlrpc/tracker/host_plugin.py
index bf199f4f50820fe27384eea4897b73bd02391c56..5e7bd55302ccac942fe740a5e3e15ad828dcb9d1 100644
--- a/ipatests/test_xmlrpc/tracker/host_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/host_plugin.py
@@ -10,6 +10,7 @@ from ipatests.test_xmlrpc.tracker.base import Tracker
 from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.util import assert_deepequal
+from ipalib import errors
 
 
 class HostTracker(Tracker):
@@ -152,3 +153,16 @@ class HostTracker(Tracker):
 summary=u'Modified host "%s"' % self.fqdn,
 result=self.filter_attrs(self.update_keys | set(extra_keys))
 ), result)
+
+def add_finalizer_certcleanup(self, request):
+""" Fixture to cleanup certificate from local host """
+cleanup_command = self.make_update_command(
+updates={'usercertificate':''})
+
+def cleanup():
+try:
+cleanup_command()
+except errors.EmptyModlist:
+pass
+
+request.addfinalizer(cleanup)
-- 
2.5.5

-- 
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] Improving bug reporting

2016-05-03 Thread Martin Basti



On 03.05.2016 13:45, David Kupka wrote:

Hello everyone!

I often miss proper reproducer and other important info in trac 
tickets. Asking for the missing info or guessing and trying is as 
ineffective as it sounds and costs us a lot of time and effort. I 
believe we can improve that.


We have guidelines for reporting a bug [1] but it obviously isn't 
enough. I propose to prefill track ticket's description with following 
(or similar) template and be strict on refusing (i.e. closing as 
invalid) tickets that are incomplete.


Any thoughts, suggestions, agreement or disagreement?

[1] http://www.freeipa.org/page/Troubleshooting#Reporting_bugs

--8<- trac-ticket-template-proposal --->8--
Related SW versions:
On server:
{{{
$ rpm -q freeipa-server pki-base 389-ds-base bind samba krb5-server 
certmonger

}}}
On client:
{{{
$ rpm -q freeipa-client krb5-workstation certmonger
}}}

Reproducible:
How often the issue occurs or what special condition is required to be 
met.


Examples:
Always / Happened X times of Y tries / Only at noon 29th February when 
it's also Thursday / Only on Raspberry Pi


Steps to reproduce:
Precise description of all related steps you have done. List of 
commands to run is the best form.


Example:
{{{
# ipa-server-install -a Secret123 -p Secret123 -r EXAMPLE.TEST -U
# echo Secret123 | kinit admin
}}}


Actual result:
Description of behavior you have observed (error, unexpected warning, 
...).


Example:
{{{
kinit: Client 'ad...@example.test' not found in Kerberos database 
while getting initial credentials

}}}

Expected result:
Description of behavior you have expected.

Example:
TGT for admin user is acquired.
--8<--->8--


+1

I suggest to mention also attach logs, for inspiration we may look at 
bind-dyndb-ldap trac.


Martin^2

--
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] Improving bug reporting

2016-05-03 Thread David Kupka

Hello everyone!

I often miss proper reproducer and other important info in trac tickets. 
Asking for the missing info or guessing and trying is as ineffective as 
it sounds and costs us a lot of time and effort. I believe we can 
improve that.


We have guidelines for reporting a bug [1] but it obviously isn't 
enough. I propose to prefill track ticket's description with following 
(or similar) template and be strict on refusing (i.e. closing as 
invalid) tickets that are incomplete.


Any thoughts, suggestions, agreement or disagreement?

[1] http://www.freeipa.org/page/Troubleshooting#Reporting_bugs

--8<- trac-ticket-template-proposal --->8--
Related SW versions:
On server:
{{{
$ rpm -q freeipa-server pki-base 389-ds-base bind samba krb5-server 
certmonger

}}}
On client:
{{{
$ rpm -q freeipa-client krb5-workstation certmonger
}}}

Reproducible:
How often the issue occurs or what special condition is required to be met.

Examples:
Always / Happened X times of Y tries / Only at noon 29th February when 
it's also Thursday / Only on Raspberry Pi


Steps to reproduce:
Precise description of all related steps you have done. List of commands 
to run is the best form.


Example:
{{{
# ipa-server-install -a Secret123 -p Secret123 -r EXAMPLE.TEST -U
# echo Secret123 | kinit admin
}}}


Actual result:
Description of behavior you have observed (error, unexpected warning, ...).

Example:
{{{
kinit: Client 'ad...@example.test' not found in Kerberos database while 
getting initial credentials

}}}

Expected result:
Description of behavior you have expected.

Example:
TGT for admin user is acquired.
--8<--->8--

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


Re: [Freeipa-devel] [TESTS][PATCH 0012] Provide cleanup for host certificate

2016-05-03 Thread Martin Basti



On 03.05.2016 11:18, Lenka Doudova wrote:



On 05/03/2016 10:33 AM, Martin Basti wrote:
Hello I'm quite confused what is happening in that code, can you 
explain it more to me? I see duplicated code there.

Sorry, that was just an unnecessary leftover. Fixed patch attached.
The code is expected to remove any certificates that were added to the 
local host but not to try to remove the host itself.


Lenka



Martin^2


Looks better, please follow proper naming of patches according how to 
format patch guide.
I propose following (Patch attached) changes. It looks weird to me to 
return self object


Martin

From 35136b4dedcd1d5d4c32218f8f364e4de90ee5a3 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 3 May 2016 12:14:50 +0200
Subject: [PATCH] proposed changes

---
 ipatests/test_xmlrpc/test_host_plugin.py| 3 ++-
 ipatests/test_xmlrpc/tracker/host_plugin.py | 4 +---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py
index e29a83842398a2d2935e04a5a16eac504bc5d53d..be707d3567d9100254dee1706aa9921b6c2866e9 100644
--- a/ipatests/test_xmlrpc/test_host_plugin.py
+++ b/ipatests/test_xmlrpc/test_host_plugin.py
@@ -132,7 +132,8 @@ def this_host(request):
   fqdn=api.env.host)
 tracker.exists = True
 # Fixture ensures that any certificates added to this_host are removed
-return tracker.make_fixture_certcleanup(request)
+tracker.add_finalizer_certcleanup(request)
+return tracker
 
 
 @pytest.fixture(scope='class')
diff --git a/ipatests/test_xmlrpc/tracker/host_plugin.py b/ipatests/test_xmlrpc/tracker/host_plugin.py
index 6f268de5b85c92320fee62df51beeb7c896d0742..d8b59b98907dff955f415b6edb0ce34d74e54f19 100644
--- a/ipatests/test_xmlrpc/tracker/host_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/host_plugin.py
@@ -157,7 +157,7 @@ class HostTracker(Tracker):
 result=self.filter_attrs(self.update_keys | set(extra_keys))
 ), result)
 
-def make_fixture_certcleanup(self, request):
+def add_finalizer_certcleanup(self, request):
 """ Fixture to cleanup certificate from local host """
 cleanup_command = self.make_update_command(
 updates={'usercertificate':''})
@@ -169,5 +169,3 @@ class HostTracker(Tracker):
 pass
 
 request.addfinalizer(cleanup)
-
-return self
-- 
2.5.5

-- 
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] [TESTS][PATCH 0012] Provide cleanup for host certificate

2016-05-03 Thread Lenka Doudova

Hi,

attached patch provides solution for 
https://fedorahosted.org/freeipa/ticket/5839 by removing all 
certificates added to local host during tests.


Lenka
From 031adf1f50308b70e87c93a7a853f04eae593bf0 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Mon, 2 May 2016 14:04:24 +0200
Subject: [PATCH] Test fix: Cleanup for host certificate

This fix provides means to remove certificates from host that were added during tests, but not removed.

Ticket: https://fedorahosted.org/freeipa/ticket/5839
---
 ipatests/test_xmlrpc/test_host_plugin.py|  4 ++--
 ipatests/test_xmlrpc/tracker/host_plugin.py | 20 
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py
index 47f05a403ddb519f201b11251c2acb71faa9133b..b62ce32a2206b489bdc9cdf32851f6101b004218 100644
--- a/ipatests/test_xmlrpc/test_host_plugin.py
+++ b/ipatests/test_xmlrpc/test_host_plugin.py
@@ -130,9 +130,9 @@ def this_host(request):
 """Fixture for the current master"""
 tracker = HostTracker(name=api.env.host.partition('.')[0],
   fqdn=api.env.host)
-# This host is not created/deleted, so don't call make_fixture
 tracker.exists = True
-return tracker
+# Fixture ensures that any certificates added to this_host are removed
+return tracker.make_fixture_certcleanup(request)
 
 
 @pytest.fixture(scope='class')
diff --git a/ipatests/test_xmlrpc/tracker/host_plugin.py b/ipatests/test_xmlrpc/tracker/host_plugin.py
index bf199f4f50820fe27384eea4897b73bd02391c56..a7f0445fe4119c525d481eb8eab7e5cea36706c9 100644
--- a/ipatests/test_xmlrpc/tracker/host_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/host_plugin.py
@@ -10,6 +10,7 @@ from ipatests.test_xmlrpc.tracker.base import Tracker
 from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.util import assert_deepequal
+from ipalib import errors
 
 
 class HostTracker(Tracker):
@@ -152,3 +153,22 @@ class HostTracker(Tracker):
 summary=u'Modified host "%s"' % self.fqdn,
 result=self.filter_attrs(self.update_keys | set(extra_keys))
 ), result)
+
+def make_fixture_certcleanup(self, request):
+""" Fixture to cleanup certificate from local host """
+cleanup_command = self.make_update_command(
+updates={'usercertificate':''})
+try:
+cleanup_command()
+except errors.EmptyModlist:
+pass
+
+def cleanup():
+try:
+cleanup_command()
+except errors.EmptyModlist:
+pass
+
+request.addfinalizer(cleanup)
+
+return self
-- 
2.5.5

-- 
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] 0053..0054 Configure lightweight CA key replication

2016-05-03 Thread Fraser Tweedale
On Tue, Apr 26, 2016 at 10:02:45AM +0200, Jan Cholasta wrote:
> On 21.4.2016 05:30, Fraser Tweedale wrote:
> >On Thu, Apr 14, 2016 at 04:39:37PM +1000, Fraser Tweedale wrote:
> >>Hi all,
> >>
> >>The attached patches configure lightweight CA key replication on IPA
> >>CAs, on upgrade and installation.
> >>
> >>Patches 0051..0052 from my other mail are also needed for the system
> >>to work, but this patchset does not depend on them and can be
> >>reviewed independently.
> >>
> >>There is also no hard dependency on the (unreleased) Dogtag 10.3.0b1
> >>- it just puts the necessary principals/keys/configuration in place.
> >>
> >>Cheers,
> >>Fraser
> >>
> >New patches attached;  0054-2 changes the service name from
> >'dogtag-ipa-custodia' to just 'dogtag', and adds an ACI to allow the
> >principal to search server Custodia keys.
> 
Honza, thanks for review.  Comments inline.

> Patch 53:
> 
> I'm not sure about this approach - the cn of custodia keys in LDAP is a
> free-form string, I would not tie it to service names, but rather try to
> keep it short.
> 
> In the key replication section of the design page, you mention "ca/$NAME", I
> think this is a good template for the cn and that we should stick to it.
> 
This scheme (or something like it, *without* '/' as the separator)
is needed to satisfy the ACI that allows host principals to manage
Custodia keys:

add:aci: (target = "ldap:///cn=*/($$dn),cn=custodia,cn=ipa,cn=etc,$SUFFIX")
(version 3.0; acl "IPA server hosts can create own Custodia secrets";
  allow(add) groupdn = 
"ldap:///cn=ipaservers,cn=hostgroups,cn=accounts,$SUFFIX;
 and userdn = 
"ldap:///fqdn=($$dn),cn=computers,cn=accounts,$SUFFIX";)

The CN must contain the hostname, and we must also disambiguate on
key type.  The current scheme is:

{sig,enc}~dogtag/
e.g.
enc~dogtag/f23-2.ipa.local

The first separator cannot be '/' because the '*' wildcard in the
ACI is not greedy - the captured text would include the servicename
and fail to match any userdn.

If you do not like '~' feel free to suggest a different symbol :)
The alternative is to add more ACIs.

> 
> Patch 54:
> 
> 1) This belongs to CAInstance.configure_instance():
> 
> +CA = cainstance.CAInstance(
> +api.env.realm, certs.NSS_DIR, host_name=api.env.host)
> +CA.setup_lightweight_ca_key_retrieval()
> 
See comments for (5).

> 
> 2) Any ACI changes should be in a separate patch. (What happened to patch
> 52?)
> 
Patch 52 added an ACI that allowed any authenticated user to see the
keys.  Simo wanted it limited it to the Dogtag principal so I
rescinded patch 52 and added the ACI in the same patch where the
principal was added.

Separate patch is no problem; I will resurrect number 52.

> 
> 3) This is not a platform constant, just a constant:
> 
> +PKI_GSSAPI_SERVICE_NAME = 'dogtag'
> 
Thanks, will put it in `ipalib.constants'.

> 
> 4) CAInstance.setup_lightweight_ca_key_retrieval() does too much. Please
> split it into a "setup keytab" and "setup custodia" parts.
> 
Will extract methods for next patchset.

> 
> 5) This also belongs to CAInstance.configure_instance():
> 
> +if setup_ca:
> +# CA was configured before Kerberos;
> +# add Custodia client princ and keys now
> +ca_instance.setup_lightweight_ca_key_retrieval()
> 
> In order for that to work, you need to move the ca.install_step_1() after
> krb.create_instance(), but that should be OK, since KrbInstance does not
> talk to the CA.
> 
`setup_lightweight_ca_key_retrieval' calls `kadmin_addprinc', which
fails if called before `krb.create_instance' due to missing
krb5.conf::

2016-05-03T06:29:23Z DEBUG args=kadmin.local -q addprinc -randkey 
dogtag/f23-2.ipa.local@IPA.LOCAL -x ipa-setup-override-restrictions
2016-05-03T06:29:23Z DEBUG Process finished, return code=1
2016-05-03T06:29:23Z DEBUG stdout=
2016-05-03T06:29:23Z DEBUG stderr=kadmin.local: unable to get default realm

Moving `ca.install_step_1()' to after `krb.create_instance()' does
not help, because `CAInstance.configure_instance' is called from
`ca.install_step_0()'.

However, calling `CAInstance.setup_lightweight_ca_key_retrieval()'
*directly* from `ca.install_step_1' would probably work.  Are you
happy with putting it there, instead of `configure_instance()'?

Cheers,
Fraser

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