Re: [Freeipa-devel] Ipa-server-install Firewall Support
On 04/03/2014 06:33 PM, Justin Brown wrote: This discussion morphs out of some questions that I asked over on the user's mailing list: https://www.redhat.com/archives/freeipa-users/2014-April/msg00033.html. It's also related to Trac #2110. (The subject says ipa-server-install, but this will also apply to replica installs, too.) What is the expected compatibility with RHEL 5 and 6? Neither of those comes with FirewallD. Integrating with iptables is bound to be messy, and I feel like there's too many ways to get into trouble with destroying a user's iptables configuration. (Due to the unsafety of `iptables-restore`, there's no way to guarantee that the user's configuration will persist through a reboot.) Seeing as FirewallD has been the default in Fedora since 18 and will be in RHEL 7, it seems like a reasonable thing to do. Would there be opposition to making FirewallD the only firewall that ipa-server-install will configure, and just print out iptables (or iptables-save fragments) for non-FirewallD systems? Right. I do not think there will be opposition for firewalld. As you said, it is present in recent Fedoras and RHEL-7.0 and we do not plan to release FreeIPA 4.0 in older systems than that. The implementation should be pretty simple and straightforward, but I wanted to run over it briefly to see if there were any initial feedback. +1. First, there's a question of dependence. FirewallD exposes a DBus interface on the system bus. While it is possible to interact with shell commands, I would greatly prefer to make use of DBus directly. That will require pulling in python-dbus as a package dependency. I personally have nothing against using python-dbus interface (dbus-python package in Fedora), it is programmatically easier to control that parsing output from commands. Second, FirewallD operates on zones to which interfaces are attached. To apply any sort of configuration, we'll need to attach a freeipa-server FirewallD service to the proper zone. During the current installation program, we already know the server's IP address from a call to installutils.get_server_ip_address(). I'll need to resolve on which interface this IP resides, and then FirewallD can tell me which zone should be modified. It is possible to scrape the output of `ip address` to determine, but that's a messy solution. The better method is to use the NetworkManager DBus interfaces to query for this information. This gets back to my compatibility question above. Fedora doesn't really work without NetworkManager anymore, and I assume that will hold for RHEL 7. Right, I personally do not see a problem here in your thoughts. Third, do we want to allow the user to limit the network access? I think that it makes sense to allow users to specify a network (e.g. 192.168.0.0/16) to limit accessibility to their FreeIPA server. Yet, there's always some trade-off to adding more options to ipa-server-install. Hm, I would personally prefer to avoid complicating the feature and start with something easy to configure and straightforward. We can always extend it if wee see an interest. In general, ipa-server-install does the basic, functional and recommended configuration - fine tuning is left for the administrator to continue after that. But I will leave that up to you and other developers. Fourth, FirewallD configuration is simple and quick, so I think it makes sense to get it out of the way early in the install process. I'm thinking right after ipaservices.backup_and_replace_hostname() (line 1,038) runs. I would actually do it the opposite way and open the ports after the FreeIPA server is fully configured. After all, I do not think we want to open the ports when the server is just half-configured and for example some ACIs are missing. Implementation Walkthrough ~$ ipa-server-install --firewall --firewall-allow 192.168.0.0/24 --firewall-allow 192.168.1.0/24 [...] [skip to line 1038] 1. Detect if FirewallD is running via DBus. (If not generate sample iptables rules, print/log them, and continue normal install.) We print list of ports that FreeIPA uses at the end of ipa-server-install process. I would report firewalld/iptables related information there. (We for example also create here example BIND zone content and pass it to user, it may make sense to have this around this location). On the beginning of the installation (in the interactive part of ipa-server-install) I would just check if firewalld is available and if not, I would print a warning to user so that he can for example exit the wizard and fix firewalld before continuing. 2. Use NM DBus to resolve IP-interface. (If NM not available, potentially fallback to scrapping `ip a`.) 3. Use FirewallD DBus to resolve interface to zone. 4. FreeIPA will include a service XML template (like dsinstance.INF_TEMPLATE) that specifies the necessary ports/protocols. Write this service
Re: [Freeipa-devel] [PATCHES] OTP Patches
On 03/24/2014 02:33 PM, Nathaniel McCallum wrote: On Wed, 2014-03-19 at 17:37 +0200, Alexander Bokovoy wrote: On Fri, 21 Feb 2014, Nathaniel McCallum wrote: On Fri, 2014-02-21 at 00:08 +0200, Alexander Bokovoy wrote: On Thu, 20 Feb 2014, Nathaniel McCallum wrote: There is an error in libotp's find() function which assumes that get_basedn() always returns non-NULL value. This is not true for at least cn=Directory Manager. Patch attached. More fixes required, now that Thierry produced the fix for 389-ds ticket 47699 which allows to re-arrange schema-compat and ipa-pwd-extop plugins. I'm getting crash in find() in libotp.c for internal search in some other conditions but at least user dn now is the correct one. Stay tuned. OK, finally I've got it working -- my last patch had error which could be attributed to the late night time. New patch is attached to fix libotp to work properly with empty base dn (such as cn=Directory Manager). Also I'm attaching the patch that sets precedence of schema-compat plugin to 49 (less than default 50). With this patch and 389-ds with patch from ticket 47699 compat tree binds work with OTP. When updated 389-ds-base will be released, we'll need to add Requires: to our RPM spec to depend on it. Without the updated 389-ds-base compat tree binds will not work with OTP but the rest will be working fine. Finally, ACK to all OTP patches. ACK to both of these patches. I've merged the first patch here -- https://www.redhat.com/archives/freeipa-devel/2014-February/msg00341.html I just realized the second patch shouldn't be ACK'd until we have a new 389DS release with the fix. When that happens, reissue this patch with an update versioned require. No, it can be safely merged as 389DS will use default precedence (50) unless the fix is there. So the worst we get is the same as now -- OTP binds will not work over compat tree. And when 389DS will be upgraded, they will start working after 389DS restart. But this patch doesn't actually do anything until we get the new version of 389DS. If we are ever going to add a versioned dependency on the new 389DS for this feature, it should go in this patch. Otherwise, it is an ACK from me. New 389-DS is in Fedora 20 updates stable and Rawhide already. 389-ds-base-1.3.2.16-1.fc20. Also, selinux-policy 3.12.1-135 is now in Fedora 20 updates testing, providing multiple policy enhancements that make possible Apache process to work with kernel-based credentials caches. Attached patch makes use of the new packages. ACK Pushed both patches below: [PATCH 17/17] schema-compat: set precedence to 49 to allow OTP binds over compat tree [PATCH] freeipa.spec.in: update dependencies to 389-ds and selinux-policy to master. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] OTP work, what's left?
On 04/03/2014 05:07 PM, Nathaniel McCallum wrote: On Tue, 2014-04-01 at 17:12 +0200, Martin Kosek wrote: On 03/28/2014 10:17 AM, Martin Kosek wrote: On 03/23/2014 10:26 PM, Alexander Bokovoy wrote: Hi! I've updated my COPR repo with current git master versions of FreeIPA and SSSD with few added patches on top that close OTP gaps (Nathaniel's patch 0038 and Jakub Hrozek's patch for password changes). With these patches we currently lack following parts of the OTP work: - OTP sync client. Still in development, patches and approach need additional review/discussion on the list - Password change in WebUI fails when OTP token exist for the user. More detailed examination is needed, I'm getting ACIError. http://copr.fedoraproject.org/coprs/abbra/freeipa-otp-unstable/ Alexander or Nathaniel, I see you progressed with the OTP development a lot, good job. Please provide a clean list of patches + information who acked what so that it can be pushed to master. Hint: OTP Patches thread is too chaotic for me to follow. Martin Hi Nathaniel, I did a quick search in the thread and it seems to me that at least following 2 patches are not merged (though appears to be ACKed): [PATCH 17/17] schema-compat: set precedence to 49 to allow OTP binds over compat tree [PATCH] freeipa.spec.in: update dependencies to 389-ds and selinux-policy Is that all that is left to be pushed from this long thread? Yup. Nathaniel Thanks, I pushed both to master. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Changing RCUE to PatternFly
On Thu, 03 Apr 2014, Petr Vobornik wrote: Hi all, this is a notice about what I'm going to do so you can raise objections before I spend any time on it. == Intro == First some facts: 1. RCUE was based on Bootstrap 2 (BS2), it used BS2 css file and provided it's own less files with overrides 2. the RCUE adoption started with this version at the end of last year 3. during the winter RCUE was renamed to PatternFly and rebased on Bootstrap 3. It doesn't use BS 3 CSS file anymore but it uses its LESS files to make just one output CSS. 4. PatternFly+Bootstrap 3 LESS files can be compiled only by NodeJS less compiler atm. Support in python-lesscpy is being implemented by OpenStack guys. I planned to upgrade from RCUE to PatternFly when python-lesscpy was ready but now it seems that it will happen no sooner than in F21. As the adoption goes forward I need more stuff from PatternFly (styles for tables, alerts, tabs,...). Using RCUE and cherry picking from BS3 and PatternFly is messy and creates more work. So I decided to upgrade now. The issue with css file provided by PatternFly project is that, that it contains font definitions which force us to bundle font files. == What I'm going to do == - I'll prepare simple less file which will use(combine) all Bootstrap 3, PatternFly and FontAwesome LESS files without the ones which force us to bundle fonts (we already have replacements for those). - it will be compiled by developer using nodejs-lessc - minified output CSS will be added to our git - it should change rarely - I'll document how I did it/write script so others can reproduce it later when needed No third-party LESS files will be in our git except variables.less from each project so we will be able to use the constants in our style definitions (simplifies upgrades). Our own LESS files don't use any new Less markup features so we are still able to compile it with current version of python-lesscpy. ACK. All proposed seems reasonable. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Ipa-server-install Firewall Support
On 4.4.2014 09:17, Martin Kosek wrote:
On 04/04/2014 09:04 AM, Justin Brown wrote:
I would actually do it the opposite way and open the ports after the FreeIPA
server is fully configured. After all, I do not think we want to open the ports
when the server is just half-configured and for example some ACIs are missing.
My thinking was that nothing would be listening on these ports if the
install doesn't succeed, but there's really necessity to modify the
firewall configuration early. (All of the internal install
communication will be over a local interface (to netfilter) and
unblock anyways. I don't have any problem in delaying firewall
configuration to the end of install.
If ipa-server-install does succeed without configuring the firewalld, then we
will indeed have no other option than to do it early.
I am thinking that we may want to put all the firewalld configuration in
ipaserver/install/firewalldinstance.py,
and then make the firewalld configuration the actual step of the installation.
Something like:
...
Configuring Firewall (firewalld)
[1/2]: looking up the right zone
[2/2]: allowing ports
Done configuring Firewall (firewalld).
...
The Service class derived object can be really simple, we would just reuse the
functionality it already has + let us properly hook into it in
ipa-{server,replica}-install and the uninstallation.
It would also make it easier to split this functionality to
freeipa-server-firewalld if we chose to in a future.
In general I agree with the idea, thank you Justin for working on that!
I would like to emphasis the necessity to work without NetworkManager and
FirewallD. New dependencies make Debian folks unhappy ...
On the other hand, it is perfectly fine to skip firewall configuration if
NM/FirewallD/DBus is not available.
Have a nice day!
--
Petr^2 Spacek
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] questions regarding ldap schema for pkcs11
In the review discussion for the ldap schema for pkcs11 there was one topic, which we wanted to get the opinion from a broader audience before making a final decision. In pkcs11 there are many boolean attributes, like CKA_EXTRACTABLE, CKA_DERIVE, CKA_VERIFY and there are two suggestions how to represent them in ldap. 1] one ldap attribute for each pkcs11 attribute. This was my initial proposal to define a ldap attribute with boolean syntax. Most attributes have default values and need not to be present example: pkcs11extractable: true pkcs11derive: false pkcs11verify: true 2] one ldap attribute with pkcs11 attributes as values During the review Simo suggested to have a single attribute (or a few of them, key,cert,...) and for each pkcs11 attribute with value true add it as a value example: pkcs11keyFlags: CKA_EXTRACTABLE pkcs11keyFlags: CKA_VERIFY Pros Cons pro 1] : * direct mapping of pkcs11attributes * required or allowed attributes are defined in an objectclass con 1]: * huge number of schema attributes, which will probably not be needed pro 2]: * smaller schema definition * possible to add new attributes/flags without extending the schema con 2]: * no input validation, application could set undefined flags * since presence of a flag means TRUE, and absence FALSE all default true values need to be present An other question was what should be the prefix for the ldap attribute names, the initial proposal was ipapkcs11, which was considered too ipa specific, so the next was pkcs11, where there are now concerns that this might be too ambitious pretending this is somehow official pkcs11. So there are proposals of p11,pk11,c11 which also are used already by others (nss,p11-glue) so any good ideas are welcome ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0507 Allow anonymous read access to containers
On 04/03/2014 03:28 PM, Simo Sorce wrote: On Thu, 2014-04-03 at 15:19 +0200, Petr Viktorin wrote: On 04/03/2014 02:53 PM, Simo Sorce wrote: On Thu, 2014-04-03 at 13:34 +0200, Petr Viktorin wrote: Hello, This adds anonymous read access to containers, as discussed in this thread: https://www.redhat.com/archives/freeipa-devel/2014-March/msg00442.html Additionally access is granted for $SUFFIX itself with targetfilter (objectclass=domain), and attributes objectclass, dc, info, nisDomain, associatedDomain. These are raw ACIs, not permission-based ones. Why is this not set in default-aci.ldif as well ? Simo. Because we don't want to duplicate information. So are we removing default-aci.ldif completely ? I think we already mentioned this, but I can hardly recall the discussion, sorry. Simo. Sorry for the brief answer, I was just leaving for the day. Storing the data in both the LDIFs and update files is unnecessary, and the two files will get out of sync so one would need to look at both of them to get the full picture anyway. So now the plan is to put new data only in update files (except for schema which has a special LDIF-based updater). default-aci.ldif might end up being removed completely but it doesn't really bring us anything except being cleaner, so it's not a priority. I found the discussion: http://www.redhat.com/archives/freeipa-devel/2013-September/msg00106.html; the relevant part is: Rob: The plan at the time updates were added was to move absolutely everything out of ldif and into updates. It just never happened. Petr: Good to know. Is it still the plan? Do I only need to change the update files? Rob: It would be my preference. It goes beyond only changing one set of files. The existing ldif that duplicate things need to be deprecated. We can't get to a zero-ldif install, but it can be reduced significantly. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH][RFC] 9 CA-less tests generate failure
Hi,
CA-less test suite always generate failures when installing revoked
certificates. This is a known issue, described in
https://fedorahosted.org/freeipa/ticket/4270 , this fix skips these
tests, outputting a notification message for the ticket.
Now it outputs this:
[amisnyov@host freeipa]$ ./make-test
ipatests/test_integration/test_caless.py:TestServerInstall.test_revoked_http
/usr/bin/nosetests -v --with-doctest --doctest-tests --exclude=plugins
ipatests/test_integration/test_caless.py:TestServerInstall.test_revoked_http
IPA server install with revoked HTTP certificate ... SKIP: Known
CA-less installation defect, see
https://fedorahosted.org/freeipa/ticket/4270
--
Ran 1 test in 1020.253s
OK (SKIP=1)
==
passed under '/usr/bin/python2.7'
** pass **
https://fedorahosted.org/freeipa/ticket/4271
There could be another possible solution, I could write a nose plugin to
enable raising warnings instead of skipping a test. This could be
achieved by adding a @unittest.expectedFailure for a specific test, so
if it fails, it counts as an error/warning. There is a poc in a nose
ticket located in
http://code.google.com/p/python-nose/issues/detail?id=428 , not sure
how much time it takes to implement it as a plugin, or is it even
worth, because if this is implemented, we could also use this feature
when eg. DNS is not configured, this is why RFC.
Thanks
AdamFrom e3ccd04b19675dfe1ecdcffdcf229d1f54d4d9e2 Mon Sep 17 00:00:00 2001
From: Adam Misnyovszki amisn...@redhat.com
Date: Fri, 4 Apr 2014 10:41:51 +0200
Subject: [PATCH] CA-less tests generate failure
CA-less test suite always generate failures when installing
revoked certificates. This is a known issue, described in
https://fedorahosted.org/freeipa/ticket/4270 , this fix skips
these tests, outputting a warning for the later ticket.
https://fedorahosted.org/freeipa/ticket/4271
---
ipatests/test_integration/test_caless.py | 37
1 file changed, 37 insertions(+)
diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py
index 87c523a43ed64dbf0f32fb8e0d594c8af60ce8fc..d20a8511c3741ff730e6fad13fe3d69c391cd31a 100644
--- a/ipatests/test_integration/test_caless.py
+++ b/ipatests/test_integration/test_caless.py
@@ -23,6 +23,7 @@ import shutil
import base64
import glob
import contextlib
+import nose
from ipalib import x509
from ipapython import ipautil
@@ -557,6 +558,12 @@ class TestServerInstall(CALessBase):
result = self.install_server(http_pkcs12='http.p12',
dirsrv_pkcs12='dirsrv.p12')
+
+if result.returncode == 0:
+raise nose.SkipTest(
+Known CA-less installation defect, see
++ https://fedorahosted.org/freeipa/ticket/4270;)
+
assert result.returncode 0
def test_revoked_ds(self):
@@ -569,6 +576,12 @@ class TestServerInstall(CALessBase):
result = self.install_server(http_pkcs12='http.p12',
dirsrv_pkcs12='dirsrv.p12')
+
+if result.returncode == 0:
+raise nose.SkipTest(
+Known CA-less installation defect, see
++ https://fedorahosted.org/freeipa/ticket/4270;)
+
assert result.returncode 0
def test_http_intermediate_ca(self):
@@ -917,6 +930,12 @@ class TestReplicaInstall(CALessBase):
result = self.prepare_replica(http_pkcs12='http.p12',
dirsrv_pkcs12='dirsrv.p12')
+
+if result.returncode == 0:
+raise nose.SkipTest(
+Known CA-less installation defect, see
++ https://fedorahosted.org/freeipa/ticket/4270;)
+
assert result.returncode 0
def test_revoked_ds(self):
@@ -927,6 +946,12 @@ class TestReplicaInstall(CALessBase):
result = self.prepare_replica(http_pkcs12='http.p12',
dirsrv_pkcs12='dirsrv.p12')
+
+if result.returncode == 0:
+raise nose.SkipTest(
+Known CA-less installation defect, see
++ https://fedorahosted.org/freeipa/ticket/4270;)
+
assert result.returncode 0
def test_http_intermediate_ca(self):
@@ -1336,12 +1361,24 @@ class TestCertinstall(CALessBase):
Install new revoked HTTP certificate
result = self.certinstall('w', 'ca1/server-revoked')
+
+if result.returncode == 0:
+raise nose.SkipTest(
+Known CA-less installation defect, see
++ https://fedorahosted.org/freeipa/ticket/4270;)
+
assert result.returncode 0
def test_revoked_ds(self):
Install new revoked DS certificate
Re: [Freeipa-devel] [PATCH 0017] Add wait_for_dns option to default.conf
On 04/02/2014 02:38 PM, Petr Spacek wrote: On 2.4.2014 14:36, Petr Spacek wrote: Hello, Add wait_for_dns option to default.conf. This option makes record changes in DNS tree synchronous. IPA calls will wait until new data are visible over DNS protocol or until timeout. It is intended only for testing. It should prevent tests from failing if there is bigger delay between changes in LDAP and DNS. My personal recommendation is to use value 5 (for testing!). Ah, my hands were faster than head :-) This patch was supersedes patch my patch 0015 and should apply to vanilla master (at the moment). Looks good to me. ACK, pushed to master: 34fc447c00189d53ccf44184cfd5ed48cde6bf86 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 163-166] Various ipatests fixes
On 04/03/2014 12:55 PM, Petr Viktorin wrote: On 04/03/2014 12:42 PM, Tomas Babej wrote: Hi. these fix the following: * not properly removed PKI instance on IPA uninstall * improper usage of external hostname of AD subdomain in the legacy client tests * relax regex checks in legacy client tests * put 2 seconds of sleep after restart of SSSD when clearing the cache I will take the review. ACK, thanks, pushed to: master: 50a6316d16f3cb9fcdcab03a1f205a678e2fb154 ipa-3-3: 075f9e59a859b2d53dead6b4610a417a4277fdcd -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] [DOC] Add note about additional nameservers in resolv.conf
On 03/29/2014 12:22 AM, Gabe Alford wrote: Changed 127.0.0.1 to 192.0.2.1 On Fri, Mar 28, 2014 at 1:38 AM, Petr Spacek pspa...@redhat.com mailto:pspa...@redhat.com wrote: On 28.3.2014 02:09, Gabe Alford wrote: I believe that Martin is right about the server installer no longer putting 127.0.0.1 in the resolv.conf. Here is a mod patch to address Martin's concern if the note needs to be changed to show a real IP address. Okay, that is new for me :-) Conditional ACK. Please change the IP address before push to something from: http://tools.ietf.org/html/__rfc5737#section-3 http://tools.ietf.org/html/rfc5737#section-3 192.0.2.1 sounds like a good candidate. Thanks! Petr^2 Spacek Pushed to docs master: e0c2758f20b4fd73715e320dbfa09c202ad5b142 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On 3.4.2014 15:35, Jan Cholasta wrote:
On 2.4.2014 14:07, Martin Basti wrote:
Patch 30:
2)
+if isinstance(labels, str):
+if not labels:
+raise ValueError('empty string')
...
+elif isinstance(labels, unicode):
+if not labels:
+raise ValueError('empty string')
It might be nicer to:
+if isinstance(labels, basestring) and not labels:
+raise ValueError('empty string')
+
+if isinstance(labels, str):
...
+elif isinstance(labels, unicode):
Is it expected that you can't create root name?
I would like to imitate dns-python semantics as much as possible.
In [2]: dns.name.from_text()
Out[2]: DNS name .
In [4]: dns.name.Name([])
Out[4]: DNS name @
In [5]: dns.name.Name([])
Out[5]: DNS name .
I would like to see more descriptive error texts if you insist on current
semantics.
4)
+@staticmethod
+def get_root():
+return DNSName(dns.name.root)
+
+@staticmethod
+def get_origin_sign():
+return DNSName(u'@')
+
+@staticmethod
+def get_rev_zone():
+return DNSName(u'in-addr.arpa.')
+
+@staticmethod
+def get_ip6_rev_zone():
+return DNSName(u'ip6.arpa.')
I think you should either drop the get_ prefix from the name, or (even
better) make these global constants.
I would shorten origin_sign to just sign.
Sign of what? Decay? :-) I don't think that sign is descriptive enough, I
would personally stick with origin_sign.
Can you please use tuples of str objects (i.e. what dns.name.Name uses
internally) instead of unicode objects for the initialization? I think it
should be the preferred style of initializing DNSName objects (DN objects do
the same).
Please don't forget to consult dns-python3 and try to make migration from
dns-python to dns-python3 easy.
6)
+def ToASCII(self, omit_final_dot=False):
+return super(DNSName,
self).to_text(omit_final_dot=omit_final_dot).decode('ascii')
+
+def ToUnicode(self, omit_final_dot=False):
+return super(DNSName,
self).to_unicode(omit_final_dot=omit_final_dot).decode('utf-8')
What was the reason for the unusual naming again? I would prefer PEP-8
compatible names (e.g. to_ascii and to_unicode), but if the current names
absolutely have to stay, please add a comment with explanation.
I don't like the omit_final_dot flag. IMHO it should be dropped and whether
the result includes a final dot or not should depend solely on whether the
name is absolute or relative. You can still use e.g.
name.derelativize(root).ToUnicode() to drop the final dot, which is more
explanatory.
Generally, I agree. We can get rid of the relative/absolute name madness by
using final dot everywhere.
8)
+def is_ip_reverse(self):
Please use is_ip4_reverse() instead. That name will always remind developer
not to forgot to IPv6 :-)
+if self.is_subdomain(self.get_rev_zone()):
+return True
+return False
Patch 31:
2b)
+except dns.name.NameTooLong:
+raise ValueError(_('domain name cannot be longer than 255
characters'))
Personally, I would say '255 bytes' instead of '255 characters'. Characters
are tricky when IDN is involved etc.
Patch freeipa-mbasti-0036-DNSName-conversion-in-ipaldap.patch:
@@ -255,6 +256,10 @@ class IPASimpleLDAPObject(object):
'managedtemplate': DN,
'managedbase': DN,
'originscope': DN,
+'idnsname':DNSName,
+'idnssoamname':DNSName,
+'idnssoarname':DNSName,
+'dnszoneidnsname': DNSName,
})
Maybe you can add following attributes too:
CNAMERecord
DNAMERecord
MDRecord
NSRecord
PTRRecord
Patch 38:
1)
+_dns_zone_record = DNSName(u'@')
You know you already defined a constant for this in patch 30, right?
It seems that both constants are unnecessary because IMHO
DNSName(u'@')
is more readable and you don't need to dig into code to find out what the
cryptic name means.
4)
+def _hostname_validator_idn(ugettext, value):
+assert isinstance(value, DNSName)
+
+req_len = 2
+if value.is_absolute():
+req_len = 3
+if len(value.labels) req_len:
+return _('invalid domain-name: not fully qualified')
This test is not correct because FQDN = an absolute name.
This code tries to guess if the name is FQDN or not but we can directly use
is_absolute() test.
4b)
def is_forward_record(zone, str_address):
addr = netaddr.IPAddress(str_address)
@@ -461,6 +444,7 @@ def is_forward_record(zone, str_address):
def add_forward_record(zone, name, str_address):
addr = netaddr.IPAddress(str_address)
+
try:
if addr.version == 4:
api.Command['dnsrecord_add'](zone, name, arecord=str_address)
Personally, I think that 'forward' is confusing (in this context). Could you
rename *_forward_record() functions to *_ipaddr_record(), please? (Maybe in a
separate patch...)
4c)
def
Re: [Freeipa-devel] questions regarding ldap schema for pkcs11
On 4.4.2014 10:20, Ludwig Krispenz wrote: In the review discussion for the ldap schema for pkcs11 there was one topic, which we wanted to get the opinion from a broader audience before making a final decision. I'll add my opinion for the record: In pkcs11 there are many boolean attributes, like CKA_EXTRACTABLE, CKA_DERIVE, CKA_VERIFY and there are two suggestions how to represent them in ldap. 1] one ldap attribute for each pkcs11 attribute. This was my initial proposal to define a ldap attribute with boolean syntax. Most attributes have default values and need not to be present example: pkcs11extractable: true pkcs11derive: false pkcs11verify: true 2] one ldap attribute with pkcs11 attributes as values During the review Simo suggested to have a single attribute (or a few of them, key,cert,...) and for each pkcs11 attribute with value true add it as a value example: pkcs11keyFlags: CKA_EXTRACTABLE pkcs11keyFlags: CKA_VERIFY Pros Cons pro 1] : one ldap attribute for each pkcs11 attribute. * direct mapping of pkcs11attributes * required or allowed attributes are defined in an objectclass con 1]: * huge number of schema attributes, which will probably not be needed I don't think it is a problem. We have *huge* schema full of almost never-used attributes. Look at printerAbstract objectClass ... pro 2]: one ldap attribute with pkcs11 attributes as values * smaller schema definition IPA schema + all the RFCs created a huge pile of schema definitions already and 389 can cope with it. (We are speaking about adding tens of attributes, not hundreds or thousands!) * possible to add new attributes/flags without extending the schema Schema change is a little problem in comparison with updating clients (to get any value from the new flag). Note that we are talking about booleans defined by PKCS#11 standard so we can't add any boolean anyway. IMHO any IPA-specific booleans should go to a separate object class to separate them from pure PKCS#11 schema. con 2]: * no input validation, application could set undefined flags * since presence of a flag means TRUE, and absence FALSE all default true values need to be present To conclude it - I like approach [1]: One ldap attribute for each pkcs11 attribute. -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0017] Add wait_for_dns option to default.conf
On 04/04/2014 11:57 AM, Petr Viktorin wrote: On 04/02/2014 02:38 PM, Petr Spacek wrote: On 2.4.2014 14:36, Petr Spacek wrote: Hello, Add wait_for_dns option to default.conf. This option makes record changes in DNS tree synchronous. IPA calls will wait until new data are visible over DNS protocol or until timeout. It is intended only for testing. It should prevent tests from failing if there is bigger delay between changes in LDAP and DNS. My personal recommendation is to use value 5 (for testing!). Ah, my hands were faster than head :-) This patch was supersedes patch my patch 0015 and should apply to vanilla master (at the moment). Looks good to me. ACK, pushed to master: 34fc447c00189d53ccf44184cfd5ed48cde6bf86 Petr, can we now enable wait_for_dns for our tests then? Hopefully it will solve the unit test failures we saw. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0017] Add wait_for_dns option to default.conf
On 04/04/2014 01:50 PM, Martin Kosek wrote: On 04/04/2014 11:57 AM, Petr Viktorin wrote: On 04/02/2014 02:38 PM, Petr Spacek wrote: On 2.4.2014 14:36, Petr Spacek wrote: Hello, Add wait_for_dns option to default.conf. This option makes record changes in DNS tree synchronous. IPA calls will wait until new data are visible over DNS protocol or until timeout. It is intended only for testing. It should prevent tests from failing if there is bigger delay between changes in LDAP and DNS. My personal recommendation is to use value 5 (for testing!). Ah, my hands were faster than head :-) This patch was supersedes patch my patch 0015 and should apply to vanilla master (at the moment). Looks good to me. ACK, pushed to master: 34fc447c00189d53ccf44184cfd5ed48cde6bf86 Petr, can we now enable wait_for_dns for our tests then? Hopefully it will solve the unit test failures we saw. Martin Here it is: https://github.com/encukou/freeipa-ci/commit/d36aa4fb54e4b56c040ecbe886a1156c18863023 I generally wait for a successful build before pushing to the ci config repo. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0507 Allow anonymous read access to containers
On Fri, 2014-04-04 at 10:54 +0200, Petr Viktorin wrote: On 04/03/2014 03:28 PM, Simo Sorce wrote: On Thu, 2014-04-03 at 15:19 +0200, Petr Viktorin wrote: On 04/03/2014 02:53 PM, Simo Sorce wrote: On Thu, 2014-04-03 at 13:34 +0200, Petr Viktorin wrote: Hello, This adds anonymous read access to containers, as discussed in this thread: https://www.redhat.com/archives/freeipa-devel/2014-March/msg00442.html Additionally access is granted for $SUFFIX itself with targetfilter (objectclass=domain), and attributes objectclass, dc, info, nisDomain, associatedDomain. These are raw ACIs, not permission-based ones. Why is this not set in default-aci.ldif as well ? Simo. Because we don't want to duplicate information. So are we removing default-aci.ldif completely ? I think we already mentioned this, but I can hardly recall the discussion, sorry. Simo. Sorry for the brief answer, I was just leaving for the day. Storing the data in both the LDIFs and update files is unnecessary, and the two files will get out of sync so one would need to look at both of them to get the full picture anyway. So now the plan is to put new data only in update files (except for schema which has a special LDIF-based updater). default-aci.ldif might end up being removed completely but it doesn't really bring us anything except being cleaner, so it's not a priority. I found the discussion: http://www.redhat.com/archives/freeipa-devel/2013-September/msg00106.html; the relevant part is: Rob: The plan at the time updates were added was to move absolutely everything out of ldif and into updates. It just never happened. Petr: Good to know. Is it still the plan? Do I only need to change the update files? Rob: It would be my preference. It goes beyond only changing one set of files. The existing ldif that duplicate things need to be deprecated. We can't get to a zero-ldif install, but it can be reduced significantly. Ok however at the moment this is confusing for someone searching the code. Can we schedule an effort to clean up and remove as many ldif files as possible? Also do we need to call updates earlier if we do this ? Should we add warnings in the remaining ldif files about not adding content there unless explicitly required in early installation steps and redirect people to the update files ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On Thu, 2014-04-03 at 15:35 +0200, Jan Cholasta wrote:
On 2.4.2014 14:07, Martin Basti wrote:
Helo list,
this patchset allows to use internationalized domian in DNS plugin.
- dns names are stored in ACE form(punycoded) in LDAP
- raw option shows dns data in ACE form, otherwise dns names are
converted to unicode
- plugin allow all characters in domain name, which are valid by IDN
RFCs (almost everything including non-printable), should be validation
more restrictive? (there is bug in dnspython with special characters,
will be fixed soon)
- TODO update WebUI to support DNSName objects
Required patches:
freeipa-jcholast-255-Allow-primary-keys-to-use-different-type-than-unicod.patch
freeipa-jcholast-256-Support-API-version-specific-RPC-marshalling.patch
freeipa-jcholast-257-Replace-get_syntax-method-of-IPASimpleObject-with-ne.patch
freeipa-jcholast-258-Use-raw-attribute-values-in-command-result-when-raw-.patch
freeipa-jcholast-259-Keep-original-name-when-setting-attribute-in-LDAPEnt.patch
Patches attached.
First batch of comments, so far I have only read the code/patches,
without doing actual testing.
Patch 30:
1)
It might make sense to put all of this into a new module (e.g.
dnsutil.py) rather than ipautil.
I will move it
2)
+if isinstance(labels, str):
+if not labels:
+raise ValueError('empty string')
...
+elif isinstance(labels, unicode):
+if not labels:
+raise ValueError('empty string')
It might be nicer to:
+if isinstance(labels, basestring) and not labels:
+raise ValueError('empty string')
+
+if isinstance(labels, str):
...
+elif isinstance(labels, unicode):
Ok I will use basestring
3)
+def __nonzero__(self):
+return True
It would be nice to include a comment about why DNSName always evaluates
to True (mention @).
I will add comment
4)
+@staticmethod
+def get_root():
+return DNSName(dns.name.root)
+
+@staticmethod
+def get_origin_sign():
+return DNSName(u'@')
+
+@staticmethod
+def get_rev_zone():
+return DNSName(u'in-addr.arpa.')
+
+@staticmethod
+def get_ip6_rev_zone():
+return DNSName(u'ip6.arpa.')
I think you should either drop the get_ prefix from the name, or (even
better) make these global constants.
I would shorten origin_sign to just sign.
IMHO only sign is not enough.
Can you please use tuples of str objects (i.e. what dns.name.Name uses
internally) instead of unicode objects for the initialization? I think
it should be the preferred style of initializing DNSName objects (DN
objects do the same).
I will fix it.
5)
+def __str__(self):
+return super(DNSName, self).to_text()
You don't need to use super here.
I will fix it.
6)
+def ToASCII(self, omit_final_dot=False):
+return super(DNSName,
self).to_text(omit_final_dot=omit_final_dot).decode('ascii')
+
+def ToUnicode(self, omit_final_dot=False):
+return super(DNSName,
self).to_unicode(omit_final_dot=omit_final_dot).decode('utf-8')
What was the reason for the unusual naming again? I would prefer PEP-8
compatible names (e.g. to_ascii and to_unicode), but if the current
names absolutely have to stay, please add a comment with explanation.
I used .ToUnicode .ToASCII because, python standard library uses this
naming(https://docs.python.org/2/library/codecs.html#module-encodings.idna) and
also RFC3490 section 4 specifiy it in that way.
I don't like the omit_final_dot flag. IMHO it should be dropped and
whether the result includes a final dot or not should depend solely on
whether the name is absolute or relative. You can still use e.g.
name.derelativize(root).ToUnicode() to drop the final dot, which is
more explanatory.
In ToUnicode, the call to dns.name.Name.to_unicode already returns a
unicode object, no need to call decode on it.
I will remove the flag
7)
+def concatenate(self, other):
+return DNSName(super(DNSName, self).concatenate(other).labels)
+
+def relativize(self, origin):
+return DNSName(super(DNSName, self).relativize(origin).labels)
+
+def derelativize(self, origin):
+return DNSName(super(DNSName, self).derelativize(origin).labels)
+
+def choose_relativity(self, origin=None, relativize=True):
+return DNSName(super(DNSName,
self).choose_relativity(origin=origin, relativize=relativize).labels)
Why use .labels here? The DNSName constructor knows how to deal with
dns.name.Name objects, right?
Right, I will fix it.
8)
+def is_ip_reverse(self):
+if self.is_subdomain(self.get_rev_zone()):
+return True
+return False
+
+def is_ip6_reverse(self):
+if self.is_subdomain(self.get_ip6_rev_zone()):
+return
Re: [Freeipa-devel] Ipa-server-install Firewall Support
On Fri, 2014-04-04 at 09:59 +0200, Petr Spacek wrote:
On 4.4.2014 09:17, Martin Kosek wrote:
On 04/04/2014 09:04 AM, Justin Brown wrote:
I would actually do it the opposite way and open the ports after the
FreeIPA server is fully configured. After all, I do not think we want to
open the ports when the server is just half-configured and for example
some ACIs are missing.
My thinking was that nothing would be listening on these ports if the
install doesn't succeed, but there's really necessity to modify the
firewall configuration early. (All of the internal install
communication will be over a local interface (to netfilter) and
unblock anyways. I don't have any problem in delaying firewall
configuration to the end of install.
If ipa-server-install does succeed without configuring the firewalld, then
we
will indeed have no other option than to do it early.
I am thinking that we may want to put all the firewalld configuration in
ipaserver/install/firewalldinstance.py,
and then make the firewalld configuration the actual step of the
installation.
Something like:
...
Configuring Firewall (firewalld)
[1/2]: looking up the right zone
[2/2]: allowing ports
Done configuring Firewall (firewalld).
...
The Service class derived object can be really simple, we would just reuse
the
functionality it already has + let us properly hook into it in
ipa-{server,replica}-install and the uninstallation.
It would also make it easier to split this functionality to
freeipa-server-firewalld if we chose to in a future.
In general I agree with the idea, thank you Justin for working on that!
I would like to emphasis the necessity to work without NetworkManager and
FirewallD. New dependencies make Debian folks unhappy ...
On the other hand, it is perfectly fine to skip firewall configuration if
NM/FirewallD/DBus is not available.
Have a nice day!
Should be easy, probe for the dbus firewalld service and just skip (not
error out) if it is not there.
Set a variable in that case that will cause the installer to throw the
classic banner we have now which warns you about what ports need to be
opened at the end of the install.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] questions regarding ldap schema for pkcs11
On Fri, 2014-04-04 at 13:19 +0200, Petr Spacek wrote: On 4.4.2014 10:20, Ludwig Krispenz wrote: In the review discussion for the ldap schema for pkcs11 there was one topic, which we wanted to get the opinion from a broader audience before making a final decision. I'll add my opinion for the record: In pkcs11 there are many boolean attributes, like CKA_EXTRACTABLE, CKA_DERIVE, CKA_VERIFY and there are two suggestions how to represent them in ldap. 1] one ldap attribute for each pkcs11 attribute. This was my initial proposal to define a ldap attribute with boolean syntax. Most attributes have default values and need not to be present example: pkcs11extractable: true pkcs11derive: false pkcs11verify: true 2] one ldap attribute with pkcs11 attributes as values During the review Simo suggested to have a single attribute (or a few of them, key,cert,...) and for each pkcs11 attribute with value true add it as a value example: pkcs11keyFlags: CKA_EXTRACTABLE pkcs11keyFlags: CKA_VERIFY Pros Cons pro 1] : one ldap attribute for each pkcs11 attribute. * direct mapping of pkcs11attributes * required or allowed attributes are defined in an objectclass con 1]: * huge number of schema attributes, which will probably not be needed I don't think it is a problem. We have *huge* schema full of almost never-used attributes. Look at printerAbstract objectClass ... pro 2]: one ldap attribute with pkcs11 attributes as values * smaller schema definition IPA schema + all the RFCs created a huge pile of schema definitions already and 389 can cope with it. (We are speaking about adding tens of attributes, not hundreds or thousands!) * possible to add new attributes/flags without extending the schema Schema change is a little problem in comparison with updating clients (to get any value from the new flag). Note that we are talking about booleans defined by PKCS#11 standard so we can't add any boolean anyway. IMHO any IPA-specific booleans should go to a separate object class to separate them from pure PKCS#11 schema. con 2]: * no input validation, application could set undefined flags * since presence of a flag means TRUE, and absence FALSE all default true values need to be present To conclude it - I like approach [1]: One ldap attribute for each pkcs11 attribute. After much consideration I think one attribute per boolean is ok, I think the most convincing argument came from Honza when he said sometimes the default may depend on additional factors not explicit in the object, in that case setting or not setting a string would always be wrong and we would need to end up with additional definitions like: CKA_VERIFY_TRUE/CKA_VERIFY_FALSE as opposed to them missing which could indicate default (or we end up adding also CKA_VERIFY_DEFAULT ...). Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On Fri, 2014-04-04 at 12:59 +0200, Petr Spacek wrote:
On 3.4.2014 15:35, Jan Cholasta wrote:
On 2.4.2014 14:07, Martin Basti wrote:
Patch 30:
2)
+if isinstance(labels, str):
+if not labels:
+raise ValueError('empty string')
...
+elif isinstance(labels, unicode):
+if not labels:
+raise ValueError('empty string')
It might be nicer to:
+if isinstance(labels, basestring) and not labels:
+raise ValueError('empty string')
+
+if isinstance(labels, str):
...
+elif isinstance(labels, unicode):
Is it expected that you can't create root name?
I would like to imitate dns-python semantics as much as possible.
In [2]: dns.name.from_text()
Out[2]: DNS name .
In [4]: dns.name.Name([])
Out[4]: DNS name @
In [5]: dns.name.Name([])
Out[5]: DNS name .
Cos we need to keep if user added domain with/without zone, we use
origin=None, and there is allowed to add only '.' as domain name = root
In [3]: a = dns.name.from_unicode(u'.', origin=None)
In [4]: a.labels
Out[4]: ('',)
That is more clearly to user add . as root domain, instead of empty
string
I would like to see more descriptive error texts if you insist on current
semantics.
4)
+@staticmethod
+def get_root():
+return DNSName(dns.name.root)
+
+@staticmethod
+def get_origin_sign():
+return DNSName(u'@')
+
+@staticmethod
+def get_rev_zone():
+return DNSName(u'in-addr.arpa.')
+
+@staticmethod
+def get_ip6_rev_zone():
+return DNSName(u'ip6.arpa.')
I think you should either drop the get_ prefix from the name, or (even
better) make these global constants.
I would shorten origin_sign to just sign.
Sign of what? Decay? :-) I don't think that sign is descriptive enough, I
would personally stick with origin_sign.
I agree with origin_sing, or zone_record, or origin, not sign.
Can you please use tuples of str objects (i.e. what dns.name.Name uses
internally) instead of unicode objects for the initialization? I think it
should be the preferred style of initializing DNSName objects (DN objects do
the same).
Please don't forget to consult dns-python3 and try to make migration from
dns-python to dns-python3 easy.
I will take look at dns-python3.
6)
+def ToASCII(self, omit_final_dot=False):
+return super(DNSName,
self).to_text(omit_final_dot=omit_final_dot).decode('ascii')
+
+def ToUnicode(self, omit_final_dot=False):
+return super(DNSName,
self).to_unicode(omit_final_dot=omit_final_dot).decode('utf-8')
What was the reason for the unusual naming again? I would prefer PEP-8
compatible names (e.g. to_ascii and to_unicode), but if the current
names
absolutely have to stay, please add a comment with explanation.
I don't like the omit_final_dot flag. IMHO it should be dropped and
whether
the result includes a final dot or not should depend solely on whether the
name is absolute or relative. You can still use e.g.
name.derelativize(root).ToUnicode() to drop the final dot, which is more
explanatory.
Generally, I agree. We can get rid of the relative/absolute name madness by
using final dot everywhere.
Ok I will remove it.
8)
+def is_ip_reverse(self):
Please use is_ip4_reverse() instead. That name will always remind developer
not to forgot to IPv6 :-)
As you wish.
+if self.is_subdomain(self.get_rev_zone()):
+return True
+return False
Patch 31:
2b)
+except dns.name.NameTooLong:
+raise ValueError(_('domain name cannot be longer than
255
characters'))
Personally, I would say '255 bytes' instead of '255 characters'. Characters
are tricky when IDN is involved etc.
Okay, bytes then. I will reword it.
Patch freeipa-mbasti-0036-DNSName-conversion-in-ipaldap.patch:
@@ -255,6 +256,10 @@ class IPASimpleLDAPObject(object):
'managedtemplate': DN,
'managedbase': DN,
'originscope': DN,
+'idnsname':DNSName,
+'idnssoamname':DNSName,
+'idnssoarname':DNSName,
+'dnszoneidnsname': DNSName,
})
Maybe you can add following attributes too:
CNAMERecord
DNAMERecord
MDRecord
NSRecord
PTRRecord
I'm not sure with that. Now all record data are returned as string, only
domain names and domain related attributes in zone are returned as
DNSName. It could be mess if half of records be returned as DNSName and
second as string.
Patch 38:
1)
+_dns_zone_record = DNSName(u'@')
You know you already defined a constant for this in patch 30, right?
It seems that both constants are unnecessary because IMHO
DNSName(u'@')
is more readable and you don't need to dig into code to find out what the
cryptic name means.
I like constants :-). I'm
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On Fri, 2014-04-04 at 15:46 +0200, Martin Basti wrote:
On Fri, 2014-04-04 at 12:59 +0200, Petr Spacek wrote:
On 3.4.2014 15:35, Jan Cholasta wrote:
On 2.4.2014 14:07, Martin Basti wrote:
Patch 30:
2)
+if isinstance(labels, str):
+if not labels:
+raise ValueError('empty string')
...
+elif isinstance(labels, unicode):
+if not labels:
+raise ValueError('empty string')
It might be nicer to:
+if isinstance(labels, basestring) and not labels:
+raise ValueError('empty string')
+
+if isinstance(labels, str):
...
+elif isinstance(labels, unicode):
Is it expected that you can't create root name?
I would like to imitate dns-python semantics as much as possible.
In [2]: dns.name.from_text()
Out[2]: DNS name .
In [4]: dns.name.Name([])
Out[4]: DNS name @
In [5]: dns.name.Name([])
Out[5]: DNS name .
Cos we need to keep if user added domain with/without zone, we use
I mean end dot not zone.
origin=None, and there is allowed to add only '.' as domain name = root
In [3]: a = dns.name.from_unicode(u'.', origin=None)
In [4]: a.labels
Out[4]: ('',)
That is more clearly to user add . as root domain, instead of empty
string
I would like to see more descriptive error texts if you insist on current
semantics.
4)
+@staticmethod
+def get_root():
+return DNSName(dns.name.root)
+
+@staticmethod
+def get_origin_sign():
+return DNSName(u'@')
+
+@staticmethod
+def get_rev_zone():
+return DNSName(u'in-addr.arpa.')
+
+@staticmethod
+def get_ip6_rev_zone():
+return DNSName(u'ip6.arpa.')
I think you should either drop the get_ prefix from the name, or (even
better) make these global constants.
I would shorten origin_sign to just sign.
Sign of what? Decay? :-) I don't think that sign is descriptive enough, I
would personally stick with origin_sign.
I agree with origin_sing, or zone_record, or origin, not sign.
Can you please use tuples of str objects (i.e. what dns.name.Name uses
internally) instead of unicode objects for the initialization? I think it
should be the preferred style of initializing DNSName objects (DN objects
do
the same).
Please don't forget to consult dns-python3 and try to make migration from
dns-python to dns-python3 easy.
I will take look at dns-python3.
6)
+def ToASCII(self, omit_final_dot=False):
+return super(DNSName,
self).to_text(omit_final_dot=omit_final_dot).decode('ascii')
+
+def ToUnicode(self, omit_final_dot=False):
+return super(DNSName,
self).to_unicode(omit_final_dot=omit_final_dot).decode('utf-8')
What was the reason for the unusual naming again? I would prefer PEP-8
compatible names (e.g. to_ascii and to_unicode), but if the current
names
absolutely have to stay, please add a comment with explanation.
I don't like the omit_final_dot flag. IMHO it should be dropped and
whether
the result includes a final dot or not should depend solely on whether the
name is absolute or relative. You can still use e.g.
name.derelativize(root).ToUnicode() to drop the final dot, which is more
explanatory.
Generally, I agree. We can get rid of the relative/absolute name madness by
using final dot everywhere.
Ok I will remove it.
8)
+def is_ip_reverse(self):
Please use is_ip4_reverse() instead. That name will always remind developer
not to forgot to IPv6 :-)
As you wish.
+if self.is_subdomain(self.get_rev_zone()):
+return True
+return False
Patch 31:
2b)
+except dns.name.NameTooLong:
+raise ValueError(_('domain name cannot be longer than
255
characters'))
Personally, I would say '255 bytes' instead of '255 characters'. Characters
are tricky when IDN is involved etc.
Okay, bytes then. I will reword it.
Patch freeipa-mbasti-0036-DNSName-conversion-in-ipaldap.patch:
@@ -255,6 +256,10 @@ class IPASimpleLDAPObject(object):
'managedtemplate': DN,
'managedbase': DN,
'originscope': DN,
+'idnsname':DNSName,
+'idnssoamname':DNSName,
+'idnssoarname':DNSName,
+'dnszoneidnsname': DNSName,
})
Maybe you can add following attributes too:
CNAMERecord
DNAMERecord
MDRecord
NSRecord
PTRRecord
I'm not sure with that. Now all record data are returned as string, only
domain names and domain related attributes in zone are returned as
DNSName. It could be mess if half of records be returned as DNSName and
second as string.
Patch 38:
1)
+_dns_zone_record = DNSName(u'@')
You know you
[Freeipa-devel] [PATCH] Add DRM to IPA
This patch adds the capability of installing a Dogtag DRM to an IPA instance. With this patch, when ipa-server-install is run, a Dogtag CA and a Dogtag DRM are created. The DRM shares the same tomcat instance and DS instance as the Dogtag CA. Moreover, the same admin user/agent (and agent cert) can be used for both subsystems. Certmonger is also confgured to monitor the new subsystem certificates. It is also possible to clone the DRM. When the IPA instance is cloned, if --enable-ca and --enable-drm are specified, the DRM is cloned as well. Installing a DRM requires the user to have a Dogtag CA instance. We can look into possibly relaxing that requirement in a later patch. I am still working on patches for a ipa-drm-install script, which would be used to add a DRM to an existing master (that includes a dogtag CA), or an existing clone. Please review, Thanks, Ade From 298aa20b554b5e17a0f7a1d4cf13e246fba9c8dc Mon Sep 17 00:00:00 2001 From: Ade Lee a...@redhat.com Date: Tue, 18 Mar 2014 11:23:30 -0400 Subject: [PATCH] Add a DRM to IPA This patch adds the capability of installing a Dogtag DRM to an IPA instance. With this patch, when ipa-server-install is run, a Dogtag CA and a Dogtag DRM are created. The DRM shares the same tomcat instance and DS instance as the Dogtag CA. Moreover, the same admin user/agent (and agent cert) can be used for both subsystems. Certmonger is also confgured to monitor the new subsystem certificates. It is also possible to clone the DRM. When the IPA instance is cloned, if --enable-ca and --enable-drm are specified, the DRM is cloned as well. Installing a DRM requires the user to have a Dogtag CA instance. We can look into possibly relaxing that requirement in a later patch. I am still working on patches for a ipa-drm-install script, which would be used to add a DRM to an existing master (that includes a dogtag CA), or an existing clone. --- install/conf/ipa-pki-proxy.conf | 4 +- install/tools/ipa-replica-install| 29 +- install/tools/ipa-server-install | 36 +- ipapython/dogtag.py | 1 + ipaserver/install/cainstance.py | 4 +- ipaserver/install/drminstance.py | 642 +++ ipaserver/install/installutils.py| 17 + ipaserver/install/ipa_replica_prepare.py | 1 + 8 files changed, 722 insertions(+), 12 deletions(-) create mode 100644 ipaserver/install/drminstance.py diff --git a/install/conf/ipa-pki-proxy.conf b/install/conf/ipa-pki-proxy.conf index 224cdd45b5b5f72671a179570fd15772fe8cfaab..9a6345898bfa017da03bf108a82c3639edbb0470 100644 --- a/install/conf/ipa-pki-proxy.conf +++ b/install/conf/ipa-pki-proxy.conf @@ -11,7 +11,7 @@ ProxyRequests Off /LocationMatch # matches for admin port and installer -LocationMatch ^/ca/admin/ca/getCertChain|^/ca/admin/ca/getConfigEntries|^/ca/admin/ca/getCookie|^/ca/admin/ca/getStatus|^/ca/admin/ca/securityDomainLogin|^/ca/admin/ca/getDomainXML|^/ca/rest/installer/installToken|^/ca/admin/ca/updateNumberRange|^/ca/rest/securityDomain/domainInfo|^/ca/rest/account/login|^/ca/admin/ca/tokenAuthenticate|^/ca/admin/ca/updateNumberRange|^/ca/admin/ca/updateDomainXML|^/ca/rest/account/logout|^/ca/rest/securityDomain/installToken +LocationMatch ^/ca/admin/ca/getCertChain|^/ca/admin/ca/getConfigEntries|^/ca/admin/ca/getCookie|^/ca/admin/ca/getStatus|^/ca/admin/ca/securityDomainLogin|^/ca/admin/ca/getDomainXML|^/ca/rest/installer/installToken|^/ca/admin/ca/updateNumberRange|^/ca/rest/securityDomain/domainInfo|^/ca/rest/account/login|^/ca/admin/ca/tokenAuthenticate|^/ca/admin/ca/updateNumberRange|^/ca/admin/ca/updateDomainXML|^/ca/rest/account/logout|^/ca/rest/securityDomain/installToken|^/ca/admin/ca/updateConnector|^/ca/admin/ca/getSubsystemCert|^/kra/admin/kra/updateNumberRange|^/kra/admin/kra/getConfigEntries NSSOptions +StdEnvVars +ExportCertData +StrictRequire +OptRenegotiate NSSVerifyClient none ProxyPassMatch ajp://localhost:$DOGTAG_PORT @@ -19,7 +19,7 @@ ProxyRequests Off /LocationMatch # matches for agent port and eeca port -LocationMatch ^/ca/agent/ca/displayBySerial|^/ca/agent/ca/doRevoke|^/ca/agent/ca/doUnrevoke|^/ca/agent/ca/updateDomainXML|^/ca/eeca/ca/profileSubmitSSLClient +LocationMatch ^/ca/agent/ca/displayBySerial|^/ca/agent/ca/doRevoke|^/ca/agent/ca/doUnrevoke|^/ca/agent/ca/updateDomainXML|^/ca/eeca/ca/profileSubmitSSLClient|^/kra/agent/kra/connector NSSOptions +StdEnvVars +ExportCertData +StrictRequire +OptRenegotiate NSSVerifyClient require ProxyPassMatch ajp://localhost:$DOGTAG_PORT diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install index f5e7197b5c2f03b0b070a33761debcc07a9c..70a9cb21bd48a1fbcd56c30e1f4089172f639f90 100755 --- a/install/tools/ipa-replica-install +++ b/install/tools/ipa-replica-install @@ -38,9 +38,10 @@ from ipaserver.install import
