[Freeipa-devel] [PATH 0053] Inconsistency between ipasearchrecordslimit and --sizelimit

2015-07-29 Thread Gabe Alford
Hello,

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

Thanks,

Gabe
From cba4b0d90f65be7734a977cb84f96f378e1c91d0 Mon Sep 17 00:00:00 2001
From: Gabe redhatri...@gmail.com
Date: Wed, 29 Jul 2015 09:04:32 -0600
Subject: [PATCH] Standardize minvalue for ipasearchrecordlimit and sizelimit
 for unlimited option

https://fedorahosted.org/freeipa/ticket/4023
---
 API.txt| 164 ++---
 VERSION|   4 +-
 ipalib/plugins/baseldap.py |   8 +--
 3 files changed, 88 insertions(+), 88 deletions(-)

diff --git a/API.txt b/API.txt
index 6ab30ddab41715fdbccb4f37aa1852621bca62b4..e588fe538251e84e26358abfb507dd7fce8c597f 100644
--- a/API.txt
+++ b/API.txt
@@ -272,8 +272,8 @@ option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui
 option: IA5Str('automountinformation', attribute=True, autofill=False, cli_name='info', multivalue=False, query=True, required=False)
 option: IA5Str('automountkey', attribute=True, autofill=False, cli_name='key', multivalue=False, query=True, required=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
-option: Int('sizelimit?', autofill=False, minvalue=0)
-option: Int('timelimit?', autofill=False, minvalue=0)
+option: Int('sizelimit?', autofill=False, minvalue=-1)
+option: Int('timelimit?', autofill=False, minvalue=-1)
 option: Str('version?', exclude='webui')
 output: Output('count', type 'int', None)
 output: ListOfEntries('result', (type 'list', type 'tuple'), Gettext('A list of LDAP entries', domain='ipa', localedir=None))
@@ -336,8 +336,8 @@ option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui
 option: Str('cn', attribute=True, autofill=False, cli_name='location', multivalue=False, primary_key=True, query=True, required=False)
 option: Flag('pkey_only?', autofill=True, default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
-option: Int('sizelimit?', autofill=False, minvalue=0)
-option: Int('timelimit?', autofill=False, minvalue=0)
+option: Int('sizelimit?', autofill=False, minvalue=-1)
+option: Int('timelimit?', autofill=False, minvalue=-1)
 option: Str('version?', exclude='webui')
 output: Output('count', type 'int', None)
 output: ListOfEntries('result', (type 'list', type 'tuple'), Gettext('A list of LDAP entries', domain='ipa', localedir=None))
@@ -411,8 +411,8 @@ option: IA5Str('automountmapname', attribute=True, autofill=False, cli_name='map
 option: Str('description', attribute=True, autofill=False, cli_name='desc', multivalue=False, query=True, required=False)
 option: Flag('pkey_only?', autofill=True, default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
-option: Int('sizelimit?', autofill=False, minvalue=0)
-option: Int('timelimit?', autofill=False, minvalue=0)
+option: Int('sizelimit?', autofill=False, minvalue=-1)
+option: Int('timelimit?', autofill=False, minvalue=-1)
 option: Str('version?', exclude='webui')
 output: Output('count', type 'int', None)
 output: ListOfEntries('result', (type 'list', type 'tuple'), Gettext('A list of LDAP entries', domain='ipa', localedir=None))
@@ -555,8 +555,8 @@ option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('pkey_only?', autofill=True, default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: StrEnum('servicecategory', attribute=True, autofill=False, cli_name='servicecat', multivalue=False, query=True, required=False, values=(u'all',))
-option: Int('sizelimit?', autofill=False, minvalue=0)
-option: Int('timelimit?', autofill=False, minvalue=0)
+option: Int('sizelimit?', autofill=False, minvalue=-1)
+option: Int('timelimit?', autofill=False, minvalue=-1)
 option: StrEnum('usercategory', attribute=True, autofill=False, cli_name='usercat', multivalue=False, query=True, required=False, values=(u'all',))
 option: Str('version?', exclude='webui')
 output: Output('count', type 'int', None)
@@ -711,8 +711,8 @@ option: Str('description', attribute=True, autofill=False, cli_name='desc', mult
 option: Bool('ipacertprofilestoreissued', attribute=True, autofill=False, cli_name='store', default=True, multivalue=False, query=True, required=False)
 option: Flag('pkey_only?', autofill=True, default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
-option: Int('sizelimit?', autofill=False, minvalue=0)
-option: Int('timelimit?', autofill=False, minvalue=0)
+option: Int('sizelimit?', autofill=False, minvalue=-1)
+option: Int('timelimit?', autofill=False, minvalue=-1)
 option: Str('version?', exclude='webui')
 output: Output('count', type 'int', None)
 output: ListOfEntries('result', (type 'list', type 'tuple'), Gettext('A list of LDAP entries', domain='ipa', localedir=None))
@@ -831,8 +831,8 @@ option: Int('cospriority', attribute=True, autofill=False, 

Re: [Freeipa-devel] [PATCH 0051] IPA server and replica installers can accept options from config file

2015-07-29 Thread Petr Vobornik

On 07/29/2015 05:13 PM, Martin Babinsky wrote:

On 07/29/2015 01:25 PM, Jan Cholasta wrote:

Dne 29.7.2015 v 12:20 Martin Babinsky napsal(a):

Initial attempt to implement
https://fedorahosted.org/freeipa/ticket/4517

Some points to discuss:

1.) name of the config entries: currently the option names are derived
from CLI options but have underscores in them instead of dashes. Maybe
keeping the CLI option names also for config entries will make it easier
for the user to transfer their CLI options from scripts to config files.


NACK. There is no point in generating config names from CLI names, which
are generated from knob names - use knob names directly.


The problem is that in some cases the  cli_name does not map directly to
knob name, leading in different naming of CLI options and config
entries, confusion and mayhem.

These are some offenders from `ipaserver/install/server.py`:
http://fpaste.org/249424/18226114/

On the other hand, this can be an incentive to finally put an end to
inconsistent option/knob naming across server/replica/etc. installers.


If the names are different than cli names, then they should be made 
discoverable somehow or be documented.




2.) Config sections: there is currently only one valid section named
'[global]' in accordance with the format of 'default.conf'. Should we
have separate sections equivalent to option groups in CLI (e.g. [basic],
[certificate system], [dns])?


No, because they would have to be maintained forever. For example, some
options are in wrong sections and we wouldn't be able to move them.


I'm also more inclined to a single section, at least for now since we
are pressed for time with this RFE.

That's not to say that we should ditch Alexander's idea about separate
sections with overrides for different hosts. We should consider it as a
future enhancement to this feature once the basic plumbing is in place.


3.) Handling of unattended mode when specifying a config file:
Currently there is no connection between --config-file and unattended
mode. So when you run ipa-server-install using config file, you still
get asked for missing stuff. Should '--config-file' automatically imply
'--unattended'?


The behavior should be the same as if you specified the options on the
command line. So no, --config-file should not imply --unattended.


That sound reasonable. the code behaves this way already so no changes
here.



There are probably other issues to discuss. Feel free to write
email/ping me on IRC.



(I haven't looked at the patch yet.)


Please take a look at it ASAP. I am on PTO tomorrow and on Friday, but I
will find time to work at it in the evening if you send me you comments.




--
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] [PATCH 0294] ULC: fix stageuser-add --from-delete command

2015-07-29 Thread Martin Basti

On 28/07/15 13:22, David Kupka wrote:

On 23/07/15 13:46, Martin Basti wrote:

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

Patch attached.

This patch fixes only first part of problem -- the traceback.

Removing promt for name and surname requires too big hacks in internal
API, and I'm not sure if we will be able to do that.
IMO this should be separate command, I will open a discussion.





Works for me, ACK.
It would be better to leave the ticket open until the issue is fully 
resolved.



Pushed to:
master: cea52ce186d9341f126ef6a9ac5f0287c4f16ada
ipa-4-2: 10e43f883d361ee1c376e1a1e06884cd9f8415ca


--
Martin Basti

--
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 016] Require Dogtag PKI = 10.2.6

2015-07-29 Thread Martin Basti

On 29/07/15 15:56, Martin Basti wrote:

On 23/07/15 12:26, Christian Heimes wrote:

Dogtag 10.2.6 comes with two fixes for cloning from 9.x to 10.x
instances:

   https://fedorahosted.org/pki/ticket/1495
   https://fedorahosted.org/pki/ticket/1488

https://fedorahosted.org/freeipa/ticket/5140
https://fedorahosted.org/freeipa/ticket/5129



ACK

--
Martin Basti



Pushed to:
master: 4e18a62dd5adeb4bcb63aafc4bbe50d7a5c71b9c
ipa-4-2: b01dc89967c73076c268e6bc3f1d604c3c04b221


--
Martin Basti

-- 
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 0002] Port from python-krbV to python-gssapi

2015-07-29 Thread Petr Viktorin
On 07/29/2015 11:56 AM, Christian Heimes wrote:
 On 2015-07-29 10:09, Michael Šimáček wrote:
 GSSAPI doesn't provide any method (that I'm aware of) to get default
 ccache name. In most cases this is not needed as we can simply not pass
 any name and it will use the default. The ldap plugin had to be adjusted
 for this - the connect method now takes new use_gssapi argument, which
 can turn on gssapi support without the need to supply explicit ccache
 name. The only place where the ccache name is really needed is the test
 server, where I use system klist command to obtain it.
 
 You can use ctypes or cffi for the task, too. It's much faster and more
 convenient. 

I think /usr/bin/klist should be just fine for the test server.


-- 
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] [PATCH 0051] IPA server and replica installers can accept options from config file

2015-07-29 Thread Martin Babinsky

On 07/29/2015 01:25 PM, Jan Cholasta wrote:

Dne 29.7.2015 v 12:20 Martin Babinsky napsal(a):

Initial attempt to implement
https://fedorahosted.org/freeipa/ticket/4517

Some points to discuss:

1.) name of the config entries: currently the option names are derived
from CLI options but have underscores in them instead of dashes. Maybe
keeping the CLI option names also for config entries will make it easier
for the user to transfer their CLI options from scripts to config files.


NACK. There is no point in generating config names from CLI names, which
are generated from knob names - use knob names directly.

The problem is that in some cases the  cli_name does not map directly to 
knob name, leading in different naming of CLI options and config 
entries, confusion and mayhem.


These are some offenders from `ipaserver/install/server.py`: 
http://fpaste.org/249424/18226114/


On the other hand, this can be an incentive to finally put an end to 
inconsistent option/knob naming across server/replica/etc. installers.


2.) Config sections: there is currently only one valid section named
'[global]' in accordance with the format of 'default.conf'. Should we
have separate sections equivalent to option groups in CLI (e.g. [basic],
[certificate system], [dns])?


No, because they would have to be maintained forever. For example, some
options are in wrong sections and we wouldn't be able to move them.

I'm also more inclined to a single section, at least for now since we 
are pressed for time with this RFE.


That's not to say that we should ditch Alexander's idea about separate 
sections with overrides for different hosts. We should consider it as a 
future enhancement to this feature once the basic plumbing is in place.


3.) Handling of unattended mode when specifying a config file:
Currently there is no connection between --config-file and unattended
mode. So when you run ipa-server-install using config file, you still
get asked for missing stuff. Should '--config-file' automatically imply
'--unattended'?


The behavior should be the same as if you specified the options on the
command line. So no, --config-file should not imply --unattended.


That sound reasonable. the code behaves this way already so no changes here.



There are probably other issues to discuss. Feel free to write
email/ping me on IRC.



(I haven't looked at the patch yet.)

Please take a look at it ASAP. I am on PTO tomorrow and on Friday, but I 
will find time to work at it in the evening if you send me you comments.


--
Martin^3 Babinsky

--
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 0002] Port from python-krbV to python-gssapi

2015-07-29 Thread Robbie Harwood
Michael Šimáček msima...@redhat.com writes:

 GSSAPI doesn't provide any method (that I'm aware of) to get default
 ccache name. In most cases this is not needed as we can simply not
 pass any name and it will use the default. The ldap plugin had to be
 adjusted for this - the connect method now takes new use_gssapi
 argument, which can turn on gssapi support without the need to supply
 explicit ccache name. The only place where the ccache name is really
 needed is the test server, where I use system klist command to obtain
 it.

This is sub-optimal, but not a huge deal if it's only in the test
suite.

 It's also not possible to directly get default realm name, what I do
 is importing nonexistent name, cannonicalizing it and extracting the
 realm from it. Which should work but is ugly. It would be better if we
 could modify the places that use it to not need it at all, but it's
 mostly used in ldap code and I don't understand that part of FreeIPA.
 Alternative would be parsing /etc/krb.conf.

Please try not to do this.  DEFINITELY do not parse krb.conf.
Unfortunately, I do not know enough about the LDAP code to know why this
is needed or to suggest an alternate solution.

 Sorry for long patch, but I'm afraid it cannot be reasonably split.

This is indeed really long and difficult to work through.  I have
probably missed some things; apologies if they come through in a later
round.

 +try:
 +cred = kinit_keytab(principal, keytab_name, ccache_name)
 +# would raise exception if expired
 +cred.lifetime
 +except gssapi.exceptions.ExpiredCredentialsError:
 +# delete stale ccache and try again
 +os.unlink(ccache_name)
 +cred = kinit_keytab(principal, keytab_name, ccache_name)

See next comment.

 -# The keytab may have stale key material (from older trust-add run)
 -if not os.path.isfile(oneway_ccache_name):
 -oneway_ccache = kinit_keytab(oneway_principal, oneway_keytab_name, 
 oneway_ccache_name)
 -except krbV.Krb5Error as e:
 +try:
 +# The keytab may have stale key material (from older trust-add run)
 +cred = kinit_keytab(oneway_principal, oneway_keytab_name, 
 oneway_ccache_name)
 +# would raise exception if expired
 +cred.lifetime
 +except gssapi.exceptions.ExpiredCredentialsError:
 +cred = kinit_keytab(oneway_principal, oneway_keytab_name, 
 oneway_ccache_name)
 +except gssapi.exceptions.GSSError:
  # If there was failure on using keytab, assume it is stale and retrieve 
 again
  retrieve_keytab(api, ccache_name, oneway_keytab_name, oneway_principal)

In general, it's bad practice to catch *all* possible GSS errors unless
you intend to display their status and/or abort/raise.  If there's a
specific state you want to cope with here, catch the exception related
to it, not all of them.  Up above is a place where I think this is done
right.

 -ctx = krbV.default_context()
 -ccache = ctx.default_ccache()
 -principal = ccache.principal()
 -except krbV.Krb5Error, e:
 +principal = krb_utils.get_principal()
 +except errors.CCacheError:
  sys.exit(Must have Kerberos credentials to setup AD trusts on 
 server)

Based on how GSSAPI error messages are being packed into CCache errors
(the name of which is itself unfortunate...), it would be nice to give
some hint of the problem here if it were GSSAPI; otherwise, to my eye,
it looks like the GSSAPI status is being dropped.

 +def get_credentials(name=None, ccache_name=None):
  '''
 -Kerberos stores a TGT (Ticket Granting Ticket) and the service
 -tickets bound to it in a ccache (credentials cache). ccaches are
 -bound to a Kerberos user principal. This class opens a Kerberos
 -ccache and allows one to manipulate it. Most useful is the
 -extraction of ticket entries (cred's) in the ccache and the
 -ability to examine their attributes.
 +Obtains GSSAPI credentials with given principal name from ccache. When no
 +principal name specified, it retrieves the default one for given
 +credentials cache.
 +
 +:parameters:
 +  name
 +gssapi.Name object specifying principal or None for the default
 +  ccache_name
 +string specifying Kerberos credentials cache name or None for the
 +default
 +:returns:
 +  gssapi.Credentials object
 +
 +store = None
 +if ccache_name:
 +store = {'ccache': ccache_name}
 +try:
 +return gssapi.Credentials(usage='initiate', name=name, store=store)
 +except gssapi.exceptions.GSSError as e:
 +if e.min_code == KRB5_FCC_NOFILE:
 +raise ValueError('%s, ccache=%s' % (e.message, ccache_name))
 +raise errors.CCacheError()

This is another case where it stands out that the specific error from
GSSAPI should probably be checked.

 +# FIXME this is a temporary workaround. We should find some nicer 
 solution
 +name = gssapi.Name('notempty', gssapi.NameType.user)
 +

Re: [Freeipa-devel] Wiki Access

2015-07-29 Thread Alexander Bokovoy

On Wed, 29 Jul 2015, Dan Mossor wrote:

On 07/29/2015 03:10 PM, Rob Crittenden wrote:

Dan Mossor wrote:

Greetings FreeIPA devs. I just wanted to express my sincere thank you
for all y'all do - FreeIPA is an awesome product that just keeps getting
better and better.

One area where y'all desparately need help with, however, is in
documentation. I recently had to rescue a co-worker from following a
document that was no longer applicable[0], and in researching how to fix
it I found your Wiki TODO list [1].

I would like to ask for editor access to the FreeIPA wiki so I can
contribute back some of the knowledge I've gleaned from the user list,
IRC channel, and copious amounts of research I've been doing in
deploying FreeIPA at my workplace.

Regards,
Dan danofsatx Mossor

[0]
http://www.freeipa.org/page/Active_Directory_trust_setup#On_IPA_server
[1] http://www.freeipa.org/page/Wiki_TODO


You should just need a FAS account,
https://admin.fedoraproject.org/accounts/user/new if you don't already
have one.

rob

I've got one - I just didn't see the login link on the bottom of the page.

Incidentally, this simply points out yet another area that needs 
correction in the wiki, as it states to email the freeipa-devel list 
to request access.


I'm logged in, and will get to work shortly.

Thanks in advance for doing the clean ups. Could you please make sure
you are adding at least a version information to any changes or
suggestions? One of issues with wiki pages we have is that they are
tending to cover 'latest' version at the time they were added but things
change over time and multiple edits blend together, sometimes making
more of a mess than needed. Adding a simple reference to what version we
are talking about could help others to not be in trouble next time.

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


[Freeipa-devel] Wiki Access

2015-07-29 Thread Dan Mossor
Greetings FreeIPA devs. I just wanted to express my sincere thank you 
for all y'all do - FreeIPA is an awesome product that just keeps getting 
better and better.


One area where y'all desparately need help with, however, is in 
documentation. I recently had to rescue a co-worker from following a 
document that was no longer applicable[0], and in researching how to fix 
it I found your Wiki TODO list [1].


I would like to ask for editor access to the FreeIPA wiki so I can 
contribute back some of the knowledge I've gleaned from the user list, 
IRC channel, and copious amounts of research I've been doing in 
deploying FreeIPA at my workplace.


Regards,
Dan danofsatx Mossor

[0] http://www.freeipa.org/page/Active_Directory_trust_setup#On_IPA_server
[1] http://www.freeipa.org/page/Wiki_TODO
--
Dan Mossor, RHCSA
Systems Engineer
Fedora Server WG | Fedora KDE WG | Fedora QA Team
Fedora Infrastructure Apprentice
FAS: dmossor IRC: danofsatx
San Antonio, Texas, USA

--
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] Wiki Access

2015-07-29 Thread Rob Crittenden

Dan Mossor wrote:

Greetings FreeIPA devs. I just wanted to express my sincere thank you
for all y'all do - FreeIPA is an awesome product that just keeps getting
better and better.

One area where y'all desparately need help with, however, is in
documentation. I recently had to rescue a co-worker from following a
document that was no longer applicable[0], and in researching how to fix
it I found your Wiki TODO list [1].

I would like to ask for editor access to the FreeIPA wiki so I can
contribute back some of the knowledge I've gleaned from the user list,
IRC channel, and copious amounts of research I've been doing in
deploying FreeIPA at my workplace.

Regards,
Dan danofsatx Mossor

[0] http://www.freeipa.org/page/Active_Directory_trust_setup#On_IPA_server
[1] http://www.freeipa.org/page/Wiki_TODO


You should just need a FAS account, 
https://admin.fedoraproject.org/accounts/user/new if you don't already 
have one.


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] Wiki Access

2015-07-29 Thread Dan Mossor

On 07/29/2015 03:10 PM, Rob Crittenden wrote:

Dan Mossor wrote:

Greetings FreeIPA devs. I just wanted to express my sincere thank you
for all y'all do - FreeIPA is an awesome product that just keeps getting
better and better.

One area where y'all desparately need help with, however, is in
documentation. I recently had to rescue a co-worker from following a
document that was no longer applicable[0], and in researching how to fix
it I found your Wiki TODO list [1].

I would like to ask for editor access to the FreeIPA wiki so I can
contribute back some of the knowledge I've gleaned from the user list,
IRC channel, and copious amounts of research I've been doing in
deploying FreeIPA at my workplace.

Regards,
Dan danofsatx Mossor

[0]
http://www.freeipa.org/page/Active_Directory_trust_setup#On_IPA_server
[1] http://www.freeipa.org/page/Wiki_TODO


You should just need a FAS account,
https://admin.fedoraproject.org/accounts/user/new if you don't already
have one.

rob

I've got one - I just didn't see the login link on the bottom of the page.

Incidentally, this simply points out yet another area that needs 
correction in the wiki, as it states to email the freeipa-devel list to 
request access.


I'm logged in, and will get to work shortly.

Dan

--
Dan Mossor, RHCSA
Systems Engineer
Fedora Server WG | Fedora KDE WG | Fedora QA Team
Fedora Infrastructure Apprentice
FAS: dmossor IRC: danofsatx
San Antonio, Texas, USA

--
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] 0029 Work around python-nss bug on unrecognised OIDs

2015-07-29 Thread Fraser Tweedale
The attached patch works around a bug in python-nss triggered by
unrecognised PKCS#10 request extensions.  It is needed for
https://fedorahosted.org/freeipa/ticket/4752 but can be reverted
once the python-nss bug is fixed.

Thanks,
Fraser
From b1846bd1130bb403334cdef0aaf994b45c66d4d7 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale ftwee...@redhat.com
Date: Fri, 24 Jul 2015 09:23:07 -0400
Subject: [PATCH] Work around python-nss bug on unrecognised OIDs

A bug in python-nss causes an error to be thrown when converting an
unrecognised OID to a string.  If cert-request receives a PKCS #10
CSR with an unknown extension, the error is thrown.

Work around this error by first checking if the OID is recognised
and, if it is not, using a different method to obtain its string
representation.

Once the python-nss bug is fixed, this workaround should be
reverted.  https://bugzilla.redhat.com/show_bug.cgi?id=1246729
---
 ipalib/pkcs10.py | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/ipalib/pkcs10.py b/ipalib/pkcs10.py
index 
6299dfea43b7a3f4104f0b0ec78c4f105d9daf62..64670835127e96f1d724c5f32ed7a939d37b7f16
 100644
--- a/ipalib/pkcs10.py
+++ b/ipalib/pkcs10.py
@@ -53,7 +53,20 @@ def get_extensions(csr, datatype=PEM):
 The return value is a tuple of strings
 
 request = load_certificate_request(csr, datatype)
-return tuple(nss.oid_dotted_decimal(ext.oid_tag)[4:]
+
+# Work around a bug in python-nss where nss.oid_dotted_decimal
+# errors on unrecognised OIDs
+#
+# https://bugzilla.redhat.com/show_bug.cgi?id=1246729
+#
+def get_prefixed_oid_str(ext):
+Returns a string like 'OID.1.2...'.
+if ext.oid_tag == 0:
+return repr(ext)
+else:
+return nss.oid_dotted_decimal(ext.oid)
+
+return tuple(get_prefixed_oid_str(ext)[4:]
  for ext in request.extensions)
 
 class _PrincipalName(univ.Sequence):
-- 
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] 0028 add --out option to user-show

2015-07-29 Thread Fraser Tweedale
On Thu, Jul 30, 2015 at 10:19:19AM +1000, Fraser Tweedale wrote:
 On Wed, Jul 29, 2015 at 03:48:47PM +0200, Jan Cholasta wrote:
  Dne 29.7.2015 v 15:46 Martin Basti napsal(a):
  On 29/07/15 15:41, Martin Basti wrote:
  On 25/07/15 03:40, Fraser Tweedale wrote:
  On Fri, Jul 24, 2015 at 05:53:56PM +0200, Tomas Babej wrote:
  
  On 07/24/2015 05:34 PM, Martin Basti wrote:
  On 24/07/15 16:52, Tomas Babej wrote:
  On 07/24/2015 03:40 PM, Fraser Tweedale wrote:
  The attached patch adds --out option to user-show for saving user's
  certificate(s) to file.
  
  Thanks,
  Fraser
  
  
  
  I hate to nitpick here, but is out really a descriptive option name
  here? I'd prefer something more explicit, like '--save-cert-to', or
  maybe even have this operation implemented as a separate command
  altogether.
  
  Tomas
  
  This keyword was already used with several commands. For consistency
  might be better to have it the same.
  
  True. I see this options is being used in the following commands:
  
- cert-show
- vault-retrieve
- host-show
- service-show
- user-show (proposed)
  
  While the first two seem to be an acceptable fit for an option called
  --out, as they mainly deal with cert/secret, using the '--out' for the
  latter three is a poor decision imho.
  
  I agree the consistency is important, I'm just not happy to see this
  spread further.
  
  Tomas
  Perhaps we should go with something like `--certout' instead, and
  support `--certout' in addition to `--out' in host-show and
  service-show, esentially deprecating `--out' for those commands.
  
  Cheers,
  Fraser
  Good idea, but we should do this for all commands, at the same time.
  IMO this is not for 4.2, you may file a ticket to deprecate --out
  option and replace it by --certout or something.
  
  The in option is named --certificate, so it should be --certificate-out.
  
  
  I will do review is nobody is against this patch :)
  Martin^2
  
  LGTM
  
  
  
  Is a ticket somewhere for this?
  
 No ticket; I just wanted it so I wrote the patch :)
 
 I'll file the ticket for future change to `--certificate-out'
 though.
 
Ticket: https://fedorahosted.org/freeipa/ticket/5166

-- 
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 0002] Port from python-krbV to python-gssapi

2015-07-29 Thread Michael Šimáček

Hi,

this is the first attempt to port FreeIPA from deprecated 
python3-incompatible python-krbV library to python-gssapi. The patch 
depends on python-kerberos-python-gssapi patch [1] to apply cleanly, 
but the overlap is small, so I think it can be at least partially 
reviewed without it.


Comments:
I removed Backend.krb and KRB5_CCache classes as they were wrappers 
around krbV classes. I added few utility functions to krb_utils module 
that perform part of its functionality (no need for classes, because 
gssapi acquire calls don't pass any context objects, they wouldn't have 
any state).


I merged the two different kinit_keytab functions.

GSSAPI doesn't provide any method (that I'm aware of) to get default 
ccache name. In most cases this is not needed as we can simply not pass 
any name and it will use the default. The ldap plugin had to be adjusted 
for this - the connect method now takes new use_gssapi argument, which 
can turn on gssapi support without the need to supply explicit ccache 
name. The only place where the ccache name is really needed is the test 
server, where I use system klist command to obtain it.


It's also not possible to directly get default realm name, what I do is 
importing nonexistent name, cannonicalizing it and extracting the realm 
from it. Which should work but is ugly. It would be better if we could 
modify the places that use it to not need it at all, but it's mostly 
used in ldap code and I don't understand that part of FreeIPA. 
Alternative would be parsing /etc/krb.conf.


Sorry for long patch, but I'm afraid it cannot be reasonably split.


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

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

--
Michael Simacek
From 4257e828cf733bc3c336efde6faadc5b91666909 Mon Sep 17 00:00:00 2001
From: Michael Simacek msima...@redhat.com
Date: Mon, 20 Jul 2015 16:04:07 +0200
Subject: [PATCH] Port from python-krbV to python-gssapi

python-krbV library is deprecated and doesn't work with python 3. Replacing all
it's usages with python-gssapi.

- Removed Backend.krb and KRB5_CCache classes
  They were wrappers around krbV classes that cannot really work without them
- Added few utility functions for querying GSSAPI credentials
  In krb_utils module. They provide replacements for KRB5_CCache.
- Merged two kinit_keytab functions
- Changed how ldap connection get default ccache
  Since default ccache name cannot be retireved through GSSAPI, connect method
  now takes use_gssapi boolean argument that tells it to use the default if
  none provided.
- Unified getting default realm
  But the implementation of getting it is done by a workaround (importing
  nonexistent name, canonicalizing it and extracting the realm from it),
  because GSSAPI doesn't provide any direct way to do this.
---
 BUILD.txt |   2 +-
 doc/examples/python-api.py|   4 +-
 freeipa.spec.in   |   7 +-
 install/oddjob/com.redhat.idm.trust-fetch-domains |  69 ++---
 install/tools/ipa-adtrust-install |  13 +-
 install/tools/ipa-csreplica-manage|   5 +-
 install/tools/ipa-replica-manage  |   9 +-
 ipa-client/ipa-client.spec.in |   2 +-
 ipa-client/ipa-install/ipa-client-automount   |   4 +-
 ipa-client/ipa-install/ipa-client-install |   8 +-
 ipalib/krb_utils.py   | 344 +-
 ipalib/plugins/kerberos.py| 125 
 ipalib/plugins/passwd.py  |   6 +-
 ipalib/plugins/vault.py   |   7 +-
 ipalib/rpc.py |   9 +-
 ipalib/util.py|  12 -
 ipapython/config.py   |   6 +-
 ipapython/ipautil.py  |  28 +-
 ipaserver/install/ipa_cacert_manage.py|   6 +-
 ipaserver/install/ipa_ldap_updater.py |   6 +-
 ipaserver/install/ipa_otptoken_import.py  |   6 +-
 ipaserver/install/ipa_winsync_migrate.py  |   7 +-
 ipaserver/install/ldapupdate.py   |  12 +-
 ipaserver/install/schemaupdate.py |   4 +-
 ipaserver/install/server/upgrade.py   |   5 +-
 ipaserver/plugins/join.py |  15 +-
 ipaserver/plugins/ldap2.py|  25 +-
 ipaserver/rpcserver.py|  25 +-
 ipatests/test_cmdline/cmdline.py  |   5 +-
 ipatests/test_cmdline/test_ipagetkeytab.py|  21 +-
 ipatests/test_xmlrpc/test_dns_plugin.py   |   3 +-
 ipatests/test_xmlrpc/test_netgroup_plugin.py  |   6 +-
 ipatests/test_xmlrpc/test_permission_plugin.py|   3 +-
 lite-server.py|  16 +-
 make-lint |   4 +-
 35 files changed, 218 insertions(+), 611 deletions(-)
 

Re: [Freeipa-devel] [PATCH 0286, 0290] Sysrestore: copy files instead of moving them to avoid SELinux issues

2015-07-29 Thread David Kupka

On 17/07/15 16:33, Martin Basti wrote:

On 17/07/15 13:57, Petr Vobornik wrote:

On 07/17/2015 01:46 PM, Petr Vobornik wrote:

On 07/17/2015 01:44 PM, Alexander Bokovoy wrote:

On Fri, 17 Jul 2015, Martin Basti wrote:

From b05f4a2e17ae00e5c20e5eb7bd046472f100e0ad Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Wed, 15 Jul 2015 16:20:59 +0200
Subject: [PATCH] sysrestore: copy files instead of moving them to
avoind
SELinux issues


ACK.



Pushed to:
master: 9f701283534745bf93b41a1886183e9ef1d06566
ipa-4-2: 92a73e8b2a5f26744b036a36de4b9956e8883f61


Does it really fix the whole ticket?

There is also in freeipa.spec.in %post client (i.e. upgrade):

cat /etc/krb5.conf  /etc/krb5.conf.ipanew
mv /etc/krb5.conf.ipanew /etc/krb5.conf
/sbin/restorecon /etc/krb5.conf

+ some others.

Between the mv and restorecon, SSSD tries to access the file and
raises AVC.

In this case we can freely use mv -z since target platforms are Fedora
and newest RHEL.


The new patch fixing specfile attached.




Works for me, ACK.

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


[Freeipa-devel] [PATCH 0051] IPA server and replica installers can accept options from config file

2015-07-29 Thread Martin Babinsky

Initial attempt to implement
https://fedorahosted.org/freeipa/ticket/4517

Some points to discuss:

1.) name of the config entries: currently the option names are derived 
from CLI options but have underscores in them instead of dashes. Maybe 
keeping the CLI option names also for config entries will make it easier 
for the user to transfer their CLI options from scripts to config files.


2.) Config sections: there is currently only one valid section named 
'[global]' in accordance with the format of 'default.conf'. Should we 
have separate sections equivalent to option groups in CLI (e.g. [basic], 
[certificate system], [dns])?


3.) Handling of unattended mode when specifying a config file:
Currently there is no connection between --config-file and unattended 
mode. So when you run ipa-server-install using config file, you still 
get asked for missing stuff. Should '--config-file' automatically imply 
'--unattended'?


There are probably other issues to discuss. Feel free to write 
email/ping me on IRC.


--
Martin^3 Babinsky
From 57685dfca56e5300d6c996ba6362c407b7b1a63b Mon Sep 17 00:00:00 2001
From: Martin Babinsky mbabi...@redhat.com
Date: Wed, 22 Jul 2015 13:55:26 +0200
Subject: [PATCH] IPA server and replica installers can accept options from
 config file

New option '--config-file' enables ipa-server-install and ipa-replica-install
to obtain parameters from supplied configuration file in INI format.

The file syntax is as follows:
* all options are listed in a single [global] section
* the option name can be derived from long CLI option name by replacing
  dashes with underscores
* boolean options are specified as a keyword without value: e.g. to enable
  installation of DNS component specify only 'setup_dns' in the config
  file.
  The absence of the boolean implies that the option is False
* to specify multivalued parameter, assign a list of entries on multiple
  lines to a single option like this:
   multi_valued_option = value1
   value2
   value3

Parameters specified explicitly through CLI options take precedence over the
values contained in config file.

In the case of unknown options present in the config file, the installer will
raise an error listing them.

https://fedorahosted.org/freeipa/ticket/4517
---
 ipapython/install/cli.py | 76 
 1 file changed, 76 insertions(+)

diff --git a/ipapython/install/cli.py b/ipapython/install/cli.py
index 1ba9a815c4c499dff0e7974f399f2de31eb932cd..614c0562c28362e79690783757d2a8995907bf6e 100644
--- a/ipapython/install/cli.py
+++ b/ipapython/install/cli.py
@@ -9,6 +9,7 @@ Command line support.
 import collections
 import optparse
 import signal
+from ConfigParser import ConfigParser
 
 from ipapython import admintool, ipa_log_manager
 from ipapython.ipautil import CheckedIPAddress, private_ccache
@@ -148,6 +149,17 @@ class ConfigureTool(admintool.AdminTool):
 for group, opt_group in groups.iteritems():
 parser.add_option_group(opt_group)
 
+alt_source_group = optparse.OptionGroup(
+parser, alternative configuration sources)
+alt_source_group.add_option(
+--config-file,
+dest='config_filename',
+default=None,
+metavar='FILE',
+help=get installer specific options from config file
+)
+parser.add_option_group(alt_source_group)
+
 super(ConfigureTool, cls).add_options(parser,
   debug_option=cls.debug_option)
 
@@ -274,6 +286,11 @@ class ConfigureTool(admintool.AdminTool):
 def run(self):
 kwargs = {}
 
+if self.options.config_filename is not None:
+values_from_config = self.parse_config_file(
+self.options.config_filename)
+kwargs.update(values_from_config)
+
 transformed_cls = self._transform(self.configurable_class)
 for owner_cls, name in transformed_cls.knobs():
 value = getattr(self.options, name, None)
@@ -311,6 +328,65 @@ class ConfigureTool(admintool.AdminTool):
 def __signal_handler(signum, frame):
 raise KeyboardInterrupt
 
+def parse_config_file(self, filename):
+config_parser = ConfigParser(allow_no_value=True)
+
+with open(filename, 'r') as f:
+config_parser.readfp(f)
+
+# use single section name for now
+section_name = 'global'
+config_entries = {x[0]: x[1] for x
+  in config_parser.items(section_name)}
+
+result = {}
+
+transformed_cls = self._transform(self.configurable_class)
+for owner_cls, name in transformed_cls.knobs():
+knob_cls = getattr(owner_cls, name)
+
+entry_name = (knob_cls.cli_name.replace('-', '_')
+  if knob_cls.cli_name is not None else name)
+try:
+config_entry 

Re: [Freeipa-devel] [PATCH 0051] IPA server and replica installers can accept options from config file

2015-07-29 Thread Petr Vobornik

On 07/29/2015 12:37 PM, Alexander Bokovoy wrote:

On Wed, 29 Jul 2015, Martin Babinsky wrote:

Initial attempt to implement
https://fedorahosted.org/freeipa/ticket/4517

Some points to discuss:

1.) name of the config entries: currently the option names are derived
from CLI options but have underscores in them instead of dashes. Maybe
keeping the CLI option names also for config entries will make it
easier for the user to transfer their CLI options from scripts to
config files.

I would prefer that too. Or you can simply allow both _ and -, this
should be relatively simple.


+1




2.) Config sections: there is currently only one valid section named
'[global]' in accordance with the format of 'default.conf'. Should we
have separate sections equivalent to option groups in CLI (e.g.
[basic], [certificate system], [dns])?

What about using a different approach -- allowing to specify which
section to process, defaulting to [global]. This would allow to have a
single config file for whole setup, if needed, and just vary which
section to use.


Interesting idea.



Maybe global section could always be processed and the rest could be
used to amend the configuration?

As an example,

[global]
setup_dns
realm = EXAMPLE.COM
domain = example.com
ds-password = SuperSecretPasswordHere
admin-password = EquallySecretPasswordHere
mkhomedir

[m1.example.com]
hostname=m1.example.com


[m2.example.com]
hostname=m2.example.com
setup_dns = False
mkhomedir = False


You can see I also kind of suggest to allow accepting True/Fals to
boolean options to allow _unsetting_ the effect of the default set in
the [global] section.


+1




3.) Handling of unattended mode when specifying a config file:
Currently there is no connection between --config-file and unattended
mode. So when you run ipa-server-install using config file, you still
get asked for missing stuff. Should '--config-file' automatically
imply '--unattended'?

Well, there is certain beauty of providing some arguments from the
config file and be asked for the rest. Unattended is more explicit in
the way of handling so I would still keep them separate.



+1

--
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] [PATCH 0002] Port from python-krbV to python-gssapi

2015-07-29 Thread Christian Heimes
On 2015-07-29 10:09, Michael Šimáček wrote:
 GSSAPI doesn't provide any method (that I'm aware of) to get default
 ccache name. In most cases this is not needed as we can simply not pass
 any name and it will use the default. The ldap plugin had to be adjusted
 for this - the connect method now takes new use_gssapi argument, which
 can turn on gssapi support without the need to supply explicit ccache
 name. The only place where the ccache name is really needed is the test
 server, where I use system klist command to obtain it.

You can use ctypes or cffi for the task, too. It's much faster and more
convenient. Here is a quick example how to use ctypes for the function
calls. kdcproxy uses similar code to parse /etc/krb5.conf.

 import ctypes
 LIBKRB5 = ctypes.CDLL('libkrb5.so.3')
 ctx = ctypes.c_void_p()
 ccache = ctypes.c_void_p()
 LIBKRB5.krb5_init_context(ctypes.byref(ctx))
0
 LIBKRB5.krb5_cc_default(ctx, ctypes.byref(ccache))
0
 LIBKRB5.krb5_cc_get_type.restype = ctypes.c_char_p
 LIBKRB5.krb5_cc_get_name.restype = ctypes.c_char_p
 LIBKRB5.krb5_cc_get_type(ctx, ccache)
'KEYRING'
 LIBKRB5.krb5_cc_get_name(ctx, ccache)
'persistent:1000:1000'
 LIBKRB5.krb5_cc_close(ctx, ccache)
 LIBKRB5.krb5_free_context(ctx)

If you like the approach I can write a more safe implementation with
proper error checking.

Christian



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] 906 webui: fix regressions failed auth messages

2015-07-29 Thread Martin Basti

On 28/07/15 14:05, Petr Vobornik wrote:

1. after logout, krb auth no longer shows session expired but correct
Authentication with Kerberos failed.

2. The password or username you entered is incorrect. is showed on
failed forms-based auth.

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



Works for me ACK

--
Martin Basti

-- 
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 0051] IPA server and replica installers can accept options from config file

2015-07-29 Thread Jan Cholasta

Dne 29.7.2015 v 12:50 Petr Vobornik napsal(a):

On 07/29/2015 12:37 PM, Alexander Bokovoy wrote:

On Wed, 29 Jul 2015, Martin Babinsky wrote:

2.) Config sections: there is currently only one valid section named
'[global]' in accordance with the format of 'default.conf'. Should we
have separate sections equivalent to option groups in CLI (e.g.
[basic], [certificate system], [dns])?

What about using a different approach -- allowing to specify which
section to process, defaulting to [global]. This would allow to have a
single config file for whole setup, if needed, and just vary which
section to use.


Interesting idea.


Maybe, but I don't think it's something that should be in the initial 
implementation, let's keep it simple for now.


--
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 0051] IPA server and replica installers can accept options from config file

2015-07-29 Thread Jan Cholasta

Dne 29.7.2015 v 12:20 Martin Babinsky napsal(a):

Initial attempt to implement
https://fedorahosted.org/freeipa/ticket/4517

Some points to discuss:

1.) name of the config entries: currently the option names are derived
from CLI options but have underscores in them instead of dashes. Maybe
keeping the CLI option names also for config entries will make it easier
for the user to transfer their CLI options from scripts to config files.


NACK. There is no point in generating config names from CLI names, which 
are generated from knob names - use knob names directly.




2.) Config sections: there is currently only one valid section named
'[global]' in accordance with the format of 'default.conf'. Should we
have separate sections equivalent to option groups in CLI (e.g. [basic],
[certificate system], [dns])?


No, because they would have to be maintained forever. For example, some 
options are in wrong sections and we wouldn't be able to move them.




3.) Handling of unattended mode when specifying a config file:
Currently there is no connection between --config-file and unattended
mode. So when you run ipa-server-install using config file, you still
get asked for missing stuff. Should '--config-file' automatically imply
'--unattended'?


The behavior should be the same as if you specified the options on the 
command line. So no, --config-file should not imply --unattended.




There are probably other issues to discuss. Feel free to write
email/ping me on IRC.



(I haven't looked at the patch yet.)

--
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 0051] IPA server and replica installers can accept options from config file

2015-07-29 Thread Alexander Bokovoy

On Wed, 29 Jul 2015, Martin Babinsky wrote:

Initial attempt to implement
https://fedorahosted.org/freeipa/ticket/4517

Some points to discuss:

1.) name of the config entries: currently the option names are derived 
from CLI options but have underscores in them instead of dashes. Maybe 
keeping the CLI option names also for config entries will make it 
easier for the user to transfer their CLI options from scripts to 
config files.

I would prefer that too. Or you can simply allow both _ and -, this
should be relatively simple.

2.) Config sections: there is currently only one valid section named 
'[global]' in accordance with the format of 'default.conf'. Should we 
have separate sections equivalent to option groups in CLI (e.g. 
[basic], [certificate system], [dns])?

What about using a different approach -- allowing to specify which
section to process, defaulting to [global]. This would allow to have a
single config file for whole setup, if needed, and just vary which
section to use.

Maybe global section could always be processed and the rest could be
used to amend the configuration?

As an example,

[global]
setup_dns
realm = EXAMPLE.COM
domain = example.com
ds-password = SuperSecretPasswordHere
admin-password = EquallySecretPasswordHere
mkhomedir

[m1.example.com]
hostname=m1.example.com


[m2.example.com]
hostname=m2.example.com
setup_dns = False
mkhomedir = False


You can see I also kind of suggest to allow accepting True/Fals to
boolean options to allow _unsetting_ the effect of the default set in
the [global] section.


3.) Handling of unattended mode when specifying a config file:
Currently there is no connection between --config-file and unattended 
mode. So when you run ipa-server-install using config file, you still 
get asked for missing stuff. Should '--config-file' automatically 
imply '--unattended'?

Well, there is certain beauty of providing some arguments from the
config file and be asked for the rest. Unattended is more explicit in
the way of handling so I would still keep them separate.

--
/ 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 0002] TEST: Stageuser plugin

2015-07-29 Thread Lenka Doudova

Hi,

thanks a lot for the comments, will work on it tomorrow.

Lenka

Dne 29.7.2015 v 15:27 Martin Basti napsal(a):

On 27/07/15 16:47, Lenka Doudova wrote:

Hi,

I'm attaching a patch with automated tests for stageuser plugin 
(https://fedorahosted.org/freeipa/ticket/3813). The user plugin test 
is affected as well (one class was added).
The tests seem a bit of a mess even to myself, but what with the way 
freeipa behaves I didn't know how else to implement them, but I'm 
eager to learn how to do it in a nicer way, if someone has a better 
idea.


Lenka





I just applied patches:

1) Please remove whitespace errors
$ git am freeipa-lryznaro-0002-Automated-test-for-stageuser-plugin.patch
Applying: Automated test for stageuser plugin
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:110: trailing 
whitespace.

 Tracker class for staged user LDAP object
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:113: trailing 
whitespace.

StageUserTracker object stores information about the user.
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:121: trailing 
whitespace.
u'krbprincipalexpiration', u'usercertificate', u'dn', 
u'has_keytab', u'has_password',
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:122: trailing 
whitespace.
u'street', u'postalcode', u'facsimiletelephonenumber', 
u'carlicense',
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:125: trailing 
whitespace.

u'cn', u'ipauniqueid', u'objectclass', u'description',
warning: squelched 50 whitespace errors
warning: 55 lines add whitespace errors.

2)
Please use new shorter format of license header

3) can you fix some of the most serious PEP8 errors
$ git show -U0 | pep8 --diff | wc -l
198

4)
if options != None:

Please use options *is not* None

5)
For consistency it should be u'random'
if key == 'random':
self.attrs[u'randompassword'] = fuzzy_string

Otherwise it looks good
Martin^2
--
Martin Basti


-- 
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] 0028 add --out option to user-show

2015-07-29 Thread Martin Basti

On 25/07/15 03:40, Fraser Tweedale wrote:

On Fri, Jul 24, 2015 at 05:53:56PM +0200, Tomas Babej wrote:


On 07/24/2015 05:34 PM, Martin Basti wrote:

On 24/07/15 16:52, Tomas Babej wrote:

On 07/24/2015 03:40 PM, Fraser Tweedale wrote:

The attached patch adds --out option to user-show for saving user's
certificate(s) to file.

Thanks,
Fraser




I hate to nitpick here, but is out really a descriptive option name
here? I'd prefer something more explicit, like '--save-cert-to', or
maybe even have this operation implemented as a separate command
altogether.

Tomas


This keyword was already used with several commands. For consistency
might be better to have it the same.


True. I see this options is being used in the following commands:

  - cert-show
  - vault-retrieve
  - host-show
  - service-show
  - user-show (proposed)

While the first two seem to be an acceptable fit for an option called
--out, as they mainly deal with cert/secret, using the '--out' for the
latter three is a poor decision imho.

I agree the consistency is important, I'm just not happy to see this
spread further.

Tomas

Perhaps we should go with something like `--certout' instead, and
support `--certout' in addition to `--out' in host-show and
service-show, esentially deprecating `--out' for those commands.

Cheers,
Fraser

Good idea, but we should do this for all commands, at the same time.
IMO this is not for 4.2, you may file a ticket to deprecate --out option 
and replace it by --certout or something.


I will do review is nobody is against this patch :)
Martin^2

--
Martin Basti

--
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 0002] TEST: Stageuser plugin

2015-07-29 Thread Martin Basti

On 27/07/15 16:47, Lenka Doudova wrote:

Hi,

I'm attaching a patch with automated tests for stageuser plugin 
(https://fedorahosted.org/freeipa/ticket/3813). The user plugin test 
is affected as well (one class was added).
The tests seem a bit of a mess even to myself, but what with the way 
freeipa behaves I didn't know how else to implement them, but I'm 
eager to learn how to do it in a nicer way, if someone has a better idea.


Lenka





I just applied patches:

1) Please remove whitespace errors
$ git am freeipa-lryznaro-0002-Automated-test-for-stageuser-plugin.patch
Applying: Automated test for stageuser plugin
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:110: trailing 
whitespace.

 Tracker class for staged user LDAP object
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:113: trailing 
whitespace.

StageUserTracker object stores information about the user.
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:121: trailing 
whitespace.
u'krbprincipalexpiration', u'usercertificate', u'dn', 
u'has_keytab', u'has_password',
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:122: trailing 
whitespace.
u'street', u'postalcode', u'facsimiletelephonenumber', 
u'carlicense',
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:125: trailing 
whitespace.

u'cn', u'ipauniqueid', u'objectclass', u'description',
warning: squelched 50 whitespace errors
warning: 55 lines add whitespace errors.

2)
Please use new shorter format of license header

3) can you fix some of the most serious PEP8 errors
$ git show -U0 | pep8 --diff | wc -l
198

4)
if options != None:

Please use options *is not* None

5)
For consistency it should be u'random'
if key == 'random':
self.attrs[u'randompassword'] = fuzzy_string

Otherwise it looks good
Martin^2

--
Martin Basti

-- 
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 0286, 0290] Sysrestore: copy files instead of moving them to avoid SELinux issues

2015-07-29 Thread Martin Basti

On 29/07/15 09:02, David Kupka wrote:

On 17/07/15 16:33, Martin Basti wrote:

On 17/07/15 13:57, Petr Vobornik wrote:

On 07/17/2015 01:46 PM, Petr Vobornik wrote:

On 07/17/2015 01:44 PM, Alexander Bokovoy wrote:

On Fri, 17 Jul 2015, Martin Basti wrote:
From b05f4a2e17ae00e5c20e5eb7bd046472f100e0ad Mon Sep 17 00:00:00 
2001

From: Martin Basti mba...@redhat.com
Date: Wed, 15 Jul 2015 16:20:59 +0200
Subject: [PATCH] sysrestore: copy files instead of moving them to
avoind
SELinux issues


ACK.



Pushed to:
master: 9f701283534745bf93b41a1886183e9ef1d06566
ipa-4-2: 92a73e8b2a5f26744b036a36de4b9956e8883f61


Does it really fix the whole ticket?

There is also in freeipa.spec.in %post client (i.e. upgrade):

cat /etc/krb5.conf  /etc/krb5.conf.ipanew
mv /etc/krb5.conf.ipanew /etc/krb5.conf
/sbin/restorecon /etc/krb5.conf

+ some others.

Between the mv and restorecon, SSSD tries to access the file and
raises AVC.

In this case we can freely use mv -z since target platforms are Fedora
and newest RHEL.


The new patch fixing specfile attached.




Works for me, ACK.


Pushed to:
master: 45c709112da1514d57db46f9706bc03920574adf
ipa-4-2: 21d31224780d4e1e5e4371f12c5ebae6b4aca54f


--
Martin Basti

--
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 0050] ACI plugin: correctly parse bind rules enclosed in parentheses

2015-07-29 Thread Martin Basti

On 28/07/15 14:11, Martin Basti wrote:

On 28/07/15 13:33, Martin Babinsky wrote:

On 07/27/2015 05:10 PM, Martin Basti wrote:

On 23/07/15 16:06, Martin Babinsky wrote:

This is a quick fix for https://fedorahosted.org/freeipa/ticket/5037




NACK

I do not like your change in first regexp too much.

Can you try this instead?

PermPat = re.compile(r'(\w+)\s*\(([^()]*)\)\s*(.*)', re.UNICODE)

This just removes '(' and ') ' from pattern and accept all other 
characters.


--
Martin Basti



Attaching updated patch.


ACK


Pushed to:
master: a2ba9373070b19c158be8be78f7fbeee5ccab081
ipa-4-2: d85f92c0e75f3b389edac353de2cf08105b33cc4


--
Martin Basti

--
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 016] Require Dogtag PKI = 10.2.6

2015-07-29 Thread Martin Basti

On 23/07/15 12:26, Christian Heimes wrote:

Dogtag 10.2.6 comes with two fixes for cloning from 9.x to 10.x
instances:

   https://fedorahosted.org/pki/ticket/1495
   https://fedorahosted.org/pki/ticket/1488

https://fedorahosted.org/freeipa/ticket/5140
https://fedorahosted.org/freeipa/ticket/5129



ACK

--
Martin Basti

-- 
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 0002] TEST: Stageuser plugin

2015-07-29 Thread Martin Basti

On 29/07/15 15:29, Lenka Doudova wrote:

Hi,

thanks a lot for the comments, will work on it tomorrow.

Lenka

Dne 29.7.2015 v 15:27 Martin Basti napsal(a):

On 27/07/15 16:47, Lenka Doudova wrote:

Hi,

I'm attaching a patch with automated tests for stageuser plugin 
(https://fedorahosted.org/freeipa/ticket/3813). The user plugin test 
is affected as well (one class was added).
The tests seem a bit of a mess even to myself, but what with the way 
freeipa behaves I didn't know how else to implement them, but I'm 
eager to learn how to do it in a nicer way, if someone has a better 
idea.


Lenka





I just applied patches:

1) Please remove whitespace errors
$ git am freeipa-lryznaro-0002-Automated-test-for-stageuser-plugin.patch
Applying: Automated test for stageuser plugin
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:110: trailing 
whitespace.

 Tracker class for staged user LDAP object
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:113: trailing 
whitespace.

StageUserTracker object stores information about the user.
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:121: trailing 
whitespace.
u'krbprincipalexpiration', u'usercertificate', u'dn', 
u'has_keytab', u'has_password',
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:122: trailing 
whitespace.
u'street', u'postalcode', u'facsimiletelephonenumber', 
u'carlicense',
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:125: trailing 
whitespace.

u'cn', u'ipauniqueid', u'objectclass', u'description',
warning: squelched 50 whitespace errors
warning: 55 lines add whitespace errors.

2)
Please use new shorter format of license header

3) can you fix some of the most serious PEP8 errors
$ git show -U0 | pep8 --diff | wc -l
198

4)
if options != None:

Please use options *is not* None

5)
For consistency it should be u'random'
if key == 'random':
self.attrs[u'randompassword'] = fuzzy_string

Otherwise it looks good
Martin^2
--
Martin Basti



And also fix this please

./make-lint
* Module ipatests.test_xmlrpc.test_stageuser_plugin
ipatests/test_xmlrpc/test_stageuser_plugin.py:337: 
[E0102(function-redefined), user2] function already defined line 44)


--
Martin Basti

-- 
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 0052] Add Chromium configuration note under Chrome section in ssbrowser

2015-07-29 Thread Gabe Alford
Hello,

As Chromium and Chrome are configured similarly but are configured in
different /etc directories, this patch adds a note to the Chrome section in
ssbrowser.html stating that.

Thanks,

Gabe
From a7fb316d3cc273531947768e6b93c656a6bad1bb Mon Sep 17 00:00:00 2001
From: Gabe redhatri...@gmail.com
Date: Wed, 29 Jul 2015 07:38:15 -0600
Subject: [PATCH] Add Chromium configuration note to ssbrowser

- As Chromium and Chrome share most of the same code base but are
  configured in different locations, add a note showing the different
  configuration locations.

A part of https://fedorahosted.org/freeipa/ticket/823
---
 install/html/ssbrowser.html | 5 +
 1 file changed, 5 insertions(+)

diff --git a/install/html/ssbrowser.html b/install/html/ssbrowser.html
index 685800e16e6e77c70adf905acfca2996513d1e1d..b88deac900fb1d5a1a5960741512593f9b7f3b15 100644
--- a/install/html/ssbrowser.html
+++ b/install/html/ssbrowser.html
@@ -134,6 +134,11 @@
 /code/div
 /li
 /ol
+ol
+p
+strongNote:/strong If using Chromium, use code/etc/chromium/policies/managed//code instead of code/etc/opt/chrome/policies/managed//code for the two SPNEGO Chrome configuration steps above.
+/p
+/ol
 
 h2Internet Explorer/h2
 p
-- 
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] [PATCH] 0028 add --out option to user-show

2015-07-29 Thread Martin Basti

On 29/07/15 15:41, Martin Basti wrote:

On 25/07/15 03:40, Fraser Tweedale wrote:

On Fri, Jul 24, 2015 at 05:53:56PM +0200, Tomas Babej wrote:


On 07/24/2015 05:34 PM, Martin Basti wrote:

On 24/07/15 16:52, Tomas Babej wrote:

On 07/24/2015 03:40 PM, Fraser Tweedale wrote:

The attached patch adds --out option to user-show for saving user's
certificate(s) to file.

Thanks,
Fraser




I hate to nitpick here, but is out really a descriptive option name
here? I'd prefer something more explicit, like '--save-cert-to', or
maybe even have this operation implemented as a separate command
altogether.

Tomas


This keyword was already used with several commands. For consistency
might be better to have it the same.


True. I see this options is being used in the following commands:

  - cert-show
  - vault-retrieve
  - host-show
  - service-show
  - user-show (proposed)

While the first two seem to be an acceptable fit for an option called
--out, as they mainly deal with cert/secret, using the '--out' for the
latter three is a poor decision imho.

I agree the consistency is important, I'm just not happy to see this
spread further.

Tomas

Perhaps we should go with something like `--certout' instead, and
support `--certout' in addition to `--out' in host-show and
service-show, esentially deprecating `--out' for those commands.

Cheers,
Fraser

Good idea, but we should do this for all commands, at the same time.
IMO this is not for 4.2, you may file a ticket to deprecate --out 
option and replace it by --certout or something.


I will do review is nobody is against this patch :)
Martin^2



Is a ticket somewhere for this?

--
Martin Basti

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