Re: [Freeipa-devel] [PATCH] 0046 Create server certs with DNS altname
On Tue, Jul 19, 2016 at 02:21:05PM +0200, Martin Basti wrote: > > > On 01.07.2016 13:26, Petr Spacek wrote: > > On 20.1.2016 05:04, Fraser Tweedale wrote: > > > On Tue, Dec 08, 2015 at 07:06:39PM +1000, Fraser Tweedale wrote: > > > > On Mon, Dec 07, 2015 at 05:50:05PM -0500, Rob Crittenden wrote: > > > > > Fraser Tweedale wrote: > > > > > > On Mon, Dec 07, 2015 at 01:53:15PM +0100, Martin Kosek wrote: > > > > > > > On 12/07/2015 06:26 AM, Fraser Tweedale wrote: > > > > > > > > The attached patch fixes > > > > > > > > https://fedorahosted.org/freeipa/ticket/4970. > > > > > > > > > > > > > > > > Note that the problem is addressed by adding the appropriate > > > > > > > > request > > > > > > > > extension to the CSR; the fix does not involve changing the > > > > > > > > default > > > > > > > > profile behaviour, which is complicated (see ticket for > > > > > > > > details). > > > > > > > Thanks for the patch! This is something we should really fix, I > > > > > > > already get > > > > > > > warnings in my Python scripts when I hit sites protected by such > > > > > > > HTTPS cert: > > > > > > > > > > > > > > /usr/lib/python2.7/site-packages/requests/packages/urllib3/connection.py:264: > > > > > > > SubjectAltNameWarning: Certificate for > > > > > > > projects.engineering.redhat.com has no > > > > > > > `subjectAltName`, falling back to check for a `commonName` for > > > > > > > now. This > > > > > > > feature is being removed by major browsers and deprecated by RFC > > > > > > > 2818. (See > > > > > > > https://github.com/shazow/urllib3/issues/497 for details.) > > > > > > > > > > > > > > Should we split ticket 4970, for the FreeIPA server part and then > > > > > > > for cert > > > > > > > profile part? As it looks like the FreeIPA server will be fixed > > > > > > > even in FreeIPA > > > > > > > 4.3.x and the other part later. > > > > > > > > > > > > > > How difficult do you see the general FreeIPA Certificate Profile > > > > > > > part of this > > > > > > > request? Is it a too big task to handle in 4.4 time frame? > > > > > > > > > > > > > I will split the ticket and would suggest 4.4 Backlog - it might be > > > > > > doable but is a lower priority than e.g. Sub-CAs. > > > > > If you are going to defer the profile part then you should probably > > > > > update the client to also include a SAN if --request-cert is provided. > > > > > > > > > > rob > > > > > > > > > Yes, good idea. Updated patch attached. > > > > > > > > Cheers, > > > > Fraser > > > Bump, with rebased patch. > > Hi, > > > > this seems to work for Apache on IPA server & client cert. ACK. > Pushed to master: b12db924143cd6828c596c0b8a261325f3f589f3 > > > > > Interestingly enough I found out that Dogtag cert used on port 8443 does not > > have any SAN. > > > > Is it in scope of this ticket? > I will leave the ticket open until this is answered. > It's in scope. Also in scope is to make default profile automatically add SAN dNSName if none is supplied. Thanks, Fraser > Martin^2 > > > > -- > 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 -- 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
[Freeipa-devel] [DESIGN] Text-based rules for CSR autogeneration using Jinja2
Hi,
I have updated the design page
http://www.freeipa.org/page/V4/Automatic_Certificate_Request_Generation/Mapping_Rules
with my plan for implementing user-configurable rules for mapping IPA
data into certificate requests. In brief: we will use Jinja2 for
templating. Data rules (which map individual data items) and syntax
rules (which group them into certificate fields) will both be snippets
of Jinja2 markup. The formatting process will be as follows:
1. Syntax rules will be rendered using Jinja2. Data rules (rule text,
not rendered) will be passed as the datarules attribute.
2. Rendered syntax rules will be processed by the Formatter class for
the selected CSR generation helper (e.g. openssl or certutil). The
formatter combines these partial rules into a full template for the config.
3. The template will be rendered using Jinja2. Relevant data from the
IPA database will be available in the context for this rendering.
4. The final rendered template will be returned to the caller, labeled
with its function (e.g. a command line or a config file).
Are there any comments or objections to this approach? Here's an example
to show what it might look like in practice.
Example data rules:
email={{subject.email}}
O={{config.ipacertificatesubjectbase}}\nCN={{subject.username}}
Example syntax rule:
subjectAltName=@{% section %}{{datarules|join('\n')}}{% endsection %}
Example composed config template:
[ req ]
prompt = no
encrypt_key = no
distinguished_name = {% section %}O={{config.ipacertificatesubjectbase}}
CN={{subject.username}}{% endsection %}
req_extensions = exts
[ exts ]
subjectAltName=@{% section %}email={{subject.email}}{% endsection %}
There's a lot more information about the thinking behind this at
http://blog.benjaminlipton.com/2016/07/19/csr-generation-templating.html
if you're interested, as well.
Thanks,
Ben
--
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
[Freeipa-devel] [PATCH 0001][Tests] Fix for dns_plugin tests
Greetings!
Fix for ipatests/test_xmlrpc/test_dns_plugin.py
(test_forwardzone_delegation_warnings.test)
You can't have a DNS zone with the authoritative nameserver that does not have
a A or record in the local DNS. Since in some test environments primary
hostname of the master is managed by an external DNS, this hostname can not be
used as a NS-record for IPA-managed zones. Therefore another existing fqdn
corresponding to the master's ip and managed by IPA DNS was used.
Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer
From 23104798f8350966bec3637396a85b528a47a9f4 Mon Sep 17 00:00:00 2001
From: Ganna Kaihorodova
Date: Mon, 18 Jul 2016 19:10:17 +0200
Subject: [PATCH 1/2] You can't have a dns zone with the authoritative
nameserver that does not have a A or record in the local DNS. Since in
some test environments primary hostname of the master is managed by an
external DNS, this hostname can not be used as a ns-record for IPA-managed
zones. Therefore another existing fqdn corresponding to the master's ip and
managed by IPA DNS was used
---
ipatests/test_xmlrpc/test_dns_plugin.py | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py
index f3484166dd3c5986978d89ca64589960b3d12338..038d17e9debc86a8e9afcadcea30e7cee29bbd05 100644
--- a/ipatests/test_xmlrpc/test_dns_plugin.py
+++ b/ipatests/test_xmlrpc/test_dns_plugin.py
@@ -1,6 +1,6 @@
# Authors:
# Pavel Zuna
-#
+# Ganna Kaihorodova
# Copyright (C) 2010 Red Hat
# see file 'COPYING' for use and warranty information
#
@@ -39,6 +39,7 @@ else:
_dns_zone_record = DNSName(u'@')
# default value of idnssoamname is local DNS server
+self_server_ns_1 = normalize_zone("ipa-ca.%s" % api.env.domain)
self_server_ns = normalize_zone(api.env.host)
self_server_ns_dnsname = DNSName(self_server_ns)
@@ -5078,7 +5079,7 @@ class test_forwardzone_delegation_warnings(Declarative):
desc='Delegate zone %r from zone %r using NS record' % (
zone1_sub_fw, zone1_sub),
command=('dnsrecord_add', [zone1_sub, u'fw'],
- {'nsrecord': self_server_ns}),
+ {'nsrecord': self_server_ns_1}),
expected={
'value': DNSName(u'fw'),
'summary': None,
@@ -5086,7 +5087,7 @@ class test_forwardzone_delegation_warnings(Declarative):
'objectclass': objectclasses.dnsrecord,
'dn': DN(('idnsname', u'fw'), zone1_sub_dn),
'idnsname': [DNSName(u'fw')],
-'nsrecord': [self_server_ns],
+'nsrecord': [self_server_ns_1],
},
},
),
@@ -5206,7 +5207,7 @@ class test_forwardzone_delegation_warnings(Declarative):
desc='Delegate zone %r from zone %r using NS record' % (
zone1_sub2_fw, zone1_sub),
command=('dnsrecord_add', [zone1_sub, u'fw.sub2'],
- {'nsrecord': self_server_ns}),
+ {'nsrecord': self_server_ns_1}),
expected={
'value': DNSName(u'fw.sub2'),
'summary': None,
@@ -5214,7 +5215,7 @@ class test_forwardzone_delegation_warnings(Declarative):
'objectclass': objectclasses.dnsrecord,
'dn': DN(('idnsname', u'fw.sub2'), zone1_sub_dn),
'idnsname': [DNSName(u'fw.sub2')],
-'nsrecord': [self_server_ns],
+'nsrecord': [self_server_ns_1],
},
},
),
@@ -5291,7 +5292,7 @@ class test_forwardzone_delegation_warnings(Declarative):
desc='Delegate zone %r from zone %r using NS record' % (
zone1_sub2_fw, zone1),
command=('dnsrecord_add', [zone1, u'fw.sub2.sub'],
- {'nsrecord': self_server_ns}),
+ {'nsrecord': self_server_ns_1}),
expected={
'value': DNSName(u'fw.sub2.sub'),
'summary': None,
@@ -5299,7 +5300,7 @@ class test_forwardzone_delegation_warnings(Declarative):
'objectclass': objectclasses.dnsrecord,
'dn': DN(('idnsname', u'fw.sub2.sub'), zone1_dn),
'idnsname': [DNSName(u'fw.sub2.sub')],
-'nsrecord': [self_server_ns],
+'nsrecord': [self_server_ns_1],
},
},
),
--
2.7.4
--
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 0553] CI tests: improve log collecting in tests
On 19.07.2016 16:18, Martin Basti wrote: Patch attached. self-NACK, my assumptions were wrong, this doesn't work if any of log files do not exist -- 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 0032] Secure permission and cleanup Custodia server.keys
On 12.07.2016 16:45, Christian Heimes wrote:
Custodia's server.keys file contain the private RSA keys for encrypting
and signing Custodia messages. The file was created with permission 644
and is only secured by permission 700 of the directory
/etc/ipa/custodia. The installer and upgrader ensure that the file
has 600.
The server.keys file and all keys are now removed when during
uninstallation of a server, too.
https://bugzilla.redhat.com/show_bug.cgi?id=1353936
https://fedorahosted.org/freeipa/ticket/6015
https://fedorahosted.org/freeipa/ticket/6056
NACK
ipa-server-install --uninstall doesn't work
2016-07-19T15:00:34Z INFO Remove Custodia keys
2016-07-19T15:00:34Z DEBUG Traceback (most recent call last):
File "/usr/lib/python2.7/site-packages/ipapython/install/common.py",
line 91, in _handle_exception
super(Continuous, self)._handle_exception(exc_info)
File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 394, in _handle_exception
six.reraise(*exc_info)
File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 446, in _handle_exception
super(ComponentBase, self)._handle_exception(exc_info)
File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 394, in _handle_exception
six.reraise(*exc_info)
File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 362, in __runner
step()
File "/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 359, in
step = lambda: next(self.__gen)
File "/usr/lib/python2.7/site-packages/ipapython/install/util.py",
line 81, in run_generator_with_yield_from
six.reraise(*exc_info)
File "/usr/lib/python2.7/site-packages/ipapython/install/util.py",
line 59, in run_generator_with_yield_from
value = gen.send(prev_value)
File "/usr/lib/python2.7/site-packages/ipapython/install/common.py",
line 71, in _uninstall
for nothing in self._uninstaller(self.parent):
File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",
line 1367, in main
uninstall(self)
File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",
line 265, in decorated
func(installer)
File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",
line 1075, in uninstall
custodiainstance.CustodiaInstance().uninstall()
File
"/usr/lib/python2.7/site-packages/ipaserver/install/custodiainstance.py",
line 88, in uninstall
self.__remove_keys()
File
"/usr/lib/python2.7/site-packages/ipaserver/install/custodiainstance.py",
line 74, in __remove_keys
keystore.remove_server_keys()
File "/usr/lib/python2.7/site-packages/ipapython/secrets/kem.py",
line 224, in remove_server_keys
self.remove_keys('host')
File "/usr/lib/python2.7/site-packages/ipapython/secrets/kem.py",
line 231, in remove_keys
ldapconn.remove_key(KEY_USAGE_SIG, principal)
File "/usr/lib/python2.7/site-packages/ipapython/secrets/kem.py",
line 145, in remove_key
conn = self.connect()
File "/usr/lib/python2.7/site-packages/ipapython/secrets/common.py",
line 38, in connect
conn.sasl_interactive_bind_s('', auth_tokens)
File "/usr/lib64/python2.7/site-packages/ldap/ldapobject.py", line
244, in sasl_interactive_bind_s
return
self._ldap_call(self._l.sasl_interactive_bind_s,who,auth,RequestControlTuples(serverctrls),RequestControlTuples(clientctrls),sasl_flags)
File "/usr/lib64/python2.7/site-packages/ldap/ldapobject.py", line
106, in _ldap_call
result = func(*args,**kwargs)
SERVER_DOWN: {'desc': "Can't contact LDAP server"}
--
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
[Freeipa-devel] [PATCH 0553] CI tests: improve log collecting in tests
Patch attached.
From 55a4b4a47bd859194bfb3fc4ab6acde4f8086f6e Mon Sep 17 00:00:00 2001
From: Martin Basti
Date: Tue, 19 Jul 2016 12:09:29 +0200
Subject: [PATCH] CI tests: improve log collecting
We should collect as much as possible relevant logs to be able do better
investigation from test automation
---
ipaplatform/base/paths.py | 1 +
ipatests/test_integration/tasks.py | 60 +-
2 files changed, 47 insertions(+), 14 deletions(-)
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index d6fbe32f6839a5db40148777132ba1454cbc3382..4e9b41974541acc676c3f6b2399cbf8f5808642a 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -300,6 +300,7 @@ class BasePathNamespace(object):
SLAPD_INSTANCE_ACCESS_LOG_TEMPLATE = "/var/log/dirsrv/slapd-%s/access"
SLAPD_INSTANCE_ERROR_LOG_TEMPLATE = "/var/log/dirsrv/slapd-%s/errors"
VAR_LOG_HTTPD_DIR = "/var/log/httpd"
+VAR_LOG_HTTPD_ERROR = "/var/log/httpd/error_log"
IPABACKUP_LOG = "/var/log/ipabackup.log"
IPACLIENT_INSTALL_LOG = "/var/log/ipaclient-install.log"
IPACLIENT_UNINSTALL_LOG = "/var/log/ipaclient-uninstall.log"
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 5ce2595c078e093adc3af024d41ddb5fb6892776..eabbee4dc6a1736842e5657ee596060a6f566430 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -47,6 +47,49 @@ from ipalib.constants import DOMAIN_LEVEL_0
log = log_mgr.get_logger(__name__)
+def setup_server_logs_collecting(host):
+"""
+This function setup logs to be collected on host. We should collect all
+possible logs that may be helpful to debug IPA server
+"""
+# dirsrv logs
+inst = host.domain.realm.replace('.', '-')
+host.collect_log(paths.SLAPD_INSTANCE_ERROR_LOG_TEMPLATE % inst)
+host.collect_log(paths.SLAPD_INSTANCE_ACCESS_LOG_TEMPLATE % inst)
+
+# IPA install logs
+host.collect_log(paths.IPASERVER_INSTALL_LOG)
+host.collect_log(paths.IPACLIENT_INSTALL_LOG)
+host.collect_log(paths.IPAREPLICA_INSTALL_LOG)
+host.collect_log(paths.IPAREPLICA_CONNCHECK_LOG)
+host.collect_log(paths.IPAREPLICA_CA_INSTALL_LOG)
+host.collect_log(paths.IPACLIENT_INSTALL_LOG)
+host.collect_log(paths.IPASERVER_KRA_INSTALL_LOG)
+host.collect_log(paths.IPASERVER_CA_INSTALL_LOG)
+host.collect_log(paths.IPA_CUSTODIA_AUDIT_LOG)
+
+# IPA uninstall logs
+host.collect_log(paths.IPASERVER_KRA_UNINSTALL_LOG)
+host.collect_log(paths.IPACLIENT_UNINSTALL_LOG)
+host.collect_log(paths.IPASERVER_KRA_UNINSTALL_LOG)
+
+# IPA backup and restore logs
+host.collect_log(paths.IPARESTORE_LOG)
+host.collect_log(paths.IPABACKUP_LOG)
+
+# kerberos related logs
+host.collect_log(paths.KADMIND_LOG)
+
+# httpd logs
+host.collect_log(paths.VAR_LOG_HTTPD_ERROR)
+
+# dogtag logs
+host.collect_log(os.path.join(paths.VAR_LOG_PKI_DIR, '*'))
+
+# SSSD debugging must be set after client is installed (function
+# setup_sssd_debugging)
+
+
def check_arguments_are(slice, instanceof):
"""
:param: slice - tuple of integers denoting the beginning and the end
@@ -234,12 +277,7 @@ def install_master(host, setup_dns=True, setup_kra=False, extra_args=(),
domain_level=None):
if domain_level is None:
domain_level = host.config.domain_level
-host.collect_log(paths.IPASERVER_INSTALL_LOG)
-host.collect_log(paths.IPACLIENT_INSTALL_LOG)
-inst = host.domain.realm.replace('.', '-')
-host.collect_log(paths.SLAPD_INSTANCE_ERROR_LOG_TEMPLATE % inst)
-host.collect_log(paths.SLAPD_INSTANCE_ACCESS_LOG_TEMPLATE % inst)
-
+setup_server_logs_collecting(host)
apply_common_fixes(host)
fix_apache_semaphores(host)
@@ -321,8 +359,7 @@ def install_replica(master, replica, setup_ca=True, setup_dns=False,
if domain_level is None:
domain_level = domainlevel(master)
apply_common_fixes(replica)
-replica.collect_log(paths.IPAREPLICA_INSTALL_LOG)
-replica.collect_log(paths.IPAREPLICA_CONNCHECK_LOG)
+setup_server_logs_collecting(replica)
allow_sync_ptr(master)
# Otherwise ipa-client-install would not create a PTR
# and replica installation would fail
@@ -400,12 +437,7 @@ def install_adtrust(host):
Configures the compat tree for the legacy clients.
"""
-# ipa-adtrust-install appends to ipaserver-install.log
-host.collect_log(paths.IPASERVER_INSTALL_LOG)
-
-inst = host.domain.realm.replace('.', '-')
-host.collect_log(paths.SLAPD_INSTANCE_ERROR_LOG_TEMPLATE % inst)
-host.collect_log(paths.SLAPD_INSTANCE_ACCESS_LOG_TEMPLATE % inst)
+setup_server_logs_collecting(host)
kinit_admin(host)
host.run_command(['ipa-adtrust-install', '-U',
--
2.5.5
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribu
Re: [Freeipa-devel] [PATCH] 0046 Create server certs with DNS altname
On 01.07.2016 13:26, Petr Spacek wrote: On 20.1.2016 05:04, Fraser Tweedale wrote: On Tue, Dec 08, 2015 at 07:06:39PM +1000, Fraser Tweedale wrote: On Mon, Dec 07, 2015 at 05:50:05PM -0500, Rob Crittenden wrote: Fraser Tweedale wrote: On Mon, Dec 07, 2015 at 01:53:15PM +0100, Martin Kosek wrote: On 12/07/2015 06:26 AM, Fraser Tweedale wrote: The attached patch fixes https://fedorahosted.org/freeipa/ticket/4970. Note that the problem is addressed by adding the appropriate request extension to the CSR; the fix does not involve changing the default profile behaviour, which is complicated (see ticket for details). Thanks for the patch! This is something we should really fix, I already get warnings in my Python scripts when I hit sites protected by such HTTPS cert: /usr/lib/python2.7/site-packages/requests/packages/urllib3/connection.py:264: SubjectAltNameWarning: Certificate for projects.engineering.redhat.com has no `subjectAltName`, falling back to check for a `commonName` for now. This feature is being removed by major browsers and deprecated by RFC 2818. (See https://github.com/shazow/urllib3/issues/497 for details.) Should we split ticket 4970, for the FreeIPA server part and then for cert profile part? As it looks like the FreeIPA server will be fixed even in FreeIPA 4.3.x and the other part later. How difficult do you see the general FreeIPA Certificate Profile part of this request? Is it a too big task to handle in 4.4 time frame? I will split the ticket and would suggest 4.4 Backlog - it might be doable but is a lower priority than e.g. Sub-CAs. If you are going to defer the profile part then you should probably update the client to also include a SAN if --request-cert is provided. rob Yes, good idea. Updated patch attached. Cheers, Fraser Bump, with rebased patch. Hi, this seems to work for Apache on IPA server & client cert. ACK. Pushed to master: b12db924143cd6828c596c0b8a261325f3f589f3 Interestingly enough I found out that Dogtag cert used on port 8443 does not have any SAN. Is it in scope of this ticket? I will leave the ticket open until this is answered. Martin^2 -- 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 0187] Use server API in com.redhat.idm.trust-fetch-domains oddjob helper
On 18.07.2016 10:53, Alexander Bokovoy wrote: On Mon, 18 Jul 2016, Martin Babinsky wrote: https://fedorahosted.org/freeipa/ticket/6082 -- Martin^3 Babinsky From 990f29cbfb457c6179ffc0bed452dc358ba30d21 Mon Sep 17 00:00:00 2001 From: Martin Babinsky Date: Thu, 14 Jul 2016 09:31:22 +0200 Subject: [PATCH] Use server API in com.redhat.idm.trust-fetch-domains oddjob helper https://fedorahosted.org/freeipa/ticket/6082 --- install/oddjob/com.redhat.idm.trust-fetch-domains | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/install/oddjob/com.redhat.idm.trust-fetch-domains b/install/oddjob/com.redhat.idm.trust-fetch-domains index a6b87cde917cfa5bfedf28442a6d1b2b512706f9..7c948fd53bd54bf3638ef3cc4407576b9011f4fb 100755 --- a/install/oddjob/com.redhat.idm.trust-fetch-domains +++ b/install/oddjob/com.redhat.idm.trust-fetch-domains @@ -76,7 +76,7 @@ env._bootstrap(debug=options.debug, log=None) env._finalize_core(**dict(DEFAULT_CONFIG)) # Initialize the API with the proper debug level -api.bootstrap(debug=env.debug, log=None) +api.bootstrap(in_server=True, debug=env.debug, log=None) api.finalize() # Only import trust plugin after api is initialized or internal imports ACK. Pushed to master: b144bf527db76573590255d4ac80e9dfd813ba3d -- 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 0026][Tests] RFE: Support UPN for trusted domains
On 18.07.2016 18:09, Martin Babinsky wrote:
On 07/14/2016 03:39 PM, Lenka Doudova wrote:
On 07/13/2016 06:04 PM, Martin Babinsky wrote:
On 07/01/2016 04:45 PM, Lenka Doudova wrote:
On 07/01/2016 03:04 PM, Martin Babinsky wrote:
On 07/01/2016 11:13 AM, Lenka Doudova wrote:
And, of course, a patch file :)
On 07/01/2016 11:09 AM, Lenka Doudova wrote:
Hi all,
here's patch with basic test suite for support of UPN.
Note: it needs to be applied on top of my patch 0025.2 (or
later, if
there's will be more fixes to that patch).
Lenka
Hi Lenka,
test data such as usernames, etc. should be stored either in separate
resource files or at least as class attributes like this:
diff --git a/ipatests/test_integration/test_trust.py
b/ipatests/test_integration/test_trust.py
index e8fdc6b..86ba7cc 100644
--- a/ipatests/test_integration/test_trust.py
+++ b/ipatests/test_integration/test_trust.py
@@ -394,28 +394,33 @@ class TestTrustWithUPN(ADTrustBase):
"""
Test support of UPN for trusted domains
"""
+upn_suffix = 'UPNsuffix.com'
+upn_username = 'upnuser'
+upn_princ = '{}@{}'.format(upn_username, upn_suffix)
+upn_password = 'Secret123456'
+
def test_upn_in_nonposix_trust(self):
""" Check that UPN is listed as trust attribute """
result = self.master.run_command(['ipa', 'trust-show',
self.ad_domain,
'--all', '--raw'])
-assert "ipantadditionalsuffixes: UPNsuffix.com" in
result.stdout_text
+assert ("ipantadditionalsuffixes:
{}".format(self.upn_suffix) in
+result.stdout_text)
def test_upn_user_resolution_in_nonposix_trust(self):
""" Check that user with UPN can be resolved """
-upnuser = 'upnu...@upnsuffix.com'
-result = self.master.run_command(['getent', 'passwd',
upnuser])
+result = self.master.run_command(['getent', 'passwd',
self.upn_princ])
# result will contain AD domain, not UPN
-upnuser_regex = "^upnuser@{0}:\*:(\d+):(\d+):UPN
User:/:$".format(
-self.ad_domain)
+upnuser_regex = "^{}@{}:\*:(\d+):(\d+):UPN User:/:$".format(
+self.upn_username, self.ad_domain)
assert re.search(upnuser_regex, result.stdout_text)
def test_upn_user_authentication(self):
""" Check that AD user with UPN can authenticate in IPA """
self.master.run_command(['systemctl', 'restart', 'krb5kdc'])
-self.master.run_command(['kinit', '-C', '-E',
'upnu...@upnsuffix.com'],
- stdin_text='Secret123456')
+self.master.run_command(['kinit', '-C', '-E',
self.upn_princ],
+ stdin_text=self.upn_password)
otherwise LGTM.
Thanks for review, fixed patch attached.
Few notes:
1. mbabinsky's suggestion to store testdata as class attributes or
separate resource file: I decided to use the class attribute approach.
The separate resource file is a nice idea, which I have already put on
my "to do" list - there's a lot of hardcoded stuff in the trust tests,
even in the original ones (before my patches), so when there's time
I'll
work on a way how to dynamically provide this data as test
configuration
2. previous discussion about getent vs. pwd.getpwnam(): I'll leave the
getent command, since according to mbasti the alternative would not
work
in CI.
Lenka
Hi Lenka,
I am not sure 'test_all_trustdomains_found' should be run as a part of
this test suite. Maybe yes, I'm not sure.
Also I would add a 60 second sleep after KDC restart in
'test_upn_user_authentication' so that MS-PAC cache gets refreshed
before trying to kinit as enterprise principal.
Two of the tests fail on my setup but that is probably due to
https://fedorahosted.org/freeipa/ticket/6082 .
Hi,
the "test_all_trustdomains_found" method is inherited from parent class,
and I believe it cannot hurt to have it there.
I added the sleep as you requested.
I also tried to run the tests with two way trust and since everything
was fine then, the failures you experienced are really most likely due
to the oddjob issue you linked. Of course the patch still contains tests
with one way trusts, so until the oddjob issue is solved, the tests will
probably fail, and should be fine once the fix is provided.
Fixed patch attached.
Lenka
Yes I think that the failing tests are due to bugs in trust, not the
test code. ACK.
New ticket created for tests https://fedorahosted.org/freeipa/ticket/6094
Pushed to master: 6a072f3c5c114747c190d0c309a8d53dd8e46394
--
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 0025][Tests] RFE: External trust
On 18.07.2016 18:07, Martin Babinsky wrote:
On 07/18/2016 04:59 PM, Lenka Doudova wrote:
On 07/18/2016 04:55 PM, Martin Babinsky wrote:
On 07/14/2016 11:42 AM, Lenka Doudova wrote:
On 07/13/2016 05:40 PM, Martin Babinsky wrote:
On 07/01/2016 04:15 PM, Lenka Doudova wrote:
On 07/01/2016 02:38 PM, Martin Babinsky wrote:
On 07/01/2016 06:36 AM, Lenka Doudova wrote:
On 06/30/2016 05:01 PM, Martin Babinsky wrote:
On 06/30/2016 03:47 PM, Lenka Doudova wrote:
Hi,
attaching patch with some basic coverage for external trust
feature. Bit
more detailed info in commit message.
Since the feature requires me to run commands previously used
only for
forest root domains even for subdomains, I made some changes in
ipatests/test_integration/tasks.py file, so that it would enable
me to
reuse existing function without copy-pasting them for one
variable
change.
Lenka
Hi Lenka,
I have a few comments:
1.)
-def establish_trust_with_ad(master, ad, extra_args=()):
+def establish_trust_with_ad(master, ad, extra_args=(),
subdomain=False):
This modification seems extremely kludgy to me.
I would rather change the function signature to:
-def establish_trust_with_ad(master, ad, extra_args=()):
+def establish_trust_with_ad(master, ad_domain, extra_args=()):
and pass the domain name itself to the function instead of doing
magic
inside. The same goes with `remove trust_with_ad`. You may
want to
fix
the calls to them in existing code so that the domain is passed
instead of the whole trust object.
2.)
+def sync_time_hostname(host, srv_hostname):
+"""
+Syncs time with the remote server given by its hostname.
+Please note that this function leaves ntpd stopped.
+"""
+host.run_command(['systemctl', 'stop', 'ntpd'])
+host.run_command(['ntpdate', srv_hostname])
+
+
this looks like a duplicate of the function above it, why is it
even
needed?
3.)
+class TestExternalTrustWithSubdomain(ADTrustBase):
+"""
+Test establishing external trust with subdomain
+"""
+
+@classmethod
+def install(cls, mh):
+super(ADTrustBase, cls).install(mh)
+cls.ad = cls.ad_domains[0].ads[0]
+cls.install_adtrust()
+cls.check_sid_generation()
+
+# Determine whether the subdomain AD is available
+try:
+child_ad =
cls.host_by_role(cls.optional_extra_roles[0])
+cls.ad_subdomain =
'.'.join(child_ad.hostname.split('.')[1:])
+cls.ad_subdomain_hostname = child_ad.hostname
+except LookupError:
+cls.ad_subdomain = None
+
+cls.configure_dns_and_time()
Please use proper OOP practices when overriding the behavior
of the
base test class.
For starters you could rewrite the install method like this:
+@classmethod
+def install(cls, mh):
+# Determine whether the subdomain AD is available
+try:
+cls.child_ad =
cls.host_by_role(cls.optional_extra_roles[0])
+cls.ad_subdomain =
'.'.join(child_ad.hostname.split('.')[1:])
+cls.ad_subdomain_hostname = child_ad.hostname
+except LookupError:
+raise nose.SkipTest("AFAIK this will skip the whole
test
class")
+super(ADTrustBase, cls).install(mh)
with child_ad stored as class attribute, you can override
`configure_dns_and_time`:
+def configure_dns_and_time(cls):
+tasks.configure_dns_for_trust(cls.master,
cls.ad_subdomain)
# no need to re-implement the function just to get to the
child
AD DC hostname now
+tasks.sync_time(cls.master, cls.child_ad.hostname)
You can then use this class as an intermediary in the
hierarchy and
inherit the other external/non-external trust test classes
from it,
since most setup method in them are just copy-pasted from this
one.
Hi,
thanks for review, fixed patch attached.
Lenka
Hi Lenka,
I am still not happy about the patch underutilizing the
potential for
a proper inheritance hierarchy and instead relying on a staggering
amounts of copy-pasting for implementation.
I am attaching a quick untested patch illustrating how the
implementation should look like.
Also you have some leftovers from previous revision, please fix
them:
Pylint is running, please wait ...
* Module ipatests.test_integration.test_trust
ipatests/test_integration/test_trust.py:236: [E1101(no-member),
TestExternalTrustWithSubdomain.configure_dns_and_time] Module
'ipatests.test_integration.tasks' has no 'sync_time_hostname'
member)
ipatests/test_integration/test_trust.py:243:
[E1123(unexpected-keyword-arg),
TestExternalTrustWithSubdomain.test_establish_trust] Unexpected
keyword argument 'subdomain' in function call)
ipatests/test_integration/test_trust.py:279:
[E1123(unexpected-keyword-arg),
TestExternalTrustWithSubdomain.test_remove_nonposix_trust]
Unexpected
keyword argument 'subdomain' in function call)
ipatests/test_integration/test_trust.py:330: [E1101(no-member),
TestNonexternalTrustWithSubdomain.configure_dns_and
Re: [Freeipa-devel] [PATCH] 0210 frontend: fix output validation for multiple type choices
On 07/19/2016 01:13 PM, Alexander Bokovoy wrote:
On Mon, 18 Jul 2016, Martin Babinsky wrote:
On 07/18/2016 12:29 PM, Martin Babinsky wrote:
> On 07/18/2016 10:01 AM, Jan Cholasta wrote:
> > Hi,
> > > > On 16.7.2016 12:46, Alexander Bokovoy wrote:
> > > Hi,
> > > > > > I had some time and was blocked by these bugs to do my
tickets so I
> > > actually fixed these three problems that are assigned to Martin
> > > Babinsky. Hopefully, Martin wouldn't be offended by that. :)
> > > > > > --
> > > > > > Output entry elements may have multiple types allowed. We
need to check
> > > all of them to properly validate the output. Right now, thin client
> > > receives type specifications for elements as tuples of types, so
> > > what is seen as 'None' on the server side becomes (type(None),)
tuple
> > > on the thin client side.
> > > > > > Change validation to account this by processing each
separate type
> > > of the element and account for both None and type(None). Raise type
> > > error only if all of the type checks failed.
> > > > > > https://fedorahosted.org/freeipa/ticket/6061
> > > > NACK, this only hides the real issue, which is that
trustconfig-show
> > (and automember-set-default in #6037) claims to return the primary
key
> > of the object in the 'value' output field, but the object does not
have
> > a primary key, so the client rightfully expects None.
> > > > A proper fix would be to set "has_output =
output.simple_value" for
> > these commands (all of automember_default_group_{set,remove,show},
> > trustconfig_{mod,show}).
> > > > Honza
> > > > The problem is that these commands do not return a simple
boolean in
> 'result' but a full entry dict, so 'simple_value' won't do the trick in
> this case.
> > But I agree, we should rather fix misbehaving commands rather than
bend
> the framework to accomodate their idiosyncracies, we have enough of
that
> already.
> I am attaching a patch that adds a custom shim for misbehaving
commands so that they work as before. There is also a big fat warning
added to discourage implementation of such commands.
Let's go by this patch. I originally thought changing trust.py to simply
not supply 'value' field at all but this fix is simpler.
We found out that we still would need to handle
automember-default-group-{set,remove} commands which use the value to
print out summary, so it seems that we cannot win without some more
extensive fixing.
--
Martin^3 Babinsky
--
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] 0023 Bug in the ipapwd plugin
On 19.07.2016 12:21, Simo Sorce wrote: On Tue, 2016-07-19 at 10:17 +0200, thierry bordaz wrote: On 07/13/2016 10:02 PM, Lukas Slebodnik wrote: On (13/07/16 16:50), thierry bordaz wrote: https://fedorahosted.org/freeipa/ticket/6030 >From 4efedc5e674db92f9f7c160429df543422ed8afb Mon Sep 17 00:00:00 2001 From: Thierry Bordaz Date: Wed, 13 Jul 2016 15:34:20 +0200 Subject: [PATCH] Ticket 6030 Bug in the ipapwd plugin ipapwd_encrypt_encode_key allocates 'kset' on the heap but with num_keys and keys not being initialized. Then ipa_krb5_generate_key_data initializes them with the generated keys. If ipa_krb5_generate_key_data fails (here EINVAL meaning no principal->realm.data), num_keys and keys are left uninitialized. Upon failure, ipapwd_keyset_free is called to free 'kset' that contains random num_keys and keys. allocates kset with calloc so that kset->num_keys==0 and kset->keys==NULL https://fedorahosted.org/freeipa/ticket/6030 --- daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c index 5ca155d..46bf79a 100644 --- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c +++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c @@ -148,7 +148,7 @@ Slapi_Value **ipapwd_encrypt_encode_key(struct ipapwd_krbcfg *krbcfg, pwd.length = strlen(data->password); } -kset = malloc(sizeof(struct ipapwd_keyset)); +kset = calloc(sizeof(struct ipapwd_keyset)); I though that calloc need two arguments man malloc says: void *malloc(size_t size); void *calloc(size_t nmemb, size_t size); LS Oppss, sorry for this dummy patch. Here is the right one ACK, Simo. Pushed to master: b04f617803c430b13f8796e911f78bd65f6cf55f -- 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 0024][Tests] Fix integration tests not to produce incorrect /etc/hosts file
On 06/29/2016 06:49 PM, Petr Spacek wrote:
On 29.6.2016 18:39, Oleg Fayans wrote:
In fact, I believe /etc/hosts file should not be touched at all.
Hostname resolution is usually governed by the DNS system of the lab in
which tests are running. We do not modify it when perform tests
manually, so I'd rather remove this method at all.
+1, it should not be need. Let me know if it is needed for some reason and I
will have a look.
Petr^2 Spacek
Hi,
providing new (and renamed) patch as was suggested in the discussion
above - removing manipulation with /etc/hosts file from the tests.
The "fix_etc_hosts" function was completely removed from the tasks file.
Verification that nothing is broken by this change was done by running
some random integration test (trust tests), and also on Milan's
suggestion by running a test requiring two replicas (replica promotion
tests).
Lenka
On 06/29/2016 06:27 PM, Lenka Doudova wrote:
Hi all,
a function 'fix_etc_hosts' in ipatests/test_integration/tasks.py
produces incorrect /etc/hosts file (solitary IPv6 address), and
currently parser is not able to resolve the issue, causing
ipa-server-install to fail with 'list index out of range' error.
Hence I'm attaching patch to fix this issue before parser is fixed
(related ticket to it #6014). The fix is just change of regexs
responsible for creating incorrect file so that all the lines containing
defined hostname are removed, not just specific IP/hostname/shortname
strings.
Lenka
From eb73a40d4294c062a646b8d6cf9fe495382896ed Mon Sep 17 00:00:00 2001
From: Lenka Doudova
Date: Tue, 19 Jul 2016 13:12:05 +0200
Subject: [PATCH] Tests: Removing manipulation with /etc/hosts file from
integration tests
---
ipatests/test_integration/tasks.py | 19 ---
1 file changed, 19 deletions(-)
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index cee6c15ccdc260f6ebe4cdebbc2cf6ec84931f27..55cc902b490c57b8741a644ed691254c7d83dc24 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -96,7 +96,6 @@ def allow_sync_ptr(host):
def apply_common_fixes(host):
-fix_etc_hosts(host)
fix_hostname(host)
prepare_host(host)
@@ -118,24 +117,6 @@ def backup_file(host, filename):
return False
-def fix_etc_hosts(host):
-backup_file(host, paths.HOSTS)
-contents = host.get_file_contents(paths.HOSTS)
-# Remove existing mentions of the host's FQDN, short name, and IP
-# Removing of IP must be done as first, otherwise hosts file may be
-# corrupted
-contents = re.sub('^%s.*' % re.escape(host.ip), '', contents,
- flags=re.MULTILINE)
-contents = re.sub('\s%s(\s|$)' % re.escape(host.hostname), ' ', contents,
- flags=re.MULTILINE)
-contents = re.sub('\s%s(\s|$)' % re.escape(host.shortname), ' ', contents,
- flags=re.MULTILINE)
-# Add the host's info again
-contents += '\n%s %s %s\n' % (host.ip, host.hostname, host.shortname)
-log.debug('Writing the following to /etc/hosts:\n%s', contents)
-host.put_file_contents(paths.HOSTS, contents)
-
-
def fix_hostname(host):
backup_file(host, paths.ETC_HOSTNAME)
host.put_file_contents(paths.ETC_HOSTNAME, host.hostname + '\n')
--
2.7.4
--
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
[Freeipa-devel] Location mechanism RFE
Hello, can you please check if TCs for basic test looks good for http://www.freeipa.org/page/V4/DNS_Location_Mechanism TC 1 - default values (50:50) - ipa-server-install + replica - add two location - mod SRV record (ipa server-mod) to master - location1 | replica - location2 - check by 'dig' (dig @$FIRST_IPA_DNS_SERVER _kerberos._udp.$PRIMARYDNSDOMAIN SRV) output: both servers/location listed TC 2 - set up service weight - another replica in location1 - ipa server-mod server1 --service-weight=75 location1 - ipa server-mod server2 --service-weight=25 location1 - check traffic output: trafic divided into 1:4 TC 3 - used location server go offline - ipactl stop on location1 output : switch to another location TC 4 (negative) - try to add non-existant server to location :: can't be added - wrong dns setup :: non location in dig output -- Pavel Picka Red Hat Brno -- 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] 0210 frontend: fix output validation for multiple type choices
On Mon, 18 Jul 2016, Martin Babinsky wrote:
On 07/18/2016 12:29 PM, Martin Babinsky wrote:
> On 07/18/2016 10:01 AM, Jan Cholasta wrote:
> > Hi,
> >
> > On 16.7.2016 12:46, Alexander Bokovoy wrote:
> > > Hi,
> > >
> > > I had some time and was blocked by these bugs to do my tickets so I
> > > actually fixed these three problems that are assigned to Martin
> > > Babinsky. Hopefully, Martin wouldn't be offended by that. :)
> > >
> > > --
> > >
> > > Output entry elements may have multiple types allowed. We need to check
> > > all of them to properly validate the output. Right now, thin client
> > > receives type specifications for elements as tuples of types, so
> > > what is seen as 'None' on the server side becomes (type(None),) tuple
> > > on the thin client side.
> > >
> > > Change validation to account this by processing each separate type
> > > of the element and account for both None and type(None). Raise type
> > > error only if all of the type checks failed.
> > >
> > > https://fedorahosted.org/freeipa/ticket/6061
> >
> > NACK, this only hides the real issue, which is that trustconfig-show
> > (and automember-set-default in #6037) claims to return the primary key
> > of the object in the 'value' output field, but the object does not have
> > a primary key, so the client rightfully expects None.
> >
> > A proper fix would be to set "has_output = output.simple_value" for
> > these commands (all of automember_default_group_{set,remove,show},
> > trustconfig_{mod,show}).
> >
> > Honza
> >
>
> The problem is that these commands do not return a simple boolean in
> 'result' but a full entry dict, so 'simple_value' won't do the trick in
> this case.
>
> But I agree, we should rather fix misbehaving commands rather than bend
> the framework to accomodate their idiosyncracies, we have enough of that
> already.
>
I am attaching a patch that adds a custom shim for misbehaving commands so
that they work as before. There is also a big fat warning added to
discourage implementation of such commands.
Let's go by this patch. I originally thought changing trust.py to simply
not supply 'value' field at all but this fix is simpler.
--
/ Alexander Bokovoy
--
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] 0011 server uninstall fails to remove krb principals
On 07/11/2016 09:52 AM, Florence Blanc-Renaud wrote: > Hi, > > please find a patch for the 3rd issue of ticket 6012. > > https://fedorahosted.org/freeipa/ticket/6012 > > bump for review -- Petr Vobornik -- 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] 0210 frontend: fix output validation for multiple type choices
On 19.7.2016 12:31, Alexander Bokovoy wrote:
On Tue, 19 Jul 2016, Jan Cholasta wrote:
On 19.7.2016 11:36, Alexander Bokovoy wrote:
On Tue, 19 Jul 2016, Jan Cholasta wrote:
On 19.7.2016 10:40, Alexander Bokovoy wrote:
On Tue, 19 Jul 2016, Jan Cholasta wrote:
On 18.7.2016 10:12, Alexander Bokovoy wrote:
On Mon, 18 Jul 2016, Jan Cholasta wrote:
Hi,
On 16.7.2016 12:46, Alexander Bokovoy wrote:
Hi,
I had some time and was blocked by these bugs to do my tickets
so I
actually fixed these three problems that are assigned to Martin
Babinsky. Hopefully, Martin wouldn't be offended by that. :)
--
Output entry elements may have multiple types allowed. We need to
check
all of them to properly validate the output. Right now, thin
client
receives type specifications for elements as tuples of types, so
what is seen as 'None' on the server side becomes (type(None),)
tuple
on the thin client side.
Change validation to account this by processing each separate type
of the element and account for both None and type(None). Raise
type
error only if all of the type checks failed.
https://fedorahosted.org/freeipa/ticket/6061
NACK, this only hides the real issue, which is that
trustconfig-show
(and automember-set-default in #6037) claims to return the primary
key
of the object in the 'value' output field, but the object does not
have a primary key, so the client rightfully expects None.
Why did it work before introducing thin client?
Because I took the liberty of not putting any extra effort just to
keep old hacks working on thin client. In this case,
output.PrimaryKey
was used for the 'value' output field before, which always allowed
unicode in addition to the primary key type. On thin client,
output.Output with the primary key type is used instead, which is
less
forgiving and uncovers bogus command definitions. (There aren't any
besides the ones already mentioned, I remember fixing them but the
commit got lost somehow. Oh well.)
This means thin client would not work against old server which would
return a non-primary key value. I think it is unacceptable.
Not true, as this only applies to servers with API schema (4.4+).
Only because thin client does not work against servers without API
schema at all:
[root@f24-master ~]# ipa -vv -e xmlrpc_uri=https://id.vda.li/ipa/xml
config-show
ipa: INFO: trying https://id.vda.li/ipa/json
ipa: INFO: Request: {
"id": 0,"method": "ping","params": [
[],{}
]
}
ipa: INFO: Response: {
"error": null,"id": 0,"principal": "ad...@vda.li",
"result": {
"messages": [
{
"code": 13001,"message": "API Version
number was not sent, forward compatibility not guaranteed. Assuming
server's API version, 2.156","name": "VersionMissing",
"type": "warning"
}
],"summary": "IPA server version 4.2.3. API version 2.156"
},"version": "4.2.3"
}
ipa: INFO: Forwarding 'config_show/1' to json server
'https://id.vda.li/ipa/json'
ipa: INFO: Request: {
"id": 0,"method": "config_show/1","params": [
[],{
"version": "2.210"
}
]
}
ipa: INFO: Response: {
"error": {
"code": 905,"message": "unknown command 'config_show/1'",
"name": "CommandError"
},"id": 0,"principal": "ad...@vda.li","result": null,
"version": "4.2.3"
}
ipa: ERROR: unknown command 'config_show/1'
Works fine for me:
$ rpm -q freeipa-client
freeipa-client-4.4.0.201607180602GITa2891af3-0.fc24.x86_64
$ ipa ping
---
IPA server version 4.2.3. API version 2.156
---
$ ipa config-show
Maximum username length: 32
Home directory base: /home
Default shell: /bin/sh
Default users group: ipausers
Default e-mail domain: abc.idm.lab.eng.brq.redhat.com
Search time limit: -1
Search size limit: -1
User search fields: uid,givenname,sn,telephonenumber,ou,title
Group search fields: cn,description
Enable migration mode: FALSE
Certificate Subject base: O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
Password Expiration Notification (days): 4
Password plugin features: AllowNThash
SELinux user map order:
guest_u:s0$xguest_u:s0$user_u:s0$staff_u:s0-s0:c0.c1023$unconfined_u:s0-s0:c0.c1023
Default SELinux user: unconfined_u:s0-s0:c0.c1023
Default PAC types: nfs:NONE, MS-PAC
Try removing ~/.cache/ipa/servers/ - if you
reinstalled the server in the last hour (the default cache TTL), it
should help.
Yes, this did help, thanks. I didn't reinstall the server at all but I
ran ipa command before adding CA cert of that install to my client's NSS
database. I guess it was the cache from that attempt that broke it.
Hmm, that does not sound right. If there was no CA cert for the server,
then it wasn't possible to talk to it, so there should not have been any
information cached for it. I will investigate further, but it might be a
bug in caching.
Coming back to trustconf
Re: [Freeipa-devel] [PATCH] 0210 frontend: fix output validation for multiple type choices
On Tue, 19 Jul 2016, Jan Cholasta wrote:
On 19.7.2016 11:36, Alexander Bokovoy wrote:
On Tue, 19 Jul 2016, Jan Cholasta wrote:
On 19.7.2016 10:40, Alexander Bokovoy wrote:
On Tue, 19 Jul 2016, Jan Cholasta wrote:
On 18.7.2016 10:12, Alexander Bokovoy wrote:
On Mon, 18 Jul 2016, Jan Cholasta wrote:
Hi,
On 16.7.2016 12:46, Alexander Bokovoy wrote:
Hi,
I had some time and was blocked by these bugs to do my tickets so I
actually fixed these three problems that are assigned to Martin
Babinsky. Hopefully, Martin wouldn't be offended by that. :)
--
Output entry elements may have multiple types allowed. We need to
check
all of them to properly validate the output. Right now, thin client
receives type specifications for elements as tuples of types, so
what is seen as 'None' on the server side becomes (type(None),)
tuple
on the thin client side.
Change validation to account this by processing each separate type
of the element and account for both None and type(None). Raise type
error only if all of the type checks failed.
https://fedorahosted.org/freeipa/ticket/6061
NACK, this only hides the real issue, which is that trustconfig-show
(and automember-set-default in #6037) claims to return the primary
key
of the object in the 'value' output field, but the object does not
have a primary key, so the client rightfully expects None.
Why did it work before introducing thin client?
Because I took the liberty of not putting any extra effort just to
keep old hacks working on thin client. In this case, output.PrimaryKey
was used for the 'value' output field before, which always allowed
unicode in addition to the primary key type. On thin client,
output.Output with the primary key type is used instead, which is less
forgiving and uncovers bogus command definitions. (There aren't any
besides the ones already mentioned, I remember fixing them but the
commit got lost somehow. Oh well.)
This means thin client would not work against old server which would
return a non-primary key value. I think it is unacceptable.
Not true, as this only applies to servers with API schema (4.4+).
Only because thin client does not work against servers without API
schema at all:
[root@f24-master ~]# ipa -vv -e xmlrpc_uri=https://id.vda.li/ipa/xml
config-show
ipa: INFO: trying https://id.vda.li/ipa/json
ipa: INFO: Request: {
"id": 0,"method": "ping","params": [
[],{}
]
}
ipa: INFO: Response: {
"error": null,"id": 0,"principal": "ad...@vda.li",
"result": {
"messages": [
{
"code": 13001,"message": "API Version
number was not sent, forward compatibility not guaranteed. Assuming
server's API version, 2.156","name": "VersionMissing",
"type": "warning"
}
],"summary": "IPA server version 4.2.3. API version 2.156"
},"version": "4.2.3"
}
ipa: INFO: Forwarding 'config_show/1' to json server
'https://id.vda.li/ipa/json'
ipa: INFO: Request: {
"id": 0,"method": "config_show/1","params": [
[],{
"version": "2.210"
}
]
}
ipa: INFO: Response: {
"error": {
"code": 905,"message": "unknown command 'config_show/1'",
"name": "CommandError"
},"id": 0,"principal": "ad...@vda.li","result": null,
"version": "4.2.3"
}
ipa: ERROR: unknown command 'config_show/1'
Works fine for me:
$ rpm -q freeipa-client
freeipa-client-4.4.0.201607180602GITa2891af3-0.fc24.x86_64
$ ipa ping
---
IPA server version 4.2.3. API version 2.156
---
$ ipa config-show
Maximum username length: 32
Home directory base: /home
Default shell: /bin/sh
Default users group: ipausers
Default e-mail domain: abc.idm.lab.eng.brq.redhat.com
Search time limit: -1
Search size limit: -1
User search fields: uid,givenname,sn,telephonenumber,ou,title
Group search fields: cn,description
Enable migration mode: FALSE
Certificate Subject base: O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
Password Expiration Notification (days): 4
Password plugin features: AllowNThash
SELinux user map order:
guest_u:s0$xguest_u:s0$user_u:s0$staff_u:s0-s0:c0.c1023$unconfined_u:s0-s0:c0.c1023
Default SELinux user: unconfined_u:s0-s0:c0.c1023
Default PAC types: nfs:NONE, MS-PAC
Try removing ~/.cache/ipa/servers/ - if you
reinstalled the server in the last hour (the default cache TTL), it
should help.
Yes, this did help, thanks. I didn't reinstall the server at all but I
ran ipa command before adding CA cert of that install to my client's NSS
database. I guess it was the cache from that attempt that broke it.
Coming back to trustconfig-show, older server response's 'value' field
is ignored. I guess we can actually remove the value completely because
it is of no use anywhere.
As to other commands without primary key and returning value, may be you
can revive your patch?
--
/ Alexander Bokovoy
--
Manage your subsc
Re: [Freeipa-devel] [PATCH 190] expose `--secret` option in radiusproxy-* commands
Hi, On 18.7.2016 13:51, Martin Babinsky wrote: https://fedorahosted.org/freeipa/ticket/6078 I don't think we want the secret searchable. Add a 'no_search' flag to the param to fix that. Honza -- Jan Cholasta -- 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] 0210 frontend: fix output validation for multiple type choices
On 19.7.2016 11:36, Alexander Bokovoy wrote:
On Tue, 19 Jul 2016, Jan Cholasta wrote:
On 19.7.2016 10:40, Alexander Bokovoy wrote:
On Tue, 19 Jul 2016, Jan Cholasta wrote:
On 18.7.2016 10:12, Alexander Bokovoy wrote:
On Mon, 18 Jul 2016, Jan Cholasta wrote:
Hi,
On 16.7.2016 12:46, Alexander Bokovoy wrote:
Hi,
I had some time and was blocked by these bugs to do my tickets so I
actually fixed these three problems that are assigned to Martin
Babinsky. Hopefully, Martin wouldn't be offended by that. :)
--
Output entry elements may have multiple types allowed. We need to
check
all of them to properly validate the output. Right now, thin client
receives type specifications for elements as tuples of types, so
what is seen as 'None' on the server side becomes (type(None),)
tuple
on the thin client side.
Change validation to account this by processing each separate type
of the element and account for both None and type(None). Raise type
error only if all of the type checks failed.
https://fedorahosted.org/freeipa/ticket/6061
NACK, this only hides the real issue, which is that trustconfig-show
(and automember-set-default in #6037) claims to return the primary
key
of the object in the 'value' output field, but the object does not
have a primary key, so the client rightfully expects None.
Why did it work before introducing thin client?
Because I took the liberty of not putting any extra effort just to
keep old hacks working on thin client. In this case, output.PrimaryKey
was used for the 'value' output field before, which always allowed
unicode in addition to the primary key type. On thin client,
output.Output with the primary key type is used instead, which is less
forgiving and uncovers bogus command definitions. (There aren't any
besides the ones already mentioned, I remember fixing them but the
commit got lost somehow. Oh well.)
This means thin client would not work against old server which would
return a non-primary key value. I think it is unacceptable.
Not true, as this only applies to servers with API schema (4.4+).
Only because thin client does not work against servers without API
schema at all:
[root@f24-master ~]# ipa -vv -e xmlrpc_uri=https://id.vda.li/ipa/xml
config-show
ipa: INFO: trying https://id.vda.li/ipa/json
ipa: INFO: Request: {
"id": 0,"method": "ping","params": [
[],{}
]
}
ipa: INFO: Response: {
"error": null,"id": 0,"principal": "ad...@vda.li",
"result": {
"messages": [
{
"code": 13001,"message": "API Version
number was not sent, forward compatibility not guaranteed. Assuming
server's API version, 2.156","name": "VersionMissing",
"type": "warning"
}
],"summary": "IPA server version 4.2.3. API version 2.156"
},"version": "4.2.3"
}
ipa: INFO: Forwarding 'config_show/1' to json server
'https://id.vda.li/ipa/json'
ipa: INFO: Request: {
"id": 0,"method": "config_show/1","params": [
[],{
"version": "2.210"
}
]
}
ipa: INFO: Response: {
"error": {
"code": 905,"message": "unknown command 'config_show/1'",
"name": "CommandError"
},"id": 0,"principal": "ad...@vda.li","result": null,
"version": "4.2.3"
}
ipa: ERROR: unknown command 'config_show/1'
Works fine for me:
$ rpm -q freeipa-client
freeipa-client-4.4.0.201607180602GITa2891af3-0.fc24.x86_64
$ ipa ping
---
IPA server version 4.2.3. API version 2.156
---
$ ipa config-show
Maximum username length: 32
Home directory base: /home
Default shell: /bin/sh
Default users group: ipausers
Default e-mail domain: abc.idm.lab.eng.brq.redhat.com
Search time limit: -1
Search size limit: -1
User search fields: uid,givenname,sn,telephonenumber,ou,title
Group search fields: cn,description
Enable migration mode: FALSE
Certificate Subject base: O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
Password Expiration Notification (days): 4
Password plugin features: AllowNThash
SELinux user map order:
guest_u:s0$xguest_u:s0$user_u:s0$staff_u:s0-s0:c0.c1023$unconfined_u:s0-s0:c0.c1023
Default SELinux user: unconfined_u:s0-s0:c0.c1023
Default PAC types: nfs:NONE, MS-PAC
Try removing ~/.cache/ipa/servers/ - if you reinstalled
the server in the last hour (the default cache TTL), it should help.
Same happens for every other command so I cannot even test the behavior.
It works against the same server as the thin client is.
[root@f24-master ~]# ipa -vv config-show
ipa: INFO: trying https://f24-master.ipa.ad.test/ipa/json
ipa: INFO: Forwarding 'config_show/1' to json server
'https://f24-master.ipa.ad.test/ipa/json'
ipa: INFO: Request: {
"id": 0,"method": "config_show/1","params": [
[],{
"version": "2.210"
}
]
}
ipa: INFO: Response: {
"error": null,"id": 0,"
Re: [Freeipa-devel] [PATCH] 0210 frontend: fix output validation for multiple type choices
On Tue, 19 Jul 2016, Alexander Bokovoy wrote:
On Tue, 19 Jul 2016, Jan Cholasta wrote:
On 19.7.2016 10:40, Alexander Bokovoy wrote:
On Tue, 19 Jul 2016, Jan Cholasta wrote:
On 18.7.2016 10:12, Alexander Bokovoy wrote:
On Mon, 18 Jul 2016, Jan Cholasta wrote:
Hi,
On 16.7.2016 12:46, Alexander Bokovoy wrote:
Hi,
I had some time and was blocked by these bugs to do my tickets so I
actually fixed these three problems that are assigned to Martin
Babinsky. Hopefully, Martin wouldn't be offended by that. :)
--
Output entry elements may have multiple types allowed. We need to
check
all of them to properly validate the output. Right now, thin client
receives type specifications for elements as tuples of types, so
what is seen as 'None' on the server side becomes (type(None),) tuple
on the thin client side.
Change validation to account this by processing each separate type
of the element and account for both None and type(None). Raise type
error only if all of the type checks failed.
https://fedorahosted.org/freeipa/ticket/6061
NACK, this only hides the real issue, which is that trustconfig-show
(and automember-set-default in #6037) claims to return the primary key
of the object in the 'value' output field, but the object does not
have a primary key, so the client rightfully expects None.
Why did it work before introducing thin client?
Because I took the liberty of not putting any extra effort just to
keep old hacks working on thin client. In this case, output.PrimaryKey
was used for the 'value' output field before, which always allowed
unicode in addition to the primary key type. On thin client,
output.Output with the primary key type is used instead, which is less
forgiving and uncovers bogus command definitions. (There aren't any
besides the ones already mentioned, I remember fixing them but the
commit got lost somehow. Oh well.)
This means thin client would not work against old server which would
return a non-primary key value. I think it is unacceptable.
Not true, as this only applies to servers with API schema (4.4+).
Only because thin client does not work against servers without API
schema at all:
[root@f24-master ~]# ipa -vv -e xmlrpc_uri=https://id.vda.li/ipa/xml config-show
ipa: INFO: trying https://id.vda.li/ipa/json
ipa: INFO: Request: {
"id": 0,"method": "ping","params": [
[],{}
]
}
ipa: INFO: Response: {
"error": null,"id": 0,"principal": "ad...@vda.li",
"result": {
"messages": [
{
"code": 13001,"message": "API Version
number was not sent, forward compatibility not guaranteed. Assuming
server's API version, 2.156","name": "VersionMissing",
"type": "warning"
}
],"summary": "IPA server version 4.2.3. API version
2.156"
},"version": "4.2.3"
}
ipa: INFO: Forwarding 'config_show/1' to json server
'https://id.vda.li/ipa/json'
ipa: INFO: Request: {
"id": 0,"method": "config_show/1","params": [
[],{
"version": "2.210"
}
]
}
ipa: INFO: Response: {
"error": {
"code": 905,"message": "unknown command
'config_show/1'","name": "CommandError"
},"id": 0,"principal": "ad...@vda.li","result": null,
"version": "4.2.3"
}
ipa: ERROR: unknown command 'config_show/1'
Same happens for every other command so I cannot even test the behavior.
It works against the same server as the thin client is.
[root@f24-master ~]# ipa -vv config-show
ipa: INFO: trying https://f24-master.ipa.ad.test/ipa/json
ipa: INFO: Forwarding 'config_show/1' to json server
'https://f24-master.ipa.ad.test/ipa/json'
ipa: INFO: Request: {
"id": 0,"method": "config_show/1","params": [
[],{
"version": "2.210"
}
]
}
ipa: INFO: Response: {
"error": null,"id": 0,"principal": "ad...@ipa.ad.test",
"result": {
"result": {
"ca_renewal_master_server": "f24-master.ipa.ad.test",
"ca_server_server": [
"f24-master.ipa.ad.test"
],"dn":
"cn=ipaConfig,cn=etc,dc=ipa,dc=ad,dc=test",
"ipa_master_server": [
"f24-master.ipa.ad.test"
],"ipacertificatesubjectbase": [
"O=IPA.AD.TEST"
],"ipaconfigstring": [
"AllowNThash"
],"ipadefaultemaildomain": [
"ipa.ad.test"
],"ipadefaultloginshell": [
"/bin/sh"
],"ipadefaultprimarygroup": [
"ipausers"
],"ipagroupsearchfields": [
"cn,description"
],"ipahomesrootdir": [
"/home"
],"ipakrbauthzdata": [
"nfs:NONE","MS-PAC"
],"ipamaxusernamelength": [
"32"
],"ipamigrationenabled": [
Re: [Freeipa-devel] [PATCH] 0023 Bug in the ipapwd plugin
On Tue, 2016-07-19 at 10:17 +0200, thierry bordaz wrote: > > > On 07/13/2016 10:02 PM, Lukas Slebodnik wrote: > > On (13/07/16 16:50), thierry bordaz wrote: > >> https://fedorahosted.org/freeipa/ticket/6030 > >> >From 4efedc5e674db92f9f7c160429df543422ed8afb Mon Sep 17 00:00:00 > 2001 > >> From: Thierry Bordaz > >> Date: Wed, 13 Jul 2016 15:34:20 +0200 > >> Subject: [PATCH] Ticket 6030 Bug in the ipapwd plugin > >> > >> ipapwd_encrypt_encode_key allocates 'kset' on the heap but > >> with num_keys and keys not being initialized. > >> Then ipa_krb5_generate_key_data initializes them with the > >> generated keys. > >> If ipa_krb5_generate_key_data fails (here EINVAL meaning no > >> principal->realm.data), num_keys and keys are left uninitialized. > >> Upon failure, ipapwd_keyset_free is called to free 'kset' > >> that contains random num_keys and keys. > >> > >> allocates kset with calloc so that kset->num_keys==0 and > >> kset->keys==NULL > >> > >> https://fedorahosted.org/freeipa/ticket/6030 > >> --- > >> daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c > b/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c > >> index 5ca155d..46bf79a 100644 > >> --- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c > >> +++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c > >> @@ -148,7 +148,7 @@ Slapi_Value **ipapwd_encrypt_encode_key(struct > ipapwd_krbcfg *krbcfg, > >> pwd.length = strlen(data->password); > >> } > >> > >> - kset = malloc(sizeof(struct ipapwd_keyset)); > >> + kset = calloc(sizeof(struct ipapwd_keyset)); > > I though that calloc need two arguments > > > > man malloc says: > > void *malloc(size_t size); > > void *calloc(size_t nmemb, size_t size); > > > > LS > Oppss, sorry for this dummy patch. Here is the right one ACK, Simo. -- 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] [WIP] Allow full customisability of CA subject name
On 19.7.2016 11:54, Fraser Tweedale wrote: On Tue, Jul 19, 2016 at 09:36:17AM +0200, Jan Cholasta wrote: Hi, On 15.7.2016 07:05, Fraser Tweedale wrote: On Fri, Jul 15, 2016 at 03:04:48PM +1000, Fraser Tweedale wrote: The attached patch is a work in progress for https://fedorahosted.org/freeipa/ticket/2614 (BZ 828866). I am sharing now to make the approach clear and solicit feedback. It has been tested for server install, replica install (with and without CA) and CA-replica install (all hosts running master+patch). Migration from earlier versions and server/replica/CA install on a CA-less deployment are not yet tested; these will be tested over coming days and patch will be tweaked as necessary. Commit message has a fair bit to say so I won't repeat here but let me know your questions and comments. Thanks, Fraser It does help to attach the patch, of course ^_^ IMO explicit is better than implicit, so instead of introducing additional magic around --subject, I would rather add a new separate option for specifying the CA subject name (I think --ca-subject, for consistency with --ca-signing-algorithm). The current situation - the --subject argument which specifies the not the subject but the subject base, is confusing enough (to say nothing of the limitations that give rise to the RFE). Retaining --subject for specifying the subject base and adding --ca-subject for specifying the *actual* subject DN gets us over the line in terms of the RFE, but does not make the installer less confusing. This is why I made --subject accept the full subject DN, with provisions to retain existing behaviour. IMO if we want to have separate arguments for subject DN and subject base (I am not against it), let's bite the bullet and name arguments accordingly. --subject should be used to specify full Subject DN, --subject-base (or similar) for specifying subject base. IMHO --ca-subject is better than --subject, because it is more explicit whose subject name that is (the CA's). I agree that --subject should be deprecated and replaced with --subject-base. (I intentionally defer discussion of specific behaviour if one, none or both are specified; let's resolve the question or renaming / changing meaning of arguments first). By specifying the option you would override the default "CN=Certificate Authority,$SUBJECT_BASE" subject name. If --external-ca was not used, additional validation would be done to make sure the subject name meets Dogtag's expectations. Actually, it might make sense to always do the additional validation, to be able to print a warning that if a Dogtag-incompatible subject name is used, it won't be possible to change the CA cert chaining from externally signed to self-signed later. Honza -- Jan Cholasta -- Jan Cholasta -- 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] [WIP] Allow full customisability of CA subject name
On Tue, Jul 19, 2016 at 09:36:17AM +0200, Jan Cholasta wrote: > Hi, > > On 15.7.2016 07:05, Fraser Tweedale wrote: > > On Fri, Jul 15, 2016 at 03:04:48PM +1000, Fraser Tweedale wrote: > > > The attached patch is a work in progress for > > > https://fedorahosted.org/freeipa/ticket/2614 (BZ 828866). > > > > > > I am sharing now to make the approach clear and solicit feedback. > > > > > > It has been tested for server install, replica install (with and > > > without CA) and CA-replica install (all hosts running master+patch). > > > > > > Migration from earlier versions and server/replica/CA install on a > > > CA-less deployment are not yet tested; these will be tested over > > > coming days and patch will be tweaked as necessary. > > > > > > Commit message has a fair bit to say so I won't repeat here but let > > > me know your questions and comments. > > > > > > Thanks, > > > Fraser > > > > > It does help to attach the patch, of course ^_^ > > IMO explicit is better than implicit, so instead of introducing additional > magic around --subject, I would rather add a new separate option for > specifying the CA subject name (I think --ca-subject, for consistency with > --ca-signing-algorithm). > The current situation - the --subject argument which specifies the not the subject but the subject base, is confusing enough (to say nothing of the limitations that give rise to the RFE). Retaining --subject for specifying the subject base and adding --ca-subject for specifying the *actual* subject DN gets us over the line in terms of the RFE, but does not make the installer less confusing. This is why I made --subject accept the full subject DN, with provisions to retain existing behaviour. IMO if we want to have separate arguments for subject DN and subject base (I am not against it), let's bite the bullet and name arguments accordingly. --subject should be used to specify full Subject DN, --subject-base (or similar) for specifying subject base. (I intentionally defer discussion of specific behaviour if one, none or both are specified; let's resolve the question or renaming / changing meaning of arguments first). > By specifying the option you would override the default "CN=Certificate > Authority,$SUBJECT_BASE" subject name. If --external-ca was not used, > additional validation would be done to make sure the subject name meets > Dogtag's expectations. Actually, it might make sense to always do the > additional validation, to be able to print a warning that if a > Dogtag-incompatible subject name is used, it won't be possible to change the > CA cert chaining from externally signed to self-signed later. > > Honza > > -- > Jan Cholasta -- 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] 0210 frontend: fix output validation for multiple type choices
On Tue, 19 Jul 2016, Jan Cholasta wrote:
On 19.7.2016 10:40, Alexander Bokovoy wrote:
On Tue, 19 Jul 2016, Jan Cholasta wrote:
On 18.7.2016 10:12, Alexander Bokovoy wrote:
On Mon, 18 Jul 2016, Jan Cholasta wrote:
Hi,
On 16.7.2016 12:46, Alexander Bokovoy wrote:
Hi,
I had some time and was blocked by these bugs to do my tickets so I
actually fixed these three problems that are assigned to Martin
Babinsky. Hopefully, Martin wouldn't be offended by that. :)
--
Output entry elements may have multiple types allowed. We need to
check
all of them to properly validate the output. Right now, thin client
receives type specifications for elements as tuples of types, so
what is seen as 'None' on the server side becomes (type(None),) tuple
on the thin client side.
Change validation to account this by processing each separate type
of the element and account for both None and type(None). Raise type
error only if all of the type checks failed.
https://fedorahosted.org/freeipa/ticket/6061
NACK, this only hides the real issue, which is that trustconfig-show
(and automember-set-default in #6037) claims to return the primary key
of the object in the 'value' output field, but the object does not
have a primary key, so the client rightfully expects None.
Why did it work before introducing thin client?
Because I took the liberty of not putting any extra effort just to
keep old hacks working on thin client. In this case, output.PrimaryKey
was used for the 'value' output field before, which always allowed
unicode in addition to the primary key type. On thin client,
output.Output with the primary key type is used instead, which is less
forgiving and uncovers bogus command definitions. (There aren't any
besides the ones already mentioned, I remember fixing them but the
commit got lost somehow. Oh well.)
This means thin client would not work against old server which would
return a non-primary key value. I think it is unacceptable.
Not true, as this only applies to servers with API schema (4.4+).
Only because thin client does not work against servers without API
schema at all:
[root@f24-master ~]# ipa -vv -e xmlrpc_uri=https://id.vda.li/ipa/xml config-show
ipa: INFO: trying https://id.vda.li/ipa/json
ipa: INFO: Request: {
"id": 0,
"method": "ping",
"params": [
[],
{}
]
}
ipa: INFO: Response: {
"error": null,
"id": 0,
"principal": "ad...@vda.li",
"result": {
"messages": [
{
"code": 13001,
"message": "API Version number was not sent, forward compatibility not guaranteed. Assuming server's API version, 2.156",
"name": "VersionMissing",
"type": "warning"
}
],
"summary": "IPA server version 4.2.3. API version 2.156"
},
"version": "4.2.3"
}
ipa: INFO: Forwarding 'config_show/1' to json server
'https://id.vda.li/ipa/json'
ipa: INFO: Request: {
"id": 0,
"method": "config_show/1",
"params": [
[],
{
"version": "2.210"
}
]
}
ipa: INFO: Response: {
"error": {
"code": 905,
"message": "unknown command 'config_show/1'",
"name": "CommandError"
},
"id": 0,
"principal": "ad...@vda.li",
"result": null,
"version": "4.2.3"
}
ipa: ERROR: unknown command 'config_show/1'
Same happens for every other command so I cannot even test the behavior.
It works against the same server as the thin client is.
[root@f24-master ~]# ipa -vv config-show
ipa: INFO: trying https://f24-master.ipa.ad.test/ipa/json
ipa: INFO: Forwarding 'config_show/1' to json server
'https://f24-master.ipa.ad.test/ipa/json'
ipa: INFO: Request: {
"id": 0,
"method": "config_show/1",
"params": [
[],
{
"version": "2.210"
}
]
}
ipa: INFO: Response: {
"error": null,
"id": 0,
"principal": "ad...@ipa.ad.test",
"result": {
"result": {
"ca_renewal_master_server": "f24-master.ipa.ad.test",
"ca_server_server": [
"f24-master.ipa.ad.test"
],
"dn": "cn=ipaConfig,cn=etc,dc=ipa,dc=ad,dc=test",
"ipa_master_server": [
"f24-master.ipa.ad.test"
],
"ipacertificatesubjectbase": [
"O=IPA.AD.TEST"
],
"ipaconfigstring": [
"AllowNThash"
],
"ipadefaultemaildomain": [
"ipa.ad.test"
],
"ipadefaultloginshell": [
"/bin/sh"
],
"ipadefaultprimarygroup": [
"ipausers"
],
"ipagroupsearchfields": [
"cn,description"
],
"ipahomesrootdir": [
"/home"
],
"ipakrbauthzdata": [
"nfs:NONE",
"MS-PAC"
],
"ipamaxusernamelength": [
"32"
Re: [Freeipa-devel] [PATCH] 0211-0212 Make sure --raw option works for trust-add
On Tue, 19 Jul 2016, Jan Cholasta wrote: On 18.7.2016 12:06, Martin Babinsky wrote: On 07/16/2016 12:50 PM, Alexander Bokovoy wrote: Hi, I had some time and was blocked by these bugs to do my tickets so I actually fixed these three problems that are assigned to Martin Babinsky. Hopefully, Martin wouldn't be offended by that. :) Note that this fix (patch 0211) has potential for a break but also introduces a correct behavior in my view as we should not really have non-lower cased keys in LDAP dictionaries in entry_to_dict() for both normal and --raw modes. - 0211 Since commit a8dd7aa337f25abd938a582d0fcba51d3b356410 if IPA command is called with --raw option, a retrieved LDAP entry's attribute names aren't normalized to lower case when converting the entry to a dictionary. This breaks overall assumption that dictionary keys are in lower case. Partially fixes 'ipa trust-add --raw' issues. https://fedorahosted.org/freeipa/ticket/6059 - 0212 Make sure we display raw values for 'trust-add --raw' case. https://fedorahosted.org/freeipa/ticket/6059 Hi Alexander, I am CC'ing Jan since I hope he knows why a8dd7aa337f25abd938a582d0fcba51d3b356410 was implemented in this way. I think there was a reason behind this decision and we should not revert it without further discussion. I think it was just because with --raw, the raw LDAP entry should be returned, letter case and all. I don't really care if the names are lowercased or not, but you should never assume anything about raw results in your code anyway, so I think the revert would be pointless. There were a few bugs with --raw unrelated to letter case in the past and most of the offending code was fixed, the same should be done here IMO. This is not about LDAP entry, this is about Python dictionary from entry_to_dict(). LDAP attribute name is case-insensitive. We call entry_to_dict() in multiple places and we expect that dictionary keys are lowcased in Python code. I don't think it is fair to have this assumption broken when --raw is used -- after all, we are dealing with Python dictionary and LDAP attributes are case insensitive. -- / Alexander Bokovoy -- 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] Using RPZ to overcome multi Kerberos domains and multiple DNS authorities.
On 18.7.2016 19:44, Jim Glenz wrote:
> IPA DNS configuration using Response Policy Zone (RPZ).
>
> IPA utilizes DNS extensively to locate service records (SRV) and text
> records (TXT) associated with the Kerberos realm.
> IPA also heavily require DNS A records and PTR records to function
> correctly.
> Normally all A,SRV,TXT,PTR records are part of the same DNS domain zone.
>
> The following shows how to decouple IPA "TXT and SRV" records only, and
> pass (forward) all other records to another internal DNS server when
> required to have all records (except SRV and TXT) records in the other DNS
> system.
>
> Note: Below is very customized for specific environment, your environment
> may be different. Just wanted to pass on this DNS trick.
> Methodology used was to implement a BIND instance on at least two servers
> and then configuring a Response Policy Zone (RPZ).
> The RPZ is configured to respond to specific DNS records and forward other
> DNS records onward to next hop DNS.
>
> All A and PTR records should exist in the next hop DNS authoritative server.
> As mentioned, the following solution is very specific to a unique
> environment.
>
> IPA members and clinet servers must have their primary/secondary DNS
> resolvers set to the DNS RPZ BIND servers.
>
>
> Steps
> Create your Master and Slave Bind DNS where RPZ will be used (can be your
> IPA server or any other server having Bind DNS installed)
> Create Response Policy Zone (RPZ) files.
> Test configuration.
>
> Search below for "
> nslookup
> nslookup
>
> nslookup .
> nslookup .
>
> nslookup
> nslookup .
> nslookup -type=TXT _kerberos. Bind-DNS-RPZ2-IP-address>
> nslookup -type=SRV _kerberos-master._tcp. Bind-DNS-RPZ2-IP-address>
> nslookup -type=SRV _kerberos-master._udp. Bind-DNS-RPZ2-IP-address>
> nslookup -type=SRV _kerberos._tcp. Bind-DNS-RPZ2-IP-address>
> nslookup -type=SRV _kerberos._udp. Bind-DNS-RPZ2-IP-address>
> nslookup -type=SRV _kpasswd._tcp. Bind-DNS-RPZ2-IP-address>
> nslookup -type=SRV _kpasswd._udp. Bind-DNS-RPZ2-IP-address>
> nslookup -type=SRV _ldap._tcp. Bind-DNS-RPZ2-IP-address>
>
> nslookup
> nslookup .
> nslookup -type=TXT _kerberos. Bind-DNS-RPZ1-IP-address>
> nslookup -type=SRV _kerberos-master._tcp. Bind-DNS-RPZ1-IP-address>
> nslookup -type=SRV _kerberos-master._udp. Bind-DNS-RPZ1-IP-address>
> nslookup -type=SRV _kerberos._tcp. Bind-DNS-RPZ1-IP-address>
> nslookup -type=SRV _kerberos._udp. Bind-DNS-RPZ1-IP-address>
> nslookup -type=SRV _kpasswd._tcp. Bind-DNS-RPZ1-IP-address>
> nslookup -type=SRV _kpasswd._udp. Bind-DNS-RPZ1-IP-address>
> nslookup -type=SRV _ldap._tcp. Bind-DNS-RPZ1-IP-address>
>
> nslookup google.com
> nslookup google.com
>
> nslookup -type=ptr Bind-DNS-RPZ2-IP-address>
> nslookup -type=ptr Bind-DNS-RPZ1-IP-address>
> nslookup -type=ptr Bind-DNS-RPZ2-IP-address>
> nslookup -type=ptr Bind-DNS-RPZ1-IP-address>
>
>
> Will be referencing reverse.arpa zone 10.x.x.x internal network. Adjust as
> necessary for your environment.
>
> Appendix A: Primary IPA DNS /etc/named.conf file
> # cat named.conf
> options {
> // Put files that named is allowed to write in the data/ directory:
> directory "/var/named"; // the default
> dump-file "data/cache_dump.db";
> statistics-file "data/named_stats.txt";
> memstatistics-file "data/named_mem_stats.txt";
> #forward first;
> forwarders {
> ;
> ;
> };
> response-policy {zone ""; };
>
> // Any host is permitted to issue recursive queries
> allow-recursion { any; };
>
> tkey-gssapi-credential "DNS/";
> tkey-domain "";
> };
>
> /* If you want to enable debugging, eg. using the 'rndc trace' command,
> * By default, SELinux policy does not allow named to modify the /var/named
> directory,
> * so put the default debug log file in data/ :
> */
> logging {
> channel default_debug {
> file "data/named.run";
> severity dynamic;
> };
> };
>
> zone "" IN {
> type master;
> file "";
> also-notify {;};
> };
>
> zone "10.in-addr.arpa" IN {
> type forward;
> forwarders {
> ;
> ;
> };
> };
>
> include "/etc/named.rfc1912.zones";
>
> # dynamic not used, remark out.
> #dynamic-db "ipa" {
> #library "ldap.so";
> #};
>
> Appendix B: Primary IPA DNS /var/named/ file
> $ORIGIN .
> $TTL 86400 ; 1 day
> .rpz IN SOA localhost. root.localhost. (
> 201505162150 ; serial
> 3600 ; refresh (1 hour)
> 1800 ; retry (30 minutes)
> 604800 ; expire (1 week)
>
Re: [Freeipa-devel] [PATCH] 0211-0212 Make sure --raw option works for trust-add
On 18.7.2016 12:06, Martin Babinsky wrote: On 07/16/2016 12:50 PM, Alexander Bokovoy wrote: Hi, I had some time and was blocked by these bugs to do my tickets so I actually fixed these three problems that are assigned to Martin Babinsky. Hopefully, Martin wouldn't be offended by that. :) Note that this fix (patch 0211) has potential for a break but also introduces a correct behavior in my view as we should not really have non-lower cased keys in LDAP dictionaries in entry_to_dict() for both normal and --raw modes. - 0211 Since commit a8dd7aa337f25abd938a582d0fcba51d3b356410 if IPA command is called with --raw option, a retrieved LDAP entry's attribute names aren't normalized to lower case when converting the entry to a dictionary. This breaks overall assumption that dictionary keys are in lower case. Partially fixes 'ipa trust-add --raw' issues. https://fedorahosted.org/freeipa/ticket/6059 - 0212 Make sure we display raw values for 'trust-add --raw' case. https://fedorahosted.org/freeipa/ticket/6059 Hi Alexander, I am CC'ing Jan since I hope he knows why a8dd7aa337f25abd938a582d0fcba51d3b356410 was implemented in this way. I think there was a reason behind this decision and we should not revert it without further discussion. I think it was just because with --raw, the raw LDAP entry should be returned, letter case and all. I don't really care if the names are lowercased or not, but you should never assume anything about raw results in your code anyway, so I think the revert would be pointless. There were a few bugs with --raw unrelated to letter case in the past and most of the offending code was fixed, the same should be done here IMO. I had the fix for trust-add ready on Friday but was unable to test it properly because of some issues with my VMs. I am attaching it for reference, since it is similar to your patch 212 but relies on attrs_list passed in to LDAP search in order to fetch lowercased attributes. -- Jan Cholasta -- 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] 0210 frontend: fix output validation for multiple type choices
On 19.7.2016 10:40, Alexander Bokovoy wrote: On Tue, 19 Jul 2016, Jan Cholasta wrote: On 18.7.2016 10:12, Alexander Bokovoy wrote: On Mon, 18 Jul 2016, Jan Cholasta wrote: Hi, On 16.7.2016 12:46, Alexander Bokovoy wrote: Hi, I had some time and was blocked by these bugs to do my tickets so I actually fixed these three problems that are assigned to Martin Babinsky. Hopefully, Martin wouldn't be offended by that. :) -- Output entry elements may have multiple types allowed. We need to check all of them to properly validate the output. Right now, thin client receives type specifications for elements as tuples of types, so what is seen as 'None' on the server side becomes (type(None),) tuple on the thin client side. Change validation to account this by processing each separate type of the element and account for both None and type(None). Raise type error only if all of the type checks failed. https://fedorahosted.org/freeipa/ticket/6061 NACK, this only hides the real issue, which is that trustconfig-show (and automember-set-default in #6037) claims to return the primary key of the object in the 'value' output field, but the object does not have a primary key, so the client rightfully expects None. Why did it work before introducing thin client? Because I took the liberty of not putting any extra effort just to keep old hacks working on thin client. In this case, output.PrimaryKey was used for the 'value' output field before, which always allowed unicode in addition to the primary key type. On thin client, output.Output with the primary key type is used instead, which is less forgiving and uncovers bogus command definitions. (There aren't any besides the ones already mentioned, I remember fixing them but the commit got lost somehow. Oh well.) This means thin client would not work against old server which would return a non-primary key value. I think it is unacceptable. Not true, as this only applies to servers with API schema (4.4+). Aside from that, comparing "(type(None),) is None" will never give you True on the thin client side. At the server side we have "None is None" and that works. So the question is also why there is a change like that between client and server sides? I don't see how this has anything to do with anything. In output validation, "type = None" means that value of any type is allowed, "type = (type(None),)" means that only None is allowed. Nothing changed with regard to that in thin client. Really? Previous code worked against corresponding server, new code doesn't work against the very same server. I consider it a breakage. Really, since commit b6e4972e. If something is broken, please file a ticket. -- Jan Cholasta -- 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
[Freeipa-devel] [PATCH 0029][Tests] Adding authentication test to trust test suite
Hi,
this patch adds authentication test (specifically "kinit -E
ipauser@IPADOMAIN") to basic trust test suite, as requested by Sumit.
Intended to be applied after my patches 25.4 and 26.3 (already waiting
to be pushed).
Lenka
From 394304d23ef752c30cf1f4d69d5e6116fd41ad2d Mon Sep 17 00:00:00 2001
From: Lenka Doudova
Date: Mon, 18 Jul 2016 14:38:18 +0200
Subject: [PATCH] Tests: Adding authentication test to basic trust test suite
Providing missing test case verifying authentication as IPA user, namely "kinit -E ipauser@IPADOMAIN".
---
ipatests/test_integration/test_trust.py | 19 +++
ipatests/test_integration/util.py | 13 +
2 files changed, 32 insertions(+)
diff --git a/ipatests/test_integration/test_trust.py b/ipatests/test_integration/test_trust.py
index e3fe9c89e9cd1357c8913d8fcd98699906f07f85..3dc1056dea82d0fe21a983ffe5354ff33e8ad8a3 100644
--- a/ipatests/test_integration/test_trust.py
+++ b/ipatests/test_integration/test_trust.py
@@ -161,6 +161,25 @@ class TestBasicADTrust(ADTrustBase):
assert re.search(testuser_regex, result.stdout_text)
+def test_ipauser_authentication(self):
+ipauser = u'tuser'
+original_passwd = 'Secret123'
+new_passwd = 'userPasswd123'
+
+# create an ipauser for this test
+self.master.run_command(['ipa', 'user-add', ipauser, '--first=Test',
+ '--last=User', '--password'],
+ stdin_text=original_passwd)
+
+# change password for the user to be able to kinit
+util.unlock_principal_password(ipauser, original_passwd, new_passwd,
+ self.master)
+
+# try to kinit as ipauser
+self.master.run_command(
+['kinit', '-E','{0}@{1}'.format(ipauser, self.master.domain.name)],
+stdin_text=new_passwd)
+
def test_remove_nonposix_trust(self):
tasks.remove_trust_with_ad(self.master, self.ad_domain)
tasks.clear_sssd_cache(self.master)
diff --git a/ipatests/test_integration/util.py b/ipatests/test_integration/util.py
index 594737b6d753d476cd06aeb0d5cd376b7ca46467..b317beed7756bdc50c6a17c199f4970b5af15303 100644
--- a/ipatests/test_integration/util.py
+++ b/ipatests/test_integration/util.py
@@ -20,6 +20,8 @@
import time
import re
+from ipaplatform.paths import paths
+from ipapython.ipautil import run
def run_repeatedly(host, command, assert_zero_rc=True, test=None,
timeout=30, **kwargs):
@@ -75,3 +77,14 @@ def get_host_ip_with_hostmask(host):
if match:
return match.group('full_ip')
+
+
+def unlock_principal_password(user, oldpw, newpw, master):
+container_user = "cn=users,cn=accounts"
+basedn = master.domain.basedn
+
+userdn = "uid={},{},{}".format(user, container_user, basedn)
+
+args = [paths.LDAPPASSWD, '-D', userdn, '-w', oldpw, '-a', oldpw,
+'-s', newpw, '-x']
+return run(args)
--
2.7.4
--
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] 0210 frontend: fix output validation for multiple type choices
On Tue, 19 Jul 2016, Jan Cholasta wrote: On 18.7.2016 10:12, Alexander Bokovoy wrote: On Mon, 18 Jul 2016, Jan Cholasta wrote: Hi, On 16.7.2016 12:46, Alexander Bokovoy wrote: Hi, I had some time and was blocked by these bugs to do my tickets so I actually fixed these three problems that are assigned to Martin Babinsky. Hopefully, Martin wouldn't be offended by that. :) -- Output entry elements may have multiple types allowed. We need to check all of them to properly validate the output. Right now, thin client receives type specifications for elements as tuples of types, so what is seen as 'None' on the server side becomes (type(None),) tuple on the thin client side. Change validation to account this by processing each separate type of the element and account for both None and type(None). Raise type error only if all of the type checks failed. https://fedorahosted.org/freeipa/ticket/6061 NACK, this only hides the real issue, which is that trustconfig-show (and automember-set-default in #6037) claims to return the primary key of the object in the 'value' output field, but the object does not have a primary key, so the client rightfully expects None. Why did it work before introducing thin client? Because I took the liberty of not putting any extra effort just to keep old hacks working on thin client. In this case, output.PrimaryKey was used for the 'value' output field before, which always allowed unicode in addition to the primary key type. On thin client, output.Output with the primary key type is used instead, which is less forgiving and uncovers bogus command definitions. (There aren't any besides the ones already mentioned, I remember fixing them but the commit got lost somehow. Oh well.) This means thin client would not work against old server which would return a non-primary key value. I think it is unacceptable. Aside from that, comparing "(type(None),) is None" will never give you True on the thin client side. At the server side we have "None is None" and that works. So the question is also why there is a change like that between client and server sides? I don't see how this has anything to do with anything. In output validation, "type = None" means that value of any type is allowed, "type = (type(None),)" means that only None is allowed. Nothing changed with regard to that in thin client. Really? Previous code worked against corresponding server, new code doesn't work against the very same server. I consider it a breakage. -- / Alexander Bokovoy -- 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] 0210 frontend: fix output validation for multiple type choices
On 18.7.2016 10:12, Alexander Bokovoy wrote:
On Mon, 18 Jul 2016, Jan Cholasta wrote:
Hi,
On 16.7.2016 12:46, Alexander Bokovoy wrote:
Hi,
I had some time and was blocked by these bugs to do my tickets so I
actually fixed these three problems that are assigned to Martin
Babinsky. Hopefully, Martin wouldn't be offended by that. :)
--
Output entry elements may have multiple types allowed. We need to check
all of them to properly validate the output. Right now, thin client
receives type specifications for elements as tuples of types, so
what is seen as 'None' on the server side becomes (type(None),) tuple
on the thin client side.
Change validation to account this by processing each separate type
of the element and account for both None and type(None). Raise type
error only if all of the type checks failed.
https://fedorahosted.org/freeipa/ticket/6061
NACK, this only hides the real issue, which is that trustconfig-show
(and automember-set-default in #6037) claims to return the primary key
of the object in the 'value' output field, but the object does not
have a primary key, so the client rightfully expects None.
Why did it work before introducing thin client?
Because I took the liberty of not putting any extra effort just to keep
old hacks working on thin client. In this case, output.PrimaryKey was
used for the 'value' output field before, which always allowed unicode
in addition to the primary key type. On thin client, output.Output with
the primary key type is used instead, which is less forgiving and
uncovers bogus command definitions. (There aren't any besides the ones
already mentioned, I remember fixing them but the commit got lost
somehow. Oh well.)
Aside from that, comparing "(type(None),) is None" will never give you
True on the thin client side. At the server side we have "None is None"
and that works. So the question is also why there is a change like that
between client and server sides?
I don't see how this has anything to do with anything. In output
validation, "type = None" means that value of any type is allowed, "type
= (type(None),)" means that only None is allowed. Nothing changed with
regard to that in thin client.
A proper fix would be to set "has_output = output.simple_value" for
these commands (all of automember_default_group_{set,remove,show},
trustconfig_{mod,show}).
Honza
--
Jan Cholasta
--
Jan Cholasta
--
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 0059] Fix to ipa-cacert-manage man and help differences
On 07/15/2016 02:09 PM, Stanislav Laznicka wrote: https://fedorahosted.org/freeipa/ticket/6013 Hi Stanislav, thanks for your patch. As CERTFILE is added in the arguments for install, I would suggest to mention it in the command description. For instance: install - Install a CA certificate This command can be used to install the certificate contained in CERTFILE as a new CA certificate to IPA. Flo. -- 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] 0023 Bug in the ipapwd plugin
On 07/13/2016 10:02 PM, Lukas Slebodnik wrote:
On (13/07/16 16:50), thierry bordaz wrote:
https://fedorahosted.org/freeipa/ticket/6030
>From 4efedc5e674db92f9f7c160429df543422ed8afb Mon Sep 17 00:00:00 2001
From: Thierry Bordaz
Date: Wed, 13 Jul 2016 15:34:20 +0200
Subject: [PATCH] Ticket 6030 Bug in the ipapwd plugin
ipapwd_encrypt_encode_key allocates 'kset' on the heap but
with num_keys and keys not being initialized.
Then ipa_krb5_generate_key_data initializes them with the
generated keys.
If ipa_krb5_generate_key_data fails (here EINVAL meaning no
principal->realm.data), num_keys and keys are left uninitialized.
Upon failure, ipapwd_keyset_free is called to free 'kset'
that contains random num_keys and keys.
allocates kset with calloc so that kset->num_keys==0 and
kset->keys==NULL
https://fedorahosted.org/freeipa/ticket/6030
---
daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c
b/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c
index 5ca155d..46bf79a 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c
@@ -148,7 +148,7 @@ Slapi_Value **ipapwd_encrypt_encode_key(struct
ipapwd_krbcfg *krbcfg,
pwd.length = strlen(data->password);
}
-kset = malloc(sizeof(struct ipapwd_keyset));
+kset = calloc(sizeof(struct ipapwd_keyset));
I though that calloc need two arguments
man malloc says:
void *malloc(size_t size);
void *calloc(size_t nmemb, size_t size);
LS
Oppss, sorry for this dummy patch. Here is the right one
thanks
thierry
>From 84efaaab758a07fa8cee2f6ad44ba770b67b4bbc Mon Sep 17 00:00:00 2001
From: Thierry Bordaz
Date: Mon, 18 Jul 2016 15:00:02 +0200
Subject: [PATCH] Heap corruption in ipapwd plugin
ipapwd_encrypt_encode_key allocates 'kset' on the heap but
with num_keys and keys not being initialized.
Then ipa_krb5_generate_key_data initializes them with the
generated keys.
If ipa_krb5_generate_key_data fails (here EINVAL meaning no
principal->realm.data), num_keys and keys are left uninitialized.
Upon failure, ipapwd_keyset_free is called to free 'kset'
that contains random num_keys and keys.
allocates kset with calloc so that kset->num_keys==0 and
kset->keys==NULL
https://fedorahosted.org/freeipa/ticket/6030
---
daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c
index 9c62f05..7b2f341 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/encoding.c
@@ -157,7 +157,7 @@ Slapi_Value **ipapwd_encrypt_encode_key(struct ipapwd_krbcfg *krbcfg,
pwd.length = strlen(data->password);
}
-kset = malloc(sizeof(struct ipapwd_keyset));
+kset = (struct ipapwd_keyset *) calloc(1, sizeof(struct ipapwd_keyset));
if (!kset) {
LOG_OOM();
goto enc_error;
--
2.5.0
--
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] 963 unite log file name of ipa-ca-install
On 19.7.2016 09:27, Petr Vobornik wrote: On 07/19/2016 08:01 AM, Jan Cholasta wrote: Hi, On 18.7.2016 18:50, Florence Blanc-Renaud wrote: On 07/15/2016 04:29 PM, Petr Vobornik wrote: ipa-ca-install said that it used /var/log/ipareplica-ca-install.log but in fact it used /var/log/ipaserver-ca-install.log This patch unites it to ipaserver-ca-install.log It was chosen because ipa-ca-install can be also used on master on CA-less -> CA conversion. Term "server" is valid for both master and replica. https://fedorahosted.org/freeipa/ticket/6088 Looks good to me. Ack Does not look so good to me, "ipareplica-ca-install.log" is in fact the original file name used since ipa-ca-install was introduced (in commit 8a32bb3746802a29b2655e4ad2cbbba8481e1eaf), so why the switch to "ipaserver-ca-install.log"? Honza Ideally it would be ipa-ca-install.log but for backwards compatibility, let's stick with one which we have. AFAIK the framework(run_script methodú doesn't support switching the log name which is printed in error message depending on usage. Therefore the universal was chosen - ipaserver-ca-install.log. It was introduced by your commit d27e77adc56f5a04f3bdd1aaed5440a89ed3acad Correct, but only for CA-less to CA-ful conversion. It is used exclusively only since commit 958996b9cc55b6e9ecdc23981e79599ec6826b4c and by accident, AFAICT. And I see, that I used wrong ticket number in the commit. Correct is https://fedorahosted.org/freeipa/ticket/6086 Proper solution might be to rework main and __main__ function in ipa-ca-install but IMO we spent too much time on this already. -- Jan Cholasta -- 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] [WIP] Allow full customisability of CA subject name
Hi, On 15.7.2016 07:05, Fraser Tweedale wrote: On Fri, Jul 15, 2016 at 03:04:48PM +1000, Fraser Tweedale wrote: The attached patch is a work in progress for https://fedorahosted.org/freeipa/ticket/2614 (BZ 828866). I am sharing now to make the approach clear and solicit feedback. It has been tested for server install, replica install (with and without CA) and CA-replica install (all hosts running master+patch). Migration from earlier versions and server/replica/CA install on a CA-less deployment are not yet tested; these will be tested over coming days and patch will be tweaked as necessary. Commit message has a fair bit to say so I won't repeat here but let me know your questions and comments. Thanks, Fraser It does help to attach the patch, of course ^_^ IMO explicit is better than implicit, so instead of introducing additional magic around --subject, I would rather add a new separate option for specifying the CA subject name (I think --ca-subject, for consistency with --ca-signing-algorithm). By specifying the option you would override the default "CN=Certificate Authority,$SUBJECT_BASE" subject name. If --external-ca was not used, additional validation would be done to make sure the subject name meets Dogtag's expectations. Actually, it might make sense to always do the additional validation, to be able to print a warning that if a Dogtag-incompatible subject name is used, it won't be possible to change the CA cert chaining from externally signed to self-signed later. Honza -- Jan Cholasta -- 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] 963 unite log file name of ipa-ca-install
On 07/19/2016 08:01 AM, Jan Cholasta wrote: > Hi, > > On 18.7.2016 18:50, Florence Blanc-Renaud wrote: >> On 07/15/2016 04:29 PM, Petr Vobornik wrote: >>> ipa-ca-install said that it used >>> /var/log/ipareplica-ca-install.log >>> but in fact it used >>> /var/log/ipaserver-ca-install.log >>> >>> This patch unites it to ipaserver-ca-install.log >>> >>> It was chosen because ipa-ca-install can be also used on >>> master on CA-less -> CA conversion. >>> >>> Term "server" is valid for both master and replica. >>> >>> https://fedorahosted.org/freeipa/ticket/6088 >>> >>> >>> >> >> Looks good to me. >> Ack > > Does not look so good to me, "ipareplica-ca-install.log" is in fact the > original file name used since ipa-ca-install was introduced (in commit > 8a32bb3746802a29b2655e4ad2cbbba8481e1eaf), so why the switch to > "ipaserver-ca-install.log"? > > Honza > Ideally it would be ipa-ca-install.log but for backwards compatibility, let's stick with one which we have. AFAIK the framework(run_script methodú doesn't support switching the log name which is printed in error message depending on usage. Therefore the universal was chosen - ipaserver-ca-install.log. It was introduced by your commit d27e77adc56f5a04f3bdd1aaed5440a89ed3acad And I see, that I used wrong ticket number in the commit. Correct is https://fedorahosted.org/freeipa/ticket/6086 Proper solution might be to rework main and __main__ function in ipa-ca-install but IMO we spent too much time on this already. -- Petr Vobornik -- 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 0058] Make get_entries not ignore its size_limit argument
Hi, On 14.7.2016 14:36, Stanislav Laznicka wrote: Hello, This patch fixes https://fedorahosted.org/freeipa/ticket/5640. With not so much experience with the framework, it raises question in my head whether ipaldap.get_entries is used properly throughout the system - does it always assume that it gets ALL the requested entries or just a few of those as configured by the 'ipaSearchRecordsLimit' attribute of ipaConfig.etc which it actually gets? That depends. If you call get_entries() on the ldap2 plugin (which is usually the case in the framework), then ipaSearchRecordsLimit is used. If you call it on some arbitrary LDAPClient instance, the hardcoded default (= unlimited) is used. One spot that I know the get_entries method was definitely not used properly before this patch is in the baseldap.LDAPObject.get_memberindirect() method: 692 result = self.backend.get_entries( 693 self.api.env.basedn, 694 filter=filter, 695 attrs_list=['member'], 696 size_limit=-1, # paged search will get everything anyway 697 paged_search=True) which to me seems kind of important if the environment size_limit is not set properly :) The patch does not fix the non-propagation of the paged_search, though. Why do you think size_limit is not used properly here? Anyway, this ticket is not really easily fixable without more profound changes. Often, multiple LDAP searches are done during command execution. What do you do with the size limit then? Do you pass the same size limit to all the searches? Do you subtract the result size from the size limit after each search? Do you do something else with it? ... The answer is that it depends on the purpose of each individual LDAP search (like in get_memberindirect() above, we have to do unlimited search, otherwise the resulting entry would be incomplete), and fixing this accross the whole framework is a non-trivial task. Honza -- Jan Cholasta -- 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 0183] ipa-advise: correct handling of plugin namespace iteration
On 07/18/2016 08:46 AM, Jan Cholasta wrote:
Hi,
On 11.7.2016 14:18, Martin Babinsky wrote:
https://fedorahosted.org/freeipa/ticket/6044
Note that you should use .name rather than .__name__ to get plugin
names, otherwise the code won't work with plugins with non-default names.
There currently aren't any Advice plugins with non-default name, but I
would rather fix this now to avoid surprises later.
Honza
I didn't realize this when doing the patch, here's the fix for that.
I have attached the original closed ticket to the commit message, should
I create a new ticket for such a small change?
--
Martin^3 Babinsky
From 5da15a1a56174d06120c8296d8878e71ec8a66a4 Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Mon, 18 Jul 2016 10:44:23 +0200
Subject: [PATCH] advise: Use `name` instead of `__name__` to get plugin names
This change will allow ipa-advise to correctly handle advise plugins with
custom names.
https://fedorahosted.org/freeipa/ticket/6044
---
ipaserver/advise/base.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ipaserver/advise/base.py b/ipaserver/advise/base.py
index a2dc9ccee93811da415c1e1eb0b57f47ac817a3f..f7e8ef5e49c7efc18d5c29583183e908e1284eae 100644
--- a/ipaserver/advise/base.py
+++ b/ipaserver/advise/base.py
@@ -168,11 +168,11 @@ class IpaAdvise(admintool.AdminTool):
self.print_header('List of available advices')
max_keyword_len = max(
-(len(advice.__name__) for advice in advise_api.Advice))
+(len(advice.name) for advice in advise_api.Advice))
for advice in advise_api.Advice:
description = getattr(advice, 'description', '')
-keyword = advice.__name__.replace('_', '-')
+keyword = advice.name.replace('_', '-')
# Compute the number of spaces needed for the table to be aligned
offset = max_keyword_len - len(keyword)
--
2.7.4
--
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] cert-show: show subject alternative names
On Tue, Jul 19, 2016 at 08:50:34AM +0200, Jan Cholasta wrote: > Hi, > > On 14.7.2016 13:44, Fraser Tweedale wrote: > > Hi all, > > > > The attached patch includes SANs in cert-show output. If you have > > certs with esoteric altnames (especially any that are more than just > > ASN.1 string types), please test with those certs. > > > > https://fedorahosted.org/freeipa/ticket/6022 > > I think it would be better to have a separate attribute for each supported > SAN type rather than cramming everything into subject_alt_name. That way if > you care only about a single specific type you won't have to go through all > the values and parse them. Also it would allow you to use param types > appropriate to the SAN types (DNSNameParam for DNS names, Principal for > principal names, etc.) > You are right; that would be much better. > Nitpick: please don't mix moving existing stuff and adding new stuff in a > single patch. > Will cut new patches to address both of these points. Thanks, Fraser -- 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] [freeipa] #6002: Default CA can be used without an ACL
On Tue, Jul 19, 2016 at 08:26:22AM +0200, Jan Cholasta wrote: > Hi, > > On 4.7.2016 09:06, Fraser Tweedale wrote: > > On Tue, Jun 28, 2016 at 01:47:23PM -, freeipa wrote: > > > #6002: Default CA can be used without an ACL > > > > > > Comment (by ftweedal): > > > > > > This is expected behaviour; if a CA ACL does not reference any CAs, > > > and does not have cacat=all, then it is assumed to refer to the > > > default CA. This is for backwards compatibility with existing > > > CA ACLs, which do not reference any CAs but did (and still do) > > > allow access to IPA CA. > > > > > > Leaving open for discussion about whether to break compatibility > > > for a more consistent behaviour. > > > > > Didn't get any feedback in the ticket yet so raising on list for > > visibility. If people agree with current behaviour I can add a > > clarification to caacl plugin help text and close out this ticket. > > (Sorry for the late reply, I was on vacation the last 2 weeks.) > > I would very much prefer if this was consistent with (literally) every other > member list+category attribute, that is, no member and no category means the > rule never matches. > > While documenting this as an exception to the above rule is the easy way > out, IMHO adhering to the rule is even better - anyone who touched HBAC or > sudo in IPA would immediately know their way around CA ACLs without having > to read the documentation at all, which is a win, because people don't > generally read documentation until something goes wrong. The current > behavior might surprise them, even if documented properly (it sure surprised > me at first :-). > > BTW I think this can be done without breaking compatibility, e.g. by using a > new objectclass to distinguish between "old" (CA is always implicitly the > top-level CA) and "new" (CAs are specified using the member and category > attributes) CA ACLs. > > Honza > Is there precedent of "varying behaviour based on objectclass" in IPA? Would it go along the lines of... 1. subtype 'ipaCaAcl' object class (let us call it 'ipaCaAclWithCa' for sake of discussion). (Note that moving the 'ipaCa' attribute to be part of 'ipaCaAclWithCa' would not be backwards compatible with v4.4 as currently released, so it would remain in 'ipaCaAcl' objectclass.) 2. Upgrade script to take 'ipaCaAcl' objects that are NOT 'ipaCaAclWithCa', and make them so, while adding the IPA CA. Is this what you have in mind? If so, I don't think there is actually a need to vary the behaviour based on object class, other than during upgrade. The reason I was not doing upgrades in the first place was because I could not in distinguish between "old" CA ACLs, and "new" CA ACLs that don't have any CAs defined. However, adding a new objectclass resolves this ambiguity. Lmk what you think; I could be way off track :) Cheers, Fraser -- 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
