Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins

2015-03-12 Thread Petr Spacek
On 12.3.2015 16:23, Rob Crittenden wrote:
 David Kupka wrote:
 On 03/06/2015 06:00 PM, Martin Basti wrote:
 Upgrade plugins which modify LDAP data directly should not be executed
 in --test mode.

 This patch is a workaround, to ensure update with --test option will not
 modify any LDAP data.

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

 Patch attached.




 Ideally we want to fix all plugins to dry-run the upgrade not just skip
 when there is '--test' option but it is a good first step.
 Works for me, ACK.

 
 I agree that this breaks the spirit of --test and think it should be
 fixed before committing.

Considering how often is the option is used ... I do not think that this
requires 'proper' fix now. It was broken for *years* so this patch is a huge
improvement and IMHO should be commited in current form. We can re-visit it
later on, open a ticket :-)

-- 
Petr^2 Spacek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Generic support for unknown DNS RR types (RFC 3597)

2015-03-12 Thread Petr Spacek
On 11.3.2015 17:02, Martin Kosek wrote:
 On 03/11/2015 04:55 PM, Petr Spacek wrote:
 On 11.3.2015 15:45, Martin Kosek wrote:
 On 03/11/2015 03:38 PM, Petr Spacek wrote:
 On 11.3.2015 15:28, Martin Kosek wrote:
 On 03/11/2015 12:43 PM, Petr Spacek wrote:
 On 11.3.2015 11:34, Jan Cholasta wrote:
 Dne 11.3.2015 v 11:12 Petr Spacek napsal(a):
 On 10.3.2015 20:04, Simo Sorce wrote:
 On Tue, 2015-03-10 at 19:24 +0100, Petr Spacek wrote:
 On 10.3.2015 18:36, Simo Sorce wrote:
 On Tue, 2015-03-10 at 18:26 +0100, Petr Spacek wrote:
 On 10.3.2015 17:35, Simo Sorce wrote:
 On Tue, 2015-03-10 at 16:19 +0100, Petr Spacek wrote:
 On 10.3.2015 15:53, Simo Sorce wrote:
 On Tue, 2015-03-10 at 15:32 +0100, Petr Spacek wrote:
 Hello,

 I would like to discuss Generic support for unknown DNS RR 
 types
 (RFC 3597
 [0]). Here is the proposal:

 LDAP schema
 ===
 - 1 new attribute:
 ( OID NAME 'GenericRecord' DESC 'unknown DNS record, RFC 
 3597'
 EQUALITY
 caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )

 The attribute should be added to existing idnsRecord object 
 class as
 MAY.

 This new attribute should contain data encoded according to 
 ​RFC
 3597 section
 5 [5]:

 The RDATA section of an RR of unknown type is represented as a
 sequence of white space separated words as follows:

The special token \# (a backslash immediately followed 
 by a hash
sign), which identifies the RDATA as having the generic 
 encoding
defined herein rather than a traditional type-specific 
 encoding.

An unsigned decimal integer specifying the RDATA length 
 in
 octets.

Zero or more words of hexadecimal data encoding the 
 actual RDATA
field, each containing an even number of hexadecimal 
 digits.

 If the RDATA is of zero length, the text representation 
 contains
 only
 the \# token and the single zero representing the length.

 Examples from RFC:
a.example.   CLASS32 TYPE731 \# 6 abcd (
 ef 01 23 45 )
b.example.   HS  TYPE62347   \# 0
e.example.   IN  A   \# 4 0A01
e.example.   CLASS1  TYPE1   10.0.0.2


 Open questions about LDAP format
 
 Should we include \# constant? We know that the attribute 
 contains
 record in
 RFC 3597 syntax so it is not strictly necessary.

 I think it would be better to follow RFC 3597 format. It 
 allows blind
 copypasting from other tools, including direct calls to 
 python-dns.

 It also eases writing conversion tools between DNS and LDAP 
 format
 because
 they do not need to change record values.


 Another question is if we should explicitly include length of 
 data
 represented
 in hexadecimal notation as a decimal number. I'm very strongly
 inclined to let
 it there because it is very good sanity check and again, it 
 allows
 us to
 re-use existing tools including parsers.

 I will ask Uninett.no for standardization after we sort this 
 out
 (they own the
 OID arc we use for DNS records).


 Attribute usage
 ===
 Every DNS RR type has assigned a number [1] which is used on 
 wire.
 RR types
 which are unknown to the server cannot be named by their
 mnemonic/type name
 because server would not be able to do name-number conversion 
 and
 to generate
 DNS wire format.

 As a result, we have to encode the RR type number somehow. 
 Let's use
 attribute
 sub-types.

 E.g. a record with type 65280 and hex value 0A01 will be
 represented as:
 GenericRecord;TYPE65280: \# 4 0A01


 CLI
 ===
 $ ipa dnsrecord-add zone.example owner \
--generic-type=65280 --generic-data='\# 4 0A01'

 $ ipa dnsrecord-show zone.example owner
 Record name: owner
 TYPE65280 Record: \# 4 0A01


 ACK? :-)

 Almost.
 We should refrain from using subtypes when not necessary, and 
 in this
 case it is not necessary.

 Use:
 GenericRecord: 65280 \# 4 0A01

 I was considering that too but I can see two main drawbacks:

 1) It does not work very well with DS ACI (targetattrfilter, 
 anyone?).
 Adding
 generic write access to GenericRecord == ability to add TLSA 
 records too,
 which you may not want. IMHO it is perfectly reasonable to limit 
 write
 access
 to certain types (e.g. to one from private range).

 2) We would need a separate substring index for emulating 
 filters like
 (type==65280). AFAIK GenericRecord;TYPE65280 should work with 
 presence
 index
 which will be handy one day when we decide to handle upgrades 
 like
 GenericRecord;TYPE256-UriRecord.

 Another (less important) annoyance is that conversion tools 
 would have to
 mangle record data instead of just converting attribute 
 name-record
 type.


 I can be convinced that subtypes are not necessary but I do not 
 see clear
 advantage of avoiding them. What is the problem with subtypes?

 Poor support by most clients, so it is generally discouraged.
 Hmm, it does not sound like a thing we

Re: [Freeipa-devel] Purpose of default user group

2015-03-12 Thread Petr Spacek
On 10.3.2015 16:01, Jakub Hrozek wrote:
 On Tue, Mar 10, 2015 at 03:52:44PM +0100, Martin Kosek wrote:
 On 03/10/2015 03:27 PM, Rob Crittenden wrote:
 Petr Vobornik wrote:
 Hi,

 I would like to ask what is a purpose of a default user group - by
 default ipausers? Default group is also a required field in ipa config.

 To be able to apply some (undefined) group policy to all users. I'm not
 aware that it has ever been used for this.

 I would also interested in the use cases, especially given all the pain we 
 have
 with ipausers and large user bases. Especially that for current policies 
 (SUDO,
 HBAC, SELinux user policy), we always have other means to specify all 
 users.
 
 yes, but those means usually specify both AD and IPA users, right?
 
 I always thought ipausers is a handy shortcut for selecting IPA users
 only and not AD users.

I always thought that ipausers is an equivalent of domain users in AD
world (compare with Trusted domain users).

In my admin life I considered domain users to be useful alias for real
authenticated user accounts (compare with Everyone = even unauthenticated
access, Authenticated users = includes machine accounts too.)


Moreover, getting rid of ipausers does not help with 'big groups problem' in
any way. E.g. at university you are almost inevitably going to have groups
like 'students' which will contain more than 90 % of users anyway.

-- 
Petr^2 Spacek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Generic support for unknown DNS RR types (RFC 3597)

2015-03-11 Thread Petr Spacek
On 10.3.2015 20:04, Simo Sorce wrote:
 On Tue, 2015-03-10 at 19:24 +0100, Petr Spacek wrote:
 On 10.3.2015 18:36, Simo Sorce wrote:
 On Tue, 2015-03-10 at 18:26 +0100, Petr Spacek wrote:
 On 10.3.2015 17:35, Simo Sorce wrote:
 On Tue, 2015-03-10 at 16:19 +0100, Petr Spacek wrote:
 On 10.3.2015 15:53, Simo Sorce wrote:
 On Tue, 2015-03-10 at 15:32 +0100, Petr Spacek wrote:
 Hello,

 I would like to discuss Generic support for unknown DNS RR types (RFC 
 3597
 [0]). Here is the proposal:

 LDAP schema
 ===
 - 1 new attribute:
 ( OID NAME 'GenericRecord' DESC 'unknown DNS record, RFC 3597' 
 EQUALITY
 caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )

 The attribute should be added to existing idnsRecord object class as 
 MAY.

 This new attribute should contain data encoded according to ​RFC 3597 
 section
 5 [5]:

 The RDATA section of an RR of unknown type is represented as a
sequence of white space separated words as follows:

   The special token \# (a backslash immediately followed by a hash
   sign), which identifies the RDATA as having the generic encoding
   defined herein rather than a traditional type-specific encoding.

   An unsigned decimal integer specifying the RDATA length in 
 octets.

   Zero or more words of hexadecimal data encoding the actual RDATA
   field, each containing an even number of hexadecimal digits.

If the RDATA is of zero length, the text representation contains 
 only
the \# token and the single zero representing the length.

 Examples from RFC:
   a.example.   CLASS32 TYPE731 \# 6 abcd (
ef 01 23 45 )
   b.example.   HS  TYPE62347   \# 0
   e.example.   IN  A   \# 4 0A01
   e.example.   CLASS1  TYPE1   10.0.0.2


 Open questions about LDAP format
 
 Should we include \# constant? We know that the attribute contains 
 record in
 RFC 3597 syntax so it is not strictly necessary.

 I think it would be better to follow RFC 3597 format. It allows blind
 copypasting from other tools, including direct calls to python-dns.

 It also eases writing conversion tools between DNS and LDAP format 
 because
 they do not need to change record values.


 Another question is if we should explicitly include length of data 
 represented
 in hexadecimal notation as a decimal number. I'm very strongly 
 inclined to let
 it there because it is very good sanity check and again, it allows us 
 to
 re-use existing tools including parsers.

 I will ask Uninett.no for standardization after we sort this out (they 
 own the
 OID arc we use for DNS records).


 Attribute usage
 ===
 Every DNS RR type has assigned a number [1] which is used on wire. RR 
 types
 which are unknown to the server cannot be named by their mnemonic/type 
 name
 because server would not be able to do name-number conversion and to 
 generate
 DNS wire format.

 As a result, we have to encode the RR type number somehow. Let's use 
 attribute
 sub-types.

 E.g. a record with type 65280 and hex value 0A01 will be 
 represented as:
 GenericRecord;TYPE65280: \# 4 0A01


 CLI
 ===
 $ ipa dnsrecord-add zone.example owner \
   --generic-type=65280 --generic-data='\# 4 0A01'

 $ ipa dnsrecord-show zone.example owner
 Record name: owner
 TYPE65280 Record: \# 4 0A01


 ACK? :-)

 Almost.
 We should refrain from using subtypes when not necessary, and in this
 case it is not necessary.

 Use:
 GenericRecord: 65280 \# 4 0A01

 I was considering that too but I can see two main drawbacks:

 1) It does not work very well with DS ACI (targetattrfilter, anyone?). 
 Adding
 generic write access to GenericRecord == ability to add TLSA records too,
 which you may not want. IMHO it is perfectly reasonable to limit write 
 access
 to certain types (e.g. to one from private range).

 2) We would need a separate substring index for emulating filters like
 (type==65280). AFAIK GenericRecord;TYPE65280 should work with presence 
 index
 which will be handy one day when we decide to handle upgrades like
 GenericRecord;TYPE256-UriRecord.

 Another (less important) annoyance is that conversion tools would have to
 mangle record data instead of just converting attribute name-record 
 type.


 I can be convinced that subtypes are not necessary but I do not see clear
 advantage of avoiding them. What is the problem with subtypes?

 Poor support by most clients, so it is generally discouraged.
 Hmm, it does not sound like a thing we should care in this case. DNS tree 
 is
 not meant for direct consumption by LDAP clients (compare with cn=compat).

 IMHO the only two clients we should care are FreeIPA framework and
 bind-dyndb-ldap so I do not see this as a problem, really. If someone 
 wants to
 access DNS tree by hand - sure, use a standard compliant client!

 Working ACI and LDAP filters sounds like good price for supporting

Re: [Freeipa-devel] Generic support for unknown DNS RR types (RFC 3597)

2015-03-11 Thread Petr Spacek
On 11.3.2015 08:13, Martin Kosek wrote:
 On 03/10/2015 07:24 PM, Petr Spacek wrote:
 On 10.3.2015 18:36, Simo Sorce wrote:
 On Tue, 2015-03-10 at 18:26 +0100, Petr Spacek wrote:
 On 10.3.2015 17:35, Simo Sorce wrote:
 On Tue, 2015-03-10 at 16:19 +0100, Petr Spacek wrote:
 On 10.3.2015 15:53, Simo Sorce wrote:
 On Tue, 2015-03-10 at 15:32 +0100, Petr Spacek wrote:
 Hello,

 I would like to discuss Generic support for unknown DNS RR types (RFC 
 3597
 [0]). Here is the proposal:

 LDAP schema
 ===
 - 1 new attribute:
 ( OID NAME 'GenericRecord' DESC 'unknown DNS record, RFC 3597' 
 EQUALITY
 caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )

 The attribute should be added to existing idnsRecord object class as 
 MAY.

 This new attribute should contain data encoded according to ​RFC 3597 
 section
 5 [5]:

 The RDATA section of an RR of unknown type is represented as a
sequence of white space separated words as follows:

   The special token \# (a backslash immediately followed by a hash
   sign), which identifies the RDATA as having the generic encoding
   defined herein rather than a traditional type-specific encoding.

   An unsigned decimal integer specifying the RDATA length in 
 octets.

   Zero or more words of hexadecimal data encoding the actual RDATA
   field, each containing an even number of hexadecimal digits.

If the RDATA is of zero length, the text representation contains 
 only
the \# token and the single zero representing the length.

 Examples from RFC:
   a.example.   CLASS32 TYPE731 \# 6 abcd (
ef 01 23 45 )
   b.example.   HS  TYPE62347   \# 0
   e.example.   IN  A   \# 4 0A01
   e.example.   CLASS1  TYPE1   10.0.0.2


 Open questions about LDAP format
 
 Should we include \# constant? We know that the attribute contains 
 record in
 RFC 3597 syntax so it is not strictly necessary.

 I think it would be better to follow RFC 3597 format. It allows blind
 copypasting from other tools, including direct calls to python-dns.

 It also eases writing conversion tools between DNS and LDAP format 
 because
 they do not need to change record values.


 Another question is if we should explicitly include length of data 
 represented
 in hexadecimal notation as a decimal number. I'm very strongly 
 inclined to let
 it there because it is very good sanity check and again, it allows us 
 to
 re-use existing tools including parsers.

 I will ask Uninett.no for standardization after we sort this out (they 
 own the
 OID arc we use for DNS records).


 Attribute usage
 ===
 Every DNS RR type has assigned a number [1] which is used on wire. RR 
 types
 which are unknown to the server cannot be named by their mnemonic/type 
 name
 because server would not be able to do name-number conversion and to 
 generate
 DNS wire format.

 As a result, we have to encode the RR type number somehow. Let's use 
 attribute
 sub-types.

 E.g. a record with type 65280 and hex value 0A01 will be 
 represented as:
 GenericRecord;TYPE65280: \# 4 0A01


 CLI
 ===
 $ ipa dnsrecord-add zone.example owner \
   --generic-type=65280 --generic-data='\# 4 0A01'

 $ ipa dnsrecord-show zone.example owner
 Record name: owner
 TYPE65280 Record: \# 4 0A01


 ACK? :-)

 Almost.
 We should refrain from using subtypes when not necessary, and in this
 case it is not necessary.

 Use:
 GenericRecord: 65280 \# 4 0A01

 I was considering that too but I can see two main drawbacks:

 1) It does not work very well with DS ACI (targetattrfilter, anyone?). 
 Adding
 generic write access to GenericRecord == ability to add TLSA records too,
 which you may not want. IMHO it is perfectly reasonable to limit write 
 access
 to certain types (e.g. to one from private range).

 2) We would need a separate substring index for emulating filters like
 (type==65280). AFAIK GenericRecord;TYPE65280 should work with presence 
 index
 which will be handy one day when we decide to handle upgrades like
 GenericRecord;TYPE256-UriRecord.

 Another (less important) annoyance is that conversion tools would have to
 mangle record data instead of just converting attribute name-record 
 type.


 I can be convinced that subtypes are not necessary but I do not see clear
 advantage of avoiding them. What is the problem with subtypes?

 Poor support by most clients, so it is generally discouraged.
 Hmm, it does not sound like a thing we should care in this case. DNS tree 
 is
 not meant for direct consumption by LDAP clients (compare with cn=compat).

 IMHO the only two clients we should care are FreeIPA framework and
 bind-dyndb-ldap so I do not see this as a problem, really. If someone 
 wants to
 access DNS tree by hand - sure, use a standard compliant client!

 Working ACI and LDAP filters sounds like good price for supporting only

Re: [Freeipa-devel] Generic support for unknown DNS RR types (RFC 3597)

2015-03-11 Thread Petr Spacek
On 11.3.2015 11:34, Jan Cholasta wrote:
 Dne 11.3.2015 v 11:12 Petr Spacek napsal(a):
 On 10.3.2015 20:04, Simo Sorce wrote:
 On Tue, 2015-03-10 at 19:24 +0100, Petr Spacek wrote:
 On 10.3.2015 18:36, Simo Sorce wrote:
 On Tue, 2015-03-10 at 18:26 +0100, Petr Spacek wrote:
 On 10.3.2015 17:35, Simo Sorce wrote:
 On Tue, 2015-03-10 at 16:19 +0100, Petr Spacek wrote:
 On 10.3.2015 15:53, Simo Sorce wrote:
 On Tue, 2015-03-10 at 15:32 +0100, Petr Spacek wrote:
 Hello,

 I would like to discuss Generic support for unknown DNS RR types
 (RFC 3597
 [0]). Here is the proposal:

 LDAP schema
 ===
 - 1 new attribute:
 ( OID NAME 'GenericRecord' DESC 'unknown DNS record, RFC 3597'
 EQUALITY
 caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )

 The attribute should be added to existing idnsRecord object class as
 MAY.

 This new attribute should contain data encoded according to ​RFC
 3597 section
 5 [5]:

 The RDATA section of an RR of unknown type is represented as a
 sequence of white space separated words as follows:

The special token \# (a backslash immediately followed by a 
 hash
sign), which identifies the RDATA as having the generic 
 encoding
defined herein rather than a traditional type-specific 
 encoding.

An unsigned decimal integer specifying the RDATA length in
 octets.

Zero or more words of hexadecimal data encoding the actual 
 RDATA
field, each containing an even number of hexadecimal digits.

 If the RDATA is of zero length, the text representation contains
 only
 the \# token and the single zero representing the length.

 Examples from RFC:
a.example.   CLASS32 TYPE731 \# 6 abcd (
 ef 01 23 45 )
b.example.   HS  TYPE62347   \# 0
e.example.   IN  A   \# 4 0A01
e.example.   CLASS1  TYPE1   10.0.0.2


 Open questions about LDAP format
 
 Should we include \# constant? We know that the attribute contains
 record in
 RFC 3597 syntax so it is not strictly necessary.

 I think it would be better to follow RFC 3597 format. It allows blind
 copypasting from other tools, including direct calls to python-dns.

 It also eases writing conversion tools between DNS and LDAP format
 because
 they do not need to change record values.


 Another question is if we should explicitly include length of data
 represented
 in hexadecimal notation as a decimal number. I'm very strongly
 inclined to let
 it there because it is very good sanity check and again, it allows
 us to
 re-use existing tools including parsers.

 I will ask Uninett.no for standardization after we sort this out
 (they own the
 OID arc we use for DNS records).


 Attribute usage
 ===
 Every DNS RR type has assigned a number [1] which is used on wire.
 RR types
 which are unknown to the server cannot be named by their
 mnemonic/type name
 because server would not be able to do name-number conversion and
 to generate
 DNS wire format.

 As a result, we have to encode the RR type number somehow. Let's use
 attribute
 sub-types.

 E.g. a record with type 65280 and hex value 0A01 will be
 represented as:
 GenericRecord;TYPE65280: \# 4 0A01


 CLI
 ===
 $ ipa dnsrecord-add zone.example owner \
--generic-type=65280 --generic-data='\# 4 0A01'

 $ ipa dnsrecord-show zone.example owner
 Record name: owner
 TYPE65280 Record: \# 4 0A01


 ACK? :-)

 Almost.
 We should refrain from using subtypes when not necessary, and in this
 case it is not necessary.

 Use:
 GenericRecord: 65280 \# 4 0A01

 I was considering that too but I can see two main drawbacks:

 1) It does not work very well with DS ACI (targetattrfilter, anyone?).
 Adding
 generic write access to GenericRecord == ability to add TLSA records 
 too,
 which you may not want. IMHO it is perfectly reasonable to limit write
 access
 to certain types (e.g. to one from private range).

 2) We would need a separate substring index for emulating filters like
 (type==65280). AFAIK GenericRecord;TYPE65280 should work with presence
 index
 which will be handy one day when we decide to handle upgrades like
 GenericRecord;TYPE256-UriRecord.

 Another (less important) annoyance is that conversion tools would have 
 to
 mangle record data instead of just converting attribute name-record
 type.


 I can be convinced that subtypes are not necessary but I do not see 
 clear
 advantage of avoiding them. What is the problem with subtypes?

 Poor support by most clients, so it is generally discouraged.
 Hmm, it does not sound like a thing we should care in this case. DNS
 tree is
 not meant for direct consumption by LDAP clients (compare with 
 cn=compat).

 IMHO the only two clients we should care are FreeIPA framework and
 bind-dyndb-ldap so I do not see this as a problem, really. If someone
 wants to
 access DNS tree by hand - sure, use

Re: [Freeipa-devel] [PATCHES 0015-0017] consolidation of various Kerberos auth methods in FreeIPA code

2015-03-11 Thread Petr Spacek
Hello Martin^3,

good work, we are almost there! Please see my nitpicks in-line.

On 9.3.2015 13:06, Martin Babinsky wrote:
 On 03/06/2015 01:05 PM, Martin Babinsky wrote:
 This series of patches for the master/4.1 branch attempts to implement
 some of the Rob's and Petr Vobornik's ideas which originated from a
 discussion on this list regarding my original patch fixing
 https://fedorahosted.org/freeipa/ticket/4808.

 I suppose that these patches are just a first iteration, we may further
 discuss if this is the right thing to do.

 Below is a quote from the original discussion just to get the context:



 
 The next iteration of patches is attached below. Thanks to jcholast and
 pvoborni for the comments and insights.
 
 -- 
 Martin^3 Babinsky
 
 freeipa-mbabinsk-0015-2-ipautil-new-functions-kinit_keytab-and-kinit_passwor.patch
 
 
 From 97e4eed332391bab7a12dc593152e369f347fd3c Mon Sep 17 00:00:00 2001
 From: Martin Babinsky mbabi...@redhat.com
 Date: Mon, 9 Mar 2015 12:53:10 +0100
 Subject: [PATCH 1/3] ipautil: new functions kinit_keytab and kinit_password
 
 kinit_keytab replaces kinit_hostprincipal and performs Kerberos auth using
 keytab file. Function is also able to repeat authentication multiple times
 before giving up and raising StandardError.
 
 kinit_password wraps kinit auth using password and also supports FAST
 authentication using httpd armor ccache.
 ---
  ipapython/ipautil.py | 60 
 
  1 file changed, 46 insertions(+), 14 deletions(-)
 
 diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
 index 
 4116d974e620341119b56fad3cff1bda48af3bab..4547165ccf24ff6edf5c65e756aa321aa34b9e61
  100644
 --- a/ipapython/ipautil.py
 +++ b/ipapython/ipautil.py
 @@ -1175,27 +1175,59 @@ def wait_for_open_socket(socket_name, timeout=0):
  else:
  raise e
  
 -def kinit_hostprincipal(keytab, ccachedir, principal):
 +
 +def kinit_keytab(keytab, ccache_path, principal, attempts=1):
  
 -Given a ccache directory and a principal kinit as that user.
 +Given a ccache_path , keytab file and a principal kinit as that user.
 +
 +The optional parameter 'attempts' specifies how many times the credential
 +initialization should be attempted before giving up and raising
 +StandardError.
  
  This blindly overwrites the current CCNAME so if you need to save
  it do so before calling this function.
  
  Thus far this is used to kinit as the local host.
  
 -try:
 -ccache_file = 'FILE:%s/ccache' % ccachedir
 -krbcontext = krbV.default_context()
 -ktab = krbV.Keytab(name=keytab, context=krbcontext)
 -princ = krbV.Principal(name=principal, context=krbcontext)
 -os.environ['KRB5CCNAME'] = ccache_file
 -ccache = krbV.CCache(name=ccache_file, context=krbcontext, 
 primary_principal=princ)
 -ccache.init(princ)
 -ccache.init_creds_keytab(keytab=ktab, principal=princ)
 -return ccache_file
 -except krbV.Krb5Error, e:
 -raise StandardError('Error initializing principal %s in %s: %s' % 
 (principal, keytab, str(e)))
 +root_logger.debug(Initializing principal %s using keytab %s
 +  % (principal, keytab))
 +for attempt in xrange(attempts):
I would like to see new code compatible with Python 3. Here I'm not sure what
is the generic solution for xrange but in this particular case I would
recommend you to use just range. Attempts variable should have small values so
the x/range differences do not matter here.

 +try:
 +krbcontext = krbV.default_context()
 +ktab = krbV.Keytab(name=keytab, context=krbcontext)
 +princ = krbV.Principal(name=principal, context=krbcontext)
 +os.environ['KRB5CCNAME'] = ccache_path
This is a bit scary, especially when it comes to multi-threaded execution. If
it is really necessary please add comment that this method is not thread-safe.

 +ccache = krbV.CCache(name=ccache_path, context=krbcontext,
 + primary_principal=princ)
 +ccache.init(princ)
 +ccache.init_creds_keytab(keytab=ktab, principal=princ)
 +root_logger.debug(Attempt %d/%d: success
 +  % (attempt + 1, attempts))
What about adding + 1 to range boundaries instead of + 1 here and there?

 +return
 +except krbV.Krb5Error, e:
except ... , ... syntax is not going to work in Python 3. Maybe 'as' would be
better?

 +root_logger.debug(Attempt %d/%d: failed
 +  % (attempt + 1, attempts))
 +time.sleep(1)
 +
 +root_logger.debug(Maximum number of attempts (%d) reached
 +  % attempts)
 +raise StandardError(Error initializing principal %s: %s
 +% (principal, str(e)))
 +
 +
 +def kinit_password(principal, password, env={}, armor_ccache=):
 +

Re: [Freeipa-devel] Generic support for unknown DNS RR types (RFC 3597)

2015-03-11 Thread Petr Spacek
On 11.3.2015 15:28, Martin Kosek wrote:
 On 03/11/2015 12:43 PM, Petr Spacek wrote:
 On 11.3.2015 11:34, Jan Cholasta wrote:
 Dne 11.3.2015 v 11:12 Petr Spacek napsal(a):
 On 10.3.2015 20:04, Simo Sorce wrote:
 On Tue, 2015-03-10 at 19:24 +0100, Petr Spacek wrote:
 On 10.3.2015 18:36, Simo Sorce wrote:
 On Tue, 2015-03-10 at 18:26 +0100, Petr Spacek wrote:
 On 10.3.2015 17:35, Simo Sorce wrote:
 On Tue, 2015-03-10 at 16:19 +0100, Petr Spacek wrote:
 On 10.3.2015 15:53, Simo Sorce wrote:
 On Tue, 2015-03-10 at 15:32 +0100, Petr Spacek wrote:
 Hello,

 I would like to discuss Generic support for unknown DNS RR types
 (RFC 3597
 [0]). Here is the proposal:

 LDAP schema
 ===
 - 1 new attribute:
 ( OID NAME 'GenericRecord' DESC 'unknown DNS record, RFC 3597'
 EQUALITY
 caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )

 The attribute should be added to existing idnsRecord object class 
 as
 MAY.

 This new attribute should contain data encoded according to ​RFC
 3597 section
 5 [5]:

 The RDATA section of an RR of unknown type is represented as a
 sequence of white space separated words as follows:

The special token \# (a backslash immediately followed by a 
 hash
sign), which identifies the RDATA as having the generic 
 encoding
defined herein rather than a traditional type-specific 
 encoding.

An unsigned decimal integer specifying the RDATA length in
 octets.

Zero or more words of hexadecimal data encoding the actual 
 RDATA
field, each containing an even number of hexadecimal digits.

 If the RDATA is of zero length, the text representation 
 contains
 only
 the \# token and the single zero representing the length.

 Examples from RFC:
a.example.   CLASS32 TYPE731 \# 6 abcd (
 ef 01 23 45 )
b.example.   HS  TYPE62347   \# 0
e.example.   IN  A   \# 4 0A01
e.example.   CLASS1  TYPE1   10.0.0.2


 Open questions about LDAP format
 
 Should we include \# constant? We know that the attribute 
 contains
 record in
 RFC 3597 syntax so it is not strictly necessary.

 I think it would be better to follow RFC 3597 format. It allows 
 blind
 copypasting from other tools, including direct calls to 
 python-dns.

 It also eases writing conversion tools between DNS and LDAP format
 because
 they do not need to change record values.


 Another question is if we should explicitly include length of data
 represented
 in hexadecimal notation as a decimal number. I'm very strongly
 inclined to let
 it there because it is very good sanity check and again, it allows
 us to
 re-use existing tools including parsers.

 I will ask Uninett.no for standardization after we sort this out
 (they own the
 OID arc we use for DNS records).


 Attribute usage
 ===
 Every DNS RR type has assigned a number [1] which is used on wire.
 RR types
 which are unknown to the server cannot be named by their
 mnemonic/type name
 because server would not be able to do name-number conversion and
 to generate
 DNS wire format.

 As a result, we have to encode the RR type number somehow. Let's 
 use
 attribute
 sub-types.

 E.g. a record with type 65280 and hex value 0A01 will be
 represented as:
 GenericRecord;TYPE65280: \# 4 0A01


 CLI
 ===
 $ ipa dnsrecord-add zone.example owner \
--generic-type=65280 --generic-data='\# 4 0A01'

 $ ipa dnsrecord-show zone.example owner
 Record name: owner
 TYPE65280 Record: \# 4 0A01


 ACK? :-)

 Almost.
 We should refrain from using subtypes when not necessary, and in 
 this
 case it is not necessary.

 Use:
 GenericRecord: 65280 \# 4 0A01

 I was considering that too but I can see two main drawbacks:

 1) It does not work very well with DS ACI (targetattrfilter, 
 anyone?).
 Adding
 generic write access to GenericRecord == ability to add TLSA records 
 too,
 which you may not want. IMHO it is perfectly reasonable to limit 
 write
 access
 to certain types (e.g. to one from private range).

 2) We would need a separate substring index for emulating filters 
 like
 (type==65280). AFAIK GenericRecord;TYPE65280 should work with 
 presence
 index
 which will be handy one day when we decide to handle upgrades like
 GenericRecord;TYPE256-UriRecord.

 Another (less important) annoyance is that conversion tools would 
 have to
 mangle record data instead of just converting attribute name-record
 type.


 I can be convinced that subtypes are not necessary but I do not see 
 clear
 advantage of avoiding them. What is the problem with subtypes?

 Poor support by most clients, so it is generally discouraged.
 Hmm, it does not sound like a thing we should care in this case. DNS
 tree is
 not meant for direct consumption by LDAP clients (compare with 
 cn=compat).

 IMHO the only two clients we should care are FreeIPA framework and
 bind-dyndb

Re: [Freeipa-devel] [PATCHES 0015-0017] consolidation of various Kerberos auth methods in FreeIPA code

2015-03-11 Thread Petr Spacek
On 11.3.2015 14:27, Martin Babinsky wrote:
 Actually, now that I think about it, I will try to address some of your 
 comments:

 +except krbV.Krb5Error, e:
 except ... , ... syntax is not going to work in Python 3. Maybe 'as' would be
 better?

 AFAIK except ... as ... syntax was added in Python 2.6. Using this syntax can
 break older versions of Python. If this is not a concern for us, I will fix
 this and use this syntax also in my later patches.

Please see http://www.freeipa.org/page/Python_Coding_Style :-) Python 2.7 is
required anyway.

 diff --git a/ipa-client/ipa-install/ipa-client-install
 b/ipa-client/ipa-install/ipa-client-install
 index
 ccaab5536e83b4b6ac60b81132c3455c0af19ae1..c817f9e86dbaa6a2cca7d1a463f53d491fa7badb
 100755
 --- a/ipa-client/ipa-install/ipa-client-install
 +++ b/ipa-client/ipa-install/ipa-client-install
 @@ -91,6 +91,13 @@ def parse_options():

   parser.values.ca_cert_file = value

 +def validate_kinit_attempts_option(option, opt, value, parser):
 +if value  1 or value  sys.maxint:
 +raise OptionValueError(
 +%s option has invalid value %d % (opt, value))
 It would be nice if the error message said what is the expected value.
 (Expected integer in range 1,%s % sys.maxint)

 BTW is it possible to do this using existing option parser? I would expect
 some generic support for type=uint or something similar.

 OptionParser supports 'type' keywords when adding options, which perform the
 neccessary conversions (int(), etc) and validation (see
 https://docs.python.org/2/library/optparse.html#optparse-standard-option-types).
 However, in this case you still have to manually check for values less that 1
 which do not make sense. AFAIK OptionParser has no built-in way to do this.

Okay then.

 +
 +parser.values.kinit_attempts = value
 +
   parser = IPAOptionParser(version=version.VERSION)

   basic_group = OptionGroup(parser, basic options)
 @@ -144,6 +151,11 @@ def parse_options():
 help=do not modify the nsswitch.conf and PAM
 configuration)
   basic_group.add_option(-f, --force, dest=force,
 action=store_true,
 default=False, help=force setting of LDAP/Kerberos
 conf)
 +basic_group.add_option('--kinit-attempts', dest='kinit_attempts',
 +   action='callback', type='int', default=5,

 It would be good to check lockout numbers in default configuration to make
 sure that replication delay will not lock the principal.

 I'm not sure that I follow, could you be more specific what you mean by this?

KDC and DS will lock account after n failed attempts. See $ ipa pwpolicy-find
to find out the number in your installation (keytab == password).

 freeipa-mbabinsk-0017-2-Adopted-kinit_keytab-and-kinit_password-for-kerberos.patch



  From 912113529138e5b1bd8357ae6a17376cb5d32759 Mon Sep 17 00:00:00 2001
 From: Martin Babinsky mbabi...@redhat.com
 Date: Mon, 9 Mar 2015 12:54:36 +0100
 Subject: [PATCH 3/3] Adopted kinit_keytab and kinit_password for kerberos 
 auth

 ---
   daemons/dnssec/ipa-dnskeysync-replica  |  6 ++-
   daemons/dnssec/ipa-dnskeysyncd |  2 +-
   daemons/dnssec/ipa-ods-exporter|  5 ++-
   .../certmonger/dogtag-ipa-ca-renew-agent-submit|  3 +-
   install/restart_scripts/renew_ca_cert  |  7 ++--
   install/restart_scripts/renew_ra_cert  |  4 +-
   ipa-client/ipa-install/ipa-client-automount|  9 ++--
   ipa-client/ipaclient/ipa_certupdate.py |  3 +-
   ipaserver/rpcserver.py | 49
 +++---
   9 files changed, 47 insertions(+), 41 deletions(-)

 diff --git a/daemons/dnssec/ipa-dnskeysync-replica
 b/daemons/dnssec/ipa-dnskeysync-replica
 index
 d04f360e04ee018dcdd1ba9b2ca42b1844617af9..e9cae519202203a10678b7384e5acf748f256427
 100755
 --- a/daemons/dnssec/ipa-dnskeysync-replica
 +++ b/daemons/dnssec/ipa-dnskeysync-replica
 @@ -139,14 +139,16 @@ log.setLevel(level=logging.DEBUG)
   # Kerberos initialization
   PRINCIPAL = str('%s/%s' % (DAEMONNAME, ipalib.api.env.host))
   log.debug('Kerberos principal: %s', PRINCIPAL)
 -ipautil.kinit_hostprincipal(paths.IPA_DNSKEYSYNCD_KEYTAB, WORKDIR, 
 PRINCIPAL)
 +ccache_filename = os.path.join(WORKDIR, 'ccache')
 BTW I really appreciate this patch set! We finally can use more descriptive
 names like 'ipa-dnskeysync-replica.ccache' which sometimes make debugging
 easier.

 Named ccaches seems to be a good idea. I will fix this in all places where the
 ccache is somehow persistent (and not deleted after installation).

Thank you!

 diff --git a/ipa-client/ipa-install/ipa-client-automount
 b/ipa-client/ipa-install/ipa-client-automount
 index
 7b9e701dead5f50a033a455eb62e30df78cc0249..19197d34ca580062742b3d7363e5dfb2dad0e4de
 100755
 --- a/ipa-client/ipa-install/ipa-client-automount
 +++ b/ipa-client/ipa-install/ipa-client-automount
 @@ -425,10 +425,11 @@ 

[Freeipa-devel] Generic support for unknown DNS RR types (RFC 3597)

2015-03-10 Thread Petr Spacek
Hello,

I would like to discuss Generic support for unknown DNS RR types (RFC 3597
[0]). Here is the proposal:

LDAP schema
===
- 1 new attribute:
( OID NAME 'GenericRecord' DESC 'unknown DNS record, RFC 3597' EQUALITY
caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )

The attribute should be added to existing idnsRecord object class as MAY.

This new attribute should contain data encoded according to ​RFC 3597 section
5 [5]:

The RDATA section of an RR of unknown type is represented as a
   sequence of white space separated words as follows:

  The special token \# (a backslash immediately followed by a hash
  sign), which identifies the RDATA as having the generic encoding
  defined herein rather than a traditional type-specific encoding.

  An unsigned decimal integer specifying the RDATA length in octets.

  Zero or more words of hexadecimal data encoding the actual RDATA
  field, each containing an even number of hexadecimal digits.

   If the RDATA is of zero length, the text representation contains only
   the \# token and the single zero representing the length.

Examples from RFC:
  a.example.   CLASS32 TYPE731 \# 6 abcd (
   ef 01 23 45 )
  b.example.   HS  TYPE62347   \# 0
  e.example.   IN  A   \# 4 0A01
  e.example.   CLASS1  TYPE1   10.0.0.2


Open questions about LDAP format

Should we include \# constant? We know that the attribute contains record in
RFC 3597 syntax so it is not strictly necessary.

I think it would be better to follow RFC 3597 format. It allows blind
copypasting from other tools, including direct calls to python-dns.

It also eases writing conversion tools between DNS and LDAP format because
they do not need to change record values.


Another question is if we should explicitly include length of data represented
in hexadecimal notation as a decimal number. I'm very strongly inclined to let
it there because it is very good sanity check and again, it allows us to
re-use existing tools including parsers.

I will ask Uninett.no for standardization after we sort this out (they own the
OID arc we use for DNS records).


Attribute usage
===
Every DNS RR type has assigned a number [1] which is used on wire. RR types
which are unknown to the server cannot be named by their mnemonic/type name
because server would not be able to do name-number conversion and to generate
DNS wire format.

As a result, we have to encode the RR type number somehow. Let's use attribute
sub-types.

E.g. a record with type 65280 and hex value 0A01 will be represented as:
GenericRecord;TYPE65280: \# 4 0A01


CLI
===
$ ipa dnsrecord-add zone.example owner \
  --generic-type=65280 --generic-data='\# 4 0A01'

$ ipa dnsrecord-show zone.example owner
Record name: owner
TYPE65280 Record: \# 4 0A01


ACK? :-)


Related tickets
===
https://fedorahosted.org/freeipa/ticket/4939
https://fedorahosted.org/bind-dyndb-ldap/ticket/157

[0] http://tools.ietf.org/html/rfc3597
[1]
http://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-4
[5] http://tools.ietf.org/html/rfc3597#section-5

-- 
Petr^2 Spacek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Generic support for unknown DNS RR types (RFC 3597)

2015-03-10 Thread Petr Spacek
On 10.3.2015 15:53, Simo Sorce wrote:
 On Tue, 2015-03-10 at 15:32 +0100, Petr Spacek wrote:
 Hello,

 I would like to discuss Generic support for unknown DNS RR types (RFC 3597
 [0]). Here is the proposal:

 LDAP schema
 ===
 - 1 new attribute:
 ( OID NAME 'GenericRecord' DESC 'unknown DNS record, RFC 3597' EQUALITY
 caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )

 The attribute should be added to existing idnsRecord object class as MAY.

 This new attribute should contain data encoded according to ​RFC 3597 section
 5 [5]:

 The RDATA section of an RR of unknown type is represented as a
sequence of white space separated words as follows:

   The special token \# (a backslash immediately followed by a hash
   sign), which identifies the RDATA as having the generic encoding
   defined herein rather than a traditional type-specific encoding.

   An unsigned decimal integer specifying the RDATA length in octets.

   Zero or more words of hexadecimal data encoding the actual RDATA
   field, each containing an even number of hexadecimal digits.

If the RDATA is of zero length, the text representation contains only
the \# token and the single zero representing the length.

 Examples from RFC:
   a.example.   CLASS32 TYPE731 \# 6 abcd (
ef 01 23 45 )
   b.example.   HS  TYPE62347   \# 0
   e.example.   IN  A   \# 4 0A01
   e.example.   CLASS1  TYPE1   10.0.0.2


 Open questions about LDAP format
 
 Should we include \# constant? We know that the attribute contains record 
 in
 RFC 3597 syntax so it is not strictly necessary.

 I think it would be better to follow RFC 3597 format. It allows blind
 copypasting from other tools, including direct calls to python-dns.

 It also eases writing conversion tools between DNS and LDAP format because
 they do not need to change record values.


 Another question is if we should explicitly include length of data 
 represented
 in hexadecimal notation as a decimal number. I'm very strongly inclined to 
 let
 it there because it is very good sanity check and again, it allows us to
 re-use existing tools including parsers.

 I will ask Uninett.no for standardization after we sort this out (they own 
 the
 OID arc we use for DNS records).


 Attribute usage
 ===
 Every DNS RR type has assigned a number [1] which is used on wire. RR types
 which are unknown to the server cannot be named by their mnemonic/type name
 because server would not be able to do name-number conversion and to 
 generate
 DNS wire format.

 As a result, we have to encode the RR type number somehow. Let's use 
 attribute
 sub-types.

 E.g. a record with type 65280 and hex value 0A01 will be represented as:
 GenericRecord;TYPE65280: \# 4 0A01


 CLI
 ===
 $ ipa dnsrecord-add zone.example owner \
   --generic-type=65280 --generic-data='\# 4 0A01'

 $ ipa dnsrecord-show zone.example owner
 Record name: owner
 TYPE65280 Record: \# 4 0A01


 ACK? :-)
 
 Almost.
 We should refrain from using subtypes when not necessary, and in this
 case it is not necessary.
 
 Use:
 GenericRecord: 65280 \# 4 0A01

I was considering that too but I can see two main drawbacks:

1) It does not work very well with DS ACI (targetattrfilter, anyone?). Adding
generic write access to GenericRecord == ability to add TLSA records too,
which you may not want. IMHO it is perfectly reasonable to limit write access
to certain types (e.g. to one from private range).

2) We would need a separate substring index for emulating filters like
(type==65280). AFAIK GenericRecord;TYPE65280 should work with presence index
which will be handy one day when we decide to handle upgrades like
GenericRecord;TYPE256-UriRecord.

Another (less important) annoyance is that conversion tools would have to
mangle record data instead of just converting attribute name-record type.


I can be convinced that subtypes are not necessary but I do not see clear
advantage of avoiding them. What is the problem with subtypes?

Petr^2 Spacek

 
 Done!
 
 Simo.
 

 Related tickets
 ===
 https://fedorahosted.org/freeipa/ticket/4939
 https://fedorahosted.org/bind-dyndb-ldap/ticket/157

 [0] http://tools.ietf.org/html/rfc3597
 [1]
 http://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-4
 [5] http://tools.ietf.org/html/rfc3597#section-5

 -- 
 Petr^2 Spacek


-- 
Petr^2 Spacek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Purpose of default user group

2015-03-10 Thread Petr Spacek
On 10.3.2015 16:55, Alexander Bokovoy wrote:
 On Tue, 10 Mar 2015, Petr Spacek wrote:
 On 10.3.2015 16:01, Jakub Hrozek wrote:
 On Tue, Mar 10, 2015 at 03:52:44PM +0100, Martin Kosek wrote:
 On 03/10/2015 03:27 PM, Rob Crittenden wrote:
 Petr Vobornik wrote:
 Hi,

 I would like to ask what is a purpose of a default user group - by
 default ipausers? Default group is also a required field in ipa config.

 To be able to apply some (undefined) group policy to all users. I'm not
 aware that it has ever been used for this.

 I would also interested in the use cases, especially given all the pain we
 have
 with ipausers and large user bases. Especially that for current policies
 (SUDO,
 HBAC, SELinux user policy), we always have other means to specify all
 users.

 yes, but those means usually specify both AD and IPA users, right?

 I always thought ipausers is a handy shortcut for selecting IPA users
 only and not AD users.

 I always thought that ipausers is an equivalent of domain users in AD
 world (compare with Trusted domain users).

 In my admin life I considered domain users to be useful alias for real
 authenticated user accounts (compare with Everyone = even unauthenticated
 access, Authenticated users = includes machine accounts too.)


 Moreover, getting rid of ipausers does not help with 'big groups problem' in
 any way. E.g. at university you are almost inevitably going to have groups
 like 'students' which will contain more than 90 % of users anyway.
 For what use we need this distinction in IPA itself?
 - ACI (permissions) have separate notion to describe
  anonymous/any authenticated dichotomy
 - HBAC has 'all' category for users which in HBAC context means all
  authenticated users
 
 Where else we would need ipausers other than default POSIX group which
 we are not using it for?

Ah, it is not a POSIX group? Too bad. I was using AD domain users for file
permissions so POSIX group equivalent is what I had in mind.

-- 
Petr^2 Spacek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Generic support for unknown DNS RR types (RFC 3597)

2015-03-10 Thread Petr Spacek
On 10.3.2015 18:36, Simo Sorce wrote:
 On Tue, 2015-03-10 at 18:26 +0100, Petr Spacek wrote:
 On 10.3.2015 17:35, Simo Sorce wrote:
 On Tue, 2015-03-10 at 16:19 +0100, Petr Spacek wrote:
 On 10.3.2015 15:53, Simo Sorce wrote:
 On Tue, 2015-03-10 at 15:32 +0100, Petr Spacek wrote:
 Hello,

 I would like to discuss Generic support for unknown DNS RR types (RFC 
 3597
 [0]). Here is the proposal:

 LDAP schema
 ===
 - 1 new attribute:
 ( OID NAME 'GenericRecord' DESC 'unknown DNS record, RFC 3597' EQUALITY
 caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )

 The attribute should be added to existing idnsRecord object class as MAY.

 This new attribute should contain data encoded according to ​RFC 3597 
 section
 5 [5]:

 The RDATA section of an RR of unknown type is represented as a
sequence of white space separated words as follows:

   The special token \# (a backslash immediately followed by a hash
   sign), which identifies the RDATA as having the generic encoding
   defined herein rather than a traditional type-specific encoding.

   An unsigned decimal integer specifying the RDATA length in octets.

   Zero or more words of hexadecimal data encoding the actual RDATA
   field, each containing an even number of hexadecimal digits.

If the RDATA is of zero length, the text representation contains only
the \# token and the single zero representing the length.

 Examples from RFC:
   a.example.   CLASS32 TYPE731 \# 6 abcd (
ef 01 23 45 )
   b.example.   HS  TYPE62347   \# 0
   e.example.   IN  A   \# 4 0A01
   e.example.   CLASS1  TYPE1   10.0.0.2


 Open questions about LDAP format
 
 Should we include \# constant? We know that the attribute contains 
 record in
 RFC 3597 syntax so it is not strictly necessary.

 I think it would be better to follow RFC 3597 format. It allows blind
 copypasting from other tools, including direct calls to python-dns.

 It also eases writing conversion tools between DNS and LDAP format 
 because
 they do not need to change record values.


 Another question is if we should explicitly include length of data 
 represented
 in hexadecimal notation as a decimal number. I'm very strongly inclined 
 to let
 it there because it is very good sanity check and again, it allows us to
 re-use existing tools including parsers.

 I will ask Uninett.no for standardization after we sort this out (they 
 own the
 OID arc we use for DNS records).


 Attribute usage
 ===
 Every DNS RR type has assigned a number [1] which is used on wire. RR 
 types
 which are unknown to the server cannot be named by their mnemonic/type 
 name
 because server would not be able to do name-number conversion and to 
 generate
 DNS wire format.

 As a result, we have to encode the RR type number somehow. Let's use 
 attribute
 sub-types.

 E.g. a record with type 65280 and hex value 0A01 will be represented 
 as:
 GenericRecord;TYPE65280: \# 4 0A01


 CLI
 ===
 $ ipa dnsrecord-add zone.example owner \
   --generic-type=65280 --generic-data='\# 4 0A01'

 $ ipa dnsrecord-show zone.example owner
 Record name: owner
 TYPE65280 Record: \# 4 0A01


 ACK? :-)

 Almost.
 We should refrain from using subtypes when not necessary, and in this
 case it is not necessary.

 Use:
 GenericRecord: 65280 \# 4 0A01

 I was considering that too but I can see two main drawbacks:

 1) It does not work very well with DS ACI (targetattrfilter, anyone?). 
 Adding
 generic write access to GenericRecord == ability to add TLSA records too,
 which you may not want. IMHO it is perfectly reasonable to limit write 
 access
 to certain types (e.g. to one from private range).

 2) We would need a separate substring index for emulating filters like
 (type==65280). AFAIK GenericRecord;TYPE65280 should work with presence 
 index
 which will be handy one day when we decide to handle upgrades like
 GenericRecord;TYPE256-UriRecord.

 Another (less important) annoyance is that conversion tools would have to
 mangle record data instead of just converting attribute name-record type.


 I can be convinced that subtypes are not necessary but I do not see clear
 advantage of avoiding them. What is the problem with subtypes?

 Poor support by most clients, so it is generally discouraged.
 Hmm, it does not sound like a thing we should care in this case. DNS tree is
 not meant for direct consumption by LDAP clients (compare with cn=compat).

 IMHO the only two clients we should care are FreeIPA framework and
 bind-dyndb-ldap so I do not see this as a problem, really. If someone wants 
 to
 access DNS tree by hand - sure, use a standard compliant client!

 Working ACI and LDAP filters sounds like good price for supporting only
 standards compliant clients.

 AFAIK OpenLDAP works well and I suspect that ApacheDS will work too

Re: [Freeipa-devel] [PATCHES 0200-0202] DNS fixes related to unsupported records

2015-03-06 Thread Petr Spacek
On 4.3.2015 16:35, Martin Basti wrote:
 On 04/03/15 16:17, Martin Basti wrote:
 Ticket: https://fedorahosted.org/freeipa/ticket/4930

 0200:  4.1, master
 Fixes traceback, which was raised if LDAP contained a record that was marked
 as unsupported.
 Now unsupported records are shown, if LDAP contains them.

 0200: 4.1, master
 Records marked as unsupported will not show options for editing parts.

 0202: only master
 Removes NSEC3PARAM record from record types. NSEC3PARAM can contain only
 zone, value is allowed only in idnszone objectclass, so do not confuse users.

 
  and patches attached :-)

ACK. It works for me and can be pushed to branches 4.1 and master.

-- 
Petr^2 Spacek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] New freeipa-devel footer

2015-03-06 Thread Petr Spacek
On 6.3.2015 12:01, Martin Kosek wrote:
 On 03/06/2015 11:55 AM, Jan Pazdziora wrote:
 On Fri, Mar 06, 2015 at 11:43:07AM +0100, Martin Kosek wrote:

 See the footer below. If you have any improvements proposals, just tell me.

 Given the information about the list actions is in the List-* header
 of ever email, do you need the

 Manage your subscription for the Freeipa-devel mailing list:
 https://www.redhat.com/mailman/listinfo/freeipa-devel

 bits?

 I like

 Go to http://www.freeipa.org/page/Contribute/Code for more info on how to
 contribute to the project

 but having it wrapped to  80 characters would be even nicer. Or even
 just

 Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

 might be enough.

 
 Good idea. I fixed that part.
 
 As for link to mailman, I see it as an option for people now knowing that
 something as mail headers exists. But given this is freeipa-*devel* list, we
 may indeed remove it. I will see if there are any more opinions on that 
 matter.

Personally I would nuke the footer from devel list completely :-) You are
already on devel list so what is the point of inviting you to it? :-)

-- 
Petr^2 Spacek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0190] DNSSEC: add support for CKM_RSA_PKCS_OAEP mechanism

2015-03-05 Thread Petr Spacek
On 26.2.2015 16:59, Martin Basti wrote:
 On 26/02/15 12:47, Petr Spacek wrote:
 On 11.2.2015 14:10, Martin Basti wrote:
 https://fedorahosted.org/freeipa/ticket/4657#comment:13

 Patch attached.

 -- 
 Martin Basti


 freeipa-mbasti-0190-DNSSEC-add-support-for-CKM_RSA_PKCS_OAEP-mechanism.patch


  From 4d698a5adaa94eb854c75bd9bcaf3093f31a11e5 Mon Sep 17 00:00:00 2001
 From: Martin Basti mba...@redhat.com
 Date: Wed, 11 Feb 2015 14:05:46 +0100
 Subject: [PATCH] DNSSEC add support for CKM_RSA_PKCS_OAEP mechanism

 Ticket: https://fedorahosted.org/freeipa/ticket/4657#comment:13
 ---
   ipapython/ipap11helper/p11helper.c | 72
 --
   1 file changed, 69 insertions(+), 3 deletions(-)

 diff --git a/ipapython/ipap11helper/p11helper.c
 b/ipapython/ipap11helper/p11helper.c
 index
 4e0f262057b377124793f1e3091a8c9df4794164..c638bbe849f1bbddc8004bd1c41128b1c9e7
 100644
 --- a/ipapython/ipap11helper/p11helper.c
 +++ b/ipapython/ipap11helper/p11helper.c
 @@ -53,6 +53,22 @@
   // TODO
   #define CKA_COPYABLE   (0x0017)
   +#define CKG_MGF1_SHA1 (0x0001)
 +
 +#define CKZ_DATA_SPECIFIED(0x0001)
 +
 +struct ck_rsa_pkcs_oaep_params {
 +  CK_MECHANISM_TYPE hash_alg;
 +  unsigned long mgf;
 +  unsigned long source;
 +  void *source_data;
 +  unsigned long source_data_len;
 +};
 +
 +typedef struct ck_rsa_pkcs_oaep_params CK_RSA_PKCS_OAEP_PARAMS;
 +typedef struct ck_rsa_pkcs_oaep_params *CK_RSA_PKCS_OAEP_PARAMS_PTR;
 +
 +
   CK_BBOOL true = CK_TRUE;
   CK_BBOOL false = CK_FALSE;
   @@ -118,6 +134,17 @@ CK_BBOOL* bool;
   } PyObj2Bool_mapping_t;
 /**
 + * Constants
 + */
 +static const CK_RSA_PKCS_OAEP_PARAMS CONST_RSA_PKCS_OAEP_PARAMS = {
 +.hash_alg = CKM_SHA_1,
 +.mgf = CKG_MGF1_SHA1,
 +.source = CKZ_DATA_SPECIFIED,
 +.source_data = NULL,
 +.source_data_len = 0
 +};
 +
 +/**
* ipap11helper Exceptions
*/
   static PyObject *ipap11helperException; //parent class for all exceptions
 @@ -1359,17 +1386,36 @@ P11_Helper_export_wrapped_key(P11_Helper* self,
 PyObject *args, PyObject *kwds)
   CK_BYTE_PTR wrapped_key = NULL;
   CK_ULONG wrapped_key_len = 0;
   CK_MECHANISM wrapping_mech = { CKM_RSA_PKCS, NULL, 0 };
 -CK_MECHANISM_TYPE wrapping_mech_type = CKM_RSA_PKCS;
   /* currently we don't support parameter in mechanism */
 static char *kwlist[] = { key, wrapping_key, wrapping_mech,
 NULL };
   //TODO check long overflow
   //TODO export method
   if (!PyArg_ParseTupleAndKeywords(args, kwds, kkk|, kwlist,
 object_key,
 -object_wrapping_key, wrapping_mech_type)) {
 +object_wrapping_key, wrapping_mech.mechanism)) {
   return NULL;
   }
 -wrapping_mech.mechanism = wrapping_mech_type;
 +
 +// fill mech parameters
 +switch(wrapping_mech.mechanism){
 +case CKM_RSA_PKCS:
 +case CKM_AES_KEY_WRAP:
 +case CKM_AES_KEY_WRAP_PAD:
 +//default params
 +break;
 +
 +case CKM_RSA_PKCS_OAEP:
 +/* Use the same configuration as openSSL
 + * https://www.openssl.org/docs/crypto/RSA_public_encrypt.html
 + */
 + wrapping_mech.pParameter = (void*) 
 CONST_RSA_PKCS_OAEP_PARAMS;
 + wrapping_mech.ulParameterLen =
 sizeof(CONST_RSA_PKCS_OAEP_PARAMS);
 +break;
 +
 +default:
 +PyErr_SetString(ipap11helperError, Unsupported wrapping
 mechanism);
 +return NULL;
 +}
 rv = self-p11-C_WrapKey(self-session, wrapping_mech,
   object_wrapping_key, object_key, NULL, wrapped_key_len);
 @@ -1452,6 +1498,26 @@ P11_Helper_import_wrapped_secret_key(P11_Helper*
 self, PyObject *args,
   return NULL;
   }
   +switch(wrapping_mech.mechanism){
 +case CKM_RSA_PKCS:
 +case CKM_AES_KEY_WRAP:
 +case CKM_AES_KEY_WRAP_PAD:
 +//default params
 +break;
 NACK. This switch is duplicate of the previous one. Please split it into an
 auxiliary function and call it twice.

 Thank you!

 Thanks. Updated patch attached.

ACK, it works for me.

-- 
Petr^2 Spacek

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


[Freeipa-devel] [PATCH 0023-0025] p11helper improvements

2015-03-05 Thread Petr Spacek
Hello,

please review this patch set. It should be applied on top of your previous
p11helper patch set.

Thank you!

-- 
Petr^2 Spacek
From 0195c8cac890a6a41d5ba8e48e904b6d69405bfb Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 4 Mar 2015 14:37:58 +0100
Subject: [PATCH] p11helper: standardize indentation and other visual aspects
 of the code

https://fedorahosted.org/freeipa/ticket/4657
---
 ipapython/ipap11helper/p11helper.c | 1327 
 1 file changed, 741 insertions(+), 586 deletions(-)

diff --git a/ipapython/ipap11helper/p11helper.c b/ipapython/ipap11helper/p11helper.c
index fa858c9d5954df21c28719fd4e2eadc0e19c72a7..b3cf6cd780a40d1afb3c65a22ef46fa948e7676c 100644
--- a/ipapython/ipap11helper/p11helper.c
+++ b/ipapython/ipap11helper/p11helper.c
@@ -61,11 +61,11 @@
 #define CKZ_DATA_SPECIFIED(0x0001)
 
 struct ck_rsa_pkcs_oaep_params {
-  CK_MECHANISM_TYPE hash_alg;
-  unsigned long mgf;
-  unsigned long source;
-  void *source_data;
-  unsigned long source_data_len;
+CK_MECHANISM_TYPE hash_alg;
+unsigned long mgf;
+unsigned long source;
+void *source_data;
+unsigned long source_data_len;
 };
 
 typedef struct ck_rsa_pkcs_oaep_params CK_RSA_PKCS_OAEP_PARAMS;
@@ -81,11 +81,10 @@ CK_BBOOL false = CK_FALSE;
  * P11_Helper type
  */
 typedef struct {
-PyObject_HEAD
-CK_SLOT_ID slot;
-CK_FUNCTION_LIST_PTR p11;
-CK_SESSION_HANDLE session;
-void *module_handle;
+PyObject_HEAD CK_SLOT_ID slot;
+CK_FUNCTION_LIST_PTR p11;
+CK_SESSION_HANDLE session;
+void *module_handle;
 } P11_Helper;
 
 typedef enum {
@@ -132,8 +131,8 @@ typedef enum {
 } private_key_enum;
 
 typedef struct {
-PyObject* py_obj;
-CK_BBOOL* bool;
+PyObject *py_obj;
+CK_BBOOL *bool;
 } PyObj2Bool_mapping_t;
 
 /**
@@ -150,11 +149,11 @@ static const CK_RSA_PKCS_OAEP_PARAMS CONST_RSA_PKCS_OAEP_PARAMS = {
 /**
  * ipap11helper Exceptions
  */
-static PyObject *ipap11helperException; //parent class for all exceptions
+static PyObject *ipap11helperException; // parent class for all exceptions
 
-static PyObject *ipap11helperError;  //general error
-static PyObject *ipap11helperNotFound;  //key not found
-static PyObject *ipap11helperDuplicationError; //key already exists
+static PyObject *ipap11helperError; // general error
+static PyObject *ipap11helperNotFound;  // key not found
+static PyObject *ipap11helperDuplicationError;  // key already exists
 
 /***
  * Support functions
@@ -166,17 +165,17 @@ static PyObject *ipap11helperDuplicationError; //key already exists
 goto final;   \
 } while(0);
 
-CK_BBOOL* pyobj_to_bool(PyObject* pyobj) {
+CK_BBOOL *pyobj_to_bool(PyObject *pyobj) {
 if (PyObject_IsTrue(pyobj))
 return true;
 return false;
 
 }
 
-void convert_py2bool(PyObj2Bool_mapping_t* mapping, int length) {
+void convert_py2bool(PyObj2Bool_mapping_t *mapping, int length) {
 int i;
 for (i = 0; i  length; ++i) {
-PyObject* py_obj = mapping[i].py_obj;
+PyObject *py_obj = mapping[i].py_obj;
 if (py_obj != NULL) {
 mapping[i].bool = pyobj_to_bool(py_obj);
 }
@@ -189,25 +188,25 @@ void convert_py2bool(PyObj2Bool_mapping_t* mapping, int length) {
  * :param l length: of returned string
  * Returns NULL if an error occurs, else pointer to string
  */
-unsigned char* unicode_to_char_array(PyObject *unicode, Py_ssize_t *l) {
-unsigned char* result = NULL;
-PyObject* utf8_str = PyUnicode_AsUTF8String(unicode);
+unsigned char *unicode_to_char_array(PyObject *unicode, Py_ssize_t *l) {
+unsigned char *result = NULL;
+PyObject *utf8_str = PyUnicode_AsUTF8String(unicode);
 if (utf8_str == NULL) {
 PyErr_SetString(ipap11helperError, Unable to encode UTF-8);
 return NULL;
 }
-unsigned char* bytes = (unsigned char*) PyString_AS_STRING(utf8_str);
+unsigned char *bytes = (unsigned char *) PyString_AS_STRING(utf8_str);
 if (bytes == NULL) {
 PyErr_SetString(ipap11helperError, Unable to get bytes from string);
 *l = 0;
 } else {
 *l = PyString_Size(utf8_str);
 
 /* Copy string first, then DECREF
  * https://docs.python.org/2/c-api/string.html#c.PyString_AS_STRING
  */
-result = (unsigned char *) PyMem_Malloc((size_t) *l);
-if (result == NULL){
+result = (unsigned char *) PyMem_Malloc((size_t) * l);
+if (result == NULL) {
 Py_DECREF(utf8_str);
 PyErr_NoMemory();
 return NULL;
@@ -223,22 +222,23 @@ unsigned char* unicode_to_char_array(PyObject *unicode, Py_ssize_t *l) {
 /**
  * Convert utf-8 encoded char array to unicode object
  */
-PyObject* char_array_to_unicode(const char* array, unsigned long l) {
+PyObject *char_array_to_unicode(const char *array, unsigned long l) {
 return PyUnicode_DecodeUTF8(array, l, strict

Re: [Freeipa-devel] [PATCH 0194] Remove unused method to export secret key from ipapkcs11helper module

2015-03-05 Thread Petr Spacek
On 25.2.2015 14:24, Martin Basti wrote:
 The method never been used, and never will be, because we do not want to
 export secrets.
 
 Ticket: https://fedorahosted.org/freeipa/ticket/4657
 
 Patch attached (may require mbasti-0195, mbasti-0190)

ACK, it works for me.

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0316] Fix crash triggered by zone objects with unexpected DN

2015-03-05 Thread Petr Spacek
On 4.3.2015 15:26, Tomas Hozza wrote:
 On 02/24/2015 03:01 PM, Petr Spacek wrote:
  Hello,
 
  On 18.2.2015 10:36, Tomas Hozza wrote:
   On 12/16/2014 04:32 PM, Petr Spacek wrote:
   Hello,
  
   Fix crash triggered by zone objects with unexpected DN.
  
   https://fedorahosted.org/bind-dyndb-ldap/ticket/148
  
   NACK.
  
   The patch seems to make no difference when using the reproducer from 
   ticket 148
  
   18-Feb-2015 10:34:09.067 running
   18-Feb-2015 10:34:09.139 ldap_helper.c:4876: INSIST(task == inst-task) 
   failed, back trace
   18-Feb-2015 10:34:09.139 #0 0x55587a80 in ??
   18-Feb-2015 10:34:09.139 #1 0x7620781a in ??
   18-Feb-2015 10:34:09.139 #2 0x720b00b2 in ??
   18-Feb-2015 10:34:09.140 #3 0x71e7ccf9 in ??
   18-Feb-2015 10:34:09.140 #4 0x71e7d992 in ??
   18-Feb-2015 10:34:09.140 #5 0x720a7f3b in ??
   18-Feb-2015 10:34:09.140 #6 0x75dda52a in ??
   18-Feb-2015 10:34:09.140 #7 0x7508d79d in ??
   18-Feb-2015 10:34:09.140 exiting (due to assertion failure)
  
   Program received signal SIGABRT, Aborted.
   [Switching to Thread 0x7fffea7cd700 (LWP 1719)]
   0x74fc18c7 in __GI_raise (sig=sig@entry=6) at 
   ../sysdeps/unix/sysv/linux/raise.c:55
   55  return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
   Missing separate debuginfos, use: debuginfo-install 
   cyrus-sasl-gssapi-2.1.26-19.fc21.x86_64 
   cyrus-sasl-lib-2.1.26-19.fc21.x86_64 
   cyrus-sasl-md5-2.1.26-19.fc21.x86_64 
   cyrus-sasl-plain-2.1.26-19.fc21.x86_64 gssproxy-0.3.1-4.fc21.x86_64 
   keyutils-libs-1.5.9-4.fc21.x86_64 libattr-2.4.47-9.fc21.x86_64 
   libdb-5.3.28-9.fc21.x86_64 libgcc-4.9.2-6.fc21.x86_64 
   libselinux-2.3-5.fc21.x86_64 nspr-4.10.8-1.fc21.x86_64 
   nss-3.17.4-1.fc21.x86_64 nss-softokn-freebl-3.17.4-1.fc21.x86_64 
   nss-util-3.17.4-1.fc21.x86_64 pcre-8.35-8.fc21.x86_64 
   sssd-client-1.12.3-4.fc21.x86_64 xz-libs-5.1.2-14alpha.fc21.x86_64
   (gdb) bt
   #0  0x74fc18c7 in __GI_raise (sig=sig@entry=6) at 
   ../sysdeps/unix/sysv/linux/raise.c:55
   #1  0x74fc352a in __GI_abort () at abort.c:89
   #2  0x55587c29 in assertion_failed (file=optimized out, 
   line=optimized out, type=optimized out, cond=optimized out) at 
   ./main.c:220
   #3  0x7620781a in isc_assertion_failed 
   (file=file@entry=0x720bad2a ldap_helper.c, line=line@entry=4876, 
   type=type@entry=isc_assertiontype_insist,
   cond=cond@entry=0x720baf04 task == inst-task) at 
   assertions.c:57
   #4  0x720b00b2 in syncrepl_update (chgtype=1, 
   entry=0x70125590, inst=0x77fa3160) at ldap_helper.c:4876
   #5  ldap_sync_search_entry (ls=optimized out, msg=optimized out, 
   entryUUID=optimized out, phase=LDAP_SYNC_CAPI_ADD) at 
   ldap_helper.c:5031
   #6  0x71e7ccf9 in ldap_sync_search_entry 
   (ls=ls@entry=0x7fffe40008c0, res=0x7fffe4003870) at ldap_sync.c:228
   #7  0x71e7d992 in ldap_sync_init (ls=0x7fffe40008c0, 
   mode=mode@entry=3) at ldap_sync.c:792
   #8  0x720a7f3b in ldap_syncrepl_watcher (arg=0x77fa3160) at 
   ldap_helper.c:5247
   #9  0x75dda52a in start_thread (arg=0x7fffea7cd700) at 
   pthread_create.c:310
   #10 0x7508d79d in clone () at 
   ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
 
  Thank you for catching this! I was using slightly different test which
  triggered the new code but by using different code path.
 
  This new version should be more robust. Please re-test it, thank you!
 
 ACK for version 2.

Thank, pushed to master:
9d2160ead48d64b6943cb4f0e7ec0feddd82dbc5

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0023-0025] p11helper improvements

2015-03-05 Thread Petr Spacek
On 5.3.2015 14:50, Petr Spacek wrote:
 Hello,
 
 please review this patch set. It should be applied on top of your previous
 p11helper patch set.
 
 Thank you!

Reviewer requested reworded version of the error message, here it is.

-- 
Petr^2 Spacek
From dd05ce3026b30874355ca3a441d7ccc51c65f287 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 4 Mar 2015 20:35:17 +0100
Subject: [PATCH] p11helper: clarify error message

https://fedorahosted.org/freeipa/ticket/4657
---
 ipapython/ipap11helper/p11helper.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ipapython/ipap11helper/p11helper.c b/ipapython/ipap11helper/p11helper.c
index 3874c4dc1a2fcf0f58d6b75d6379fa36c4075f86..eff49f5908412eb7b6e4c163710843e64e3bb69b 100644
--- a/ipapython/ipap11helper/p11helper.c
+++ b/ipapython/ipap11helper/p11helper.c
@@ -237,7 +237,9 @@ int check_return_value(CK_RV rv, const char *message) {
 (errmsg, Error at %s: 0x%x\n, message, (unsigned int) rv)
 == -1) {
 PyErr_SetString(ipap11helperError,
-DOUBLE ERROR: Creating the error message caused an error);
+An error occured during error message generation. 
+Please report this problem. Developers will use 
+a crystal ball to find out the root cause.);
 return 0;
 }
 if (errmsg != NULL) {
-- 
2.1.0

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

Re: [Freeipa-devel] [PATCH 0195] Fix memory leaks in ipapkcs11helper module

2015-03-05 Thread Petr Spacek
On 26.2.2015 17:01, Martin Basti wrote:
 On 26/02/15 13:06, Petr Spacek wrote:
 Hello Martin,

 thank you for patch! This NACK is only aesthetic :-)

 On 25.2.2015 14:21, Martin Basti wrote:
   if (!check_return_value(rv, import_wrapped_key: key unwrapping)) {
 +error = 1;
 +goto final;
 +}

 This exact sequence is repeated many times in the code.

 I would prefer a C macro like this:
 #define GOTO_FAIL\
  do {\
  error = 1;\
  goto final;\
  } while(0)

 This allows more dense code like:
 if (!test)
 GOTO_FAIL;

 and does not have the risk of missing error = 1 somewhere.

 +final:
   if (pkey != NULL)
   EVP_PKEY_free(pkey);
 +if (label != NULL) PyMem_Free(label);
 +if (error){
 +return NULL;
 +}
   return ret;
   }
 Apparently, this is inconsistent with itself.

 Please pick one style and use it, e.g.
 if (label != NULL)
 PyMem_Free(label)

 ... and do not add curly braces when unnecessary.

 If you want, we can try running $ indent on current sources and committing
 changes separately so you do not have to make changes like this by hand.

 Thanks. Updated patch attached.

ACK, it works for me.

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] IPA Server upgrade 4.2 design

2015-03-03 Thread Petr Spacek
On 3.3.2015 10:58, Martin Kosek wrote:
 On 03/03/2015 09:36 AM, Petr Spacek wrote:
 On 3.3.2015 09:33, Jan Cholasta wrote:
 Dne 3.3.2015 v 09:06 Martin Basti napsal(a):
 On 03/03/15 07:31, Jan Cholasta wrote:
 Dne 2.3.2015 v 13:51 Martin Basti napsal(a):
 On 02/03/15 13:12, Jan Cholasta wrote:
 Dne 2.3.2015 v 12:23 Martin Kosek napsal(a):
 On 03/02/2015 07:49 AM, Jan Cholasta wrote:
 Hi,

 Dne 24.2.2015 v 19:10 Martin Basti napsal(a):
 Hello all,

 please read the design page, any objections/suggestions appreciated
 http://www.freeipa.org/page/V4/Server_Upgrade_Refactoring


 1)

 
 * Merge server update commands into the one command
 (ipa-server-upgrade)
 

 So there is ipa-server-install to install the server,
 ipa-server-install
 --uninstall to uninstall the server and ipa-server-upgrade to
 upgrade the
 server. Maybe we should bring some consistency here and have one of:

   a) ipa-server-install [--install], ipa-server-install
 --uninstall,
 ipa-server-install --upgrade

   b) ipa-server-install [install], ipa-server-install uninstall,
 ipa-server-install upgrade

   c) ipa-server-install, ipa-server-uninstall,
 ipa-server-upgrade

 Long term, I think we want C. Besides other advantages, it will let
 us have
 independent sets of options, based on what you want to do.

 2)

 
 * Prevent to run IPA service, if code version and configuration
 version does
 not match
* ipactl should execute ipa-server-upgrade if needed
 

 There should be no configuration version, configuration update
 should be run
 always. It's fast and hence does not need to be optimized like data
 update by
 using a monolithic version number, which brings more than a few
 problems on its
 own.

 I do not agree in this section. Why would you like to run it always,
 even if it
 was fast? No run is still faster than fast run.

 In the ideal case the installer would be idempotent and upgrade would
 be re-running the installer and we should aim to do just that. We kind
 of do that already, but there is a lot of code duplication in
 installers and ipa-upgradeconfig (I would like to fix that when
 refactoring installers). IMO it's better to always make 100% sure the
 configuration is correct rather than to save a second or two.
 I doesn't like this idea, if user wants to fix something, the one should
 use --skip-version-check option, and the IPA upgrade will be executed.

 Well, what I don't like is dealing with meaningless version numbers.
 They are causing us grief in API versioning and I don't see why it
 would be any different here.
 However you must keep the version because of schema and data upgrade, so
 why not to execute update as one batch instead of doing config upgrade
 all the time, and then data upgrade only if required.

 Because there is no exact mapping between version number and what features 
 are
 actually available. A state file is tons better than a single version 
 number.


 Some configuration upgrades, like adding new DNS related services,
 requires new schema, how we can handle this?

 This does not sound right. Could you be more specific?

 Running schema upgrade every time?

 What if a service changes in a way, the IPA configuration will not work?

 Then it's a bug and needs to be fixed, like any other bug. IIRC there
 was only one or two occurences of such bug in the past 3 years (I
 remember sshd_config), so I don't think you have a strong case here.
 Ok

 The user will need to change it manually, but after each restart,
 upgrade will change the value back into IPA required configuration which
 will not work.

 Says who? It's our code, we can do whatever we want, it doesn't have
 to be dumb like this.

 Yes, we have upgrade state file, but then the comparing of one value is
 faster then checking each state if was executed.

 How faster is that, like, few milliseconds? Are you seriously
 considering this the right optimization in a process that is
 magnitudes slower?
 Ok the speed is not so important, but I still do not like the idea of
 executing the code which is not needed to be executed, because I know
 the version is the same as was before last restart, so nothing changed.

 Weren't clever optimizations like this what got us into this whole
 refactoring bussiness in the first place?

 I very much agree with Honza. We should always start with something
 stupidly-simply and enhance it later, when it is clear if it is really 
 necessary.

 Do not over-engineer it from the very beginning.
 
 I completely agree with starting stupid and simply and improving in time.
 However, are we sure that what Honza proposed is the simple and stupid way?
 
 Doing config upgrade only when needed and thus not depending on the efficiency
 and idempotency of the config upgraders seems to me as *the* stupid and simple
 way for upgrade refactoring.

Maybe I'm missing something but

if not state.get('dns_forward_zones_supported'):
# upgrade to forward zones here

seems less error-prone than

if version  4.0

Re: [Freeipa-devel] IPA Server upgrade 4.2 design

2015-03-03 Thread Petr Spacek
On 3.3.2015 11:01, Jan Cholasta wrote:
 I would very much prefer to do it the other way around, so that most bugs in
 the code are caught early, instead of hidden behind the version comparison.

+1

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] IPA Server upgrade 4.2 design

2015-03-03 Thread Petr Spacek
On 3.3.2015 09:33, Jan Cholasta wrote:
 Dne 3.3.2015 v 09:06 Martin Basti napsal(a):
 On 03/03/15 07:31, Jan Cholasta wrote:
 Dne 2.3.2015 v 13:51 Martin Basti napsal(a):
 On 02/03/15 13:12, Jan Cholasta wrote:
 Dne 2.3.2015 v 12:23 Martin Kosek napsal(a):
 On 03/02/2015 07:49 AM, Jan Cholasta wrote:
 Hi,

 Dne 24.2.2015 v 19:10 Martin Basti napsal(a):
 Hello all,

 please read the design page, any objections/suggestions appreciated
 http://www.freeipa.org/page/V4/Server_Upgrade_Refactoring


 1)

 
 * Merge server update commands into the one command
 (ipa-server-upgrade)
 

 So there is ipa-server-install to install the server,
 ipa-server-install
 --uninstall to uninstall the server and ipa-server-upgrade to
 upgrade the
 server. Maybe we should bring some consistency here and have one of:

   a) ipa-server-install [--install], ipa-server-install
 --uninstall,
 ipa-server-install --upgrade

   b) ipa-server-install [install], ipa-server-install uninstall,
 ipa-server-install upgrade

   c) ipa-server-install, ipa-server-uninstall,
 ipa-server-upgrade

 Long term, I think we want C. Besides other advantages, it will let
 us have
 independent sets of options, based on what you want to do.

 2)

 
 * Prevent to run IPA service, if code version and configuration
 version does
 not match
* ipactl should execute ipa-server-upgrade if needed
 

 There should be no configuration version, configuration update
 should be run
 always. It's fast and hence does not need to be optimized like data
 update by
 using a monolithic version number, which brings more than a few
 problems on its
 own.

 I do not agree in this section. Why would you like to run it always,
 even if it
 was fast? No run is still faster than fast run.

 In the ideal case the installer would be idempotent and upgrade would
 be re-running the installer and we should aim to do just that. We kind
 of do that already, but there is a lot of code duplication in
 installers and ipa-upgradeconfig (I would like to fix that when
 refactoring installers). IMO it's better to always make 100% sure the
 configuration is correct rather than to save a second or two.
 I doesn't like this idea, if user wants to fix something, the one should
 use --skip-version-check option, and the IPA upgrade will be executed.

 Well, what I don't like is dealing with meaningless version numbers.
 They are causing us grief in API versioning and I don't see why it
 would be any different here.
 However you must keep the version because of schema and data upgrade, so
 why not to execute update as one batch instead of doing config upgrade
 all the time, and then data upgrade only if required.
 
 Because there is no exact mapping between version number and what features are
 actually available. A state file is tons better than a single version number.
 

 Some configuration upgrades, like adding new DNS related services,
 requires new schema, how we can handle this?
 
 This does not sound right. Could you be more specific?
 
 Running schema upgrade every time?

 What if a service changes in a way, the IPA configuration will not work?

 Then it's a bug and needs to be fixed, like any other bug. IIRC there
 was only one or two occurences of such bug in the past 3 years (I
 remember sshd_config), so I don't think you have a strong case here.
 Ok

 The user will need to change it manually, but after each restart,
 upgrade will change the value back into IPA required configuration which
 will not work.

 Says who? It's our code, we can do whatever we want, it doesn't have
 to be dumb like this.

 Yes, we have upgrade state file, but then the comparing of one value is
 faster then checking each state if was executed.

 How faster is that, like, few milliseconds? Are you seriously
 considering this the right optimization in a process that is
 magnitudes slower?
 Ok the speed is not so important, but I still do not like the idea of
 executing the code which is not needed to be executed, because I know
 the version is the same as was before last restart, so nothing changed.
 
 Weren't clever optimizations like this what got us into this whole
 refactoring bussiness in the first place?

I very much agree with Honza. We should always start with something
stupidly-simply and enhance it later, when it is clear if it is really 
necessary.

Do not over-engineer it from the very beginning.

Petr^2 Spacek

 My personal opinion is, application should not try to fix itself every
 restart.

 One could say that application should not try to upgrade itself every
 restart, but here we are, doing it anyway...
 I want to do upgrade only if needed not every restart.
 
 If there is nothing to upgrade, nothing will be upgraded. The effect is the 
 same.
 


 In the past, I do not recall
 ipa-upgradeconfig as being really fast, especially certmonger/Dogtag
 related
 updates were really slow thank to service restarts, etc.

 Correct, but I was talking about configuration file updates, not
 (re)starts, which 

Re: [Freeipa-devel] IPA Server upgrade 4.2 design

2015-03-03 Thread Petr Spacek
On 2.3.2015 18:54, Martin Basti wrote:
 On 02/03/15 18:28, Martin Kosek wrote:
 On 03/02/2015 06:12 PM, Martin Basti wrote:
 On 02/03/15 15:43, Rob Crittenden wrote:
 Martin Basti wrote:
 ...
 But you haven't explained any case why LDAPI would fail. If LDAPI fails
 then you've got more serious problems that I'm not sure binding as DM is
 going to solve.

 The only case where DM would be handy IMHO is either some worst case
 scenario upgrade where 389-ds is up but not binding to LDAPI or if you
 want to allow running LDAP updates as non-root.
 I don't know cases when LDAPI would failed, except the case LDAPI is
 misconfigured by user, or disabled by user.
 Wasn't LDAPI needed for the DM password less upgrade so that upgrader could
 simply bind as root with EXTERNAL auth?
 We can do upgrade in both way, using LDAPI or using DM password, preferred is
 LDAPI.
 Question is, what is the use case for using DM password instead of LDAPI
 during upgrade.

 It is not big effort to keep both DM binding and LDAPI in code.  A user can
 always found som unexpected use case for LDAP update with DM password.

 On ipactl, would it be overkill if there is a tty to prompt the user to
 upgrade? In a non-container world it might be surprising to have an
 upgrade happen esp since upgrades take a while.
 In non-container enviroment, we can still use upgrade during RPM
 transaction.

 So you suggest  not to do update automaticaly, just write Error the IPA
 upgrade is required?
 People do all sorts of strange things. Installing the packages with
 --no-script isn't in the range of impossible. A prompt, and I'm not
 saying it's a great idea, is 2 lines of code.

 I guess it just makes me nervous.
 So lets summarize this:
 * DO upgrade if possible during RPM transaction
 Umm, I thought we want to get rid of running upgrade during RPM transaction. 
 It
 is extremely difficult to debug upgrade stuck during RPM transaction, it also
 makes RPM upgrade run longer than needed. It also makes admins nervous when
 their rpm upgrade is suddenly waiting right before the end. I even see the
 fingers slowly reaching to CTRL+C combo... (You can see the consequences)
 People are used to have IPA upgraded and ready after RPM upgrade.
 They may be shocked if IPA services will be in shutdown state after RPM
 transaction.
 
 I have no more objections.

IMHO the problem with long-running RPM upgrade should be approached from the
other way: Just print message 'IPA server upgrade is running, press CTRL+C if
you want to destroy your IPA server'.

bind-dyndb-ldap prints a message about SELinux in RPM scriptlets for couple
releases now and nobody complained (yet? :-).

Petr^2 Spacek

 

 * ipactl will NOT run upgrade, just print Error: 'please upgrade '
 * User has to run ipa-server-upgrade manually

 Does I understand it correctly?
 With --skip-version-check what sorts of problems can we foresee? I
 assume a big warning will be added to at least the man page, if not
 the cli?
 For this big warning everywhere.
 The main problem is try to run older IPA with newer data. In containers
 the problem is to run different platform specific versions which differ
 in functionality/bugfixes etc..
 Ok, pretty much the things I was thinking as well. A scary warning
 definitely seems warranted.

 Where does platform come from? I'm wondering how Debian will handle this.
 platform is derived from ipaplatform file which is used with the
 particular build. Debian should have own platform file.
 Ok, I'd add that detail to the design.

 rob

 
 


-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] IPA Server upgrade 4.2 design

2015-02-26 Thread Petr Spacek
On 25.2.2015 17:49, Martin Basti wrote:
 On 25/02/15 17:15, Petr Spacek wrote:
 On 24.2.2015 19:10, Martin Basti wrote:
 Hello all,

 please read the design page, any objections/suggestions appreciated
 http://www.freeipa.org/page/V4/Server_Upgrade_Refactoring
 Thank you for the design, I have only few nitpicks.

 Increase update files numbers range
 Update files number will be extended into 4 digits values.
 IMHO the dependency on particular number format should be removed altogether.
 It should be perfectly enough to say that updates are executed in ASCII
 lexicographic order and be done with it.
 4.1.3-2  4.1.3-12 in lexicographic order, this will not fit.

Well, sure, but it allows you to use
00-a
01-b

and renumber it to

001-a
002-b

at will without changes to code. (Lexicographic order is what 'ls' uses by
default so you can see the real ordering at any time very easily.)

Also, as you pointed out, it allows you to do things like
12.345-a
12.666-bbb
without changing code, again :-)

Petr^2 Spacek

 To resolve issues mentioned above only one command should do upgrade:
 ipa-server-upgrade.
 I very much agree with this.


 ipa-server-upgrade characteristics
 ...
 4. LDAP data update (+ update plugins)
 5. upgrade configuration
 At this point I would appreciate explanatory text what is 'LDAP data update'
 and what is 'upgrade configuration'. Maybe some examples could be enough.
 LDAP data update == upgrading data stored in LDAP (user data + cn=config)
 upgrade configuration == upgrading configuration of services in filesystem
 (apache, named)
 
 I will add some explanation there.

 ipactl checks if installed version and version stored in LDAP are the same:
 ...
 ipactl start|restart option --force overrides this check.
 I would like to see a separate option. --force currently skips rollback if
 some services could not start but this is fundamentally different from
 version/upgrade checks.
 Ohh, good catch thank you, maybe --skip-version-check ?
Sounds fine to me.


 ipa-server-upgrade
 --version show program's version number and exit
 Maybe it could print code version + data version (if available). It could be
 handy debugging tool.
 Good idea thanks
 ipa-server-upgrade
 --test Note: for developing only
 Is it really worth the effort to keep the option and invest more time in it?

 I do not expect any extra effort (except fixing 3 plugins - 6 lines of code
 approx), so if it will help to develop updates it could stay there (personally
 I do not use it, usually updates broke during write to server on some
 constraints)

Okay, I thought that it is broken significantly.

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0190] DNSSEC: add support for CKM_RSA_PKCS_OAEP mechanism

2015-02-26 Thread Petr Spacek
On 11.2.2015 14:10, Martin Basti wrote:
 https://fedorahosted.org/freeipa/ticket/4657#comment:13
 
 Patch attached.
 
 -- 
 Martin Basti
 
 
 freeipa-mbasti-0190-DNSSEC-add-support-for-CKM_RSA_PKCS_OAEP-mechanism.patch
 
 
 From 4d698a5adaa94eb854c75bd9bcaf3093f31a11e5 Mon Sep 17 00:00:00 2001
 From: Martin Basti mba...@redhat.com
 Date: Wed, 11 Feb 2015 14:05:46 +0100
 Subject: [PATCH] DNSSEC add support for CKM_RSA_PKCS_OAEP mechanism
 
 Ticket: https://fedorahosted.org/freeipa/ticket/4657#comment:13
 ---
  ipapython/ipap11helper/p11helper.c | 72 
 --
  1 file changed, 69 insertions(+), 3 deletions(-)
 
 diff --git a/ipapython/ipap11helper/p11helper.c 
 b/ipapython/ipap11helper/p11helper.c
 index 
 4e0f262057b377124793f1e3091a8c9df4794164..c638bbe849f1bbddc8004bd1c41128b1c9e7
  100644
 --- a/ipapython/ipap11helper/p11helper.c
 +++ b/ipapython/ipap11helper/p11helper.c
 @@ -53,6 +53,22 @@
  // TODO
  #define CKA_COPYABLE   (0x0017)
  
 +#define CKG_MGF1_SHA1 (0x0001)
 +
 +#define CKZ_DATA_SPECIFIED(0x0001)
 +
 +struct ck_rsa_pkcs_oaep_params {
 +  CK_MECHANISM_TYPE hash_alg;
 +  unsigned long mgf;
 +  unsigned long source;
 +  void *source_data;
 +  unsigned long source_data_len;
 +};
 +
 +typedef struct ck_rsa_pkcs_oaep_params CK_RSA_PKCS_OAEP_PARAMS;
 +typedef struct ck_rsa_pkcs_oaep_params *CK_RSA_PKCS_OAEP_PARAMS_PTR;
 +
 +
  CK_BBOOL true = CK_TRUE;
  CK_BBOOL false = CK_FALSE;
  
 @@ -118,6 +134,17 @@ CK_BBOOL* bool;
  } PyObj2Bool_mapping_t;
  
  /**
 + * Constants
 + */
 +static const CK_RSA_PKCS_OAEP_PARAMS CONST_RSA_PKCS_OAEP_PARAMS = {
 +.hash_alg = CKM_SHA_1,
 +.mgf = CKG_MGF1_SHA1,
 +.source = CKZ_DATA_SPECIFIED,
 +.source_data = NULL,
 +.source_data_len = 0
 +};
 +
 +/**
   * ipap11helper Exceptions
   */
  static PyObject *ipap11helperException; //parent class for all exceptions
 @@ -1359,17 +1386,36 @@ P11_Helper_export_wrapped_key(P11_Helper* self, 
 PyObject *args, PyObject *kwds)
  CK_BYTE_PTR wrapped_key = NULL;
  CK_ULONG wrapped_key_len = 0;
  CK_MECHANISM wrapping_mech = { CKM_RSA_PKCS, NULL, 0 };
 -CK_MECHANISM_TYPE wrapping_mech_type = CKM_RSA_PKCS;
  /* currently we don't support parameter in mechanism */
  
  static char *kwlist[] = { key, wrapping_key, wrapping_mech, NULL };
  //TODO check long overflow
  //TODO export method
  if (!PyArg_ParseTupleAndKeywords(args, kwds, kkk|, kwlist, object_key,
 -object_wrapping_key, wrapping_mech_type)) {
 +object_wrapping_key, wrapping_mech.mechanism)) {
  return NULL;
  }
 -wrapping_mech.mechanism = wrapping_mech_type;
 +
 +// fill mech parameters
 +switch(wrapping_mech.mechanism){
 +case CKM_RSA_PKCS:
 +case CKM_AES_KEY_WRAP:
 +case CKM_AES_KEY_WRAP_PAD:
 +//default params
 +break;
 +
 +case CKM_RSA_PKCS_OAEP:
 +/* Use the same configuration as openSSL
 + * https://www.openssl.org/docs/crypto/RSA_public_encrypt.html
 + */
 + wrapping_mech.pParameter = (void*) CONST_RSA_PKCS_OAEP_PARAMS;
 + wrapping_mech.ulParameterLen = 
 sizeof(CONST_RSA_PKCS_OAEP_PARAMS);
 +break;
 +
 +default:
 +PyErr_SetString(ipap11helperError, Unsupported wrapping 
 mechanism);
 +return NULL;
 +}
  
  rv = self-p11-C_WrapKey(self-session, wrapping_mech,
  object_wrapping_key, object_key, NULL, wrapped_key_len);
 @@ -1452,6 +1498,26 @@ P11_Helper_import_wrapped_secret_key(P11_Helper* self, 
 PyObject *args,
  return NULL;
  }
  
 +switch(wrapping_mech.mechanism){
 +case CKM_RSA_PKCS:
 +case CKM_AES_KEY_WRAP:
 +case CKM_AES_KEY_WRAP_PAD:
 +//default params
 +break;

NACK. This switch is duplicate of the previous one. Please split it into an
auxiliary function and call it twice.

Thank you!

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0195] Fix memory leaks in ipapkcs11helper module

2015-02-26 Thread Petr Spacek
Hello Martin,

thank you for patch! This NACK is only aesthetic :-)

On 25.2.2015 14:21, Martin Basti wrote:
  if (!check_return_value(rv, import_wrapped_key: key unwrapping)) {
 +error = 1;
 +goto final;
 +}


This exact sequence is repeated many times in the code.

I would prefer a C macro like this:
#define GOTO_FAIL   \
do {\
error = 1;  \
goto final; \
} while(0)

This allows more dense code like:
if (!test)
GOTO_FAIL;

and does not have the risk of missing error = 1 somewhere.

 +final:
  if (pkey != NULL)
  EVP_PKEY_free(pkey);
 +if (label != NULL) PyMem_Free(label);
 +if (error){
 +return NULL;
 +}
  return ret;
  }

Apparently, this is inconsistent with itself.

Please pick one style and use it, e.g.
if (label != NULL)
PyMem_Free(label)

... and do not add curly braces when unnecessary.

If you want, we can try running $ indent on current sources and committing
changes separately so you do not have to make changes like this by hand.

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] IPA Server upgrade 4.2 design

2015-02-25 Thread Petr Spacek
On 24.2.2015 19:10, Martin Basti wrote:
 Hello all,
 
 please read the design page, any objections/suggestions appreciated
 http://www.freeipa.org/page/V4/Server_Upgrade_Refactoring

Thank you for the design, I have only few nitpicks.

 Increase update files numbers range
 Update files number will be extended into 4 digits values.

IMHO the dependency on particular number format should be removed altogether.
It should be perfectly enough to say that updates are executed in ASCII
lexicographic order and be done with it.

 To resolve issues mentioned above only one command should do upgrade: 
 ipa-server-upgrade. 
I very much agree with this.


 ipa-server-upgrade characteristics
...
 4. LDAP data update (+ update plugins)
 5. upgrade configuration
At this point I would appreciate explanatory text what is 'LDAP data update'
and what is 'upgrade configuration'. Maybe some examples could be enough.

 ipactl checks if installed version and version stored in LDAP are the same: 
...
 ipactl start|restart option --force overrides this check. 
I would like to see a separate option. --force currently skips rollback if
some services could not start but this is fundamentally different from
version/upgrade checks.

 ipa-server-upgrade
 --version show program's version number and exit 
Maybe it could print code version + data version (if available). It could be
handy debugging tool.

 ipa-server-upgrade
 --testNote: for developing only

Is it really worth the effort to keep the option and invest more time in it?

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0319] Fix crash caused by race condition during resolver cache flushing

2015-02-24 Thread Petr Spacek
On 29.1.2015 15:42, Tomas Hozza wrote:
 On 01/13/2015 02:16 PM, Petr Spacek wrote:
  Hello,
  
  This patch should be applied to v2 branch.
  
  Fix crash caused by race condition during resolver cache flushing.
  
  dns_view_flushcache() call has to be always done in single-thread mode.
  Locking around the call was missing in forwarder reconfiguration and
  zone deletion which could cause crash.
  
  https://fedorahosted.org/bind-dyndb-ldap/ticket/142
 ACK.

Rebased and pushed to v2 branch:
274a3b1cd0ed58f307f4a8d0a7c2f4006fb5e35e

-- 
Petr Spacek  @  Red Hat

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


Re: [Freeipa-devel] [PATCH 0316] Fix crash triggered by zone objects with unexpected DN

2015-02-24 Thread Petr Spacek
Hello,

On 18.2.2015 10:36, Tomas Hozza wrote:
 On 12/16/2014 04:32 PM, Petr Spacek wrote:
 Hello,

 Fix crash triggered by zone objects with unexpected DN.

 https://fedorahosted.org/bind-dyndb-ldap/ticket/148

 NACK.
 
 The patch seems to make no difference when using the reproducer from ticket 
 148
 
 18-Feb-2015 10:34:09.067 running
 18-Feb-2015 10:34:09.139 ldap_helper.c:4876: INSIST(task == inst-task) 
 failed, back trace
 18-Feb-2015 10:34:09.139 #0 0x55587a80 in ??
 18-Feb-2015 10:34:09.139 #1 0x7620781a in ??
 18-Feb-2015 10:34:09.139 #2 0x720b00b2 in ??
 18-Feb-2015 10:34:09.140 #3 0x71e7ccf9 in ??
 18-Feb-2015 10:34:09.140 #4 0x71e7d992 in ??
 18-Feb-2015 10:34:09.140 #5 0x720a7f3b in ??
 18-Feb-2015 10:34:09.140 #6 0x75dda52a in ??
 18-Feb-2015 10:34:09.140 #7 0x7508d79d in ??
 18-Feb-2015 10:34:09.140 exiting (due to assertion failure)
 
 Program received signal SIGABRT, Aborted.
 [Switching to Thread 0x7fffea7cd700 (LWP 1719)]
 0x74fc18c7 in __GI_raise (sig=sig@entry=6) at 
 ../sysdeps/unix/sysv/linux/raise.c:55
 55  return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
 Missing separate debuginfos, use: debuginfo-install 
 cyrus-sasl-gssapi-2.1.26-19.fc21.x86_64 cyrus-sasl-lib-2.1.26-19.fc21.x86_64 
 cyrus-sasl-md5-2.1.26-19.fc21.x86_64 cyrus-sasl-plain-2.1.26-19.fc21.x86_64 
 gssproxy-0.3.1-4.fc21.x86_64 keyutils-libs-1.5.9-4.fc21.x86_64 
 libattr-2.4.47-9.fc21.x86_64 libdb-5.3.28-9.fc21.x86_64 
 libgcc-4.9.2-6.fc21.x86_64 libselinux-2.3-5.fc21.x86_64 
 nspr-4.10.8-1.fc21.x86_64 nss-3.17.4-1.fc21.x86_64 
 nss-softokn-freebl-3.17.4-1.fc21.x86_64 nss-util-3.17.4-1.fc21.x86_64 
 pcre-8.35-8.fc21.x86_64 sssd-client-1.12.3-4.fc21.x86_64 
 xz-libs-5.1.2-14alpha.fc21.x86_64
 (gdb) bt
 #0  0x74fc18c7 in __GI_raise (sig=sig@entry=6) at 
 ../sysdeps/unix/sysv/linux/raise.c:55
 #1  0x74fc352a in __GI_abort () at abort.c:89
 #2  0x55587c29 in assertion_failed (file=optimized out, 
 line=optimized out, type=optimized out, cond=optimized out) at 
 ./main.c:220
 #3  0x7620781a in isc_assertion_failed 
 (file=file@entry=0x720bad2a ldap_helper.c, line=line@entry=4876, 
 type=type@entry=isc_assertiontype_insist,
 cond=cond@entry=0x720baf04 task == inst-task) at assertions.c:57
 #4  0x720b00b2 in syncrepl_update (chgtype=1, entry=0x70125590, 
 inst=0x77fa3160) at ldap_helper.c:4876
 #5  ldap_sync_search_entry (ls=optimized out, msg=optimized out, 
 entryUUID=optimized out, phase=LDAP_SYNC_CAPI_ADD) at ldap_helper.c:5031
 #6  0x71e7ccf9 in ldap_sync_search_entry (ls=ls@entry=0x7fffe40008c0, 
 res=0x7fffe4003870) at ldap_sync.c:228
 #7  0x71e7d992 in ldap_sync_init (ls=0x7fffe40008c0, 
 mode=mode@entry=3) at ldap_sync.c:792
 #8  0x720a7f3b in ldap_syncrepl_watcher (arg=0x77fa3160) at 
 ldap_helper.c:5247
 #9  0x75dda52a in start_thread (arg=0x7fffea7cd700) at 
 pthread_create.c:310
 #10 0x7508d79d in clone () at 
 ../sysdeps/unix/sysv/linux/x86_64/clone.S:109

Thank you for catching this! I was using slightly different test which
triggered the new code but by using different code path.

This new version should be more robust. Please re-test it, thank you!

-- 
Petr^2 Spacek
From bfe9f7767ed86cdd562b36b801ce3ee6e6f2063b Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 24 Feb 2015 14:45:42 +0100
Subject: [PATCH] Fix crash triggered by zone objects with unexpected DN.

https://fedorahosted.org/bind-dyndb-ldap/ticket/148
---
 src/ldap_convert.c | 24 
 src/ldap_convert.h |  4 
 src/ldap_helper.c  |  3 +++
 3 files changed, 31 insertions(+)

diff --git a/src/ldap_convert.c b/src/ldap_convert.c
index b51d402492415d6630a42435b823925c8246a06f..dde10d6e989159e9f6f5086a4a12bbd165b73646 100644
--- a/src/ldap_convert.c
+++ b/src/ldap_convert.c
@@ -193,6 +193,30 @@ cleanup:
 }
 
 /**
+ * Evaluate if DN has/does not have expected format with one or two components
+ * and error out if a mismatch is detected.
+ *
+ * @param[in] prefix  Prefix for error messages, usually a function name.
+ * @param[in] dn
+ * @param[in] dniszoneBoolean returned by dn_to_dnsname for given DN.
+ * @param[in] classiszone ISC_TRUE if DN should be a zone, ISC_FALSE otherwise.
+ * @retval ISC_R_SUCCESS or ISC_R_UNEXPECTED if values do not match.
+ */
+isc_result_t
+dn_want_zone(const char * const prefix, const char * const dn,
+	 isc_boolean_t dniszone, isc_boolean_t classiszone) {
+	if (dniszone != classiszone) {
+		log_error(%s: object '%s' does%s have a zone object class 
+			  but DN format suggests that it is%s a zone,
+			  prefix, dn, classiszone ?  :  not,
+			  dniszone ?  :  not);
+		return ISC_R_UNEXPECTED;
+	}
+
+	return ISC_R_SUCCESS;
+}
+
+/**
  * WARNING! This function is used to mangle input from network
  *  and it is security sensitive.
  *
diff --git a/src/ldap_convert.h b/src/ldap_convert.h
index

Re: [Freeipa-devel] [PATCHES 0005-0011] Fix some of the defects reported by covscan on freeipa-master

2015-01-28 Thread Petr Spacek
On 27.1.2015 18:36, Martin Babinsky wrote:
 On 01/27/2015 06:05 PM, Petr Spacek wrote:
 On 27.1.2015 18:02, Alexander Bokovoy wrote:

 -slapi_search_internal_get_entry(sdn, attrs, entry,
 -otp_config_plugin_id(otp_config));
 +search_result = slapi_search_internal_get_entry(sdn, attrs, entry,
 +otp_config_plugin_id(otp_config));
 +if (search_result != LDAP_SUCCESS) {
 +LOG_TRACE(Entry not found. Error code: %d\n, search_result);
 +}
 I would rather say something more defensive here:
 Unable to access LDAP entry. Perhaps it does not exist? Error code: %d\n

 Would it be possible to print the DN? It may be useful in cases where it
 actually happened ...

 I will look into it.

BTW the error message should also contain __FILE__ and __LINE__ values to
unambiguously identify what happened - even if the error message
Unable to access LDAP entry. Perhaps it does not exist? Error code: %d\n
is the same on multiple places.

Thank you!

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCHES 0005-0011] Fix some of the defects reported by covscan on freeipa-master

2015-01-27 Thread Petr Spacek
On 27.1.2015 17:56, Alexander Bokovoy wrote:
 On Tue, 27 Jan 2015, Martin Babinsky wrote:
 From 23a823c3c5933d5c14342e15c00599af74b84118 Mon Sep 17 00:00:00 2001
 From: Martin Babinsky mbabi...@redhat.com
 Date: Tue, 27 Jan 2015 13:21:33 +0100
 Subject: [PATCH 3/7] proposed fix fo a defect reported in
 'ipa_kdb_principals.c'

 The patch addresses the following defect reported by covscan in FreeIPA
 master:

 
 Error: FORWARD_NULL (CWE-476): /daemons/ipa-kdb/ipa_kdb_principals.c:1886:
 assign_zero: Assigning:
 principal = NULL.  /daemons/ipa-kdb/ipa_kdb_principals.c:1929:
 var_deref_model: Passing null pointer principal to ipadb_entry_to_mods,
 which dereferences it.  /daemons/ipa-kdb/ipa_kdb_principals.c:1491:9:
 deref_parm_in_call: Function ipadb_get_ldap_mod_str dereferences
 principal.  /daemons/ipa-kdb/ipa_kdb_principals.c:1174:5:
 deref_parm_in_call: Function strdup dereferences value
 

 Again I have consulted this with Simo and he pointed out that this may indeed
 be a bug and that we should always parse principal name.

 This is a part of series of patches related to
 https://fedorahosted.org/freeipa/ticket/4795
 ---
 daemons/ipa-kdb/ipa_kdb_principals.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

 diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c
 b/daemons/ipa-kdb/ipa_kdb_principals.c
 index
 e158c236eab5c7c5a7c12664dbde5d51cc55406d..760faeef224701c3dc4ee69d16df043e9c622d9a
 100644
 --- a/daemons/ipa-kdb/ipa_kdb_principals.c
 +++ b/daemons/ipa-kdb/ipa_kdb_principals.c
 @@ -1894,19 +1894,16 @@ static krb5_error_code
 ipadb_modify_principal(krb5_context kcontext,
 if (!ipactx) {
 return KRB5_KDB_DBNOTINITED;
 }
 -
 +kerr = krb5_unparse_name(kcontext, entry-princ, principal);
 +if (kerr != 0) {
 +goto done;
 +}
 ied = (struct ipadb_e_data *)entry-e_data;
 if (!ied || !ied-entry_dn) {
 -kerr = krb5_unparse_name(kcontext, entry-princ, principal);
 -if (kerr != 0) {
 -goto done;
 -}
 -
 kerr = ipadb_fetch_principals(ipactx, 0, principal, res);
 if (kerr != 0) {
 goto done;
 }
 -
 /* FIXME: no alias allowed for now, should we allow modifies
  * by alias name ? */
 kerr = ipadb_find_principal(kcontext, 0, res, principal, lentry);
 -- 
 2.1.0

 NACK.
 
 This part actually looked to me familiar and it was indeed raised in
 past. I marked the Coverity report for this as a false positive but it
 sprung again.
 
 Last time I wrote (2014-04-07):
 --
 I remember looking at this bug already in past and I looked at it again.
 I think it is false positive. ipadb_modify_principal() is only called
 from ipadb_put_principal() when entry for the principal has no
 KMASK_PRINCIPAL flag set. Next, ipadb_modify_principal() calls
 ipadb_entry_to_mods() with a principal which might be set to NULL.
 However, ipadb_entry_to_mods() will only dereference this principal when
 KMASK_PRINCIPAL flag is set in the entry, i.e. never for this code path.
 ---

Hmm... It may work for now but it also may break silently in future if we
break current assumption ipadb_modify_principal() is only called from
ipadb_put_principal() when entry for the principal has no KMASK_PRINCIPAL flag
set.

Personally I'm against this kind of implicit assumptions in code. I was bitten
to may times by exactly this in BIND and I don't think we should do that
in our code.

Alexander, if you really don't want to move krb5_unparse_name() call then we
really need to handle this impossible case in some other way. Maybe with an
assert()? It would do nothing as long as your assumption holds and will
clearly show where the problem is if we break the assumption in future.

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCHES 0005-0011] Fix some of the defects reported by covscan on freeipa-master

2015-01-27 Thread Petr Spacek
On 27.1.2015 18:23, Alexander Bokovoy wrote:
 On Tue, 27 Jan 2015, Petr Spacek wrote:
 On 27.1.2015 17:56, Alexander Bokovoy wrote:
 On Tue, 27 Jan 2015, Martin Babinsky wrote:
 From 23a823c3c5933d5c14342e15c00599af74b84118 Mon Sep 17 00:00:00 2001
 From: Martin Babinsky mbabi...@redhat.com
 Date: Tue, 27 Jan 2015 13:21:33 +0100
 Subject: [PATCH 3/7] proposed fix fo a defect reported in
 'ipa_kdb_principals.c'

 The patch addresses the following defect reported by covscan in FreeIPA
 master:

 
 Error: FORWARD_NULL (CWE-476): /daemons/ipa-kdb/ipa_kdb_principals.c:1886:
 assign_zero: Assigning:
 principal = NULL.  /daemons/ipa-kdb/ipa_kdb_principals.c:1929:
 var_deref_model: Passing null pointer principal to ipadb_entry_to_mods,
 which dereferences it.  /daemons/ipa-kdb/ipa_kdb_principals.c:1491:9:
 deref_parm_in_call: Function ipadb_get_ldap_mod_str dereferences
 principal.  /daemons/ipa-kdb/ipa_kdb_principals.c:1174:5:
 deref_parm_in_call: Function strdup dereferences value
 

 Again I have consulted this with Simo and he pointed out that this may 
 indeed
 be a bug and that we should always parse principal name.

 This is a part of series of patches related to
 https://fedorahosted.org/freeipa/ticket/4795
 ---
 daemons/ipa-kdb/ipa_kdb_principals.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

 diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c
 b/daemons/ipa-kdb/ipa_kdb_principals.c
 index
 e158c236eab5c7c5a7c12664dbde5d51cc55406d..760faeef224701c3dc4ee69d16df043e9c622d9a

 100644
 --- a/daemons/ipa-kdb/ipa_kdb_principals.c
 +++ b/daemons/ipa-kdb/ipa_kdb_principals.c
 @@ -1894,19 +1894,16 @@ static krb5_error_code
 ipadb_modify_principal(krb5_context kcontext,
 if (!ipactx) {
 return KRB5_KDB_DBNOTINITED;
 }
 -
 +kerr = krb5_unparse_name(kcontext, entry-princ, principal);
 +if (kerr != 0) {
 +goto done;
 +}
 ied = (struct ipadb_e_data *)entry-e_data;
 if (!ied || !ied-entry_dn) {
 -kerr = krb5_unparse_name(kcontext, entry-princ, principal);
 -if (kerr != 0) {
 -goto done;
 -}
 -
 kerr = ipadb_fetch_principals(ipactx, 0, principal, res);
 if (kerr != 0) {
 goto done;
 }
 -
 /* FIXME: no alias allowed for now, should we allow modifies
  * by alias name ? */
 kerr = ipadb_find_principal(kcontext, 0, res, principal, lentry);
 -- 
 2.1.0

 NACK.

 This part actually looked to me familiar and it was indeed raised in
 past. I marked the Coverity report for this as a false positive but it
 sprung again.

 Last time I wrote (2014-04-07):
 --
 I remember looking at this bug already in past and I looked at it again.
 I think it is false positive. ipadb_modify_principal() is only called
 from ipadb_put_principal() when entry for the principal has no
 KMASK_PRINCIPAL flag set. Next, ipadb_modify_principal() calls
 ipadb_entry_to_mods() with a principal which might be set to NULL.
 However, ipadb_entry_to_mods() will only dereference this principal when
 KMASK_PRINCIPAL flag is set in the entry, i.e. never for this code path.
 ---

 Hmm... It may work for now but it also may break silently in future if we
 break current assumption ipadb_modify_principal() is only called from
 ipadb_put_principal() when entry for the principal has no KMASK_PRINCIPAL 
 flag
 set.

 Personally I'm against this kind of implicit assumptions in code. I was 
 bitten
 to may times by exactly this in BIND and I don't think we should do that
 in our code.

 Alexander, if you really don't want to move krb5_unparse_name() call then we
 really need to handle this impossible case in some other way. Maybe with an
 assert()? It would do nothing as long as your assumption holds and will
 clearly show where the problem is if we break the assumption in future.
 What about following just before creating mods?
 
 if (principal == NULL  entry-mask  KMASK_PRINCIPAL) {
   goto done;
 }

I don't know this piece of code so I can't say anything in particular.

Are you 100 % sure that it will not cause any harm in future? I'm hesitant to
introduce a goto branching which is not testable nowadays.

Personally I would prefer either assert() or moving the code as proposed in
the patch.

Thank you for understanding.

-- 
Petr Spacek  @  Red Hat

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


Re: [Freeipa-devel] [PATCHES 0005-0011] Fix some of the defects reported by covscan on freeipa-master

2015-01-27 Thread Petr Spacek
On 27.1.2015 18:02, Alexander Bokovoy wrote:

 -slapi_search_internal_get_entry(sdn, attrs, entry,
 -otp_config_plugin_id(otp_config));
 +search_result = slapi_search_internal_get_entry(sdn, attrs, entry,
 +otp_config_plugin_id(otp_config));
 +if (search_result != LDAP_SUCCESS) {
 +LOG_TRACE(Entry not found. Error code: %d\n, search_result);
 +}
 I would rather say something more defensive here:
 Unable to access LDAP entry. Perhaps it does not exist? Error code: %d\n

Would it be possible to print the DN? It may be useful in cases where it
actually happened ...

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCHES 0005-0011] Fix some of the defects reported by covscan on freeipa-master

2015-01-27 Thread Petr Spacek
On 27.1.2015 18:41, Alexander Bokovoy wrote:
 On Tue, 27 Jan 2015, Petr Spacek wrote:
 On 27.1.2015 18:23, Alexander Bokovoy wrote:
 On Tue, 27 Jan 2015, Petr Spacek wrote:
 On 27.1.2015 17:56, Alexander Bokovoy wrote:
 On Tue, 27 Jan 2015, Martin Babinsky wrote:
 From 23a823c3c5933d5c14342e15c00599af74b84118 Mon Sep 17 00:00:00 2001
 From: Martin Babinsky mbabi...@redhat.com
 Date: Tue, 27 Jan 2015 13:21:33 +0100
 Subject: [PATCH 3/7] proposed fix fo a defect reported in
 'ipa_kdb_principals.c'

 The patch addresses the following defect reported by covscan in FreeIPA
 master:

 
 Error: FORWARD_NULL (CWE-476): 
 /daemons/ipa-kdb/ipa_kdb_principals.c:1886:
 assign_zero: Assigning:
 principal = NULL.  /daemons/ipa-kdb/ipa_kdb_principals.c:1929:
 var_deref_model: Passing null pointer principal to 
 ipadb_entry_to_mods,
 which dereferences it.  /daemons/ipa-kdb/ipa_kdb_principals.c:1491:9:
 deref_parm_in_call: Function ipadb_get_ldap_mod_str dereferences
 principal.  /daemons/ipa-kdb/ipa_kdb_principals.c:1174:5:
 deref_parm_in_call: Function strdup dereferences value
 

 Again I have consulted this with Simo and he pointed out that this may
 indeed
 be a bug and that we should always parse principal name.

 This is a part of series of patches related to
 https://fedorahosted.org/freeipa/ticket/4795
 ---
 daemons/ipa-kdb/ipa_kdb_principals.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

 diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c
 b/daemons/ipa-kdb/ipa_kdb_principals.c
 index
 e158c236eab5c7c5a7c12664dbde5d51cc55406d..760faeef224701c3dc4ee69d16df043e9c622d9a


 100644
 --- a/daemons/ipa-kdb/ipa_kdb_principals.c
 +++ b/daemons/ipa-kdb/ipa_kdb_principals.c
 @@ -1894,19 +1894,16 @@ static krb5_error_code
 ipadb_modify_principal(krb5_context kcontext,
 if (!ipactx) {
 return KRB5_KDB_DBNOTINITED;
 }
 -
 +kerr = krb5_unparse_name(kcontext, entry-princ, principal);
 +if (kerr != 0) {
 +goto done;
 +}
 ied = (struct ipadb_e_data *)entry-e_data;
 if (!ied || !ied-entry_dn) {
 -kerr = krb5_unparse_name(kcontext, entry-princ, principal);
 -if (kerr != 0) {
 -goto done;
 -}
 -
 kerr = ipadb_fetch_principals(ipactx, 0, principal, res);
 if (kerr != 0) {
 goto done;
 }
 -
 /* FIXME: no alias allowed for now, should we allow modifies
  * by alias name ? */
 kerr = ipadb_find_principal(kcontext, 0, res, principal, 
 lentry);
 -- 
 2.1.0

 NACK.

 This part actually looked to me familiar and it was indeed raised in
 past. I marked the Coverity report for this as a false positive but it
 sprung again.

 Last time I wrote (2014-04-07):
 --
 I remember looking at this bug already in past and I looked at it again.
 I think it is false positive. ipadb_modify_principal() is only called
 from ipadb_put_principal() when entry for the principal has no
 KMASK_PRINCIPAL flag set. Next, ipadb_modify_principal() calls
 ipadb_entry_to_mods() with a principal which might be set to NULL.
 However, ipadb_entry_to_mods() will only dereference this principal when
 KMASK_PRINCIPAL flag is set in the entry, i.e. never for this code path.
 ---

 Hmm... It may work for now but it also may break silently in future if we
 break current assumption ipadb_modify_principal() is only called from
 ipadb_put_principal() when entry for the principal has no KMASK_PRINCIPAL
 flag
 set.

 Personally I'm against this kind of implicit assumptions in code. I was
 bitten
 to may times by exactly this in BIND and I don't think we should do 
 that
 in our code.

 Alexander, if you really don't want to move krb5_unparse_name() call then 
 we
 really need to handle this impossible case in some other way. Maybe with 
 an
 assert()? It would do nothing as long as your assumption holds and will
 clearly show where the problem is if we break the assumption in future.
 What about following just before creating mods?

 if (principal == NULL  entry-mask  KMASK_PRINCIPAL) {
   goto done;
 }

 I don't know this piece of code so I can't say anything in particular.

 Are you 100 % sure that it will not cause any harm in future? I'm hesitant to
 introduce a goto branching which is not testable nowadays.

 Personally I would prefer either assert() or moving the code as proposed in
 the patch.
 assert() (with exit) is wrong here. Logging a note -- OK.
 
 I'm opposing moving the code because this is one of functions that gets
 called for _every_ Kerberos authentication as we modify last
 authentication time.

Does it matter? Does krb5_unparse_name() have an significant performance
impact? (I did not read the code so I'm asking.)


Given that the function is in authentication path, I think that it should fail
if original assumptions do not hold anymore.

I.e. I would modify the code to do this:
if (principal

[Freeipa-devel] [PATCH 0320] Fix description of idnsAllowQuery attribute in README

2015-01-21 Thread Petr Spacek
Hello,

Fix description of idnsAllowQuery attribute in README.

https://fedorahosted.org/bind-dyndb-ldap/ticket/154

I got off-list ACK from Martin^2.

Pushed to master:
a4565b3ef843e4464d2e950f0716818e7c7ce09b

-- 
Petr^2 Spacek
From 94169b05e3a5e2d5b4c522274c50e523b0c1f030 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 21 Jan 2015 13:53:02 +0100
Subject: [PATCH] Fix description of idnsAllowQuery attribute in README.

https://fedorahosted.org/bind-dyndb-ldap/ticket/154
---
 README | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/README b/README
index 1b8f2a713ad901d3aab5ed799cbb9c5e9c746c44..47cd9f68a3d27131ba95a07839cc3c2c5f2e8e49 100644
--- a/README
+++ b/README
@@ -62,23 +62,22 @@ Attributes:
 	value dyn_update from plugin configuration will be used.
 
 * idnsAllowQuery
-	Specifies BIND9 zone ACL element. This attribute can be set multiple
-	times and are merged together to the one ACL.
+	Specifies BIND9 zone ACL element as one string.
 
-	Example:
-		idnsAllowQuery: 127.0.0.1
-		idnsAllowQuery: ::1
-		idnsAllowQuery: 192.168.1.0/24
+	Example 1:  idnsAllowQuery: 192.0.2.1
+	In the first example above, only the client with 192.0.2.1 IP address
+	is allowed to query records from the zone.
 
-	In the example above clients with 127.0.0.1 and ::1 IP addresses and
-	clients from the 192.168.1.0/24 network are allowed to obtain records
-	from the zone.
+	Example 2:  idnsAllowQuery: !192.0.2.33; 192.0.2.0/24;
+	In the second example, queries from client 192.0.2.33 are refused
+	but queries from all other clients in the 192.0.2.0/24 network
+	are allowed.
 
 	You can specify IPv4/IPv6 address, IPv4/IPv6 network address in CIDR
-	format and any or none keywords. The ! prefix (for example
-	!192.168.1.0/24) means negation of the ACL element.
+	format, and any or none keywords. The ! prefix (for example
+	!192.0.2.33) means negation of the ACL element.
 
-	If not set then zone inherits global allow-query from named.conf.
+	If not set, then zone inherits global allow-query from named.conf.
 
 * idnsAllowTransfer
 	Uses same format as idnsAllowQuery. Allows zone transfers for matching
-- 
2.1.0

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

Re: [Freeipa-devel] [PATCH] 0035 client: Update DNS with all available local IP addresses.

2015-01-20 Thread Petr Spacek
On 15.1.2015 20:49, Lukas Slebodnik wrote:
 On (15/01/15 20:38), Martin Basti wrote:
 On 15/01/15 20:24, Martin Basti wrote:
 On 15/01/15 17:13, David Kupka wrote:
 On 01/15/2015 03:22 PM, David Kupka wrote:
 On 01/15/2015 12:43 PM, David Kupka wrote:
 On 01/12/2015 06:34 PM, Martin Basti wrote:
 On 09/01/15 14:43, David Kupka wrote:
 On 01/07/2015 04:15 PM, Martin Basti wrote:
 On 07/01/15 12:27, David Kupka wrote:
 https://fedorahosted.org/freeipa/ticket/4249

 Thank you for patch:

 1)
 -root_logger.error(Cannot update DNS records! 
 -  Failed to connect to server '%s'.,
 server)
 +ips = get_local_ipaddresses()
 +except CalledProcessError as e:
 +root_logger.error(Cannot update DNS records. %s % e)

 IMO the error message should be more specific,  add there something
 like
 Unable to get local IP addresses. at least in log.debug()

 2)
 +lines = ipresult[0].replace('\\', '').split('\n')

 .replace() is not needed

 3)
 +if len(ips) == 0:

 if not ips:

 is more pythonic by PEP8


 Thanks for catching these. Updated patch attached.

 merciful NACK

 Thank you for the patch, unfortunately I hit one issue which needs
 to be
 resolved.

 If sync PTR is activated in zone settings, and reverse zone doesn't
 exists, nsupdate/BIND returns SERVFAIL and ipa-client-install print
 Error message, 'DNS update failed'. In fact, all A/ records was
 succesfully updated, only PTR records failed.

 Bind log:
 named-pkcs11[28652]: updating zone 'example.com/IN': adding an RR at
 'vm-101.example.com' 

 named-pkcs11[28652]: PTR record synchronization (addition) for A/
 'vm-101.example.com.' refused: unable to find active reverse zone
 for IP
 address '2620:52:0:104c:21a:4aff:fe10:4eaa': not found

 With IPv6 we have several addresses from different reverse zones and
 this situation may happen often.
 I suggest following:
 1) Print list of addresses which will be updated. (Now if update
 fails,
 user needs to read log, which addresses installer tried to update)
 2) Split nsupdates per A/ record.
 3a) If failed, check with DNS query if A/ and PTR record are
 there
 and print proper error message
 3b) Just print A/ (or PTR) record may not be updated for
 particular
 IP address.

 Any other suggestions are welcome.


 After long discussion with DNS and UX guru I've implemented it this
 way:
 1. Call nsupdate only once with all updates.
 2. Verify that the expected records are resolvable.
 3. If no print list of missing A/, list of missing PTR records and
 list to mismatched PTR record.

 As this is running inside client we can't much more and it's up to
 user
 to check what's rotten in his DNS setup.

 Updated patch attached.


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



 One more change to behave well in -crazy- exotic environments that
 resolves more PTR records for single IP.



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


 Yet another change to make language nerds and our UX guru happy :-)
 Sorry, but NACK.

 1) BIND/dyndb-ldap bug? (if sync_ptr is enabled)
 +try:
 +answers = dns.resolver.query(fqdn, record_type)
 +except dns.resolver.NoAnswer:
 +if record_type == dns.rdatatype.A:
 +root_logger.debug('No A record for %s' % fqdn)
 +elif record_type == dns.rdatatype.:
 +root_logger.debug('No  record for %s' % fqdn)
 +except dns.exception.DNSException as e:
 +root_logger.debug('DNS resolver error: ' % e)
 +else:
 +for rdata in answers:
 +try:
 +missing_ips.remove(rdata.address)
 +except ValueError:
 +extra_ips.append(rdata.address)

 This somehow doesn't work, for missing A/ records (4 A/ records
 expected)
 $host `hostname`
 vm-024.example.com has address 10.16.78.24
 vm-024.example.com has IPv6 address fed0:babe:baab:0:21a:4aff:fe10:4e37
 But I get *no warning*.

 == Why ==
 Probably bug in BIND, all /A records *exists for several seconds*, then
 bind remove all A/ records without PTR record.
 (Needs more investigation, maybe it is dependent on bind version, in
 previous testing, the A/ records stay untouched )

 This it the older journal from the *same machine* with same packages, where
 record without PTR haven't been deleted after few seconds
 EXAMPLE.COM: updating zone 'example.com/IN': deleting rrset at
 'vm-101.example.com' A
 EXAMPLE.COM: updating zone 'example.com/IN': deleting rrset at
 'vm-101.example.com' 
 EXAMPLE.COM: updating zone 'example.com/IN': adding an RR at
 'vm-101.example.com' A
 EXAMPLE.COM: updating zone 'example.com/IN': adding an RR at
 'vm-101.example.com' 

Re: [Freeipa-devel] [PATCH 0170, 0183] Detect and warn about invalid forwardzone configuration

2015-01-15 Thread Petr Spacek
On 14.1.2015 17:20, Martin Basti wrote:
 On 12/12/14 13:52, Martin Basti wrote:
 On 12/12/14 13:50, Martin Kosek wrote:
 On 12/11/2014 05:44 PM, Petr Spacek wrote:
 On 11.12.2014 16:50, Martin Basti wrote:
 Updated aptch attached:

 Nice work, ACK!


 Can we also add some tests? This is a lot of new code that could break 
 stuff.

 We can, in week maybe or after christmas,  currently I do some work with
 tests and adding new tests required by QE, I will add forwardzone warnings 
 tests when finish this.

 
 Added tests (required patch 0170).
 Original and new patch attached.

Well done! ACK.

-- 
Petr^2 Spacek

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


[Freeipa-devel] [PATCH 0022] Fix default value type for wait_for_dns optio

2015-01-13 Thread Petr Spacek
Hello,

Fix default value type for wait_for_dns option

wait_for_dns value should be an integer so default value was changed from
False to 0.

-- 
Petr^2 Spacek
From 15b0d338d7eb9b11cee7acfb1171367cbb8e723e Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 13 Jan 2015 10:14:43 +0100
Subject: [PATCH] Fix default value type for wait_for_dns option

wait_for_dns value should be an integer so default value was changed from
False to 0.
---
 ipalib/constants.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipalib/constants.py b/ipalib/constants.py
index df31a208822bc6901307424019e1fb70b0e3933c..50a2b1f7aa7f0d447bacfd005b102c7451e670ce 100644
--- a/ipalib/constants.py
+++ b/ipalib/constants.py
@@ -147,7 +147,7 @@ DEFAULT_CONFIG = (
 ('debug', False),
 ('startup_traceback', False),
 ('mode', 'production'),
-('wait_for_dns', False),
+('wait_for_dns', 0),
 
 # CA plugin:
 ('ca_host', FQDN),  # Set in Env._finalize_core()
-- 
2.1.0

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

Re: [Freeipa-devel] [PATCH 0319] Fix crash caused by race condition during resolver cache flushing

2015-01-13 Thread Petr Spacek
On 13.1.2015 14:16, Petr Spacek wrote:
 Hello,
 
 This patch should be applied to v2 branch.
 
 Fix crash caused by race condition during resolver cache flushing.
 
 dns_view_flushcache() call has to be always done in single-thread mode.
 Locking around the call was missing in forwarder reconfiguration and
 zone deletion which could cause crash.
 
 https://fedorahosted.org/bind-dyndb-ldap/ticket/142

Note: master branch should not be affected, this patch is relevant only to
versions  5.3.

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0022] Fix default value type for wait_for_dns optio

2015-01-13 Thread Petr Spacek
On 13.1.2015 10:57, Martin Kosek wrote:
 On 01/13/2015 10:16 AM, Petr Spacek wrote:
 Hello,

 Fix default value type for wait_for_dns option

 wait_for_dns value should be an integer so default value was changed from
 False to 0.

 
 Thanks. I stumbled on this value this morning, when setting it to True did not
 work.
 
 IMO, it would make sense to even rename the option to something that does not
 indicate boolean semantics, like dns_wait_retry_attempts or similar. But it
 may be too disruptive as at least our CI scripts would have to be updated.
 
 Up to you.

Well, open a ticket for option rename :-)

Anyway, this change can be pushed as-is (after review) even if we decide to
rename it later.

-- 
Petr^2 Spacek

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


[Freeipa-devel] FYI: LANGSEC: Sane protocol design and input parsing

2015-01-13 Thread Petr Spacek
Hello,

FYI, I came across an interesting article/idea:

LANGSEC: Language-theoretic Security
The Language-theoretic approach (LANGSEC) regards the Internet insecurity
epidemic as a consequence of ad hoc programming of input handling at all
layers of network stacks, and in other kinds of software stacks.

IMHO it is worth few minutes of your time:
http://www.cs.dartmouth.edu/~sergey/langsec/

TL;DR version in pictures:
http://www.cs.dartmouth.edu/~sergey/langsec/occupy/

Enjoy :-)

-- 
Petr^2 Spacek

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


[Freeipa-devel] [PATCH 0319] Fix crash caused by race condition during resolver cache flushing

2015-01-13 Thread Petr Spacek
Hello,

This patch should be applied to v2 branch.

Fix crash caused by race condition during resolver cache flushing.

dns_view_flushcache() call has to be always done in single-thread mode.
Locking around the call was missing in forwarder reconfiguration and
zone deletion which could cause crash.

https://fedorahosted.org/bind-dyndb-ldap/ticket/142

-- 
Petr^2 Spacek
From dce6ac00e48834c4c81e7041e4418c3d49b79725 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 19 Dec 2014 15:34:47 +0100
Subject: [PATCH] Fix crash caused by race condition during resolver cache
 flushing.

dns_view_flushcache() call has to be always done in single-thread mode.
Locking around the call was missing in forwarder reconfiguration and
zone deletion which could cause crash.
---
 src/ldap_helper.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 8febc2d7d0b985d423d7b64e7397f9c579487caf..44b31e9ee9175e2f4bffd312c498ee83e8aab149 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -854,6 +854,7 @@ ldap_delete_zone2(ldap_instance_t *inst, dns_name_t *name, isc_boolean_t lock,
 {
 	isc_result_t result;
 	isc_result_t isforward = ISC_R_NOTFOUND;
+	isc_result_t lock_result;
 	isc_boolean_t unlock = ISC_FALSE;
 	isc_boolean_t freeze = ISC_FALSE;
 	dns_zone_t *zone = NULL;
@@ -886,7 +887,13 @@ ldap_delete_zone2(ldap_instance_t *inst, dns_name_t *name, isc_boolean_t lock,
 		if (isforward == ISC_R_SUCCESS)
 			log_debug(1, forward zone '%s': unloaded, zone_name_char);
 		log_debug(1, zone '%s' not found in zone register, zone_name_char);
+		lock_result = isc_task_beginexclusive(inst-task);
+		/* dns_view_flushcache() has to be always locked no matter what */
+		RUNTIME_CHECK(lock_result == ISC_R_SUCCESS ||
+			  lock_result == ISC_R_LOCKBUSY);
 		result = dns_view_flushcache(inst-view);
+		if (lock_result == ISC_R_SUCCESS)
+			isc_task_endexclusive(inst-task);
 		goto cleanup;
 	} else if (result != ISC_R_SUCCESS)
 		goto cleanup;
@@ -988,6 +995,7 @@ configure_zone_forwarders(ldap_entry_t *entry, ldap_instance_t *inst,
 	const char *dn = entry-dn;
 	isc_result_t result;
 	isc_result_t orig_result;
+	isc_result_t lock_result;
 	ldap_valuelist_t values;
 	ldap_value_t *value;
 	isc_sockaddrlist_t addrs;
@@ -1165,7 +1173,12 @@ cleanup:
 		log_debug(5, %s '%s': forwarder table was updated: %s,
 			  msg_obj_type, dn, dns_result_totext(result));
 		orig_result = result;
+		lock_result = isc_task_beginexclusive(inst-task);
+		RUNTIME_CHECK(lock_result == ISC_R_SUCCESS ||
+			  lock_result == ISC_R_LOCKBUSY);
 		result = dns_view_flushcache(inst-view);
+		if (lock_result == ISC_R_SUCCESS)
+			isc_task_endexclusive(inst-task);
 		if (result == ISC_R_SUCCESS)
 			result = orig_result;
 	}
-- 
2.1.0

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

Re: [Freeipa-devel] Reviewed-By for design pages?

2015-01-12 Thread Petr Spacek
On 9.1.2015 14:26, Martin Kosek wrote:
 On 01/07/2015 05:41 PM, Alexander Bokovoy wrote:
 On Wed, 07 Jan 2015, Simo Sorce wrote:
 On Wed, 07 Jan 2015 10:34:59 +0100
 Petr Spacek pspa...@redhat.com wrote:

 Hello,

 I wonder if we should add something like Reviewed-by tag to newly
 created design pages. It would serve as reminder and check that page
 was really reviewed by someone. (And that we should not spend much
 time on implementation before the tag is present on the page.)

 It will also add 'psychological pressure' to the reviewer because his
 name will be on the design page forever :-)


 +1
 +1 too.
 
 Maybe we could move author and reviewer to the Feature info box with
 feature page metadata? (see box e.g. in http://www.freeipa.org/page/V4/OTP)
 
 I just wonder that, given that authorship and review is often collaborative
 work, multiple people may be interested to be listed in these fields in the
 info box or the page itself.

Feature box is IMHO a nice idea.

I was also wondering about templates like {{review_pending}} and
{{review_done}} with some nice icons - to make obvious if the design is ready
for implementation or not (yet).

Review-done icon:
http://commons.wikimedia.org/wiki/File:Green_tick.svg

Review-pending icon:
http://commons.wikimedia.org/wiki/File:Incomplete.svg

(Both icons are in public domain.)

-- 
Petr^2 Spacek

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


[Freeipa-devel] [PATCH 0317-0318] Preparation for 7.0 release

2015-01-12 Thread Petr Spacek
Hello,

I'm going to release v7.0 with BIND 9.10 support.

Following patches were pushed to master:

Bump NVR to 7.0.
257618e618a27b3c818f75faf5762130df0701eb

Update NEWS for upcoming 7.0 release.
c125f23501f8c53047375e45b72e73690fe791a3

-- 
Petr^2 Spacek
From c125f23501f8c53047375e45b72e73690fe791a3 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Mon, 12 Jan 2015 15:16:29 +0100
Subject: [PATCH] Update NEWS for upcoming 7.0 release.

---
 NEWS | 5 +
 1 file changed, 5 insertions(+)

diff --git a/NEWS b/NEWS
index 313151ab197e4529def7953a1fbffa8fc3e8c5a7..fb1e8b8d6758c755dcb31a529389dacf0425f91e 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,8 @@
+7.0
+
+[1] Support for BIND 9.10 was added.
+https://fedorahosted.org/bind-dyndb-ldap/ticket/139
+
 6.1
 
 [1] Crash caused by interaction between forward and master zones was fixed.
-- 
2.1.0

From 257618e618a27b3c818f75faf5762130df0701eb Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Mon, 12 Jan 2015 15:16:43 +0100
Subject: [PATCH] Bump NVR to 7.0.

---
 configure.ac | 2 +-
 contrib/bind-dyndb-ldap.spec | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 9f8f07c01e3feefe4254f73203bbc06b2bce28cd..9026f6d70fb008813681ab3f3eb51e9e2fec7be0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
 AC_PREREQ([2.59])
-AC_INIT([bind-dyndb-ldap], [6.1], [freeipa-devel@redhat.com])
+AC_INIT([bind-dyndb-ldap], [7.0], [freeipa-devel@redhat.com])
 
 AM_INIT_AUTOMAKE([-Wall foreign dist-bzip2])
 
diff --git a/contrib/bind-dyndb-ldap.spec b/contrib/bind-dyndb-ldap.spec
index 01f083bcd57bbdc939840c036a53771a8e0ebcfa..1ef5bcffb1d3f91291d9ac0510a892b3b75e4820 100644
--- a/contrib/bind-dyndb-ldap.spec
+++ b/contrib/bind-dyndb-ldap.spec
@@ -1,7 +1,7 @@
 %define VERSION %{version}
 
 Name:   bind-dyndb-ldap
-Version:6.1
+Version:7.0
 Release:0%{?dist}
 Summary:LDAP back-end plug-in for BIND
 
-- 
2.1.0

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

Re: [Freeipa-devel] [PATCH 0315] Support BIND 9.10

2015-01-12 Thread Petr Spacek
On 9.1.2015 16:34, Tomas Hozza wrote:
 On 12/04/2014 05:04 PM, Petr Spacek wrote:
 Hello,

 Support BIND 9.10.

 https://fedorahosted.org/bind-dyndb-ldap/ticket/139


 This patch definitely needs more testing but ...:
 - It compiles with BIND 9.9 and BIND 9.10.
 - It seems that it is able to load master and forward zones.
 - Basic dynamic updates work.

 I did not test other features yet.

 ACK.

Thank you!

I have branched v6 before push so v6 stays at
5adca1bbfc2c19c53ecda649b94960f6abe73cf6

... and pushed this patch to master (to-be-v7):
7101194d6bcf99c8cc5c8fec405ee716cd6e7b07

-- 
Petr^2 Spacek

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


[Freeipa-devel] automatic backup before FreeIPA upgrade?

2015-01-09 Thread Petr Spacek
Hello,

I wonder if it is feasible to automatically run backup before FreeIPA upgrade 
...

How big the backup file is (I'm asking about X in expression X + database
size)? Is there any reason not to do auto-backup?

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCHES 180-181] Fix forwardzone update

2015-01-09 Thread Petr Spacek
On 8.1.2015 16:53, Martin Basti wrote:
 On 08/01/15 16:42, Petr Spacek wrote:
 On 7.1.2015 13:56, Martin Basti wrote:
 +for config_option in container_entry.get(ipaConfigString, []):
 +matched = re.match(r^DNSVersion\s+(?Pversion%d)$,
 %d is C-ishm which does not work

 +   config_option, flags=re.I)
 +if matched and matched.group(version) = 1:
 group(version) should be converted to int before comparison

 Otherwise it seems that zone update itself works flawlessly for me.

 HUGE NACK :-D

 Stupid me.
 Updated patches attached.

ACK

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCHES 180-181] Fix forwardzone update

2015-01-08 Thread Petr Spacek
On 7.1.2015 13:56, Martin Basti wrote:
 +for config_option in container_entry.get(ipaConfigString, []):
 +matched = re.match(r^DNSVersion\s+(?Pversion%d)$,
%d is C-ishm which does not work

 +   config_option, flags=re.I)
 +if matched and matched.group(version) = 1:
group(version) should be converted to int before comparison

Otherwise it seems that zone update itself works flawlessly for me.

HUGE NACK :-D

-- 
Petr^2 Spacek

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


[Freeipa-devel] Reviewed-By for design pages?

2015-01-07 Thread Petr Spacek
Hello,

I wonder if we should add something like Reviewed-by tag to newly created
design pages. It would serve as reminder and check that page was really
reviewed by someone. (And that we should not spend much time on implementation
before the tag is present on the page.)

It will also add 'psychological pressure' to the reviewer because his name
will be on the design page forever :-)

-- 
Petr^2 Spacek

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


[Freeipa-devel] DNS forward zone upgrade problem: post-mortem

2015-01-05 Thread Petr Spacek
Hello,

as you may now, me and Martin^2 Basti screwed upgrades from RHEL 6.x to RHEL 
7.1+.

Cause
=
RHEL 7.1/bind-dyndb-ldap 6.x supports new object class idnsForwardZone and has
modified idnsZone object class semantics . This new semantics match what is
called master zones in BIND terminology.

Motivation
==
We have two main reasons for the change:
1) Clearly define (and un-obscure) idnsZone semantics to make implementation
of master root zones  global forwarding possible at the same time.

2) Match BIND semantics for master and forward zones, i.e. make all the BIND
docs and how-tos applicable to FreeIPA. We had many user-reported problems
with forward zone configuration in the past just because the semantics was
obscure and ill-defined.

Upgrade
===
To make upgrade possible we decided to add support for new idnsForwardZone
object class to RHEL 7.0/bind-dyndb-ldap 3.5. This version serves as
'transitional' element between old and new semantics because it supports new
object class idnsForwardZone but still expects old semantics on the old object
class idnsZone.

RHEL 7.1/bind-dyndb-ldap 6.x/FreeIPA 4.x supports new object class and at the
same time expects new semantics of the idnsZone object class.
FreeIPA upgrade automatically transforms existing data to the new format which
is understood by RHEL 7.0.

After upgrade, the new idnsZone semantics will stay be unused until user
decides to use it so this is covered by new features will not be available on
old servers clause in our release notes.

For some reason nobody realized that RHEL 6.x replicas will never have
bind-dyndb-ldap 3.5, i.e. the hybrid version/'transitional' element which
helps with upgrade.

Solution (for now)

I have patched bind-dyndb-ldap 2.x in RHEL 6.x to add support for new
idnsForwardZone object class to it:
https://bugzilla.redhat.com/show_bug.cgi?id=1175318

Upgrade will work as expected if users install this patched version before
introducing RHEL 7.1 to their topology.


All the gory details are in the design document:
http://www.freeipa.org/page/V4/Forward_zones

(Clearly, domain levels would help ...)

-- 
Petr^2 Spacek

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


[Freeipa-devel] [PATCH 0316] Fix crash triggered by zone objects with unexpected DN

2014-12-16 Thread Petr Spacek
Hello,

Fix crash triggered by zone objects with unexpected DN.

https://fedorahosted.org/bind-dyndb-ldap/ticket/148

-- 
Petr^2 Spacek
From d9e2bd9a838882706ca95d60eefd459a95ae7579 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 16 Dec 2014 16:31:16 +0100
Subject: [PATCH] Fix crash triggered by zone objects with unexpected DN.

https://fedorahosted.org/bind-dyndb-ldap/ticket/148
---
 src/ldap_convert.c | 24 
 src/ldap_convert.h |  4 
 src/ldap_helper.c  | 12 ++--
 3 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/src/ldap_convert.c b/src/ldap_convert.c
index b51d402492415d6630a42435b823925c8246a06f..6db90375ca1465208d6cce8772637ddc20a7e48e 100644
--- a/src/ldap_convert.c
+++ b/src/ldap_convert.c
@@ -193,6 +193,30 @@ cleanup:
 }
 
 /**
+ * Evaluate if DN has/does not have expected format with one or two components
+ * and error out if a mismatch is detected.
+ *
+ * @param[in] prefix Prefix for error messages, usually a function name.
+ * @param[in] dn
+ * @param[in] dniszone   Boolean returned by dn_to_dnsname for given DN.
+ * @param[in] wantszone  ISC_TRUE if DN should be a zone, ISC_FALSE otherwise.
+ * @retval ISC_R_SUCCESS or ISC_R_UNEXPECTED if values do not match.
+ */
+isc_result_t
+dn_want_zone(const char * const prefix, const char * const dn,
+	 isc_boolean_t dniszone, isc_boolean_t wantszone) {
+	if (dniszone != wantszone) {
+		log_error(%s: object '%s' should%s be a zone but DN format 
+			  suggests that it is%s a zone,
+			  prefix, dn, wantszone ?  :  not,
+			  dniszone ?  :  not);
+		return ISC_R_UNEXPECTED;
+	}
+
+	return ISC_R_SUCCESS;
+}
+
+/**
  * WARNING! This function is used to mangle input from network
  *  and it is security sensitive.
  *
diff --git a/src/ldap_convert.h b/src/ldap_convert.h
index f0b09262dbbe588c5c12b074242a9f7db4361880..21107307fc614c7af43b02ff50f9c60bacd224dd 100644
--- a/src/ldap_convert.h
+++ b/src/ldap_convert.h
@@ -42,6 +42,10 @@ isc_result_t dn_to_dnsname(isc_mem_t *mctx, const char *dn,
 			   isc_boolean_t *iszone)
 			   ATTR_NONNULL(1, 2, 3) ATTR_CHECKRESULT;
 
+isc_result_t
+dn_want_zone(const char * const func, const char * const dn,
+		isc_boolean_t dniszone, isc_boolean_t wantszone);
+
 isc_result_t dnsname_to_dn(zone_register_t *zr, dns_name_t *name, dns_name_t *zone,
 			   ld_string_t *target) ATTR_NONNULLS ATTR_CHECKRESULT;
 
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 9427dfbee800bacf3960c623ede2a10a7bc988cb..26630be2183cfb0257e5adfd06952a6dc1ab1eee 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1407,9 +1407,9 @@ ldap_delete_zone(ldap_instance_t *inst, const char *dn, isc_boolean_t lock,
 	isc_boolean_t iszone;
 	dns_name_t name;
 	dns_name_init(name, NULL);
-	
+
 	CHECK(dn_to_dnsname(inst-mctx, dn, name, NULL, iszone));
-	INSIST(iszone == ISC_TRUE);
+	CHECK(dn_want_zone(__func__, dn, iszone, ISC_TRUE));
 
 	result = ldap_delete_zone2(inst, name, lock, preserve_forwarding);
 
@@ -1794,7 +1794,7 @@ ldap_parse_fwd_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
 	/* Derive the DNS name of the zone from the DN. */
 	dn = entry-dn;
 	CHECK(dn_to_dnsname(inst-mctx, dn, name, NULL, iszone));
-	INSIST(iszone == ISC_TRUE);
+	CHECK(dn_want_zone(__func__, dn, iszone, ISC_TRUE));
 
 	CHECK(ldap_entry_getvalues(entry, idnsZoneActive, values));
 	if (HEAD(values) != NULL 
@@ -2445,7 +2445,7 @@ ldap_parse_master_zoneentry(ldap_entry_t * const entry, dns_db_t * const olddb,
 	/* Derive the dns name of the zone from the DN. */
 	dn = entry-dn;
 	CHECK(dn_to_dnsname(inst-mctx, dn, name, NULL, iszone));
-	INSIST(iszone == ISC_TRUE);
+	CHECK(dn_want_zone(__func__, dn, iszone, ISC_TRUE));
 
 	run_exclusive_enter(inst, lock_state);
 
@@ -4422,7 +4422,7 @@ update_zone(isc_task_t *task, isc_event_t *event)
 	CHECK(manager_get_ldap_instance(pevent-dbname, inst));
 	INSIST(task == inst-task); /* For task-exclusive mode */
 	CHECK(dn_to_dnsname(inst-mctx, pevent-dn, currname, NULL, iszone));
-	INSIST(iszone == ISC_TRUE);
+	CHECK(dn_want_zone(__func__, pevent-dn, iszone, ISC_TRUE));
 
 	if (SYNCREPL_DEL(pevent-chgtype)) {
 		CHECK(ldap_delete_zone(inst, pevent-dn, ISC_TRUE, ISC_FALSE));
@@ -4579,7 +4579,7 @@ update_record(isc_task_t *task, isc_event_t *event)
 
 	CHECK(manager_get_ldap_instance(pevent-dbname, inst));
 	CHECK(dn_to_dnsname(mctx, pevent-dn, name, origin, iszone));
-	INSIST(iszone == ISC_FALSE);
+	CHECK(dn_want_zone(__func__, pevent-dn, iszone, ISC_FALSE));
 	CHECK(zr_get_zone_ptr(inst-zone_register, origin, raw, secure));
 	zone_found = ISC_TRUE;
 
-- 
2.1.0

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

Re: [Freeipa-devel] FreeIPA integration with external DNS services

2014-12-11 Thread Petr Spacek
On 10.12.2014 18:50, Simo Sorce wrote:
 On Wed, 10 Dec 2014 15:13:30 +0100
 Petr Spacek pspa...@redhat.com wrote:
 
 I think that external DNS could depend on Vault (assuming that
 external DNS support will be purely optional).
 
 TBH, I do not think this is a sensible option, the Vault will drag huge
 dependencies for now, and I would like to avoid that if all we need is
 to add a couple of A/SRV records to an external DNS.
 
 If we can't come up with a service, I think I am ok telling admins they
 need to manually copy the TKEY (or use puppet or other similar
 configuration manager to push the key file around) on each replica, and
 we defer automatic distribution of TKEYs.
 
 We will have a service that can give out keys, it is identified as
 necessary in the replica promotion proposal, so we'll eventually get
 there.

Thank you for discussion. Now I would like to know in which direction are we
heading with external DNS support :-)

I have to admit that I don't understand why we are spending time on Vault and
at the same time we refuse to use it ...

Anyway, someone competent has to decide if we want to implement external DNS
support and:
- defer key distribution for now
- use Vault
- re-invent Vault and use that new cool thing

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0170] Detect and warn about invalid forwardzone configuration

2014-12-11 Thread Petr Spacek
Hello,

I have only few nitpicks and one minor non-nitpick. Rest is in-line.

On 10.12.2014 18:20, Martin Basti wrote:
 freeipa-mbasti-0170.4-Detect-and-warn-about-invalid-DNS-forward-zone-confi.patch
 
 
 From a1b70e7a12ffdb08941d43587a05d7e36b57ab2b Mon Sep 17 00:00:00 2001
 From: Martin Basti mba...@redhat.com
 Date: Fri, 21 Nov 2014 16:54:09 +0100
 Subject: [PATCH] Detect and warn about invalid DNS forward zone configuration
 
 Shows warning if forward and parent authoritative zone do not have
 proper NS record delegation, which can cause the forward zone will be
 ineffective and forwarding will not work.
 
 Ticket: https://fedorahosted.org/freeipa/ticket/4721
 ---
  ipalib/messages.py|  13 ++
  ipalib/plugins/dns.py | 332 
 --
  2 files changed, 334 insertions(+), 11 deletions(-)
 
 diff --git a/ipalib/messages.py b/ipalib/messages.py
 index 
 102e35275dbe37328c84ecb3cd5b2a8d8578056f..b44beca729f5483a7241e4c98a9f724ed663e70f
  100644
 --- a/ipalib/messages.py
 +++ b/ipalib/messages.py
 @@ -200,6 +200,19 @@ class 
 DNSServerDoesNotSupportDNSSECWarning(PublicMessage):
 uIf DNSSEC validation is enabled on IPA server(s), 
 uplease disable it.)
  
 +class ForwardzoneIsNotEffectiveWarning(PublicMessage):
 +
 +**13008** Forwardzone is not effective, forwarding will not work because
 +there is authoritative parent zone, without proper NS delegation
 +
 +
 +errno = 13008
 +type = warning
 +format = _(uforward zone \%(fwzone)s\ is not effective because of 
 +   umissing proper NS delegation in authoritative zone 
 +   u\%(authzone)s\. Please add NS record 
 +   u\%(ns_rec)s\ to parent zone \%(authzone)s\.)
 +
  
  def iter_messages(variables, base):
  Return a tuple with all subclasses
 diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
 index 
 c5d96a8c4fcdf101254ecefb60cb83d63bee6310..5c3a017989b23a1c6076d9dc4d93be65dd66cc67
  100644
 --- a/ipalib/plugins/dns.py
 +++ b/ipalib/plugins/dns.py
 @@ -1725,6 +1725,241 @@ def _normalize_zone(zone):
  return zone
  
  
 +def _get_auth_zone_ldap(name):
 +
 +Find authoritative zone in LDAP for name
Nitpick: Please add this note:
. Only active zones are considered.

 +:param name:
 +:return: (zone, truncated)
 +zone: authoritative zone, or None if authoritative zone is not in LDAP
 +
 +assert isinstance(name, DNSName)
 +ldap = api.Backend.ldap2
 +
 +# Create all possible parent zone names
 +search_name = name.make_absolute()
 +zone_names = []
 +for i in xrange(len(search_name)):
 +zone_name_abs = DNSName(search_name[i:]).ToASCII()
 +zone_names.append(zone_name_abs)
 +# compatibility with IPA  4.0, zone name can be relative
 +zone_names.append(zone_name_abs[:-1])
 +
 +# Create filters
 +objectclass_filter = ldap.make_filter({'objectclass':'idnszone'})
 +zonenames_filter = ldap.make_filter({'idnsname': zone_names})
 +zoneactive_filter = ldap.make_filter({'idnsZoneActive': 'true'})
 +complete_filter = ldap.combine_filters(
 +[objectclass_filter, zonenames_filter, zoneactive_filter],
 +rules=ldap.MATCH_ALL
 +)
 +
 +try:
 +entries, truncated = ldap.find_entries(
 +filter=complete_filter,
 +attrs_list=['idnsname'],
 +base_dn=DN(api.env.container_dns, api.env.basedn),
 +scope=ldap.SCOPE_ONELEVEL
 +)
 +except errors.NotFound:
 +return None, False
 +
 +# always use absolute zones
 +matched_auth_zones = [entry.single_value['idnsname'].make_absolute()
 +  for entry in entries]
 +
 +# return longest match
 +return max(matched_auth_zones, key=len), truncated
 +
 +
 +def _get_longest_match_ns_delegation_ldap(zone, name):
 +
 +Finds record in LDAP which has the longest match with name.
 +
 +NOTE: does not search in zone apex, returns None if there is no NS
 +delegation outside of zone apex
Nitpick:
Searches for deepest delegation for name in LDAP zone.

NOTE: NS record in zone apex is not considered as delegation. It returns None
if there is no delegation outside of zone apex.

 +
 +Example:
 +zone: example.com.
 +name: ns.sub.example.com.
 +
 +records:
 +extra.ns.sub.example.com.
 +sub.example.com.
 +example.com
 +
 +result: sub.example.com.
 +
 +:param zone: zone name
 +:param name:
 +:return: (record, truncated);
 +record: record name if success, or None if no such record exists, or
 +record is zone apex record
Nitpick:
:return: (match, truncated);
match: delegation name if success, or None if no delegation record exists

 +
 +assert isinstance(zone, DNSName)
 +assert isinstance(name, DNSName)
 +
 +ldap = api.Backend.ldap2
 +
 +# get zone DN
 +zone_dn = 

Re: [Freeipa-devel] topology management question

2014-12-11 Thread Petr Spacek
On 11.12.2014 15:20, Simo Sorce wrote:
 On Thu, 11 Dec 2014 14:18:36 +0100
 Ludwig Krispenz lkris...@redhat.com wrote:
 

 On 12/05/2014 04:50 PM, Simo Sorce wrote:
 On Thu, 04 Dec 2014 14:33:09 +0100
 Ludwig Krispenz lkris...@redhat.com wrote:

 hi,

 I just have another (hopefully this will end soon) issue I want to
 get your input. (please read to teh end first)

 To recapture the conditions:
 -  the topology plugin manages the connections between servers as
 segments in the shared tree
 - it is authoritative for managed servers, eg it controls all
 connections between servers listed under cn=masters,
 it is permissive for connection to other servers
 - it rejects any removal of a segment, which would disconnect the
 topology.
 - a change in topology can be applied to any server in the
 topology, it will reach the respective servers and the plugin will
 act upon it

 Now there is a special case, causing a bit of trouble. If a replica
 is to be removed from the topology, this means that
 the replication agreements from and to this replica should be
 removed, the server should be removed from the manages servers.
 The problem is that:
 - if you remove the server first, the server becomes unmanaged and
 removal of the segment will not trigger a removal of the
 replication agreement
 Can you explain what you mean if you remove the server first
 exactly ? What LDAP operation will be performed, by the management
 tools ?
 as far as the plugin is concerned a removal of a replica triggers two 
 operations:
 - removal of the host from the sservers in cn=masters, so the server
 is no longer considered as managed
 - removal of the segment(s) connecting the to be removed replica to 
 other still amnaged servers, which should remove the corresponding 
 replication agreements.
 It was the order of these two operations I was talking
 
 We can define a correct order, the plugin can refuse to do any other
 order for direct operations (we need to be careful not to refuse
 replication operations I think).
 

 - if you remove the segments first, one segment will be the last
 one connecting this replica to the topology and removal will be
 rejected
 We should never remove the segments first indeed.
 if we can fully control that only specific management tools can be
 used, we can define the order, but an admin could apply individual
 operations and still it would be good if nothing breaks
 
 I think we had a plan to return UNWILLING_TO_PERFORM if the admin tries
 to remove the last segment first. So we would have no problem really,
 the admin can try and fail. If he wants to remove a master he'll have
 to remove it from the masters group, and this will trigger the removal
 of all segments.
 
 Now, with some effort this can be resolved, eg
 if the server is removed, keep it internally as removed server and
 for segments connecting this server trigger removal of replication
 agreements or mark a the last segment, when tried to remove, as
 pending and once the server is removed also remove the
 corresponding repl agreements
 Why should we keep it internally ?
 If you mark the agreements as managed by setting an attribute on
 them, then you will never have any issue recognizing a managed
 agreement in cn=config, and you will also immediately find out it
 is old as it is not backed by a segment so you will safely remove
 it.
 I didn't want to add new flags/fields to the replication agreements
 as long as anything can be handled by the data in the shared tree.
 
 We have too. I think it is a must or we will find numerous corner cases.
 Is there a specific reason why you do not want to add flags to
 replication agreements in cn=config ?
 
 internally was probably misleading, but I will think about it again
 
 Ok, it is important we both understand what issues we see with any of
 the possible approaches so we can agree on the best one.
 
 Segments (and their agreements) should be removed as trigger on the
 master entry getting removed. This should be done even if it causes
 a split brain, because if the server is removed, no matter how much
 we wish to keep tropology integrity we effectively are in a split
 brain situation, keeping toplogy agreements alive w/o the server
 entry doesn't help.
 If we can agree on that, that presence/removal of masters is the
 primary trigger that's fine.
 
 Yes I think we can definitely agree that this is the primary trigger
 for server removal/addition.
 
 I was thinking of situations where a server was removed, 
 but not uninstalled.
 
 Understood, but even then it makes no real difference, once the server
 is removed from the group of masters it will not be able to replicate
 outbound anymore as the other master's ACIs will not recognize this
 server credentials as valid replicator creds.
 
 Just taking it out of the topology, but it could still be reached
 
 It can be reached, and that may be a problem for clients. But in the
 long term this should be true only for clients manually configured 

Re: [Freeipa-devel] [PATCH 0170] Detect and warn about invalid forwardzone configuration

2014-12-11 Thread Petr Spacek
On 11.12.2014 16:50, Martin Basti wrote:
 Updated aptch attached:

Nice work, ACK!

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] FreeIPA integration with external DNS services

2014-12-10 Thread Petr Spacek
On 9.12.2014 13:40, Martin Kosek wrote:
 On 12/03/2014 05:04 PM, Petr Spacek wrote:
 On 2.12.2014 17:21, Simo Sorce wrote:
 On Tue, 02 Dec 2014 15:56:28 +0100
 Petr Spacek pspa...@redhat.com wrote:

 On 1.12.2014 17:12, Simo Sorce wrote:
 On Mon, 01 Dec 2014 16:17:54 +0100
 Petr Spacek pspa...@redhat.com wrote:

 On 14.11.2014 17:31, Petr Spacek wrote:
 On 14.11.2014 02:22, Simo Sorce wrote:
 On Tue, 11 Nov 2014 16:29:51 +0100
 Petr Spacek pspa...@redhat.com wrote:

 Hello,

 this thread is about RFE
 IPA servers when installed should register themselves in the
 external DNS https://fedorahosted.org/freeipa/ticket/4424

 It is not a complete design, just a raw idea.


 Use case
 
 FreeIPA installation to a network with existing DNS
 infrastructure + network administrator who is not willing to
 add/maintain new DNS servers just for FreeIPA.


 High-level idea
 ===
 - Transform dns* commands from FreeIPA framework to equivalent
 nsupdate commands and send DNS updates to existing DNS
 servers.
 - Provide necessary encryption/signing keys to nsupdate.


 1) Integration to FreeIPA framework
 ===
 First of all, we need to decide if external DNS integration
 can be used at the same time with FreeIPA-integrated DNS or not.
 Side-question is what to do if a first server is installed with
 external-DNS but another replica is being installed with
 integrated-DNS and so on.

 I think being able to integrate with an external DNS is
 important, and not just at the server level, being able to
 distribute TSIG keys to client would be nice too, though a lot
 less important, than getting server integration..

 Using TSIG on many clients is very problematic. Keep in mind that
 TSIG is basically HMAC computed using symmetric key so:
 a) Every client would have to use the same key - this is a
 security nightmare. b) We would have to distribute and maintain
 many keys for many^2 clients, which is an administrative
 nightmare.

 For *clients* I would rather stay with GSS-TSIG which is much more
 manageable because we can use Kerberos trust and Keytab
 distribution is already solved by ipa-client-install.

 Alternative for clients would be to use FreeIPA server as proxy
 which encapsulates TSIG keys and issues update requests on behalf
 of clients. This would be FreeIPA-specific but any
 TSIG-distribution mechanism will be FreeIPA-specific anyway.

 TSIG standard explicitly says that there is no standardized
 distribution mechanism.

 In other words, the question is if current dns.py plugin
 shipped with FreeIPA framework should be:

 a) Extended dns.py with dnsexternal-* commands
 --
 Disadvantages:
 - It complicate FreeIPA DNS interface which is a complex beast
 even now.
 - We would have add condition to every DNS API call in
 installers which would increase horribleness of the installer
 code even more (or add another layer of abstraction...).

 I agree having a special command is undesirable.

 - I don't see a point in using integrated-DNS with external-DNS
 at the same time. To use integrated-DNS you have to get a
 proper DNS delegation from parent domain - and if you can get
 the delegation then there is no point in using external DNS ...

 I disagree on this point, it makes a lot of sense to have some
 zones maintained by IPA and ... some not.

 Advantages:
 - You can use external  integrated DNS at the same time.

 We can achieve the same w/o a new ipa level command.

 This needs to be decided by FreeIPA framework experts ...

 Petr^3 came up with clever 'proxy' idea for IPA-commands. This
 proxy would provide all ipa dns* commands and dispatch user-issued
 commands to appropriate FreeIPA-plugin (internal-dns or
 external-dns). This decision could depend on some values in LDAP.

 b) Replace dns.py with another implementation of current
 dnszone-*  dnsrecord-* API.
 -
 This seems like a cleaner approach to me. It could be shipped as
 ipa-server-dns-external package (opposed to standard
 ipa-server-dns package).

 Advantages:
 - It could seamlessly work with FreeIPA client installer because
 the dns*-nsupdate command transformation would be done on
 FreeIPA server and client doesn't need to know about it.
 - Does not require re-training/not much new documentation
 because commands are the same.

 Disadvantages:
 - You can't use integrated  external DNS at the same time (but
 I don't think it justifies the added complexity).

 I disagree.

 One of the reason to use a mix is to allow smooth migrations
 where zones are moved into (or out of) IPA one by one.

 Good point, I agree. I will focus on supporting both (internal and
 external) DNS at the same time.

 Petr^3 or anyone else, what do you propose?

 I think what I'd like to see is to be able to configure a DNS
 zone in LDAP and mark it external.
 The zone would held the TSIG keys necessary to perform DNS

Re: [Freeipa-devel] FreeIPA integration with external DNS services

2014-12-10 Thread Petr Spacek
On 10.12.2014 15:50, Martin Kosek wrote:
 On 12/10/2014 03:13 PM, Petr Spacek wrote:
 On 9.12.2014 13:40, Martin Kosek wrote:
 On 12/03/2014 05:04 PM, Petr Spacek wrote:
 On 2.12.2014 17:21, Simo Sorce wrote:
 On Tue, 02 Dec 2014 15:56:28 +0100
 Petr Spacek pspa...@redhat.com wrote:

 On 1.12.2014 17:12, Simo Sorce wrote:
 On Mon, 01 Dec 2014 16:17:54 +0100
 Petr Spacek pspa...@redhat.com wrote:

 On 14.11.2014 17:31, Petr Spacek wrote:
 On 14.11.2014 02:22, Simo Sorce wrote:
 On Tue, 11 Nov 2014 16:29:51 +0100
 Petr Spacek pspa...@redhat.com wrote:

 Hello,

 this thread is about RFE
 IPA servers when installed should register themselves in the
 external DNS https://fedorahosted.org/freeipa/ticket/4424

 It is not a complete design, just a raw idea.


 Use case
 
 FreeIPA installation to a network with existing DNS
 infrastructure + network administrator who is not willing to
 add/maintain new DNS servers just for FreeIPA.


 High-level idea
 ===
 - Transform dns* commands from FreeIPA framework to equivalent
 nsupdate commands and send DNS updates to existing DNS
 servers.
 - Provide necessary encryption/signing keys to nsupdate.


 1) Integration to FreeIPA framework
 ===
 First of all, we need to decide if external DNS integration
 can be used at the same time with FreeIPA-integrated DNS or not.
 Side-question is what to do if a first server is installed with
 external-DNS but another replica is being installed with
 integrated-DNS and so on.

 I think being able to integrate with an external DNS is
 important, and not just at the server level, being able to
 distribute TSIG keys to client would be nice too, though a lot
 less important, than getting server integration..

 Using TSIG on many clients is very problematic. Keep in mind that
 TSIG is basically HMAC computed using symmetric key so:
 a) Every client would have to use the same key - this is a
 security nightmare. b) We would have to distribute and maintain
 many keys for many^2 clients, which is an administrative
 nightmare.

 For *clients* I would rather stay with GSS-TSIG which is much more
 manageable because we can use Kerberos trust and Keytab
 distribution is already solved by ipa-client-install.

 Alternative for clients would be to use FreeIPA server as proxy
 which encapsulates TSIG keys and issues update requests on behalf
 of clients. This would be FreeIPA-specific but any
 TSIG-distribution mechanism will be FreeIPA-specific anyway.

 TSIG standard explicitly says that there is no standardized
 distribution mechanism.

 In other words, the question is if current dns.py plugin
 shipped with FreeIPA framework should be:

 a) Extended dns.py with dnsexternal-* commands
 --
 Disadvantages:
 - It complicate FreeIPA DNS interface which is a complex beast
 even now.
 - We would have add condition to every DNS API call in
 installers which would increase horribleness of the installer
 code even more (or add another layer of abstraction...).

 I agree having a special command is undesirable.

 - I don't see a point in using integrated-DNS with external-DNS
 at the same time. To use integrated-DNS you have to get a
 proper DNS delegation from parent domain - and if you can get
 the delegation then there is no point in using external DNS ...

 I disagree on this point, it makes a lot of sense to have some
 zones maintained by IPA and ... some not.

 Advantages:
 - You can use external  integrated DNS at the same time.

 We can achieve the same w/o a new ipa level command.

 This needs to be decided by FreeIPA framework experts ...

 Petr^3 came up with clever 'proxy' idea for IPA-commands. This
 proxy would provide all ipa dns* commands and dispatch user-issued
 commands to appropriate FreeIPA-plugin (internal-dns or
 external-dns). This decision could depend on some values in LDAP.

 b) Replace dns.py with another implementation of current
 dnszone-*  dnsrecord-* API.
 -
 This seems like a cleaner approach to me. It could be shipped as
 ipa-server-dns-external package (opposed to standard
 ipa-server-dns package).

 Advantages:
 - It could seamlessly work with FreeIPA client installer because
 the dns*-nsupdate command transformation would be done on
 FreeIPA server and client doesn't need to know about it.
 - Does not require re-training/not much new documentation
 because commands are the same.

 Disadvantages:
 - You can't use integrated  external DNS at the same time (but
 I don't think it justifies the added complexity).

 I disagree.

 One of the reason to use a mix is to allow smooth migrations
 where zones are moved into (or out of) IPA one by one.

 Good point, I agree. I will focus on supporting both (internal and
 external) DNS at the same time.

 Petr^3 or anyone else, what do you propose?

 I think what I'd like to see is to be able to configure a DNS
 zone in LDAP

[Freeipa-devel] [PATCH 0315] Support BIND 9.10

2014-12-04 Thread Petr Spacek
Hello,

Support BIND 9.10.

https://fedorahosted.org/bind-dyndb-ldap/ticket/139


This patch definitely needs more testing but ...:
- It compiles with BIND 9.9 and BIND 9.10.
- It seems that it is able to load master and forward zones.
- Basic dynamic updates work.

I did not test other features yet.

-- 
Petr^2 Spacek
From 7101194d6bcf99c8cc5c8fec405ee716cd6e7b07 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 4 Dec 2014 14:52:18 +0100
Subject: [PATCH] Support BIND 9.10.

https://fedorahosted.org/bind-dyndb-ldap/ticket/139
---
 src/acl.c |  29 ++---
 src/acl.h |   7 ++-
 src/ldap_driver.c |  88 ++---
 src/ldap_helper.c | 128 +++---
 4 files changed, 204 insertions(+), 48 deletions(-)

diff --git a/src/acl.c b/src/acl.c
index 449a5ad85186bc4e2309add77ff551897afac92b..233a3208dcb361b4b7c0cc8bd46a472d9b61939d 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -37,6 +37,8 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include config.h
+
 #include isccfg/aclconf.h
 #include isccfg/cfg.h
 #include isccfg/namedconf.h
@@ -47,9 +49,11 @@
 #include isc/mem.h
 #include isc/once.h
 #include isc/result.h
+#include isc/types.h
 #include isc/util.h
 
 #include dns/fixedname.h
+#include dns/forward.h
 #include dns/log.h
 #include dns/rdatatype.h
 #include dns/ssu.h
@@ -604,21 +608,27 @@ cleanup:
  *
  */
 isc_result_t
-acl_parse_forwarder(const char *forwarder_str, isc_mem_t *mctx, isc_sockaddr_t **sa)
+acl_parse_forwarder(const char *forwarder_str, isc_mem_t *mctx,
+#if LIBDNS_VERSION_MAJOR  140
+		isc_sockaddr_t **fw)
+#else /* LIBDNS_VERSION_MAJOR = 140 */
+		dns_forwarder_t **fw)
+#endif
 {
 	isc_result_t result = ISC_R_SUCCESS;
 	cfg_parser_t *parser = NULL;
 
 	cfg_obj_t *forwarders_cfg = NULL;
 	ld_string_t *new_forwarder_str = NULL;
 	const cfg_obj_t *faddresses;
 	const cfg_listelt_t *element;
 	const cfg_obj_t *forwarder;
+	isc_sockaddr_t addr;
 
 	in_port_t port = 53;
 
 	REQUIRE(forwarder_str != NULL);
-	REQUIRE(sa != NULL  *sa == NULL);
+	REQUIRE(fw != NULL  *fw == NULL);
 
 	/* add semicolon and brackets as necessary for parser */
 	if (!index(forwarder_str, ';'))
@@ -637,10 +647,17 @@ acl_parse_forwarder(const char *forwarder_str, isc_mem_t *mctx, isc_sockaddr_t *
 	}
 
 	forwarder = cfg_listelt_value(element);
-	CHECKED_MEM_GET_PTR(mctx, *sa);
-	**sa = *cfg_obj_assockaddr(forwarder);
-	if (isc_sockaddr_getport(*sa) == 0)
-		isc_sockaddr_setport(*sa, port);
+	CHECKED_MEM_GET_PTR(mctx, *fw);
+	addr = *cfg_obj_assockaddr(forwarder);
+	if (isc_sockaddr_getport(addr) == 0)
+		isc_sockaddr_setport(addr, port);
+#if LIBDNS_VERSION_MAJOR  140
+	**fw = addr;
+#else /* LIBDNS_VERSION_MAJOR = 140 */
+	(*fw)-addr = addr;
+	(*fw)-dscp = cfg_obj_getdscp(forwarder);
+#endif
+
 
 cleanup:
 	if (forwarders_cfg != NULL)
diff --git a/src/acl.h b/src/acl.h
index 9ca7644d792d6da6060bcb48e10e9c579d890503..e438132f623b4efb2c3e2d595b1d83065f8583dc 100644
--- a/src/acl.h
+++ b/src/acl.h
@@ -47,6 +47,11 @@ acl_from_ldap(isc_mem_t *mctx, const char *aclstr, acl_type_t type,
 
 isc_result_t
 acl_parse_forwarder(const char *forwarders_str, isc_mem_t *mctx,
-		isc_sockaddr_t **sa) ATTR_NONNULLS ATTR_CHECKRESULT;
+#if LIBDNS_VERSION_MAJOR  140
+		isc_sockaddr_t **fw)
+#else /* LIBDNS_VERSION_MAJOR = 140 */
+		dns_forwarder_t **fw)
+#endif
+ATTR_NONNULLS ATTR_CHECKRESULT;
 
 #endif /* !_LD_ACL_H_ */
diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index 6161c96938b429923aee983a37312f64260226de..8b78c960cfb05cc0f4c0fb50e3fbdaa9cfdcae50 100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -198,12 +198,18 @@ detach(dns_db_t **dbp)
 
 /* !!! This could be required for optimizations (like on-disk cache). */
 static isc_result_t
+#if LIBDNS_VERSION_MAJOR  140
 beginload(dns_db_t *db, dns_addrdatasetfunc_t *addp, dns_dbload_t **dbloadp)
 {
 
 	UNUSED(db);
 	UNUSED(addp);
 	UNUSED(dbloadp);
+#else /* LIBDNS_VERSION_MAJOR = 140 */
+beginload(dns_db_t *db, dns_rdatacallbacks_t *callbacks) {
+	UNUSED(db);
+	UNUSED(callbacks);
+#endif /* LIBDNS_VERSION_MAJOR = 140 */
 
 	fatal_error(ldapdb: method beginload() should never be called);
 
@@ -218,18 +224,35 @@ beginload(dns_db_t *db, dns_addrdatasetfunc_t *addp, dns_dbload_t **dbloadp)
 
 /* !!! This could be required for optimizations (like on-disk cache). */
 static isc_result_t
+#if LIBDNS_VERSION_MAJOR  140
 endload(dns_db_t *db, dns_dbload_t **dbloadp)
 {
 
 	UNUSED(db);
 	UNUSED(dbloadp);
+#else /* LIBDNS_VERSION_MAJOR = 140 */
+endload(dns_db_t *db, dns_rdatacallbacks_t *callbacks) {
+	UNUSED(db);
+	UNUSED(callbacks);
+#endif /* LIBDNS_VERSION_MAJOR = 140 */
 
 	fatal_error(ldapdb: method endload() should never be called);
 
 	/* Not reached */
 	return ISC_R_SUCCESS;
 }
 
+#if LIBDNS_VERSION_MAJOR = 140
+static isc_result_t
+serialize(dns_db_t *db, dns_dbversion_t *version, FILE *file)
+{
+	ldapdb_t *ldapdb = (ldapdb_t *) db;
+
+	REQUIRE(VALID_LDAPDB(ldapdb

[Freeipa-devel] disaster recovery if replica was compromised

2014-12-03 Thread Petr Spacek
Hello,

I wonder what we can recommend as disaster recovery procedure for cases where
a replica (its LDAP database) was compromised.

Saying you are screwed doesn't sound like the right answer :-D

It is clear that all passwords and keys have to be changed and complete
replica re-installation is inevitable.

I would expect something like:
- install fresh FreeIPA server and do not connect it to the compromised topology
- run migrate-ds to get users, groups etc. (review is necessary)
- use this to force all users to change passwords
- use this to re-generate all certificates ...

This sounds like yet another case for FreeIPA-FreeIPA migration tool which
could import SUDO rules and all other FreeIPA-specific stuff.

Any ideas?

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] FreeIPA integration with external DNS services

2014-12-03 Thread Petr Spacek
On 2.12.2014 17:21, Simo Sorce wrote:
 On Tue, 02 Dec 2014 15:56:28 +0100
 Petr Spacek pspa...@redhat.com wrote:
 
 On 1.12.2014 17:12, Simo Sorce wrote:
 On Mon, 01 Dec 2014 16:17:54 +0100
 Petr Spacek pspa...@redhat.com wrote:

 On 14.11.2014 17:31, Petr Spacek wrote:
 On 14.11.2014 02:22, Simo Sorce wrote:
 On Tue, 11 Nov 2014 16:29:51 +0100
 Petr Spacek pspa...@redhat.com wrote:

 Hello,

 this thread is about RFE
 IPA servers when installed should register themselves in the
 external DNS https://fedorahosted.org/freeipa/ticket/4424

 It is not a complete design, just a raw idea.


 Use case
 
 FreeIPA installation to a network with existing DNS
 infrastructure + network administrator who is not willing to
 add/maintain new DNS servers just for FreeIPA.


 High-level idea
 ===
 - Transform dns* commands from FreeIPA framework to equivalent
 nsupdate commands and send DNS updates to existing DNS
 servers.
 - Provide necessary encryption/signing keys to nsupdate.


 1) Integration to FreeIPA framework
 ===
 First of all, we need to decide if external DNS integration
 can be used at the same time with FreeIPA-integrated DNS or not.
 Side-question is what to do if a first server is installed with
 external-DNS but another replica is being installed with
 integrated-DNS and so on.

 I think being able to integrate with an external DNS is
 important, and not just at the server level, being able to
 distribute TSIG keys to client would be nice too, though a lot
 less important, than getting server integration..

 Using TSIG on many clients is very problematic. Keep in mind that
 TSIG is basically HMAC computed using symmetric key so:
 a) Every client would have to use the same key - this is a
 security nightmare. b) We would have to distribute and maintain
 many keys for many^2 clients, which is an administrative
 nightmare.

 For *clients* I would rather stay with GSS-TSIG which is much more
 manageable because we can use Kerberos trust and Keytab
 distribution is already solved by ipa-client-install.

 Alternative for clients would be to use FreeIPA server as proxy
 which encapsulates TSIG keys and issues update requests on behalf
 of clients. This would be FreeIPA-specific but any
 TSIG-distribution mechanism will be FreeIPA-specific anyway.

 TSIG standard explicitly says that there is no standardized
 distribution mechanism.

 In other words, the question is if current dns.py plugin
 shipped with FreeIPA framework should be:

 a) Extended dns.py with dnsexternal-* commands
 --
 Disadvantages:
 - It complicate FreeIPA DNS interface which is a complex beast
 even now.
 - We would have add condition to every DNS API call in
 installers which would increase horribleness of the installer
 code even more (or add another layer of abstraction...).

 I agree having a special command is undesirable.

 - I don't see a point in using integrated-DNS with external-DNS
 at the same time. To use integrated-DNS you have to get a
 proper DNS delegation from parent domain - and if you can get
 the delegation then there is no point in using external DNS ...

 I disagree on this point, it makes a lot of sense to have some
 zones maintained by IPA and ... some not.

 Advantages:
 - You can use external  integrated DNS at the same time.

 We can achieve the same w/o a new ipa level command.

 This needs to be decided by FreeIPA framework experts ...

 Petr^3 came up with clever 'proxy' idea for IPA-commands. This
 proxy would provide all ipa dns* commands and dispatch user-issued
 commands to appropriate FreeIPA-plugin (internal-dns or
 external-dns). This decision could depend on some values in LDAP.

 b) Replace dns.py with another implementation of current
 dnszone-*  dnsrecord-* API.
 -
 This seems like a cleaner approach to me. It could be shipped as
 ipa-server-dns-external package (opposed to standard
 ipa-server-dns package).

 Advantages:
 - It could seamlessly work with FreeIPA client installer because
 the dns*-nsupdate command transformation would be done on
 FreeIPA server and client doesn't need to know about it.
 - Does not require re-training/not much new documentation
 because commands are the same.

 Disadvantages:
 - You can't use integrated  external DNS at the same time (but
 I don't think it justifies the added complexity).

 I disagree.

 One of the reason to use a mix is to allow smooth migrations
 where zones are moved into (or out of) IPA one by one.

 Good point, I agree. I will focus on supporting both (internal and
 external) DNS at the same time.

 Petr^3 or anyone else, what do you propose?

 I think what I'd like to see is to be able to configure a DNS
 zone in LDAP and mark it external.
 The zone would held the TSIG keys necessary to perform DNS
 updates.

 When the regular ipa dnsrecord-add etc... commands are called

Re: [Freeipa-devel] [PATCH 0036] Add missing python files to Makefile

2014-12-03 Thread Petr Spacek
On 3.12.2014 14:38, Gabe Alford wrote:
 Yeah. This is what I was talking about ipaclient builds still relying on
 Makefile.am. Plus if you remove the ipaclient/Makefile.am and then run
 `make rpm`, it fails to find the *.py files to package into the rpm.
 
 Gabe
 
 On Wed, Dec 3, 2014 at 6:05 AM, Martin Kosek mko...@redhat.com wrote:
 
 On 12/03/2014 05:13 AM, Gabe Alford wrote:
 This patch removes the changelog and Makefile.am for ipaclient as well.

 Thanks,

 Gabe

 On Mon, Dec 1, 2014 at 8:28 AM, Martin Kosek mko...@redhat.com wrote:

 On 12/01/2014 04:25 PM, Rob Crittenden wrote:
 Gabe Alford wrote:

 On Mon, Dec 1, 2014 at 6:05 AM, Martin Kosek mko...@redhat.com
 mailto:mko...@redhat.com wrote:

 On 11/30/2014 03:28 AM, Gabe Alford wrote:
  Ignore the last patch. Updated patch attached.
 
  On Sat, Nov 29, 2014 at 6:03 PM, Gabe Alford
 redhatri...@gmail.com mailto:redhatri...@gmail.com wrote:
 
  This patch removes the app_PYTHON usage.
 
  Thanks,
 
  Gabe
 
  On Thu, Nov 27, 2014 at 9:40 AM, Martin Kosek 
 mko...@redhat.com
 mailto:mko...@redhat.com wrote:
 
  Exactly, this was the message from Martin :-) I did not test
 it
 myself,
  but
  removing all app_PYTHON should be benign given we use Python
 setup.py
  packaging.
 
  On 11/27/2014 04:27 PM, Gabe Alford wrote:
  Thanks guys. Sounds like it would be better to submit a patch
 that
  removes
  app_PYTHON if it is considered dead code.
 
  Gabe
 
  On Thursday, November 27, 2014, Petr Spacek 
 pspa...@redhat.com
 mailto:pspa...@redhat.com wrote:
 
  On 27.11.2014 11:00, Martin Basti wrote:
  On 27/11/14 00:50, Gabe Alford wrote:
  Hello,
 
 Wondering if I could get a review. Updated patch
 attached.
 
  Thanks,
  Gabe
 
  On Tue, Nov 11, 2014 at 7:21 AM, Gabe Alford
 redhatri...@gmail.com mailto:redhatri...@gmail.com
  javascript:;
  mailto:redhatri...@gmail.com mailto:
 redhatri...@gmail.com

 javascript:; wrote:
 
  Hello,
 
  Fix for https://fedorahosted.org/freeipa/ticket/4700
 
  Thanks,
 
  Gabe
 
 
 
  Hello,
 
  sorry for late response.
 
  We push this ticket to backlog, as it would be part of
 build
 system
  refactoring.
  The app_PYTHON statement is not used anymore in IPA, the
 better
  solution is
  remove it, instead of keeping dead code up-to-date.
 
  Just to clarify:
  It can be pushed if it works, there is no need to postpone
 accepting
  patch
  if
  the patch seems okay and doesn't break anything.
 
  Martin, please keep in mind that contributions are welcome
 at
 any time.
 
  Milestones in Trac reflect our view of priorities but it
 doesn't
  prevent us
  from accepting correct patches from contributions at any
 time, no
  matter
  which
  priority is stated in Trac (or even if there is no ticket
 for
 it ...).
 
  --
  Petr^2 Spacek

 Worked in my tests, I did not see any breakage. I guess we can
 also
 remove the
 ipa-client/ipaclient/Makefile.am while we are at it.

 Martin


 It looks like the ipaclient/Makefile.am is still being used. I tried
 removing it and there were errors in the build, but maybe I am wrong?

 It is needed to build ipa-join, ipa-getkeytab and ipa-rmkeytab.

 Feel free to rip out the outdated hg ChangeLog stuff though.

 rob

 I think Gabe was asking about ipa-client/ipaclient/Makefile.am and not
 about
 ipa-client/Makefile.am - we still need this one as Rob correctly said.

 The failure that Gabe hit in build probably comes from the the SUBDIR
 reference
 in ipa-client/Makefile.am file. I assume that if the reference is
 removed,
 the
 removal should work.

 And yes, you can remove the Changelog too if you are OK with it :)

 Martin



 I think you did some error during the process. This is what I got:

 $ git clean -fxd
 $ git apply ...
 $ make rpms
 ...
 checking that generated files are newer than configure... done
 configure: creating ./config.status
 config.status: error: cannot find input file: `Makefile.in'
 Makefile:84: recipe for target 'client-autogen' failed
 make: *** [client-autogen] Error 1

Lukas, could you please help us to find a way from the Autotools maze? :-)

Thank you!

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] Gaps in upstream tests

2014-12-03 Thread Petr Spacek
On 25.11.2014 10:43, Petr Spacek wrote:
 On 7.11.2014 14:41, Martin Kosek wrote:
 FreeIPA team will soon grow with a new member focusing on upstream QE tests. 
 I
 would like to collect ideas what are the biggest gaps in the current upstream
 test suite from your POV.

 Existing requests are tracked here:
 https://fedorahosted.org/freeipa/query?status=assignedstatus=newstatus=reopenedcomponent=Testscol=idcol=summarycol=componentcol=statuscol=ownercol=typecol=prioritycol=milestonegroup=milestoneorder=priority


 First idea that I head proposed are Upgrade tests. These are often done
 manually. I think that upgrade test from currently supported FreeIPA/Fedora
 version would go a long way (like 3.3.5 on F20 upgraded built RPMs and 
 running
 unit tests).

 Second, it would be nice to try testing FreeIPA server in a container. Not
 only it would verify our container efforts, but it may also allow easy
 multi-master tests on one Jenkins VM or local host instead of expensive VM
 orchestration.

 Any other areas worth focusing on (besides of course testing newly developed
 features)?
 
 At least simple automated MitM attack against TLS.
 
 First thing which comes to mind is CLI-server interaction and also
 certmonger-server interaction.
 
 TLS is hard to get right and if I recall it correctly we already had a problem
 with certificate validation...

Related link:
http://thehackernews.com/2014/11/nogotofail-Network-Security-Testing-Tool.html

The Nogotofail tool requires Python 2.7 and pyOpenSSL=0.13. It features an
on-path network Man-in-the-Middle (MiTM), designed to work on Linux machines,
as well and optional clients for the devices being tested.

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0173] Throw zonemgr error message before installation proceeds

2014-12-02 Thread Petr Spacek
On 1.12.2014 13:32, Jan Cholasta wrote:
 Actually, sratch that, exceptions thrown by python-dns do not have messages.

Oh yes, it is very annoying. I have asked upstream if potential patches about
this issues can be accepted:
https://github.com/rthalley/dnspython/issues/84

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] FreeIPA integration with external DNS services

2014-12-02 Thread Petr Spacek
On 1.12.2014 17:12, Simo Sorce wrote:
 On Mon, 01 Dec 2014 16:17:54 +0100
 Petr Spacek pspa...@redhat.com wrote:
 
 On 14.11.2014 17:31, Petr Spacek wrote:
 On 14.11.2014 02:22, Simo Sorce wrote:
 On Tue, 11 Nov 2014 16:29:51 +0100
 Petr Spacek pspa...@redhat.com wrote:

 Hello,

 this thread is about RFE
 IPA servers when installed should register themselves in the
 external DNS https://fedorahosted.org/freeipa/ticket/4424

 It is not a complete design, just a raw idea.


 Use case
 
 FreeIPA installation to a network with existing DNS
 infrastructure + network administrator who is not willing to
 add/maintain new DNS servers just for FreeIPA.


 High-level idea
 ===
 - Transform dns* commands from FreeIPA framework to equivalent
 nsupdate commands and send DNS updates to existing DNS servers.
 - Provide necessary encryption/signing keys to nsupdate.


 1) Integration to FreeIPA framework
 ===
 First of all, we need to decide if external DNS integration can
 be used at the same time with FreeIPA-integrated DNS or not.
 Side-question is what to do if a first server is installed with
 external-DNS but another replica is being installed with
 integrated-DNS and so on.

 I think being able to integrate with an external DNS is important,
 and not just at the server level, being able to distribute TSIG
 keys to client would be nice too, though a lot less important,
 than getting server integration..

 Using TSIG on many clients is very problematic. Keep in mind that
 TSIG is basically HMAC computed using symmetric key so:
 a) Every client would have to use the same key - this is a security
 nightmare. b) We would have to distribute and maintain many keys
 for many^2 clients, which is an administrative nightmare.

 For *clients* I would rather stay with GSS-TSIG which is much more
 manageable because we can use Kerberos trust and Keytab
 distribution is already solved by ipa-client-install.

 Alternative for clients would be to use FreeIPA server as proxy
 which encapsulates TSIG keys and issues update requests on behalf
 of clients. This would be FreeIPA-specific but any
 TSIG-distribution mechanism will be FreeIPA-specific anyway.

 TSIG standard explicitly says that there is no standardized
 distribution mechanism.

 In other words, the question is if current dns.py plugin shipped
 with FreeIPA framework should be:

 a) Extended dns.py with dnsexternal-* commands
 --
 Disadvantages:
 - It complicate FreeIPA DNS interface which is a complex beast
 even now.
 - We would have add condition to every DNS API call in installers
 which would increase horribleness of the installer code even more
 (or add another layer of abstraction...).

 I agree having a special command is undesirable.

 - I don't see a point in using integrated-DNS with external-DNS at
 the same time. To use integrated-DNS you have to get a proper DNS
 delegation from parent domain - and if you can get the delegation
 then there is no point in using external DNS ...

 I disagree on this point, it makes a lot of sense to have some
 zones maintained by IPA and ... some not.

 Advantages:
 - You can use external  integrated DNS at the same time.

 We can achieve the same w/o a new ipa level command.

 This needs to be decided by FreeIPA framework experts ...

 Petr^3 came up with clever 'proxy' idea for IPA-commands. This
 proxy would provide all ipa dns* commands and dispatch user-issued
 commands to appropriate FreeIPA-plugin (internal-dns or
 external-dns). This decision could depend on some values in LDAP.

 b) Replace dns.py with another implementation of current
 dnszone-*  dnsrecord-* API.
 -
 This seems like a cleaner approach to me. It could be shipped as
 ipa-server-dns-external package (opposed to standard
 ipa-server-dns package).

 Advantages:
 - It could seamlessly work with FreeIPA client installer because
 the dns*-nsupdate command transformation would be done on
 FreeIPA server and client doesn't need to know about it.
 - Does not require re-training/not much new documentation because
 commands are the same.

 Disadvantages:
 - You can't use integrated  external DNS at the same time (but I
 don't think it justifies the added complexity).

 I disagree.

 One of the reason to use a mix is to allow smooth migrations where
 zones are moved into (or out of) IPA one by one.

 Good point, I agree. I will focus on supporting both (internal and
 external) DNS at the same time.

 Petr^3 or anyone else, what do you propose?

 I think what I'd like to see is to be able to configure a DNS zone
 in LDAP and mark it external.
 The zone would held the TSIG keys necessary to perform DNS updates.

 When the regular ipa dnsrecord-add etc... commands are called, the
 framework detects that the zone is external, fewttches the TSIG
 key (if the user has access to it) and spawn an nsupdate

[Freeipa-devel] Announcing bind-dyndb-ldap version 6.1

2014-12-02 Thread Petr Spacek
The FreeIPA team is proud to announce bind-dyndb-ldap version 6.1.

It can be downloaded from https://fedorahosted.org/released/bind-dyndb-ldap/

The new version has also been built for Fedora 21+ and and is on its way to
updates-testing:
https://admin.fedoraproject.org/updates/bind-dyndb-ldap-6.1-1.fc21

This version is also available from FreeIPA COPR repo:
http://copr.fedoraproject.org/coprs/mkosek/freeipa/


6.1

[1] Crash caused by interaction between forward and master zones was fixed.
https://fedorahosted.org/bind-dyndb-ldap/ticket/145

[2] DNS NOTIFY messages are sent after any modification to the zone.
https://fedorahosted.org/bind-dyndb-ldap/ticket/144

[3] Misleading error message about forward zones during reconnect was fixed.


== Upgrading ==
A server can be upgraded by installing updated RPM. BIND has to be restarted
manually after the RPM installation.

Downgrading back to any 5.x version is supported if idnsZoneActive is always
set to TRUE.


== Feedback ==
Please provide comments, report bugs and send any other feedback via the
freeipa-users mailing list:
http://www.redhat.com/mailman/listinfo/freeipa-users

-- 
Petr^2 Spacek

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


[Freeipa-devel] Threat model abuse cases in design documents

2014-12-01 Thread Petr Spacek
Hello,

while wondering about design for 'external DNS integration' feature I have
realized that I did not see any explicit threat model for FreeIPA.

Do we have any? IMHO it would be handy to have it somewhere on wiki so it
could be used as 'checklist' while developing design documents for security
reviews.


Threat model

IMHO our assumed attacker should have these powers:
1) Do active man-in-the-middle attack on network:
- All network communication can be monitored and altered by attacked.
- This includes client-FreeIPA server communication,
- and also server-server communication.

2) Compromise normal user account:
I think that in in large networks the probability of successful attack against
at least one user account is almost 1.
So, we should assume that at least one user account was compromised. I.e. our
attacker knows user's password or has equivalent of keytab.

3) Compromise a client machine:
Again, I think that in in large networks the probability of successful attack
against at least one machine is almost 1.
So, we should assume that at least one machine account was compromised. I.e.
our attacker has equivalent of machine keytab and keytabs for services running
on it.

What did I miss? Maybe we should explicitly say how replica files and other
'secrets' (DM password ...) should be handled. It would help with discussion
about automated FreeIPA deployment too.


Also, we should explicitly say that FreeIPA server itself and its LDAP
database is the key to everything. If the FreeIPA server and its LDAP database
is compromised then the game is over - attacker has access to everything.


Abuse cases
===
IMHO security sensitive design documents (e.g. automated FreeIPA deployment,
sub-CAs, Vault, external DNS integration) should explicitly walk through the
thread model and state what is going to happen if FreeIPA infrastructure is
under attack we assume.

E.g. for external DNS integration with symmetric TSIG keys:

Proposal in design document:
- TSIG keys are distributed all to FreeIPA clients using LDAP  GSSAPI and
thus are accessible using any host/client.example.com credentials.

Design assessment with thread model in mind:
- MitM attack will not reveal anything because we trust GSSAPI.
- User account compromise will not reveal anything because uses doesn't have
access to TSIG keys.
- Single compromised client will reveal TSIG keys to attacker so
authentication to external DNS will be completely compromised. This will allow
attacker to modify any records in external DNS.

This could be have very serious consequences if DNSSEC is in place so clients
can fully trust the records and use them for e.g. TLS validation.
-- This could be a reason to re-think the design and remove this weak point.


What do you think?

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] FreeIPA integration with external DNS services

2014-12-01 Thread Petr Spacek
On 14.11.2014 17:31, Petr Spacek wrote:
 On 14.11.2014 02:22, Simo Sorce wrote:
 On Tue, 11 Nov 2014 16:29:51 +0100
 Petr Spacek pspa...@redhat.com wrote:

 Hello,

 this thread is about RFE
 IPA servers when installed should register themselves in the
 external DNS https://fedorahosted.org/freeipa/ticket/4424

 It is not a complete design, just a raw idea.


 Use case
 
 FreeIPA installation to a network with existing DNS infrastructure +
 network administrator who is not willing to add/maintain new DNS
 servers just for FreeIPA.


 High-level idea
 ===
 - Transform dns* commands from FreeIPA framework to equivalent
 nsupdate commands and send DNS updates to existing DNS servers.
 - Provide necessary encryption/signing keys to nsupdate.


 1) Integration to FreeIPA framework
 ===
 First of all, we need to decide if external DNS integration can be
 used at the same time with FreeIPA-integrated DNS or not.
 Side-question is what to do if a first server is installed with
 external-DNS but another replica is being installed with
 integrated-DNS and so on.

 I think being able to integrate with an external DNS is important, and
 not just at the server level, being able to distribute TSIG keys to
 client would be nice too, though a lot less important, than getting
 server integration..
 
 Using TSIG on many clients is very problematic. Keep in mind that TSIG is
 basically HMAC computed using symmetric key so:
 a) Every client would have to use the same key - this is a security nightmare.
 b) We would have to distribute and maintain many keys for many^2 clients,
 which is an administrative nightmare.
 
 For *clients* I would rather stay with GSS-TSIG which is much more manageable
 because we can use Kerberos trust and Keytab distribution is already solved
 by ipa-client-install.
 
 Alternative for clients would be to use FreeIPA server as proxy which
 encapsulates TSIG keys and issues update requests on behalf of clients. This
 would be FreeIPA-specific but any TSIG-distribution mechanism will be
 FreeIPA-specific anyway.
 
 TSIG standard explicitly says that there is no standardized distribution
 mechanism.
 
 In other words, the question is if current dns.py plugin shipped
 with FreeIPA framework should be:

 a) Extended dns.py with dnsexternal-* commands
 --
 Disadvantages:
 - It complicate FreeIPA DNS interface which is a complex beast even
 now.
 - We would have add condition to every DNS API call in installers
 which would increase horribleness of the installer code even more (or
 add another layer of abstraction...).

 I agree having a special command is undesirable.

 - I don't see a point in using integrated-DNS with external-DNS at
 the same time. To use integrated-DNS you have to get a proper DNS
 delegation from parent domain - and if you can get the delegation
 then there is no point in using external DNS ...

 I disagree on this point, it makes a lot of sense to have some zones
 maintained by IPA and ... some not.

 Advantages:
 - You can use external  integrated DNS at the same time.

 We can achieve the same w/o a new ipa level command.
 
 This needs to be decided by FreeIPA framework experts ...
 
 Petr^3 came up with clever 'proxy' idea for IPA-commands. This proxy would
 provide all ipa dns* commands and dispatch user-issued commands to appropriate
 FreeIPA-plugin (internal-dns or external-dns). This decision could depend on
 some values in LDAP.
 
 b) Replace dns.py with another implementation of current dnszone-* 
 dnsrecord-* API.
 -
 This seems like a cleaner approach to me. It could be shipped as
 ipa-server-dns-external package (opposed to standard ipa-server-dns
 package).

 Advantages:
 - It could seamlessly work with FreeIPA client installer because the
 dns*-nsupdate command transformation would be done on FreeIPA server
 and client doesn't need to know about it.
 - Does not require re-training/not much new documentation because
 commands are the same.

 Disadvantages:
 - You can't use integrated  external DNS at the same time (but I
 don't think it justifies the added complexity).

 I disagree.

 One of the reason to use a mix is to allow smooth migrations where
 zones are moved into (or out of) IPA one by one.
 
 Good point, I agree. I will focus on supporting both (internal and external)
 DNS at the same time.
 
 Petr^3 or anyone else, what do you propose?

 I think what I'd like to see is to be able to configure a DNS zone in
 LDAP and mark it external.
 The zone would held the TSIG keys necessary to perform DNS updates.

 When the regular ipa dnsrecord-add etc... commands are called, the
 framework detects that the zone is external, fewttches the TSIG key
 (if the user has access to it) and spawn an nsupdate command that will
 perform the update against whatever DNS server is authoritative for the
 zone.

 Some

Re: [Freeipa-devel] [PATCH 0170] Detect and warn about invalid forwardzone configuration

2014-12-01 Thread Petr Spacek
On 1.12.2014 14:39, Martin Basti wrote:
 On 24/11/14 17:24, Petr Spacek wrote:
 Hello!

 Thank you for the patch. It is not ready yet but the overall direction seems
 good. Please see my comments in-line.

 On 24.11.2014 14:35, Martin Basti wrote:
 Ticket: https://fedorahosted.org/freeipa/ticket/4721
 Patch attached

 -- 
 Martin Basti


 freeipa-mbasti-0170-Detect-and-warn-about-invalid-DNS-forward-zone-confi.patch


  From a5a19137e3ddf2d2d48cfbdb2968b6f68ac8f772 Mon Sep 17 00:00:00 2001
 From: Martin Basti mba...@redhat.com
 Date: Fri, 21 Nov 2014 16:54:09 +0100
 Subject: [PATCH] Detect and warn about invalid DNS forward zone 
 configuration

 Shows warning if forward and parent authoritative zone do not have
 proper NS record delegation, which can cause the forward zone will be
 ineffective and forwarding will not work.

 The chande in the test was required due python changes order of elemnts
 in dict (python doesnt guarantee an order in dict)

 Ticket: https://fedorahosted.org/freeipa/ticket/4721
 ---
   ipalib/messages.py  |  12 +++
   ipalib/plugins/dns.py   | 147
 +---
   ipatests/test_xmlrpc/test_dns_plugin.py |   2 +-
   3 files changed, 149 insertions(+), 12 deletions(-)

 diff --git a/ipalib/messages.py b/ipalib/messages.py
 index
 102e35275dbe37328c84ecb3cd5b2a8d8578056f..cc9bae3d6e5c7d3a7401dab89fafb1b60dc1e15f
 100644
 --- a/ipalib/messages.py
 +++ b/ipalib/messages.py
 @@ -200,6 +200,18 @@ class
 DNSServerDoesNotSupportDNSSECWarning(PublicMessage):
  uIf DNSSEC validation is enabled on IPA server(s), 
  uplease disable it.)
   +class ForwardzoneIsNotEffectiveWarning(PublicMessage):
 +
 +**13008** Forwardzone is not effective, forwarding will not work 
 because
 +there is authoritative parent zone, without proper NS delegation
 +
 +
 +errno = 13008
 +type = warning
 +format = _(uforward zone \%(fwzone)s\ is not effective (missing
 proper 
 +   uNS delegation in authoritative zone \%(authzone)s\). 
 +   uForwarding may not work.)
 I think that the error message could be made mode useful:

 Forwarding will not work. Please add NS record 
 fwdzone.relativize(authzone)
 to parent zone %(authzone)s (or something like that).

 +
 def iter_messages(variables, base):
   Return a tuple with all subclasses
 diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
 index
 c5d96a8c4fcdf101254ecefb60cb83d63bee6310..4f92da645e0faf784c7deb4b8ce386c1440f4229
 100644
 --- a/ipalib/plugins/dns.py
 +++ b/ipalib/plugins/dns.py
 @@ -1725,6 +1725,79 @@ def _normalize_zone(zone):
   return zone
 +def _find_zone_which_makes_fw_zone_ineffective(fwzonename):
 Generally, this method finds an authoritative zone for given fwzonename.
 What about method name _find_auth_zone_ldap(name) ?

 +
 +Check if forward zone is effective.
 +
 +If parent zone exists as authoritative zone, forward zone will not
 +forward queries. It is necessary to delegate authority of forward zone
 I would clarify it: forward queries by default.


 +to another server (non IPA DNS server).
 I would personally omit (non IPA DNS server) because this mechanism is not
 related to IPA domain at all.

 +Example:
 +
 +Forward zone: sub.example.com
 +Zone: example.com
 +
 +Forwarding will not work, because server thinks it is authoritative
 +and returns NXDOMAIN
 +
 +Adding record: sub.example.com NS nameserver.out.of.ipa.domain.
 Again, this is not related to IPA domain at all. I propose this text:
 Adding record: sub.example.com NS ns.sub.example.com.

 +will delegate authority to another server, and IPA DNS server will
 +forward DNS queries.
 +
 +:param fwzonename: forwardzone
 +:return: None if effective, name of authoritative zone otherwise
 +
 +assert isinstance(fwzonename, DNSName)
 +
 +# get all zones
 +zones = api.Command.dnszone_find(pkey_only=True,
 +sizelimit=0)['result']
 Ooooh, are you serious? :-) I don't like this approach, I would rather chop
 left-most labels from name and so dnszone_find() one by one. What if you
 have many DNS zones?


 This could be thrown away if you iterate only over relevant zones.
 +zonenames = [zone['idnsname'][0].make_absolute() for zone in zones]
 +zonenames.sort(reverse=True, key=len)  # sort, we need longest match
 +
 +# check if is there a zone which can cause the forward zone will
 +# be ineffective
 +sub_of_auth_zone = None
 +for name in zonenames:
 +if fwzonename.is_subdomain(name):
 +sub_of_auth_zone = name
 +break
 +
 +if sub_of_auth_zone is None:
 +return None
 +
 +# check if forwardzone is delegated in authoritative zone
 +# test if exists and get NS record
 +try:
 +ns_records = api.Command.dnsrecord_show(
 +sub_of_auth_zone

[Freeipa-devel] [PATCH 0311-0314] Preparation for 6.1 release

2014-11-28 Thread Petr Spacek
Hello,

Bump NVR to 6.1.
Pushed to master: 5adca1bbfc2c19c53ecda649b94960f6abe73cf6

Add release engineering tools to source tree.
Pushed to master: b85fed4c4512537d9dce39f525c556a02e436753

Update NEWS for upcoming 6.1 release.
Pushed to master: e92d96de044c1b1a36d1d1a2a444656278edf0a6

Update .gitignore to skip compile script from Autotools.
Pushed to master: 75a706252bf816cbe236791e187c80d83774ad7d

-- 
Petr^2 Spacek
From 75a706252bf816cbe236791e187c80d83774ad7d Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 28 Nov 2014 10:09:30 +0100
Subject: [PATCH] Update .gitignore to skip compile script from Autotools.

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 40dfa22c5c8485ef0a4228ac6f5dede3730f9320..04a689bf723c287b47eb46a3a343d0a376cb2080 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,6 +10,7 @@ Makefile.in
 /aclocal.m4
 /autom4te.cache
 /ar-lib
+/compile
 /config.guess
 /config.h.in
 /config.sub
-- 
2.1.0

From e92d96de044c1b1a36d1d1a2a444656278edf0a6 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 28 Nov 2014 10:10:40 +0100
Subject: [PATCH] Update NEWS for upcoming 6.1 release.

---
 NEWS | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/NEWS b/NEWS
index f88d8552d34a774f684081f14340208cdf00a43b..313151ab197e4529def7953a1fbffa8fc3e8c5a7 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,17 @@
+6.1
+
+[1] Crash caused by interaction between forward and master zones was fixed.
+https://fedorahosted.org/bind-dyndb-ldap/ticket/145
+
+[2] DNS NOTIFY messages are sent after any modification to the zone.
+https://fedorahosted.org/bind-dyndb-ldap/ticket/144
+
+[3] Misleading error message about forward zones during reconnect was fixed.
+
+[4] Informational message about number of defined/loaded zones was improved.
+
+[5] Various build system improvements.
+
 6.0
 
 [1] idnsZoneActive attribute is supported again and zones can be activated
-- 
2.1.0

From b85fed4c4512537d9dce39f525c556a02e436753 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 28 Nov 2014 13:40:52 +0100
Subject: [PATCH] Add release engineering tools to source tree.

See releng/README for details.
---
 .gitignore   |  4 +++
 releng/README| 33 ++
 releng/bumpver.py| 86 +++
 releng/srcversion.py | 95 
 releng/trac.py   | 39 +
 releng/tracvers.py   | 44 
 6 files changed, 301 insertions(+)
 create mode 100644 releng/README
 create mode 100755 releng/bumpver.py
 create mode 100755 releng/srcversion.py
 create mode 100644 releng/trac.py
 create mode 100755 releng/tracvers.py

diff --git a/.gitignore b/.gitignore
index 04a689bf723c287b47eb46a3a343d0a376cb2080..53d7ed414ab0ee45afa2cb8ac84e8ab23de358ac 100644
--- a/.gitignore
+++ b/.gitignore
@@ -25,3 +25,7 @@ Makefile.in
 .project
 .cproject
 .settings
+
+# Python
+*.pyc
+__pycache__
diff --git a/releng/README b/releng/README
new file mode 100644
index ..8bdb1693077a54f3c910643a354725a9ebc93d17
--- /dev/null
+++ b/releng/README
@@ -0,0 +1,33 @@
+Release Engineering
+===
+Latest cheat sheet is available on:
+https://fedorahosted.org/bind-dyndb-ldap/wiki/ReleaseProcess
+
+This directory contains assorted set of tools related to release enginnering.
+All scripts should be run from root of the source tree.
+
+NOTE!
+Scripts can make changes in local repo so all changes should be carefully
+reviewed before git push!
+
+Final push to remore repo has to be done manually.
+
+
+User scripts
+
+
+bumpver.py
+~~
+Increments version number in configure.ac and SPEC file and creates a signed
+tag for current release.
+
+tracvers.py
+~~~
+Creates Trac version for each Git tag.
+
+
+Auxiliary scripts
+-
+These scripts are not intended for usage from command line:
+- srcversion.py
+- trac.py
diff --git a/releng/bumpver.py b/releng/bumpver.py
new file mode 100755
index ..191ff2ffe68451f88ae112718a8ea961216b7e3d
--- /dev/null
+++ b/releng/bumpver.py
@@ -0,0 +1,86 @@
+#!/usr/bin/env python
+
+
+Bump version number in source tree, commit and tag resulting tree.
+
+Usage:
+- bumpver.py - bump minor version by 1
+- bumpver.py 6.1 - set major and minor versions to 6.1
+
+Assumptions:
+- README file was updated  commited by hand.
+
+Checks:
+- Working directory is clean.
+- NEWS file contains notes for new version.
+
+Actions:
+- Bump version in configure.ac  bind-dyndb-ldap.spec.
+- Commits bumped version.
+- Tags new version.
+
+
+import logging
+import re
+from subprocess import check_output, check_call
+import sys
+
+from srcversion import FileVersion, FakeVersion
+
+logging.basicConfig(level=logging.DEBUG)
+log = logging.getLogger('bump')
+
+def bump_version

Re: [Freeipa-devel] [PATCH 0036] Add missing python files to Makefile

2014-11-27 Thread Petr Spacek
On 27.11.2014 11:00, Martin Basti wrote:
 On 27/11/14 00:50, Gabe Alford wrote:
 Hello,

Wondering if I could get a review. Updated patch attached.

 Thanks,
 Gabe

 On Tue, Nov 11, 2014 at 7:21 AM, Gabe Alford redhatri...@gmail.com
 mailto:redhatri...@gmail.com wrote:

 Hello,

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

 Thanks,

 Gabe



 Hello,
 
 sorry for late response.
 
 We push this ticket to backlog, as it would be part of build system 
 refactoring.
 The app_PYTHON statement is not used anymore in IPA, the better solution is
 remove it, instead of keeping dead code up-to-date.

Just to clarify:
It can be pushed if it works, there is no need to postpone accepting patch if
the patch seems okay and doesn't break anything.

Martin, please keep in mind that contributions are welcome at any time.

Milestones in Trac reflect our view of priorities but it doesn't prevent us
from accepting correct patches from contributions at any time, no matter which
priority is stated in Trac (or even if there is no ticket for it ...).

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0307] Send DNS NOTIFY message after any modification to the zone

2014-11-27 Thread Petr Spacek
On 27.11.2014 15:56, Tomas Hozza wrote:
 On 11/26/2014 01:46 PM, Martin Basti wrote:
  On 07/11/14 15:34, Petr Spacek wrote:
   Hello,
  
   Send DNS NOTIFY message after any modification to the zone.
  
   https://fedorahosted.org/bind-dyndb-ldap/ticket/144
  
  Works for me. But don't push it before Tomas check the code please.
  Martin^2
 
 ACK. Works for me...

Pushed to master: 7dd6ba6c70273fef0ffd34b265e6f1a1b6988a26

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0228] Drop unnecessary #define _BSD_SOURCE

2014-11-26 Thread Petr Spacek
On 26.11.2014 16:47, Lukas Slebodnik wrote:
 On (12/11/14 16:34), Petr Spacek wrote:
 On 25.2.2014 15:05, Lukas Slebodnik wrote:
 On (25/02/14 09:54), Petr Spacek wrote:
 On 24.2.2014 18:56, Lukas Slebodnik wrote:
 On (24/02/14 16:48), Petr Spacek wrote:
 Hello,

 Drop unnecessary #define _BSD_SOURCE.

 --
 Petr^2 Spacek

 From 1b5105e3ab92f2a898313da5f7e20e6f3e9d1d2a Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Mon, 24 Feb 2014 16:48:09 +0100
 Subject: [PATCH] Drop unnecessary #define _BSD_SOURCE.

 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
 src/krb5_helper.c | 2 --
 1 file changed, 2 deletions(-)

 diff --git a/src/krb5_helper.c b/src/krb5_helper.c
 index 
 d1787209483f2ae49b480492290ff5d4bafc677c..71f4fff9fec551abbd81e25c59de80d2ded0dfc6
  100644
 --- a/src/krb5_helper.c
 +++ b/src/krb5_helper.c
 @@ -15,8 +15,6 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 
 USA
  */

 -#define _BSD_SOURCE
 -
 #include isc/util.h
 #include string.h
 #include stdlib.h
 --
 1.8.5.3


 Simo is an author (according to git blame)
 He defined this macro due to function setenv

 from man setenv:
 NAME
setenv - change or add an environment variable

 SYNOPSIS
#include stdlib.h

int setenv(const char *name, const char *value, int overwrite);

int unsetenv(const char *name);

Feature Test Macro Requirements for glibc (see feature_test_macros(7)):

setenv(), unsetenv():
_BSD_SOURCE || _POSIX_C_SOURCE = 200112L || _XOPEN_SOURCE = 
 600
 

 Macros _BSD_SOURCE _POSIX_C_SOURCE were defined when I included
 header file stdlib.h. I tested only on fedora 20. It can be used
 on the other distributions.

 I would rather let this macro as is.

 Wow, I didn't expect that somebody will spend time on this :-)

 See build logs from Fedora 21
 http://koji.fedoraproject.org/koji/getfile?taskID=6565007name=build.log

 You should have noticed this in the 1st mail. Because it is difference 
 between
 removing unnecessary macro and depprecated usage of macro.

 /usr/include/features.h:145:3: error: #warning _BSD_SOURCE and _SVID_SOURCE
 are deprecated, use _DEFAULT_SOURCE [-Werror=cpp]
  # warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use 
 _DEFAULT_SOURCE

 Patches with 'the right' solution are welcome. I'm not going to spend
 more time on this.

 Attached patch should fix the warning in the 'proper' way, I hope. Without
 this patch the warning constantly pops up on Fedora 21.

 -- 
 Petr^2 Spacek
 
From 873334fb1ede302b3a6cbf52ac8bc7e98a4659f9 Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Wed, 12 Nov 2014 16:30:56 +0100
 Subject: [PATCH] Replace deprecated macro #define _BSD_SOURCE with
 _POSIX_C_SOURCE.

 See
 https://sourceware.org/glibc/wiki/Release/2.20#Packaging_Changes
 ---
 src/krb5_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/src/krb5_helper.c b/src/krb5_helper.c
 index 
 169d384cddb5ab9fc9cce1f5ec773836a4c383bb..85c8df9f15af839786ded50d41313763f6463579
  100644
 --- a/src/krb5_helper.c
 +++ b/src/krb5_helper.c
 @@ -15,7 +15,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  */

 -#define _BSD_SOURCE
 +#define _POSIX_C_SOURCE 200112L /* setenv */
 I'm not sure wheather it is good idea to define special value.
 It should be autodetected.
 
 One way could be to test available extensions in configure time
 AC_USE_SYSTEM_EXTENSIONS.
 
 or use _BSD_SOURCE conditioanally.
 
 diff --git a/configure.ac b/configure.ac
 index 9e33116..95a1440 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -30,7 +30,7 @@ AC_CHECK_HEADERS([stddef.h stdlib.h string.h strings.h])
  AC_TYPE_SIZE_T
  
  # Checks for library functions.
 -AC_CHECK_FUNCS([memset strcasecmp strncasecmp])
 +AC_CHECK_FUNCS([memset strcasecmp strncasecmp setenv])
  
  # Check if build chain supports symbol visibility
  AC_MSG_CHECKING([for -fvisibility=hidden compiler flag])
 diff --git a/src/krb5_helper.c b/src/krb5_helper.c
 index d178720..8ce11b8 100644
 --- a/src/krb5_helper.c
 +++ b/src/krb5_helper.c
 @@ -15,7 +15,10 @@
   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
   */
  
 +#include config.h
 +#ifndef HAVE_SETENV
  #define _BSD_SOURCE
 +#endif /* HAVE_SETENV */
  
  #include isc/util.h
  #include string.h

Hello,

thank you for your patch but I'm going to stick to standard POSIX way.

NACK

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH][bind-dyndb-ldap] AUTOCONF: Improve detection of bind9 header files

2014-11-26 Thread Petr Spacek
On 26.11.2014 15:57, Lukas Slebodnik wrote:
 On (12/11/14 15:30), Petr Spacek wrote:
 On 24.7.2014 11:00, Petr Spacek wrote:
 On 27.2.2014 15:19, Lukas Slebodnik wrote:
 ehlo,

 I did some reviews of bind-dyndb-ldap last week  and it was little bit 
 annoying
 to export special CFLAGS for bind9 header files. It can be automatically
 detected in configure script using utility isc-config.

 Attached patch should improve this and CFLAGS needn't be exported.

 Kind NACK. It would be valuable to test if isc/errno2result.h header is
 present and throw appropriate error.

 Current check with isc-config.sh only will pass if you have bind-devel 
 package
 installed but you are missing bind-lite-devel package.


 I have a question: How
 +ldap_la_CFLAGS = $(BIND9_CFLAGS) -Wall -Wextra @WERROR@ -std=gnu99
 works?

 Will it take user-defined CFLAGS into account? I would like to place
 user-defined flags at the end of the list so you can easily override 
 settings
 given by autotools.

 Thank you for clarification :-)


 I will be really happy to commit complete fix. Thank you for cleaning this
 autotools mess!

 This version actually works. Previous version did not take CFLAGS from
 isc-config.sh into account during libdns version check so it actually did not
 work at all :-)

 Please review it (and send me a modified patch if you see a problem).

 Thank you for your time!

 -- 
 Petr^2 Spacek
 
From 4b17099abe2169ddb86b24e53cd2769b76f3ea2d Mon Sep 17 00:00:00 2001
 From: Lukas Slebodnik lsleb...@redhat.com
 Date: Tue, 25 Feb 2014 10:46:50 +0100
 Subject: [PATCH] Improve detection of BIND 9 header files and necessary
 CFLAGS.

 BIND 9 header files can be stored in non-default path (/usr/include/bind9).
 The isc-config.sh utility can provide necessary CFLAGS.
 ---
 configure.ac | 43 ++-
 contrib/bind-dyndb-ldap.spec |  1 -
 2 files changed, 34 insertions(+), 10 deletions(-)

 diff --git a/configure.ac b/configure.ac
 index 
 d471038ada54c07dcfc211c8a2572850e3b28205..c985908c760c974f7c02b6fa3d183e839bbeb9ad
  100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -15,14 +15,6 @@ m4_ifdef([AM_PROG_AR], [AM_PROG_AR])
 AC_PROG_CC
 AC_PROG_LIBTOOL

 -# Checks for libraries.
 -AC_CHECK_LIB([dns], [dns_name_init], [],
 -AC_MSG_ERROR([Install BIND9 development files]))
 -AC_CHECK_LIB([ldap], [ldap_initialize], [],
 -AC_MSG_ERROR([Install OpenLDAP development files]))
 -AC_CHECK_LIB([krb5], [krb5_cc_initialize], [],
 -AC_MSG_ERROR([Install Kerberos 5 development files]))
 -
 # Checks for header files.
 AC_CHECK_HEADERS([stddef.h stdlib.h string.h strings.h])

 @@ -47,6 +39,39 @@ AC_TRY_COMPILE([
 [CFLAGS=$SAVED_CFLAGS
  AC_MSG_RESULT([no])])

 +# Get CFLAGS from isc-config.sh
 +AC_ARG_VAR([BIND9_CFLAGS],
 +   [C compiler flags for bind9, overriding isc-config.sh])
 +AC_SUBST(BIND9_CFLAGS)
 +
 +dnl do not override enviroment variables BIND9_CFLAGS
 +if test -z $BIND9_CFLAGS; then
   ^
 What is a purpose of this condition.
 IIRC AC_SUBST(BIND9_CFLAGS) should allow you to override BIND9_CFLAGS
 from command line.

Don's ask me, it was in your original version of the patch :-)

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0228] Drop unnecessary #define _BSD_SOURCE

2014-11-26 Thread Petr Spacek
On 26.11.2014 13:04, Tomas Hozza wrote:
 On 11/25/2014 07:53 PM, Martin Basti wrote:
  On 12/11/14 16:34, Petr Spacek wrote:
   On 25.2.2014 15:05, Lukas Slebodnik wrote:
   On (25/02/14 09:54), Petr Spacek wrote:
   On 24.2.2014 18:56, Lukas Slebodnik wrote:
   On (24/02/14 16:48), Petr Spacek wrote:
   Hello,
  
   Drop unnecessary #define _BSD_SOURCE.
  
   --
   Petr^2 Spacek
   From 1b5105e3ab92f2a898313da5f7e20e6f3e9d1d2a Mon Sep 17 
   00:00:00 2001
   From: Petr Spacek pspa...@redhat.com
   Date: Mon, 24 Feb 2014 16:48:09 +0100
   Subject: [PATCH] Drop unnecessary #define _BSD_SOURCE.
  
   Signed-off-by: Petr Spacek pspa...@redhat.com
   ---
   src/krb5_helper.c | 2 --
   1 file changed, 2 deletions(-)
  
   diff --git a/src/krb5_helper.c b/src/krb5_helper.c
   index
   d1787209483f2ae49b480492290ff5d4bafc677c..71f4fff9fec551abbd81e25c59de80d2ded0dfc6
   100644
   --- a/src/krb5_helper.c
   +++ b/src/krb5_helper.c
   @@ -15,8 +15,6 @@
 * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 
   02111-1307 USA
 */
  
   -#define _BSD_SOURCE
   -
   #include isc/util.h
   #include string.h
   #include stdlib.h
   --
   1.8.5.3
  
   Simo is an author (according to git blame)
   He defined this macro due to function setenv
  
   from man setenv:
   NAME
   setenv - change or add an environment variable
  
   SYNOPSIS
   #include stdlib.h
  
   int setenv(const char *name, const char *value, int 
   overwrite);
  
   int unsetenv(const char *name);
  
   Feature Test Macro Requirements for glibc (see 
   feature_test_macros(7)):
  
   setenv(), unsetenv():
   _BSD_SOURCE || _POSIX_C_SOURCE = 200112L || 
   _XOPEN_SOURCE = 600
   
  
   Macros _BSD_SOURCE _POSIX_C_SOURCE were defined when I included
   header file stdlib.h. I tested only on fedora 20. It can be used
   on the other distributions.
  
   I would rather let this macro as is.
   Wow, I didn't expect that somebody will spend time on this :-)
  
   See build logs from Fedora 21
   http://koji.fedoraproject.org/koji/getfile?taskID=6565007name=build.log
   You should have noticed this in the 1st mail. Because it is 
   difference between
   removing unnecessary macro and depprecated usage of macro.
  
   /usr/include/features.h:145:3: error: #warning _BSD_SOURCE and 
   _SVID_SOURCE
   are deprecated, use _DEFAULT_SOURCE [-Werror=cpp]
 # warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use 
   _DEFAULT_SOURCE
  
   Patches with 'the right' solution are welcome. I'm not going to 
   spend
   more time on this.
   Attached patch should fix the warning in the 'proper' way, I hope. 
   Without
   this patch the warning constantly pops up on Fedora 21.
  
  Works for me, I haven't had warning there.
 
 ACK.

Pushed to master: 8ad9965136ab15f14cdb356a81a141575b2a84aa

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0309] Fix crash caused by interaction between forward and master zones

2014-11-26 Thread Petr Spacek
On 26.11.2014 13:33, Tomas Hozza wrote:
 On 11/25/2014 07:07 PM, Martin Basti wrote:
  On 25/11/14 18:11, Petr Spacek wrote:
   Hello,
  
   Fix crash caused by interaction between forward and master zones.
  
   LDAP modifications made to idnsName=sub, idnsName=example.com, cn=dns 
   object
   were incorrectly processed using update_zone() in cases where forward 
   zone
   sub.example.com. existed in LDAP as object idnsName=sub.example.com, 
   cn=dns.
  
   https://fedorahosted.org/bind-dyndb-ldap/ticket/145
  
  
   Tomas and Martin^2, please review it ASAP. Thank you!
  
  Works for me, IPA tests passed, can you Tomas check the code before push?
 
 ACK.

Pushed to master: 584f9ceeef131145feb32a741a8f5dbc04b9a2cd

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0306] Improve info messages about number of defined/loaded zones

2014-11-26 Thread Petr Spacek
On 26.11.2014 13:07, Tomas Hozza wrote:
 On 11/07/2014 01:33 PM, Petr Spacek wrote:
  Hello,
 
  Improve info messages about number of defined/loaded zones.
 
 ACK.
 
 The new message looks good.

Pushed to master: eb600df6af932292e0a15817710cfc674f5c952b

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH][bind-dyndb-ldap] AUTOCONF: Improve detection of bind9 header files

2014-11-26 Thread Petr Spacek
On 26.11.2014 12:33, Tomas Hozza wrote:
 On 11/12/2014 03:30 PM, Petr Spacek wrote:
  On 24.7.2014 11:00, Petr Spacek wrote:
   On 27.2.2014 15:19, Lukas Slebodnik wrote:
   ehlo,
  
   I did some reviews of bind-dyndb-ldap last week  and it was little 
   bit annoying
   to export special CFLAGS for bind9 header files. It can be 
   automatically
   detected in configure script using utility isc-config.
  
   Attached patch should improve this and CFLAGS needn't be exported.
  
   Kind NACK. It would be valuable to test if isc/errno2result.h header is
   present and throw appropriate error.
  
   Current check with isc-config.sh only will pass if you have bind-devel 
   package
   installed but you are missing bind-lite-devel package.
  
  
   I have a question: How
   +ldap_la_CFLAGS = $(BIND9_CFLAGS) -Wall -Wextra @WERROR@ -std=gnu99
   works?
  
   Will it take user-defined CFLAGS into account? I would like to place
   user-defined flags at the end of the list so you can easily override 
   settings
   given by autotools.
  
   Thank you for clarification :-)
  
  
   I will be really happy to commit complete fix. Thank you for cleaning 
   this
   autotools mess!
 
  This version actually works. Previous version did not take CFLAGS from
  isc-config.sh into account during libdns version check so it actually did 
  not
  work at all :-)
 
  Please review it (and send me a modified patch if you see a problem).
 
  Thank you for your time!
 
 ACK.

Pushed to master: 2af6e66e251a0d142b8fa05d7ad5a95fb9946e3e

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0310] Fix misleading error message about forward zones on reconnect

2014-11-26 Thread Petr Spacek
On 26.11.2014 14:13, Tomas Hozza wrote:
 On 11/25/2014 07:25 PM, Martin Basti wrote:
  On 25/11/14 18:27, Petr Spacek wrote:
   Hello,
  
   Fix misleading error message about forward zones on reconnect.
  
   Previously the plugin could log 'already exist' error after
   successful reconnection to LDAP for each active forward zone.
  
   Now it prints message:
   forward zone 'fw.example.com': loaded
  
  
  Log looks better now,  ACK if Tomas has no objections.
 
 ACK.

Pushed to master: a0ef923d7074a2aff2089f6affcda94843fd9455

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0308] Improve detection of BIND 9 isc__errno2result header file

2014-11-26 Thread Petr Spacek
On 26.11.2014 13:39, Tomas Hozza wrote:
 On 11/25/2014 07:48 PM, Martin Basti wrote:
  On 12/11/14 16:11, Petr Spacek wrote:
   Hello,
  
   Improve detection of BIND 9 isc__errno2result header file.
  
   This header file is not in standard distribution so normal isc-config.sh
   detection is not enough.
  
   With this patch, ./configure should work even without explicit CFLAGS 
   and it
   should also  detect that bind-devel or bind-lite-devel packages are 
   missing.
  
  
  Works for me
 
 ACK.

Pushed to master: 3e364c72f79e3cec69fc4c3263cd0bccbc467bf5

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] RFE - Number of thoughts on FreeIPA

2014-11-25 Thread Petr Spacek
On 25.11.2014 04:09, Simo Sorce wrote:
 On Tue, 25 Nov 2014 08:31:33 +1030
 William B will...@firstyear.id.au wrote:
 
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1

 Hi,

 I have been using FreeIPA for some time now. I have done a lot of
 testing for the project, and have a desire to see FreeIPA do well.

 As some background, I'm a system admin for a University, who currently
 runs an unmanaged instance of 389ds. In the future I would love to
 move to FreeIPA, but I want to explain some concerns first.

 I have always noticed that FreeIPA is feature rich, but over time I
 have noticed that this comes at a cost. Most components don't get as
 much testing as they deserve, and just installing and running FreeIPA
 for a few hours, one can quickly find rough edges and issues. Run it
 for longer, and you quickly find more. As a business we value
 reliable, and consistent software that doesn't have any surprises
 when we use it. Unforseen issues sour peoples taste for things like
 FreeIPA, as many people get stuck on their first impressions.

 With these features also comes a lack of advanced documentation. Too
 often the basics are well covered, but there are lots of simple tasks
 that an admin would wish to perform that aren't covered in the
 documentation. High quality, and advanced documentation is really key
 to my work, as not everyone has as much time as I might to learn the
 inside-out of FreeIPA. People want to reference documentation. Again,
 one only needs to sit down and use FreeIPA for a few days, to try and
 use it in their environment and you will quickly find that many tasks
 aren't covered by the documentation. The documentation itself is also
 hard to find, or out of date (Such as on fedoraproject.org, which is
 the first google hit for me).

 FreeIPA also pushes a some policies and ideas by default. Consider the
 password reset functionality. By default, when the password is reset,
 the password is also expired. In our environment, where we have a
 third party tool that does password resets on accounts (Password
 manager), this breaks our expectation as a user would then have to
 reset their password again in the FreeIPA environment. Little things
 like this remove flexibility and inhibit people making the swap.
 These options shouldn't be hardcoded, they should be defaults that
 can be tuned. If someone wants to do stupid things with those
 options, that is their choice, but that flexibility will help FreeIPA
 gain acceptance into businesses.

 Finally, back to our rich features. Not all businesses want all the
 features of FreeIPA. For example, we don't want the Dogtag CA, NTP,
 DNS or Kerberos components. But the default install, installs all
 these packages even if we don't use them, and it configures services
 that we don't necesarily need. Kerberos is especially a risk for us
 as there are plenty of unforseen issues that can arise when you have
 an AD kerberos domain on the same network, even if they live in
 different DNS namespaces. Contractors install systems into unix
 networks, unix systems end up in windows networks. Over time, as
 process and better discipline is involved, these little mistakes will
 be removed, but if we were to deploy FreeIPA tomorrow, I have no
 doubt the kerberos install would interfere with other parts of the
 network. I would really like to see the FreeIPA build, split into
 freeipa-servers and freeipa-servers-core where the core has only
 the 389ds, web ui and kerberos components, and perhaps even one day,
 could even be kerberos free. This might be taking a step back in
 some ways, but the simplicity would be attractive to complex
 environments wanting to step up from unmanaged 389ds, to something
 like FreeIPA, but without all the complexity and overhead of a full
 install. Over time the extra modules can be enabled as administrators
 please in a controlled fashion.
 - - Yes, these things can be controlled through the use of the server
 install command line switches, but if I'm installing and using only
 389 and krb from FreeIPA, I shouldn't need to install all of dogtag
 as well.


 These are just my thoughts on the project, and I think it boils down
 to a few things:

 * RFE to split freeipa packages to core and full.
 * Asking for a review and enhancement of documentation.
 * Better functional testing of FreeIPA server and tasks to help iron
 out obvious issues before release.

 Don't take this as harsh criticism. I think FreeIPA is a great
 project, and a great system. I would like to see it improve and be
 used more widely.
 
 Hi William, good news is, Dogtag, DNS and NTP are all optional
 components, you can install a FreeIPa server withouth the CA and
 without DNS. NTP is installed by default, but it is very easy to
 diasable it if you want.
 
 Kerberos is a core feature and cannot be disabled, but I thing you
 figured that out already.
 
 We have some plans to split the rpm packaging so that DNS and CA
 components can be split into separate 

Re: [Freeipa-devel] Gaps in upstream tests

2014-11-25 Thread Petr Spacek
On 7.11.2014 14:41, Martin Kosek wrote:
 FreeIPA team will soon grow with a new member focusing on upstream QE tests. I
 would like to collect ideas what are the biggest gaps in the current upstream
 test suite from your POV.
 
 Existing requests are tracked here:
 https://fedorahosted.org/freeipa/query?status=assignedstatus=newstatus=reopenedcomponent=Testscol=idcol=summarycol=componentcol=statuscol=ownercol=typecol=prioritycol=milestonegroup=milestoneorder=priority
 
 
 First idea that I head proposed are Upgrade tests. These are often done
 manually. I think that upgrade test from currently supported FreeIPA/Fedora
 version would go a long way (like 3.3.5 on F20 upgraded built RPMs and running
 unit tests).
 
 Second, it would be nice to try testing FreeIPA server in a container. Not
 only it would verify our container efforts, but it may also allow easy
 multi-master tests on one Jenkins VM or local host instead of expensive VM
 orchestration.
 
 Any other areas worth focusing on (besides of course testing newly developed
 features)?

At least simple automated MitM attack against TLS.

First thing which comes to mind is CLI-server interaction and also
certmonger-server interaction.

TLS is hard to get right and if I recall it correctly we already had a problem
with certificate validation...

-- 
Petr^2 Spacek

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


[Freeipa-devel] [PATCH 0309] Fix crash caused by interaction between forward and master zones

2014-11-25 Thread Petr Spacek
Hello,

Fix crash caused by interaction between forward and master zones.

LDAP modifications made to idnsName=sub, idnsName=example.com, cn=dns object
were incorrectly processed using update_zone() in cases where forward zone
sub.example.com. existed in LDAP as object idnsName=sub.example.com, cn=dns.

https://fedorahosted.org/bind-dyndb-ldap/ticket/145


Tomas and Martin^2, please review it ASAP. Thank you!

-- 
Petr^2 Spacek
From b2f94e6e6d60c376519e92a466b9706eae141a37 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 25 Nov 2014 18:05:13 +0100
Subject: [PATCH] Fix crash caused by interaction between forward and master
 zones.

LDAP modifications made to idnsName=sub, idnsName=example.com, cn=dns object
were incorrectly processed using update_zone() in cases where forward zone
sub.example.com. existed in LDAP as object idnsName=sub.example.com, cn=dns.

https://fedorahosted.org/bind-dyndb-ldap/ticket/145
---
 src/fwd_register.h |  3 +++
 src/ldap_entry.c   | 26 ++
 src/ldap_entry.h   |  7 +++
 src/ldap_helper.c  | 14 --
 4 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/src/fwd_register.h b/src/fwd_register.h
index 02ca7092d35ffbd684a4b531ac4ffbd94addd765..f7182ea0942ec0df811898c6de914f3302a722e3 100644
--- a/src/fwd_register.h
+++ b/src/fwd_register.h
@@ -4,6 +4,9 @@
 #include dns/rbt.h
 #include dns/result.h
 
+#include util.h
+#include rbt_helper.h
+
 #define FORWARDING_SET_MARK ((void *)1)
 /*
 #if FORWARDING_SET_MARK == NULL
diff --git a/src/ldap_entry.c b/src/ldap_entry.c
index 9823fddfe6cb9805565152ccec9f130d01cc0f8f..18e6980f075f5f916826599a30abd9173ad583f7 100644
--- a/src/ldap_entry.c
+++ b/src/ldap_entry.c
@@ -476,6 +476,32 @@ ldap_entry_getclass(ldap_entry_t *entry, ldap_entryclass_t *class)
 	return ISC_R_SUCCESS;
 }
 
+/**
+ * Infer entry class from auxiliary information.
+ *
+ * This is a fallback method for cases where objectClass values
+ * are not available.
+ *
+ * TODO: Object class information should be stored in UUID database
+ * 	 (once we have it).
+ */
+isc_result_t
+ldap_entry_guessclass(dns_name_t *entry_name, isc_boolean_t iszone,
+		  fwd_register_t *fwd_register, ldap_entryclass_t *class) {
+	REQUIRE(class != NULL);
+
+	if (iszone == ISC_TRUE) {
+		if (fwdr_zone_ispresent(fwd_register, entry_name)
+		== ISC_R_SUCCESS)
+			*class = LDAP_ENTRYCLASS_FORWARD;
+		else /* master zone */
+			*class = (LDAP_ENTRYCLASS_MASTER | LDAP_ENTRYCLASS_RR);
+	} else
+		*class = LDAP_ENTRYCLASS_RR;
+
+	return ISC_R_SUCCESS;
+}
+
 isc_result_t
 ldap_attr_firstvalue(ldap_attribute_t *attr, ld_string_t *str)
 {
diff --git a/src/ldap_entry.h b/src/ldap_entry.h
index 420fcde5c06b46c9dd11e98ef9744be5b2b9524c..76a958520b8eb1c9f039e399ac9f4e0f1b346414 100644
--- a/src/ldap_entry.h
+++ b/src/ldap_entry.h
@@ -26,6 +26,8 @@
 #include isc/util.h
 #include dns/types.h
 
+#include fwd_register.h
+#include util.h
 #include str.h
 
 #define LDAP_DEPRECATED 1
@@ -137,6 +139,11 @@ isc_result_t
 ldap_entry_getclass(ldap_entry_t *entry, ldap_entryclass_t *class) ATTR_NONNULLS ATTR_CHECKRESULT;
 
 isc_result_t
+ldap_entry_guessclass(dns_name_t *entry_name, isc_boolean_t iszone,
+		  fwd_register_t *fwd_register, ldap_entryclass_t *class)
+		  ATTR_NONNULLS ATTR_CHECKRESULT;
+
+isc_result_t
 ldap_attr_firstvalue(ldap_attribute_t *attr, ld_string_t *str) ATTR_NONNULLS ATTR_CHECKRESULT;
 
 /*
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 091be5ddd473cce170e2b84c7477143cf4a6ee15..ce0db37d7dfd51504d6e3f798c5b8734457e92d7 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -4795,7 +4795,7 @@ syncrepl_update(ldap_instance_t *inst, ldap_entry_t *entry, int chgtype)
 	CHECKED_MEM_STRDUP(mctx, entry-dn, dn);
 	CHECKED_MEM_STRDUP(mctx, inst-db_name, dbname);
 
-	/* TODO: handle config objects properly - via UUID database */
+	/* TODO: handle object class inference properly - via UUID database */
 	CHECK(setting_get_str(base, inst-local_settings, ldap_base));
 	CHECK(ldap_dn_compare(ldap_base, entry-dn, isbase));
 	if (isbase == ISC_TRUE) {
@@ -4813,15 +4813,9 @@ syncrepl_update(ldap_instance_t *inst, ldap_entry_t *entry, int chgtype)
 			/* deleted entry doesn't contain objectClass, so
 			 * we need to find if the entry is zone or not
 			 * in other way */
-			result = fwdr_zone_ispresent(inst-fwd_register,
-		 entry_name);
-			if (result == ISC_R_SUCCESS)
-class = LDAP_ENTRYCLASS_FORWARD;
-			else if (iszone == ISC_TRUE)
-class = (LDAP_ENTRYCLASS_MASTER |
-	 LDAP_ENTRYCLASS_RR);
-			else
-class = LDAP_ENTRYCLASS_RR;
+			CHECK(ldap_entry_guessclass(entry_name, iszone,
+		inst-fwd_register,
+		class));
 			break;
 		}
 	}
-- 
2.1.0

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

[Freeipa-devel] [PATCH 0310] Fix misleading error message about forward zones on reconnect

2014-11-25 Thread Petr Spacek
Hello,

Fix misleading error message about forward zones on reconnect.

Previously the plugin could log 'already exist' error after
successful reconnection to LDAP for each active forward zone.

Now it prints message:
forward zone 'fw.example.com': loaded

-- 
Petr^2 Spacek
From d5335dcf75e4d35177f477b9efd5a24db36d10d9 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 25 Nov 2014 18:24:11 +0100
Subject: [PATCH] Fix misleading error message about forward zones on
 reconnect.

Previously the plugin could log 'already exist' error after
succesfull reconnection to LDAP for each active forward zone.

Now it prints message:
forward zone 'fw.example.com': loaded
---
 src/ldap_helper.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index ce0db37d7dfd51504d6e3f798c5b8734457e92d7..ddb787c152b522118357bb6dc5542dce6af8ee0e 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1756,15 +1756,15 @@ ldap_parse_fwd_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
 	}
 
 	result = fwdr_add_zone(inst-fwd_register, name);
-	if (result == ISC_R_SUCCESS) {
-		dns_name_format(name, name_txt, DNS_NAME_FORMATSIZE);
-		log_info(forward zone '%s': loaded, name_txt);
-	} else if (result != ISC_R_EXISTS) {
+	if (result != ISC_R_EXISTS  result != ISC_R_SUCCESS) {
 		dns_name_format(name, name_txt, DNS_NAME_FORMATSIZE);
 		log_error_r(failed to add forward zone '%s' 
 			to the forwarding register, name_txt);
 		goto cleanup;
 	}
+	result = ISC_R_SUCCESS;
+	dns_name_format(name, name_txt, DNS_NAME_FORMATSIZE);
+	log_info(forward zone '%s': loaded, name_txt);
 
 cleanup:
 	if (dns_name_dynamic(name))
-- 
2.1.0

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

Re: [Freeipa-devel] [PATCH 0170] Detect and warn about invalid forwardzone configuration

2014-11-24 Thread Petr Spacek
Hello!

Thank you for the patch. It is not ready yet but the overall direction seems
good. Please see my comments in-line.

On 24.11.2014 14:35, Martin Basti wrote:
 Ticket: https://fedorahosted.org/freeipa/ticket/4721
 Patch attached
 
 -- 
 Martin Basti
 
 
 freeipa-mbasti-0170-Detect-and-warn-about-invalid-DNS-forward-zone-confi.patch
 
 
 From a5a19137e3ddf2d2d48cfbdb2968b6f68ac8f772 Mon Sep 17 00:00:00 2001
 From: Martin Basti mba...@redhat.com
 Date: Fri, 21 Nov 2014 16:54:09 +0100
 Subject: [PATCH] Detect and warn about invalid DNS forward zone configuration
 
 Shows warning if forward and parent authoritative zone do not have
 proper NS record delegation, which can cause the forward zone will be
 ineffective and forwarding will not work.
 
 The chande in the test was required due python changes order of elemnts
 in dict (python doesnt guarantee an order in dict)
 
 Ticket: https://fedorahosted.org/freeipa/ticket/4721
 ---
  ipalib/messages.py  |  12 +++
  ipalib/plugins/dns.py   | 147 
 +---
  ipatests/test_xmlrpc/test_dns_plugin.py |   2 +-
  3 files changed, 149 insertions(+), 12 deletions(-)
 
 diff --git a/ipalib/messages.py b/ipalib/messages.py
 index 
 102e35275dbe37328c84ecb3cd5b2a8d8578056f..cc9bae3d6e5c7d3a7401dab89fafb1b60dc1e15f
  100644
 --- a/ipalib/messages.py
 +++ b/ipalib/messages.py
 @@ -200,6 +200,18 @@ class 
 DNSServerDoesNotSupportDNSSECWarning(PublicMessage):
 uIf DNSSEC validation is enabled on IPA server(s), 
 uplease disable it.)
  
 +class ForwardzoneIsNotEffectiveWarning(PublicMessage):
 +
 +**13008** Forwardzone is not effective, forwarding will not work because
 +there is authoritative parent zone, without proper NS delegation
 +
 +
 +errno = 13008
 +type = warning
 +format = _(uforward zone \%(fwzone)s\ is not effective (missing 
 proper 
 +   uNS delegation in authoritative zone \%(authzone)s\). 
 +   uForwarding may not work.)
I think that the error message could be made mode useful:

Forwarding will not work. Please add NS record fwdzone.relativize(authzone)
to parent zone %(authzone)s (or something like that).

 +
  
  def iter_messages(variables, base):
  Return a tuple with all subclasses
 diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
 index 
 c5d96a8c4fcdf101254ecefb60cb83d63bee6310..4f92da645e0faf784c7deb4b8ce386c1440f4229
  100644
 --- a/ipalib/plugins/dns.py
 +++ b/ipalib/plugins/dns.py
 @@ -1725,6 +1725,79 @@ def _normalize_zone(zone):
  return zone
  
  
 +def _find_zone_which_makes_fw_zone_ineffective(fwzonename):
Generally, this method finds an authoritative zone for given fwzonename.
What about method name _find_auth_zone_ldap(name) ?

 +
 +Check if forward zone is effective.
 +
 +If parent zone exists as authoritative zone, forward zone will not
 +forward queries. It is necessary to delegate authority of forward zone
I would clarify it: forward queries by default.


 +to another server (non IPA DNS server).
I would personally omit (non IPA DNS server) because this mechanism is not
related to IPA domain at all.

 +Example:
 +
 +Forward zone: sub.example.com
 +Zone: example.com
 +
 +Forwarding will not work, because server thinks it is authoritative
 +and returns NXDOMAIN
 +
 +Adding record: sub.example.com NS nameserver.out.of.ipa.domain.
Again, this is not related to IPA domain at all. I propose this text:
Adding record: sub.example.com NS ns.sub.example.com.

 +will delegate authority to another server, and IPA DNS server will
 +forward DNS queries.
 +
 +:param fwzonename: forwardzone
 +:return: None if effective, name of authoritative zone otherwise
 +
 +assert isinstance(fwzonename, DNSName)
 +
 +# get all zones
 +zones = api.Command.dnszone_find(pkey_only=True,
 +sizelimit=0)['result']
Ooooh, are you serious? :-) I don't like this approach, I would rather chop
left-most labels from name and so dnszone_find() one by one. What if you
have many DNS zones?


This could be thrown away if you iterate only over relevant zones.
 +zonenames = [zone['idnsname'][0].make_absolute() for zone in zones]
 +zonenames.sort(reverse=True, key=len)  # sort, we need longest match
 +
 +# check if is there a zone which can cause the forward zone will
 +# be ineffective
 +sub_of_auth_zone = None
 +for name in zonenames:
 +if fwzonename.is_subdomain(name):
 +sub_of_auth_zone = name
 +break
 +
 +if sub_of_auth_zone is None:
 +return None
 +
 +# check if forwardzone is delegated in authoritative zone
 +# test if exists and get NS record
 +try:
 +ns_records = api.Command.dnsrecord_show(
 +sub_of_auth_zone, fwzonename, raw=True)['result']['nsrecord']
 +except (errors.NotFound, KeyError):
 +   

Re: [Freeipa-devel] FreeIPA integration with external DNS services

2014-11-14 Thread Petr Spacek
On 14.11.2014 02:22, Simo Sorce wrote:
 On Tue, 11 Nov 2014 16:29:51 +0100
 Petr Spacek pspa...@redhat.com wrote:
 
 Hello,

 this thread is about RFE
 IPA servers when installed should register themselves in the
 external DNS https://fedorahosted.org/freeipa/ticket/4424

 It is not a complete design, just a raw idea.


 Use case
 
 FreeIPA installation to a network with existing DNS infrastructure +
 network administrator who is not willing to add/maintain new DNS
 servers just for FreeIPA.


 High-level idea
 ===
 - Transform dns* commands from FreeIPA framework to equivalent
 nsupdate commands and send DNS updates to existing DNS servers.
 - Provide necessary encryption/signing keys to nsupdate.


 1) Integration to FreeIPA framework
 ===
 First of all, we need to decide if external DNS integration can be
 used at the same time with FreeIPA-integrated DNS or not.
 Side-question is what to do if a first server is installed with
 external-DNS but another replica is being installed with
 integrated-DNS and so on.
 
 I think being able to integrate with an external DNS is important, and
 not just at the server level, being able to distribute TSIG keys to
 client would be nice too, though a lot less important, than getting
 server integration..

Using TSIG on many clients is very problematic. Keep in mind that TSIG is
basically HMAC computed using symmetric key so:
a) Every client would have to use the same key - this is a security nightmare.
b) We would have to distribute and maintain many keys for many^2 clients,
which is an administrative nightmare.

For *clients* I would rather stay with GSS-TSIG which is much more manageable
because we can use Kerberos trust and Keytab distribution is already solved
by ipa-client-install.

Alternative for clients would be to use FreeIPA server as proxy which
encapsulates TSIG keys and issues update requests on behalf of clients. This
would be FreeIPA-specific but any TSIG-distribution mechanism will be
FreeIPA-specific anyway.

TSIG standard explicitly says that there is no standardized distribution
mechanism.

 In other words, the question is if current dns.py plugin shipped
 with FreeIPA framework should be:

 a) Extended dns.py with dnsexternal-* commands
 --
 Disadvantages:
 - It complicate FreeIPA DNS interface which is a complex beast even
 now.
 - We would have add condition to every DNS API call in installers
 which would increase horribleness of the installer code even more (or
 add another layer of abstraction...).
 
 I agree having a special command is undesirable.
 
 - I don't see a point in using integrated-DNS with external-DNS at
 the same time. To use integrated-DNS you have to get a proper DNS
 delegation from parent domain - and if you can get the delegation
 then there is no point in using external DNS ...
 
 I disagree on this point, it makes a lot of sense to have some zones
 maintained by IPA and ... some not.
 
 Advantages:
 - You can use external  integrated DNS at the same time.
 
 We can achieve the same w/o a new ipa level command.

This needs to be decided by FreeIPA framework experts ...

Petr^3 came up with clever 'proxy' idea for IPA-commands. This proxy would
provide all ipa dns* commands and dispatch user-issued commands to appropriate
FreeIPA-plugin (internal-dns or external-dns). This decision could depend on
some values in LDAP.

 b) Replace dns.py with another implementation of current dnszone-* 
 dnsrecord-* API.
 -
 This seems like a cleaner approach to me. It could be shipped as
 ipa-server-dns-external package (opposed to standard ipa-server-dns
 package).

 Advantages:
 - It could seamlessly work with FreeIPA client installer because the
 dns*-nsupdate command transformation would be done on FreeIPA server
 and client doesn't need to know about it.
 - Does not require re-training/not much new documentation because
 commands are the same.

 Disadvantages:
 - You can't use integrated  external DNS at the same time (but I
 don't think it justifies the added complexity).
 
 I disagree.
 
 One of the reason to use a mix is to allow smooth migrations where
 zones are moved into (or out of) IPA one by one.

Good point, I agree. I will focus on supporting both (internal and external)
DNS at the same time.

 Petr^3 or anyone else, what do you propose?
 
 I think what I'd like to see is to be able to configure a DNS zone in
 LDAP and mark it external.
 The zone would held the TSIG keys necessary to perform DNS updates.
 
 When the regular ipa dnsrecord-add etc... commands are called, the
 framework detects that the zone is external, fewttches the TSIG key
 (if the user has access to it) and spawn an nsupdate command that will
 perform the update against whatever DNS server is authoritative for the
 zone.
 
 Some commands may be disabled if the zone is external and appropriate

Re: [Freeipa-devel] [PATCH][bind-dyndb-ldap] AUTOCONF: Improve detection of bind9 header files

2014-11-12 Thread Petr Spacek
On 24.7.2014 11:00, Petr Spacek wrote:
 On 27.2.2014 15:19, Lukas Slebodnik wrote:
 ehlo,

 I did some reviews of bind-dyndb-ldap last week  and it was little bit 
 annoying
 to export special CFLAGS for bind9 header files. It can be automatically
 detected in configure script using utility isc-config.

 Attached patch should improve this and CFLAGS needn't be exported.
 
 Kind NACK. It would be valuable to test if isc/errno2result.h header is
 present and throw appropriate error.
 
 Current check with isc-config.sh only will pass if you have bind-devel package
 installed but you are missing bind-lite-devel package.
 
 
 I have a question: How
 +ldap_la_CFLAGS = $(BIND9_CFLAGS) -Wall -Wextra @WERROR@ -std=gnu99
 works?
 
 Will it take user-defined CFLAGS into account? I would like to place
 user-defined flags at the end of the list so you can easily override settings
 given by autotools.
 
 Thank you for clarification :-)
 
 
 I will be really happy to commit complete fix. Thank you for cleaning this
 autotools mess!

This version actually works. Previous version did not take CFLAGS from
isc-config.sh into account during libdns version check so it actually did not
work at all :-)

Please review it (and send me a modified patch if you see a problem).

Thank you for your time!

-- 
Petr^2 Spacek
From 4b17099abe2169ddb86b24e53cd2769b76f3ea2d Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik lsleb...@redhat.com
Date: Tue, 25 Feb 2014 10:46:50 +0100
Subject: [PATCH] Improve detection of BIND 9 header files and necessary
 CFLAGS.

BIND 9 header files can be stored in non-default path (/usr/include/bind9).
The isc-config.sh utility can provide necessary CFLAGS.
---
 configure.ac | 43 ++-
 contrib/bind-dyndb-ldap.spec |  1 -
 2 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/configure.ac b/configure.ac
index d471038ada54c07dcfc211c8a2572850e3b28205..c985908c760c974f7c02b6fa3d183e839bbeb9ad 100644
--- a/configure.ac
+++ b/configure.ac
@@ -15,14 +15,6 @@ m4_ifdef([AM_PROG_AR], [AM_PROG_AR])
 AC_PROG_CC
 AC_PROG_LIBTOOL
 
-# Checks for libraries.
-AC_CHECK_LIB([dns], [dns_name_init], [],
-	AC_MSG_ERROR([Install BIND9 development files]))
-AC_CHECK_LIB([ldap], [ldap_initialize], [],
-	AC_MSG_ERROR([Install OpenLDAP development files]))
-AC_CHECK_LIB([krb5], [krb5_cc_initialize], [],
-	AC_MSG_ERROR([Install Kerberos 5 development files]))
-
 # Checks for header files.
 AC_CHECK_HEADERS([stddef.h stdlib.h string.h strings.h])
 
@@ -47,6 +39,39 @@ AC_TRY_COMPILE([
 [CFLAGS=$SAVED_CFLAGS
  AC_MSG_RESULT([no])])
 
+# Get CFLAGS from isc-config.sh
+AC_ARG_VAR([BIND9_CFLAGS],
+   [C compiler flags for bind9, overriding isc-config.sh])
+AC_SUBST(BIND9_CFLAGS)
+
+dnl do not override enviroment variables BIND9_CFLAGS
+if test -z $BIND9_CFLAGS; then
+	AC_PATH_PROG(ISC_CONFIG, [isc-config.sh])
+	AC_MSG_CHECKING([for working isc-config])
+	if test -x $ISC_CONFIG; then
+		AC_MSG_RESULT([yes]);
+		BIND9_CFLAGS=`$ISC_CONFIG --cflags dns`
+		dnl We do not need all libraries suggested by isc-config.sh
+		dnl {-lcrypto, -lcap} are useless
+		dnl BIND9_LIBS=`$ISC_CONFIG --libs dns`
+	else
+		AC_MSG_RESULT([no])
+		AC_MSG_WARN([
+	Could not detect script isc-config.sh. Compilation may fail.
+	Defining variable BIND9_CFLAGS will fix this problem.
+	])
+	fi
+fi
+CFLAGS=$BIND9_CFLAGS $CFLAGS
+
+# Checks for libraries.
+AC_CHECK_LIB([dns], [dns_name_init], [],
+	AC_MSG_ERROR([Install BIND9 development files]))
+AC_CHECK_LIB([ldap], [ldap_initialize], [],
+	AC_MSG_ERROR([Install OpenLDAP development files]))
+AC_CHECK_LIB([krb5], [krb5_cc_initialize], [],
+	AC_MSG_ERROR([Install Kerberos 5 development files]))
+
 # Check version of libdns
 AC_MSG_CHECKING([libdns version])
 AC_TRY_RUN([
@@ -62,7 +87,7 @@ int main(void) {
 [AC_MSG_ERROR([Cross compiling is not supported.])]
 )
 
-# Older autoconf (2.59, for example) doesn't define docdir
+dnl Older autoconf (2.59, for example) doesn't define docdir
 [[ ! -n $docdir ]]  docdir='${datadir}/doc/${PACKAGE_TARNAME}'
 AC_SUBST([docdir])
 
diff --git a/contrib/bind-dyndb-ldap.spec b/contrib/bind-dyndb-ldap.spec
index 572076597a7597c8495ac7a977f26578e840603e..574519982463d811c482bdaaa70839e2c15e9e12 100644
--- a/contrib/bind-dyndb-ldap.spec
+++ b/contrib/bind-dyndb-ldap.spec
@@ -28,7 +28,6 @@ off of your LDAP server.
 %setup -q -n %{name}-%{VERSION}
 
 %build
-export CFLAGS=`isc-config.sh --cflags dns` $RPM_OPT_FLAGS
 %configure
 make %{?_smp_mflags}
 
-- 
2.1.0

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

[Freeipa-devel] [PATCH 0308] Improve detection of BIND 9 isc__errno2result header file

2014-11-12 Thread Petr Spacek
Hello,

Improve detection of BIND 9 isc__errno2result header file.

This header file is not in standard distribution so normal isc-config.sh
detection is not enough.

With this patch, ./configure should work even without explicit CFLAGS and it
should also  detect that bind-devel or bind-lite-devel packages are missing.

-- 
Petr^2 Spacek
From e8feffa54b8e5835d32bfba2c20ef686b8349ec7 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 12 Nov 2014 16:03:12 +0100
Subject: [PATCH] Improve detection of BIND 9 isc__errno2result header file.

This header file is not in standard distribution so normal isc-config.sh
detection is not enough.
---
 configure.ac | 17 +
 1 file changed, 17 insertions(+)

diff --git a/configure.ac b/configure.ac
index c985908c760c974f7c02b6fa3d183e839bbeb9ad..d12ef7bb8c32f320e872d74405b980ced9bd28d8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -65,6 +65,8 @@ fi
 CFLAGS=$BIND9_CFLAGS $CFLAGS
 
 # Checks for libraries.
+AC_CHECK_LIB([isc], [isc_dir_open], [],
+	AC_MSG_ERROR([Install BIND9 ISC development files]))
 AC_CHECK_LIB([dns], [dns_name_init], [],
 	AC_MSG_ERROR([Install BIND9 development files]))
 AC_CHECK_LIB([ldap], [ldap_initialize], [],
@@ -87,6 +89,21 @@ int main(void) {
 [AC_MSG_ERROR([Cross compiling is not supported.])]
 )
 
+dnl isc__errno2result() is typically not present in standard header files
+AC_MSG_CHECKING([isc__errno2result availability in header files])
+AC_TRY_RUN([
+#include isc/errno2result.h
+int main(void) {
+	isc__errno2result(0);
+	return 0;
+}],
+[AC_MSG_RESULT([yes])],
+[AC_MSG_ERROR([
+ Can't find isc__errno2result() or header isc/errno2result.h:
+ Please install bind-lite-devel package or similar.])],
+[AC_MSG_ERROR([Cross compiling is not supported.])]
+)
+
 dnl Older autoconf (2.59, for example) doesn't define docdir
 [[ ! -n $docdir ]]  docdir='${datadir}/doc/${PACKAGE_TARNAME}'
 AC_SUBST([docdir])
-- 
2.1.0

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

Re: [Freeipa-devel] [PATCH 0228] Drop unnecessary #define _BSD_SOURCE

2014-11-12 Thread Petr Spacek
On 25.2.2014 15:05, Lukas Slebodnik wrote:
 On (25/02/14 09:54), Petr Spacek wrote:
 On 24.2.2014 18:56, Lukas Slebodnik wrote:
 On (24/02/14 16:48), Petr Spacek wrote:
 Hello,

 Drop unnecessary #define _BSD_SOURCE.

 --
 Petr^2 Spacek

 From 1b5105e3ab92f2a898313da5f7e20e6f3e9d1d2a Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Mon, 24 Feb 2014 16:48:09 +0100
 Subject: [PATCH] Drop unnecessary #define _BSD_SOURCE.

 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
 src/krb5_helper.c | 2 --
 1 file changed, 2 deletions(-)

 diff --git a/src/krb5_helper.c b/src/krb5_helper.c
 index 
 d1787209483f2ae49b480492290ff5d4bafc677c..71f4fff9fec551abbd81e25c59de80d2ded0dfc6
  100644
 --- a/src/krb5_helper.c
 +++ b/src/krb5_helper.c
 @@ -15,8 +15,6 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  */

 -#define _BSD_SOURCE
 -
 #include isc/util.h
 #include string.h
 #include stdlib.h
 --
 1.8.5.3


 Simo is an author (according to git blame)
 He defined this macro due to function setenv

 from man setenv:
 NAME
setenv - change or add an environment variable

 SYNOPSIS
#include stdlib.h

int setenv(const char *name, const char *value, int overwrite);

int unsetenv(const char *name);

Feature Test Macro Requirements for glibc (see feature_test_macros(7)):

setenv(), unsetenv():
_BSD_SOURCE || _POSIX_C_SOURCE = 200112L || _XOPEN_SOURCE = 600
 

 Macros _BSD_SOURCE _POSIX_C_SOURCE were defined when I included
 header file stdlib.h. I tested only on fedora 20. It can be used
 on the other distributions.

 I would rather let this macro as is.

 Wow, I didn't expect that somebody will spend time on this :-)

 See build logs from Fedora 21
 http://koji.fedoraproject.org/koji/getfile?taskID=6565007name=build.log
 
 You should have noticed this in the 1st mail. Because it is difference between
 removing unnecessary macro and depprecated usage of macro.
 
 /usr/include/features.h:145:3: error: #warning _BSD_SOURCE and _SVID_SOURCE
 are deprecated, use _DEFAULT_SOURCE [-Werror=cpp]
  # warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE
 
 Patches with 'the right' solution are welcome. I'm not going to spend
 more time on this.

Attached patch should fix the warning in the 'proper' way, I hope. Without
this patch the warning constantly pops up on Fedora 21.

-- 
Petr^2 Spacek
From 873334fb1ede302b3a6cbf52ac8bc7e98a4659f9 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 12 Nov 2014 16:30:56 +0100
Subject: [PATCH] Replace deprecated macro #define _BSD_SOURCE with
 _POSIX_C_SOURCE.

See
https://sourceware.org/glibc/wiki/Release/2.20#Packaging_Changes
---
 src/krb5_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/krb5_helper.c b/src/krb5_helper.c
index 169d384cddb5ab9fc9cce1f5ec773836a4c383bb..85c8df9f15af839786ded50d41313763f6463579 100644
--- a/src/krb5_helper.c
+++ b/src/krb5_helper.c
@@ -15,7 +15,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  */
 
-#define _BSD_SOURCE
+#define _POSIX_C_SOURCE 200112L /* setenv */
 
 #include isc/util.h
 #include string.h
-- 
2.1.0

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

Re: [Freeipa-devel] FreeIPA 4.1 release preparations

2014-11-11 Thread Petr Spacek
On 8.11.2014 14:43, Lukas Slebodnik wrote:
 On (20/10/14 16:08), Martin Kosek wrote:
 On 10/20/2014 04:00 PM, Jan Pazdziora wrote:
 On Mon, Oct 20, 2014 at 03:58:27PM +0200, Petr Vobornik wrote:

 The plan is to release 4.1 and then 4.0.4. Besides usual tarballs, 4.1 will
 go into Fedora rawhide, f21-updates-testing and mkosek/freeipa copr repo 
 (to
 be usable on F20).

 And RHEL 7 / CentOS 7?

 For now, I would only maintain RHEL/CentOS 7.0 compatibility for main
 mkosek/freeipa repo.

 It is almost 3 weeks from this mail and freeipa-server cannot be installed 
 from
 mkosek/freeipa repo on  RHEL/CentOS 7.0.
 
 bash-4.2# yum install freeipa-server
 //snip
 
 --- Package pki-base.noarch 0:10.2.0-3.el7.centos will be installed
 -- Processing Dependency: jackson-jaxrs-json-provider for package: 
 pki-base-10.2.0-3.el7.centos.noarch
 -- Finished Dependency Resolution
 Error: Package: pki-base-10.2.0-3.el7.centos.noarch (mkosek-freeipa)
Requires: jackson-jaxrs-json-provider
  You could try using --skip-broken to work around the problem
  You could try running: rpm -Va --nofiles --nodigest
 
 There were some promises on freeipa-users but nothing has changed.
 
 Is somebody working on this problem?
BTW I tried to build few missing packages but I gave it up, the dependency
tree is pretty long.

Anyway, nothing prevents you from grabbing SRPMs from Koji, editing them as
appropriate and rebuilding them in mkosek's COPR :-)

 Maybe it is another candidate for inegtation tests.

-- 
Petr^2 Spacek

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


[Freeipa-devel] FreeIPA integration with external DNS services

2014-11-11 Thread Petr Spacek
Hello,

this thread is about RFE
IPA servers when installed should register themselves in the external DNS
https://fedorahosted.org/freeipa/ticket/4424

It is not a complete design, just a raw idea.


Use case

FreeIPA installation to a network with existing DNS infrastructure + network
administrator who is not willing to add/maintain new DNS servers just for
FreeIPA.


High-level idea
===
- Transform dns* commands from FreeIPA framework to equivalent nsupdate
commands and send DNS updates to existing DNS servers.
- Provide necessary encryption/signing keys to nsupdate.


1) Integration to FreeIPA framework
===
First of all, we need to decide if external DNS integration can be used at
the same time with FreeIPA-integrated DNS or not. Side-question is what to do
if a first server is installed with external-DNS but another replica is being
installed with integrated-DNS and so on.

In other words, the question is if current dns.py plugin shipped with
FreeIPA framework should be:

a) Extended dns.py with dnsexternal-* commands
--
Disadvantages:
- It complicate FreeIPA DNS interface which is a complex beast even now.
- We would have add condition to every DNS API call in installers which would
increase horribleness of the installer code even more (or add another layer of
abstraction...).
- I don't see a point in using integrated-DNS with external-DNS at the same
time. To use integrated-DNS you have to get a proper DNS delegation from
parent domain - and if you can get the delegation then there is no point in
using external DNS ...

Advantages:
- You can use external  integrated DNS at the same time.


b) Replace dns.py with another implementation of current dnszone-* 
dnsrecord-* API.
-
This seems like a cleaner approach to me. It could be shipped as
ipa-server-dns-external package (opposed to standard ipa-server-dns package).

Advantages:
- It could seamlessly work with FreeIPA client installer because the
dns*-nsupdate command transformation would be done on FreeIPA server and
client doesn't need to know about it.
- Does not require re-training/not much new documentation because commands are
the same.

Disadvantages:
- You can't use integrated  external DNS at the same time (but I don't think
it justifies the added complexity).


Petr^3 or anyone else, what do you propose?


2) Authentication to external DNS server/keys
=
This is separate problem from FreeIPA framework integration.
We will have to somehow store raw symmetric keys (for DNS TSIG) or keytabs
(for DNS GSS-TSIG) and distribute them somehow to replicas so every replica
can update DNS records as necessary.

This will be the funny part because in case of AD trusts we have chicken-egg
problem. You need to establish trust to get ticket for DNS/dc1.ad.example@AD
principal but you can't (I guess) establish trust until proper DNS records are
in place ...

For 'experimental' phase I would go with pre-populated CCcache, i.e. admin
will manually do kinit Administrator@AD and then run FreeIPA installer.

Maybe we can re-use trust secret somehow? I don't know, I will reach out to AD
experts with questions.

This area needs more research but for now it seems feasible to re-use DNSSEC
key distribution system for TSIG keys and keytabs so only the chicken-egg
problem is left.

This will need new LDAP schema but I will propose something when I'm done with
investigation.

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCHES] 366-372 Additional Coverity fixes

2014-11-11 Thread Petr Spacek
On 11.11.2014 12:27, Jan Cholasta wrote:
 Dne 11.11.2014 v 11:40 Alexander Bokovoy napsal(a):
 On Tue, 11 Nov 2014, Jan Cholasta wrote:
 From 82d7d37ca310af015018ebb2da2f9a72c4dabcaa Mon Sep 17 00:00:00 2001
 From: Jan Cholasta jchol...@redhat.com
 Date: Mon, 10 Nov 2014 18:10:27 +
 Subject: [PATCH 4/7] Fix unchecked return value in ipa-kdb

 https://fedorahosted.org/freeipa/ticket/4713
 ---
 daemons/ipa-kdb/ipa_kdb_mspac.c | 3 +++
 1 file changed, 3 insertions(+)

 diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c
 b/daemons/ipa-kdb/ipa_kdb_mspac.c
 index c8f6c76..debcd1b 100644
 --- a/daemons/ipa-kdb/ipa_kdb_mspac.c
 +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
 @@ -2071,6 +2071,9 @@ krb5_error_code ipadb_sign_authdata(krb5_context
 context,
 ipactx-kdc_hostname,
 strlen(ipactx-kdc_hostname),
 NULL, NULL, result) == 0) {
 kerr = ipadb_reinit_mspac(ipactx, true);
 +if (kerr != 0  kerr != ENOENT) {
 +goto done;
 +}
 }
 }

 I'm not sure we should drop the sign_authdata request here. If we were
 able to re-initialize our view of trusted domains, we simply cannot
 re-sign incoming PAC but this is handled in ipadb_verify_pac() and
 ipadb_sign_pac() and if the former returns NULL value for PAC, we exit
 with a return code of 0 while this change will fail a cross-realm TGT
 request unconditionally.

 
 OK, what would be a proper fix? Just ignore the return value of
 ipadb_reinit_mspac here?

Guys, I did not see the code but all instances of ignore return code I have
seen were wrong, including cases where code comment explicitly said we ignore
return code on purpose :-)

At least log an error message if you can't think of anything better ...

-- 
Petr^2 Spacek

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


[Freeipa-devel] [PATCH 0307] Send DNS NOTIFY message after any modification to the zone

2014-11-07 Thread Petr Spacek
Hello,

Send DNS NOTIFY message after any modification to the zone.

https://fedorahosted.org/bind-dyndb-ldap/ticket/144

-- 
Petr^2 Spacek
From 8980758721b57789c0f984465845f89c4705b872 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 7 Nov 2014 15:12:38 +0100
Subject: [PATCH] Send DNS NOTIFY message after any modification to the zone.

https://fedorahosted.org/bind-dyndb-ldap/ticket/144
---
 src/ldap_helper.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index cb1ada64635406552f6b231cdb19a888a0f92244..091be5ddd473cce170e2b84c7477143cf4a6ee15 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1017,7 +1017,7 @@ cleanup:
  * @warning Never call this on raw part of in-line secure zone.
  */
 static isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT
-load_zone(dns_zone_t *zone) {
+load_zone(dns_zone_t *zone, isc_boolean_t log) {
 	isc_result_t result;
 	isc_boolean_t zone_dynamic;
 	isc_uint32_t serial;
@@ -1036,15 +1036,18 @@ load_zone(dns_zone_t *zone) {
 	}
 
 	CHECK(dns_zone_getserial2(raw, serial));
-	dns_zone_log(raw, ISC_LOG_INFO, loaded serial %u, serial);
+	if (log == ISC_TRUE)
+		dns_zone_log(raw, ISC_LOG_INFO, loaded serial %u, serial);
 	if (zone != NULL) {
 		result = dns_zone_getserial2(zone, serial);
-		if (result == ISC_R_SUCCESS)
+		if (result == ISC_R_SUCCESS  log == ISC_TRUE)
 			dns_zone_log(zone, ISC_LOG_INFO, loaded serial %u,
  serial);
 		/* in-line secure zone is loaded asynchonously in background */
 		else if (result == DNS_R_NOTLOADED) {
-			dns_zone_log(zone, ISC_LOG_INFO, signing in progress);
+			if (log == ISC_TRUE)
+dns_zone_log(zone, ISC_LOG_INFO,
+	 signing in progress);
 			result = ISC_R_SUCCESS;
 		} else
 			goto cleanup;
@@ -1154,7 +1157,7 @@ activate_zone(isc_task_t *task, ldap_instance_t *inst, dns_name_t *name) {
 		goto cleanup;
 	}
 
-	CHECK(load_zone(toview));
+	CHECK(load_zone(toview, ISC_TRUE));
 	if (secure != NULL) {
 		CHECK(zr_get_zone_settings(inst-zone_register, name,
 	   zone_settings));
@@ -2491,9 +2494,7 @@ ldap_parse_master_zoneentry(ldap_entry_t * const entry, dns_db_t * const olddb,
 	if (isactive == ISC_TRUE) {
 		if (new_zone == ISC_TRUE || activity_changed == ISC_TRUE)
 			CHECK(publish_zone(task, inst, toview));
-		if (data_changed == ISC_TRUE || olddb != NULL ||
-		activity_changed == ISC_TRUE)
-			CHECK(load_zone(toview));
+		CHECK(load_zone(toview, ISC_FALSE));
 	} else if (activity_changed == ISC_TRUE) { /* Zone was deactivated */
 		CHECK(unpublish_zone(inst, name, entry-dn));
 		dns_zone_log(toview, ISC_LOG_INFO, zone deactivated 
@@ -4668,9 +4669,9 @@ cleanup:
 			 reload triggered by change in '%s',
 			 pevent-dn);
 		if (secure != NULL)
-			result = load_zone(secure);
+			result = load_zone(secure, ISC_TRUE);
 		else if (raw != NULL)
-			result = load_zone(raw);
+			result = load_zone(raw, ISC_TRUE);
 		if (result == ISC_R_SUCCESS || result == DNS_R_UPTODATE ||
 		result == DNS_R_DYNAMIC || result == DNS_R_CONTINUE) {
 			/* zone reload succeeded, fire current event again */
-- 
1.9.3

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

[Freeipa-devel] [PATCH 0021] Fix minimal version of BIND for Fedora 20 and 21

2014-11-07 Thread Petr Spacek
Hello,

Fix minimal version of BIND for Fedora 20 and 21.

We should build new mkosek/freeipa COPR package ASAP to solve conflicts on
upgrade.

-- 
Petr^2 Spacek
From 822014a9ed130c05469d80b0cc200cda52d015c5 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Fri, 7 Nov 2014 16:53:48 +0100
Subject: [PATCH] Fix minimal version of BIND for Fedora 20 and 21

---
 freeipa.spec.in | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 36c2a35e7a0c60d4f68e2d945688ee30506e47c6..b2ff97a11dcbb675940086ab9af9aea9bf7988be 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -160,7 +160,13 @@ Obsoletes: freeipa-server-selinux  3.3.0
 # IPA but if it is configured we need a way to require versions
 # that work for us.
 Conflicts: bind-dyndb-ldap  6.0-4
-Conflicts: bind  9.9.6-2
+%if 0%{?fedora} = 21
+Conflicts: bind  9.9.6-3
+Conflicts: bind-utils  9.9.6-3
+%else
+Conflicts: bind  9.9.4-19
+Conflicts: bind-utils  9.9.4-19
+%endif
 # DNSSEC
 Conflicts: opendnssec  1.4.6-4
 
-- 
1.9.3

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

Re: [Freeipa-devel] [PATCH] 357 Added symmetric and asymmetric vaults.

2014-11-05 Thread Petr Spacek

On 5.11.2014 09:32, Martin Kosek wrote:

On 11/05/2014 08:14 AM, Jan Cholasta wrote:

Hi,

Dne 4.11.2014 v 17:54 Endi Sukma Dewata napsal(a):

Hi,

In this patch I'm adding ipaVaultSalt and ipaVaultPublicKey attribute
types to store salt and public key for vault. Are there existing
attribute types that I can use instead? I see there's an ipaPublicKey,
should I use that and maybe add ipaSalt/ipaEncSalt? Thanks.



yes, please re-use existing attributes where possible.

Honza



+1. Also, if ipaSalt/ipaEncSalt is usable outside of Vault, I would go with it,
instead of adding ipaVaultSalt.


Existing schema including ipaPublicKey attribute is described on:
http://www.freeipa.org/page/V4/PKCS11_in_LDAP/Schema

Please note that there are defined data formats too, not only OIDs.

--
Petr^2 Spacek

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


Re: [Freeipa-devel] Releasing testing tools as standalone projects

2014-11-04 Thread Petr Spacek

On 3.11.2014 16:47, Rob Crittenden wrote:

Petr Viktorin wrote:

Hello!

There's been some interest in releasing pieces of FreeIPA's testing
infrastructure so it can be reused in other projects.
I will soon take the pytest-beakerlib plugin (currently in my patch
0672), and making a stand-alone project out of it. Later I'll extract
the common pieces of the integration testign framework, and release that
independently.


Do we want projects projects like these to be hosted on Fedorahosted?
That would be the 100% open-source solution.

Or do we want to put it under a freeipa organization on Github, since
we're more likely to get external contributors there?


Why do you think it would get more contributors from github? Because yet
another account isn't required, or the contributor process is perhaps
better understood (via pull requests)?


IMHO yes. Even for me it is much easier to quickly do

- git clone
- edit source
- git push
- create pull request
(*this is the same for every project hosted on Github*)

instead of

- git clone
- edit source
(re-do following again for every single project)
- hunt submission how-to
- join mailing list/create account in project's tracker
- prepare patch in project's format-of-choice
- send patch


Or both? (Would we want to officially mirror the project to Github
from FH?)


I'd be in favor of fedorahosted because you get a tracker and wiki as
well, and having the repo there would round things out.

rob


--
Petr^2 Spacek

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


[Freeipa-devel] User life-cycle management as additional plugin

2014-11-04 Thread Petr Spacek

Hello,

I wonder if user life-cycle extensions [1] can be in form of separate FreeIPA 
plugin for FreeIPA framework.


Reasoning behind this is that different organizations will have different 
requirements (including no life-cycle management).


I don't think that one-size-fits-all so from my point of view it makes sense 
to ease replacing our life-cycle management by some home-grown plugin.


Is something like that possible?

[1] http://www.freeipa.org/page/V4/User_Life-Cycle_Management

--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0249-0250] Propagate DNS updates changes from LDAP to signed version of the zone

2014-11-03 Thread Petr Spacek

On 23.4.2014 18:16, Petr Spacek wrote:

Hello,

this patch set enables DNS updates to secure zones and also propagates changes
made in LDAP to secure zones.

NSEC3 doesn't work for some reason so don't waste time messing with NSEC3PARAM
:-)


This is delayed push notice:
170d38dd1b27a5f78eb96fe8c80141f6dd56ec97
98d3deac7b75dfe71f6b0e1306c4c52e38e27f3f

--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0246-0248] Follow query/transfer/update policies for secure zones

2014-11-03 Thread Petr Spacek

On 7.5.2014 15:22, Petr Spacek wrote:

On 23.4.2014 18:14, Petr Spacek wrote:

This patch set configures secure zones according to policies in LDAP.


Patch 246 v2 fixes incorrect ATTR_NONNULLS usage which causes segfaults when
compiled with -O0.

Patch 246 v2 obsoletes patch 253.


This is delayed push notice:
b002846b94826d89e7577ad2ed3d852e5296e9d5
748602ed229d3925cc838a9baf2c9888aef7fb3c
0cee0a351c03522aea8ae643644776ed34b5c01f

--
Petr^2 Spacek

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


<    2   3   4   5   6   7   8   9   10   11   >