Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
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)
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
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)
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)
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)
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
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)
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
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)
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)
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
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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?
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
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
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?
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
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
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?
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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