date:20140620

On Fri, 2014-06-20 at 16:50 -0400, Nathaniel McCallum wrote:
> On Fri, 2014-06-20 at 16:05 -0400, Simo Sorce wrote:
> > On Fri, 2014-06-20 at 14:47 -0400, Nathaniel McCallum wrote:
> > > This change would have very small impact on your patch set, but would
> > > be
> > > much clearer for the future consumers of this protocol. Code can be
> > > changed; protocols can't.
> > 
> > Ok this new patchset implements the requested change.
> > Initial testing seem to indicate it all works as expected.
> 
> 0001: Line 555 has very wrong indentation.
> 
> Other than that, I have looked over 0001 and 0002 very closely and built
> and tested them. Everything works. So conditional (indent fix) ACK on
> both of these. I don't see any reason to avoid merging these as soon as
> the indent fix is completed. It should substantially reduce your
> differential from master.
> 
> In the new ASN.1, "Newkeys" has the wrong capitalization. This affects
> patches 0003 and 0005.
> 
> I think patch 0003 may still have a permissions problem. For instance,
> this works for me with no error:
> 
> $ ipa user-find --whoami
> --
> 1 user matched
> --
>   User login: admin
>   Last name: Administrator
>   Home directory: /home/admin
>   Login shell: /bin/bash
>   UID: 156960
>   GID: 156960
>   Account disabled: False
>   Password: True
>   Kerberos keys available: True
> 
> Number of entries returned 1
> 
> 
> $ ipa-getkeytab -s ipa.example.com -p foo/ipa.example.com -r -k bar
> Keytab successfully retrieved and stored in: bar
> 
> Is that really correct behavior or did I screw something up? I thought
> we had restricted the admin user from reading keys without changing
> them...

Can you check if ipaProtectedOperation is in the aci attribute in the
base tree object ?
It should be there as excluded, and that should cause admin to not be
able to retrieve keytabs.

Simo.

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

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

Re: [Freeipa-devel] [PATCHES] 0594-0606 Convert default permissions to managed


On 06/20/2014 05:06 PM, Petr Viktorin wrote:

All these should be independent, except for conflicts in ACI.txt that are
easily solved by running makeaci.


Umh, now the fun begins as I see :) There will probably need to be some rebase, 
it clashed with some other ACI patches in my tree (namely Hosts which I acked).


594: we miss permissions for Automount Locations. Permissions for keys&maps 
look ok.


595: "System: Modify Group Membership" is probably waiting for the group 
objectclass fix - the filter is different. Otherwise it looks ok.


596-598: HBAC is ok

599: hostgroup is OK

600: there must have been some DS problem on my side as my regular user could 
not see any netgroup


601: privileges - we miss CRUD ACIs

602: roles were ok

603: ok

I got this far today, the rest will need to wait for the next week.

Thanks,
Martin

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

Re: [Freeipa-devel] Design Review Keytab Retrieval

On Fri, 2014-06-20 at 16:05 -0400, Simo Sorce wrote:
> On Fri, 2014-06-20 at 14:47 -0400, Nathaniel McCallum wrote:
> > This change would have very small impact on your patch set, but would
> > be
> > much clearer for the future consumers of this protocol. Code can be
> > changed; protocols can't.
> 
> Ok this new patchset implements the requested change.
> Initial testing seem to indicate it all works as expected.

0001: Line 555 has very wrong indentation.

Other than that, I have looked over 0001 and 0002 very closely and built
and tested them. Everything works. So conditional (indent fix) ACK on
both of these. I don't see any reason to avoid merging these as soon as
the indent fix is completed. It should substantially reduce your
differential from master.

In the new ASN.1, "Newkeys" has the wrong capitalization. This affects
patches 0003 and 0005.

I think patch 0003 may still have a permissions problem. For instance,
this works for me with no error:

$ ipa user-find --whoami
--
1 user matched
--
  User login: admin
  Last name: Administrator
  Home directory: /home/admin
  Login shell: /bin/bash
  UID: 156960
  GID: 156960
  Account disabled: False
  Password: True
  Kerberos keys available: True

Number of entries returned 1

$ ipa-getkeytab -s ipa.example.com -p foo/ipa.example.com -r -k bar
Keytab successfully retrieved and stored in: bar

Is that really correct behavior or did I screw something up? I thought
we had restricted the admin user from reading keys without changing
them...

Nathaniel

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

Re: [Freeipa-devel] [PATCHES] 0578-0579 Convert Host default permissions to managed


On 06/19/2014 01:41 PM, Petr Viktorin wrote:

On 06/18/2014 05:46 PM, Martin Kosek wrote:

On 06/11/2014 06:39 PM, Petr Viktorin wrote:

Patch 0578 does the conversion

Patch 0579 fixes https://fedorahosted.org/freeipa/ticket/4252 and provides
permissions needed for automatic enrollment (from
http://projects.theforeman.org/projects/foreman/wiki/IPASmartProxyUser)


1) Inconsistent casing in permission names:

System: Add Hosts
System: Add krbPrincipalName to a host
System: Enroll a host
System: Manage Host SSH Public Keys
System: Manage host keytab
System: Modify Hosts
System: Remove Hosts


Fixed


2) This ACI does not look right, missing enrolledby:

+'System: Enroll a host': {
+'ipapermright': {'write'},
+'ipapermdefaultattr': {'objectclass'},

When I fixed 2) via permission-mod, client enrollment with user with "Host
Administrators" privilege worked fine.


Added


3) I hit one issue when I open the Web UI host tab, I get "Insufficient access:
No such virtual command" error triggered by "cert-show" command.


Virtual operations seem to be quite a can of worms.
I've sent a separate reply for these.


We will need to add the permission "System: Read Virtual Operations" that Honza
is creating also to "Host Administrators" to fix that part.


4) I ran unit tests and few missing attributes:
- update hosts ACI should get "macaddress" attribute


Added


5) I hit one nasty issue when running the unit tests (when my master stopped
working as host account was deleted) - host_is_master function in baseldap no
longer works as we hid cn=masters from regular users:

def host_is_master(ldap, fqdn):
 """
 Check to see if this host is a master.

 Raises an exception if a master, otherwise returns nothing.
 """
 master_dn = DN(('cn', fqdn), ('cn', 'masters'), ('cn', 'ipa'), ('cn',
'etc'), api. env.basedn)
 try:
 ldap.get_entry(master_dn, ['objectclass'])
 raise errors.ValidationError(name='hostname', error=_('An IPA master
host  cannot be deleted or disabled'))
 except errors.NotFound:
 # Good, not a master
 return

This means, that host-del on a master machine or service-del on master service
happily passes.

We need to make sure this functionality is still working after the permission
refactoring. Should we reconsider the cn=masters tree and allow authenticated
users see the list of IPA servers (without digging into any other detail like
services) then?


Nasty indeed, thanks for the catch!

Sent as patch 0590, since it's a different issue than converting the host
permissions.


Everything worked as expected, I tested both enrollments with privileged user 
and setting the OTP/class.


I have just one request (you will not like this) - before pushing please also 
fix casing for the new host permissions to match others:


+'System: Manage host certificates': {
+'System: Manage host enrollment password': {

When this is fixed (and ACI.txt properly updated), it is an ACK.

Martin

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

Re: [Freeipa-devel] [PATCH] 0593 Allow anonymous read access to virtual operation entries (Re: Virtual operation ACIs)


On 06/20/2014 04:49 PM, Petr Viktorin wrote:

On 06/19/2014 02:13 PM, Martin Kosek wrote:

On 06/19/2014 12:52 PM, Petr Viktorin wrote:

I'll address the other issues separately.

On 06/18/2014 05:46 PM, Martin Kosek wrote:

3) I hit one issue when I open the Web UI host tab, I get "Insufficient
access:
No such virtual command" error triggered by "cert-show" command.

We will need to add the permission "System: Read Virtual Operations" that
Honza
is creating also to "Host Administrators" to fix that part.


I'm not familiar with Honza's effort, but that seems right.
I'm curious, why don't we just allow reading virtual operations by anybody? It
seems to me they're the same in every IPA installation, what's there to hide?


They are indeed the same. This is an old (very old) mean to check access when
ACI cannot be used. I admit it is a bit clumsy.

I agree that we should indeed allow reading the list of virtual operations as
the list can be retrieved from our git anyway. The virtual operations do not
even show list of it's members as permissions hold it, so it really should not
leak any sensitive information.


Anyway, I poked around in how it works now: for cert-show you need write access
to the objectClass of the "retrieve certificate" virt op entry. So that right
you can actually remove the "ipaVirtualOperation" objectClass.
Aand the new "Anonymous read access to containers" ACI has a
(!(objectclass=ipaVirtualOperation)) filter, so any user privileged for a virt
op can allow everyone see that virt op).
Shouldn't we base the check on some other attribute instead?

And curiously, for cert-find there is no virt op based access check.


I think we should eventually invent something better than current virtual
operations. For now (4.0), we should do something simple and straightforward.
The simplest thing to do is stick to the old behavior, i.e.:

1) Remove the (!(objectclass=ipaVirtualOperation)) part of the filter (should
improve performance, right?)
2) Remove the ipaVirtualOperation objectclass again from the virtual operations
as it would be useless after change 1)


Patch attached.


Yup, works fine. This will remove all the pain we had with hidden virtual 
operations.


ACK. Pushed to master: f486d23ad67a7337c7633e4216c5a0b0374002fc

Martin

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

Re: [Freeipa-devel] Design Review Keytab Retrieval

On Fri, 2014-06-20 at 15:55 -0400, Nathaniel McCallum wrote:
> On Fri, 2014-06-20 at 15:50 -0400, Nathaniel McCallum wrote:
> > On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote:
> > > Although the code is all done it would be nice to have a review of the
> > > feature, to see if it has all been captured:
> > > http://www.freeipa.org/page/V4/Keytab_Retrieval
> > 
> > Is there any need to create different permissions for password
> > generation vs random generation? I think the answer is no because the
> > only risk is a substandard password which you can control with password
> > policy. I just wanted to bring it up.
> 
> Other than this question and the ASN.1 issue, I am very happy with the
> design. The new permissions ACI style looks very useful.

Thanks, I changed the page to reflect the new controls too.

Simo.

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

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

Re: [Freeipa-devel] Design Review Keytab Retrieval

On Fri, 2014-06-20 at 15:50 -0400, Nathaniel McCallum wrote:
> On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote:
> > Although the code is all done it would be nice to have a review of the
> > feature, to see if it has all been captured:
> > http://www.freeipa.org/page/V4/Keytab_Retrieval
> 
> Is there any need to create different permissions for password
> generation vs random generation? I think the answer is no because the
> only risk is a substandard password which you can control with password
> policy. I just wanted to bring it up.

Indeed we do not differentiate, password policy is what you want to use
if you want minimum guarantees, for now.

Simo.

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

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

Re: [Freeipa-devel] Design Review Keytab Retrieval

On Fri, 2014-06-20 at 15:50 -0400, Nathaniel McCallum wrote:
> On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote:
> > Although the code is all done it would be nice to have a review of the
> > feature, to see if it has all been captured:
> > http://www.freeipa.org/page/V4/Keytab_Retrieval
> 
> Is there any need to create different permissions for password
> generation vs random generation? I think the answer is no because the
> only risk is a substandard password which you can control with password
> policy. I just wanted to bring it up.

Other than this question and the ASN.1 issue, I am very happy with the
design. The new permissions ACI style looks very useful.

Nathaniel

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

Re: [Freeipa-devel] Design Review Keytab Retrieval

On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote:
> Although the code is all done it would be nice to have a review of the
> feature, to see if it has all been captured:
> http://www.freeipa.org/page/V4/Keytab_Retrieval

Is there any need to create different permissions for password
generation vs random generation? I think the answer is no because the
only risk is a substandard password which you can control with password
policy. I just wanted to bring it up.

Nathaniel

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

Re: [Freeipa-devel] [PATCH] [IMPORTANT] Make otptoken use os.urandom() for random data


On 06/20/2014 05:59 PM, Simo Sorce wrote:

On Fri, 2014-06-20 at 11:56 -0400, Nathaniel McCallum wrote:

On Thu, 2014-06-19 at 12:43 -0400, Simo Sorce wrote:

On Thu, 2014-06-19 at 12:36 -0400, Nathaniel McCallum wrote:

This also fixes an error where the default value was not respecting
the KEY_LENGTH variable.

(NOTE: the os.urandom() change should not change the security properties
of the existing code. However, the failure of the previous code to
respect KEY_LENGTH causes us to violate the RFC.)


LGTM!
I do prefer using os.urandom() directly, as random.SystemRandom uses it
under the hood anyway.


Is that an ACK? Because we need to merge a fix of some kind soon.


If someone can actually test it I would prefer, as I did not, and I am
not sure I will find the time today, that's why I did not give a full
ACK.

Simo.




I tested at least the lambda and it worked as expected.

Pushed to master: cf8f143e9823c06ed069c6a031c0c4aa80288840

Martin

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

Re: [Freeipa-devel] #4389: DS deref broken after ACI refactoring


On 06/20/2014 05:51 PM, Jakub Hrozek wrote:

On Fri, Jun 20, 2014 at 04:45:45PM +0200, Martin Kosek wrote:

On 06/20/2014 04:24 PM, Jakub Hrozek wrote:

On Fri, Jun 20, 2014 at 04:06:16PM +0200, Martin Kosek wrote:

...

I think we should just make a note to self to allow users to fix the
ACIs manually should they run into the problem while being unable to
upgrade for whatever reason.

So we only seem to dereference member and memberof. We dereference either user
groups to get users, host groups to get hosts. For hosts we are
interested about these attributes:
 "ipa_host_object_class"
 "ipa_host_name"
 "ipa_host_fqdn"
 "ipa_host_serverhostname"
 "ipa_host_member_of"
 "ipa_host_ssh_public_key"
 "ipa_host_uuid"

For users and groups, the list is longer and can be found here:
https://git.fedorahosted.org/cgit/sssd.git/tree/src/providers/ipa/ipa_opts.h#n166

Look for ipa_user_map and ipa_group_map.

But in general I agree with Simo that we shouldn't spend too much time
on this when the DS is fixed.


Ok, makes sense.







For IPA we only care about memberof, but keep in mind that attribute
maps in SSSD are configurable.


Hm, makes the option 2) even more challenging...



But because the ACIs would only be applied on IPA servers, I think we
can limit ourselves to the IPA schema only for the workaround.


Thanks all for responses. It seems that plan is clear:

1) Prepare a fix for DS deref access control issue 
(https://fedorahosted.org/389/ticket/47821). Ludwig, could you now please start 
working on this one? It takes precedence before 4.1 or 4.2 work you were 
working on.


2) Backport the fix to supported platforms along with other ACI fixes that 
Ludwig already found - Fedora 19 (?), Fedora 20, next RHEL-6.x.


3) 4.0 release note will contain a warning about the minimal DS version of the 
replicas. We will have a workaround ready based on the data that Jakub provided 
in case someone hit the issue and cannot update to fixes DS version.


Martin

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

Re: [Freeipa-devel] Design Review Keytab Retrieval

On Fri, 2014-06-20 at 14:38 -0400, Simo Sorce wrote:
> On Fri, 2014-06-20 at 14:30 -0400, Nathaniel McCallum wrote:
> > On Fri, 2014-06-20 at 14:10 -0400, Simo Sorce wrote:
> > > On Fri, 2014-06-20 at 14:05 -0400, Nathaniel McCallum wrote:
> > > > On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote:
> > > > > Although the code is all done it would be nice to have a review of the
> > > > > feature, to see if it has all been captured:
> > > > > http://www.freeipa.org/page/V4/Keytab_Retrieval
> > > > 
> > > > I'm a bit confused about the behavior of enctypes in the Request.
> > > > 
> > > > "A list of enctypes is always necessary in input when a new keytab is
> > > > requested. However the list is filtered though the allowable enctypes
> > > > list and if nothing is left the operation is refused."
> > > > 
> > > > +1. However, the generated keys should be the set of allowed enctypes,
> > > > not the intersection between allowed and requested enctypes. This would
> > > > permit the later requesting of enctypes that were allowed at the time of
> > > > creation, but not requested.
> > > 
> > > Which you absolutely do not want.
> > > If your service understands only RC4-HMAC and you provide it with AES
> > > keys then communication from a third party client will fail because the
> > > KDC will give that client an AES encrypted ticket that the service does
> > > not know how to decrypt.
> > > 
> > > This is particularly important for old NFS Servers (like RHEL5) that
> > > understand only DES (sigh!)
> > > 
> > > > "If the getNew attribute is false, then the existing key is being
> > > > requested. In this case password and enctypes MUST NOT be set."
> > > > 
> > > > I don't get this. Shouldn't the return value of this request include
> > > > only the intersection between allowed and requested enctypes?
> > > 
> > > Not when you are retrieving existing keys. Only when you are creating
> > > new keys.
> > > 
> > > > There is no point in responding with enctypes the client has not 
> > > > requested.
> > > > And indeed, this provides extra data points to attack.
> > > 
> > > ??
> > > 
> > > > Having this proposed behavior also means you can remove OPTIONAL from
> > > > enctypes.
> > > 
> > > OPTIONAL is there because when you request existing keys it makes no
> > > sense to send a lit of enctypes, you get what the KDC has, the whole
> > > package, because you must be able to decrypt any ticket you get, getting
> > > a subset of keys would make no sense.
> > > 
> > > > So as it stands, enctypes currently controls what keys are generated. I
> > > > would prefer that enctypes controls what keys are returned. Am I missing
> > > > something?
> > > 
> > > I most definitely think you are :-)
> > 
> > Ah, right. This boils down to the KDC not having any way to know what
> > enctypes the destination principal supports. Thanks for clarifying that.
> > 
> > In this case, the ASN.1 provided is confusing. I think something like
> > this would be clearer and less error prone:
> > 
> > KeytabGetMessage ::= CHOICE {
> > fetch[0] Fetch,
> > create   [1] Create
> > reply[2] Reply
> > }
> > 
> > Fetch ::= SEQUENCE {
> > serviceIdentity [0] OCTET STRING
> > }
> > 
> > Create ::= SEQUENCE {
> > serviceIdentity [0] OCTET STRING,
> > enctypes[1] SEQUENCE OF Int16,
> > password[2] OCTET STRING OPTIONAL
> > }
> > 
> > This would also, I think, result in a cleaner implementation where the
> > type of operation is logically built into the decoding step.
> 
> Well yes this looks better, but honestly, I think it is a bit late for
> this kind of feedback, given I have a working patchset, and asked months
> ago if people wanted to review the logical design.

You asked for a design review on Monday of this week. I am providing
that review. Do you want it or not?

> How strongly do you feel we absolutely need to change this and go
> through a new review cycle ?

This change would have very small impact on your patch set, but would be
much clearer for the future consumers of this protocol. Code can be
changed; protocols can't.

I am finishing the design doc now and will move to your patch set
shortly. I'd be happy to provide a quick code review for this change.

Nathaniel

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

Re: [Freeipa-devel] Design Review Keytab Retrieval

On Fri, 2014-06-20 at 14:30 -0400, Nathaniel McCallum wrote:
> On Fri, 2014-06-20 at 14:10 -0400, Simo Sorce wrote:
> > On Fri, 2014-06-20 at 14:05 -0400, Nathaniel McCallum wrote:
> > > On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote:
> > > > Although the code is all done it would be nice to have a review of the
> > > > feature, to see if it has all been captured:
> > > > http://www.freeipa.org/page/V4/Keytab_Retrieval
> > > 
> > > I'm a bit confused about the behavior of enctypes in the Request.
> > > 
> > > "A list of enctypes is always necessary in input when a new keytab is
> > > requested. However the list is filtered though the allowable enctypes
> > > list and if nothing is left the operation is refused."
> > > 
> > > +1. However, the generated keys should be the set of allowed enctypes,
> > > not the intersection between allowed and requested enctypes. This would
> > > permit the later requesting of enctypes that were allowed at the time of
> > > creation, but not requested.
> > 
> > Which you absolutely do not want.
> > If your service understands only RC4-HMAC and you provide it with AES
> > keys then communication from a third party client will fail because the
> > KDC will give that client an AES encrypted ticket that the service does
> > not know how to decrypt.
> > 
> > This is particularly important for old NFS Servers (like RHEL5) that
> > understand only DES (sigh!)
> > 
> > > "If the getNew attribute is false, then the existing key is being
> > > requested. In this case password and enctypes MUST NOT be set."
> > > 
> > > I don't get this. Shouldn't the return value of this request include
> > > only the intersection between allowed and requested enctypes?
> > 
> > Not when you are retrieving existing keys. Only when you are creating
> > new keys.
> > 
> > > There is no point in responding with enctypes the client has not 
> > > requested.
> > > And indeed, this provides extra data points to attack.
> > 
> > ??
> > 
> > > Having this proposed behavior also means you can remove OPTIONAL from
> > > enctypes.
> > 
> > OPTIONAL is there because when you request existing keys it makes no
> > sense to send a lit of enctypes, you get what the KDC has, the whole
> > package, because you must be able to decrypt any ticket you get, getting
> > a subset of keys would make no sense.
> > 
> > > So as it stands, enctypes currently controls what keys are generated. I
> > > would prefer that enctypes controls what keys are returned. Am I missing
> > > something?
> > 
> > I most definitely think you are :-)
> 
> Ah, right. This boils down to the KDC not having any way to know what
> enctypes the destination principal supports. Thanks for clarifying that.
> 
> In this case, the ASN.1 provided is confusing. I think something like
> this would be clearer and less error prone:
> 
> KeytabGetMessage ::= CHOICE {
> fetch[0] Fetch,
> create   [1] Create
> reply[2] Reply
> }
> 
> Fetch ::= SEQUENCE {
> serviceIdentity [0] OCTET STRING
> }
> 
> Create ::= SEQUENCE {
> serviceIdentity [0] OCTET STRING,
> enctypes[1] SEQUENCE OF Int16,
> password[2] OCTET STRING OPTIONAL
> }
> 
> This would also, I think, result in a cleaner implementation where the
> type of operation is logically built into the decoding step.

Well yes this looks better, but honestly, I think it is a bit late for
this kind of feedback, given I have a working patchset, and asked months
ago if people wanted to review the logical design.

How strongly do you feel we absolutely need to change this and go
through a new review cycle ?

Simo.

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

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

Re: [Freeipa-devel] Design Review Keytab Retrieval

On Fri, 2014-06-20 at 14:10 -0400, Simo Sorce wrote:
> On Fri, 2014-06-20 at 14:05 -0400, Nathaniel McCallum wrote:
> > On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote:
> > > Although the code is all done it would be nice to have a review of the
> > > feature, to see if it has all been captured:
> > > http://www.freeipa.org/page/V4/Keytab_Retrieval
> > 
> > I'm a bit confused about the behavior of enctypes in the Request.
> > 
> > "A list of enctypes is always necessary in input when a new keytab is
> > requested. However the list is filtered though the allowable enctypes
> > list and if nothing is left the operation is refused."
> > 
> > +1. However, the generated keys should be the set of allowed enctypes,
> > not the intersection between allowed and requested enctypes. This would
> > permit the later requesting of enctypes that were allowed at the time of
> > creation, but not requested.
> 
> Which you absolutely do not want.
> If your service understands only RC4-HMAC and you provide it with AES
> keys then communication from a third party client will fail because the
> KDC will give that client an AES encrypted ticket that the service does
> not know how to decrypt.
> 
> This is particularly important for old NFS Servers (like RHEL5) that
> understand only DES (sigh!)
> 
> > "If the getNew attribute is false, then the existing key is being
> > requested. In this case password and enctypes MUST NOT be set."
> > 
> > I don't get this. Shouldn't the return value of this request include
> > only the intersection between allowed and requested enctypes?
> 
> Not when you are retrieving existing keys. Only when you are creating
> new keys.
> 
> > There is no point in responding with enctypes the client has not requested.
> > And indeed, this provides extra data points to attack.
> 
> ??
> 
> > Having this proposed behavior also means you can remove OPTIONAL from
> > enctypes.
> 
> OPTIONAL is there because when you request existing keys it makes no
> sense to send a lit of enctypes, you get what the KDC has, the whole
> package, because you must be able to decrypt any ticket you get, getting
> a subset of keys would make no sense.
> 
> > So as it stands, enctypes currently controls what keys are generated. I
> > would prefer that enctypes controls what keys are returned. Am I missing
> > something?
> 
> I most definitely think you are :-)

Ah, right. This boils down to the KDC not having any way to know what
enctypes the destination principal supports. Thanks for clarifying that.

In this case, the ASN.1 provided is confusing. I think something like
this would be clearer and less error prone:

KeytabGetMessage ::= CHOICE {
fetch[0] Fetch,
create   [1] Create
reply[2] Reply
}

Fetch ::= SEQUENCE {
serviceIdentity [0] OCTET STRING
}

Create ::= SEQUENCE {
serviceIdentity [0] OCTET STRING,
enctypes[1] SEQUENCE OF Int16,
password[2] OCTET STRING OPTIONAL
}

This would also, I think, result in a cleaner implementation where the
type of operation is logically built into the decoding step.

Nathaniel

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

Re: [Freeipa-devel] LDAP schema for DNSSEC keys

On Fri, 2014-06-20 at 20:04 +0200, Petr Spacek wrote:
> ipk11Private;privatekey: TRUE
> ipk11Private;publickey: FALSE

can these two ever hold a different value ?
ie a privatekey be FALSE and a publickey be TRUE ?

If not I suggest you do not add this attribute at all and assume their
value ?
(btw I forgot what's the point of that attribute)

Simo.

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

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

Re: [Freeipa-devel] Design Review Keytab Retrieval

On Fri, 2014-06-20 at 14:05 -0400, Nathaniel McCallum wrote:
> On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote:
> > Although the code is all done it would be nice to have a review of the
> > feature, to see if it has all been captured:
> > http://www.freeipa.org/page/V4/Keytab_Retrieval
> 
> I'm a bit confused about the behavior of enctypes in the Request.
> 
> "A list of enctypes is always necessary in input when a new keytab is
> requested. However the list is filtered though the allowable enctypes
> list and if nothing is left the operation is refused."
> 
> +1. However, the generated keys should be the set of allowed enctypes,
> not the intersection between allowed and requested enctypes. This would
> permit the later requesting of enctypes that were allowed at the time of
> creation, but not requested.

Which you absolutely do not want.
If your service understands only RC4-HMAC and you provide it with AES
keys then communication from a third party client will fail because the
KDC will give that client an AES encrypted ticket that the service does
not know how to decrypt.

This is particularly important for old NFS Servers (like RHEL5) that
understand only DES (sigh!)

> "If the getNew attribute is false, then the existing key is being
> requested. In this case password and enctypes MUST NOT be set."
> 
> I don't get this. Shouldn't the return value of this request include
> only the intersection between allowed and requested enctypes?

Not when you are retrieving existing keys. Only when you are creating
new keys.

> There is no point in responding with enctypes the client has not requested.
> And indeed, this provides extra data points to attack.

??

> Having this proposed behavior also means you can remove OPTIONAL from
> enctypes.

OPTIONAL is there because when you request existing keys it makes no
sense to send a lit of enctypes, you get what the KDC has, the whole
package, because you must be able to decrypt any ticket you get, getting
a subset of keys would make no sense.

> So as it stands, enctypes currently controls what keys are generated. I
> would prefer that enctypes controls what keys are returned. Am I missing
> something?

I most definitely think you are :-)

Simo.

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

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

Re: [Freeipa-devel] Design Review Keytab Retrieval

On Mon, 2014-06-16 at 11:34 -0400, Simo Sorce wrote:
> Although the code is all done it would be nice to have a review of the
> feature, to see if it has all been captured:
> http://www.freeipa.org/page/V4/Keytab_Retrieval

I'm a bit confused about the behavior of enctypes in the Request.

"A list of enctypes is always necessary in input when a new keytab is
requested. However the list is filtered though the allowable enctypes
list and if nothing is left the operation is refused."

+1. However, the generated keys should be the set of allowed enctypes,
not the intersection between allowed and requested enctypes. This would
permit the later requesting of enctypes that were allowed at the time of
creation, but not requested.

"If the getNew attribute is false, then the existing key is being
requested. In this case password and enctypes MUST NOT be set."

I don't get this. Shouldn't the return value of this request include
only the intersection between allowed and requested enctypes? There is
no point in responding with enctypes the client has not requested. And
indeed, this provides extra data points to attack.

Having this proposed behavior also means you can remove OPTIONAL from
enctypes.

So as it stands, enctypes currently controls what keys are generated. I
would prefer that enctypes controls what keys are returned. Am I missing
something?

Nathaniel

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

Re: [Freeipa-devel] LDAP schema for DNSSEC keys

2014-06-20 Thread Petr Spacek


On 12.6.2014 16:23, Petr Spacek wrote:

On 30.4.2014 18:19, Petr Spacek wrote:

following text summarizes schema & DIT layout for DNSSEC key storage in LDAP.


I have added object classes and default values for attributes I consider
important. This is final proposal for implementation. Please review it ASAP.


This is subset of full PKCS#11 schema [0]. It stores bare keys with few
metadata attributes when necessary.

The intention is to make transition to full PKCS#11-in-LDAP schema [0] as easy
as possible. This transition should happen in next minor version of FreeIPA.

In theory, the transition should be just adding few object classes to existing
objects and populating few new metadata attributes. Related object classes are
marked below with "(in long-term)".

Please comment on it soon. We want to implement it ASAP :-)


DNSSEC key
==
- Asymmetric
- Private key is stored in LDAP as encrypted PKCS#8 blob
- Public key is published in LDAP
- Encrypted with symmetric "DNSSEC master key" (see below)
- Private key - represented as LDAP object with object classes:
ipaEPrivateKey  [1] # encrypted data
ipaWrappedKey   [2] # pointer to master key, outside scope of pure PKCS#11
ipk11PrivateKey [3] (in long-term) # PKCS#11 metadata
- Public key - represented as LDAP object with object classes:
ipaPublicKey[1] # public key data
ipk11PublicKey  [3] (in long-term) # PKCS#11 metadata


Master key
==
- Symmetric
- Stored in LDAP as encrypted blob
- Encrypted with asymmetric "replica key" (see below)
- 1 replica = 1 blob, n replicas = n blobs encrypted with different keys
- A replica uses it's own key for master key en/decryption
- Represented as LDAP object with object classes:
ipaESecretKey  [1]
ipk11SecretKey [3] (in long-term)

Replica key
===
- Asymmetric
- Private key is stored on replica's disk only
- Public key for all replicas is stored in LDAP
- Represented as LDAP object with object classes:
ipaPublicKey   [1]
ipk11PublicKey [3] (in long-term)


DIT layout
==
  DNSSEC key material
  ---
  - Container: cn=keys, cn=sec, cn=dns, dc=example
  - Private and public keys are stored as separate objects to accommodate all
PKCS#11 metadata.
  - We need to decide about object naming:
   - One obvious option for RDN is to use uniqueID but I don't like it. It is
hard to read for humans.
   - Other option is to use uniqueID+PKCS#11 label or other attributes to make
it more readable. Can we use multi-valued RDN? If not, why? What are technical
reasons behind it?

It is question if we like:
  nsUniqID = 0b0b7e53-957d11e3-a51dc0e5-9a05ecda
  nsUniqID = 8ae4190d-957a11e3-a51dc0e5-9a05ecda
more than:
  ipk11Label=meaningful_label+ipk11Private=TRUE
  ipk11Label=meaningful_label+ipk11Private=FALSE


dn: ipk11Label=zone1_keyid123_public, cn=keys, cn=sec, cn=dns, dc=example
objectClass: ipk11Object
objectClass: ipk11PublicKey
objectClass: ipaPublicKeyObject
ipk11UniqueId: 
ipk11Label: zone1_keyid123_public
ipk11Wrap: FALSE
ipk11Verify: TRUE
ipaPublicKey: 

dn: ipk11Label=zone1_keyid123_private, cn=keys, cn=sec, cn=dns, dc=example
objectClass: ipk11Object
objectClass: ipk11PrivateKey
objectClass: ipaPrivateKeyObject
objectClass: ipaWrappedKey
ipaWrappingKey:ipk11Label=dnssec_m1,cn=master,cn=keys,cn=sec,cn=dns,dc=example
ipk11Sign: TRUE
ipk11Decrypt: FALSE
ipk11Unwrap: FALSE
ipaPrivateKey: 



  DNSSEC key metadata
  ---
  - Container (per-zone): cn=keys, idnsname=example.net, cn=dns
  - Key metadata can be linked to key material via DN or ipk11Id.
  - This allows key sharing between zones.
(DNSSEC-metadata will be specified later. That is not important for key
storage.)

This will be sorted out in separate thread.



  Replica public keys
  ---
  - Container: cn=DNS,cn=,cn=masters,cn=ipa,cn=etc,dc=example
   - or it's child object like cn=wrappingKey


- Please note that private part of this key is stored on disk.

dn: ipk11Label=wrapkey_replica1,cn=DNS,cn=,cn=masters,cn=ipa,cn=etc,dc=example
objectClass: ipk11Object
objectClass: ipk11PublicKey
objectClass: ipaPublicKeyObject
ipk11UniqueId: 
ipk11Label: wrapkey_replica1
ipk11Wrap: TRUE
ipk11Encrypt: FALSE
ipk11Verify: FALSE
ipaPublicKey: 


  Master keys
  ---
  - Container: cn=master, cn=keys, cn=sec, cn=dns, dc=example
  - Single key = single object.
  - We can use ipk11Label or ipk11Id for naming:
  ipk11Label=dnssecMaster1, ipk11Label=dnssecMaster2, etc.


dn: ipk11Label=dnssec_m1, cn=master, cn=keys, cn=sec, cn=dns, dc=example
objectClass: ipk11Object
objectClass: ipk11SecretKey
objectClass: ipaWrappedKey
objectClass: ipaSecretKeyObject
ipk11UniqueId: 
ipk11Label: dnssec_m1
ipk11Wrap: TRUE
ipk11UnWrap: TRUE
ipk11Encrypt: FALSE
ipk11Decrypt: FALSE
ipk11Sign: FALSE
ipk11Verify: FALSE
ipaWrappingAlgo: 
ipaSecretKeyWrappedData: 
ipaSecretKeyWrappedData: 
ipaWrappingKey: 
ipaWrappingKey: 


Today it turned out that storing private and public keys in separate LDAP 
objects is probably not neces

Re: [Freeipa-devel] [PATCH] 647-651 [webui] Make utility section of navigation extensible

2014-06-20 Thread Endi Sukma Dewata


On 6/18/2014 6:11 AM, Petr Vobornik wrote:

1. As discussed on IRC, the plugin is causing an error due to missing
extend.js. This needs to be fixed.


Fixed



4. I agree that the facet shouldn't define the hash. The hash should be
part of the plugin declaration.


Ideally, facet should be router agnostic. We need a mechanism of mapping
facet's state to hash and vice versa.

Originally I did not want to do it now because it requires bigger
refactoring of a router.

While I was designing it, I ended up with a patch - attached.

It's a replacement for original patch #649. Patch #651 and example
plugin are updated accordingly.

Basically I refactored router's `navigate_to_*`, `_create_*_hash` and
`*_route_handler` methods into separate classes which are registered and
mapped to facets. All in navigation.routing module.

Btw, can you think of some better name(s)/placement for the module?


ACK. Some comments below, but I think they can be addressed later.

1. I'm not sure if we really need a HashCreator. Ideally the router 
should map a hash to a page. Links to another page can be hardcoded too 
(and substitute the parameters).


2. Ideally there should be no entity-specific navigation code. All 
routing should be handled in a generic way. This probably depends on the 
entity-facet separation that we discussed previously. So routes like this:


  /e/:entity/:facet/:pkeys/*args

should be replaced by individual routes for each entity:

  /user/:facet/:pkeys/*args
  /group/:facet/:pkeys/*args

Probably the entities should be written like plugins.

3. If we fix #1 and #2, is the "routing" module still necessary? The 
"routing" object probably should have been a "router", but are you 
concerned about conflicting with Dojo's router and the existing Router 
module?


4. The args/query in the navigation paths should be separated by a more 
standard delimiter "?" instead of "//". For example:


  /ipa/ui/#/e/user/search//filter=test

should be replaced with:

  /ipa/ui/#/e/user/search?filter=test

--
Endi S. Dewata

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

Re: [Freeipa-devel] [PATCH] [IMPORTANT] Make otptoken use os.urandom() for random data

On Fri, 2014-06-20 at 11:56 -0400, Nathaniel McCallum wrote:
> On Thu, 2014-06-19 at 12:43 -0400, Simo Sorce wrote:
> > On Thu, 2014-06-19 at 12:36 -0400, Nathaniel McCallum wrote:
> > > This also fixes an error where the default value was not respecting
> > > the KEY_LENGTH variable.
> > > 
> > > (NOTE: the os.urandom() change should not change the security properties
> > > of the existing code. However, the failure of the previous code to
> > > respect KEY_LENGTH causes us to violate the RFC.)
> > 
> > LGTM!
> > I do prefer using os.urandom() directly, as random.SystemRandom uses it
> > under the hood anyway.
> 
> Is that an ACK? Because we need to merge a fix of some kind soon.

If someone can actually test it I would prefer, as I did not, and I am
not sure I will find the time today, that's why I did not give a full
ACK.

Simo.


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

Re: [Freeipa-devel] [PATCH] [IMPORTANT] Make otptoken use os.urandom() for random data

On Thu, 2014-06-19 at 12:43 -0400, Simo Sorce wrote:
> On Thu, 2014-06-19 at 12:36 -0400, Nathaniel McCallum wrote:
> > This also fixes an error where the default value was not respecting
> > the KEY_LENGTH variable.
> > 
> > (NOTE: the os.urandom() change should not change the security properties
> > of the existing code. However, the failure of the previous code to
> > respect KEY_LENGTH causes us to violate the RFC.)
> 
> LGTM!
> I do prefer using os.urandom() directly, as random.SystemRandom uses it
> under the hood anyway.

Is that an ACK? Because we need to merge a fix of some kind soon.

Nathaniel

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

Re: [Freeipa-devel] #4389: DS deref broken after ACI refactoring

2014-06-20 Thread Jakub Hrozek

On Fri, Jun 20, 2014 at 04:45:45PM +0200, Martin Kosek wrote:
> On 06/20/2014 04:24 PM, Jakub Hrozek wrote:
> > On Fri, Jun 20, 2014 at 04:06:16PM +0200, Martin Kosek wrote:
> >> Hello all,
> >>
> >> I would like to discuss what should we do with the latest issue we found in
> >> SSSD-DS communication which is broken after the ACI refactoring.
> > 
> > It's not just SSSD-DS communication, any client, including ldapsearch
> > currently fails.
> 
> Sure.
> 
> >> I was working with Ludwig, there is a problem in the way how deref plugin
> >> checks the access to the referenced entry. Instead of checking the target 
> >> entry
> >> itself, Ludwig found out that the deref plugin checks a dummy entry created
> >> from the dereferenced DN, not the real entry. Details in DS ticket:
> >> https://fedorahosted.org/389/ticket/47821
> >>
> >> Previously, we allowed read access globally so it worked fine. Now, when we
> >> have targeted ACIs using objectclass targetfilter, the access control goes
> >> wrong, deref plugin does not return all attributes and SSSD does not work 
> >> (see
> >> 4389).
> >>
> >> Question is, what should we do in 4.0. We could have the DS team to fix the
> >> deref plugin, but this would break SSSD connected to old RHEL/CentOs 6.x
> >> replicas which would not have the fix. So we need to be cautious about 
> >> this one.
> > 
> > Why? SSSD Simply uses a client control. See
> > http://tools.ietf.org/html/draft-masarati-ldap-deref-00 for all the
> > details.
> 
> Because this bug affects all client machines enrolled in FreeIPA.
> 
> >> I see couple ways:
> >> 1) Fix DS deref plugin in F20 and next RHEL 6.x and specify the RHEL-6.x as
> >> minimal version of FreeIPA/DS required when replicating with FreeIPA 4.0. 
> >> This
> >> option is a bit clumsy.
> > 
> > The fact that this 'fix' seems to be backwards incompatible sounds
> > strange to me. How exactly did you intend to fix the plugin? If the fix
> > was about changing DS to check the target entry instead of some dummy
> > entry, then I see no impact on the client.
> 
> There is no impact on clients connected to the "fixed DS". This is the 
> scenario
> I am concerned about:
> 
> User has RHEL/CentOS 6.x IPA server and wants to try the new nice and shiny
> FreeIPA 4.0. He installs the FreeIPA 4.0 replica (with fixed DS), the replica
> upgrades the ACIs to the new model. SSSD connected to FreeIPA 4.0 replica will
> work, SSSD connected to the old RHEL-6.x replica will not.

Ah, I didn't realize the bug could propagate to released versions via
replicas, sorry.

> 
> >> 2) Add temporary ACIs allowing access to attributes that SSSD needs for 
> >> deref
> >> calls. I tested it with Jakub's example call and it fixed this query:
> >>
> >> # ipa permission-add --subtree
> >> cn=computers,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
> >> --right={read,search,compare} --attrs={objectclass,memberof,managedby}
> >> --bindtype all deref_managedby
> >> # kinit -kt /etc/krb5.keytab
> >>
> >> # ldapsearch -Y GSSAPI -h vm-236.idm.lab.eng.brq.redhat.com -b
> >> fqdn=vm-086.idm.lab.bos.redhat.com,cn=computers,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
> >> -E 'deref=managedBy:objectClass'
> >> ...
> >> dn: 
> >> fqdn=vm-086.idm.lab.bos.redhat.com,cn=computers,cn=accounts,dc=idm,dc=lab,
> >>  dc=eng,dc=brq,dc=redhat,dc=com
> >> control: 1.3.6.1.4.1.4203.666.5.16 false 
> >> MIQAAAEgMIQAAAEaBAltYW5hZ2VkQnkEaGZxZ
> >>  
> >> G49dm0tMDg2LmlkbS5sYWIuYm9zLnJlZGhhdC5jb20sY249Y29tcHV0ZXJzLGNuPWFjY291bnRzLG
> >>  
> >> RjPWlkbSxkYz1sYWIsZGM9ZW5nLGRjPWJycSxkYz1yZWRoYXQsZGM9Y29toIQAAACfMIQAAACZBAt
> >>  
> >> vYmplY3RDbGFzczGEhgQJaXBhb2JqZWN0BA1pZWVlODAyZGV2aWNlBAZuc2hvc3QECmlwYXNl
> >>  
> >> cnZpY2UEB3BraXVzZXIEB2lwYWhvc3QEDGtyYnByaW5jaXBhbAQPa3JicHJpbmNpcGFsYXV4BAppc
> >>  GFzc2hob3N0BAN0b3AEFGlwYVNzaEdyb3VwT2ZQdWJLZXlz
> >> # managedBy: 
> >> ;; >>  
> >> =nshost>;;; >>  
> >> >;;; >>  
> >> shhost>;;;fqdn=vm-086.idm
> >>  
> >> .lab.bos.redhat.com,cn=computers,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=
> >>  redhat,dc=com
> >>
> >> Jakub, what else would we need to allow? After this change, login/sudo 
> >> seemed
> >> to work for me on F20.
> > 
> > Are you asking about the list of attributes we dereference or the list
> > of attributes we read from the dereferenced entries?
> 
> Actually both, so that we know for what type of objects and what attributes we
> would need to allow access.

I think we should just make a note to self to allow users to fix the
ACIs manually should they run into the problem while being unable to
upgrade for whatever reason.

So we only seem to dereference member and memberof. We dereference either user
groups to get users, host groups to get hosts. For hosts we are
interested about these attributes:
"ipa_host_object_class"
"ipa_host_name"
"ipa_host_fqdn"
"ipa_host_serverhostname"
"ipa_host_member_of"
"ipa_host_ssh_public_key"
"ipa_host_uuid"

For users and groups, the list is long

Re: [Freeipa-devel] [PATCH 0058] Add the otptoken-add-yubikey command

On Thu, 2014-06-19 at 16:30 -0400, Nathaniel McCallum wrote:
> This command behaves almost exactly like otptoken-add except:
> 1. The new token data is written directly to a YubiKey
> 2. The vendor/model/serial fields are populated from the YubiKey
> 
> === NOTE ===
> 1. This patch depends on the new Fedora package: python-yubico. If you
> would like to help with the package review, please assign yourself here:
> https://bugzilla.redhat.com/show_bug.cgi?id=334

New version of the patch. This one works (yay!).

1. Because of the dependency on python-yubico, is this feature something
we want in core FreeIPA? As a subpackage? Separate project altogether?
The only dependency for python-yubico is pyusb.

2. Should the "import yubico" statement be inside of the
otptoken_add_yubikey.forward() method to reduce server dependencies?

3. This code currently emits a warning from the call to otptoken-add:
WARNING: API Version number was not sent, forward compatibility not
guaranteed. Assuming server's API version, 2.89

How do I fix this?

4. I am not sure why I have to delete the summary and value keys from
the return dictionary. It would be nice to display this summary message
just like otptoken-add.

5. Am I doing the ipatoken(vendor|model|serial) options correctly? These
aren't user settable, but we need to pass them from the yubikey
(client-side) to the server.

6. I'm not sure my use of assert or ValueError are correct. What should
I do here?

7. Considering that this is just a specialized invocation of
otptoken-add, can't we do this all on the client-side? This is why I had
originally used frontend.Local rather than frontend.Command.

Nathaniel
From 06ab253297b1428bcdd2a8c9a1a82532e828081a Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum 
Date: Thu, 19 Jun 2014 12:28:32 -0400
Subject: [PATCH] Add the otptoken-add-yubikey command

This command behaves almost exactly like otptoken-add except:
1. The new token data is written directly to a YubiKey
2. The vendor/model/serial fields are populated from the YubiKey
---
 freeipa.spec.in|   1 +
 ipalib/plugins/otptoken.py | 106 -
 2 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index b719b4a21fd2263a6fb58b3dffd4784868e630e9..4228c88f939a361a32da6f875c48db8adee3cdc1 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -158,6 +158,7 @@ Requires(pre): certmonger >= 0.65
 Requires(pre): 389-ds-base >= 1.3.2.11
 Requires: fontawesome-fonts
 Requires: open-sans-fonts
+Requires: python-yubico
 
 # With FreeIPA 3.3, package freeipa-server-selinux was obsoleted as the
 # entire SELinux policy is stored in the system policy
diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index d834d582a16d95ab08c3f1fe1aef29160c77ae23..7d5fff1665afe1ce48254ff1ecb5472e7d3b0b87 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -21,6 +21,7 @@ from ipalib.plugins.baseldap import DN, LDAPObject, LDAPAddMember, LDAPRemoveMem
 from ipalib.plugins.baseldap import LDAPCreate, LDAPDelete, LDAPUpdate, LDAPSearch, LDAPRetrieve
 from ipalib import api, Int, Str, Bool, Flag, Bytes, IntEnum, StrEnum, _, ngettext
 from ipalib.plugable import Registry
+from ipalib.frontend import Command
 from ipalib.errors import PasswordMismatch, ConversionError, LastMemberError, NotFound
 from ipalib.request import context
 import base64
@@ -28,6 +29,7 @@ import uuid
 import urllib
 import qrcode
 import os
+import yubico
 
 __doc__ = _("""
 OTP Tokens
@@ -196,7 +198,7 @@ class otptoken(LDAPObject):
 ),
 IntEnum('ipatokenotpdigits?',
 cli_name='digits',
-label=_('Display length'),
+label=_('Digits'),
 values=(6, 8),
 default=6,
 autofill=True,
@@ -383,3 +385,105 @@ class otptoken_remove_managedby(LDAPRemoveMember):
 __doc__ = _('Remove hosts that can manage this host.')
 
 member_attributes = ['managedby']
+
+@register()
+class otptoken_add_yubikey(Command):
+__doc__ = _('Add a new YubiKey OTP token.')
+
+takes_args = (
+Str('ipatokenuniqueid?',
+cli_name='id',
+label=_('Unique ID'),
+primary_key=True,
+),
+)
+
+takes_options = Command.takes_options + (
+IntEnum('slot?',
+cli_name='slot',
+label=_('YubiKey slot'),
+values=(1, 2),
+),
+Str('ipatokenvendor?',
+cli_name='vendor',
+label=_('Vendor'),
+flags=('no_option')
+),
+Str('ipatokenmodel?',
+cli_name='model',
+label=_('Model'),
+flags=('no_option')
+),
+Str('ipatokenserial?',
+cli_name='serial',
+label=_('Serial'),
+flags=('no_option')
+),
+) + tuple([x for x in otptoken.takes_params if x.name in (
+'description',
+'ipatokenowner',
+

Re: [Freeipa-devel] [PATCH] 659-666 Support of password reset with OTP


On 11.6.2014 15:19, Petr Vobornik wrote:

Patch set contains both API/server and Web UI parts.

[PATCH] 659 ldap2: add otp support to modify_password
[PATCH] 660 rpcserver: add otp support to change_password handler
[PATCH] 661 ipa-passwd: add OTP support
[PATCH] 662 webui: support password change with OTP in login screen
[PATCH] 663 webui: placeholder attribute support in textbox and textarea
[PATCH] 664 webui: add placeholders to login screen
[PATCH] 665 webui: rebase user password dialog on password dialog and
add otp support
[PATCH] 666 webui: support otp in reset_password.html

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


attaching rebased patches (mainly because of VERSION conflict)


--
Petr Vobornik
From 7722d49fdc782d14e5bcc3fbefaee70e5fff62a4 Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Thu, 22 May 2014 14:47:44 +0200
Subject: [PATCH] webui: placeholder attribute support in textbox and textarea

---
 install/ui/src/freeipa/widget.js | 8 
 1 file changed, 8 insertions(+)

diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js
index cfa9417c5ddbaf94b45aa35498d6076a55fac493..9677a168eb2a137ed1c42b9b2b5d62f43c3a735d 100644
--- a/install/ui/src/freeipa/widget.js
+++ b/install/ui/src/freeipa/widget.js
@@ -269,6 +269,12 @@ IPA.input_widget = function(spec) {
 var that = IPA.widget(spec);
 
 /**
+ * Placeholder
+ * @property {string}
+ */
+that.placeholder = text.get(spec.placeholder);
+
+/**
  * Widget's width.
  * @deprecated
  * @property {number}
@@ -709,6 +715,7 @@ IPA.text_widget = function(spec) {
 'class': 'form-control',
 size: that.size,
 title: that.tooltip,
+placeholder: that.placeholder,
 keyup: function() {
 that.on_value_changed();
 }
@@ -1975,6 +1982,7 @@ IPA.textarea_widget = function (spec) {
 'class': 'form-control',
 readOnly: !!that.read_only,
 title: that.tooltip,
+placeholder: that.placeholder,
 keyup: function() {
 that.on_value_changed();
 }
-- 
1.9.0

From 6b680e0092ca8518b6aebb974ee982139e85d266 Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Thu, 22 May 2014 14:48:22 +0200
Subject: [PATCH] webui: add placeholders to login screen

---
 install/ui/src/freeipa/widgets/LoginScreen.js | 8 +++-
 install/ui/test/data/ipa_init.json| 3 +++
 ipalib/plugins/internal.py| 3 +++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/install/ui/src/freeipa/widgets/LoginScreen.js b/install/ui/src/freeipa/widgets/LoginScreen.js
index 701c88cf12f07e7d7e4d4efc44d980f332656042..78991d0a93d6b9d3507d3afe31587ed1f3b55ed9 100644
--- a/install/ui/src/freeipa/widgets/LoginScreen.js
+++ b/install/ui/src/freeipa/widgets/LoginScreen.js
@@ -568,6 +568,7 @@ define(['dojo/_base/declare',
 $type: 'text',
 name: 'username',
 label: text.get('@i18n:login.username', "Username"),
+placeholder: text.get('@i18n:login.username', "Username"),
 show_errors: false,
 undo: false
 },
@@ -575,6 +576,7 @@ define(['dojo/_base/declare',
 $type: 'password',
 name: 'password',
 label: text.get('@i18n:login.password', "Password"),
+placeholder: text.get('@i18n:login.password_and_otp', 'Password or Password+One-Time-Password'),
 show_errors: false,
 undo: false
 },
@@ -589,13 +591,15 @@ define(['dojo/_base/declare',
 name: 'current_password',
 $type: 'password',
 label: text.get('@i18n:login.current_password', "Current Password"),
+placeholder: text.get('@i18n:login.current_password', "Current Password"),
 show_errors: false,
 undo: false
 },
 {
 name: 'otp',
 $type: 'password',
-label: text.get('@i18n:login.current_password', "OTP"),
+label: text.get('@i18n:password.otp', "OTP"),
+placeholder: text.get('@i18n:password.otp_long', 'One-Time-Password'),
 show_errors: false,
 undo: false
 },
@@ -604,6 +608,7 @@ define(['dojo/_base/declare',
 $type: 'password',
 required: true,
 label: text.get('@i18n:password.new_password)', "New Password"),
+placeholder: text.get('@i18n:password.new_password)', "New Password"),
 show_errors: false,
 undo: false
 },
@@ -612,6 +617,7 @@ define(['dojo/_base/declare',
 $type: 'password',
 required: true,
 label: text.get('@i18n:password.verify_password', "Verify Password"),
+placeholder: text.get('@i18n:password.new_password)', "New Password"),
 validators: [{
 $type: 'same_password',
 other_fiel

[Freeipa-devel] Design for new top level DN functionality in Dogtag

2014-06-20 Thread Ade Lee

Design at:
http://pki.fedoraproject.org/wiki/Top-Level_Tree

This is a feature to change the tree structure of the Dogtag internal
database so that a new top level baseDN is available.  This will
simplify the replication topology by allowing one to replicate all
subsystems in a tomcat instance with a single replication agreement,
instead of needing a separate replication agreement for each subsystem.

This is a feature that I plan to start implementing very shortly for
Dogtag 10.2  -- ie. within the next couple of weeks.  

There are implications both for IPA (and how Dogtag is deployed within
IPA) as well as implications for Dogtag.

Please take a look and provide comments.
Thanks, 
Ade

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

Re: [Freeipa-devel] #4389: DS deref broken after ACI refactoring

On Fri, 2014-06-20 at 16:45 +0200, Martin Kosek wrote:
> There is no impact on clients connected to the "fixed DS". This is the
> scenario
> I am concerned about:
> 
> User has RHEL/CentOS 6.x IPA server and wants to try the new nice and
> shiny FreeIPA 4.0. He installs the FreeIPA 4.0 replica (with fixed
> DS), the replica upgrades the ACIs to the new model. SSSD connected to
> FreeIPA 4.0 replica will work, SSSD connected to the old RHEL-6.x
> replica will not.

This is the only "issue", and I do not think we can/should jump through
many hoops here.

The best way IMO, is to fix DS in RHEL6, and make a release note that
before migrating to FreeIPA 4.0, you must make sure all replicas have an
updated DS version (list versions for all major distros we know about).

I do not think we should add any special detection code in 4.0, if the
admin fails to update DS on an older replica he has 2 choices:
1. update DS
2. decommission the old replica

Simo.

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

Re: [Freeipa-devel] #4389: DS deref broken after ACI refactoring

2014-06-20 Thread Ludwig Krispenz

On 06/20/2014 04:45 PM, Martin Kosek wrote:

On 06/20/2014 04:24 PM, Jakub Hrozek wrote:

On Fri, Jun 20, 2014 at 04:06:16PM +0200, Martin Kosek wrote:

Hello all,

I would like to discuss what should we do with the latest issue we found in
SSSD-DS communication which is broken after the ACI refactoring.

It's not just SSSD-DS communication, any client, including ldapsearch
currently fails.

Sure.

I was working with Ludwig, there is a problem in the way how deref plugin
checks the access to the referenced entry. Instead of checking the target entry
itself, Ludwig found out that the deref plugin checks a dummy entry created
from the dereferenced DN, not the real entry. Details in DS ticket:
https://fedorahosted.org/389/ticket/47821

Previously, we allowed read access globally so it worked fine. Now, when we
have targeted ACIs using objectclass targetfilter, the access control goes
wrong, deref plugin does not return all attributes and SSSD does not work (see
4389).

Question is, what should we do in 4.0. We could have the DS team to fix the
deref plugin, but this would break SSSD connected to old RHEL/CentOs 6.x
replicas which would not have the fix. So we need to be cautious about this one.

Why? SSSD Simply uses a client control. See
http://tools.ietf.org/html/draft-masarati-ldap-deref-00 for all the
details.

Because this bug affects all client machines enrolled in FreeIPA.

I see couple ways:
1) Fix DS deref plugin in F20 and next RHEL 6.x and specify the RHEL-6.x as
minimal version of FreeIPA/DS required when replicating with FreeIPA 4.0. This
option is a bit clumsy.

There is no impact on clients connected to the "fixed DS". This is the scenario
I am concerned about:

User has RHEL/CentOS 6.x IPA server and wants to try the new nice and shiny
FreeIPA 4.0. He installs the FreeIPA 4.0 replica (with fixed DS), the replica
upgrades the ACIs to the new model. SSSD connected to FreeIPA 4.0 replica will
work, SSSD connected to the old RHEL-6.x replica will not.

About the options, I think 1] is mandatory: fix DS
and then, in a mixed environment, we could either request to either
upgrade DS to a version including the fix or to add an aci the way you did.

2) Add temporary ACIs allowing access to attributes that SSSD needs for deref
calls. I tested it with Jakub's example call and it fixed this query:

# ipa permission-add --subtree
cn=computers,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
--right={read,search,compare} --attrs={objectclass,memberof,managedby}
--bindtype all deref_managedby
# kinit -kt /etc/krb5.keytab

# ldapsearch -Y GSSAPI -h vm-236.idm.lab.eng.brq.redhat.com -b
fqdn=vm-086.idm.lab.bos.redhat.com,cn=computers,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
-E 'deref=managedBy:objectClass'
...
dn: fqdn=vm-086.idm.lab.bos.redhat.com,cn=computers,cn=accounts,dc=idm,dc=lab,
dc=eng,dc=brq,dc=redhat,dc=com
control: 1.3.6.1.4.1.4203.666.5.16 false MIQAAAEgMIQAAAEaBAltYW5hZ2VkQnkEaGZxZ
G49dm0tMDg2LmlkbS5sYWIuYm9zLnJlZGhhdC5jb20sY249Y29tcHV0ZXJzLGNuPWFjY291bnRzLG
RjPWlkbSxkYz1sYWIsZGM9ZW5nLGRjPWJycSxkYz1yZWRoYXQsZGM9Y29toIQAAACfMIQAAACZBAt
vYmplY3RDbGFzczGEhgQJaXBhb2JqZWN0BA1pZWVlODAyZGV2aWNlBAZuc2hvc3QECmlwYXNl
cnZpY2UEB3BraXVzZXIEB2lwYWhvc3QEDGtyYnByaW5jaXBhbAQPa3JicHJpbmNpcGFsYXV4BAppc
GFzc2hob3N0BAN0b3AEFGlwYVNzaEdyb3VwT2ZQdWJLZXlz
# managedBy: ;;;fqdn=vm-086.idm
.lab.bos.redhat.com,cn=computers,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=
redhat,dc=com

Jakub, what else would we need to allow? After this change, login/sudo seemed
to work for me on F20.

Are you asking about the list of attributes we dereference or the list
of attributes we read from the dereferenced entries?

Actually both, so that we know for what type of objects and what attributes we
would need to allow access.

For IPA we only care about memberof, but keep in mind that attribute
maps in SSSD are configurable.

Hm, makes the option 2) even more challenging...

Martin

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

[Freeipa-devel] [PATCH] 0593 Allow anonymous read access to virtual operation entries (Re: Virtual operation ACIs)

2014-06-20 Thread Petr Viktorin


On 06/19/2014 02:13 PM, Martin Kosek wrote:

On 06/19/2014 12:52 PM, Petr Viktorin wrote:

I'll address the other issues separately.

On 06/18/2014 05:46 PM, Martin Kosek wrote:

3) I hit one issue when I open the Web UI host tab, I get "Insufficient access:
No such virtual command" error triggered by "cert-show" command.

We will need to add the permission "System: Read Virtual Operations" that Honza
is creating also to "Host Administrators" to fix that part.


I'm not familiar with Honza's effort, but that seems right.
I'm curious, why don't we just allow reading virtual operations by anybody? It
seems to me they're the same in every IPA installation, what's there to hide?


They are indeed the same. This is an old (very old) mean to check access when
ACI cannot be used. I admit it is a bit clumsy.

I agree that we should indeed allow reading the list of virtual operations as
the list can be retrieved from our git anyway. The virtual operations do not
even show list of it's members as permissions hold it, so it really should not
leak any sensitive information.


Anyway, I poked around in how it works now: for cert-show you need write access
to the objectClass of the "retrieve certificate" virt op entry. So that right
you can actually remove the "ipaVirtualOperation" objectClass.
Aand the new "Anonymous read access to containers" ACI has a
(!(objectclass=ipaVirtualOperation)) filter, so any user privileged for a virt
op can allow everyone see that virt op).
Shouldn't we base the check on some other attribute instead?

And curiously, for cert-find there is no virt op based access check.


I think we should eventually invent something better than current virtual
operations. For now (4.0), we should do something simple and straightforward.
The simplest thing to do is stick to the old behavior, i.e.:

1) Remove the (!(objectclass=ipaVirtualOperation)) part of the filter (should
improve performance, right?)
2) Remove the ipaVirtualOperation objectclass again from the virtual operations
as it would be useless after change 1)


Patch attached.

--
Petr³
From 0aaa4cb3e152b2a3c0b6728207d493075e786e4b Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Fri, 20 Jun 2014 16:21:35 +0200
Subject: [PATCH] Allow anonymous read access to virtual operation entries

These entries are the same in all IPA installations, so there's
no need to hide them.

Also remove the ipaVirtualOperation objectclass, since it is
no longer needed.
---
 install/share/60basev3.ldif  | 1 -
 install/updates/20-aci.update| 2 +-
 install/updates/40-delegation.update | 6 --
 3 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/install/share/60basev3.ldif b/install/share/60basev3.ldif
index 552045b63d9485ccd3685942b10c3f0e5b6105b6..8b92af247c742516c867a1f0666f4770cd4273d2 100644
--- a/install/share/60basev3.ldif
+++ b/install/share/60basev3.ldif
@@ -64,4 +64,3 @@ dn: cn=schema
 objectClasses: (2.16.840.1.113730.3.8.12.19 NAME 'ipaUserAuthTypeClass' SUP top AUXILIARY DESC 'Class for authentication methods definition' MAY ipaUserAuthType X-ORIGIN 'IPA v3')
 objectClasses: (2.16.840.1.113730.3.8.12.20 NAME 'ipaUser' AUXILIARY MUST ( uid ) MAY ( userClass ) X-ORIGIN 'IPA v3' )
 objectClasses: (2.16.840.1.113730.3.8.12.21 NAME 'ipaPermissionV2' DESC 'IPA Permission objectclass, version 2' SUP ipaPermission AUXILIARY MUST ( ipaPermBindRuleType $ ipaPermLocation ) MAY ( ipaPermDefaultAttr $ ipaPermIncludedAttr $ ipaPermExcludedAttr $ ipaPermRight $ ipaPermTargetFilter $ ipaPermTarget ) X-ORIGIN 'IPA v3' )
-objectClasses: (2.16.840.1.113730.3.8.12.23 NAME 'ipaVirtualOperation' DESC 'IPA Virtual operation objectclass' SUP top AUXILIARY MUST ( cn ) X-ORIGIN 'IPA v3' )
diff --git a/install/updates/20-aci.update b/install/updates/20-aci.update
index 42fca71f33cfa2e4f145ed2bfc6faf35d82ecc05..4eb5c737a9c794b01ca551228169fcf816a73eb3 100644
--- a/install/updates/20-aci.update
+++ b/install/updates/20-aci.update
@@ -23,7 +23,7 @@ dn: $SUFFIX
 
 # Read access to containers
 dn: $SUFFIX
-add:aci:'(targetfilter="(&(objectclass=nsContainer)(!(objectclass=krbPwdPolicy))(!(objectclass=ipaVirtualOperation)))")(target!="ldap:///cn=masters,cn=ipa,cn=etc,$SUFFIX";)(targetattr="objectclass || cn")(version 3.0; acl "Anonymous read access to containers"; allow(read, search, compare) userdn = "ldap:///anyone";;)'
+add:aci:'(targetfilter="(&(objectclass=nsContainer)(!(objectclass=krbPwdPolicy)))")(target!="ldap:///cn=masters,cn=ipa,cn=etc,$SUFFIX";)(targetattr="objectclass || cn")(version 3.0; acl "Anonymous read access to containers"; allow(read, search, compare) userdn = "ldap:///anyone";;)'
 
 dn: cn=replicas,cn=ipa,cn=etc,$SUFFIX
 add:aci:'(targetfilter="(objectclass=nsContainer)")(version 3.0; acl "Deny read access to replica configuration"; deny(read, search, compare) userdn = "ldap:///anyone";;)'
diff --git a/install/updates/40-delegation.update b/install/updates/40-delegation.update
index fdbed5ba5416cc5ada449801e049e050540730a1..7d65e9e19def7

Re: [Freeipa-devel] #4389: DS deref broken after ACI refactoring

On 06/20/2014 04:24 PM, Jakub Hrozek wrote:
> On Fri, Jun 20, 2014 at 04:06:16PM +0200, Martin Kosek wrote:
>> Hello all,
>>
>> I would like to discuss what should we do with the latest issue we found in
>> SSSD-DS communication which is broken after the ACI refactoring.
> 
> It's not just SSSD-DS communication, any client, including ldapsearch
> currently fails.

Sure.

>> I was working with Ludwig, there is a problem in the way how deref plugin
>> checks the access to the referenced entry. Instead of checking the target 
>> entry
>> itself, Ludwig found out that the deref plugin checks a dummy entry created
>> from the dereferenced DN, not the real entry. Details in DS ticket:
>> https://fedorahosted.org/389/ticket/47821
>>
>> Previously, we allowed read access globally so it worked fine. Now, when we
>> have targeted ACIs using objectclass targetfilter, the access control goes
>> wrong, deref plugin does not return all attributes and SSSD does not work 
>> (see
>> 4389).
>>
>> Question is, what should we do in 4.0. We could have the DS team to fix the
>> deref plugin, but this would break SSSD connected to old RHEL/CentOs 6.x
>> replicas which would not have the fix. So we need to be cautious about this 
>> one.
> 
> Why? SSSD Simply uses a client control. See
> http://tools.ietf.org/html/draft-masarati-ldap-deref-00 for all the
> details.

Because this bug affects all client machines enrolled in FreeIPA.

>> I see couple ways:
>> 1) Fix DS deref plugin in F20 and next RHEL 6.x and specify the RHEL-6.x as
>> minimal version of FreeIPA/DS required when replicating with FreeIPA 4.0. 
>> This
>> option is a bit clumsy.
> 
> The fact that this 'fix' seems to be backwards incompatible sounds
> strange to me. How exactly did you intend to fix the plugin? If the fix
> was about changing DS to check the target entry instead of some dummy
> entry, then I see no impact on the client.

There is no impact on clients connected to the "fixed DS". This is the scenario
I am concerned about:

User has RHEL/CentOS 6.x IPA server and wants to try the new nice and shiny
FreeIPA 4.0. He installs the FreeIPA 4.0 replica (with fixed DS), the replica
upgrades the ACIs to the new model. SSSD connected to FreeIPA 4.0 replica will
work, SSSD connected to the old RHEL-6.x replica will not.

>> 2) Add temporary ACIs allowing access to attributes that SSSD needs for deref
>> calls. I tested it with Jakub's example call and it fixed this query:
>>
>> # ipa permission-add --subtree
>> cn=computers,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
>> --right={read,search,compare} --attrs={objectclass,memberof,managedby}
>> --bindtype all deref_managedby
>> # kinit -kt /etc/krb5.keytab
>>
>> # ldapsearch -Y GSSAPI -h vm-236.idm.lab.eng.brq.redhat.com -b
>> fqdn=vm-086.idm.lab.bos.redhat.com,cn=computers,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
>> -E 'deref=managedBy:objectClass'
>> ...
>> dn: 
>> fqdn=vm-086.idm.lab.bos.redhat.com,cn=computers,cn=accounts,dc=idm,dc=lab,
>>  dc=eng,dc=brq,dc=redhat,dc=com
>> control: 1.3.6.1.4.1.4203.666.5.16 false 
>> MIQAAAEgMIQAAAEaBAltYW5hZ2VkQnkEaGZxZ
>>  
>> G49dm0tMDg2LmlkbS5sYWIuYm9zLnJlZGhhdC5jb20sY249Y29tcHV0ZXJzLGNuPWFjY291bnRzLG
>>  
>> RjPWlkbSxkYz1sYWIsZGM9ZW5nLGRjPWJycSxkYz1yZWRoYXQsZGM9Y29toIQAAACfMIQAAACZBAt
>>  
>> vYmplY3RDbGFzczGEhgQJaXBhb2JqZWN0BA1pZWVlODAyZGV2aWNlBAZuc2hvc3QECmlwYXNl
>>  
>> cnZpY2UEB3BraXVzZXIEB2lwYWhvc3QEDGtyYnByaW5jaXBhbAQPa3JicHJpbmNpcGFsYXV4BAppc
>>  GFzc2hob3N0BAN0b3AEFGlwYVNzaEdyb3VwT2ZQdWJLZXlz
>> # managedBy: ;;>  =nshost>;;;>  >;;;>  shhost>;;;fqdn=vm-086.idm
>>  .lab.bos.redhat.com,cn=computers,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=
>>  redhat,dc=com
>>
>> Jakub, what else would we need to allow? After this change, login/sudo seemed
>> to work for me on F20.
> 
> Are you asking about the list of attributes we dereference or the list
> of attributes we read from the dereferenced entries?

Actually both, so that we know for what type of objects and what attributes we
would need to allow access.

> For IPA we only care about memberof, but keep in mind that attribute
> maps in SSSD are configurable.

Hm, makes the option 2) even more challenging...

Martin

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

Re: [Freeipa-devel] [PATCHES 0072-0075] Add DLV record (Update DNSSEC attributes in LDAP schema)


On 20.6.2014 15:23, Martin Basti wrote:

Patches attached

Petr please review WebUI patch.



Patch 72: ACK
Patch 73: ACK
Patch 74: ACK
Patch 75: ACK

pushed to master:
* 7cdc4178b0fb0972a7aed3e0604a835fc45ac7a8 DNSSEC: DLVRecord type added
* ee6e634c28b7261930c8cee556c8ebef9a01603e DNSSEC: Test: DLV record
* 2229e89bbb2b89ad72e467de83f735b308a7bca1 Digest part in DLV/DS records 
allows only heaxadecimal characters

* 0eef37908c580f4550618244e661594138f7b382 DNSSEC: WebUI add DLV record type
--
Petr Vobornik

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

Re: [Freeipa-devel] #4389: DS deref broken after ACI refactoring

2014-06-20 Thread Ludwig Krispenz

On 06/20/2014 04:24 PM, Jakub Hrozek wrote:

On Fri, Jun 20, 2014 at 04:06:16PM +0200, Martin Kosek wrote:

Hello all,

I would like to discuss what should we do with the latest issue we found in
SSSD-DS communication which is broken after the ACI refactoring.

It's not just SSSD-DS communication, any client, including ldapsearch
currently fails.

Why? SSSD Simply uses a client control. See
http://tools.ietf.org/html/draft-masarati-ldap-deref-00 for all the
details.

I see couple ways:
1) Fix DS deref plugin in F20 and next RHEL 6.x and specify the RHEL-6.x as
minimal version of FreeIPA/DS required when replicating with FreeIPA 4.0. This
option is a bit clumsy.

The fact that this 'fix' seems to be backwards incompatible sounds
strange to me. How exactly did you intend to fix the plugin? If the fix
was about changing DS to check the target entry instead of some dummy
entry, then I see no impact on the client.
The DS fix will be in checking the target entry, but martins concern
seem to be that this fix will not be automatically present in all
deployments

2) Add temporary ACIs allowing access to attributes that SSSD needs for deref
calls. I tested it with Jakub's example call and it fixed this query:

Jakub, what else would we need to allow? After this change, login/sudo seemed
to work for me on F20.

Are you asking about the list of attributes we dereference or the list
of attributes we read from the dereferenced entries?

For IPA we only care about memberof, but keep in mind that attribute
maps in SSSD are configurable.

The ACIs would be removed when all our supported DS versions have the deref
plugin fixed.

--
Martin Kosek
Supervisor, Software Engineering - Identity Management Team
Red Hat Inc.

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

Re: [Freeipa-devel] #4389: DS deref broken after ACI refactoring

2014-06-20 Thread Jakub Hrozek

On Fri, Jun 20, 2014 at 04:06:16PM +0200, Martin Kosek wrote:
> Hello all,
> 
> I would like to discuss what should we do with the latest issue we found in
> SSSD-DS communication which is broken after the ACI refactoring.

It's not just SSSD-DS communication, any client, including ldapsearch
currently fails.

> 
> I was working with Ludwig, there is a problem in the way how deref plugin
> checks the access to the referenced entry. Instead of checking the target 
> entry
> itself, Ludwig found out that the deref plugin checks a dummy entry created
> from the dereferenced DN, not the real entry. Details in DS ticket:
> https://fedorahosted.org/389/ticket/47821
> 
> Previously, we allowed read access globally so it worked fine. Now, when we
> have targeted ACIs using objectclass targetfilter, the access control goes
> wrong, deref plugin does not return all attributes and SSSD does not work (see
> 4389).
> 
> Question is, what should we do in 4.0. We could have the DS team to fix the
> deref plugin, but this would break SSSD connected to old RHEL/CentOs 6.x
> replicas which would not have the fix. So we need to be cautious about this 
> one.

Why? SSSD Simply uses a client control. See
http://tools.ietf.org/html/draft-masarati-ldap-deref-00 for all the
details.

> 
> I see couple ways:
> 1) Fix DS deref plugin in F20 and next RHEL 6.x and specify the RHEL-6.x as
> minimal version of FreeIPA/DS required when replicating with FreeIPA 4.0. This
> option is a bit clumsy.

The fact that this 'fix' seems to be backwards incompatible sounds
strange to me. How exactly did you intend to fix the plugin? If the fix
was about changing DS to check the target entry instead of some dummy
entry, then I see no impact on the client.

> 
> 2) Add temporary ACIs allowing access to attributes that SSSD needs for deref
> calls. I tested it with Jakub's example call and it fixed this query:
> 
> # ipa permission-add --subtree
> cn=computers,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
> --right={read,search,compare} --attrs={objectclass,memberof,managedby}
> --bindtype all deref_managedby
> # kinit -kt /etc/krb5.keytab
> 
> # ldapsearch -Y GSSAPI -h vm-236.idm.lab.eng.brq.redhat.com -b
> fqdn=vm-086.idm.lab.bos.redhat.com,cn=computers,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
> -E 'deref=managedBy:objectClass'
> ...
> dn: fqdn=vm-086.idm.lab.bos.redhat.com,cn=computers,cn=accounts,dc=idm,dc=lab,
>  dc=eng,dc=brq,dc=redhat,dc=com
> control: 1.3.6.1.4.1.4203.666.5.16 false MIQAAAEgMIQAAAEaBAltYW5hZ2VkQnkEaGZxZ
>  G49dm0tMDg2LmlkbS5sYWIuYm9zLnJlZGhhdC5jb20sY249Y29tcHV0ZXJzLGNuPWFjY291bnRzLG
>  RjPWlkbSxkYz1sYWIsZGM9ZW5nLGRjPWJycSxkYz1yZWRoYXQsZGM9Y29toIQAAACfMIQAAACZBAt
>  vYmplY3RDbGFzczGEhgQJaXBhb2JqZWN0BA1pZWVlODAyZGV2aWNlBAZuc2hvc3QECmlwYXNl
>  cnZpY2UEB3BraXVzZXIEB2lwYWhvc3QEDGtyYnByaW5jaXBhbAQPa3JicHJpbmNpcGFsYXV4BAppc
>  GFzc2hob3N0BAN0b3AEFGlwYVNzaEdyb3VwT2ZQdWJLZXlz
> # managedBy: ;;  =nshost>;;;  >;;;  shhost>;;;fqdn=vm-086.idm
>  .lab.bos.redhat.com,cn=computers,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=
>  redhat,dc=com
> 
> Jakub, what else would we need to allow? After this change, login/sudo seemed
> to work for me on F20.

Are you asking about the list of attributes we dereference or the list
of attributes we read from the dereferenced entries?

For IPA we only care about memberof, but keep in mind that attribute
maps in SSSD are configurable.

> 
> The ACIs would be removed when all our supported DS versions have the deref
> plugin fixed.
> 
> -- 
> Martin Kosek 
> Supervisor, Software Engineering - Identity Management Team
> Red Hat Inc.

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

[Freeipa-devel] #4389: DS deref broken after ACI refactoring

Hello all,

I would like to discuss what should we do with the latest issue we found in
SSSD-DS communication which is broken after the ACI refactoring.

I was working with Ludwig, there is a problem in the way how deref plugin
checks the access to the referenced entry. Instead of checking the target entry
itself, Ludwig found out that the deref plugin checks a dummy entry created
from the dereferenced DN, not the real entry. Details in DS ticket:
https://fedorahosted.org/389/ticket/47821

Previously, we allowed read access globally so it worked fine. Now, when we
have targeted ACIs using objectclass targetfilter, the access control goes
wrong, deref plugin does not return all attributes and SSSD does not work (see
4389).

Question is, what should we do in 4.0. We could have the DS team to fix the
deref plugin, but this would break SSSD connected to old RHEL/CentOs 6.x
replicas which would not have the fix. So we need to be cautious about this one.

I see couple ways:
1) Fix DS deref plugin in F20 and next RHEL 6.x and specify the RHEL-6.x as
minimal version of FreeIPA/DS required when replicating with FreeIPA 4.0. This
option is a bit clumsy.

2) Add temporary ACIs allowing access to attributes that SSSD needs for deref
calls. I tested it with Jakub's example call and it fixed this query:

# ipa permission-add --subtree
cn=computers,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
--right={read,search,compare} --attrs={objectclass,memberof,managedby}
--bindtype all deref_managedby
# kinit -kt /etc/krb5.keytab

# ldapsearch -Y GSSAPI -h vm-236.idm.lab.eng.brq.redhat.com -b
fqdn=vm-086.idm.lab.bos.redhat.com,cn=computers,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
-E 'deref=managedBy:objectClass'
...
dn: fqdn=vm-086.idm.lab.bos.redhat.com,cn=computers,cn=accounts,dc=idm,dc=lab,
 dc=eng,dc=brq,dc=redhat,dc=com
control: 1.3.6.1.4.1.4203.666.5.16 false MIQAAAEgMIQAAAEaBAltYW5hZ2VkQnkEaGZxZ
 G49dm0tMDg2LmlkbS5sYWIuYm9zLnJlZGhhdC5jb20sY249Y29tcHV0ZXJzLGNuPWFjY291bnRzLG
 RjPWlkbSxkYz1sYWIsZGM9ZW5nLGRjPWJycSxkYz1yZWRoYXQsZGM9Y29toIQAAACfMIQAAACZBAt
 vYmplY3RDbGFzczGEhgQJaXBhb2JqZWN0BA1pZWVlODAyZGV2aWNlBAZuc2hvc3QECmlwYXNl
 cnZpY2UEB3BraXVzZXIEB2lwYWhvc3QEDGtyYnByaW5jaXBhbAQPa3JicHJpbmNpcGFsYXV4BAppc
 GFzc2hob3N0BAN0b3AEFGlwYVNzaEdyb3VwT2ZQdWJLZXlz
# managedBy: ;;;fqdn=vm-086.idm
 .lab.bos.redhat.com,cn=computers,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=
 redhat,dc=com

Jakub, what else would we need to allow? After this change, login/sudo seemed
to work for me on F20.

The ACIs would be removed when all our supported DS versions have the deref
plugin fixed.

-- 
Martin Kosek 
Supervisor, Software Engineering - Identity Management Team
Red Hat Inc.

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

Re: [Freeipa-devel] [PATCH] 0059-0063 Update DNSSEC attributes/record types


On 20.6.2014 15:30, Petr Vobornik wrote:

On 20.6.2014 14:35, Martin Basti wrote:

On Thu, 2014-06-19 at 18:37 +0200, Martin Basti wrote:

On Fri, 2014-06-13 at 09:55 +0200, Martin Basti wrote:

On Thu, 2014-06-12 at 16:20 +0200, Martin Basti wrote:

On Thu, 2014-06-12 at 13:17 +0200, Petr Vobornik wrote:

On 9.6.2014 17:28, Martin Basti wrote:

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

Petr please make the WebUI patch review (0062) :-)

Patches attached.



Patch #0059: LGTM

Patch #0060:

1. Please add `pattern_errmsg` to `salt` part of nsec3param.
Otherwise
you get general "Text does not match field pattern" error message
in Web UI.


I will add the message.


2. Could be in one if:
+if nsec3params is not None:
+if len(nsec3params) > 1:

Maybe I'm missing something. But why does the dns plugin code use
following all over the place:

  try:
  nsec3params = rrattrs['nsec3paramrecord']
  except KeyError:
  pass
  else:
  if nsec3params is not None:

instead of:

  nsec3params = rrattrs.get('nsec3paramrecord')
  if nsec3params is not None:

btw you use both patterns in the patch.

I will use shorten form, I wrote it in the same way as the other
code in
the block was written, that's why.



Patch #0061: ACK


Patch #0062:

3. Why are there the idnafsdbrec1 variables?


It was replace for NSEC records, which are not supported anymore.
Now I realize, there is wrong description, so the idnafsbrec1 variable
is not needed.
I will remove it.


4. related to ^^:
./ipatests/test_xmlrpc/test_dns_plugin.py:199:33: E231 missing
whitespace after ','


Patch #0063: LGTM
IDK if they work because I'm experiencing some weird issues with
xmlrpc_tests in general.

I had troubles only with permission tests, but all DNS test worked
fine for me.

Thank you for review.


Updated patches attached.



Rebased patches attached


Updated patch attached, fixed missing update permission



ACK


Pushed to master:
* 48865aed5f15ae94db664c4cebed125ef8f223cc DNSSEC: remove unsuported records
* 5b95be802c6aa12b9464813441f85eaee3e3e82b DNSSEC: added NSEC3PARAM 
record type
* 4d90d3d572caedfd8813ccef0fe44551aed80d2b DNSSEC: webui update DNSSEC 
attributes
* cbc64454b026993d3724c9640e6ec738f549fdcd Tests: remove unused records 
from tests
* 4c88fdd9046c682c4b2cdce760e4c5440f2d41de Tests: tests for NSEC3PARAM 
records

--
Petr Vobornik

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

[Freeipa-devel] [PATCH 0077] Add dnssecinlinesigning attribute to ACI

Required patches: mbasti-0060, mbasti-0073

Patch attached.
-- 
Martin^2 Basti
>From 749807eef26245caec535d1da2ffb48cd69e30a0 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Fri, 20 Jun 2014 15:11:57 +0200
Subject: [PATCH] Fix: add dnssecinlinesigning attribute to ACI

---
 ACI.txt|  4 ++--
 install/share/dns.ldif |  2 +-
 ipalib/plugins/dns.py  | 32 
 3 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/ACI.txt b/ACI.txt
index fef79653a7fe213d24ec888abd1b4779fc96e16d..9b2d19d8c7fed9922c8da349e145abcba01ebbb0 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -23,11 +23,11 @@ aci: (targetattr = "idnsallowsyncptr || idnsforwarders || idnsforwardpolicy || i
 dn: cn=System: Add DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example
 aci: (target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example";)(version 3.0;acl "permission:System: Add DNS Entries";allow (add) groupdn = "ldap:///cn=System: Add DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=System: Read DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example
-aci: (targetattr = "a6record || record || afsdbrecord || arecord || certrecord || cn || cnamerecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || objectclass || ptrrecord || rrsigrecord || sigrecord || srvrecord || sshfprecord || txtrecord")(target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example";)(version 3.0;acl "permission:System: Read DNS Entries";allow (compare,read,search) groupdn = "ldap:///cn=System: Read DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+aci: (targetattr = "a6record || record || afsdbrecord || arecord || certrecord || cn || cnamerecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssecinlinesigning || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || objectclass || ptrrecord || rrsigrecord || sigrecord || srvrecord || sshfprecord || txtrecord")(target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example";)(version 3.0;acl "permission:System: Read DNS Entries";allow (compare,read,search) groupdn = "ldap:///cn=System: Read DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=System: Remove DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example
 aci: (target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example";)(version 3.0;acl "permission:System: Remove DNS Entries";allow (delete) groupdn = "ldap:///cn=System: Remove DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=System: Update DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example
-aci: (targetattr = "a6record || record || afsdbrecord || arecord || certrecord || cn || cnamerecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || ptrrecord || rrsigrecord || sigrecord || srvrecord || sshfprecord || txtrecord")(target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example";)(version 3.0;acl "permission:System: Update DNS Entries";allow (write) groupdn = "ldap:///cn=System: Update DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+aci: (targetattr = "a6record || record || afsdbrecord || arecord || certrecord || cn || cnamerecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssecinlinesigning || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxt

Re: [Freeipa-devel] [PATCH] 0059-0063 Update DNSSEC attributes/record types


On 20.6.2014 14:35, Martin Basti wrote:

On Thu, 2014-06-19 at 18:37 +0200, Martin Basti wrote:

On Fri, 2014-06-13 at 09:55 +0200, Martin Basti wrote:

On Thu, 2014-06-12 at 16:20 +0200, Martin Basti wrote:

On Thu, 2014-06-12 at 13:17 +0200, Petr Vobornik wrote:

On 9.6.2014 17:28, Martin Basti wrote:

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

Petr please make the WebUI patch review (0062) :-)

Patches attached.



Patch #0059: LGTM

Patch #0060:

1. Please add `pattern_errmsg` to `salt` part of nsec3param. Otherwise
you get general "Text does not match field pattern" error message in Web UI.


I will add the message.


2. Could be in one if:
+if nsec3params is not None:
+if len(nsec3params) > 1:

Maybe I'm missing something. But why does the dns plugin code use
following all over the place:

  try:
  nsec3params = rrattrs['nsec3paramrecord']
  except KeyError:
  pass
  else:
  if nsec3params is not None:

instead of:

  nsec3params = rrattrs.get('nsec3paramrecord')
  if nsec3params is not None:

btw you use both patterns in the patch.

I will use shorten form, I wrote it in the same way as the other code in
the block was written, that's why.



Patch #0061: ACK


Patch #0062:

3. Why are there the idnafsdbrec1 variables?


It was replace for NSEC records, which are not supported anymore.
Now I realize, there is wrong description, so the idnafsbrec1 variable
is not needed.
I will remove it.


4. related to ^^:
./ipatests/test_xmlrpc/test_dns_plugin.py:199:33: E231 missing
whitespace after ','


Patch #0063: LGTM
IDK if they work because I'm experiencing some weird issues with
xmlrpc_tests in general.

I had troubles only with permission tests, but all DNS test worked fine for me.

Thank you for review.


Updated patches attached.



Rebased patches attached


Updated patch attached, fixed missing update permission



ACK
--
Petr Vobornik

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

[Freeipa-devel] [PATCH 0076] Fix incompatible DNS permission

Patch attached.

Ticket:https://fedorahosted.org/freeipa/ticket/4383
-- 
Martin^2 Basti
>From a01f6f623e7cf9261fa0029f271f8a310812f895 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Fri, 20 Jun 2014 13:52:12 +0200
Subject: [PATCH] Fix incompatible DNS permission

dns(forward)zone-add/remove-permission can work with permissions with
relative zone name

Ticket:https://fedorahosted.org/freeipa/ticket/4383
---
 ipalib/plugins/dns.py | 30 +-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index a81fb575b4af8f8a7df577c6a6bf230056f6c660..4614fb49481b0caba06255d55eb6fdfa7e44cc5b 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -1876,6 +1876,22 @@ class DNSZoneBase_add_permission(LDAPQuery):
 self.obj.handle_not_found(*keys)
 
 permission_name = self.obj.permission_name(keys[-1])
+
+# compatibility with older IPA versions which allows relative zonenames
+permission_name_rel = self.obj.permission_name(
+keys[-1].relativize(DNSName.root)
+)
+try:
+api.Command['permission_show'](permission_name_rel)
+except errors.NotFound:
+pass
+else:
+# permission exists without absolute domain name
+raise errors.DuplicateEntry(
+message=_('permission "%(value)s" already exists' %
+  {'value': permission_name})
+)
+
 permission = api.Command['permission_add_noaci'](permission_name,
  ipapermissiontype=u'SYSTEM'
  )['result']
@@ -1922,7 +1938,19 @@ class DNSZoneBase_remove_permission(LDAPQuery):
 pass
 
 permission_name = self.obj.permission_name(keys[-1])
-api.Command['permission_del'](permission_name, force=True)
+try:
+api.Command['permission_del'](permission_name, force=True)
+except errors.NotFound, e:
+# compatibility, older IPA versions which allows to create zone
+# without absolute zone name
+permission_name_rel = self.obj.permission_name(
+keys[-1].relativize(DNSName.root)
+)
+try:
+api.Command['permission_del'](permission_name_rel, force=True)
+except errors.NotFound:
+raise e  # re-raise original exception
+
 
 return dict(
 result=True,
-- 
1.8.3.1

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

[Freeipa-devel] [PATCHES 0072-0075] Add DLV record (Update DNSSEC attributes in LDAP schema)

Patches attached

Petr please review WebUI patch.
-- 
Martin^2 Basti
>From 5492f997702d8b773cd1675a320a79371f5e5b19 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 17 Jun 2014 17:04:46 +0200
Subject: [PATCH 1/4] DNSSEC: DLVRecord type added

Ticket: https://fedorahosted.org/freeipa/ticket/4328
---
 ACI.txt |  4 ++--
 API.txt | 12 ++--
 VERSION |  4 ++--
 install/share/60ipadns.ldif |  3 ++-
 install/share/dns.ldif  |  2 +-
 ipalib/plugins/dns.py   | 32 +---
 6 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/ACI.txt b/ACI.txt
index d3bcef17ea83c9ec6234a741bd9c6294b845a322..fef79653a7fe213d24ec888abd1b4779fc96e16d 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -23,11 +23,11 @@ aci: (targetattr = "idnsallowsyncptr || idnsforwarders || idnsforwardpolicy || i
 dn: cn=System: Add DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example
 aci: (target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example";)(version 3.0;acl "permission:System: Add DNS Entries";allow (add) groupdn = "ldap:///cn=System: Add DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=System: Read DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example
-aci: (targetattr = "a6record || record || afsdbrecord || arecord || certrecord || cn || cnamerecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || objectclass || ptrrecord || rrsigrecord || sigrecord || srvrecord || sshfprecord || txtrecord")(target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example";)(version 3.0;acl "permission:System: Read DNS Entries";allow (compare,read,search) groupdn = "ldap:///cn=System: Read DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+aci: (targetattr = "a6record || record || afsdbrecord || arecord || certrecord || cn || cnamerecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || objectclass || ptrrecord || rrsigrecord || sigrecord || srvrecord || sshfprecord || txtrecord")(target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example";)(version 3.0;acl "permission:System: Read DNS Entries";allow (compare,read,search) groupdn = "ldap:///cn=System: Read DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=System: Remove DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example
 aci: (target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example";)(version 3.0;acl "permission:System: Remove DNS Entries";allow (delete) groupdn = "ldap:///cn=System: Remove DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=System: Update DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example
-aci: (targetattr = "a6record || record || afsdbrecord || arecord || certrecord || cn || cnamerecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || ptrrecord || rrsigrecord || sigrecord || srvrecord || sshfprecord || txtrecord")(target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example";)(version 3.0;acl "permission:System: Update DNS Entries";allow (write) groupdn = "ldap:///cn=System: Update DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+aci: (targetattr = "a6record || record || afsdbrecord || arecord || certrecord || cn || cnamerecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || keyrecord || kxrecord || locrecord || managedby || mdrecord

Re: [Freeipa-devel] [PATCH] 0059-0063 Update DNSSEC attributes/record types

On Thu, 2014-06-19 at 18:37 +0200, Martin Basti wrote:
> On Fri, 2014-06-13 at 09:55 +0200, Martin Basti wrote:
> > On Thu, 2014-06-12 at 16:20 +0200, Martin Basti wrote:
> > > On Thu, 2014-06-12 at 13:17 +0200, Petr Vobornik wrote:
> > > > On 9.6.2014 17:28, Martin Basti wrote:
> > > > > Ticket: https://fedorahosted.org/freeipa/ticket/4328
> > > > >
> > > > > Petr please make the WebUI patch review (0062) :-)
> > > > >
> > > > > Patches attached.
> > > > >
> > > > 
> > > > Patch #0059: LGTM
> > > > 
> > > > Patch #0060:
> > > > 
> > > > 1. Please add `pattern_errmsg` to `salt` part of nsec3param. Otherwise 
> > > > you get general "Text does not match field pattern" error message in 
> > > > Web UI.
> > > > 
> > > I will add the message.
> > > 
> > > > 2. Could be in one if:
> > > > +if nsec3params is not None:
> > > > +if len(nsec3params) > 1:
> > > > 
> > > > Maybe I'm missing something. But why does the dns plugin code use 
> > > > following all over the place:
> > > > 
> > > >  try:
> > > >  nsec3params = rrattrs['nsec3paramrecord']
> > > >  except KeyError:
> > > >  pass
> > > >  else:
> > > >  if nsec3params is not None:
> > > > 
> > > > instead of:
> > > > 
> > > >  nsec3params = rrattrs.get('nsec3paramrecord')
> > > >  if nsec3params is not None:
> > > > 
> > > > btw you use both patterns in the patch.
> > > I will use shorten form, I wrote it in the same way as the other code in
> > > the block was written, that's why.
> > > 
> > > > 
> > > > Patch #0061: ACK
> > > > 
> > > > 
> > > > Patch #0062:
> > > > 
> > > > 3. Why are there the idnafsdbrec1 variables?
> > > > 
> > > It was replace for NSEC records, which are not supported anymore.
> > > Now I realize, there is wrong description, so the idnafsbrec1 variable
> > > is not needed.
> > > I will remove it.
> > > 
> > > > 4. related to ^^:
> > > > ./ipatests/test_xmlrpc/test_dns_plugin.py:199:33: E231 missing 
> > > > whitespace after ','
> > > > 
> > > > 
> > > > Patch #0063: LGTM
> > > > IDK if they work because I'm experiencing some weird issues with 
> > > > xmlrpc_tests in general.
> > > I had troubles only with permission tests, but all DNS test worked fine 
> > > for me.
> > > 
> > > Thank you for review.
> > > 
> > Updated patches attached.
> > 
> > ___
> > Freeipa-devel mailing list
> > Freeipa-devel@redhat.com
> > https://www.redhat.com/mailman/listinfo/freeipa-devel
> 
> Rebased patches attached
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

Updated patch attached, fixed missing update permission
-- 
Martin^2 Basti
>From 87ee296b5f90d35605df65f96e6274e89f864d22 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 3 Jun 2014 11:21:10 +0200
Subject: [PATCH 2/5] DNSSEC: added NSEC3PARAM record type

Ticket: https://fedorahosted.org/freeipa/ticket/4328
---
 ACI.txt |  4 ++--
 API.txt | 12 --
 VERSION |  4 ++--
 install/share/60ipadns.ldif |  3 ++-
 install/share/dns.ldif  |  2 +-
 ipalib/plugins/dns.py   | 54 -
 6 files changed, 66 insertions(+), 13 deletions(-)

diff --git a/ACI.txt b/ACI.txt
index cd19fb90fdd0ac1947393aabfd667e46a6f015fc..d3bcef17ea83c9ec6234a741bd9c6294b845a322 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -23,11 +23,11 @@ aci: (targetattr = "idnsallowsyncptr || idnsforwarders || idnsforwardpolicy || i
 dn: cn=System: Add DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example
 aci: (target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example";)(version 3.0;acl "permission:System: Add DNS Entries";allow (add) groupdn = "ldap:///cn=System: Add DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=System: Read DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example
-aci: (targetattr = "a6record || record || afsdbrecord || arecord || certrecord || cn || cnamerecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || mxrecord || naptrrecord || nsecrecord || nsrecord || nxtrecord || objectclass || ptrrecord || rrsigrecord || sigrecord || srvrecord || sshfprecord || txtrecord")(target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example";)(version 3.0;acl "permission:System: Read DNS Entries";allow (compare,read,search) groupdn = "ldap:///cn=System: Read DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+aci: (targetattr = "a6record || record || afsdbrecord || arecord || certrecord

[Freeipa-devel] [PATCHES] 0591-0593 Change group permission object filter

2014-06-20 Thread Petr Viktorin

My patch 0580 was wrong; non-POSIX groups obviously lack the posixgroup 
objectclass. Actually the only objectclasses that all groups share are 
top and ipaobject.


This makes permission plugin & updater join multiple 
permission_filter_objectclasses filters with OR, and changes the --type 
group filter to (|(objectclass=ipausergroup)(objectclass=posixgroup)).


The first patch has some unrelated test fixes.


--
Petr³

From 4555ed124fc24ad4c84e74795de170c0a0f4eb57 Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Thu, 19 Jun 2014 19:06:19 +0200
Subject: [PATCH] Test and docstring fixes

The recent conversions to managed permissions left behind a few
failing tests. Fix them.

Also fix a now incorrect docstring in ipalib.config.
---
 ipalib/config.py   |  6 +-
 ipatests/test_xmlrpc/test_old_permission_plugin.py | 14 --
 ipatests/test_xmlrpc/test_permission_plugin.py | 14 --
 ipatests/test_xmlrpc/test_realmdomains_plugin.py   |  2 +-
 4 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/ipalib/config.py b/ipalib/config.py
index b12cfd32184edf964353fda9304dbb5149eb525f..e8958205dbe12e1bae1ac24351d46901a536f3e7 100644
--- a/ipalib/config.py
+++ b/ipalib/config.py
@@ -105,15 +105,11 @@ class Env(object):
 u'false'
 
 If an ``str`` value looks like an integer, it's automatically converted to
-the ``int`` type.  Likewise, if an ``str`` value looks like a floating-point
-number, it's automatically converted to the ``float`` type.  For example:
+the ``int`` type.
 
 >>> env.lucky = '7'
 >>> env.lucky
 7
->>> env.three_halves = '1.5'
->>> env.three_halves
-1.5
 
 Leading and trailing white-space is automatically stripped from ``str``
 values.  For example:
diff --git a/ipatests/test_xmlrpc/test_old_permission_plugin.py b/ipatests/test_xmlrpc/test_old_permission_plugin.py
index 9de397d67979bf54f808b0b61628ee444087af4b..e69c4f824981d5d0f7b3beecdbc5df098e764b3b 100644
--- a/ipatests/test_xmlrpc/test_old_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_old_permission_plugin.py
@@ -466,9 +466,9 @@ class test_old_permission(Declarative):
 summary=u'2 permissions matched',
 result=[
 {
-'dn': DN(('cn','Modify Group Password Policy'),
+'dn': DN(('cn', 'System: Modify Group Password Policy'),
  api.env.container_permission, api.env.basedn),
-'cn': [u'Modify Group Password Policy'],
+'cn': [u'System: Modify Group Password Policy'],
 },
 {
 'dn': DN(('cn', 'System: Read Group Password Policy'),
@@ -796,10 +796,10 @@ class test_old_permission(Declarative):
 summary=u'1 permission matched',
 result=[
 {
-'dn': DN(('cn','Add user to default group'),
+'dn': DN(('cn', 'System: Add User to default group'),
  api.env.container_permission, api.env.basedn),
-'cn': [u'Add user to default group'],
-'objectclass': objectclasses.system_permission,
+'cn': [u'System: Add User to default group'],
+'objectclass': objectclasses.permission,
 'member_privilege': [u'User Administrators'],
 'attrs': [u'member'],
 'targetgroup': u'ipausers',
@@ -807,7 +807,9 @@ class test_old_permission(Declarative):
 'permissions': [u'write'],
 'ipapermbindruletype': [u'permission'],
 'ipapermtarget': [DN('cn=ipausers', groups_dn)],
-'subtree': u'ldap:///%s' % api.env.basedn,
+'subtree': u'ldap:///%s' % groups_dn,
+'ipapermdefaultattr': [u'member'],
+'ipapermissiontype': [u'V2', u'MANAGED', u'SYSTEM'],
 }
 ],
 ),
diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
index a37876bea5c2d666548f7efa94f32ddbf044c0b4..feffc2eb1285d173d63a07bb42df8e64f816b618 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -758,9 +758,9 @@ class test_permission(Declarative):
 summary=u'2 permissions matched',
 result=[
 {
-'dn': DN(('cn','Modify Group Password Policy'),
+'dn': DN(('cn', 'System: Modify Group Password Policy'),
  api.env.container_permission, api.env.basedn),
-'cn': [u'Modify Group Password Policy'],
+

Re: [Freeipa-devel] [PATCH 0071] Fix - handle python-dns UnicodeError

On 06/20/2014 01:28 PM, Jan Cholasta wrote:
> On 20.6.2014 13:06, Martin Basti wrote:
>> Patch attached
>>
> 
> ACK.
> 

Pushed to master: 9f5e77f686a974b837da6eb92cec741fcbb33603

Martin

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

Re: [Freeipa-devel] [PATCH 0071] Fix - handle python-dns UnicodeError

2014-06-20 Thread Jan Cholasta


On 20.6.2014 13:06, Martin Basti wrote:

Patch attached



ACK.

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCHES] 0052-0055 Separate master and forward DNS zones to separate objectClasses


On 19.6.2014 16:55, Martin Basti wrote:

On Thu, 2014-06-19 at 15:16 +0200, Petr Vobornik wrote:

On 18.6.2014 13:42, Martin Basti wrote:

Rebased patches with pep8 fixes attached


git diff HEAD~4 -U0 | pep8 --diff --ignore=E501,E126,E128,E124
./ipalib/plugins/dns.py:1754:9: E265 block comment should start with '# '
./ipalib/plugins/dns.py:1755:9: E265 block comment should start with '# '
./ipalib/plugins/dns.py:2550:13: E265 block comment should start with '# '
./ipalib/plugins/dns.py:2551:16: E713 test for membership should be 'not in'
./ipalib/plugins/dns.py:3516:9: E265 block comment should start with '# '
./ipalib/plugins/dns.py:3686:12: E713 test for membership should be 'not in'

Thanks, I had an older pep8 version which didn't show me that


I don't like how the patches are structured. It's hard to review. IMHO
you should have started with creation of the base class and then build
the forward zone on top of it. Reading a bunch of copy-pasted code with
minor changes which is then removed in the next patch is a waste of
time.  Current approach is acceptable if the patch set is huge and
rebases would be really difficult. Or if it's sent and reviewed
separately. But what's done is done.

Sorry for that.



1. Is it possible to disable the interactive wizard for dnsrecord-add
forward.zone.? It would be nice to show the error right at the beginning.

It is partially possible. It requires to change interactive wizard for
dnsrecord-mod, dnsrecord-del too.
I vote for don't change it.


Patch 54-5:
2. You forgot to remove the 'execute' method from 'dnszone_disable' and
'dnszone_enable'

3. 'pre_callback' in 'dnsforwardzone_find' seems to be redundant, it
just calls the parent's pre_callback with no additional logic

Thank you, I will remove it


General question:

Looking at the help text, especially the `--name=DNSNAMEPARAM`, I wonder
if have somewhere documented the formats of various param types.

I dont know, I found nothing.
I'm thinking about, rename it to DNSNAME only

Rebased and fixed patches attached
Thank you.



ACK, pushed to master:

* 49068ade92b3fa4874b3107923bbd5b84e1a04f3 Separate master and forward 
DNS zones
* 266015c3e28c04894cd3a52ea2f99c25eef2c6a9 Prevent commands to modify 
different type of a zone

* 727f5f33732df252fa99d5c168d6727589ee6076 Create BASE zone class
* 11c250a6125da300f85880e090e5db1659078daa Tests DNS: forward zones


--
Petr Vobornik

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

[Freeipa-devel] [PATCH 0071] Fix - handle python-dns UnicodeError

Patch attached
-- 
Martin^2 Basti
>From a28ead1232de4cf84c31e942ed2be1ed4ab4a3b3 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Fri, 20 Jun 2014 12:53:06 +0200
Subject: [PATCH] Fix handle python-dns UnicodeError

---
 ipapython/dnsutil.py | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/ipapython/dnsutil.py b/ipapython/dnsutil.py
index 9c91578a8ccbd5cd30d959e02f23374c47da3fde..3602d22cd1585dd9d6e6546695cb05857a77e865 100644
--- a/ipapython/dnsutil.py
+++ b/ipapython/dnsutil.py
@@ -18,6 +18,7 @@
 #
 
 import dns.name
+import dns.exception
 import copy
 
 
@@ -35,10 +36,10 @@ class DNSName(dns.name.Name):
 labels = labels.labels
 try:
 super(DNSName, self).__init__(labels)
-except UnicodeError:
-#dnspython bug, punycoded label longer than 63 returns UnicodeError
-#instead of LabelTooLong
-raise dns.name.LabelTooLong()
+except UnicodeError, e:
+# dnspython bug, an invalid domain name returns the UnicodeError
+# instead of a dns.exception
+raise dns.exception.SyntaxError(e)
 
 def __nonzero__(self):
 #dns.name.from_text('@') is represented like empty tuple
-- 
1.8.3.1

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

Re: [Freeipa-devel] [PATCH 0019] Clarify LDAPClient docstrings about get_entry, get_entries and find_entrie

On 06/20/2014 11:06 AM, Martin Basti wrote:
> On Wed, 2014-06-18 at 17:36 +0200, Petr Spacek wrote:
>> Hello,
>>
>> Clarify LDAPClient docstrings about get_entry, get_entries and find_entries.
>>
>>
>> BTW what is the purpose of size_limit in LDAPClient.get_entry()?
>>
>> def get_entry(self, dn, attrs_list=None, time_limit=None,
>>size_limit=None)
>>
> I dont know, IMHO LDAP should return only one or no entry.
> 
> ACK
> 

Pushed to master: e821576129907d89a812edde45121d365a1f696d

Martin

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

Re: [Freeipa-devel] [PATCH 0019] Clarify LDAPClient docstrings about get_entry, get_entries and find_entrie

2014-06-20 Thread thierry bordaz


On 06/20/2014 11:06 AM, Martin Basti wrote:

On Wed, 2014-06-18 at 17:36 +0200, Petr Spacek wrote:

Hello,

Clarify LDAPClient docstrings about get_entry, get_entries and find_entries.


BTW what is the purpose of size_limit in LDAPClient.get_entry()?

def get_entry(self, dn, attrs_list=None, time_limit=None,
size_limit=None)


I dont know, IMHO LDAP should return only one or no entry.
I agree get_entry is a base search so setting a size limit (max entres 
to get) does not bring any value.


ACK


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

Re: [Freeipa-devel] [PATCH 0019] Clarify LDAPClient docstrings about get_entry, get_entries and find_entrie

On Wed, 2014-06-18 at 17:36 +0200, Petr Spacek wrote:
> Hello,
> 
> Clarify LDAPClient docstrings about get_entry, get_entries and find_entries.
> 
> 
> BTW what is the purpose of size_limit in LDAPClient.get_entry()?
> 
> def get_entry(self, dn, attrs_list=None, time_limit=None,
>size_limit=None)
> 
I dont know, IMHO LDAP should return only one or no entry.

ACK
-- 
Martin^2 Basti

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

Re: [Freeipa-devel] [PATCH 0058] Add the otptoken-add-yubikey command

2014-06-20 Thread Jan Cholasta


Hi,

On 19.6.2014 22:30, Nathaniel McCallum wrote:

This command behaves almost exactly like otptoken-add except:
1. The new token data is written directly to a YubiKey
2. The vendor/model/serial fields are populated from the YubiKey

=== NOTE ===
1. This patch depends on the new Fedora package: python-yubico. If you
would like to help with the package review, please assign yourself here:
https://bugzilla.redhat.com/show_bug.cgi?id=334

2. This patch doesn't actually work and I could use some help. The call
to api.Command.otptoken_add() fails with:
ipa: ERROR: non-public: AttributeError: no context.rpcclient in thread
'MainThread'
Traceback (most recent call last):
   File "/usr/lib/python2.7/site-packages/ipalib/backend.py", line 129,
in execute
 result = self.Command[_name](*args, **options)
   File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 439,
in __call__
 ret = self.run(*args, **options)
   File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 1118,
in run
 return self.forward(*args, **options)
   File "/usr/lib/python2.7/site-packages/ipalib/plugins/otptoken.py",
line 471, in forward
 **options)
   File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 439,
in __call__
 ret = self.run(*args, **options)
   File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 755,
in run
 return self.forward(*args, **options)
   File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 776,
in forward
 return self.Backend.rpcclient.forward(self.name, *args, **kw)
   File "/usr/lib/python2.7/site-packages/ipalib/rpc.py", line 874, in
forward
 command = getattr(self.conn, name)
   File "/usr/lib/python2.7/site-packages/ipalib/backend.py", line 97, in
__get_conn
 self.id, threading.currentThread().getName())
AttributeError: no context.rpcclient in thread 'MainThread'
ipa: ERROR: an internal error has occurred



This happens because when you use frontend.Local, no connection to the 
server is created (see cli.run()). Instead of using frontend.Local and 
overriding forward(), use frontend.Command and override execute().


Honza

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH 0070] Normalization check only for IDNA domains