Re: [Freeipa-devel] Required descriptions

2014-09-16 Thread Martin Kosek
On 09/15/2014 05:37 PM, Petr Vobornik wrote:
 On 15.9.2014 17:21, Tomas Babej wrote:
 Hi folks,

 while developing parts of the upcoming views feature
 (http://www.freeipa.org/page/V4/Migrating_existing_environments_to_Trust),
 we stumbled upon the question of having descriptions required by the
 framework.

 There are arguments for the description being required, at least for
 overriding attributes of IPA users. However, nothing stops irresponsible
 admins from entering descriptions like 'foo'.

 There is related ticket in the Trac:

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

 I'd like to avoid having this argument over again. Can we establish a
 guideline we wish to follow? Having tickets like #4387 and requiring
 descriptions in new features is too inconsistent for my taste :) we
 should either:

 1.) Define a clear line - when it makes sense to require description and
 when not.
 2.) Decide never to require description, since it is a non-enforcible
 requirement (nothing stops you from entering meaningless description).

 
 Description is a helper tool for users and it's not required for any
 functionality. Ideally user (company policy) should choose whether it should 
 be
 required. We should only give recommendations, e.g., in documentation.
 
 Making it configurable seems like a lot of effort with little added value.
 
 I'm for #2.
 
 Btw, idview plugin is inconsistent by itself atm - overrides have it required
 but idview doesn't.
 
 my 2c

+1, please don't require description unless it is really required for the
actual functionality. As already discussed wrt #4387, we cannot force admins to
enter something sensible in the description, so let us not punish those who do
not want to document it and make adding views, SUDO rules, ... easier.

Adding description to MUST part of objectclass is rather unprecedented move.
Just for fun I listed all objectclasses in my DS that use  description field
and neither of them had it in MUST part. I could partially live with
description being in MAY part of objectclass + the framework requiring it, but
I do not think it is a good idea either.

Martin

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


Re: [Freeipa-devel] [PATCHES 0114-0115, 0120-0121] DNS: allow to add root zone '.'

2014-09-16 Thread Martin Basti

On 15/09/14 20:31, Martin Kosek wrote:

On 09/15/2014 05:16 PM, Martin Basti wrote:

On 15/09/14 17:10, Petr Spacek wrote:

On 12.9.2014 15:19, Martin Basti wrote:

On 03/09/14 12:45, Martin Basti wrote:

On 03/09/14 12:27, Martin Kosek wrote:

On 09/02/2014 05:46 PM, Petr Spacek wrote:

On 25.8.2014 14:52, Martin Basti wrote:

Patches attached.

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

There is a bug in bind-dyndb-ldap (or worse in dirsrv), which 
cause the

named
service is stopped after deleting zone.
Bug ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/138
Functional ACK, it works for me. It can be pushed if Python 
gurus are okay

with
the code.
Is it safe to commit the change given that bind-dyndb-ldap still 
crash when

.
is removed? Wouldn't it break our CI tests?

Maybe we should wait until fixed bind-dydnb-ldap is released. 
Hopefully it

would be soon.

Martin

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

It will broke tests, don't push it until bind-dyndb-ldap is fixed.
Currently I'm testing bind-dyndb-ldap related patch.

Added patches 120 and 121, which are required by DNS to work 
correctly.
Patches 120 and 121 add all DNS replicas to zone apex as NS, 
--name-server

option doesn't add NS record, only changes the SOA MNAME attribute

Original and new patches attached.


NACK, unfortunately it doesn't work for me:
# ipa dnszone-add tri.test. --name-server=ns.test.
Administrator e-mail address [hostmaster.tri.test.]:
ipa: WARNING: '--name-server' is used only for setting up the SOA 
MNAME record.

To edit NS record(s) in zone apex, use command 'dnsrecord-mod [zone] @
--ns-rec=nameserver'.
  Zone name: tri.test.
  Active zone: TRUE
  Authoritative nameserver: ns.test.
  Administrator e-mail address: hostmaster.tri.test.
  SOA serial: 1410793406
  SOA refresh: 3600
  SOA retry: 900
  SOA expire: 1209600
  SOA minimum: 3600
  BIND update policy: grant IPA.EXAMPLE krb5-self * A; grant 
IPA.EXAMPLE

krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP;
  Dynamic update: FALSE
  Allow query: any;
  Allow transfer: none;

[root@vm-035 rpms]# ipa dnszone-show tri.test. --all --raw
  dn: idnsname=tri.test.,cn=dns,dc=ipa,dc=example
  idnsname: tri.test.
  idnszoneactive: TRUE
  idnssoamname: ns.test.
  idnssoarname: hostmaster.tri.test.
  idnssoaserial: 1410793408
  idnssoarefresh: 3600
  idnssoaretry: 900
  idnssoaexpire: 1209600
  idnssoaminimum: 3600
  idnsallowquery: any;
  idnsallowtransfer: none;
  idnsAllowDynUpdate: FALSE
  idnsUpdatePolicy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE
krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP;
  nsrecord: vm-035.idm.lab.eng.brq.redhat.com.
  objectClass: idnszone
  objectClass: top
  objectClass: idnsrecord

[root@vm-035 rpms]# ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname).
ipa: ERROR: tri.test.: DNS resource record not found


NACKing NACK
ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname).
you switched order zone and record, it should be
ipa dnsrecord-mod tri.test. @ --ns-rec=$(hostname).



BTW, since we are so nicely breaking the dnszone-add interface, can we 
also get rid of always asking for Administrator e-mail address?


 # ipa dnszone-add tri.test. --name-server=ns.test.
 Administrator e-mail address [hostmaster.tri.test.]:
...

Is there any risk in filling that with default as any other attribute? 
IMO it would simplify adding zones for one more redundant step. CCing 
Rob in case he knows some historical reasons why this is requested 
every time.


Martin

There is no risk, because ipa-replica-prepare do that with default values

--
Martin Basti

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


Re: [Freeipa-devel] [PATCHES 0114-0115, 0120-0121] DNS: allow to add root zone '.'

2014-09-16 Thread Martin Basti

On 16/09/14 09:32, Martin Basti wrote:

On 15/09/14 20:31, Martin Kosek wrote:

On 09/15/2014 05:16 PM, Martin Basti wrote:

On 15/09/14 17:10, Petr Spacek wrote:

On 12.9.2014 15:19, Martin Basti wrote:

On 03/09/14 12:45, Martin Basti wrote:

On 03/09/14 12:27, Martin Kosek wrote:

On 09/02/2014 05:46 PM, Petr Spacek wrote:

On 25.8.2014 14:52, Martin Basti wrote:

Patches attached.

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

There is a bug in bind-dyndb-ldap (or worse in dirsrv), which 
cause the

named
service is stopped after deleting zone.
Bug ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/138
Functional ACK, it works for me. It can be pushed if Python 
gurus are okay

with
the code.
Is it safe to commit the change given that bind-dyndb-ldap still 
crash when

.
is removed? Wouldn't it break our CI tests?

Maybe we should wait until fixed bind-dydnb-ldap is released. 
Hopefully it

would be soon.

Martin

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

It will broke tests, don't push it until bind-dyndb-ldap is fixed.
Currently I'm testing bind-dyndb-ldap related patch.

Added patches 120 and 121, which are required by DNS to work 
correctly.
Patches 120 and 121 add all DNS replicas to zone apex as NS, 
--name-server

option doesn't add NS record, only changes the SOA MNAME attribute

Original and new patches attached.


NACK, unfortunately it doesn't work for me:
# ipa dnszone-add tri.test. --name-server=ns.test.
Administrator e-mail address [hostmaster.tri.test.]:
ipa: WARNING: '--name-server' is used only for setting up the SOA 
MNAME record.

To edit NS record(s) in zone apex, use command 'dnsrecord-mod [zone] @
--ns-rec=nameserver'.
  Zone name: tri.test.
  Active zone: TRUE
  Authoritative nameserver: ns.test.
  Administrator e-mail address: hostmaster.tri.test.
  SOA serial: 1410793406
  SOA refresh: 3600
  SOA retry: 900
  SOA expire: 1209600
  SOA minimum: 3600
  BIND update policy: grant IPA.EXAMPLE krb5-self * A; grant 
IPA.EXAMPLE

krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP;
  Dynamic update: FALSE
  Allow query: any;
  Allow transfer: none;

[root@vm-035 rpms]# ipa dnszone-show tri.test. --all --raw
  dn: idnsname=tri.test.,cn=dns,dc=ipa,dc=example
  idnsname: tri.test.
  idnszoneactive: TRUE
  idnssoamname: ns.test.
  idnssoarname: hostmaster.tri.test.
  idnssoaserial: 1410793408
  idnssoarefresh: 3600
  idnssoaretry: 900
  idnssoaexpire: 1209600
  idnssoaminimum: 3600
  idnsallowquery: any;
  idnsallowtransfer: none;
  idnsAllowDynUpdate: FALSE
  idnsUpdatePolicy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE
krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP;
  nsrecord: vm-035.idm.lab.eng.brq.redhat.com.
  objectClass: idnszone
  objectClass: top
  objectClass: idnsrecord

[root@vm-035 rpms]# ipa dnsrecord-mod @ tri.test. 
--ns-rec=$(hostname).

ipa: ERROR: tri.test.: DNS resource record not found


NACKing NACK
ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname).
you switched order zone and record, it should be
ipa dnsrecord-mod tri.test. @ --ns-rec=$(hostname).



BTW, since we are so nicely breaking the dnszone-add interface, can 
we also get rid of always asking for Administrator e-mail address?


 # ipa dnszone-add tri.test. --name-server=ns.test.
 Administrator e-mail address [hostmaster.tri.test.]:
...

Is there any risk in filling that with default as any other 
attribute? IMO it would simplify adding zones for one more redundant 
step. CCing Rob in case he knows some historical reasons why this is 
requested every time.


Martin

There is no risk, because ipa-replica-prepare do that with default values

However, this will not work with root zone .,  and I'm not sure how 
often an admin email is used. I think whois is better utility to get 
contact email.


Also RIPE-203 [1] recommends to use 'hostmaster' alias.

[1] http://www.ripe.net/ripe/docs/ripe-203

--
Martin Basti

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


Re: [Freeipa-devel] [PATCHES 0114-0115, 0120-0121] DNS: allow to add root zone '.'

2014-09-16 Thread Martin Kosek
On 09/16/2014 09:57 AM, Martin Basti wrote:
 On 16/09/14 09:32, Martin Basti wrote:
 On 15/09/14 20:31, Martin Kosek wrote:
 On 09/15/2014 05:16 PM, Martin Basti wrote:
 On 15/09/14 17:10, Petr Spacek wrote:
 On 12.9.2014 15:19, Martin Basti wrote:
 On 03/09/14 12:45, Martin Basti wrote:
 On 03/09/14 12:27, Martin Kosek wrote:
 On 09/02/2014 05:46 PM, Petr Spacek wrote:
 On 25.8.2014 14:52, Martin Basti wrote:
 Patches attached.

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

 There is a bug in bind-dyndb-ldap (or worse in dirsrv), which cause 
 the
 named
 service is stopped after deleting zone.
 Bug ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/138
 Functional ACK, it works for me. It can be pushed if Python gurus are
 okay
 with
 the code.
 Is it safe to commit the change given that bind-dyndb-ldap still crash
 when
 .
 is removed? Wouldn't it break our CI tests?

 Maybe we should wait until fixed bind-dydnb-ldap is released. 
 Hopefully it
 would be soon.

 Martin

 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel
 It will broke tests, don't push it until bind-dyndb-ldap is fixed.
 Currently I'm testing bind-dyndb-ldap related patch.

 Added patches 120 and 121, which are required by DNS to work correctly.
 Patches 120 and 121 add all DNS replicas to zone apex as NS, 
 --name-server
 option doesn't add NS record, only changes the SOA MNAME attribute

 Original and new patches attached.

 NACK, unfortunately it doesn't work for me:
 # ipa dnszone-add tri.test. --name-server=ns.test.
 Administrator e-mail address [hostmaster.tri.test.]:
 ipa: WARNING: '--name-server' is used only for setting up the SOA MNAME
 record.
 To edit NS record(s) in zone apex, use command 'dnsrecord-mod [zone] @
 --ns-rec=nameserver'.
   Zone name: tri.test.
   Active zone: TRUE
   Authoritative nameserver: ns.test.
   Administrator e-mail address: hostmaster.tri.test.
   SOA serial: 1410793406
   SOA refresh: 3600
   SOA retry: 900
   SOA expire: 1209600
   SOA minimum: 3600
   BIND update policy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE
 krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP;
   Dynamic update: FALSE
   Allow query: any;
   Allow transfer: none;

 [root@vm-035 rpms]# ipa dnszone-show tri.test. --all --raw
   dn: idnsname=tri.test.,cn=dns,dc=ipa,dc=example
   idnsname: tri.test.
   idnszoneactive: TRUE
   idnssoamname: ns.test.
   idnssoarname: hostmaster.tri.test.
   idnssoaserial: 1410793408
   idnssoarefresh: 3600
   idnssoaretry: 900
   idnssoaexpire: 1209600
   idnssoaminimum: 3600
   idnsallowquery: any;
   idnsallowtransfer: none;
   idnsAllowDynUpdate: FALSE
   idnsUpdatePolicy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE
 krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP;
   nsrecord: vm-035.idm.lab.eng.brq.redhat.com.
   objectClass: idnszone
   objectClass: top
   objectClass: idnsrecord

 [root@vm-035 rpms]# ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname).
 ipa: ERROR: tri.test.: DNS resource record not found

 NACKing NACK
 ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname).
 you switched order zone and record, it should be
 ipa dnsrecord-mod tri.test. @ --ns-rec=$(hostname).


 BTW, since we are so nicely breaking the dnszone-add interface, can we also
 get rid of always asking for Administrator e-mail address?

  # ipa dnszone-add tri.test. --name-server=ns.test.
  Administrator e-mail address [hostmaster.tri.test.]:
 ...

 Is there any risk in filling that with default as any other attribute? IMO
 it would simplify adding zones for one more redundant step. CCing Rob in
 case he knows some historical reasons why this is requested every time.

 Martin
 There is no risk, because ipa-replica-prepare do that with default values

Then let us do this, as we are already simplifying the dnszone-add command.

 However, this will not work with root zone .,  and I'm not sure how often an
 admin email is used. I think whois is better utility to get contact email.
 
 Also RIPE-203 [1] recommends to use 'hostmaster' alias.
 
 [1] http://www.ripe.net/ripe/docs/ripe-203

DNS zone . is quite an exception, you are not adding that zone every day. So
I would not keep asking for admin mail just for this one. You can add a
interactive prompt callback to ask in this case and otherwise just use the
default - up to you.

As for the mail alias, this can be an RFE.

Martin

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


Re: [Freeipa-devel] [PATCHES 0114-0115, 0120-0121] DNS: allow to add root zone '.'

2014-09-16 Thread Petr Spacek

On 16.9.2014 10:09, Martin Kosek wrote:

On 09/16/2014 09:57 AM, Martin Basti wrote:

On 16/09/14 09:32, Martin Basti wrote:

On 15/09/14 20:31, Martin Kosek wrote:

On 09/15/2014 05:16 PM, Martin Basti wrote:

On 15/09/14 17:10, Petr Spacek wrote:

On 12.9.2014 15:19, Martin Basti wrote:

On 03/09/14 12:45, Martin Basti wrote:

On 03/09/14 12:27, Martin Kosek wrote:

On 09/02/2014 05:46 PM, Petr Spacek wrote:

On 25.8.2014 14:52, Martin Basti wrote:

Patches attached.

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

There is a bug in bind-dyndb-ldap (or worse in dirsrv), which cause the
named
service is stopped after deleting zone.
Bug ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/138

Functional ACK, it works for me. It can be pushed if Python gurus are
okay
with
the code.

Is it safe to commit the change given that bind-dyndb-ldap still crash
when
.
is removed? Wouldn't it break our CI tests?

Maybe we should wait until fixed bind-dydnb-ldap is released. Hopefully it
would be soon.

Martin

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

It will broke tests, don't push it until bind-dyndb-ldap is fixed.
Currently I'm testing bind-dyndb-ldap related patch.


Added patches 120 and 121, which are required by DNS to work correctly.
Patches 120 and 121 add all DNS replicas to zone apex as NS, --name-server
option doesn't add NS record, only changes the SOA MNAME attribute

Original and new patches attached.


NACK, unfortunately it doesn't work for me:
# ipa dnszone-add tri.test. --name-server=ns.test.
Administrator e-mail address [hostmaster.tri.test.]:
ipa: WARNING: '--name-server' is used only for setting up the SOA MNAME
record.
To edit NS record(s) in zone apex, use command 'dnsrecord-mod [zone] @
--ns-rec=nameserver'.
   Zone name: tri.test.
   Active zone: TRUE
   Authoritative nameserver: ns.test.
   Administrator e-mail address: hostmaster.tri.test.
   SOA serial: 1410793406
   SOA refresh: 3600
   SOA retry: 900
   SOA expire: 1209600
   SOA minimum: 3600
   BIND update policy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE
krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP;
   Dynamic update: FALSE
   Allow query: any;
   Allow transfer: none;

[root@vm-035 rpms]# ipa dnszone-show tri.test. --all --raw
   dn: idnsname=tri.test.,cn=dns,dc=ipa,dc=example
   idnsname: tri.test.
   idnszoneactive: TRUE
   idnssoamname: ns.test.
   idnssoarname: hostmaster.tri.test.
   idnssoaserial: 1410793408
   idnssoarefresh: 3600
   idnssoaretry: 900
   idnssoaexpire: 1209600
   idnssoaminimum: 3600
   idnsallowquery: any;
   idnsallowtransfer: none;
   idnsAllowDynUpdate: FALSE
   idnsUpdatePolicy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE
krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP;
   nsrecord: vm-035.idm.lab.eng.brq.redhat.com.
   objectClass: idnszone
   objectClass: top
   objectClass: idnsrecord

[root@vm-035 rpms]# ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname).
ipa: ERROR: tri.test.: DNS resource record not found


NACKing NACK
ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname).
you switched order zone and record, it should be
ipa dnsrecord-mod tri.test. @ --ns-rec=$(hostname).



BTW, since we are so nicely breaking the dnszone-add interface, can we also
get rid of always asking for Administrator e-mail address?


# ipa dnszone-add tri.test. --name-server=ns.test.
Administrator e-mail address [hostmaster.tri.test.]:

...

Is there any risk in filling that with default as any other attribute? IMO
it would simplify adding zones for one more redundant step. CCing Rob in
case he knows some historical reasons why this is requested every time.

Martin

There is no risk, because ipa-replica-prepare do that with default values


Then let us do this, as we are already simplifying the dnszone-add command.


However, this will not work with root zone .,  and I'm not sure how often an
admin email is used. I think whois is better utility to get contact email.

Also RIPE-203 [1] recommends to use 'hostmaster' alias.

[1] http://www.ripe.net/ripe/docs/ripe-203


This will likely generate tons of invalid e-mail addresses which is somehow 
unfortunate.


Please keep in mind that:
1) E-mail hostmaster@ipa.domain.example. will be useful only if 
ipa.domain.example. has MX record or at least A/ record (which is usually 
not the case for domains).


2) WHOIS is not useful for internal domains which is the main deployment 
scenario for IPA, right?



DNS zone . is quite an exception, you are not adding that zone every day. So
I would not keep asking for admin mail just for this one. You can add a
interactive prompt callback to ask in this case and otherwise just use the
default - up to you.

As for the mail alias, this can be an RFE.


It would be nice to have some IPA-global default like 'DNS administrator 
e-mail address' and to use this value for all DNS zones by 

Re: [Freeipa-devel] [PATCHES 0114-0115, 0120-0121] DNS: allow to add root zone '.'

2014-09-16 Thread Martin Kosek
On 09/16/2014 10:30 AM, Martin Basti wrote:
 On 16/09/14 10:29, Petr Spacek wrote:
 On 16.9.2014 10:09, Martin Kosek wrote:
 On 09/16/2014 09:57 AM, Martin Basti wrote:
 On 16/09/14 09:32, Martin Basti wrote:
 On 15/09/14 20:31, Martin Kosek wrote:
 On 09/15/2014 05:16 PM, Martin Basti wrote:
 On 15/09/14 17:10, Petr Spacek wrote:
 On 12.9.2014 15:19, Martin Basti wrote:
 On 03/09/14 12:45, Martin Basti wrote:
 On 03/09/14 12:27, Martin Kosek wrote:
 On 09/02/2014 05:46 PM, Petr Spacek wrote:
 On 25.8.2014 14:52, Martin Basti wrote:
 Patches attached.

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

 There is a bug in bind-dyndb-ldap (or worse in dirsrv), which
 cause the
 named
 service is stopped after deleting zone.
 Bug ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/138
 Functional ACK, it works for me. It can be pushed if Python gurus 
 are
 okay
 with
 the code.
 Is it safe to commit the change given that bind-dyndb-ldap still 
 crash
 when
 .
 is removed? Wouldn't it break our CI tests?

 Maybe we should wait until fixed bind-dydnb-ldap is released.
 Hopefully it
 would be soon.

 Martin

 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel
 It will broke tests, don't push it until bind-dyndb-ldap is fixed.
 Currently I'm testing bind-dyndb-ldap related patch.

 Added patches 120 and 121, which are required by DNS to work 
 correctly.
 Patches 120 and 121 add all DNS replicas to zone apex as NS,
 --name-server
 option doesn't add NS record, only changes the SOA MNAME attribute

 Original and new patches attached.

 NACK, unfortunately it doesn't work for me:
 # ipa dnszone-add tri.test. --name-server=ns.test.
 Administrator e-mail address [hostmaster.tri.test.]:
 ipa: WARNING: '--name-server' is used only for setting up the SOA MNAME
 record.
 To edit NS record(s) in zone apex, use command 'dnsrecord-mod [zone] @
 --ns-rec=nameserver'.
Zone name: tri.test.
Active zone: TRUE
Authoritative nameserver: ns.test.
Administrator e-mail address: hostmaster.tri.test.
SOA serial: 1410793406
SOA refresh: 3600
SOA retry: 900
SOA expire: 1209600
SOA minimum: 3600
BIND update policy: grant IPA.EXAMPLE krb5-self * A; grant 
 IPA.EXAMPLE
 krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP;
Dynamic update: FALSE
Allow query: any;
Allow transfer: none;

 [root@vm-035 rpms]# ipa dnszone-show tri.test. --all --raw
dn: idnsname=tri.test.,cn=dns,dc=ipa,dc=example
idnsname: tri.test.
idnszoneactive: TRUE
idnssoamname: ns.test.
idnssoarname: hostmaster.tri.test.
idnssoaserial: 1410793408
idnssoarefresh: 3600
idnssoaretry: 900
idnssoaexpire: 1209600
idnssoaminimum: 3600
idnsallowquery: any;
idnsallowtransfer: none;
idnsAllowDynUpdate: FALSE
idnsUpdatePolicy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE
 krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP;
nsrecord: vm-035.idm.lab.eng.brq.redhat.com.
objectClass: idnszone
objectClass: top
objectClass: idnsrecord

 [root@vm-035 rpms]# ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname).
 ipa: ERROR: tri.test.: DNS resource record not found

 NACKing NACK
 ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname).
 you switched order zone and record, it should be
 ipa dnsrecord-mod tri.test. @ --ns-rec=$(hostname).


 BTW, since we are so nicely breaking the dnszone-add interface, can we 
 also
 get rid of always asking for Administrator e-mail address?

 # ipa dnszone-add tri.test. --name-server=ns.test.
 Administrator e-mail address [hostmaster.tri.test.]:
 ...

 Is there any risk in filling that with default as any other attribute? 
 IMO
 it would simplify adding zones for one more redundant step. CCing Rob in
 case he knows some historical reasons why this is requested every time.

 Martin
 There is no risk, because ipa-replica-prepare do that with default values

 Then let us do this, as we are already simplifying the dnszone-add command.

 However, this will not work with root zone .,  and I'm not sure how 
 often an
 admin email is used. I think whois is better utility to get contact email.

 Also RIPE-203 [1] recommends to use 'hostmaster' alias.

 [1] http://www.ripe.net/ripe/docs/ripe-203

 This will likely generate tons of invalid e-mail addresses which is somehow
 unfortunate.

 Please keep in mind that:
 1) E-mail hostmaster@ipa.domain.example. will be useful only if
 ipa.domain.example. has MX record or at least A/ record (which is usually
 not the case for domains).

 2) WHOIS is not useful for internal domains which is the main deployment
 scenario for IPA, right?

 DNS zone . is quite an exception, you are not adding that zone every day. 
 So
 I would not keep asking for admin mail just for this one. You can add a
 interactive prompt callback to ask in this case and otherwise just use the
 default - up to you.

 

[Freeipa-devel] [PATCH 0269] ipalib: host_del: Extend LDAPDelete's takes_options instead

2014-09-16 Thread Tomas Babej
Hi,

The host-del command did not accept --continue option, since the
takes_options was overriden and did not take the options from LDAPDelete.

Fix the behaviour.

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

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 


From b38301078116b2fb9bbefcb2875ce1a600e37de8 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 5 Aug 2014 09:12:24 +0200
Subject: [PATCH] ipalib: host_del: Extend LDAPDelete's takes_options instead
 of overriding

The host-del command did not accept --continue option, since the
takes_options was overriden and did not take the options from LDAPDelete.

Fix the behaviour.

https://fedorahosted.org/freeipa/ticket/4473
---
 API.txt  | 3 ++-
 ipalib/plugins/host.py   | 2 +-
 ipatests/test_xmlrpc/test_host_plugin.py | 5 +
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/API.txt b/API.txt
index 1243fefa7d37b34ac3dd0fac3df8820142c9f2c7..09464dbe2bc9fb7fa22a91879dc957da92aadc8f 100644
--- a/API.txt
+++ b/API.txt
@@ -1820,8 +1820,9 @@ output: Output('completed', type 'int', None)
 output: Output('failed', type 'dict', None)
 output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 command: host_del
-args: 1,2,3
+args: 1,3,3
 arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=True, primary_key=True, query=True, required=True)
+option: Flag('continue', autofill=True, cli_name='continue', default=False)
 option: Flag('updatedns?', autofill=True, default=False)
 option: Str('version?', exclude='webui')
 output: Output('result', type 'dict', None)
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 4ec788d38ead1b987e94d5a68c14163d4dec2ea7..bbee09395dc55730d400944729a5bed48670f785 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -650,7 +650,7 @@ class host_del(LDAPDelete):
 msg_summary = _('Deleted host %(value)s')
 member_attributes = ['managedby']
 
-takes_options = (
+takes_options = LDAPDelete.takes_options + (
 Flag('updatedns?',
 doc=_('Remove entries from DNS'),
 default=False,
diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py
index a98fed1b64853e548d489f1bc12d012d18f986a7..7c0312bae5e9e652085aac8f1970610044504281 100644
--- a/ipatests/test_xmlrpc/test_host_plugin.py
+++ b/ipatests/test_xmlrpc/test_host_plugin.py
@@ -139,10 +139,7 @@ sshpubkeyfp = u'13:67:6B:BF:4E:A2:05:8E:AE:25:8B:A1:31:DE:6F:1B public key test
 class test_host(Declarative):
 
 cleanup_commands = [
-('host_del', [fqdn1], {}),
-('host_del', [fqdn2], {}),
-('host_del', [fqdn3], {}),
-('host_del', [fqdn4], {}),
+('host_del', [fqdn1, fqdn2, fqdn3, fqdn4], {'continue': True}),
 ('service_del', [service1], {}),
 ]
 
-- 
1.9.3

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

Re: [Freeipa-devel] [PATCH 0269] ipalib: host_del: Extend LDAPDelete's takes_options instead

2014-09-16 Thread Jan Cholasta

Dne 16.9.2014 v 13:21 Tomas Babej napsal(a):

Hi,

The host-del command did not accept --continue option, since the
takes_options was overriden and did not take the options from LDAPDelete.

Fix the behaviour.

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


ACK.

--
Jan Cholasta

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


Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.

2014-09-16 Thread David Kupka

On 09/12/2014 07:24 PM, Martin Basti wrote:

snip /

Be careful, reviewed on friday! :-)

1)
whitespace error + pep8 error
patch:76: trailing whitespace.
 # there is reverse zone for every ip address
warning: 1 line adds whitespace errors.

./ipaserver/install/bindinstance.py:640:9: E265 block comment should
start with '# '



Stupid mistake, sorry.



2) (server install)
+for ip in ip_addresses:
+for rev_zone in reverse_zones:
+if bindinstance.verify_reverse_zone(rev_zone, str(ip)):
+break
+else:
+sys.exit(1)

Please add there error message instead 1 to exit function


You are right, it's better to say what's wrong.



3) (server install)
Code above, bindinstance.verify_reverse_zone(rev_zone, str(ip)):
IMHO this will cause errors (not tested) try:
rev-zones: [10.10.10.in-addr.arpa., 16.172.in-addr.arpa.]
ip_addreses: [10.10.10.1, 172.16.0.1]

it should be any() of zone can handle ip


I indented the else branch more that I wanted.



4) (replica-install)
I'm not sure about behavior of combination ip addresses, and reverse zones,
In original code, if user specify reverse zone, the ip address must match.

In patched version, you just add new zones for ip-addresses which
doesn't math the user zones.

IMO we should check if all addresses belongs to user specified zones,
otherwise raise an error.
But I'm open to suggestions.


The behavior now basically is:

Check if IP address matches any specified zone
  a. if yes we are good
  b. else search if there is existing zone that can be used
ba. if yes we are good
bb. else ask user for zone or create default if --unattended



5)
Code for ipa-replica-install, and ipa-server-install, differs in parts
where we expecting same behavior
   - ip addresses and reverse zones


The behavoir now should be almost the same. The only difference is that 
when installing replica we need to search for existing reverse zones as 
they could be added during on another server.




6)
https://engineering.redhat.com/irc-services.txt+# there is at
least one ip address in every zone
+if options.reverse_zones:
+for reverse_zone in options.reverse_zones:
+for ip in config.ips:
--
A
+if bindinstance.verify_reverse_zone(reverse_zone, ip):
+ reverse_zones.append(bindinstance.normalize_zone(reverse_zone))
+break
+else:
+sys.exit(1)
*
+# there is reverse zone for every ip address
+for ip in config.ips:
+for reverse_zone in options.reverse_zones:
--- B
+if bindinstance.verify_reverse_zone(reverse_zone, ip):
+if reverse_zone not in reverse_zones:
+ reverse_zones.append(bindinstance.normalize_zone(reverse_zone))
+break
+else:

C
+reverse_zone = bindinstance.find_reverse_zone(ip)
+if reverse_zone is None and not options.no_reverse:
+reverse_zone = util.get_reverse_zone_default(ip)
- D1
+if not options.unattended and
bindinstance.create_reverse(): - D
+reverse_zone =
bindinstance.read_reverse_zone(reverse_zone, ip)
+ reverse_zones.append(bindinstance.normalize_zone(reverse_zone))
- D2

You added all possible zones in step A,  B step is not needed.
If user doesn't specify reverse_zones you can just jump to C step
*add error message
C: elif not options.no_reverse
D: you asked user if wants to create zone, but you don't care about
answer, and in step D2 append zone from D1
note: --no-reverse and --reverse-zones cant be used together, you can
use it in new code, test it before cycle


Rewritten.



7)
  # Always use force=True as named is not set up yet
  add_zone(self.domain, self.zonemgr, dns_backup=self.dns_backup,
-ns_hostname=api.env.host,
ns_ip_address=nameserver_ip_address,
-force=True)
+ ns_hostname=api.env.host, ns_ip_address=None, force=True)
+#for ns_ip_address in nameserver_ip_address:
+#add_zone(self.domain, self.zonemgr,
dns_backup=self.dns_backup,
+#ns_hostname=api.env.host, ns_ip_address=ns_ip_address,
+#force=True)

Please remove commented section


I forget to clean the trash, thanks.



You can remove 'ns_ip_address=None' from function

8)
+if set(options.ip_addresses) = set(ips):
+ips = options.ip_addresses
+else:
+print sys.stderr, Error: the hostname resolves to an
IP address that is 

Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name

2014-09-16 Thread Nathaniel McCallum
On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote:
 Admins need the ability to specify the token ID in the case of imports.
 However, generally, this ability is not needed.
 
 Is it possible to offload the ID generation to the ipa-uuid plugin? I'm
 not quite sure how to enable this (I think it involves passing a magic
 value?). But I'm not quite sure how this fits in with the IPA framework
 as the generated value is the DN.
 
 However, assuming this can be used, I propose the following. The token
 ID is removed from the UI for regular users (but retained for admins).
 We change the ACIs for token addition/modification to prevent regular
 users from specifying the ID in an add or mod operation. The CLI would
 retain the option to set it, but this option would only be usable by
 admins.
 
 Make sense?

Nobody has responded to this. :)

However, since investigating it a bit more, this approach won't really
work without further effort. Here is the problem.

First, the UUID plugin doesn't currently support this kind of operation.
Either it needs to be modified or a new plugin needs to be created.

Second, the client needs to know the ID in order to generate the token
URI. If we generate the UUID inside the DS, the UUID is unknown to the
client and the URI can't be generated. This would mean a new control.

As I see it we have three options:
1. Remove the option to specify the ipatokenUniqueID from the GUI. Don't
make any change in the CLI.

ENFORCEMENT: none
EFFORT: low

2. Perform a server-side check for admin membership. Raise an exception
if the ipatokenUniqueID is specified and the user is not an admin.

ENFORCEMENT: API-level
EFFORT: medium

3. Modify otptoken-add to create tokens with a magical ipatokenUniqueID
value by default. An ACI would prevent normal users from adding tokens
without this magic value. Create/modify a plugin to generate UUIDs when
the magic value is found. Send a control back to the client indicating
the real ipatokenUniqueID value. Modify otptoken-add to read this
control.

ENFORCEMENT: DS-level
EFFORT: high

I think my preference for now is #1. Thoughts?

Nathaniel

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


Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name

2014-09-16 Thread Petr Vobornik

On 16.9.2014 17:26, Nathaniel McCallum wrote:

On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote:

Admins need the ability to specify the token ID in the case of imports.
However, generally, this ability is not needed.

Is it possible to offload the ID generation to the ipa-uuid plugin? I'm
not quite sure how to enable this (I think it involves passing a magic
value?). But I'm not quite sure how this fits in with the IPA framework
as the generated value is the DN.

However, assuming this can be used, I propose the following. The token
ID is removed from the UI for regular users (but retained for admins).
We change the ACIs for token addition/modification to prevent regular
users from specifying the ID in an add or mod operation. The CLI would
retain the option to set it, but this option would only be usable by
admins.

Make sense?


Nobody has responded to this. :)

However, since investigating it a bit more, this approach won't really
work without further effort. Here is the problem.

First, the UUID plugin doesn't currently support this kind of operation.
Either it needs to be modified or a new plugin needs to be created.

Second, the client needs to know the ID in order to generate the token
URI. If we generate the UUID inside the DS, the UUID is unknown to the
client and the URI can't be generated. This would mean a new control.

As I see it we have three options:
1. Remove the option to specify the ipatokenUniqueID from the GUI. Don't
make any change in the CLI.

ENFORCEMENT: none
EFFORT: low

2. Perform a server-side check for admin membership. Raise an exception
if the ipatokenUniqueID is specified and the user is not an admin.

ENFORCEMENT: API-level
EFFORT: medium

3. Modify otptoken-add to create tokens with a magical ipatokenUniqueID
value by default. An ACI would prevent normal users from adding tokens
without this magic value. Create/modify a plugin to generate UUIDs when
the magic value is found. Send a control back to the client indicating
the real ipatokenUniqueID value. Modify otptoken-add to read this
control.

ENFORCEMENT: DS-level
EFFORT: high

I think my preference for now is #1. Thoughts?

Nathaniel



I want to raise a question if we really want to disable this feature for 
normal users. Wouldn't it be better to improve error message or check if 
the name is taken?


UUID is the value which is displayed in the Soft. Token application as 
name. Sure, one can rename it there to match the description in IPA but 
that seems quite unpleasant to me. Also user has to know that he can 
rename the token in FreeOTP,


Atm the existence check might be little problematic - disclose of 
information which is not readable to user by default. But current state 
is basically it, just unfriendly.

--
Petr Vobornik

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


Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name

2014-09-16 Thread Nathaniel McCallum
On Tue, 2014-09-16 at 17:51 +0200, Petr Vobornik wrote:
 On 16.9.2014 17:26, Nathaniel McCallum wrote:
  On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote:
  Admins need the ability to specify the token ID in the case of imports.
  However, generally, this ability is not needed.
 
  Is it possible to offload the ID generation to the ipa-uuid plugin? I'm
  not quite sure how to enable this (I think it involves passing a magic
  value?). But I'm not quite sure how this fits in with the IPA framework
  as the generated value is the DN.
 
  However, assuming this can be used, I propose the following. The token
  ID is removed from the UI for regular users (but retained for admins).
  We change the ACIs for token addition/modification to prevent regular
  users from specifying the ID in an add or mod operation. The CLI would
  retain the option to set it, but this option would only be usable by
  admins.
 
  Make sense?
 
  Nobody has responded to this. :)
 
  However, since investigating it a bit more, this approach won't really
  work without further effort. Here is the problem.
 
  First, the UUID plugin doesn't currently support this kind of operation.
  Either it needs to be modified or a new plugin needs to be created.
 
  Second, the client needs to know the ID in order to generate the token
  URI. If we generate the UUID inside the DS, the UUID is unknown to the
  client and the URI can't be generated. This would mean a new control.
 
  As I see it we have three options:
  1. Remove the option to specify the ipatokenUniqueID from the GUI. Don't
  make any change in the CLI.
 
  ENFORCEMENT: none
  EFFORT: low
 
  2. Perform a server-side check for admin membership. Raise an exception
  if the ipatokenUniqueID is specified and the user is not an admin.
 
  ENFORCEMENT: API-level
  EFFORT: medium
 
  3. Modify otptoken-add to create tokens with a magical ipatokenUniqueID
  value by default. An ACI would prevent normal users from adding tokens
  without this magic value. Create/modify a plugin to generate UUIDs when
  the magic value is found. Send a control back to the client indicating
  the real ipatokenUniqueID value. Modify otptoken-add to read this
  control.
 
  ENFORCEMENT: DS-level
  EFFORT: high
 
  I think my preference for now is #1. Thoughts?
 
  Nathaniel
 
 
 I want to raise a question if we really want to disable this feature for 
 normal users. Wouldn't it be better to improve error message or check if 
 the name is taken?
 
 UUID is the value which is displayed in the Soft. Token application as 
 name. Sure, one can rename it there to match the description in IPA but 
 that seems quite unpleasant to me. Also user has to know that he can 
 rename the token in FreeOTP,
 
 Atm the existence check might be little problematic - disclose of 
 information which is not readable to user by default. But current state 
 is basically it, just unfriendly.

The existence check is impossible: users can't see tokens they don't
own.


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


Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.

2014-09-16 Thread Martin Basti

On 16/09/14 15:59, David Kupka wrote:

On 09/12/2014 07:24 PM, Martin Basti wrote:

snip /

Be careful, reviewed on friday! :-)

1)
whitespace error + pep8 error
patch:76: trailing whitespace.
 # there is reverse zone for every ip address
warning: 1 line adds whitespace errors.

./ipaserver/install/bindinstance.py:640:9: E265 block comment should
start with '# '



Stupid mistake, sorry.



2) (server install)
+for ip in ip_addresses:
+for rev_zone in reverse_zones:
+if bindinstance.verify_reverse_zone(rev_zone, str(ip)):
+break
+else:
+sys.exit(1)

Please add there error message instead 1 to exit function


You are right, it's better to say what's wrong.



3) (server install)
Code above, bindinstance.verify_reverse_zone(rev_zone, str(ip)):
IMHO this will cause errors (not tested) try:
rev-zones: [10.10.10.in-addr.arpa., 16.172.in-addr.arpa.]
ip_addreses: [10.10.10.1, 172.16.0.1]

it should be any() of zone can handle ip


I indented the else branch more that I wanted.



4) (replica-install)
I'm not sure about behavior of combination ip addresses, and reverse 
zones,
In original code, if user specify reverse zone, the ip address must 
match.


In patched version, you just add new zones for ip-addresses which
doesn't math the user zones.

IMO we should check if all addresses belongs to user specified zones,
otherwise raise an error.
But I'm open to suggestions.


The behavior now basically is:

Check if IP address matches any specified zone
  a. if yes we are good
  b. else search if there is existing zone that can be used
ba. if yes we are good
bb. else ask user for zone or create default if --unattended



5)
Code for ipa-replica-install, and ipa-server-install, differs in parts
where we expecting same behavior
   - ip addresses and reverse zones


The behavoir now should be almost the same. The only difference is 
that when installing replica we need to search for existing reverse 
zones as they could be added during on another server.




6)
https://engineering.redhat.com/irc-services.txt+# there is at
least one ip address in every zone
+if options.reverse_zones:
+for reverse_zone in options.reverse_zones:
+for ip in config.ips:
-- 


A
+if bindinstance.verify_reverse_zone(reverse_zone, ip):
+ reverse_zones.append(bindinstance.normalize_zone(reverse_zone))
+break
+else:
+sys.exit(1)
* 


+# there is reverse zone for every ip address
+for ip in config.ips:
+for reverse_zone in options.reverse_zones:
--- B
+if bindinstance.verify_reverse_zone(reverse_zone, ip):
+if reverse_zone not in reverse_zones:
+ reverse_zones.append(bindinstance.normalize_zone(reverse_zone))
+break
+else:
 


C
+reverse_zone = bindinstance.find_reverse_zone(ip)
+if reverse_zone is None and not options.no_reverse:
+reverse_zone = util.get_reverse_zone_default(ip)
- D1
+if not options.unattended and
bindinstance.create_reverse(): - D
+reverse_zone =
bindinstance.read_reverse_zone(reverse_zone, ip)
+ reverse_zones.append(bindinstance.normalize_zone(reverse_zone))
- D2

You added all possible zones in step A,  B step is not needed.
If user doesn't specify reverse_zones you can just jump to C step
*add error message
C: elif not options.no_reverse
D: you asked user if wants to create zone, but you don't care about
answer, and in step D2 append zone from D1
note: --no-reverse and --reverse-zones cant be used together, you can
use it in new code, test it before cycle


Rewritten.



7)
  # Always use force=True as named is not set up yet
  add_zone(self.domain, self.zonemgr, 
dns_backup=self.dns_backup,

-ns_hostname=api.env.host,
ns_ip_address=nameserver_ip_address,
-force=True)
+ ns_hostname=api.env.host, ns_ip_address=None, 
force=True)

+#for ns_ip_address in nameserver_ip_address:
+#add_zone(self.domain, self.zonemgr,
dns_backup=self.dns_backup,
+#ns_hostname=api.env.host, 
ns_ip_address=ns_ip_address,

+#force=True)

Please remove commented section


I forget to clean the trash, thanks.



You can remove 'ns_ip_address=None' from function

8)
+if set(options.ip_addresses) = set(ips):
+ips = options.ip_addresses
+else:
+print sys.stderr, 

Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name

2014-09-16 Thread Nathaniel McCallum
On Tue, 2014-09-16 at 12:22 -0400, Simo Sorce wrote:
 On Tue, 16 Sep 2014 11:52:43 -0400
 Nathaniel McCallum npmccal...@redhat.com wrote:
 
  On Tue, 2014-09-16 at 17:51 +0200, Petr Vobornik wrote:
   On 16.9.2014 17:26, Nathaniel McCallum wrote:
On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote:
Admins need the ability to specify the token ID in the case of
imports. However, generally, this ability is not needed.
   
Is it possible to offload the ID generation to the ipa-uuid
plugin? I'm not quite sure how to enable this (I think it
involves passing a magic value?). But I'm not quite sure how
this fits in with the IPA framework as the generated value is
the DN.
   
However, assuming this can be used, I propose the following. The
token ID is removed from the UI for regular users (but retained
for admins). We change the ACIs for token addition/modification
to prevent regular users from specifying the ID in an add or mod
operation. The CLI would retain the option to set it, but this
option would only be usable by admins.
   
Make sense?
   
Nobody has responded to this. :)
   
However, since investigating it a bit more, this approach won't
really work without further effort. Here is the problem.
   
First, the UUID plugin doesn't currently support this kind of
operation. Either it needs to be modified or a new plugin needs
to be created.
   
Second, the client needs to know the ID in order to generate the
token URI. If we generate the UUID inside the DS, the UUID is
unknown to the client and the URI can't be generated. This would
mean a new control.
   
As I see it we have three options:
1. Remove the option to specify the ipatokenUniqueID from the
GUI. Don't make any change in the CLI.
   
ENFORCEMENT: none
EFFORT: low
   
2. Perform a server-side check for admin membership. Raise an
exception if the ipatokenUniqueID is specified and the user is
not an admin.
   
ENFORCEMENT: API-level
EFFORT: medium
   
3. Modify otptoken-add to create tokens with a magical
ipatokenUniqueID value by default. An ACI would prevent normal
users from adding tokens without this magic value. Create/modify
a plugin to generate UUIDs when the magic value is found. Send a
control back to the client indicating the real ipatokenUniqueID
value. Modify otptoken-add to read this control.
   
ENFORCEMENT: DS-level
EFFORT: high
   
I think my preference for now is #1. Thoughts?
   
Nathaniel
   
   
   I want to raise a question if we really want to disable this
   feature for normal users. Wouldn't it be better to improve error
   message or check if the name is taken?
   
   UUID is the value which is displayed in the Soft. Token application
   as name. Sure, one can rename it there to match the description in
   IPA but that seems quite unpleasant to me. Also user has to know
   that he can rename the token in FreeOTP,
   
   Atm the existence check might be little problematic - disclose of 
   information which is not readable to user by default. But current
   state is basically it, just unfriendly.
  
  The existence check is impossible: users can't see tokens they don't
  own.
 
 Why don't we simply allow user to use an arbitrary name of their
 choosing (or a random string if not specified) and then append their
 uid string to it in the UI ?
 
 I wonder if we have ACIs where the value must match a pattern defined
 by the bound user ...

This would present a strange scenario where a token is created and then
reassigned to another user.

Nathaniel

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


Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name

2014-09-16 Thread Simo Sorce
On Tue, 16 Sep 2014 11:52:43 -0400
Nathaniel McCallum npmccal...@redhat.com wrote:

 On Tue, 2014-09-16 at 17:51 +0200, Petr Vobornik wrote:
  On 16.9.2014 17:26, Nathaniel McCallum wrote:
   On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote:
   Admins need the ability to specify the token ID in the case of
   imports. However, generally, this ability is not needed.
  
   Is it possible to offload the ID generation to the ipa-uuid
   plugin? I'm not quite sure how to enable this (I think it
   involves passing a magic value?). But I'm not quite sure how
   this fits in with the IPA framework as the generated value is
   the DN.
  
   However, assuming this can be used, I propose the following. The
   token ID is removed from the UI for regular users (but retained
   for admins). We change the ACIs for token addition/modification
   to prevent regular users from specifying the ID in an add or mod
   operation. The CLI would retain the option to set it, but this
   option would only be usable by admins.
  
   Make sense?
  
   Nobody has responded to this. :)
  
   However, since investigating it a bit more, this approach won't
   really work without further effort. Here is the problem.
  
   First, the UUID plugin doesn't currently support this kind of
   operation. Either it needs to be modified or a new plugin needs
   to be created.
  
   Second, the client needs to know the ID in order to generate the
   token URI. If we generate the UUID inside the DS, the UUID is
   unknown to the client and the URI can't be generated. This would
   mean a new control.
  
   As I see it we have three options:
   1. Remove the option to specify the ipatokenUniqueID from the
   GUI. Don't make any change in the CLI.
  
   ENFORCEMENT: none
   EFFORT: low
  
   2. Perform a server-side check for admin membership. Raise an
   exception if the ipatokenUniqueID is specified and the user is
   not an admin.
  
   ENFORCEMENT: API-level
   EFFORT: medium
  
   3. Modify otptoken-add to create tokens with a magical
   ipatokenUniqueID value by default. An ACI would prevent normal
   users from adding tokens without this magic value. Create/modify
   a plugin to generate UUIDs when the magic value is found. Send a
   control back to the client indicating the real ipatokenUniqueID
   value. Modify otptoken-add to read this control.
  
   ENFORCEMENT: DS-level
   EFFORT: high
  
   I think my preference for now is #1. Thoughts?
  
   Nathaniel
  
  
  I want to raise a question if we really want to disable this
  feature for normal users. Wouldn't it be better to improve error
  message or check if the name is taken?
  
  UUID is the value which is displayed in the Soft. Token application
  as name. Sure, one can rename it there to match the description in
  IPA but that seems quite unpleasant to me. Also user has to know
  that he can rename the token in FreeOTP,
  
  Atm the existence check might be little problematic - disclose of 
  information which is not readable to user by default. But current
  state is basically it, just unfriendly.
 
 The existence check is impossible: users can't see tokens they don't
 own.

Why don't we simply allow user to use an arbitrary name of their
choosing (or a random string if not specified) and then append their
uid string to it in the UI ?

I wonder if we have ACIs where the value must match a pattern defined
by the bound user ...

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] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name

2014-09-16 Thread Simo Sorce
On Tue, 16 Sep 2014 12:23:51 -0400
Nathaniel McCallum npmccal...@redhat.com wrote:

 On Tue, 2014-09-16 at 12:22 -0400, Simo Sorce wrote:
  On Tue, 16 Sep 2014 11:52:43 -0400
  Nathaniel McCallum npmccal...@redhat.com wrote:
  
   On Tue, 2014-09-16 at 17:51 +0200, Petr Vobornik wrote:
On 16.9.2014 17:26, Nathaniel McCallum wrote:
 On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote:
 Admins need the ability to specify the token ID in the case
 of imports. However, generally, this ability is not needed.

 Is it possible to offload the ID generation to the ipa-uuid
 plugin? I'm not quite sure how to enable this (I think it
 involves passing a magic value?). But I'm not quite sure how
 this fits in with the IPA framework as the generated value is
 the DN.

 However, assuming this can be used, I propose the following.
 The token ID is removed from the UI for regular users (but
 retained for admins). We change the ACIs for token
 addition/modification to prevent regular users from
 specifying the ID in an add or mod operation. The CLI would
 retain the option to set it, but this option would only be
 usable by admins.

 Make sense?

 Nobody has responded to this. :)

 However, since investigating it a bit more, this approach
 won't really work without further effort. Here is the problem.

 First, the UUID plugin doesn't currently support this kind of
 operation. Either it needs to be modified or a new plugin
 needs to be created.

 Second, the client needs to know the ID in order to generate
 the token URI. If we generate the UUID inside the DS, the
 UUID is unknown to the client and the URI can't be generated.
 This would mean a new control.

 As I see it we have three options:
 1. Remove the option to specify the ipatokenUniqueID from the
 GUI. Don't make any change in the CLI.

 ENFORCEMENT: none
 EFFORT: low

 2. Perform a server-side check for admin membership. Raise an
 exception if the ipatokenUniqueID is specified and the user is
 not an admin.

 ENFORCEMENT: API-level
 EFFORT: medium

 3. Modify otptoken-add to create tokens with a magical
 ipatokenUniqueID value by default. An ACI would prevent normal
 users from adding tokens without this magic value.
 Create/modify a plugin to generate UUIDs when the magic value
 is found. Send a control back to the client indicating the
 real ipatokenUniqueID value. Modify otptoken-add to read this
 control.

 ENFORCEMENT: DS-level
 EFFORT: high

 I think my preference for now is #1. Thoughts?

 Nathaniel


I want to raise a question if we really want to disable this
feature for normal users. Wouldn't it be better to improve error
message or check if the name is taken?

UUID is the value which is displayed in the Soft. Token
application as name. Sure, one can rename it there to match the
description in IPA but that seems quite unpleasant to me. Also
user has to know that he can rename the token in FreeOTP,

Atm the existence check might be little problematic - disclose
of information which is not readable to user by default. But
current state is basically it, just unfriendly.
   
   The existence check is impossible: users can't see tokens they
   don't own.
  
  Why don't we simply allow user to use an arbitrary name of their
  choosing (or a random string if not specified) and then append their
  uid string to it in the UI ?
  
  I wonder if we have ACIs where the value must match a pattern
  defined by the bound user ...
 
 This would present a strange scenario where a token is created and
 then reassigned to another user.


The idea is to strip away the tail when visualizing to the user, so it
wouldn't really bee seen ?
Or perhaps the token can simply be renamed when assigned.

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] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name

2014-09-16 Thread Nathaniel McCallum
On Tue, 2014-09-16 at 12:28 -0400, Simo Sorce wrote:
 On Tue, 16 Sep 2014 12:23:51 -0400
 Nathaniel McCallum npmccal...@redhat.com wrote:
 
  On Tue, 2014-09-16 at 12:22 -0400, Simo Sorce wrote:
   On Tue, 16 Sep 2014 11:52:43 -0400
   Nathaniel McCallum npmccal...@redhat.com wrote:
   
On Tue, 2014-09-16 at 17:51 +0200, Petr Vobornik wrote:
 On 16.9.2014 17:26, Nathaniel McCallum wrote:
  On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote:
  Admins need the ability to specify the token ID in the case
  of imports. However, generally, this ability is not needed.
 
  Is it possible to offload the ID generation to the ipa-uuid
  plugin? I'm not quite sure how to enable this (I think it
  involves passing a magic value?). But I'm not quite sure how
  this fits in with the IPA framework as the generated value is
  the DN.
 
  However, assuming this can be used, I propose the following.
  The token ID is removed from the UI for regular users (but
  retained for admins). We change the ACIs for token
  addition/modification to prevent regular users from
  specifying the ID in an add or mod operation. The CLI would
  retain the option to set it, but this option would only be
  usable by admins.
 
  Make sense?
 
  Nobody has responded to this. :)
 
  However, since investigating it a bit more, this approach
  won't really work without further effort. Here is the problem.
 
  First, the UUID plugin doesn't currently support this kind of
  operation. Either it needs to be modified or a new plugin
  needs to be created.
 
  Second, the client needs to know the ID in order to generate
  the token URI. If we generate the UUID inside the DS, the
  UUID is unknown to the client and the URI can't be generated.
  This would mean a new control.
 
  As I see it we have three options:
  1. Remove the option to specify the ipatokenUniqueID from the
  GUI. Don't make any change in the CLI.
 
  ENFORCEMENT: none
  EFFORT: low
 
  2. Perform a server-side check for admin membership. Raise an
  exception if the ipatokenUniqueID is specified and the user is
  not an admin.
 
  ENFORCEMENT: API-level
  EFFORT: medium
 
  3. Modify otptoken-add to create tokens with a magical
  ipatokenUniqueID value by default. An ACI would prevent normal
  users from adding tokens without this magic value.
  Create/modify a plugin to generate UUIDs when the magic value
  is found. Send a control back to the client indicating the
  real ipatokenUniqueID value. Modify otptoken-add to read this
  control.
 
  ENFORCEMENT: DS-level
  EFFORT: high
 
  I think my preference for now is #1. Thoughts?
 
  Nathaniel
 
 
 I want to raise a question if we really want to disable this
 feature for normal users. Wouldn't it be better to improve error
 message or check if the name is taken?
 
 UUID is the value which is displayed in the Soft. Token
 application as name. Sure, one can rename it there to match the
 description in IPA but that seems quite unpleasant to me. Also
 user has to know that he can rename the token in FreeOTP,
 
 Atm the existence check might be little problematic - disclose
 of information which is not readable to user by default. But
 current state is basically it, just unfriendly.

The existence check is impossible: users can't see tokens they
don't own.
   
   Why don't we simply allow user to use an arbitrary name of their
   choosing (or a random string if not specified) and then append their
   uid string to it in the UI ?
   
   I wonder if we have ACIs where the value must match a pattern
   defined by the bound user ...
  
  This would present a strange scenario where a token is created and
  then reassigned to another user.
 
 
 The idea is to strip away the tail when visualizing to the user, so it
 wouldn't really bee seen ?
 Or perhaps the token can simply be renamed when assigned.

It just feels like a kludge to me. Also, automated stripping may run
afoul of names assigned by the imports.

What is wrong with the three options I provided above? We can do #1 now
and #2 or #3 later.

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


Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name

2014-09-16 Thread Simo Sorce
On Tue, 16 Sep 2014 12:33:16 -0400
Nathaniel McCallum npmccal...@redhat.com wrote:

 On Tue, 2014-09-16 at 12:28 -0400, Simo Sorce wrote:
  On Tue, 16 Sep 2014 12:23:51 -0400
  Nathaniel McCallum npmccal...@redhat.com wrote:
  
   On Tue, 2014-09-16 at 12:22 -0400, Simo Sorce wrote:
On Tue, 16 Sep 2014 11:52:43 -0400
Nathaniel McCallum npmccal...@redhat.com wrote:

 On Tue, 2014-09-16 at 17:51 +0200, Petr Vobornik wrote:
  On 16.9.2014 17:26, Nathaniel McCallum wrote:
   On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum
   wrote:
   Admins need the ability to specify the token ID in the
   case of imports. However, generally, this ability is not
   needed.
  
   Is it possible to offload the ID generation to the
   ipa-uuid plugin? I'm not quite sure how to enable this
   (I think it involves passing a magic value?). But I'm
   not quite sure how this fits in with the IPA framework
   as the generated value is the DN.
  
   However, assuming this can be used, I propose the
   following. The token ID is removed from the UI for
   regular users (but retained for admins). We change the
   ACIs for token addition/modification to prevent regular
   users from specifying the ID in an add or mod operation.
   The CLI would retain the option to set it, but this
   option would only be usable by admins.
  
   Make sense?
  
   Nobody has responded to this. :)
  
   However, since investigating it a bit more, this approach
   won't really work without further effort. Here is the
   problem.
  
   First, the UUID plugin doesn't currently support this
   kind of operation. Either it needs to be modified or a
   new plugin needs to be created.
  
   Second, the client needs to know the ID in order to
   generate the token URI. If we generate the UUID inside
   the DS, the UUID is unknown to the client and the URI
   can't be generated. This would mean a new control.
  
   As I see it we have three options:
   1. Remove the option to specify the ipatokenUniqueID from
   the GUI. Don't make any change in the CLI.
  
   ENFORCEMENT: none
   EFFORT: low
  
   2. Perform a server-side check for admin membership.
   Raise an exception if the ipatokenUniqueID is specified
   and the user is not an admin.
  
   ENFORCEMENT: API-level
   EFFORT: medium
  
   3. Modify otptoken-add to create tokens with a magical
   ipatokenUniqueID value by default. An ACI would prevent
   normal users from adding tokens without this magic value.
   Create/modify a plugin to generate UUIDs when the magic
   value is found. Send a control back to the client
   indicating the real ipatokenUniqueID value. Modify
   otptoken-add to read this control.
  
   ENFORCEMENT: DS-level
   EFFORT: high
  
   I think my preference for now is #1. Thoughts?
  
   Nathaniel
  
  
  I want to raise a question if we really want to disable this
  feature for normal users. Wouldn't it be better to improve
  error message or check if the name is taken?
  
  UUID is the value which is displayed in the Soft. Token
  application as name. Sure, one can rename it there to match
  the description in IPA but that seems quite unpleasant to
  me. Also user has to know that he can rename the token in
  FreeOTP,
  
  Atm the existence check might be little problematic -
  disclose of information which is not readable to user by
  default. But current state is basically it, just unfriendly.
 
 The existence check is impossible: users can't see tokens they
 don't own.

Why don't we simply allow user to use an arbitrary name of their
choosing (or a random string if not specified) and then append
their uid string to it in the UI ?

I wonder if we have ACIs where the value must match a pattern
defined by the bound user ...
   
   This would present a strange scenario where a token is created and
   then reassigned to another user.
  
  
  The idea is to strip away the tail when visualizing to the user, so
  it wouldn't really bee seen ?
  Or perhaps the token can simply be renamed when assigned.
 
 It just feels like a kludge to me. Also, automated stripping may run
 afoul of names assigned by the imports.
 
 What is wrong with the three options I provided above? We can do #1
 now and #2 or #3 later.

Nothing wrong, really, #1 is fine by me.

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] [PATCH 0064] Create ipa-otp-decrement 389DS plugin

2014-09-16 Thread thierry bordaz

On 09/15/2014 09:05 PM, Nathaniel McCallum wrote:

This plugin ensures that all counter/watermark operations are atomic
and never decrement. Also, deletion is not permitted.

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


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

Hello Nathaniel,

   Starting looking at it, I have just a question about sanitize_input.
   If the modification (replace) is related to counter/watermark, it
   triggers an internal search on the target entry itself.
   The original/modified entry is also present in the pblock. The
   internal search will check the filter but except that what is the
   benefit vs
   taking the entry directly in the pblock.

   thanks
   thierry

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

Re: [Freeipa-devel] [PATCH 0064] Create ipa-otp-decrement 389DS plugin

2014-09-16 Thread Nathaniel McCallum
On Tue, 2014-09-16 at 19:24 +0200, thierry bordaz wrote:
 On 09/15/2014 09:05 PM, Nathaniel McCallum wrote:
 
  This plugin ensures that all counter/watermark operations are atomic
  and never decrement. Also, deletion is not permitted.
  
  https://fedorahosted.org/freeipa/ticket/4494
  
  
  ___
  Freeipa-devel mailing list
  Freeipa-devel@redhat.com
  https://www.redhat.com/mailman/listinfo/freeipa-devel
 Hello Nathaniel,
 
 Starting looking at it, I have just a question about
 sanitize_input.
 If the modification (replace) is related to counter/watermark,
 it triggers an internal search on the target entry itself.
 The original/modified entry is also present in the pblock. The
 internal search will check the filter but except that what is
 the benefit vs
 taking the entry directly in the pblock.

I didn't know the entry was already in the pblock. What loads it? And
when? How do I access it?

Nathaniel

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


[Freeipa-devel] [PATCH] [dyndb] Fix error handling in configure_view() to prevent deadlocks

2014-09-16 Thread Petr Spacek

Hello,

attached patches fix
https://bugzilla.redhat.com/show_bug.cgi?id=1142150
https://bugzilla.redhat.com/show_bug.cgi?id=1142152

... and improve related error messages.

I will push it to https://github.com/spacekpe/bind-dynamic_db if you are okay 
with it.


--
Petr^2 Spacek
From d2d0b25e7586c306fc5e6ac62a82f680e70a91e8 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 16 Sep 2014 19:25:34 +0200
Subject: [PATCH] Fix error handling in configure_view() to prevent deadlocks.

Signed-off-by: Petr Spacek pspa...@redhat.com
---
 bin/named/server.c   |  8 +++-
 lib/dns/dynamic_db.c | 10 +-
 lib/dns/include/dns/dynamic_db.h |  2 +-
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/bin/named/server.c b/bin/named/server.c
index 6236c1014f07bdb826308290d98a0c5112732b31..0ea512b4b7673a1bf3a88506c6922e5d74d6b909 100644
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -2178,6 +2178,7 @@ configure_view(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig,
 	unsigned int query_timeout, ndisp;
 	struct cfg_context *nzctx;
 	dns_rpz_zone_t *rpz;
+	dns_dyndb_arguments_t *args = NULL;
 
 	REQUIRE(DNS_VIEW_VALID(view));
 
@@ -3341,8 +3342,6 @@ configure_view(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig,
 		(void)cfg_map_get(config, dynamic-db, dynamic_db_list);
 	element = cfg_list_first(dynamic_db_list);
 	if (element != NULL) {
-		dns_dyndb_arguments_t *args;
-
 		args = dns_dyndb_arguments_create(mctx);
 		if (args == NULL) {
 			result = ISC_R_NOMEMORY;
@@ -3355,11 +3354,8 @@ configure_view(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig,
 		while (element != NULL) {
 			obj = cfg_listelt_value(element);
 			CHECK(configure_dynamic_db(obj, mctx, args));
-
 			element = cfg_list_next(element);
 		}
-
-		dns_dyndb_arguments_destroy(mctx, args);
 	}
 
 	/*
@@ -3547,6 +3543,8 @@ configure_view(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig,
 
 	if (cache != NULL)
 		dns_cache_detach(cache);
+	if (args != NULL)
+		dns_dyndb_arguments_destroy(mctx, args);
 
 	return (result);
 }
diff --git a/lib/dns/dynamic_db.c b/lib/dns/dynamic_db.c
index bf831617b391778ec540b2a5ca0df341937f2427..30c56a65c7227497c3e772c3e1b58ff49eacbd35 100644
--- a/lib/dns/dynamic_db.c
+++ b/lib/dns/dynamic_db.c
@@ -280,16 +280,24 @@ dns_dyndb_arguments_create(isc_mem_t *mctx)
 }
 
 void
-dns_dyndb_arguments_destroy(isc_mem_t *mctx, dns_dyndb_arguments_t *args)
+dns_dyndb_arguments_destroy(isc_mem_t *mctx, dns_dyndb_arguments_t **argsp)
 {
+	dns_dyndb_arguments_t *args;
+
 	REQUIRE(args != NULL);
 
+	args = *argsp;
+	if (args == NULL)
+		return;
+
 	dns_dyndb_set_view(args, NULL);
 	dns_dyndb_set_zonemgr(args, NULL);
 	dns_dyndb_set_task(args, NULL);
 	dns_dyndb_set_timermgr(args, NULL);
 
 	isc_mem_put(mctx, args, sizeof(*args));
+
+	*argsp = NULL;
 }
 
 void
diff --git a/lib/dns/include/dns/dynamic_db.h b/lib/dns/include/dns/dynamic_db.h
index 673ad4bc12bfe0875d7bac69a219574e11674bcf..e50491d33c4cd4a9e600110926349af32622c24a 100644
--- a/lib/dns/include/dns/dynamic_db.h
+++ b/lib/dns/include/dns/dynamic_db.h
@@ -35,7 +35,7 @@ isc_result_t dns_dynamic_db_load(const char *libname, const char *name,
 void dns_dynamic_db_cleanup(isc_boolean_t exiting);
 
 dns_dyndb_arguments_t *dns_dyndb_arguments_create(isc_mem_t *mctx);
-void dns_dyndb_arguments_destroy(isc_mem_t *mctx, dns_dyndb_arguments_t *args);
+void dns_dyndb_arguments_destroy(isc_mem_t *mctx, dns_dyndb_arguments_t **args);
 
 void dns_dyndb_set_view(dns_dyndb_arguments_t *args, dns_view_t *view);
 dns_view_t *dns_dyndb_get_view(dns_dyndb_arguments_t *args);
-- 
1.9.3

From 378bdc0646e1738b39f5da2e26da7753d7b562a6 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 16 Sep 2014 19:26:03 +0200
Subject: [PATCH] Log error if dynamic database configuration failed.

Signed-off-by: Petr Spacek pspa...@redhat.com
---
 bin/named/server.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/bin/named/server.c b/bin/named/server.c
index 0ea512b4b7673a1bf3a88506c6922e5d74d6b909..590b32923b8fdfd68caabfc57a1dbd127a234ce3 100644
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -1308,6 +1308,11 @@ configure_dynamic_db(const cfg_obj_t *dynamic_db, isc_mem_t *mctx,
 	CHECK(dns_dynamic_db_load(libname, name, mctx, argv, dyndb_args));
 
 cleanup:
+	if (result != ISC_R_SUCCESS)
+		isc_log_write(ns_g_lctx, NS_LOGCATEGORY_GENERAL,
+		  NS_LOGMODULE_SERVER, ISC_LOG_ERROR,
+		  dynamic database '%s' configuration failed: %s,
+		  name, isc_result_totext(result));
 	if (argv != NULL)
 		isc_mem_free(mctx, argv);
 
-- 
1.9.3

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

[Freeipa-devel] [PATCH 0065] Don't allow users to create tokens with a specified ID

2014-09-16 Thread Nathaniel McCallum
We perform this enforcement at the API level since:
* DS level enforcement would be difficult
* ipatokenUniqueID generation already happens at the API level

It may be nice in the future to perform enforcement in the DS itself.
However, the question of the location of enforcement is largely an
aesthetic issue.

https://fedorahosted.org/freeipa/ticket/4456
From d52d7b5ed8514057a0d55ac2009576dadcec1cef Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Tue, 16 Sep 2014 13:21:05 -0400
Subject: [PATCH] Don't allow users to create tokens with a specified ID

We perform this enforcement at the API level since:
* DS level enforcement would be difficult
* ipatokenUniqueID generation already happens at the API level

It may be nice in the future to perform enforcement in the DS itself.
However, the question of the location of enforcement is largely an
aesthetic issue.

https://fedorahosted.org/freeipa/ticket/4456
---
 ipalib/plugins/otptoken.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index 1bd85d4b952dc51ea800ed37c49b3c50aeb31492..231e383e3a3bc8506bed1c72f2859c2832c240a1 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -259,6 +259,14 @@ class otptoken_add(LDAPCreate):
 entry_attrs['ipatokenuniqueid'] = str(uuid.uuid4())
 dn = DN(ipatokenuniqueid=%s % entry_attrs['ipatokenuniqueid'], dn)
 
+# Ensure that no token id is specified if the user is not an admin.
+else:
+gresult = self.api.Command.group_show(cn=u'admins')['result']
+uresult = self.api.Command.user_find(whoami=True)['result']
+if uresult[0]['uid'][0] not in gresult['member_user']:
+raise ValidationError(name='ipatokenuniqueid',
+  error='can only be specified by admins')
+
 if not _check_interval(options.get('ipatokennotbefore', None),
options.get('ipatokennotafter', None)):
 raise ValidationError(name='not_after',
-- 
2.1.0

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

Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name

2014-09-16 Thread Nathaniel McCallum
On Tue, 2014-09-16 at 12:52 -0400, Simo Sorce wrote:
 On Tue, 16 Sep 2014 12:33:16 -0400
 Nathaniel McCallum npmccal...@redhat.com wrote:
 
  On Tue, 2014-09-16 at 12:28 -0400, Simo Sorce wrote:
   On Tue, 16 Sep 2014 12:23:51 -0400
   Nathaniel McCallum npmccal...@redhat.com wrote:
   
On Tue, 2014-09-16 at 12:22 -0400, Simo Sorce wrote:
 On Tue, 16 Sep 2014 11:52:43 -0400
 Nathaniel McCallum npmccal...@redhat.com wrote:
 
  On Tue, 2014-09-16 at 17:51 +0200, Petr Vobornik wrote:
   On 16.9.2014 17:26, Nathaniel McCallum wrote:
On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum
wrote:
Admins need the ability to specify the token ID in the
case of imports. However, generally, this ability is not
needed.
   
Is it possible to offload the ID generation to the
ipa-uuid plugin? I'm not quite sure how to enable this
(I think it involves passing a magic value?). But I'm
not quite sure how this fits in with the IPA framework
as the generated value is the DN.
   
However, assuming this can be used, I propose the
following. The token ID is removed from the UI for
regular users (but retained for admins). We change the
ACIs for token addition/modification to prevent regular
users from specifying the ID in an add or mod operation.
The CLI would retain the option to set it, but this
option would only be usable by admins.
   
Make sense?
   
Nobody has responded to this. :)
   
However, since investigating it a bit more, this approach
won't really work without further effort. Here is the
problem.
   
First, the UUID plugin doesn't currently support this
kind of operation. Either it needs to be modified or a
new plugin needs to be created.
   
Second, the client needs to know the ID in order to
generate the token URI. If we generate the UUID inside
the DS, the UUID is unknown to the client and the URI
can't be generated. This would mean a new control.
   
As I see it we have three options:
1. Remove the option to specify the ipatokenUniqueID from
the GUI. Don't make any change in the CLI.
   
ENFORCEMENT: none
EFFORT: low
   
2. Perform a server-side check for admin membership.
Raise an exception if the ipatokenUniqueID is specified
and the user is not an admin.
   
ENFORCEMENT: API-level
EFFORT: medium
   
3. Modify otptoken-add to create tokens with a magical
ipatokenUniqueID value by default. An ACI would prevent
normal users from adding tokens without this magic value.
Create/modify a plugin to generate UUIDs when the magic
value is found. Send a control back to the client
indicating the real ipatokenUniqueID value. Modify
otptoken-add to read this control.
   
ENFORCEMENT: DS-level
EFFORT: high
   
I think my preference for now is #1. Thoughts?
   
Nathaniel
   
   
   I want to raise a question if we really want to disable this
   feature for normal users. Wouldn't it be better to improve
   error message or check if the name is taken?
   
   UUID is the value which is displayed in the Soft. Token
   application as name. Sure, one can rename it there to match
   the description in IPA but that seems quite unpleasant to
   me. Also user has to know that he can rename the token in
   FreeOTP,
   
   Atm the existence check might be little problematic -
   disclose of information which is not readable to user by
   default. But current state is basically it, just unfriendly.
  
  The existence check is impossible: users can't see tokens they
  don't own.
 
 Why don't we simply allow user to use an arbitrary name of their
 choosing (or a random string if not specified) and then append
 their uid string to it in the UI ?
 
 I wonder if we have ACIs where the value must match a pattern
 defined by the bound user ...

This would present a strange scenario where a token is created and
then reassigned to another user.
   
   
   The idea is to strip away the tail when visualizing to the user, so
   it wouldn't really bee seen ?
   Or perhaps the token can simply be renamed when assigned.
  
  It just feels like a kludge to me. Also, automated stripping may run
  afoul of names assigned by the imports.
  
  What is wrong with the three options I provided above? We can do #1
  now and #2 or #3 later.
 
 Nothing wrong, really, #1 is fine by me.

I settled on #2 since it turned out to be somewhat easy. I think we also
need an update to the UI (aka #1).

This should be an easy review:

Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.

2014-09-16 Thread David Kupka

On 09/16/2014 06:09 PM, Martin Basti wrote:

On 16/09/14 15:59, David Kupka wrote:

On 09/12/2014 07:24 PM, Martin Basti wrote:

snip /

Be careful, reviewed on friday! :-)

1)
whitespace error + pep8 error
patch:76: trailing whitespace.
 # there is reverse zone for every ip address
warning: 1 line adds whitespace errors.

./ipaserver/install/bindinstance.py:640:9: E265 block comment should
start with '# '



Stupid mistake, sorry.



2) (server install)
+for ip in ip_addresses:
+for rev_zone in reverse_zones:
+if bindinstance.verify_reverse_zone(rev_zone, str(ip)):
+break
+else:
+sys.exit(1)

Please add there error message instead 1 to exit function


You are right, it's better to say what's wrong.



3) (server install)
Code above, bindinstance.verify_reverse_zone(rev_zone, str(ip)):
IMHO this will cause errors (not tested) try:
rev-zones: [10.10.10.in-addr.arpa., 16.172.in-addr.arpa.]
ip_addreses: [10.10.10.1, 172.16.0.1]

it should be any() of zone can handle ip


I indented the else branch more that I wanted.



4) (replica-install)
I'm not sure about behavior of combination ip addresses, and reverse
zones,
In original code, if user specify reverse zone, the ip address must
match.

In patched version, you just add new zones for ip-addresses which
doesn't math the user zones.

IMO we should check if all addresses belongs to user specified zones,
otherwise raise an error.
But I'm open to suggestions.


The behavior now basically is:

Check if IP address matches any specified zone
  a. if yes we are good
  b. else search if there is existing zone that can be used
ba. if yes we are good
bb. else ask user for zone or create default if --unattended



5)
Code for ipa-replica-install, and ipa-server-install, differs in parts
where we expecting same behavior
   - ip addresses and reverse zones


The behavoir now should be almost the same. The only difference is
that when installing replica we need to search for existing reverse
zones as they could be added during on another server.



6)
https://engineering.redhat.com/irc-services.txt+# there is at
least one ip address in every zone
+if options.reverse_zones:
+for reverse_zone in options.reverse_zones:
+for ip in config.ips:
--

A
+if bindinstance.verify_reverse_zone(reverse_zone, ip):
+ reverse_zones.append(bindinstance.normalize_zone(reverse_zone))
+break
+else:
+sys.exit(1)
*

+# there is reverse zone for every ip address
+for ip in config.ips:
+for reverse_zone in options.reverse_zones:
--- B
+if bindinstance.verify_reverse_zone(reverse_zone, ip):
+if reverse_zone not in reverse_zones:
+ reverse_zones.append(bindinstance.normalize_zone(reverse_zone))
+break
+else:


C
+reverse_zone = bindinstance.find_reverse_zone(ip)
+if reverse_zone is None and not options.no_reverse:
+reverse_zone = util.get_reverse_zone_default(ip)
- D1
+if not options.unattended and
bindinstance.create_reverse(): - D
+reverse_zone =
bindinstance.read_reverse_zone(reverse_zone, ip)
+ reverse_zones.append(bindinstance.normalize_zone(reverse_zone))
- D2

You added all possible zones in step A,  B step is not needed.
If user doesn't specify reverse_zones you can just jump to C step
*add error message
C: elif not options.no_reverse
D: you asked user if wants to create zone, but you don't care about
answer, and in step D2 append zone from D1
note: --no-reverse and --reverse-zones cant be used together, you can
use it in new code, test it before cycle


Rewritten.



7)
  # Always use force=True as named is not set up yet
  add_zone(self.domain, self.zonemgr,
dns_backup=self.dns_backup,
-ns_hostname=api.env.host,
ns_ip_address=nameserver_ip_address,
-force=True)
+ ns_hostname=api.env.host, ns_ip_address=None,
force=True)
+#for ns_ip_address in nameserver_ip_address:
+#add_zone(self.domain, self.zonemgr,
dns_backup=self.dns_backup,
+#ns_hostname=api.env.host,
ns_ip_address=ns_ip_address,
+#force=True)

Please remove commented section


I forget to clean the trash, thanks.



You can remove 'ns_ip_address=None' from function

8)
+if set(options.ip_addresses) = set(ips):
+ips = options.ip_addresses
+else:
+