[Freeipa-devel] [PATCH 0023][Tests] Fix frontend tests - #5987

[Lenka Doudova]

Hi,

I've made patch to fix for https://fedorahosted.org/freeipa/ticket/5987.

Please note, that this patch must be applied on top on my patch no. 
0018, which provides other fixes on the same file (and same test).



Lenka

From ef04d8d013643fd5d2a0a7de32ab23686e46531d Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Tue, 28 Jun 2016 06:27:41 +0200
Subject: [PATCH] Tests: Fix frontend tests

Test ipatests/test_ipalib/test_frontend.py::test_Command::test_validate fails due to attributes that are no longer present, therefore assertion for these values was removed.

https://fedorahosted.org/freeipa/ticket/5987
---
 ipatests/test_ipalib/test_frontend.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/ipatests/test_ipalib/test_frontend.py b/ipatests/test_ipalib/test_frontend.py
index 0a5951159978706a2265bb688f08deca85de4c0d..c3dd9104c152dc9e9da774a53e0c6a42b9a89bf8 100644
--- a/ipatests/test_ipalib/test_frontend.py
+++ b/ipatests/test_ipalib/test_frontend.py
@@ -493,10 +493,7 @@ class test_Command(ClassChecker):
 fail['option0'] = u'whatever'
 e = raises(errors.ValidationError, sub.validate, **fail)
 assert_equal(e.name, u'option0')
-assert_equal(e.value, u'whatever')
 assert_equal(e.error, u"must equal 'option0'")
-assert e.rule.__class__.__name__ == 'Rule'
-assert e.index is None
 
 # Check with a missing required arg
 fail = dict(okay)
-- 
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] 0008 Do not allow installation in FIPS mode

[Rob Crittenden]

Florence Blanc-Renaud wrote:

Hi all,

thanks for your suggestions. Updated patch attached.
Flo.



The invocation in ipactl should say server, not client.

Otherwise LGTM (untested).

rob

--
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] 0008 Do not allow installation in FIPS mode

[Florence Blanc-Renaud]

On 06/27/2016 03:55 PM, Rob Crittenden wrote:

Petr Spacek wrote:

On 27.6.2016 08:38, Florence Blanc-Renaud wrote:

Hi,

this fix is a port of Bug 1131570 - Do not allow IdM
server/replica/client
installation in a FIPS-140 mode
It prevents installation of FreeIPA if the host is fips-enabled.

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

freeipa-frenaud-0008-Do-not-allow-installation-in-FIPS-mode.patch


>From afecbb3d228cf1d6cee59da53bf7a803f030d0b1 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud 
Date: Fri, 24 Jun 2016 16:16:22 +0200
Subject: [PATCH] Do not allow installation in FIPS mode

https://fedorahosted.org/freeipa/ticket/5761
---
  client/ipa-client-install  | 4 
  install/tools/ipactl   | 6 ++
  ipaserver/install/server/install.py| 5 +
  ipaserver/install/server/replicainstall.py | 5 +
  4 files changed, 20 insertions(+)

diff --git a/client/ipa-client-install b/client/ipa-client-install
index
0a601b63118b0a3568066495837121c65e5df04f..f80ff9c469709ea3b63902610b3b8b5c35448904
100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -3064,6 +3064,10 @@ def main():

  if not os.getegid() == 0:
  sys.exit("\nYou must be root to run ipa-client-install.\n")
+if os.path.exists('/proc/sys/crypto/fips_enabled'):
+with open('/proc/sys/crypto/fips_enabled', 'r') as f:


Usually it is safer to call open() and catch exception if the file
does not
exist. The code above has inherent problem with race-conditions
between time
of check (path.exists) and time of use (open).

Of course it is not a problem here because this file is part of kernel's
interface but in general please use the try: open() except: form.


+if f.read().strip() != '0':
+sys.exit("Cannot install IPA client in FIPS mode")


Personally I would like to see more informative messages.

I would recommend something like " is not supported in FIPS
mode".

In my eyes it is difference between "How do I ...? You dont!" vs "How
do I
...? Sorry, we do not support that right now."


Given that this code is duplicated 4 times I'd also move it to a
function in ipapython, is_fips_enabled() or something .

rob




Sorry for nitpicking! :-)

Petr^2 Spacek




  tasks.check_selinux_status()
  logging_setup(options)
  root_logger.debug(
diff --git a/install/tools/ipactl b/install/tools/ipactl
index
547b21d875dff7231fae8dfc10faf995b0ca230b..9c68fffe73bfdd97789907226f8765c09707d552
100755
--- a/install/tools/ipactl
+++ b/install/tools/ipactl
@@ -545,6 +545,12 @@ def main():
  elif args[0] != "start" and args[0] != "stop" and args[0] !=
"restart" and args[0] != "status":
  raise IpactlError("Unrecognized action [" + args[0] + "]", 2)

+if (args[0] in ('start', 'restart') and
+os.path.exists('/proc/sys/crypto/fips_enabled')):
+with open('/proc/sys/crypto/fips_enabled', 'r') as f:
+if f.read().strip() != '0':
+raise IpactlError("Cannot start IPA server in FIPS
mode")
+
  # check if IPA is configured at all
  try:
  check_IPA_configuration()
diff --git a/ipaserver/install/server/install.py
b/ipaserver/install/server/install.py
index
930cca7b31ca06c04ab92deff49b6a4f198c2b6e..0c0683733ef38444a82d085f771596a9b066ef1d
100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -319,6 +319,11 @@ def install_check(installer):
  external_ca_file = installer._external_ca_file
  http_ca_cert = installer._ca_cert

+if os.path.exists('/proc/sys/crypto/fips_enabled'):
+with open('/proc/sys/crypto/fips_enabled', 'r') as f:
+if f.read().strip() != '0':
+sys.exit("Cannot install IPA server in FIPS mode")
+
  tasks.check_selinux_status()

  if options.master_password:
diff --git a/ipaserver/install/server/replicainstall.py
b/ipaserver/install/server/replicainstall.py
index
52b2ea5b0691cd99c6cb566af5a15af3b2dffb14..a2946339c7aeee8529f6ecf8ec4d85c9291fd291
100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -485,6 +485,11 @@ def install_check(installer):
  options = installer
  filename = installer.replica_file

+if os.path.exists('/proc/sys/crypto/fips_enabled'):
+with open('/proc/sys/crypto/fips_enabled', 'r') as f:
+if f.read().strip() != '0':
+sys.exit("Cannot install IPA server in FIPS mode")
+
  tasks.check_selinux_status()

  if is_ipa_configured():
-- 2.7.4





Hi all,

thanks for your suggestions. Updated patch attached.
Flo.

>From 26d77345490711934cf7a63bb0cef670b3e5c85c Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud 
Date: Mon, 27 Jun 2016 10:23:14 +0200
Subject: [PATCH] Do not allow installation in FIPS mode

https://fedorahosted.org/freeipa/ticket/5761
---
 client/ipa-client-install  |  5 

Re: [Freeipa-devel] Broken pki 10.3.3-1 packages in freeipa-master COPR

[Lukas Slebodnik]

On (27/06/16 17:55), Milan Kubík wrote:
>Hi all,
>
>the pki packages that are currently in the COPR repo [1] are broken. There is
>a conflict between pki-server and pki-base:
>
>Error: Transaction check error:
>  file /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyc 
> from install of pki-server-10.3.3-1.fc24.noarch conflicts with file from 
> package pki-base-10.3.3-1.fc24.noarch
>  file /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyo 
> from install of pki-server-10.3.3-1.fc24.noarch conflicts with file from 
> package pki-base-10.3.3-1.fc24.noarch
>
>[1]: https://copr.fedorainfracloud.org/coprs/g/freeipa/freeipa-master/
>
I can see the same with pki-core in fedora 24 updates-testing.
File a fedora bug.

LS

-- 
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] Broken pki 10.3.3-1 packages in freeipa-master COPR

[Milan Kubík]

Hi all,

the pki packages that are currently in the COPR repo [1] are broken. 
There is a conflict between pki-server and pki-base:


Error: Transaction check error:
  file /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyc 
from install of pki-server-10.3.3-1.fc24.noarch conflicts with file from 
package pki-base-10.3.3-1.fc24.noarch
  file /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyo 
from install of pki-server-10.3.3-1.fc24.noarch conflicts with file from 
package pki-base-10.3.3-1.fc24.noarch

[1]: https://copr.fedorainfracloud.org/coprs/g/freeipa/freeipa-master/

Regards

--
Milan Kubik

-- 
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] 0064: webui: simplify confirmation messages in confirmation dialogs

[Pavel Vomacka]

Hello,

Please review attached patch which simplifies confirmation messages for 
'remove cert hold' and 'restore cert' actions.


--
Pavel^3 Vomacka

From d3d10e8481be242dac5f66cc1ba6c622696a6758 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Mon, 27 Jun 2016 17:35:31 +0200
Subject: [PATCH 2/2] Simplify the confirmation messages

The confirmation of revoke and remove the certificate hold action is simplier
and more consistent with another parts of WebUI.

Part of: https://fedorahosted.org/freeipa/ticket/5381
---
 install/ui/test/data/ipa_init.json | 4 ++--
 ipaserver/plugins/internal.py  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index f55904ecd2fc80122eb8a2adca29abbb8cfa48b0..2f5cd6bf4303b8965eccba7cf1a2248dc2016f6a 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -276,7 +276,7 @@
 "remove_hold": "Remove Hold",
 "remove_certificate_hold": "Remove Certificate Hold for ${entity} ${primary_key}",
 "remove_certificate_hold_simple": "Remove Certificate Hold",
-"remove_certificate_hold_confirmation": "To confirm your intention to remove the certificate hold, click the \"Remove hold\" button.",
+"remove_certificate_hold_confirmation": "Do you want to remove the certificate hold?",
 "remove_from_crl": "Remove from CRL",
 "request_message": " Create a certificate database or use an existing one. To create a new database: # certutil -N -d database path  Create a CSR with subject CN=${cn_name},O=realm, for example: # certutil -R -d database path -a -g key size -s 'CN=${cn},O=${realm}'${san}   Copy and paste the CSR (from -BEGIN NEW CERTIFICATE REQUEST- to -END NEW CERTIFICATE REQUEST-) into the text area below:  ",
 "request_message_san": " -8 '${cn}'",
@@ -284,7 +284,7 @@
 "revocation_reason": "Revocation reason",
 "revoke_certificate": "Revoke Certificate for ${entity} ${primary_key}",
 "revoke_certificate_simple": "Revoke Certificate",
-"revoke_confirmation": "To confirm your intention to revoke this certificate, select a reason from the pull-down list, and click the \"Revoke\" button.",
+"revoke_confirmation": "Do you want to revoke this certificate? You can select a reason from the pull-down list.",
 "revoked": "Certificate Revoked",
 "serial_number": "Serial Number",
 "serial_number_hex": "Serial Number (hex)",
diff --git a/ipaserver/plugins/internal.py b/ipaserver/plugins/internal.py
index a1d193ac73a55666a6da4795406e6fbe86c7fe7e..c8795b4804c820c6f2cc3414ae8024fc71a529d3 100644
--- a/ipaserver/plugins/internal.py
+++ b/ipaserver/plugins/internal.py
@@ -413,7 +413,7 @@ class i18n_messages(Command):
 "remove_hold": _("Remove Hold"),
 "remove_certificate_hold": _("Remove Certificate Hold for ${entity} ${primary_key}"),
 "remove_certificate_hold_simple": _("Remove Certificate Hold"),
-"remove_certificate_hold_confirmation": _("To confirm your intention to remove the certificate hold, click the \"Remove hold\" button."),
+"remove_certificate_hold_confirmation": _("Do you want to remove the certificate hold?"),
 "remove_from_crl": _("Remove from CRL"),
 "request_message": _(" Create a certificate database or use an existing one. To create a new database: # certutil -N -d database path  Create a CSR with subject CN=${cn_name},O=realm, for example: # certutil -R -d database path -a -g key size -s 'CN=${cn},O=${realm}'${san}   Copy and paste the CSR (from -BEGIN NEW CERTIFICATE REQUEST- to -END NEW CERTIFICATE REQUEST-) into the text area below:  "),
 "request_message_san": _(" -8 '${cn}'"),
@@ -421,7 +421,7 @@ class i18n_messages(Command):
 "revocation_reason": _("Revocation reason"),
 "revoke_certificate": _("Revoke Certificate for ${entity} ${primary_key}"),
 "revoke_certificate_simple": _("Revoke Certificate"),
-"revoke_confirmation": _("To confirm your intention to revoke this certificate, select a reason from the pull-down list, and click the \"Revoke\" button."),
+"revoke_confirmation": _("Do you want to revoke this certificate? You can select a reason from the pull-down list."),
 "revoked": _("Certificate Revoked"),
 "serial_number": _("Serial Number"),
 "serial_number_hex": _("Serial Number (hex)"),
-- 
2.5.5

-- 

Re: [Freeipa-devel] [PATCH] 0061: webui: Add support for 'dns_update_system_records' command

[Pavel Vomacka]

On 06/23/2016 04:58 PM, Petr Vobornik wrote:

On 06/23/2016 04:34 PM, Martin Basti wrote:


On 23.06.2016 09:57, Pavel Vomacka wrote:

Hello,

please review attached patch.

Part of: https://fedorahosted.org/freeipa/ticket/5905




Works for me


In this patch and also in some other(cert patches) the confirm message
has following structure:
   To confirm your intention to $action, click the $button_name button.

On other places of Web UI, more human and easier structure is used:
   Do you want to update DNS records?
   [Update] [Cancel]

IMHO we should use it here as well. And the same for(separate path):
   remove_certificate_hold_confirmation
   revoke_confirmation"


otherwise the patch is OK.

The patch with simplified confirm message is attached. Patch with new 
'remove cert hold' and 'revoke cert' messages will be sent in separate 
thread.


--
Pavel^3 Vomacka

From 15b2c1b37c62c9e5c7e63255f0ea45263ce3dd0d Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Wed, 22 Jun 2016 18:13:27 +0200
Subject: [PATCH 1/2] Add button for dns_update_system_records command

Part of: https://fedorahosted.org/freeipa/ticket/5905
---
 install/ui/src/freeipa/dns.js  | 44 +-
 install/ui/test/data/ipa_init.json |  5 -
 ipaserver/plugins/internal.py  |  3 +++
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/install/ui/src/freeipa/dns.js b/install/ui/src/freeipa/dns.js
index c086251a8730abd4824609e9bc54a590dd138b14..2d424aeae8ef735d02426a0f08b6261ec2f04c19 100644
--- a/install/ui/src/freeipa/dns.js
+++ b/install/ui/src/freeipa/dns.js
@@ -100,7 +100,9 @@ return {
 ]
 }
 ],
-needs_update: true
+needs_update: true,
+actions: [ 'update_dns_records' ],
+header_actions: [ 'update_dns_records' ]
 }
 ]
 };};
@@ -554,6 +556,45 @@ var make_dnsserver_spec = function() {
 };
 
 
+IPA.dns.update_dns_records_action = function(spec) {
+
+spec = spec || {};
+
+spec.name = spec.name || 'update_dns_records';
+spec.label = spec.label || '@i18n:objects.dnsconfig.update_dns';
+
+var that = IPA.action(spec);
+
+that.execute_action = function() {
+var spec = {
+title: '@i18n:objects.dnsconfig.update_dns',
+message: '@i18n:objects.dnsconfig.update_dns_dialog_msg',
+ok_label: '@i18n:buttons.update'
+};
+
+that.dialog = IPA.confirm_dialog(spec);
+
+that.dialog.on_ok = function() {
+
+var command = rpc.command({
+entity: 'dns',
+method: 'update_system_records',
+on_success: function(data) {
+var status = data.result.value;
+if (status) IPA.notify_success(
+'@i18n:objects.dnsconfig.updated_dns');
+}
+});
+
+command.execute();
+};
+
+that.dialog.open();
+};
+
+return that;
+};
+
 IPA.dnszone_details_facet = function(spec, no_init) {
 
 spec = spec || {};
@@ -2617,6 +2658,7 @@ exp.register = function() {
 
 a.register('dns_add_permission', IPA.dns.add_permission_action);
 a.register('dns_remove_permission', IPA.dns.remove_permission_action);
+a.register('update_dns_records', IPA.dns.update_dns_records_action);
 };
 
 phases.on('registration', exp.register);
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 1d7f5d883dd923dc5ad749b592e55f23e7d4771b..f55904ecd2fc80122eb8a2adca29abbb8cfa48b0 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -309,7 +309,10 @@
 "forward_first": "Forward first",
 "forward_none": "Forwarding disabled",
 "forward_only": "Forward only",
-"options": "Options"
+"options": "Options",
+"update_dns": "Update DNS Records",
+"update_dns_dialog_msg": "Do you want to update DNS records?",
+"updated_dns": "DNS records updated"
 },
 "dnsrecord": {
 "data": "Data",
diff --git a/ipaserver/plugins/internal.py b/ipaserver/plugins/internal.py
index c0360567cbca883dda018a7a2f7f9f9536c3d118..a1d193ac73a55666a6da4795406e6fbe86c7fe7e 100644
--- a/ipaserver/plugins/internal.py
+++ b/ipaserver/plugins/internal.py
@@ -448,6 +448,9 @@ class i18n_messages(Command):
 "forward_none": _("Forwarding disabled"),
 "forward_only": _("Forward only"),
 "options": _("Options"),
+"update_dns": _("Update DNS Records"),
+"update_dns_dialog_msg": _("Do you want to update DNS records?"),
+"updated_dns": _("DNS records 

Re: [Freeipa-devel] [WIP] Thin client

[Jan Cholasta]

On 27.6.2016 14:55, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patch set:
schema: client-side cleanup
automember: fix automember to work with thin client
schema: do not crash in command_defaults if argument is None
schema: fix param default value handling

works* for me, ACK.


Thanks, pushed to master: f7cc15f0990ef2db57717a3c6a8e9db2c3dee951

Attaching the patches for reference.



*) xmlrpc test for automember_plugin test started failing with
automember patch. Fix for test attached.


LGTM

--
Jan Cholasta
From fd0553ca1216e68edb7ae88997a321339b08b612 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 22 Jun 2016 15:15:32 +0200
Subject: [PATCH 1/4] schema: fix param default value handling

Advertise param's default value even when `autofill` is False. When
`autofill` is False, set `alwaysask` to True in the schema, as it is
semantically equivallent and removes redundancy.

This fixes default value disappearing in CLI for some params.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipaclient/remote_plugins/schema.py |  6 +++---
 ipaserver/plugins/schema.py| 23 +--
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index c21da5f..b944fb0 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -213,7 +213,6 @@ def _create_param(meta):
 
 for key, value in meta.items():
 if key in ('alwaysask',
-   'autofill',
'doc',
'label',
'multivalue',
@@ -229,11 +228,9 @@ def _create_param(meta):
 kwargs[key] = value
 elif key == 'default':
 default = value
-kwargs['autofill'] = True
 elif key == 'default_from_param':
 kwargs['default_from'] = DefaultFrom(_nope,
  *(str(k) for k in value))
-kwargs['autofill'] = True
 elif key in ('exclude',
  'include'):
 kwargs[key] = tuple(str(v) for v in value)
@@ -246,6 +243,9 @@ def _create_param(meta):
 default = tmp._convert_scalar(default[0])
 kwargs['default'] = default
 
+if 'default' in kwargs or 'default_from' in kwargs:
+kwargs['autofill'] = not kwargs.pop('alwaysask', False)
+
 param = cls(str(meta['name']), **kwargs)
 
 if sensitive:
diff --git a/ipaserver/plugins/schema.py b/ipaserver/plugins/schema.py
index a67d7b2..c3a0e60 100644
--- a/ipaserver/plugins/schema.py
+++ b/ipaserver/plugins/schema.py
@@ -542,23 +542,26 @@ class param(BaseParam):
  'include'):
 obj[key] = list(unicode(v) for v in value)
 if isinstance(metaobj, Command):
-if key in ('alwaysask',
-   'confirm'):
+if key == 'alwaysask':
+obj.setdefault(key, value)
+elif key == 'confirm':
 obj[key] = value
 elif key in ('cli_metavar',
  'cli_name',
  'option_group'):
 obj[key] = unicode(value)
 elif key == 'default':
-if param.autofill:
-if param.multivalue:
-obj[key] = [unicode(v) for v in value]
-else:
-obj[key] = [unicode(value)]
+if param.multivalue:
+obj[key] = [unicode(v) for v in value]
+else:
+obj[key] = [unicode(value)]
+if not param.autofill:
+obj['alwaysask'] = True
 elif key == 'default_from':
-if param.autofill:
-obj['default_from_param'] = list(unicode(k)
- for k in value.keys)
+obj['default_from_param'] = list(unicode(k)
+ for k in value.keys)
+if not param.autofill:
+obj['alwaysask'] = True
 elif key in ('exponential',
  'normalizer',
  'only_absolute',
-- 
2.7.4

From 294bc271dafb021730174cfc4eacaaf187a53e68 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 23 Jun 2016 12:14:38 +0200
Subject: [PATCH 2/4] schema: do not crash in command_defaults if argument is
 None

Re: [Freeipa-devel] [PATCH 0167] test_serverroles: ensure that test API is initialized with correct ldap_uri

[Lenka Doudova]

On 06/27/2016 02:04 PM, Martin Babinsky wrote:

Makes the test suite play nice with others during CI.

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


ACK, thank you!

Lenka

--
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] 0008 Do not allow installation in FIPS mode

[Rob Crittenden]

Gabe Alford wrote:

On Mon, Jun 27, 2016 at 12:38 AM, Florence Blanc-Renaud
> wrote:

Hi,

this fix is a port of Bug 1131570 - Do not allow IdM
server/replica/client installation in a FIPS-140 mode
It prevents installation of FreeIPA if the host is fips-enabled.

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


Shouldn't this be about fixing FreeIPA to allow installation/operation
in FIPS mode rather than disabling it? There are many environments where
FIPS is required, and FreeIPA should support it.


This is a stop-gap measure to provide users with reasonable feedback on 
the current state of things.


Getting FIPS working, particularly in the server, is a somewhat 
non-trivial task.


rob

--
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] 0008 Do not allow installation in FIPS mode

[Rob Crittenden]

Petr Spacek wrote:

On 27.6.2016 08:38, Florence Blanc-Renaud wrote:

Hi,

this fix is a port of Bug 1131570 - Do not allow IdM server/replica/client
installation in a FIPS-140 mode
It prevents installation of FreeIPA if the host is fips-enabled.

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

freeipa-frenaud-0008-Do-not-allow-installation-in-FIPS-mode.patch


>From afecbb3d228cf1d6cee59da53bf7a803f030d0b1 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud 
Date: Fri, 24 Jun 2016 16:16:22 +0200
Subject: [PATCH] Do not allow installation in FIPS mode

https://fedorahosted.org/freeipa/ticket/5761
---
  client/ipa-client-install  | 4 
  install/tools/ipactl   | 6 ++
  ipaserver/install/server/install.py| 5 +
  ipaserver/install/server/replicainstall.py | 5 +
  4 files changed, 20 insertions(+)

diff --git a/client/ipa-client-install b/client/ipa-client-install
index 
0a601b63118b0a3568066495837121c65e5df04f..f80ff9c469709ea3b63902610b3b8b5c35448904
 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -3064,6 +3064,10 @@ def main():

  if not os.getegid() == 0:
  sys.exit("\nYou must be root to run ipa-client-install.\n")
+if os.path.exists('/proc/sys/crypto/fips_enabled'):
+with open('/proc/sys/crypto/fips_enabled', 'r') as f:


Usually it is safer to call open() and catch exception if the file does not
exist. The code above has inherent problem with race-conditions between time
of check (path.exists) and time of use (open).

Of course it is not a problem here because this file is part of kernel's
interface but in general please use the try: open() except: form.


+if f.read().strip() != '0':
+sys.exit("Cannot install IPA client in FIPS mode")


Personally I would like to see more informative messages.

I would recommend something like " is not supported in FIPS mode".

In my eyes it is difference between "How do I ...? You dont!" vs "How do I
...? Sorry, we do not support that right now."


Given that this code is duplicated 4 times I'd also move it to a 
function in ipapython, is_fips_enabled() or something .


rob




Sorry for nitpicking! :-)

Petr^2 Spacek




  tasks.check_selinux_status()
  logging_setup(options)
  root_logger.debug(
diff --git a/install/tools/ipactl b/install/tools/ipactl
index 
547b21d875dff7231fae8dfc10faf995b0ca230b..9c68fffe73bfdd97789907226f8765c09707d552
 100755
--- a/install/tools/ipactl
+++ b/install/tools/ipactl
@@ -545,6 +545,12 @@ def main():
  elif args[0] != "start" and args[0] != "stop" and args[0] != "restart" and args[0] 
!= "status":
  raise IpactlError("Unrecognized action [" + args[0] + "]", 2)

+if (args[0] in ('start', 'restart') and
+os.path.exists('/proc/sys/crypto/fips_enabled')):
+with open('/proc/sys/crypto/fips_enabled', 'r') as f:
+if f.read().strip() != '0':
+raise IpactlError("Cannot start IPA server in FIPS mode")
+
  # check if IPA is configured at all
  try:
  check_IPA_configuration()
diff --git a/ipaserver/install/server/install.py 
b/ipaserver/install/server/install.py
index 
930cca7b31ca06c04ab92deff49b6a4f198c2b6e..0c0683733ef38444a82d085f771596a9b066ef1d
 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -319,6 +319,11 @@ def install_check(installer):
  external_ca_file = installer._external_ca_file
  http_ca_cert = installer._ca_cert

+if os.path.exists('/proc/sys/crypto/fips_enabled'):
+with open('/proc/sys/crypto/fips_enabled', 'r') as f:
+if f.read().strip() != '0':
+sys.exit("Cannot install IPA server in FIPS mode")
+
  tasks.check_selinux_status()

  if options.master_password:
diff --git a/ipaserver/install/server/replicainstall.py 
b/ipaserver/install/server/replicainstall.py
index 
52b2ea5b0691cd99c6cb566af5a15af3b2dffb14..a2946339c7aeee8529f6ecf8ec4d85c9291fd291
 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -485,6 +485,11 @@ def install_check(installer):
  options = installer
  filename = installer.replica_file

+if os.path.exists('/proc/sys/crypto/fips_enabled'):
+with open('/proc/sys/crypto/fips_enabled', 'r') as f:
+if f.read().strip() != '0':
+sys.exit("Cannot install IPA server in FIPS mode")
+
  tasks.check_selinux_status()

  if is_ipa_configured():
-- 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] 0008 Do not allow installation in FIPS mode

[Gabe Alford]

On Mon, Jun 27, 2016 at 12:38 AM, Florence Blanc-Renaud 
wrote:

> Hi,
>
> this fix is a port of Bug 1131570 - Do not allow IdM server/replica/client
> installation in a FIPS-140 mode
> It prevents installation of FreeIPA if the host is fips-enabled.
>
> https://fedorahosted.org/freeipa/ticket/5761
>

Shouldn't this be about fixing FreeIPA to allow installation/operation in
FIPS mode rather than disabling it? There are many environments where FIPS
is required, and FreeIPA should support it.

Gabe
-- 
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] 0018-0030, 52 webui: add support for more certificates

[Pavel Vomacka]

On 06/23/2016 03:17 PM, Petr Vobornik wrote:

comments inline

On 06/20/2016 02:37 PM, Pavel Vomacka wrote:


On 06/14/2016 09:41 PM, Pavel Vomacka wrote:


On 05/13/2016 06:56 PM, Petr Vobornik wrote:

On 04/26/2016 04:23 PM, Pavel Vomacka wrote:

Self-NACK for patches 0027, 28, 29, 30 - used incorrect policy. I also attach
all patches which were not changed - it is easier to get the whole patchset.

On 04/26/2016 02:02 PM, Pavel Vomacka wrote:

I forgot to mention that my patches requires patches from :
https://www.redhat.com/archives/freeipa-devel/2016-April/msg00209.html


On 04/26/2016 01:33 PM, Pavel Vomacka wrote:

Hello,

the attached patches add support for more certificates and ability to add and
remove certificates. Fixes these two tickets:
https://fedorahosted.org/freeipa/ticket/5108
https://fedorahosted.org/freeipa/ticket/5381

These patches add ability to view, get, download, revoke, restore and delete
each certificate directly from user/host/service details page. There is also
button for adding new certificates.

There is one known issue, that after page save action is performed some data
disappear (includes certificates). This issue has a ticket already:
https://fedorahosted.org/freeipa/ticket/5776

--
Pavel^3 Vomacka


Great stuff, couple comments below.

We can discuss some items in person. Not everything needs to be done.

I didn't run it, just reading the code.

Patch 0018:

1. Nit pick: When a value should be boolean, then following method won't
make sure that dropdown_menu won't be e.g. an object.
   +that.dropdown_menu = spec.dropdown_menu || false;

I would prefer:
   +that.dropdown_menu = !!spec.dropdown_menu;

Which retypes it to boolean. If default should be true (not this case) then:
that.dropdown_menu = spec.dropdown_menu !== undefined ?
!!spec.dropdown_menu : true;

Also the interface is very specific. It says that the child widget will
have dropdown menu. What if the actions won't be in dropdown menu but,
e.g., some overlay menu.

Imho the interface should be:

   that.custom_actions = !!spec.custom_actions;

Than the child object would have define,e.g., :

 action_object get_custom_actions()

Interface of action_object would be e.g.:
 get_items()
 set_items(items)
 enable_item(name)
 disable_item(name)

Dropdown menu would have to define these methods.


It calls ,render on custom actions object. But this method is not part
of the interface:
+custom_actions.render();

IMHO it should be called internally in the widget as a result of
set_items or disable_item, enable_item call.

Fixed.

Patch 0019:

1. Shouldn't disable_item or enable_item automatically rerender the items?

Same for set and get_items. As mentioned above.

The render code in disable/enable_item should be executed only if
this.ul_node exists.


Fixed.

2. The rerender, used in later patches. Imo it should do only:

if (this.ul_node) {
   construct.empty(this.ul_node);
   this._render_items(this.items);
}

Or just re-render the one item.
$( "li[data-name=" + item.name +"]", this.ul_node ).replaceWith(
this._render_item(item));

3. in future for loops write:
for (var i=0, l=this.items.length; i

Re: [Freeipa-devel] [WIP] Thin client

[David Kupka]

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patch set:
schema: client-side cleanup
automember: fix automember to work with thin client
schema: do not crash in command_defaults if argument is None
schema: fix param default value handling

works* for me, ACK.

*) xmlrpc test for automember_plugin test started failing with 
automember patch. Fix for test attached.


--
David Kupka
From e736f1d1c9e5247b95fab49a5ea8fa15d6077981 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Mon, 27 Jun 2016 14:52:27 +0200
Subject: [PATCH] test: automember: Fix expected exception message

---
 ipatests/test_xmlrpc/test_automember_plugin.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_automember_plugin.py b/ipatests/test_xmlrpc/test_automember_plugin.py
index 9060b69bd57aa8a8a2704dfd233ff3fa75da715a..ffbc91104ab504a98099babb024f9edab114ac5b 100644
--- a/ipatests/test_xmlrpc/test_automember_plugin.py
+++ b/ipatests/test_xmlrpc/test_automember_plugin.py
@@ -196,7 +196,7 @@ class TestNonexistentAutomember(XMLRPC_test):
 group1.ensure_missing()
 command = automember_group.make_delete_command()
 with raises_exact(errors.NotFound(
-reason=u': Automember rule not found')):
+reason=u'%s: Automember rule not found' % group1.cn)):
 command()
 
 def test_create_with_nonexistent_hostgroup(self, automember_hostgroup,
@@ -214,7 +214,7 @@ class TestNonexistentAutomember(XMLRPC_test):
 hostgroup1.ensure_missing()
 command = automember_hostgroup.make_delete_command()
 with raises_exact(errors.NotFound(
-reason=u': Automember rule not found')):
+reason=u'%s: Automember rule not found' % hostgroup1.cn)):
 command()
 
 
-- 
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] 0079 Set default OCSP URI on install and upgrade

[Martin Basti]

On 27.06.2016 14:10, Fraser Tweedale wrote:

On Mon, Jun 27, 2016 at 02:02:15PM +0200, Martin Basti wrote:


On 27.06.2016 13:58, Fraser Tweedale wrote:

Hi all,

The attached patch fixes the OCSP URI in the Dogtag CA and system
certificates (https://fedorahosted.org/freeipa/ticket/5956).  It
depends on a patch[1] for Dogtag which is expected to be released in
v10.3.4.  In the meantime, you can test with the build of v10.3.4
from my COPR[2].

[1] https://www.redhat.com/archives/pki-devel/2016-June/msg00138.html
[2] https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/

Cheers,
Fraser



Hello,

this upgrade is executed always, is it on purpose?
If not please use sysupgrade and run upgrade only once


It is intentional; the directive only gets added if it is missing.
I do not see any benefit in gating it with the sysupgrade mechanism.

Thanks,
Fraser

Ok

--
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] 0079 Set default OCSP URI on install and upgrade

[Fraser Tweedale]

On Mon, Jun 27, 2016 at 02:02:15PM +0200, Martin Basti wrote:
> 
> 
> On 27.06.2016 13:58, Fraser Tweedale wrote:
> > Hi all,
> > 
> > The attached patch fixes the OCSP URI in the Dogtag CA and system
> > certificates (https://fedorahosted.org/freeipa/ticket/5956).  It
> > depends on a patch[1] for Dogtag which is expected to be released in
> > v10.3.4.  In the meantime, you can test with the build of v10.3.4
> > from my COPR[2].
> > 
> > [1] https://www.redhat.com/archives/pki-devel/2016-June/msg00138.html
> > [2] https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/
> > 
> > Cheers,
> > Fraser
> > 
> > 
> Hello,
> 
> this upgrade is executed always, is it on purpose?
> If not please use sysupgrade and run upgrade only once
> 
It is intentional; the directive only gets added if it is missing.
I do not see any benefit in gating it with the sysupgrade mechanism.

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

[Freeipa-devel] [PATCH 0167] test_serverroles: ensure that test API is initialized with correct ldap_uri

[Martin Babinsky]

Makes the test suite play nice with others during CI.

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

--
Martin^3 Babinsky
From c3443cd12e6fdb9e29033c5395146114d5f7 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 27 Jun 2016 13:44:17 +0200
Subject: [PATCH] test_serverroles: ensure that test API is initialized with
 correct ldap_uri

This ensures that the serverroles test works also when run together with other
iaserver test suites.

https://fedorahosted.org/freeipa/ticket/6000
---
 ipatests/test_ipaserver/test_serverroles.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_ipaserver/test_serverroles.py b/ipatests/test_ipaserver/test_serverroles.py
index ef8cca3d978ddb0fec131c0d23dd8cf4816f5bd0..370e86dde3f7cc5e204dcb6488a59162c551637d 100644
--- a/ipatests/test_ipaserver/test_serverroles.py
+++ b/ipatests/test_ipaserver/test_serverroles.py
@@ -469,12 +469,17 @@ class MockMasterTopology(object):
 @pytest.fixture(scope='module')
 def mock_api(request):
 test_api = create_api(mode=None)
-test_api.bootstrap(in_server=True, in_tree=True)
+test_api.bootstrap(in_server=True, ldap_uri=api.env.ldap_uri)
 test_api.finalize()
 
 if not test_api.Backend.ldap2.isconnected():
 test_api.Backend.ldap2.connect()
 
+def finalize():
+test_api.Backend.ldap2.disconnect()
+
+request.addfinalizer(finalize)
+
 return test_api
 
 
-- 
2.5.5

-- 
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 0138] replica-install: Compare domain names as DNS names and not string

[Martin Basti]

On 27.06.2016 14:02, Petr Spacek wrote:

On 27.6.2016 11:20, Petr Spacek wrote:

On 27.6.2016 10:30, Martin Basti wrote:


On 23.06.2016 18:32, Petr Spacek wrote:

Hello,

replica-install: Compare domain names as DNS names and not strings

This fixes false possitive where user inputs "example.com" and "EXAMPLE.COM"
were not considered equivalent and installation was wrongly refused.

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


NACK, client installer should normalize domain name as host-add does, because
it will blow up in different places, we cannot compare this part as DNS name
when other parts works with it as strings

ipa.ipapython.install.cli.install_tool(Replica): ERRORCannot promote this
client to a replica. Local domain 'ipa.example.COM' does not match IPA domain
'ipa.example.com'.

Okay, I will use the same validator as ipa-server-install and normalize it as
you suggested.

Here you go. I was not able to find a corner case which would break this.


LGTM

--
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] 0079 Set default OCSP URI on install and upgrade

[Martin Basti]

On 27.06.2016 13:58, Fraser Tweedale wrote:

Hi all,

The attached patch fixes the OCSP URI in the Dogtag CA and system
certificates (https://fedorahosted.org/freeipa/ticket/5956).  It
depends on a patch[1] for Dogtag which is expected to be released in
v10.3.4.  In the meantime, you can test with the build of v10.3.4
from my COPR[2].

[1] https://www.redhat.com/archives/pki-devel/2016-June/msg00138.html
[2] https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/

Cheers,
Fraser



Hello,

this upgrade is executed always, is it on purpose?
If not please use sysupgrade and run upgrade only once

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 0138] replica-install: Compare domain names as DNS names and not string

[Petr Spacek]

On 27.6.2016 11:20, Petr Spacek wrote:
> On 27.6.2016 10:30, Martin Basti wrote:
>> > 
>> > 
>> > On 23.06.2016 18:32, Petr Spacek wrote:
>>> >> Hello,
>>> >>
>>> >> replica-install: Compare domain names as DNS names and not strings
>>> >>
>>> >> This fixes false possitive where user inputs "example.com" and 
>>> >> "EXAMPLE.COM"
>>> >> were not considered equivalent and installation was wrongly refused.
>>> >>
>>> >> https://fedorahosted.org/freeipa/ticket/5976
>>> >>
>> > 
>> > NACK, client installer should normalize domain name as host-add does, 
>> > because
>> > it will blow up in different places, we cannot compare this part as DNS 
>> > name
>> > when other parts works with it as strings
>> > 
>> > ipa.ipapython.install.cli.install_tool(Replica): ERRORCannot promote 
>> > this
>> > client to a replica. Local domain 'ipa.example.COM' does not match IPA 
>> > domain
>> > 'ipa.example.com'.
> Okay, I will use the same validator as ipa-server-install and normalize it as
> you suggested.

Here you go. I was not able to find a corner case which would break this.

-- 
Petr^2 Spacek
From b964f784519442361695695fbde36385066506e3 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Mon, 27 Jun 2016 14:00:01 +0200
Subject: [PATCH] client: Share validator and domain name normalization with
 server install

https://fedorahosted.org/freeipa/ticket/5976
---
 client/ipa-client-install | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/client/ipa-client-install b/client/ipa-client-install
index 0a601b63118b0a3568066495837121c65e5df04f..2da2720d1f959b452a4895ebb23e0efadae2a7fc 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -54,7 +54,8 @@ try:
 from ipapython.config import IPAOptionParser
 from ipalib import api, errors
 from ipalib import x509, certstore
-from ipalib.util import verify_host_resolvable
+from ipalib.util import (
+normalize_hostname, validate_domain_name, verify_host_resolvable)
 from ipalib.constants import CACERT
 from ipapython.dn import DN
 from ipapython.ssh import SSHPublicKey
@@ -230,6 +231,13 @@ def parse_options():
 if (options.server and not options.domain):
 parser.error("--server cannot be used without providing --domain")
 
+if options.domain:
+try:
+validate_domain_name(options.domain)
+except ValueError as ex:
+parser.error("invalid domain name: %s" % ex)
+options.domain = normalize_hostname(options.domain)
+
 if options.force_ntpd and not options.conf_ntp:
 parser.error("--force-ntpd cannot be used together with --no-ntp")
 
-- 
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] [PATCH] 0079 Set default OCSP URI on install and upgrade

[Fraser Tweedale]

Hi all,

The attached patch fixes the OCSP URI in the Dogtag CA and system
certificates (https://fedorahosted.org/freeipa/ticket/5956).  It
depends on a patch[1] for Dogtag which is expected to be released in
v10.3.4.  In the meantime, you can test with the build of v10.3.4
from my COPR[2].

[1] https://www.redhat.com/archives/pki-devel/2016-June/msg00138.html
[2] https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/

Cheers,
Fraser
From f1a08357deeeb0012eb1a00f13934f8a0522fc36 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Mon, 27 Jun 2016 15:49:30 +1000
Subject: [PATCH] Set default OCSP URI on install and upgrade

Dogtag has been updated to support a default OCSP URI when the
profile includes AuthInfoAccess with URI method but does not specify
the URI (instead of constructing one based on Dogtag's hostname and
port).

Add the pkispawn config to ensure that the OCSP URI is set before
issuing CA and system certificates, and add the config to existing
CA instances on upgrade.

Fixes: https://fedorahosted.org/freeipa/ticket/5956
---
 freeipa.spec.in |  6 +++---
 ipaserver/install/cainstance.py |  3 +++
 ipaserver/install/server/upgrade.py | 23 +++
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 
c86fc3157920f77e66f38241692c3cf45c637ebb..a4d3c067c6c2c0cf911476d950257170fa74e059
 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -94,7 +94,7 @@ BuildRequires:  libunistring-devel
 BuildRequires:  python-lesscpy
 BuildRequires:  python-yubico >= 1.2.3
 BuildRequires:  openssl-devel
-BuildRequires:  pki-base >= 10.3.3
+BuildRequires:  pki-base >= 10.3.4
 BuildRequires:  python-pytest-multihost >= 0.5
 BuildRequires:  python-pytest-sourceorder
 BuildRequires:  python-kdcproxy >= 0.3
@@ -155,8 +155,8 @@ Requires(post): systemd-units
 Requires: selinux-policy >= %{selinux_policy_version}
 Requires(post): selinux-policy-base >= %{selinux_policy_version}
 Requires: slapi-nis >= 0.56.0
-Requires: pki-ca >= 10.3.3
-Requires: pki-kra >= 10.3.3
+Requires: pki-ca >= 10.3.4
+Requires: pki-kra >= 10.3.4
 Requires(preun): python systemd-units
 Requires(postun): python systemd-units
 Requires: zip
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 
8dfb71528d2dc020e05ccd7ff42199218a1c0839..a575f02677112258b8cf5aed56b33898bb5fd8c0
 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -463,6 +463,9 @@ class CAInstance(DogtagInstance):
 config.set("CA", "pki_backup_keys", "True")
 config.set("CA", "pki_backup_password", self.admin_password)
 config.set("CA", "pki_profiles_in_ldap", "True")
+config.set("CA", "pki_default_ocsp_uri",
+"http://{}.{}/ca/ocsp".format(
+IPA_CA_RECORD, ipautil.format_netloc(api.env.domain)))
 
 # Client security database
 config.set("CA", "pki_client_database_dir", self.agent_db)
diff --git a/ipaserver/install/server/upgrade.py 
b/ipaserver/install/server/upgrade.py
index 
b4b6243ac19c40f7216a898607d28db52822170f..3955a8cb9faf8e5c3350fc3912ea9f05a4b97719
 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -356,6 +356,28 @@ def ca_ensure_lightweight_cas_container(ca):
 return cainstance.ensure_lightweight_cas_container()
 
 
+def ca_add_default_ocsp_uri(ca):
+root_logger.info('[Adding default OCSP URI configuration]')
+if not ca.is_configured():
+root_logger.info('CA is not configured')
+return False
+
+value = installutils.get_directive(
+paths.CA_CS_CFG_PATH,
+'ca.defaultOcspUri',
+separator='=')
+if value:
+return False  # already set; restart not needed
+
+installutils.set_directive(
+paths.CA_CS_CFG_PATH,
+'ca.defaultOcspUri',
+'http://ipa-ca.%s/ca/ocsp' % ipautil.format_netloc(api.env.domain),
+quotes=False,
+separator='=')
+return True  # restart needed
+
+
 def upgrade_ca_audit_cert_validity(ca):
 """
 Update the Dogtag audit signing certificate.
@@ -1725,6 +1747,7 @@ def upgrade_configuration():
 ca_enable_pkix(ca),
 ca_configure_profiles_acl(ca),
 ca_configure_lightweight_ca_acls(ca),
+ca_add_default_ocsp_uri(ca),
 ])
 
 if ca_restart:
-- 
2.5.5

-- 
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] [Test][Patch-0047] Added a test for Ticket N 5964

[Oleg Fayans]

Hi guys,

Is there a chance the patches NN 0047.1 and 0048.1 get reviewed before
4.4 release? They cover a good part of the Managed Topology 4.4 feature.

On 06/17/2016 11:18 AM, Oleg Fayans wrote:
> One more test was added to the patch-0048
> 
> On 06/17/2016 09:43 AM, Oleg Fayans wrote:
>> Fixed a bug in the previous patch, automated 2 more testcases from
>> http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan
>>
>> On 06/16/2016 04:46 PM, Oleg Fayans wrote:
>>>
>>>
>>>
>>
>>
>>
> 
> 
> 

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

-- 
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 0165] keep setting ipakrbprincipal objectclass on new service entries

[Martin Basti]

On 27.06.2016 13:02, Petr Spacek wrote:

On 27.6.2016 12:42, Martin Basti wrote:


On 27.06.2016 09:39, Martin Babinsky wrote:

On 06/27/2016 07:56 AM, Martin Babinsky wrote:

On 06/24/2016 04:07 PM, Martin Babinsky wrote:

This patch reverts commits 705f66f7490c64de1adc129221b31927616c485 and
06d945a04607dc36e25af78688b4295420489fb9 responsible for
https://fedorahosted.org/freeipa/ticket/5996

This should unblock replica promotion.




self-NACK, disregard this patch, it should not be necessary to revert
the whole commit


This version only reverts the change that actually breaks stuff.

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




Works for me


Worked for me too, I tried to install replica using admin credentials against
master which was upgraded from 4.3.1 to latest master.


ACK
Pushed to master: 7b8247a485081a6f1f5201e286ac17228f976355

--
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 0538-0540] DNS locations: epilogue

[Martin Basti]

On 27.06.2016 13:25, Petr Spacek wrote:

On 27.6.2016 11:43, Martin Basti wrote:


On 27.06.2016 10:56, Petr Spacek wrote:

On 24.6.2016 12:25, Martin Basti wrote:

On 23.06.2016 18:26, Petr Spacek wrote:

On 23.6.2016 16:38, Martin Basti wrote:

Patches attached.


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


freeipa-mbasti-0538-Revert-DNS-Locations-do-not-generate-location-record.patch



   From 28499422115cbfbb343033511319c7c8710e1ff5 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 21 Jun 2016 18:04:13 +0200
Subject: [PATCH 1/4] Revert "DNS Locations: do not generate location records
for unused locations"

This reverts commit bbf8227e3fd678d4bd6659a12055ba3dbe1c8230.

After deeper investigation, we found out that empty locations are needed
for clients, because clients may have cached records for longer time for
that particular location. Only way how to remove location is to remove
it using location-del

https://fedorahosted.org/freeipa/ticket/2008
---
ipaserver/dns_data_management.py | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/ipaserver/dns_data_management.py
b/ipaserver/dns_data_management.py
index
a9e9c0a3856961b5494c8d3ca30ddb2e4aa5c523..eac2e7d1a5618ea92372bd81b7d12752791ef117

100644
--- a/ipaserver/dns_data_management.py
+++ b/ipaserver/dns_data_management.py
@@ -68,7 +68,6 @@ class IPASystemRecords(object):
self.api_instance = api_instance
self.domain_abs =
DNSName(self.api_instance.env.domain).make_absolute()
self.servers_data = {}
-self.used_locations = set()
self.__init_data()
  def reload_data(self):
@@ -92,7 +91,6 @@ class IPASystemRecords(object):
  def __init_data(self):
self.servers_data = {}
-self.used_locations = set()
  servers_result = self.api_instance.Command.server_find(
pkey_only=True)['result']
@@ -104,8 +102,6 @@ class IPASystemRecords(object):
'location': location,
'roles': roles,
}
-if location:
-self.used_locations.add(location)
  def __add_srv_records(
self, zone_obj, hostname, rname_port_map,
@@ -353,12 +349,13 @@ class IPASystemRecords(object):
pkey_only=True)['result']
servers = [s['cn'][0] for s in servers_result]
-# generate only records for used location, records for
unassigned
-# locations are useless
+locations_result =
self.api_instance.Command.location_find()['result']
+locations = [l['idnsname'][0] for l in locations_result]
+
for server in servers:
self._get_location_dns_records_for_server(
zone_obj, server,
-self.used_locations, roles=roles,
+locations, roles=roles,
include_master_role=include_master_role)
return zone_obj
-- 2.5.5


freeipa-mbasti-0539-DNS-Locations-hide-option-no-msdcs-in-adtrust-instal.patch



   From 37cae4f05cd3c0a2c4de037402938a5437dbc072 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 21 Jun 2016 18:17:55 +0200
Subject: [PATCH 2/4] DNS Locations: hide option --no-msdcs in
adtrust-install

Since DNS location mechanism is active, this option has no effect,
because records are generate dynamically.

https://fedorahosted.org/freeipa/ticket/2008
---
install/tools/ipa-adtrust-install| 10 +++---
ipaserver/install/adtrustinstance.py | 21 -
2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/install/tools/ipa-adtrust-install
b/install/tools/ipa-adtrust-install
index
5babcdb7cb169e4a944acca55739064e0464d41e..5ba72a65d00ca683239a4ff3c5e7cfdc62c0bb6c

100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -29,6 +29,8 @@ import ldap
  import six
+from optparse import SUPPRESS_HELP
+
from ipaserver.install import adtrustinstance
from ipaserver.install.installutils import (
read_password,
@@ -54,9 +56,11 @@ def parse_options():
  default=False, help="print debugging information")
parser.add_option("--netbios-name", dest="netbios_name",
  help="NetBIOS name of the IPA domain")
+
+# no-msdcs has not effect, option is here just for backward
compatibility
parser.add_option("--no-msdcs", dest="no_msdcs", action="store_true",
-  default=False, help="Do not create DNS service
records " \
-  "for Windows in managed DNS
server")
+  default=False, help=SUPPRESS_HELP)
+
parser.add_option("--rid-base", dest="rid_base", type=int,
default=1000,
  help="Start value for mapping UIDs and GIDs to
RIDs")
parser.add_option("--secondary-rid-base", 

Re: [Freeipa-devel] [patch 0038-0040] Sub CA test patches

[Milan Kubík]

On 06/27/2016 02:57 AM, Fraser Tweedale wrote:

On Fri, Jun 24, 2016 at 12:08:24PM +0200, Milan Kubík wrote:

On 06/24/2016 03:42 AM, Fraser Tweedale wrote:

On Tue, Jun 21, 2016 at 05:01:35PM +0200, Milan Kubík wrote:

Hi Fraser and list,

I have made changes to the test plan on the wiki [1] according to the
information in "[Testplan review] Sub CAs" thread.

I also implemented the tests in the test plan:

patch 0038 - CATracker and CA CRUD test
patch 0039 - extension to CA ACL test
patch 0040 - functional test with ACLs and certificate profile, reusing my
previous S/MIME based tests. This patch also tests for the cert-request
behavior when profile ID or CA cn are ommited.

The tests ATM do not verify the Issuer name in the certificate itself, just
from the ipa entry of the certificate.


The approach you are using::

  assert cert_info['result']['issuer'] == smime_signing_ca.ipasubjectdn

is not quite as you describe (these are virtual attributes, not
attributes of an actual entry); but the approach is valid.

The issue then is in the wording? The other approach I could have used here
is to retrieve the two certificates and compare the fields manually.
Are these virtual attributes created from the certificate itself?


That's correct.


Fraser, could you please verify my reasoning behind the test cases for
cert-request in the patch 40?


The tests look OK.  With the default CA / default profiles, is there
appropriate isolation between test cases to ensure that if, e.g.
some other test case adds/modifies CA ACLs such that these
expected-to-fail tests now pass, that this does not affect the
TestCertSignMIMEwithSubCA test case?

Thanks,
Fraser

The ACL, SMIME CA and S/MIME profile lifetime is constrained by the class
scope
enforced by pytest.
The two test cases depend on the fact documented in the designs and that is
what
cert-request fallbacks to when CA or profile ID are not provided.
Unless something changes caIPAserviceCert profile or affiliated ACL, then
the test cases
are safe.


If you have thought about possible interference from other tests, I
am happy.

Note another problematic scenario: what if a different (preceding)
test adds a CA ACL that would allow the requests that you expect to
fail?  Just something to think about :)

Thanks,
Fraser
Then the failure would be problem of the preceding test and we would 
need to fix it. We are dealing with test side effects

in other parts of the execution already...

The test is constructed in a way that isolates it (to a certain degree) 
by the mechanisms available
in pytest. Of course I cannot make the test future-proof or guarantee 
that a bug in some other test
will not affect the execution of other tests as they all run against one 
IPA instance.
I do not think, however, that potential misbehaving test case that will 
interfere

should prevent us from implementing this and similar test cases.

If you have some specific issue that is in the patch, I'm happy to fix them.

I will try to think more about corner cases here.

[1]: http://www.freeipa.org/page/V4/Sub-CAs/Test_Plan

Cheers

--
Milan Kubik


Attaching rebased patches and removing the expected fail from one of the
tests as ticket 5981 has fix posted.

--
Milan Kubik




--
Milan Kubik

--
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 0538-0540] DNS locations: epilogue

[Petr Spacek]

On 27.6.2016 11:43, Martin Basti wrote:
> 
> 
> On 27.06.2016 10:56, Petr Spacek wrote:
>> On 24.6.2016 12:25, Martin Basti wrote:
>>>
>>> On 23.06.2016 18:26, Petr Spacek wrote:
 On 23.6.2016 16:38, Martin Basti wrote:
> Patches attached.
>
>
> https://fedorahosted.org/freeipa/ticket/2008
>
>
> freeipa-mbasti-0538-Revert-DNS-Locations-do-not-generate-location-record.patch
>
>
>
>   From 28499422115cbfbb343033511319c7c8710e1ff5 Mon Sep 17 00:00:00 2001
> From: Martin Basti 
> Date: Tue, 21 Jun 2016 18:04:13 +0200
> Subject: [PATCH 1/4] Revert "DNS Locations: do not generate location 
> records
>for unused locations"
>
> This reverts commit bbf8227e3fd678d4bd6659a12055ba3dbe1c8230.
>
> After deeper investigation, we found out that empty locations are needed
> for clients, because clients may have cached records for longer time for
> that particular location. Only way how to remove location is to remove
> it using location-del
>
> https://fedorahosted.org/freeipa/ticket/2008
> ---
>ipaserver/dns_data_management.py | 11 ---
>1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/ipaserver/dns_data_management.py
> b/ipaserver/dns_data_management.py
> index
> a9e9c0a3856961b5494c8d3ca30ddb2e4aa5c523..eac2e7d1a5618ea92372bd81b7d12752791ef117
>
> 100644
> --- a/ipaserver/dns_data_management.py
> +++ b/ipaserver/dns_data_management.py
> @@ -68,7 +68,6 @@ class IPASystemRecords(object):
>self.api_instance = api_instance
>self.domain_abs =
> DNSName(self.api_instance.env.domain).make_absolute()
>self.servers_data = {}
> -self.used_locations = set()
>self.__init_data()
>  def reload_data(self):
> @@ -92,7 +91,6 @@ class IPASystemRecords(object):
>  def __init_data(self):
>self.servers_data = {}
> -self.used_locations = set()
>  servers_result = self.api_instance.Command.server_find(
>pkey_only=True)['result']
> @@ -104,8 +102,6 @@ class IPASystemRecords(object):
>'location': location,
>'roles': roles,
>}
> -if location:
> -self.used_locations.add(location)
>  def __add_srv_records(
>self, zone_obj, hostname, rname_port_map,
> @@ -353,12 +349,13 @@ class IPASystemRecords(object):
>pkey_only=True)['result']
>servers = [s['cn'][0] for s in servers_result]
>-# generate only records for used location, records for
> unassigned
> -# locations are useless
> +locations_result =
> self.api_instance.Command.location_find()['result']
> +locations = [l['idnsname'][0] for l in locations_result]
> +
>for server in servers:
>self._get_location_dns_records_for_server(
>zone_obj, server,
> -self.used_locations, roles=roles,
> +locations, roles=roles,
>include_master_role=include_master_role)
>return zone_obj
>-- 2.5.5
>
>
> freeipa-mbasti-0539-DNS-Locations-hide-option-no-msdcs-in-adtrust-instal.patch
>
>
>
>   From 37cae4f05cd3c0a2c4de037402938a5437dbc072 Mon Sep 17 00:00:00 2001
> From: Martin Basti 
> Date: Tue, 21 Jun 2016 18:17:55 +0200
> Subject: [PATCH 2/4] DNS Locations: hide option --no-msdcs in
> adtrust-install
>
> Since DNS location mechanism is active, this option has no effect,
> because records are generate dynamically.
>
> https://fedorahosted.org/freeipa/ticket/2008
> ---
>install/tools/ipa-adtrust-install| 10 +++---
>ipaserver/install/adtrustinstance.py | 21 -
>2 files changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/install/tools/ipa-adtrust-install
> b/install/tools/ipa-adtrust-install
> index
> 5babcdb7cb169e4a944acca55739064e0464d41e..5ba72a65d00ca683239a4ff3c5e7cfdc62c0bb6c
>
> 100755
> --- a/install/tools/ipa-adtrust-install
> +++ b/install/tools/ipa-adtrust-install
> @@ -29,6 +29,8 @@ import ldap
>  import six
>+from optparse import SUPPRESS_HELP
> +
>from ipaserver.install import adtrustinstance
>from ipaserver.install.installutils import (
>read_password,
> @@ -54,9 +56,11 @@ def parse_options():
>  default=False, help="print debugging 
> information")
>parser.add_option("--netbios-name", dest="netbios_name",
>  help="NetBIOS name of the IPA 

Re: [Freeipa-devel] [PATCH] 0078 Fix IssuerDN presence check in cert search result

[Martin Basti]

On 27.06.2016 08:34, Fraser Tweedale wrote:

Attached patch fixes a problem with check for IssuerDN in Dogtag
cert search results (found by Coverity; thanks to mbasti for brining
to my attention).

Cheers,
Fraser



ACK

master:
* 47d33f36507d7af16daff5b9f7e4b4acfc6d963b Fix IssuerDN presence check 
in cert search result


-- 
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 0165] keep setting ipakrbprincipal objectclass on new service entries

[Petr Spacek]

On 27.6.2016 12:42, Martin Basti wrote:
> 
> 
> On 27.06.2016 09:39, Martin Babinsky wrote:
>> On 06/27/2016 07:56 AM, Martin Babinsky wrote:
>>> On 06/24/2016 04:07 PM, Martin Babinsky wrote:
 This patch reverts commits 705f66f7490c64de1adc129221b31927616c485 and
 06d945a04607dc36e25af78688b4295420489fb9 responsible for
 https://fedorahosted.org/freeipa/ticket/5996

 This should unblock replica promotion.



>>> self-NACK, disregard this patch, it should not be necessary to revert
>>> the whole commit
>>>
>>
>> This version only reverts the change that actually breaks stuff.
>>
>> https://fedorahosted.org/freeipa/ticket/5996
>>
>>
>>
> 
> Works for me


Worked for me too, I tried to install replica using admin credentials against
master which was upgraded from 4.3.1 to latest master.

-- 
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 0164] Fix incorrect construction of service principal during replica cleanup

[Martin Basti]

On 24.06.2016 10:36, Martin Babinsky wrote:

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




ACK

Pushed to master: 9392b212719032a694ff47ae8802b46f9f58e718

-- 
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 0165] keep setting ipakrbprincipal objectclass on new service entries

[Martin Basti]

On 27.06.2016 09:39, Martin Babinsky wrote:

On 06/27/2016 07:56 AM, Martin Babinsky wrote:

On 06/24/2016 04:07 PM, Martin Babinsky wrote:

This patch reverts commits 705f66f7490c64de1adc129221b31927616c485 and
06d945a04607dc36e25af78688b4295420489fb9 responsible for
https://fedorahosted.org/freeipa/ticket/5996

This should unblock replica promotion.




self-NACK, disregard this patch, it should not be necessary to revert
the whole commit



This version only reverts the change that actually breaks stuff.

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





Works for me
-- 
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 0544] ipa-rmkeytab, ipa-join: dont fail if gettext cannot be initialized

[Martin Basti]

On 27.06.2016 12:22, Petr Spacek wrote:

On 27.6.2016 08:36, Martin Basti wrote:

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

Patch attached.

ACK for this patch set.

Interestingly it does not fix
https://fedorahosted.org/freeipa/ticket/5978


On my broken IPA client it prints following message:
# /usr/sbin/ipa-rmkeytab -k /etc/krb5.keytab -r
DOM-046.ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
Failed to load translations
realm not found

# echo $?
5

Maybe your client is broken too much, I tried reproducer from ticket and 
it is working, I don't have to do rm /etc/krb5.keytab anymore (tried twice)


Pushed to master: a07030f3867168969d32f0f46e792ae0697529bc


--
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 0544] ipa-rmkeytab, ipa-join: dont fail if gettext cannot be initialized

[Petr Spacek]

On 27.6.2016 08:36, Martin Basti wrote:
> https://fedorahosted.org/freeipa/ticket/5973
> 
> Patch attached.

ACK for this patch set.

Interestingly it does not fix
https://fedorahosted.org/freeipa/ticket/5978


On my broken IPA client it prints following message:
# /usr/sbin/ipa-rmkeytab -k /etc/krb5.keytab -r
DOM-046.ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
Failed to load translations
realm not found

# echo $?
5

-- 
Petr^2 Spacek

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

[Freeipa-devel] [PATCH 0139] DNS: Fix tests for realm domains integration with DNS zone ad

[Petr Spacek]

Hello,

DNS: Fix tests for realm domains integration with DNS zone add

We forgot to update tests after change in
22f4045f72daf182c44ce574291c0d8a7733713b.

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


It should go to master, 4-3, and 4-2 as well (as the original change).

-- 
Petr^2 Spacek
From 0c0620eca56434f54bd6c1b4b04768d4dcb5abe7 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Mon, 27 Jun 2016 11:46:09 +0200
Subject: [PATCH] DNS: Fix tests for realm domains integration with DNS zone
 add

We forgot to update tests after change in
22f4045f72daf182c44ce574291c0d8a7733713b.

https://fedorahosted.org/freeipa/ticket/5980
---
 ipatests/test_xmlrpc/test_dns_realmdomains_integration.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_dns_realmdomains_integration.py b/ipatests/test_xmlrpc/test_dns_realmdomains_integration.py
index 9482ef6abf24d87f28447b1219f628662617528b..4b3b5666d8d8c5c4ac7a001008e87c88621d63b9 100644
--- a/ipatests/test_xmlrpc/test_dns_realmdomains_integration.py
+++ b/ipatests/test_xmlrpc/test_dns_realmdomains_integration.py
@@ -135,12 +135,12 @@ class test_dns_realmdomains_integration(Declarative):
 ),
 
 dict(
-desc='Check realmdomain and TXT record do not get created '
- 'during dnszone_add for forwarded zone',
+desc='Check realmdomain and TXT record gets created '
+ 'during dnszone_add for master zone with a forwarder',
 command=(
 'dnszone_add', [dnszone_2], {
 'idnssoarname': idnssoarname,
-'idnsforwarders': u'1.2.3.4',
+'idnsforwarders': u'198.18.19.20',
 'idnsforwardpolicy': u'only',
 }
 ),
@@ -182,7 +182,7 @@ class test_dns_realmdomains_integration(Declarative):
 
 },
 },
-extra_check=assert_realmdomain_and_txt_record_not_present,
+extra_check=assert_realmdomain_and_txt_record_present,
 ),
 
 dict(
-- 
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 0538-0540] DNS locations: epilogue

[Martin Basti]

On 27.06.2016 10:56, Petr Spacek wrote:

On 24.6.2016 12:25, Martin Basti wrote:


On 23.06.2016 18:26, Petr Spacek wrote:

On 23.6.2016 16:38, Martin Basti wrote:

Patches attached.


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


freeipa-mbasti-0538-Revert-DNS-Locations-do-not-generate-location-record.patch


  From 28499422115cbfbb343033511319c7c8710e1ff5 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 21 Jun 2016 18:04:13 +0200
Subject: [PATCH 1/4] Revert "DNS Locations: do not generate location records
   for unused locations"

This reverts commit bbf8227e3fd678d4bd6659a12055ba3dbe1c8230.

After deeper investigation, we found out that empty locations are needed
for clients, because clients may have cached records for longer time for
that particular location. Only way how to remove location is to remove
it using location-del

https://fedorahosted.org/freeipa/ticket/2008
---
   ipaserver/dns_data_management.py | 11 ---
   1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/ipaserver/dns_data_management.py
b/ipaserver/dns_data_management.py
index
a9e9c0a3856961b5494c8d3ca30ddb2e4aa5c523..eac2e7d1a5618ea92372bd81b7d12752791ef117
100644
--- a/ipaserver/dns_data_management.py
+++ b/ipaserver/dns_data_management.py
@@ -68,7 +68,6 @@ class IPASystemRecords(object):
   self.api_instance = api_instance
   self.domain_abs =
DNSName(self.api_instance.env.domain).make_absolute()
   self.servers_data = {}
-self.used_locations = set()
   self.__init_data()
 def reload_data(self):
@@ -92,7 +91,6 @@ class IPASystemRecords(object):
 def __init_data(self):
   self.servers_data = {}
-self.used_locations = set()
 servers_result = self.api_instance.Command.server_find(
   pkey_only=True)['result']
@@ -104,8 +102,6 @@ class IPASystemRecords(object):
   'location': location,
   'roles': roles,
   }
-if location:
-self.used_locations.add(location)
 def __add_srv_records(
   self, zone_obj, hostname, rname_port_map,
@@ -353,12 +349,13 @@ class IPASystemRecords(object):
   pkey_only=True)['result']
   servers = [s['cn'][0] for s in servers_result]
   -# generate only records for used location, records for unassigned
-# locations are useless
+locations_result =
self.api_instance.Command.location_find()['result']
+locations = [l['idnsname'][0] for l in locations_result]
+
   for server in servers:
   self._get_location_dns_records_for_server(
   zone_obj, server,
-self.used_locations, roles=roles,
+locations, roles=roles,
   include_master_role=include_master_role)
   return zone_obj
   -- 2.5.5


freeipa-mbasti-0539-DNS-Locations-hide-option-no-msdcs-in-adtrust-instal.patch


  From 37cae4f05cd3c0a2c4de037402938a5437dbc072 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 21 Jun 2016 18:17:55 +0200
Subject: [PATCH 2/4] DNS Locations: hide option --no-msdcs in adtrust-install

Since DNS location mechanism is active, this option has no effect,
because records are generate dynamically.

https://fedorahosted.org/freeipa/ticket/2008
---
   install/tools/ipa-adtrust-install| 10 +++---
   ipaserver/install/adtrustinstance.py | 21 -
   2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/install/tools/ipa-adtrust-install
b/install/tools/ipa-adtrust-install
index
5babcdb7cb169e4a944acca55739064e0464d41e..5ba72a65d00ca683239a4ff3c5e7cfdc62c0bb6c
100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -29,6 +29,8 @@ import ldap
 import six
   +from optparse import SUPPRESS_HELP
+
   from ipaserver.install import adtrustinstance
   from ipaserver.install.installutils import (
   read_password,
@@ -54,9 +56,11 @@ def parse_options():
 default=False, help="print debugging information")
   parser.add_option("--netbios-name", dest="netbios_name",
 help="NetBIOS name of the IPA domain")
+
+# no-msdcs has not effect, option is here just for backward compatibility
   parser.add_option("--no-msdcs", dest="no_msdcs", action="store_true",
-  default=False, help="Do not create DNS service
records " \
-  "for Windows in managed DNS
server")
+  default=False, help=SUPPRESS_HELP)
+
   parser.add_option("--rid-base", dest="rid_base", type=int, default=1000,
 help="Start value for mapping UIDs and GIDs to RIDs")
   parser.add_option("--secondary-rid-base", dest="secondary_rid_base",
@@ -390,7 +394,7 @@ def main():
   smb.setup(api.env.host, api.env.realm,
 netbios_name, 

Re: [Freeipa-devel] [PATCH 0138] replica-install: Compare domain names as DNS names and not string

[Petr Spacek]

On 27.6.2016 10:30, Martin Basti wrote:
> 
> 
> On 23.06.2016 18:32, Petr Spacek wrote:
>> Hello,
>>
>> replica-install: Compare domain names as DNS names and not strings
>>
>> This fixes false possitive where user inputs "example.com" and "EXAMPLE.COM"
>> were not considered equivalent and installation was wrongly refused.
>>
>> https://fedorahosted.org/freeipa/ticket/5976
>>
> 
> NACK, client installer should normalize domain name as host-add does, because
> it will blow up in different places, we cannot compare this part as DNS name
> when other parts works with it as strings
> 
> ipa.ipapython.install.cli.install_tool(Replica): ERRORCannot promote this
> client to a replica. Local domain 'ipa.example.COM' does not match IPA domain
> 'ipa.example.com'.

Okay, I will use the same validator as ipa-server-install and normalize it as
you suggested.

-- 
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

[Lenka Doudova]

On 06/27/2016 10:33 AM, Martin Babinsky wrote:

On 06/27/2016 10:28 AM, Petr Spacek wrote:

On 27.6.2016 10:26, Petr Spacek wrote:

On 27.6.2016 10:18, Martin Babinsky wrote:

On 06/27/2016 10:04 AM, Petr Vobornik wrote:

On 06/27/2016 09:42 AM, Lenka Doudova wrote:

Hi!

With newly created AD machines in Brno lab, existing trust tests 
fail on
'ipa dnsforwardzone-add' command claiming the zone is already 
present,

as new AD domain is dom-221.idm.lab.eng.brq.redhat.com.

To prevent these failures I prepared attached patch, that will still
attempt to add the forward zone, but in case of non-zero return code
will check the message if it says that the forward zone is already
configured, and lets the tests continue, if it is so.


Lenka




Current approach expects that every error of ipa dnsforward-add here
will mean that the zone exists. So it might hide other issues - 
not very

good.

On the other hand it is not very robust to parse error message.

Question for general audience: What do you think if IPA client's exit
status would be the IPA error code instead of "1" for every error. 
E.g.

in DuplicateEntry case it's 4002.

Btw, this is not a NACK.



Well AFAIK the exit status on POSIX systems is encoded into a 
single byte so
you cannot have the return value greater that 255. We would have to 
devise
some mapping between our XMLRPC status codes and subprocess return 
codes.


Some of our exceptions have defined return values outside plain 
'1', e.g.
NotFound has return value of 2. It would be possible to extend this 
concept on

other common errors.


Even more importantly, the forward zone is completely unnecessary 
because DNS
when DNS is set up properly. I would simply remove the 
dnsforwardzone-add.



Grr, I meant this:
Even more importantly, the forward zone is completely unnecessary 
when DNS is

set up properly. I would simply remove the dnsforwardzone-add.

+1, our tests should not fiddle with the provisioned environment as 
much as they sometimes do.


Well, I have nothing against removing it completely, but left it there 
just because with previous AD machines with "wild" domains it was necessary.
Looking at the code, your suggestion would probably mean to entirely 
remove method configure_dns_for_trust from 
ipatests/test_integration/tasks.py, right? I'll have to verify this 
won't break anything else.


Lenka

--
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 0538-0540] DNS locations: epilogue

[Petr Spacek]

On 24.6.2016 12:25, Martin Basti wrote:
> 
> 
> On 23.06.2016 18:26, Petr Spacek wrote:
>> On 23.6.2016 16:38, Martin Basti wrote:
>>> Patches attached.
>>>
>>>
>>> https://fedorahosted.org/freeipa/ticket/2008
>>>
>>>
>>> freeipa-mbasti-0538-Revert-DNS-Locations-do-not-generate-location-record.patch
>>>
>>>
>>>  From 28499422115cbfbb343033511319c7c8710e1ff5 Mon Sep 17 00:00:00 2001
>>> From: Martin Basti 
>>> Date: Tue, 21 Jun 2016 18:04:13 +0200
>>> Subject: [PATCH 1/4] Revert "DNS Locations: do not generate location records
>>>   for unused locations"
>>>
>>> This reverts commit bbf8227e3fd678d4bd6659a12055ba3dbe1c8230.
>>>
>>> After deeper investigation, we found out that empty locations are needed
>>> for clients, because clients may have cached records for longer time for
>>> that particular location. Only way how to remove location is to remove
>>> it using location-del
>>>
>>> https://fedorahosted.org/freeipa/ticket/2008
>>> ---
>>>   ipaserver/dns_data_management.py | 11 ---
>>>   1 file changed, 4 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/ipaserver/dns_data_management.py
>>> b/ipaserver/dns_data_management.py
>>> index
>>> a9e9c0a3856961b5494c8d3ca30ddb2e4aa5c523..eac2e7d1a5618ea92372bd81b7d12752791ef117
>>> 100644
>>> --- a/ipaserver/dns_data_management.py
>>> +++ b/ipaserver/dns_data_management.py
>>> @@ -68,7 +68,6 @@ class IPASystemRecords(object):
>>>   self.api_instance = api_instance
>>>   self.domain_abs =
>>> DNSName(self.api_instance.env.domain).make_absolute()
>>>   self.servers_data = {}
>>> -self.used_locations = set()
>>>   self.__init_data()
>>> def reload_data(self):
>>> @@ -92,7 +91,6 @@ class IPASystemRecords(object):
>>> def __init_data(self):
>>>   self.servers_data = {}
>>> -self.used_locations = set()
>>> servers_result = self.api_instance.Command.server_find(
>>>   pkey_only=True)['result']
>>> @@ -104,8 +102,6 @@ class IPASystemRecords(object):
>>>   'location': location,
>>>   'roles': roles,
>>>   }
>>> -if location:
>>> -self.used_locations.add(location)
>>> def __add_srv_records(
>>>   self, zone_obj, hostname, rname_port_map,
>>> @@ -353,12 +349,13 @@ class IPASystemRecords(object):
>>>   pkey_only=True)['result']
>>>   servers = [s['cn'][0] for s in servers_result]
>>>   -# generate only records for used location, records for unassigned
>>> -# locations are useless
>>> +locations_result =
>>> self.api_instance.Command.location_find()['result']
>>> +locations = [l['idnsname'][0] for l in locations_result]
>>> +
>>>   for server in servers:
>>>   self._get_location_dns_records_for_server(
>>>   zone_obj, server,
>>> -self.used_locations, roles=roles,
>>> +locations, roles=roles,
>>>   include_master_role=include_master_role)
>>>   return zone_obj
>>>   -- 2.5.5
>>>
>>>
>>> freeipa-mbasti-0539-DNS-Locations-hide-option-no-msdcs-in-adtrust-instal.patch
>>>
>>>
>>>  From 37cae4f05cd3c0a2c4de037402938a5437dbc072 Mon Sep 17 00:00:00 2001
>>> From: Martin Basti 
>>> Date: Tue, 21 Jun 2016 18:17:55 +0200
>>> Subject: [PATCH 2/4] DNS Locations: hide option --no-msdcs in 
>>> adtrust-install
>>>
>>> Since DNS location mechanism is active, this option has no effect,
>>> because records are generate dynamically.
>>>
>>> https://fedorahosted.org/freeipa/ticket/2008
>>> ---
>>>   install/tools/ipa-adtrust-install| 10 +++---
>>>   ipaserver/install/adtrustinstance.py | 21 -
>>>   2 files changed, 15 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/install/tools/ipa-adtrust-install
>>> b/install/tools/ipa-adtrust-install
>>> index
>>> 5babcdb7cb169e4a944acca55739064e0464d41e..5ba72a65d00ca683239a4ff3c5e7cfdc62c0bb6c
>>> 100755
>>> --- a/install/tools/ipa-adtrust-install
>>> +++ b/install/tools/ipa-adtrust-install
>>> @@ -29,6 +29,8 @@ import ldap
>>> import six
>>>   +from optparse import SUPPRESS_HELP
>>> +
>>>   from ipaserver.install import adtrustinstance
>>>   from ipaserver.install.installutils import (
>>>   read_password,
>>> @@ -54,9 +56,11 @@ def parse_options():
>>> default=False, help="print debugging information")
>>>   parser.add_option("--netbios-name", dest="netbios_name",
>>> help="NetBIOS name of the IPA domain")
>>> +
>>> +# no-msdcs has not effect, option is here just for backward 
>>> compatibility
>>>   parser.add_option("--no-msdcs", dest="no_msdcs", action="store_true",
>>> -  default=False, help="Do not create DNS service
>>> records " \
>>> -  "for Windows in managed DNS
>>> server")
>>> +  

Re: [Freeipa-devel] [PATCH 0052] Added missing nsSystemIndex attributes to .update file

[Martin Basti]

On 24.06.2016 14:24, Stanislav Laznicka wrote:

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




ACK

Pushed to master: e136db019210e3e373fe96fe8331976c93b166f3

-- 
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 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

[Martin Babinsky]

On 06/27/2016 10:28 AM, Petr Spacek wrote:

On 27.6.2016 10:26, Petr Spacek wrote:

On 27.6.2016 10:18, Martin Babinsky wrote:

On 06/27/2016 10:04 AM, Petr Vobornik wrote:

On 06/27/2016 09:42 AM, Lenka Doudova wrote:

Hi!

With newly created AD machines in Brno lab, existing trust tests fail on
'ipa dnsforwardzone-add' command claiming the zone is already present,
as new AD domain is dom-221.idm.lab.eng.brq.redhat.com.

To prevent these failures I prepared attached patch, that will still
attempt to add the forward zone, but in case of non-zero return code
will check the message if it says that the forward zone is already
configured, and lets the tests continue, if it is so.


Lenka




Current approach expects that every error of ipa dnsforward-add here
will mean that the zone exists. So it might hide other issues - not very
good.

On the other hand it is not very robust to parse error message.

Question for general audience: What do you think if IPA client's exit
status would be the IPA error code instead of "1" for every error. E.g.
in DuplicateEntry case it's 4002.

Btw, this is not a NACK.



Well AFAIK the exit status on POSIX systems is encoded into a single byte so
you cannot have the return value greater that 255. We would have to devise
some mapping between our XMLRPC status codes and subprocess return codes.

Some of our exceptions have defined return values outside plain '1', e.g.
NotFound has return value of 2. It would be possible to extend this concept on
other common errors.


Even more importantly, the forward zone is completely unnecessary because DNS
when DNS is set up properly. I would simply remove the dnsforwardzone-add.


Grr, I meant this:
Even more importantly, the forward zone is completely unnecessary when DNS is
set up properly. I would simply remove the dnsforwardzone-add.

+1, our tests should not fiddle with the provisioned environment as much 
as they sometimes do.


--
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 0138] replica-install: Compare domain names as DNS names and not string

[Martin Basti]

On 23.06.2016 18:32, Petr Spacek wrote:

Hello,

replica-install: Compare domain names as DNS names and not strings

This fixes false possitive where user inputs "example.com" and "EXAMPLE.COM"
were not considered equivalent and installation was wrongly refused.

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



NACK, client installer should normalize domain name as host-add does, 
because it will blow up in different places, we cannot compare this part 
as DNS name when other parts works with it as strings


ipa.ipapython.install.cli.install_tool(Replica): ERRORCannot promote 
this client to a replica. Local domain 'ipa.example.COM' does not match 
IPA domain 'ipa.example.com'.


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 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

[Petr Spacek]

On 27.6.2016 10:26, Petr Spacek wrote:
> On 27.6.2016 10:18, Martin Babinsky wrote:
>> On 06/27/2016 10:04 AM, Petr Vobornik wrote:
>>> On 06/27/2016 09:42 AM, Lenka Doudova wrote:
 Hi!

 With newly created AD machines in Brno lab, existing trust tests fail on
 'ipa dnsforwardzone-add' command claiming the zone is already present,
 as new AD domain is dom-221.idm.lab.eng.brq.redhat.com.

 To prevent these failures I prepared attached patch, that will still
 attempt to add the forward zone, but in case of non-zero return code
 will check the message if it says that the forward zone is already
 configured, and lets the tests continue, if it is so.


 Lenka

>>>
>>>
>>> Current approach expects that every error of ipa dnsforward-add here
>>> will mean that the zone exists. So it might hide other issues - not very
>>> good.
>>>
>>> On the other hand it is not very robust to parse error message.
>>>
>>> Question for general audience: What do you think if IPA client's exit
>>> status would be the IPA error code instead of "1" for every error. E.g.
>>> in DuplicateEntry case it's 4002.
>>>
>>> Btw, this is not a NACK.
>>>
>>
>> Well AFAIK the exit status on POSIX systems is encoded into a single byte so
>> you cannot have the return value greater that 255. We would have to devise
>> some mapping between our XMLRPC status codes and subprocess return codes.
>>
>> Some of our exceptions have defined return values outside plain '1', e.g.
>> NotFound has return value of 2. It would be possible to extend this concept 
>> on
>> other common errors.
> 
> Even more importantly, the forward zone is completely unnecessary because DNS
> when DNS is set up properly. I would simply remove the dnsforwardzone-add.
> 
Grr, I meant this:
Even more importantly, the forward zone is completely unnecessary when DNS is
set up properly. I would simply remove the dnsforwardzone-add.

-- 
Petr^2 Spacek

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

Re: [Freeipa-devel] [Testplan Review] Certs in ID overrides

[Sumit Bose]

On Mon, Jun 27, 2016 at 10:06:23AM +0200, Oleg Fayans wrote:
> Hi Sumit,
> 
> I've updated the testplan. (Thank you for the link to Fraser's blogpost,
> it was really very useful!). All the operations described  were
> performed manually and succeed. Could you please review it again in case
> I forgot something?

Thank you, the tests are looking good.

I have two comments. First, for your information, I#m not sure if WebUI
is in the scope of this tests, Pavel already send '0058 WebUI:
certificate widget on ID override user page' to the freeipa-devel list,
so adding certificates to idoverrides with the WebUI should work soon as
well.

Second, the LDAP attribute used to store the certificates is a
multi-value attribute. Adding a test where a second certificate is added
to the override and removed (without deleting the other certificate)
might be useful here.

bye,
Sumit

> 
> 
> On 06/09/2016 05:06 PM, Sumit Bose wrote:
> > On Thu, Jun 09, 2016 at 04:48:57PM +0200, Oleg Fayans wrote:
> >> Hi guys,
> >>
> >> Here is the first somewhat skeletal and pretty short version of the
> >> testplan. Could you please review it anyone?
> >>
> >> http://www.freeipa.org/page/V4/Certs_in_ID_overrides/Test_Plan
> > 
> > Hi Oleg,
> > 
> > 'Make sure the id view is applied to ipa master host' the IPA
> > masters/servers will always and only have the 'Default Trust View'. But
> > it is ok to use the 'Default Trust View' for testing the certificates in
> > the ID override.
> > 
> > The 'openssl req ...' call will only generate a certificate request and
> > not the certificate itself. The request must still be signed by e.g. the
> > IPA CA. Please see the blog posts of Fraser
> > (https://blog-ftweedal.rhcloud.com/2015/08/user-certificates-and-custom-profiles-with-freeipa-4-2/)
> > and Nathan (https://blog-nkinder.rhcloud.com/?p=184) for details.
> > 
> > Since you want to test certificates in overrides you should use
> > idoverrideuser-add-cert and idoverrideuser-remove-cert instead of
> > user-add-cert and user-remove-cert.
> > 
> > bye,
> > Sumit
> > 
> >> -- 
> >> Oleg Fayans
> >> Quality Engineer
> >> FreeIPA team
> >> RedHat.
> >>
> >> -- 
> >> 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
> 
> -- 
> Oleg Fayans
> Quality Engineer
> FreeIPA team
> RedHat.

-- 
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 0020][Tests] Make ID views test reflect new krbcanonicalname attribute

[Martin Babinsky]

On 06/23/2016 03:51 PM, Lenka Doudova wrote:

Patch attached.

Lenka





Thanks for catching this.

conditional ACK if you add https://fedorahosted.org/freeipa/ticket/3864 
to the commit message.


--
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 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

[Petr Spacek]

On 27.6.2016 10:18, Martin Babinsky wrote:
> On 06/27/2016 10:04 AM, Petr Vobornik wrote:
>> On 06/27/2016 09:42 AM, Lenka Doudova wrote:
>>> Hi!
>>>
>>> With newly created AD machines in Brno lab, existing trust tests fail on
>>> 'ipa dnsforwardzone-add' command claiming the zone is already present,
>>> as new AD domain is dom-221.idm.lab.eng.brq.redhat.com.
>>>
>>> To prevent these failures I prepared attached patch, that will still
>>> attempt to add the forward zone, but in case of non-zero return code
>>> will check the message if it says that the forward zone is already
>>> configured, and lets the tests continue, if it is so.
>>>
>>>
>>> Lenka
>>>
>>
>>
>> Current approach expects that every error of ipa dnsforward-add here
>> will mean that the zone exists. So it might hide other issues - not very
>> good.
>>
>> On the other hand it is not very robust to parse error message.
>>
>> Question for general audience: What do you think if IPA client's exit
>> status would be the IPA error code instead of "1" for every error. E.g.
>> in DuplicateEntry case it's 4002.
>>
>> Btw, this is not a NACK.
>>
> 
> Well AFAIK the exit status on POSIX systems is encoded into a single byte so
> you cannot have the return value greater that 255. We would have to devise
> some mapping between our XMLRPC status codes and subprocess return codes.
> 
> Some of our exceptions have defined return values outside plain '1', e.g.
> NotFound has return value of 2. It would be possible to extend this concept on
> other common errors.

Even more importantly, the forward zone is completely unnecessary because DNS
when DNS is set up properly. I would simply remove the dnsforwardzone-add.

-- 
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

[Petr Vobornik]

On 06/27/2016 10:10 AM, Lenka Doudova wrote:
> 
> 
> On 06/27/2016 10:04 AM, Petr Vobornik wrote:
>> On 06/27/2016 09:42 AM, Lenka Doudova wrote:
>>> Hi!
>>>
>>> With newly created AD machines in Brno lab, existing trust tests fail on
>>> 'ipa dnsforwardzone-add' command claiming the zone is already present,
>>> as new AD domain is dom-221.idm.lab.eng.brq.redhat.com.
>>>
>>> To prevent these failures I prepared attached patch, that will still
>>> attempt to add the forward zone, but in case of non-zero return code
>>> will check the message if it says that the forward zone is already
>>> configured, and lets the tests continue, if it is so.
>>>
>>>
>>> Lenka
>>>
>>
>> Current approach expects that every error of ipa dnsforward-add here
>> will mean that the zone exists. So it might hide other issues - not very
>> good.
> If I understand your comment correctly, you think that the patch would
> pass ANY dnsforwardzone-add error and being OK and continue, right?
> That's not intended behaviour - I have an assertion there that checks
> that it's really the 'correct' error:
> 
> assert "already exists in DNS" in result.stderr_text
> 
> So any other error should still prevent continuing in tests.
> 

Ah, good, I've overlooked that.


-- 
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] DNS Locations: fix an issue found by coverity

[Martin Basti]

On 27.06.2016 10:21, Martin Babinsky wrote:

On 06/27/2016 08:34 AM, Martin Basti wrote:

Shame, shame, shame on me. I forgot how to python when I was writing
that originally.


Patch attached.

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




ACK.


master:
* c6f7d94d5b39c213483909de34c61016b8eba0ac DNS Locations: server-mod: 
fix if statement


--
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] DNS Locations: fix an issue found by coverity

[Martin Babinsky]

On 06/27/2016 08:34 AM, Martin Basti wrote:

Shame, shame, shame on me. I forgot how to python when I was writing
that originally.


Patch attached.

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




ACK.

--
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 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

[Martin Babinsky]

On 06/27/2016 10:04 AM, Petr Vobornik wrote:

On 06/27/2016 09:42 AM, Lenka Doudova wrote:

Hi!

With newly created AD machines in Brno lab, existing trust tests fail on
'ipa dnsforwardzone-add' command claiming the zone is already present,
as new AD domain is dom-221.idm.lab.eng.brq.redhat.com.

To prevent these failures I prepared attached patch, that will still
attempt to add the forward zone, but in case of non-zero return code
will check the message if it says that the forward zone is already
configured, and lets the tests continue, if it is so.


Lenka




Current approach expects that every error of ipa dnsforward-add here
will mean that the zone exists. So it might hide other issues - not very
good.

On the other hand it is not very robust to parse error message.

Question for general audience: What do you think if IPA client's exit
status would be the IPA error code instead of "1" for every error. E.g.
in DuplicateEntry case it's 4002.

Btw, this is not a NACK.



Well AFAIK the exit status on POSIX systems is encoded into a single 
byte so you cannot have the return value greater that 255. We would have 
to devise some mapping between our XMLRPC status codes and subprocess 
return codes.


Some of our exceptions have defined return values outside plain '1', 
e.g. NotFound has return value of 2. It would be possible to extend this 
concept on other common errors.


--
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 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

[Lenka Doudova]

On 06/27/2016 10:04 AM, Petr Vobornik wrote:

On 06/27/2016 09:42 AM, Lenka Doudova wrote:

Hi!

With newly created AD machines in Brno lab, existing trust tests fail on
'ipa dnsforwardzone-add' command claiming the zone is already present,
as new AD domain is dom-221.idm.lab.eng.brq.redhat.com.

To prevent these failures I prepared attached patch, that will still
attempt to add the forward zone, but in case of non-zero return code
will check the message if it says that the forward zone is already
configured, and lets the tests continue, if it is so.


Lenka



Current approach expects that every error of ipa dnsforward-add here
will mean that the zone exists. So it might hide other issues - not very
good.
If I understand your comment correctly, you think that the patch would 
pass ANY dnsforwardzone-add error and being OK and continue, right? 
That's not intended behaviour - I have an assertion there that checks 
that it's really the 'correct' error:


assert "already exists in DNS" in result.stderr_text

So any other error should still prevent continuing in tests.



On the other hand it is not very robust to parse error message.

Question for general audience: What do you think if IPA client's exit
status would be the IPA error code instead of "1" for every error. E.g.
in DuplicateEntry case it's 4002.
Personally I think it would be nice to have DuplicateEntry error rather 
the just "1" in this case. Even for testing purposes I believe it would 
be better than bunch of asserts.


Btw, this is not a NACK.


--
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] [Testplan Review] Certs in ID overrides

[Oleg Fayans]

Hi Sumit,

I've updated the testplan. (Thank you for the link to Fraser's blogpost,
it was really very useful!). All the operations described  were
performed manually and succeed. Could you please review it again in case
I forgot something?


On 06/09/2016 05:06 PM, Sumit Bose wrote:
> On Thu, Jun 09, 2016 at 04:48:57PM +0200, Oleg Fayans wrote:
>> Hi guys,
>>
>> Here is the first somewhat skeletal and pretty short version of the
>> testplan. Could you please review it anyone?
>>
>> http://www.freeipa.org/page/V4/Certs_in_ID_overrides/Test_Plan
> 
> Hi Oleg,
> 
> 'Make sure the id view is applied to ipa master host' the IPA
> masters/servers will always and only have the 'Default Trust View'. But
> it is ok to use the 'Default Trust View' for testing the certificates in
> the ID override.
> 
> The 'openssl req ...' call will only generate a certificate request and
> not the certificate itself. The request must still be signed by e.g. the
> IPA CA. Please see the blog posts of Fraser
> (https://blog-ftweedal.rhcloud.com/2015/08/user-certificates-and-custom-profiles-with-freeipa-4-2/)
> and Nathan (https://blog-nkinder.rhcloud.com/?p=184) for details.
> 
> Since you want to test certificates in overrides you should use
> idoverrideuser-add-cert and idoverrideuser-remove-cert instead of
> user-add-cert and user-remove-cert.
> 
> bye,
> Sumit
> 
>> -- 
>> Oleg Fayans
>> Quality Engineer
>> FreeIPA team
>> RedHat.
>>
>> -- 
>> 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

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

-- 
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 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

[Petr Vobornik]

On 06/27/2016 09:42 AM, Lenka Doudova wrote:
> Hi!
> 
> With newly created AD machines in Brno lab, existing trust tests fail on
> 'ipa dnsforwardzone-add' command claiming the zone is already present,
> as new AD domain is dom-221.idm.lab.eng.brq.redhat.com.
> 
> To prevent these failures I prepared attached patch, that will still
> attempt to add the forward zone, but in case of non-zero return code
> will check the message if it says that the forward zone is already
> configured, and lets the tests continue, if it is so.
> 
> 
> Lenka
> 


Current approach expects that every error of ipa dnsforward-add here
will mean that the zone exists. So it might hide other issues - not very
good.

On the other hand it is not very robust to parse error message.

Question for general audience: What do you think if IPA client's exit
status would be the IPA error code instead of "1" for every error. E.g.
in DuplicateEntry case it's 4002.

Btw, this is not a NACK.
-- 
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

[Freeipa-devel] [PATCH 0166] test-{service, host}-plugin: only expect krbcanonicalname when all=True

[Martin Babinsky]

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

--
Martin^3 Babinsky
From 4f63ed2c710cac33da2fb11f354c6a46a265f42b Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 27 Jun 2016 09:40:17 +0200
Subject: [PATCH] test-{service,host}-plugin: only expect krbcanonicalname when
 all=True

fixes incorrect assertions in tests that create, retrieve, and search for
services

https://fedorahosted.org/freeipa/ticket/3864
---
 ipatests/test_xmlrpc/test_host_plugin.py| 1 -
 ipatests/test_xmlrpc/test_service_plugin.py | 5 -
 2 files changed, 6 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py
index 4ddabefff14e61e8e2f33c0dbcb55f657330c438..e6fc68a15cb9e7176979148462c469d1a737b040 100644
--- a/ipatests/test_xmlrpc/test_host_plugin.py
+++ b/ipatests/test_xmlrpc/test_host_plugin.py
@@ -357,7 +357,6 @@ class TestHostWithService(XMLRPC_test):
 result=dict(
 dn=service1dn,
 krbprincipalname=[service1],
-krbcanonicalname=[service1],
 objectclass=objectclasses.service,
 managedby_host=[host.fqdn],
 ipauniqueid=[fuzzy_uuid],
diff --git a/ipatests/test_xmlrpc/test_service_plugin.py b/ipatests/test_xmlrpc/test_service_plugin.py
index f22824f9ab101e10961eecf241420eac92315a68..2e38b8d6b65158848bbd25eaebbf7aecddf6b24a 100644
--- a/ipatests/test_xmlrpc/test_service_plugin.py
+++ b/ipatests/test_xmlrpc/test_service_plugin.py
@@ -193,7 +193,6 @@ class test_service(Declarative):
 result=dict(
 dn=service1dn,
 krbprincipalname=[service1],
-krbcanonicalname=[service1],
 objectclass=objectclasses.service,
 ipauniqueid=[fuzzy_uuid],
 managedby_host=[fqdn1],
@@ -263,7 +262,6 @@ class test_service(Declarative):
 dict(
 dn=service1dn,
 krbprincipalname=[service1],
-krbcanonicalname=service1,
 managedby_host=[fqdn1],
 has_keytab=False,
 ),
@@ -283,7 +281,6 @@ class test_service(Declarative):
 dict(
 dn=service1dn,
 krbprincipalname=[service1],
-krbcanonicalname=service1,
 has_keytab=False,
 ),
 ],
@@ -718,7 +715,6 @@ class test_service_in_role(Declarative):
 result=dict(
 dn=service1dn,
 krbprincipalname=[service1],
-krbcanonicalname=[service1],
 objectclass=objectclasses.service,
 ipauniqueid=[fuzzy_uuid],
 managedby_host=[fqdn1],
@@ -923,7 +919,6 @@ class test_service_allowed_to(Declarative):
 result=dict(
 dn=service1dn,
 krbprincipalname=[service1],
-krbcanonicalname=[service1],
 objectclass=objectclasses.service,
 ipauniqueid=[fuzzy_uuid],
 managedby_host=[fqdn1],
-- 
2.5.5

-- 
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 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

[Lenka Doudova]

Hi!

With newly created AD machines in Brno lab, existing trust tests fail on 
'ipa dnsforwardzone-add' command claiming the zone is already present, 
as new AD domain is dom-221.idm.lab.eng.brq.redhat.com.


To prevent these failures I prepared attached patch, that will still 
attempt to add the forward zone, but in case of non-zero return code 
will check the message if it says that the forward zone is already 
configured, and lets the tests continue, if it is so.



Lenka

From 33761de592e867d63665ebe974dbec7c29294367 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Mon, 27 Jun 2016 09:29:31 +0200
Subject: [PATCH] Tests: Prevent trust test failures caused by adding duplicate
 DNS forward zone

When a DNS forward zone is already configured, the 'ipa dnsforwardzone-add' command fails and prevents running trust test. New check is added that will allow tests to continue even if the command raises error caused by attempt to add already existing zone.
---
 ipatests/test_integration/tasks.py | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 38218fa709c2c220d5fea98a092b55e995d48d77..662fa3b73b9d1bea597199cad4d438e1ab339b01 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -473,11 +473,13 @@ def configure_dns_for_trust(master, ad):
 master.run_command(['ipa', 'dnszone-mod', master.domain.name,
 '--allow-transfer', ad.ip])
 else:
-master.run_command(['ipa', 'dnsforwardzone-add', ad.domain.name,
-'--forwarder', ad.ip,
-'--forward-policy', 'only',
-])
-
+result = master.run_command(['ipa', 'dnsforwardzone-add',
+ ad.domain.name,
+ '--forwarder', ad.ip,
+ '--forward-policy', 'only'
+ ], raiseonerr=False)
+if result.returncode != 0:
+assert "already exists in DNS" in result.stderr_text
 
 def establish_trust_with_ad(master, ad, extra_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 0165] keep setting ipakrbprincipal objectclass on new service entries

[Martin Babinsky]

On 06/27/2016 07:56 AM, Martin Babinsky wrote:

On 06/24/2016 04:07 PM, Martin Babinsky wrote:

This patch reverts commits 705f66f7490c64de1adc129221b31927616c485 and
06d945a04607dc36e25af78688b4295420489fb9 responsible for
https://fedorahosted.org/freeipa/ticket/5996

This should unblock replica promotion.




self-NACK, disregard this patch, it should not be necessary to revert
the whole commit



This version only reverts the change that actually breaks stuff.

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

--
Martin^3 Babinsky
From 700a29dc8dc87220bcbf301e32d8ea32b63d4ac0 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 27 Jun 2016 08:48:29 +0200
Subject: [PATCH] keep setting ipakrbprincipal objectclass on new service
 entries

this is required for replica promotion to work, since the ACI allowing hosts
to add their own services uses this objectclass as target filter.

This partially reverts changes from commit
705f66f7490c64de1adc129221b31927616c485d

https://fedorahosted.org/freeipa/ticket/5996
---
 ipaserver/plugins/service.py| 9 +
 ipatests/test_xmlrpc/objectclasses.py   | 1 +
 ipatests/test_xmlrpc/test_service_plugin.py | 4 +++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py
index cb9952d4479a543321999269cb4bd6ace0714436..701314f8d9f2ac14c2b92fea1b75c7bf1754dac3 100644
--- a/ipaserver/plugins/service.py
+++ b/ipaserver/plugins/service.py
@@ -576,6 +576,15 @@ class service_add(LDAPCreate):
 if not 'managedby' in entry_attrs:
 entry_attrs['managedby'] = hostresult['dn']
 
+# Enforce ipaKrbPrincipalAlias to aid case-insensitive searches
+# as krbPrincipalName/krbCanonicalName are case-sensitive in Kerberos
+# schema
+entry_attrs['ipakrbprincipalalias'] = keys[-1]
+
+# Objectclass ipakrbprincipal providing ipakrbprincipalalias is not in
+# in a list of default objectclasses, add it manually
+entry_attrs['objectclass'].append('ipakrbprincipal')
+
 # set krbcanonicalname attribute to enable principal canonicalization
 util.set_krbcanonicalname(entry_attrs)
 
diff --git a/ipatests/test_xmlrpc/objectclasses.py b/ipatests/test_xmlrpc/objectclasses.py
index 7050de289760ede29d057e42658c2f68d8506249..134a08803f3abca1124c4d26274d9e3fc981b941 100644
--- a/ipatests/test_xmlrpc/objectclasses.py
+++ b/ipatests/test_xmlrpc/objectclasses.py
@@ -100,6 +100,7 @@ service = [
 u'ipaobject',
 u'ipaservice',
 u'pkiuser',
+u'ipakrbprincipal',
 u'top',
 ]
 
diff --git a/ipatests/test_xmlrpc/test_service_plugin.py b/ipatests/test_xmlrpc/test_service_plugin.py
index 3009521c3b2d9c496bff4e11b96838ce50a2eefa..f22824f9ab101e10961eecf241420eac92315a68 100644
--- a/ipatests/test_xmlrpc/test_service_plugin.py
+++ b/ipatests/test_xmlrpc/test_service_plugin.py
@@ -239,6 +239,7 @@ class test_service(Declarative):
 result=dict(
 dn=service1dn,
 krbprincipalname=[service1],
+ipakrbprincipalalias=[service1],
 krbcanonicalname=[service1],
 objectclass=objectclasses.service,
 ipauniqueid=[fuzzy_uuid],
@@ -301,7 +302,8 @@ class test_service(Declarative):
 dict(
 dn=service1dn,
 krbprincipalname=[service1],
-krbcanonicalname=service1,
+ipakrbprincipalalias=[service1],
+krbcanonicalname=[service1],
 objectclass=objectclasses.service,
 ipauniqueid=[fuzzy_uuid],
 has_keytab=False,
-- 
2.5.5

-- 
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 0542] ipa-getkeytab: increase LDAP timeout

[Martin Basti]

On 27.06.2016 09:31, Petr Spacek wrote:

On 23.6.2016 17:28, Martin Basti wrote:

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


Patch attached.

I do not have reproducer but it seems reasonable. ACK.


Pushed to master: deb99c11d4c0f7c5f68ed36b183f69281bf6

--
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 0542] ipa-getkeytab: increase LDAP timeout

[Petr Spacek]

On 23.6.2016 17:28, Martin Basti wrote:
> https://fedorahosted.org/freeipa/ticket/5842
> 
> 
> Patch attached.

I do not have reproducer but it seems reasonable. ACK.

-- 
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH] 0008 Do not allow installation in FIPS mode

[Petr Spacek]

On 27.6.2016 08:38, Florence Blanc-Renaud wrote:
> Hi,
> 
> this fix is a port of Bug 1131570 - Do not allow IdM server/replica/client
> installation in a FIPS-140 mode
> It prevents installation of FreeIPA if the host is fips-enabled.
> 
> https://fedorahosted.org/freeipa/ticket/5761
> 
> freeipa-frenaud-0008-Do-not-allow-installation-in-FIPS-mode.patch
> 
> 
>>From afecbb3d228cf1d6cee59da53bf7a803f030d0b1 Mon Sep 17 00:00:00 2001
> From: Florence Blanc-Renaud 
> Date: Fri, 24 Jun 2016 16:16:22 +0200
> Subject: [PATCH] Do not allow installation in FIPS mode
> 
> https://fedorahosted.org/freeipa/ticket/5761
> ---
>  client/ipa-client-install  | 4 
>  install/tools/ipactl   | 6 ++
>  ipaserver/install/server/install.py| 5 +
>  ipaserver/install/server/replicainstall.py | 5 +
>  4 files changed, 20 insertions(+)
> 
> diff --git a/client/ipa-client-install b/client/ipa-client-install
> index 
> 0a601b63118b0a3568066495837121c65e5df04f..f80ff9c469709ea3b63902610b3b8b5c35448904
>  100755
> --- a/client/ipa-client-install
> +++ b/client/ipa-client-install
> @@ -3064,6 +3064,10 @@ def main():
>  
>  if not os.getegid() == 0:
>  sys.exit("\nYou must be root to run ipa-client-install.\n")
> +if os.path.exists('/proc/sys/crypto/fips_enabled'):
> +with open('/proc/sys/crypto/fips_enabled', 'r') as f:

Usually it is safer to call open() and catch exception if the file does not
exist. The code above has inherent problem with race-conditions between time
of check (path.exists) and time of use (open).

Of course it is not a problem here because this file is part of kernel's
interface but in general please use the try: open() except: form.

> +if f.read().strip() != '0':
> +sys.exit("Cannot install IPA client in FIPS mode")

Personally I would like to see more informative messages.

I would recommend something like " is not supported in FIPS mode".

In my eyes it is difference between "How do I ...? You dont!" vs "How do I
...? Sorry, we do not support that right now."


Sorry for nitpicking! :-)

Petr^2 Spacek



>  tasks.check_selinux_status()
>  logging_setup(options)
>  root_logger.debug(
> diff --git a/install/tools/ipactl b/install/tools/ipactl
> index 
> 547b21d875dff7231fae8dfc10faf995b0ca230b..9c68fffe73bfdd97789907226f8765c09707d552
>  100755
> --- a/install/tools/ipactl
> +++ b/install/tools/ipactl
> @@ -545,6 +545,12 @@ def main():
>  elif args[0] != "start" and args[0] != "stop" and args[0] != "restart" 
> and args[0] != "status":
>  raise IpactlError("Unrecognized action [" + args[0] + "]", 2)
>  
> +if (args[0] in ('start', 'restart') and
> +os.path.exists('/proc/sys/crypto/fips_enabled')):
> +with open('/proc/sys/crypto/fips_enabled', 'r') as f:
> +if f.read().strip() != '0':
> +raise IpactlError("Cannot start IPA server in FIPS mode")
> +
>  # check if IPA is configured at all
>  try:
>  check_IPA_configuration()
> diff --git a/ipaserver/install/server/install.py 
> b/ipaserver/install/server/install.py
> index 
> 930cca7b31ca06c04ab92deff49b6a4f198c2b6e..0c0683733ef38444a82d085f771596a9b066ef1d
>  100644
> --- a/ipaserver/install/server/install.py
> +++ b/ipaserver/install/server/install.py
> @@ -319,6 +319,11 @@ def install_check(installer):
>  external_ca_file = installer._external_ca_file
>  http_ca_cert = installer._ca_cert
>  
> +if os.path.exists('/proc/sys/crypto/fips_enabled'):
> +with open('/proc/sys/crypto/fips_enabled', 'r') as f:
> +if f.read().strip() != '0':
> +sys.exit("Cannot install IPA server in FIPS mode")
> +
>  tasks.check_selinux_status()
>  
>  if options.master_password:
> diff --git a/ipaserver/install/server/replicainstall.py 
> b/ipaserver/install/server/replicainstall.py
> index 
> 52b2ea5b0691cd99c6cb566af5a15af3b2dffb14..a2946339c7aeee8529f6ecf8ec4d85c9291fd291
>  100644
> --- a/ipaserver/install/server/replicainstall.py
> +++ b/ipaserver/install/server/replicainstall.py
> @@ -485,6 +485,11 @@ def install_check(installer):
>  options = installer
>  filename = installer.replica_file
>  
> +if os.path.exists('/proc/sys/crypto/fips_enabled'):
> +with open('/proc/sys/crypto/fips_enabled', 'r') as f:
> +if f.read().strip() != '0':
> +sys.exit("Cannot install IPA server in FIPS mode")
> +
>  tasks.check_selinux_status()
>  
>  if is_ipa_configured():
> -- 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] [PATCH] 0008 Do not allow installation in FIPS mode

[Florence Blanc-Renaud]

Hi,

this fix is a port of Bug 1131570 - Do not allow IdM 
server/replica/client installation in a FIPS-140 mode

It prevents installation of FreeIPA if the host is fips-enabled.

https://fedorahosted.org/freeipa/ticket/5761
>From afecbb3d228cf1d6cee59da53bf7a803f030d0b1 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud 
Date: Fri, 24 Jun 2016 16:16:22 +0200
Subject: [PATCH] Do not allow installation in FIPS mode

https://fedorahosted.org/freeipa/ticket/5761
---
 client/ipa-client-install  | 4 
 install/tools/ipactl   | 6 ++
 ipaserver/install/server/install.py| 5 +
 ipaserver/install/server/replicainstall.py | 5 +
 4 files changed, 20 insertions(+)

diff --git a/client/ipa-client-install b/client/ipa-client-install
index 0a601b63118b0a3568066495837121c65e5df04f..f80ff9c469709ea3b63902610b3b8b5c35448904 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -3064,6 +3064,10 @@ def main():
 
 if not os.getegid() == 0:
 sys.exit("\nYou must be root to run ipa-client-install.\n")
+if os.path.exists('/proc/sys/crypto/fips_enabled'):
+with open('/proc/sys/crypto/fips_enabled', 'r') as f:
+if f.read().strip() != '0':
+sys.exit("Cannot install IPA client in FIPS mode")
 tasks.check_selinux_status()
 logging_setup(options)
 root_logger.debug(
diff --git a/install/tools/ipactl b/install/tools/ipactl
index 547b21d875dff7231fae8dfc10faf995b0ca230b..9c68fffe73bfdd97789907226f8765c09707d552 100755
--- a/install/tools/ipactl
+++ b/install/tools/ipactl
@@ -545,6 +545,12 @@ def main():
 elif args[0] != "start" and args[0] != "stop" and args[0] != "restart" and args[0] != "status":
 raise IpactlError("Unrecognized action [" + args[0] + "]", 2)
 
+if (args[0] in ('start', 'restart') and
+os.path.exists('/proc/sys/crypto/fips_enabled')):
+with open('/proc/sys/crypto/fips_enabled', 'r') as f:
+if f.read().strip() != '0':
+raise IpactlError("Cannot start IPA server in FIPS mode")
+
 # check if IPA is configured at all
 try:
 check_IPA_configuration()
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index 930cca7b31ca06c04ab92deff49b6a4f198c2b6e..0c0683733ef38444a82d085f771596a9b066ef1d 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -319,6 +319,11 @@ def install_check(installer):
 external_ca_file = installer._external_ca_file
 http_ca_cert = installer._ca_cert
 
+if os.path.exists('/proc/sys/crypto/fips_enabled'):
+with open('/proc/sys/crypto/fips_enabled', 'r') as f:
+if f.read().strip() != '0':
+sys.exit("Cannot install IPA server in FIPS mode")
+
 tasks.check_selinux_status()
 
 if options.master_password:
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 52b2ea5b0691cd99c6cb566af5a15af3b2dffb14..a2946339c7aeee8529f6ecf8ec4d85c9291fd291 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -485,6 +485,11 @@ def install_check(installer):
 options = installer
 filename = installer.replica_file
 
+if os.path.exists('/proc/sys/crypto/fips_enabled'):
+with open('/proc/sys/crypto/fips_enabled', 'r') as f:
+if f.read().strip() != '0':
+sys.exit("Cannot install IPA server in FIPS mode")
+
 tasks.check_selinux_status()
 
 if is_ipa_configured():
-- 
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] [PATCH 0544] ipa-rmkeytab, ipa-join: dont fail if gettext cannot be initialized

[Martin Basti]

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

Patch attached.

From 37e296f12ed10edb12a3c34f62c0397e37fa3bda Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Fri, 24 Jun 2016 17:34:02 +0200
Subject: [PATCH] ipa-rmkeytab, ipa-join: don't fail if init of gettext failed

If locale setting was incorect, gettext failed to initialize and scripts
failed. this commit replaces error exit with warning message. (Better to
have untranslated output than fail)

https://fedorahosted.org/freeipa/ticket/5973
---
 client/ipa-join.c | 2 +-
 client/ipa-rmkeytab.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/client/ipa-join.c b/client/ipa-join.c
index ac8251fef3b0a03de0920e8732b44bac208f55a6..837e7ce398adef1cd3b9d9748ee091e4b8ee8f06 100644
--- a/client/ipa-join.c
+++ b/client/ipa-join.c
@@ -1130,7 +1130,7 @@ main(int argc, const char **argv) {
 
 ret = init_gettext();
 if (ret) {
-exit(2);
+fprintf(stderr, "Failed to load translations\n");
 }
 
 pc = poptGetContext("ipa-join", argc, (const char **)argv, options, 0);
diff --git a/client/ipa-rmkeytab.c b/client/ipa-rmkeytab.c
index 3687b1dc7ea0ab4484af3385bb87c5b9155e53da..e3d9eedc4f8b0d27f645be0624ef29bba5cf0605 100644
--- a/client/ipa-rmkeytab.c
+++ b/client/ipa-rmkeytab.c
@@ -180,7 +180,7 @@ main(int argc, const char **argv)
 
 ret = init_gettext();
 if (ret) {
-exit(1);
+fprintf(stderr, "Failed to load translations\n");
 }
 
 memset(, 0, sizeof(ktid));
-- 
2.5.5

-- 
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] DNS Locations: fix an issue found by coverity

[Martin Basti]

Shame, shame, shame on me. I forgot how to python when I was writing 
that originally.



Patch attached.

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

From fc0ba11ce71c5585874745764dbdcda4fa615e8c Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Mon, 27 Jun 2016 08:23:59 +0200
Subject: [PATCH] DNS Locations: server-mod: fix if statement

Statement used for detection if objeclass change is needed was logically
wrong, this fixes it.

https://fedorahosted.org/freeipa/ticket/2008
---
 ipaserver/plugins/server.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipaserver/plugins/server.py b/ipaserver/plugins/server.py
index b93f72c72447a235087253741998b2edfd4780c8..c1446ae9731b2cc1eced65348442251a321d71e9 100644
--- a/ipaserver/plugins/server.py
+++ b/ipaserver/plugins/server.py
@@ -226,7 +226,7 @@ class server_mod(LDAPUpdate):
 self.api.Object.location.handle_not_found(
 options['ipalocation_location'])
 
-if 'ipalocation' or 'ipaserviceweight' in entry_attrs:
+if 'ipalocation' in entry_attrs or 'ipaserviceweight' in entry_attrs:
 server_entry = ldap.get_entry(dn, ['objectclass'])
 
 # we need to extend object with ipaLocationMember objectclass
-- 
2.5.5

-- 
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