Re: [Freeipa-devel] [PATCHES 681-682] cert: speed up cert-find, do not crash on invalid data in cert-find

[Martin Basti]

On 12.08.2016 08:29, Jan Cholasta wrote:

On 11.8.2016 19:43, Martin Basti wrote:



On 11.08.2016 16:09, Jan Cholasta wrote:

On 11.8.2016 14:27, Martin Basti wrote:



On 01.08.2016 10:27, Jan Cholasta wrote:

On 1.8.2016 10:19, Jan Cholasta wrote:

Hi,

the attached patches fix

and .


Self-NACK, proper patches attached.

Honza





IMHO this is caused by your patches, test_cert_plugin.py:


Fixed.

Updated and rebased patches attached.


Hello,

It works for me, but:

1)
Is this py2/3 compatible?
ra_obj = ra.get_certificate(str(serial_number))


I don't see why not. Do you have any particular incompatibility in mind?


Because there is function str() used, where result is unicode in py3 but 
not in py2






2)
Are you sure you need tuple() here?
+for key in tuple(six.iterkeys(result)):


Yes, I'm modifying `result` inside the loop.

I don't need the six.iterkeys() though.

sorry, I overlooked that.





3)
  if cert is not None:
filter = ldap.make_filter_from_attr('usercertificate', 
value)

filters.append(filter)

Variable "value" may be referenced before assignment


Right, it should be `cert`, not `value`.



I haven't tested performace improvements yet, and it is quite big change
so I will continue with testing tomorrow.

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] [PATCHES 681-682] cert: speed up cert-find, do not crash on invalid data in cert-find

[Jan Cholasta]

On 11.8.2016 19:43, Martin Basti wrote:



On 11.08.2016 16:09, Jan Cholasta wrote:

On 11.8.2016 14:27, Martin Basti wrote:



On 01.08.2016 10:27, Jan Cholasta wrote:

On 1.8.2016 10:19, Jan Cholasta wrote:

Hi,

the attached patches fix

and .


Self-NACK, proper patches attached.

Honza





IMHO this is caused by your patches, test_cert_plugin.py:


Fixed.

Updated and rebased patches attached.


Hello,

It works for me, but:

1)
Is this py2/3 compatible?
ra_obj = ra.get_certificate(str(serial_number))


I don't see why not. Do you have any particular incompatibility in mind?



2)
Are you sure you need tuple() here?
+for key in tuple(six.iterkeys(result)):


Yes, I'm modifying `result` inside the loop.

I don't need the six.iterkeys() though.



3)
  if cert is not None:
filter = ldap.make_filter_from_attr('usercertificate', value)
filters.append(filter)

Variable "value" may be referenced before assignment


Right, it should be `cert`, not `value`.



I haven't tested performace improvements yet, and it is quite big change
so I will continue with testing tomorrow.

Martin^2



--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH] 0001 Added new authentication method

[Petr Vobornik]

On 08/11/2016 07:21 PM, Martin Basti wrote:
> 
> 
> On 11.08.2016 18:57, Pavel Vomacka wrote:
>>
>>
>> On 08/11/2016 02:00 PM, Petr Vobornik wrote:
>>> On 08/11/2016 10:54 AM, Alexander Bokovoy wrote:
 On Thu, 11 Aug 2016, Jan Cholasta wrote:
> On 4.8.2016 17:27, Jan Pazdziora wrote:
>> On Wed, Aug 03, 2016 at 10:29:52AM +0300, Alexander Bokovoy wrote:
>>> Got it. One thing I would correct, though, -- don't use
>>> kadmin.local, we
>>> do support setting ok_as_delegate on the service principals via IPA
>>> CLI:
>>> $ ipa service-mod --help |grep -A1 ok-as-delegate
>>> --ok-as-delegate=BOOL
>>>Client credentials may be delegated to the
>>> service
>> I've tried
>>
>>  ipa service-mod --ok-as-delegate=True HTTP/$(hostname)
>>
>> but that does not seem to have the same effect as
>>
>>  modprinc +ok_to_auth_as_delegate HTTP/ipa.example.test
>>
>> -- obtaining the delegated certificated fails.
> That's because ok_as_delegate and ok_to_auth_as_delegate are different
> flags.
 Right. The following patch adds ok_to_auth_as_delegate to the service
 principal.

 I haven't added any tickets to it yet.


>>> This might deserve also nice Web UI checkbox similar to "Trusted for
>>> delegation". CCing Pavel.
>>>
>> Here is patch with new checkbox. It is without ticket in commit message so 
>> once we will have the ticket I will send another patch witch updated commit 
>> message.
> 
> https://fedorahosted.org/freeipa/newticket
> 
> ;-)

It's prerequisite for https://fedorahosted.org/freeipa/ticket/5764 so we
might use 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] [PATCHES 681-682] cert: speed up cert-find, do not crash on invalid data in cert-find

[Martin Basti]

On 11.08.2016 16:09, Jan Cholasta wrote:

On 11.8.2016 14:27, Martin Basti wrote:



On 01.08.2016 10:27, Jan Cholasta wrote:

On 1.8.2016 10:19, Jan Cholasta wrote:

Hi,

the attached patches fix 


and .


Self-NACK, proper patches attached.

Honza





IMHO this is caused by your patches, test_cert_plugin.py:


Fixed.

Updated and rebased patches attached.


Hello,

It works for me, but:

1)
Is this py2/3 compatible?
ra_obj = ra.get_certificate(str(serial_number))

2)
Are you sure you need tuple() here?
+for key in tuple(six.iterkeys(result)):

3)
  if cert is not None:
filter = ldap.make_filter_from_attr('usercertificate', value)
filters.append(filter)

Variable "value" may be referenced before assignment

I haven't tested performace improvements yet, and it is quite big change 
so I will continue with testing tomorrow.


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] 0001 Added new authentication method

[Martin Basti]

On 11.08.2016 18:57, Pavel Vomacka wrote:



On 08/11/2016 02:00 PM, Petr Vobornik wrote:

On 08/11/2016 10:54 AM, Alexander Bokovoy wrote:

On Thu, 11 Aug 2016, Jan Cholasta wrote:

On 4.8.2016 17:27, Jan Pazdziora wrote:

On Wed, Aug 03, 2016 at 10:29:52AM +0300, Alexander Bokovoy wrote:

Got it. One thing I would correct, though, -- don't use
kadmin.local, we
do support setting ok_as_delegate on the service principals via IPA
CLI:
$ ipa service-mod --help |grep -A1 ok-as-delegate
--ok-as-delegate=BOOL
   Client credentials may be delegated to the
service

I've tried

 ipa service-mod --ok-as-delegate=True HTTP/$(hostname)

but that does not seem to have the same effect as

 modprinc +ok_to_auth_as_delegate HTTP/ipa.example.test

-- obtaining the delegated certificated fails.

That's because ok_as_delegate and ok_to_auth_as_delegate are different
flags.

Right. The following patch adds ok_to_auth_as_delegate to the service
principal.

I haven't added any tickets to it yet.



This might deserve also nice Web UI checkbox similar to "Trusted for
delegation". CCing Pavel.

Here is patch with new checkbox. It is without ticket in commit 
message so once we will have the ticket I will send another patch 
witch updated commit message.


https://fedorahosted.org/freeipa/newticket

;-)







-- 
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] 0001 Added new authentication method

[Pavel Vomacka]

On 08/11/2016 02:00 PM, Petr Vobornik wrote:

On 08/11/2016 10:54 AM, Alexander Bokovoy wrote:

On Thu, 11 Aug 2016, Jan Cholasta wrote:

On 4.8.2016 17:27, Jan Pazdziora wrote:

On Wed, Aug 03, 2016 at 10:29:52AM +0300, Alexander Bokovoy wrote:

Got it. One thing I would correct, though, -- don't use
kadmin.local, we
do support setting ok_as_delegate on the service principals via IPA
CLI:
$ ipa service-mod --help |grep -A1 ok-as-delegate
--ok-as-delegate=BOOL
   Client credentials may be delegated to the
service

I've tried

 ipa service-mod --ok-as-delegate=True HTTP/$(hostname)

but that does not seem to have the same effect as

 modprinc +ok_to_auth_as_delegate HTTP/ipa.example.test

-- obtaining the delegated certificated fails.

That's because ok_as_delegate and ok_to_auth_as_delegate are different
flags.

Right. The following patch adds ok_to_auth_as_delegate to the service
principal.

I haven't added any tickets to it yet.



This might deserve also nice Web UI checkbox similar to "Trusted for
delegation". CCing Pavel.

Here is patch with new checkbox. It is without ticket in commit message 
so once we will have the ticket I will send another patch witch updated 
commit message.


--
Pavel^3 Vomacka

From 6cb9d1152789c2d015b3a85ded622980241a2137 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Thu, 11 Aug 2016 18:53:55 +0200
Subject: [PATCH] Add 'trusted to auth as user' checkbox

---
 install/ui/src/freeipa/service.js | 5 +
 1 file changed, 5 insertions(+)

diff --git a/install/ui/src/freeipa/service.js b/install/ui/src/freeipa/service.js
index 35d486605ebfee41d8b3ffa5bb77bf9e72a60c01..30e336c35b8eece2e5e3ef55629d0c98f097fbf5 100644
--- a/install/ui/src/freeipa/service.js
+++ b/install/ui/src/freeipa/service.js
@@ -142,6 +142,11 @@ return {
 acl_param: 'krbticketflags'
 },
 {
+name: 'ipakrboktoauthasdelegate',
+$type: 'checkbox',
+acl_param: 'krbticketflags'
+},
+{
 name: 'ipakrbrequirespreauth',
 $type: 'checkbox',
 acl_param: 'krbticketflags'
-- 
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 0156] server upgrade: do not start BIND if it was not running before the upgrad

[Martin Basti]

On 11.08.2016 15:10, Petr Spacek wrote:

Hello,

server upgrade: do not start BIND if it was not running before the upgrade

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




ACK
-- 
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 0155] DNS server upgrade: do not fail when DNS server did not respond

[Martin Basti]

On 11.08.2016 15:18, Petr Spacek wrote:

On 11.8.2016 15:08, Petr Spacek wrote:

Hello,

DNS server upgrade: do not fail when DNS server did not respond

Previously, update_dnsforward_emptyzones failed with an exeception if
DNS query failed for some reason. Now the error is logged and upgrade
continues.

I assume that this is okay because the DNS query is used as heuristics
of last resort in the upgrade logic and failure to do so should not have
catastrophics consequences: In the worst case, the admin needs to
manually change forwarding policy from 'first' to 'only'.

In the end I have decided not to auto-start BIND because BIND depends on
GSSAPI for authentication, which in turn depends on KDC ... Alternative
like reconfiguring BIND to use LDAPI+EXTERNAL and reconfiguring DS to
accept LDAP external bind from named user are too complicated.

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

Here is variant for master branch. Enjoy.


ACK

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

Re: [Freeipa-devel] [PATCH] 0024 memory leak in ipapwd plugin

[Alexander Bokovoy]

On Thu, 11 Aug 2016, thierry bordaz wrote:
+/* rc should always be 0 (else slapi_sdn_new_dn_byref 
should have sigsev)
+ * but if we end in rc==LDAP_OPERATIONS_ERROR be sure to 
stop here

+ * because ret is not significant */

A short note here. You talk about slapi_sdn_new_dn_byref() but your
patch replaces that with slapi_sdn_new_dn_byval(). Does the comment
still apply?


+if (rc != 0) {
+LOG_OOM();
+goto free_and_return;
+}
+
   if (ret == 0) {
   Slapi_Value *cpw[2] = { NULL, NULL };
   Slapi_Value *pw;
--
2.7.4





Good catch Alexander. Yes the comment contained a wrong cut/paste

ACK.


--
/ Alexander Bokovoy

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

Re: [Freeipa-devel] [PATCH 0035] Remove Custodia server keys from LDAP

[Martin Basti]

On 08.08.2016 16:10, Christian Heimes wrote:

The server-del plugin now removes the Custodia keys for encryption and
key signing from LDAP.

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



ACK for master

For 4.3, it requires new patch

Martin
-- 
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] 0102-0104: webui: Add support for setting custom table pagination size

[Pavel Vomacka]

Hello,

please review attached patches.

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

--
Pavel^3 Vomacka

From 0b4898eb7eed06c4e50f6b5606736d0023a58de5 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Thu, 11 Aug 2016 15:51:33 +0200
Subject: [PATCH 1/3] Add javascript integer validator

Javascript integer validator checks whether value entered into field is number
and is not higher than Number.MAX_SAFE_INTEGER constant.

Part of: https://fedorahosted.org/freeipa/ticket/5742
---
 install/ui/src/freeipa/field.js | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/install/ui/src/freeipa/field.js b/install/ui/src/freeipa/field.js
index d8b957f5ab28b5ee4bc4ebce2ae6f454083bc4fd..59525a46bb9d1106e89f364165d567824f37d4c5 100644
--- a/install/ui/src/freeipa/field.js
+++ b/install/ui/src/freeipa/field.js
@@ -962,6 +962,39 @@ field.validator = IPA.validator = function(spec) {
 };
 
 /**
+ * Javascript integer validator
+ *
+ * It allows to insert only integer numbers which can be safely represented by
+ * Javascript.
+ *
+ * @class
+ * @alternateClassName IPA.metadata_validator
+ * @extends IPA.validator
+ */
+ field.integer_validator = IPA.integer_validator = function(spec) {
+
+ var that = IPA.validator(spec);
+ 
+ /**
+  * @inheritDoc
+  */
+ that.validate = function(value) {
+
+ if (!value.match(/^-?\d+$/)) {
+ return that.false_result(text.get('@i18n:widget.validation.integer'));
+ }
+
+ if (!Number.isSafeInteger(parseInt(value, 10))) {
+ return that.false_result(text.get('@i18n:widget.validation.unsupported'));
+ }
+
+ return that.true_result();
+ };
+
+ return that;
+ };
+
+/**
  * Metadata validator
  *
  * Validates value according to supplied metadata
@@ -1574,6 +1607,7 @@ field.register = function() {
 v.register('metadata', field.metadata_validator);
 v.register('unsupported', field.unsupported_validator);
 v.register('same_password', field.same_password_validator);
+v.register('integer', field.integer_validator);
 
 l.register('adapter', field.Adapter);
 l.register('object_adapter', field.ObjectAdapter);
-- 
2.5.5

From ee2a7b415712db3a72d4478e42a36782d03e0ee5 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Thu, 11 Aug 2016 15:56:01 +0200
Subject: [PATCH 2/3] Make singleton from config module

Also added general setter and getter for attributes of config.

Part of: https://fedorahosted.org/freeipa/ticket/5742
---
 install/ui/src/freeipa/config.js | 51 +++-
 1 file changed, 45 insertions(+), 6 deletions(-)

diff --git a/install/ui/src/freeipa/config.js b/install/ui/src/freeipa/config.js
index 61922d454583c393ad68a11fcd571997b6bfab60..3bf017bdc0cab580783f7903261ebbd9a29fdc87 100644
--- a/install/ui/src/freeipa/config.js
+++ b/install/ui/src/freeipa/config.js
@@ -20,14 +20,18 @@
 
 
 
-define([], function() {
+define([
+'dojo/_base/declare',
+'dojo/topic'
+],
+function(declare, topic) {
 
 /**
  * Application configuration
  * @class config
  * @singleton
  */
-var config = {
+var config = declare([], {
 
 /**
  * Selector for application container node
@@ -82,8 +86,43 @@ define([], function() {
  * Hide sections without any visible widget
  * @property {boolean}
  */
-hide_empty_sections: true
-};
+hide_empty_sections: true,
 
-return config;
-});
\ No newline at end of file
+/**
+ * Number of lines in table on table_facets
+ * @property {Integer}
+ */
+table_page_size: 20,
+
+/**
+ * Genereal setter for config values.
+ * @param item_name {string}
+ * @param value
+ * @param store {Boolean} sets whether the value will be stored into
+ *  local storage
+ */
+set: function(item_name, value, store) {
+if (!item_name) return;
+this[item_name] = value;
+
+if (store) {
+window.localStorage.setItem(item_name, value);
+}
+},
+
+/**
+ * Genereal setter for config values.
+ * @param item_name {string}
+ */
+get: function(item_name) {
+return this[item_name];
+},
+
+constructor: function() {
+var user_limit = window.localStorage.getItem('table_page_size');
+if (user_limit) this.table_page_size = user_limit;
+}
+});
+
+return new config();
+});
-- 
2.5.5

From 9c8c20fb0898156d61f5911710a86aa796ce3c67 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Thu, 11 Aug 2016 15:58:23 +0200
Subject: [PATCH 3/3] Add support for custom table pagination size

New customization button opens dialog with field for setting the number of lines
in tables. After saving the new value there is new topic which starts refreshing
current tabl

Re: [Freeipa-devel] [PATCHES 681-682] cert: speed up cert-find, do not crash on invalid data in cert-find

[Jan Cholasta]

On 11.8.2016 14:27, Martin Basti wrote:



On 01.08.2016 10:27, Jan Cholasta wrote:

On 1.8.2016 10:19, Jan Cholasta wrote:

Hi,

the attached patches fix 
and .


Self-NACK, proper patches attached.

Honza





IMHO this is caused by your patches, test_cert_plugin.py:


Fixed.

Updated and rebased patches attached.

--
Jan Cholasta
From 46a8a0bb15d1bf5a69a6b9340e67a47129a85b59 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Mon, 1 Aug 2016 09:53:39 +0200
Subject: [PATCH] cert: speed up cert-find

Use issuer+serial rather than raw DER blob to identify certificates in
cert-find's intermediate result.

Restructure the code to make it (hopefully) easier to follow.

https://fedorahosted.org/freeipa/ticket/6098
---
 ipaserver/plugins/cert.py | 398 +-
 1 file changed, 216 insertions(+), 182 deletions(-)

diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 06041d3..8f10068 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -21,6 +21,7 @@
 
 import base64
 import binascii
+import collections
 import datetime
 import os
 
@@ -295,18 +296,24 @@ class BaseCertObject(Object):
 ),
 )
 
-def _parse(self, obj):
-cert = x509.load_certificate(obj['certificate'])
-obj['subject'] = DN(unicode(cert.subject))
-obj['issuer'] = DN(unicode(cert.issuer))
-obj['valid_not_before'] = unicode(cert.valid_not_before_str)
-obj['valid_not_after'] = unicode(cert.valid_not_after_str)
-obj['md5_fingerprint'] = unicode(
-nss.data_to_hex(nss.md5_digest(cert.der_data), 64)[0])
-obj['sha1_fingerprint'] = unicode(
-nss.data_to_hex(nss.sha1_digest(cert.der_data), 64)[0])
-obj['serial_number'] = cert.serial_number
-obj['serial_number_hex'] = u'0x%X' % cert.serial_number
+def _parse(self, obj, full=True):
+cert = obj.get('certificate')
+if cert is not None:
+cert = x509.load_certificate(cert)
+obj['subject'] = DN(unicode(cert.subject))
+obj['issuer'] = DN(unicode(cert.issuer))
+obj['serial_number'] = cert.serial_number
+if full:
+obj['valid_not_before'] = unicode(cert.valid_not_before_str)
+obj['valid_not_after'] = unicode(cert.valid_not_after_str)
+obj['md5_fingerprint'] = unicode(
+nss.data_to_hex(nss.md5_digest(cert.der_data), 64)[0])
+obj['sha1_fingerprint'] = unicode(
+nss.data_to_hex(nss.sha1_digest(cert.der_data), 64)[0])
+
+serial_number = obj.get('serial_number')
+if serial_number is not None:
+obj['serial_number_hex'] = u'0x%X' % serial_number
 
 
 class BaseCertMethod(Method):
@@ -691,10 +698,14 @@ class cert(BaseCertObject):
 yield self.api.Object[name]
 
 def _fill_owners(self, obj):
+dns = obj.pop('owner', None)
+if dns is None:
+return
+
 for owner in self._owners():
 container_dn = DN(owner.container_dn, self.api.env.basedn)
 name = 'owner_' + owner.name
-for dn in obj['owner']:
+for dn in dns:
 if dn.endswith(container_dn, 1):
 value = owner.get_primary_key_from_dn(dn)
 obj.setdefault(name, []).append(value)
@@ -776,9 +787,7 @@ class cert_show(Retrieve, CertMethod, VirtualCommand):
 result['certificate'] = result['certificate'].replace('\r\n', '')
 self.obj._parse(result)
 result['revoked'] = ('revocation_reason' in result)
-if 'owner' in result:
-self.obj._fill_owners(result)
-del result['owner']
+self.obj._fill_owners(result)
 
 if hostname:
 # If we have a hostname we want to verify that the subject
@@ -984,36 +993,171 @@ class cert_find(Search, CertMethod):
 label=owner.object_name,
 )
 
-def execute(self, criteria=None, all=False, raw=False, pkey_only=False,
-no_members=True, timelimit=None, sizelimit=None, **options):
-ca_options = {'cacn',
-  'revocation_reason',
-  'issuer',
-  'subject',
-  'min_serial_number', 'max_serial_number',
-  'exactly',
-  'validnotafter_from', 'validnotafter_to',
-  'validnotbefore_from', 'validnotbefore_to',
-  'issuedon_from', 'issuedon_to',
-  'revokedon_from', 'revokedon_to'}
-ldap_options = {prefix + owner.name
-for owner in self.obj._owners()
-for prefix in ('', 'no_')}
-has_ca_options = (
-any(name in options for name in ca

Re: [Freeipa-devel] [PATCH] 0024 memory leak in ipapwd plugin

[thierry bordaz]

On 08/10/2016 07:19 PM, Alexander Bokovoy wrote:

On Wed, 10 Aug 2016, thierry bordaz wrote:



On 08/10/2016 11:24 AM, Alexander Bokovoy wrote:

On Wed, 10 Aug 2016, thierry bordaz wrote:




From 13bb55f9d97f82062f5b496d4164acb562afc7a0 Mon Sep 17 00:00:00 
2001

From: Thierry Bordaz 
Date: Tue, 9 Aug 2016 16:46:25 +0200
Subject: [PATCH] ipa-pwd-extop memory leak during passord update

During an extend op password update, there is a test if the
user is changing the password is himself. It uses local Slapi_SDN
variable that are not freed
---
daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git 
a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c 
b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c

index 6a87a27..2eda6c6 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -398,16 +398,20 @@ parse_req_done:
   /* if the user changing the password is self, we must 
request the

* old password and verify it matches the current one before
* proceeding with the password change */
-bind_sdn = slapi_sdn_new_dn_byref(bindDN);
-target_sdn = slapi_sdn_new_dn_byref(dn);
+bind_sdn = slapi_sdn_new_dn_byval(bindDN);
+target_sdn = slapi_sdn_new_dn_byval(dn);
   if (!bind_sdn || !target_sdn) {
   LOG_OOM();
+slapi_sdn_free(&bind_sdn);
+slapi_sdn_free(&target_sdn);
   rc = LDAP_OPERATIONS_ERROR;
   goto free_and_return;
   }
   /* this one will normalize and compare, so difference in 
case will be

* correctly handled */
   ret = slapi_sdn_compare(bind_sdn, target_sdn);
+slapi_sdn_free(&bind_sdn);
+slapi_sdn_free(&target_sdn);
   if (ret == 0) {
   Slapi_Value *cpw[2] = { NULL, NULL };
   Slapi_Value *pw;

I would prefer to unify memory freeing. Because slapi_sdn_compare() can
be run with NULL arguments (it will return 0), and slapi_sdn_free() is
no-op for NULL argument, you can actually do comparison, then free the
SDNs and then do error handling:


bind_sdn = slapi_sdn_new_dn_byval(bindDN);
target_sdn = slapi_sdn_new_dn_byval(dn);

rc = (!bind_sdn || !target_sdn) ? LDAP_OPERATIONS_ERROR : 0;
ret = slapi_sdn_compare(bind_sdn, target_sdn);

slapi_sdn_free(&bind_sdn);
slapi_sdn_free(&target_sdn);

if (rc != 0) {
   LOG_OOM();
   goto free_and_return;
}

if (ret == 0) {
 
}




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




Thanks again Alexander for the review. Here is the revisited patch



From db4211d855b4d21354dc619952b2b2e1ad31f3b9 Mon Sep 17 00:00:00 2001
From: Thierry Bordaz 
Date: Tue, 9 Aug 2016 16:46:25 +0200
Subject: [PATCH] ipa-pwd-extop memory leak during passord update

During an extend op password update, there is a test if the
user is changing the password is himself. It uses local Slapi_SDN
variable that are not freed
---
.../ipa-pwd-extop/ipa_pwd_extop.c  | 24 
+++---

1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c 
b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c

index 6a87a27..eaca0dc 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -398,16 +398,26 @@ parse_req_done:
/* if the user changing the password is self, we must request 
the

 * old password and verify it matches the current one before
 * proceeding with the password change */
-bind_sdn = slapi_sdn_new_dn_byref(bindDN);
-target_sdn = slapi_sdn_new_dn_byref(dn);
-if (!bind_sdn || !target_sdn) {
-LOG_OOM();
-rc = LDAP_OPERATIONS_ERROR;
-goto free_and_return;
-}
+bind_sdn = slapi_sdn_new_dn_byval(bindDN);
+target_sdn = slapi_sdn_new_dn_byval(dn);
+
+rc = (!bind_sdn || !target_sdn) ? LDAP_OPERATIONS_ERROR : 0;
+
/* this one will normalize and compare, so difference in case 
will be

 * correctly handled */
ret = slapi_sdn_compare(bind_sdn, target_sdn);
+
+slapi_sdn_free(&bind_sdn);
+slapi_sdn_free(&target_sdn);
+
+/* rc should always be 0 (else slapi_sdn_new_dn_byref should 
have sigsev)
+ * but if we end in rc==LDAP_OPERATIONS_ERROR be sure to 
stop here

+ * because ret is not significant */

A short note here. You talk about slapi_sdn_new_dn_byref() but your
patch replaces that with slapi_sdn_new_dn_byval(). Does the comment
still apply?


+if (rc != 0) {
+LOG_OOM();
+goto free_and_return;
+}
+
if (ret == 0) {
Slapi_Value *cpw[2] = {

Re: [Freeipa-devel] [PATCH] 0001: Silence sshd messages during install

[Martin Basti]

On 11.08.2016 15:40, Jan Cholasta wrote:

On 8.8.2016 14:25, Martin Basti wrote:



On 08.08.2016 13:58, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Jan Cholasta wrote:

On 19.7.2016 08:40, Jan Cholasta wrote:

Hi,

On 9.7.2016 14:46, Ben Lipton wrote:

On 07/07/2016 11:19 AM, Ben Lipton wrote:


Thanks for the review! Comments below.


On 07/01/2016 07:42 AM, Martin Basti wrote:




On 29.06.2016 20:46, Ben Lipton wrote:

The attached patch silences some annoying messages I've been
getting
when upgrading the freeipa-client package on F24:
"""
WARNING: 'UseLogin yes' is not supported in Fedora and may cause
several problems.

This will be fixed by openssh-7.2p2-9.fc24
(https://bugzilla.redhat.com/show_bug.cgi?id=1350347) so we 
probably

shouldn't worry about it.

Could not load host key: /etc/ssh/ssh_host_dsa_key

This is because by default sshd looks for all of
/etc/ssh/ssh_host_dsa_key, /etc/ssh/ssh_host_ecdsa_key,
/etc/ssh/ssh_host_ed25519_key and /etc/ssh/ssh_host_rsa_key, but
Fedora doesn't generate a DSA key by default.

"""

Since the script causing the message only looks at the return 
code
from sshd to determine the right options to use, I thought it 
might

be ok to discard the output. What do you think?

Ben




Hello, I don't like to hiding errors/warnings. Can you 
determine and

solve the root cause?


I definitely agree with this in principle, but in this case the
purpose of this code is to try different, potentially wrong,
parameters to sshd until it finds a combination that it accepts. It
seems like in some environments this would produce error messages
that
aren't actionable and don't indicate any problem for package
function,
which is why I didn't think these messages were necessarily worth
preserving.

On the other hand, if the code makes the wrong decision about sshd
version we might be interested in error logs that show why. Can we
log
this to a file instead of the console, maybe?

If you'd prefer just addressing the root cause, a patch that 
prevents

the missing host key error is attached, but it won't stop the error
messages showing up when openssh is an older version.

Thanks,
Ben



Whoops, realized that my patch created a tempfile and didn't delete
it.
Updated.


I think the first version of the patch was OK. sshd is called only to
check which set of authorized keys options to use, we don't really 
care

about anything else, so we can safely ignore whatever it puts to
stderr.


Bump.

ACK on the first version of the patch
(freeipa-blipton-0001-Silence-sshd-messages-during-install.patch).

Anyone against pushing it?
Given that newer OpenSSH version will silence it anyway, I'm OK with 
the

interim fix.

Pushed to master: c15ba1f9e8c7d236586d46271fce7c3950b509da


You pushed the wrong patch (0002).



Yes, sorry, I forgot how to numbers

Fixed patch attached.
From 2cd5037ee89bcb3ba3007c9c20ba3458d628eef0 Mon Sep 17 00:00:00 2001
From: Ben Lipton 
Date: Thu, 11 Aug 2016 15:39:35 +0200
Subject: [PATCH] Silence sshd messages during install

Fix for accidentally pushed commit c15ba1f9e8c7d236586d46271fce7c3950b509da

During install we call sshd with no config file, sometimes leading to it
complaining about missing files or bad config options. Since we're just
looking for the return code to see if the options are correct, we can
discard these error messages.
---
 freeipa.spec.in | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 78ab8ca5800eceba633fd9d1e3412ee3bde94c0e..ea580a20ac3fea42916271e7d9e906c0d67450e3 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -1009,21 +1009,17 @@ if [ -f '/etc/ssh/sshd_config' -a $restore -ge 2 ]; then
 /^(AuthorizedKeysCommand(User|RunAs)|PubKeyAgentRunAs)[ \t]/ d
 ' /etc/ssh/sshd_config >/etc/ssh/sshd_config.ipanew
 
-# Prevent complaints about missing host keys by using the configured ones
-tmp_config=$(mktemp sshd_config.XX)
-sed -n '/^HostKey[ \t]/ p' /etc/ssh/sshd_config > $tmp_config
-
-if /usr/sbin/sshd -t -f $tmp_config -o 'AuthorizedKeysCommand=/usr/bin/sss_ssh_authorizedkeys' -o 'AuthorizedKeysCommandUser=nobody'; then
+if /usr/sbin/sshd -t -f /dev/null -o 'AuthorizedKeysCommand=/usr/bin/sss_ssh_authorizedkeys' -o 'AuthorizedKeysCommandUser=nobody' 2>/dev/null; then
 sed -ri '
 s/^PubKeyAgent (.+) %u$/AuthorizedKeysCommand \1/
 s/^AuthorizedKeysCommand .*$/\0\nAuthorizedKeysCommandUser nobody/
 ' /etc/ssh/sshd_config.ipanew
-elif /usr/sbin/sshd -t -f $tmp_config -o 'AuthorizedKeysCommand=/usr/bin/sss_ssh_authorizedkeys' -o 'AuthorizedKeysCommandRunAs=nobody'; then
+elif /usr/sbin/sshd -t -f /dev/null -o 'AuthorizedKeysCommand=/usr/bin/sss_ssh_authorizedkeys' -o 'AuthorizedKeysCommandRunAs=nobody' 2>/dev/null; then
 sed -ri '
 s/^PubKeyAgent (.+) %u$/AuthorizedKeysCommand \1/
 s/^Authoriz

Re: [Freeipa-devel] [PATCH 0057] Don't show part of warning containing --force-ntpd in replica install

[Martin Basti]

On 09.08.2016 15:27, Stanislav Laznicka wrote:

On 08/04/2016 07:34 AM, Jan Cholasta wrote:

On 3.8.2016 19:39, Martin Basti wrote:



On 03.08.2016 18:10, Petr Vobornik wrote:

On 07/13/2016 12:36 PM, Stanislav Laznicka wrote:

On 07/13/2016 09:51 AM, Petr Vobornik wrote:

On 07/13/2016 08:26 AM, Stanislav Laznicka wrote:

On 07/12/2016 08:44 AM, Stanislav Laznicka wrote:

On 07/11/2016 04:27 PM, Petr Vobornik wrote:

On 07/11/2016 01:23 PM, Stanislav Laznicka wrote:

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




Isn't the bug about something else?

The issue was that ipa-replica-install doesn't have --force-ntpd
option.
It is an option of ipa-client-install which is run from replica
installer.

The unattended mode is unrelated.

My understanding is that the bug says that '--force-ntpd' option
should not be shown when ipa-client-install is run during replica
installation.

During replica installation, the ipa-client-install script is run
with
the '--unattended' flag in the 'ensure_enrolled()' function. 
Being a

separate script, there's not many options on how to pass the
information not to show the message to ipa-client-install. 
Using the

already used flag to get rid of the message seemed easiest to me.
Introducing a new 'hidden' flag (like '--from-replica'), on the 
other

hand, seems a bit harsh.


Just to throw it out there - it's possible that the '--force-join'
client option would also appear as a hint from the client install
script
(during replica installation). Should this also be muted somehow?
To me,
it seems reasonable to rather add it as an argument to
ipa-replica-install to pass it to the client install script.

IMO client installation initiated from replica needs to have a 
special

option(hidden in help) similar to --on-server (or what's its name).
E.g.
the name can be --replica-install. Maybe --on-server can be used 
but it
may have other implication which might not be valid for this use 
case.


Anything else are just workarounds. Imagine that admin runs
ipa-client-install with --unattended or --force-join. He would 
then not

get the message as now.

Reviving thread to get other opinion.

The --on-master option won't do here as it seems that the client 
would
require some IPA pre-configuration for successful install. A new 
option

will have to be created, then.

I'm for new "hidden" option.


I'm against any hidden options, this should be made correctly by
modularization/fixing of client install, to be able call it from python
not as external process


+1, but this is non-trivial and definitely not material for 4.4.1. 
For 4.4.1 the hidden option should be OK.




Just from top of my head, can we just use option --no-ntp with client
install in replica installer? Server NTP should not depend on client 
ntp

config.
I'm just afraid that we may get kerberos time issue during client
install if client time does not match server time.

Or second approach, always call client install from replica with
--force-ntpd, unless there is --no-ntp used for replica, then call
ipa-client-install with --no-ntp

But it needs investigation.


CCing David as he knows everything NTP-related.



Martin^2



As I was trying to point out, the situation about --force-join is 
a bit

different. The option again would be shown and is not available in
ipa-replica-install. I think it should be available to allow direct
replica installation even when previous installation failed/left some
mess on the master (ofc the user could run `ipa-replica-manage del
 --cleanup` on the master instead).


That could work but imho is out of scope of this ticket.





Please see the attached patch that always adds the --no-ntp option to 
ipa-client-install.



ACK

Pushed to master: 0745c5d0f96f572a3780b32a3f2dce4f3512c396

--
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] 0001: Silence sshd messages during install

[Jan Cholasta]

On 8.8.2016 14:25, Martin Basti wrote:



On 08.08.2016 13:58, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Jan Cholasta wrote:

On 19.7.2016 08:40, Jan Cholasta wrote:

Hi,

On 9.7.2016 14:46, Ben Lipton wrote:

On 07/07/2016 11:19 AM, Ben Lipton wrote:


Thanks for the review! Comments below.


On 07/01/2016 07:42 AM, Martin Basti wrote:




On 29.06.2016 20:46, Ben Lipton wrote:

The attached patch silences some annoying messages I've been
getting
when upgrading the freeipa-client package on F24:
"""
WARNING: 'UseLogin yes' is not supported in Fedora and may cause
several problems.

This will be fixed by openssh-7.2p2-9.fc24
(https://bugzilla.redhat.com/show_bug.cgi?id=1350347) so we probably
shouldn't worry about it.

Could not load host key: /etc/ssh/ssh_host_dsa_key

This is because by default sshd looks for all of
/etc/ssh/ssh_host_dsa_key, /etc/ssh/ssh_host_ecdsa_key,
/etc/ssh/ssh_host_ed25519_key and /etc/ssh/ssh_host_rsa_key, but
Fedora doesn't generate a DSA key by default.

"""

Since the script causing the message only looks at the return code
from sshd to determine the right options to use, I thought it might
be ok to discard the output. What do you think?

Ben




Hello, I don't like to hiding errors/warnings. Can you determine and
solve the root cause?


I definitely agree with this in principle, but in this case the
purpose of this code is to try different, potentially wrong,
parameters to sshd until it finds a combination that it accepts. It
seems like in some environments this would produce error messages
that
aren't actionable and don't indicate any problem for package
function,
which is why I didn't think these messages were necessarily worth
preserving.

On the other hand, if the code makes the wrong decision about sshd
version we might be interested in error logs that show why. Can we
log
this to a file instead of the console, maybe?

If you'd prefer just addressing the root cause, a patch that prevents
the missing host key error is attached, but it won't stop the error
messages showing up when openssh is an older version.

Thanks,
Ben



Whoops, realized that my patch created a tempfile and didn't delete
it.
Updated.


I think the first version of the patch was OK. sshd is called only to
check which set of authorized keys options to use, we don't really care
about anything else, so we can safely ignore whatever it puts to
stderr.


Bump.

ACK on the first version of the patch
(freeipa-blipton-0001-Silence-sshd-messages-during-install.patch).

Anyone against pushing it?

Given that newer OpenSSH version will silence it anyway, I'm OK with the
interim fix.

Pushed to master: c15ba1f9e8c7d236586d46271fce7c3950b509da


You pushed the wrong patch (0002).

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH 0155] DNS server upgrade: do not fail when DNS server did not respond

[Petr Spacek]

On 11.8.2016 15:08, Petr Spacek wrote:
> Hello,
> 
> DNS server upgrade: do not fail when DNS server did not respond
> 
> Previously, update_dnsforward_emptyzones failed with an exeception if
> DNS query failed for some reason. Now the error is logged and upgrade
> continues.
> 
> I assume that this is okay because the DNS query is used as heuristics
> of last resort in the upgrade logic and failure to do so should not have
> catastrophics consequences: In the worst case, the admin needs to
> manually change forwarding policy from 'first' to 'only'.
> 
> In the end I have decided not to auto-start BIND because BIND depends on
> GSSAPI for authentication, which in turn depends on KDC ... Alternative
> like reconfiguring BIND to use LDAPI+EXTERNAL and reconfiguring DS to
> accept LDAP external bind from named user are too complicated.
> 
> https://fedorahosted.org/freeipa/ticket/6205

Here is variant for master branch. Enjoy.

-- 
Petr^2 Spacek
From 4816abee9150db26b330fa4ce99b4fb8f51597a1 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Thu, 11 Aug 2016 13:44:29 +0200
Subject: [PATCH] DNS server upgrade: do not fail when DNS server did not
 respond

Previously, update_dnsforward_emptyzones failed with an exeception if
DNS query failed for some reason. Now the error is logged and upgrade
continues.

I assume that this is okay because the DNS query is used as heuristics
of last resort in the upgrade logic and failure to do so should not have
catastrophics consequences: In the worst case, the admin needs to
manually change forwarding policy from 'first' to 'only'.

In the end I have decided not to auto-start BIND because BIND depends on
GSSAPI for authentication, which in turn depends on KDC ... Alternative
like reconfiguring BIND to use LDAPI+EXTERNAL and reconfiguring DS to
accept LDAP external bind from named user are too complicated.

https://fedorahosted.org/freeipa/ticket/6205
---
 ipaserver/install/plugins/dns.py | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/ipaserver/install/plugins/dns.py b/ipaserver/install/plugins/dns.py
index 32247eedbac7fc7e00c7277ef0bc593a74cd22e4..7b06a5c0d3a59e5825af75fae87c9739a53d9913 100644
--- a/ipaserver/install/plugins/dns.py
+++ b/ipaserver/install/plugins/dns.py
@@ -17,6 +17,9 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see .
 
+from __future__ import absolute_import
+
+import dns.exception
 import re
 import traceback
 import time
@@ -489,8 +492,15 @@ class update_dnsforward_emptyzones(DNSUpdater):
 self.api.Command['dnsconfig_mod'](ipadnsversion=2)
 
 self.update_zones()
-if dnsutil.has_empty_zone_addresses(self.api.env.host):
-self.update_global_ldap_forwarder()
+try:
+if dnsutil.has_empty_zone_addresses(self.api.env.host):
+self.update_global_ldap_forwarder()
+except dns.exception.DNSException as ex:
+self.log.error('Skipping update of global DNS forwarder in LDAP: '
+   'Unable to determine if local server is using an '
+   'IP address belonging to an automatic empty zone. '
+   'Consider changing forwarding policy to "only". '
+   'DNS exception: %s', ex)
 
 return False, []
 
-- 
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 0156] server upgrade: do not start BIND if it was not running before the upgrad

[Petr Spacek]

On 11.8.2016 15:17, Petr Spacek wrote:
> On 11.8.2016 15:10, Petr Spacek wrote:
>> Hello,
>>
>> server upgrade: do not start BIND if it was not running before the upgrade
>>
>> https://fedorahosted.org/freeipa/ticket/6206
> 
> Here is variant for master branch.

Grr, this is a wrong thread. Please ignore this.

-- 
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 0156] server upgrade: do not start BIND if it was not running before the upgrad

[Petr Spacek]

On 11.8.2016 15:10, Petr Spacek wrote:
> Hello,
> 
> server upgrade: do not start BIND if it was not running before the upgrade
> 
> https://fedorahosted.org/freeipa/ticket/6206

Here is variant for master branch.

-- 
Petr^2 Spacek
From 4816abee9150db26b330fa4ce99b4fb8f51597a1 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Thu, 11 Aug 2016 13:44:29 +0200
Subject: [PATCH] DNS server upgrade: do not fail when DNS server did not
 respond

Previously, update_dnsforward_emptyzones failed with an exeception if
DNS query failed for some reason. Now the error is logged and upgrade
continues.

I assume that this is okay because the DNS query is used as heuristics
of last resort in the upgrade logic and failure to do so should not have
catastrophics consequences: In the worst case, the admin needs to
manually change forwarding policy from 'first' to 'only'.

In the end I have decided not to auto-start BIND because BIND depends on
GSSAPI for authentication, which in turn depends on KDC ... Alternative
like reconfiguring BIND to use LDAPI+EXTERNAL and reconfiguring DS to
accept LDAP external bind from named user are too complicated.

https://fedorahosted.org/freeipa/ticket/6205
---
 ipaserver/install/plugins/dns.py | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/ipaserver/install/plugins/dns.py b/ipaserver/install/plugins/dns.py
index 32247eedbac7fc7e00c7277ef0bc593a74cd22e4..7b06a5c0d3a59e5825af75fae87c9739a53d9913 100644
--- a/ipaserver/install/plugins/dns.py
+++ b/ipaserver/install/plugins/dns.py
@@ -17,6 +17,9 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see .
 
+from __future__ import absolute_import
+
+import dns.exception
 import re
 import traceback
 import time
@@ -489,8 +492,15 @@ class update_dnsforward_emptyzones(DNSUpdater):
 self.api.Command['dnsconfig_mod'](ipadnsversion=2)
 
 self.update_zones()
-if dnsutil.has_empty_zone_addresses(self.api.env.host):
-self.update_global_ldap_forwarder()
+try:
+if dnsutil.has_empty_zone_addresses(self.api.env.host):
+self.update_global_ldap_forwarder()
+except dns.exception.DNSException as ex:
+self.log.error('Skipping update of global DNS forwarder in LDAP: '
+   'Unable to determine if local server is using an '
+   'IP address belonging to an automatic empty zone. '
+   'Consider changing forwarding policy to "only". '
+   'DNS exception: %s', ex)
 
 return False, []
 
-- 
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 0156] server upgrade: do not start BIND if it was not running before the upgrad

[Petr Spacek]

Hello,

server upgrade: do not start BIND if it was not running before the upgrade

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

-- 
Petr^2 Spacek
From a01799ca093cc5572c11d9f73c90b8ee71a48d70 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Thu, 11 Aug 2016 15:10:04 +0200
Subject: [PATCH] server upgrade: do not start BIND if it was not running
 before the upgrade

https://fedorahosted.org/freeipa/ticket/6206
---
 ipaserver/install/server/upgrade.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index cc0af2b243d0c5693024e105fd0eab925b3f9b6c..d3ecde34f36d5adda730421d516110efd8ff29dd 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -1678,7 +1678,8 @@ def upgrade_configuration():
 root_logger.info('Changes to named.conf have been made, restart named')
 bind = bindinstance.BindInstance(fstore)
 try:
-bind.restart()
+if bind.is_running():
+bind.restart()
 except ipautil.CalledProcessError as e:
 root_logger.error("Failed to restart %s: %s", bind.service_name, e)
 
-- 
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 0003] Test validity of URIs in certificate

[Martin Basti]

On 11.08.2016 08:40, Lenka Doudova wrote:




On 08/10/2016 05:48 PM, Martin Basti wrote:




On 08.08.2016 10:30, Martin Basti wrote:




On 02.08.2016 14:50, Lenka Doudova wrote:




On 07/29/2016 11:43 AM, Lenka Doudova wrote:




On 07/29/2016 11:41 AM, Lenka Doudova wrote:


On 07/28/2016 01:35 PM, Peter Lacko wrote:

Hops, fixed.

Peter


- Original Message -
From: "Lenka Doudova"
To:freeipa-devel@redhat.com
Sent: Thursday, July 28, 2016 1:32:25 PM
Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in  
certificate

Hi,

I cannot find any attached patch :)

Lenka


On 07/28/2016 01:30 PM, Peter Lacko wrote:

Attached you can find a patch adding test for URIs in generated certificate
ipatests/test_xmlrpc/test_cert_plugin.py
Since I'm leaving Red Hat in end of July, I won't be able to modify this patch 
anymore.

Regards,

Peter





Hi,

NACK. Code looks fine and works well on master branch, but patch 
does not apply on 4-3 and 4-2 branches.
Peter left the company but claimed he can fix the patch if 
necessary, I'll communicate it with him or fix it myself.


Lenka



Oh, and forgot this one - PEP8 error:
./ipatests/test_xmlrpc/test_cert_plugin.py:191:80: E501 line too 
long (105 > 79 characters)


Lenka



Hi,

since Peter has quit already, I took it upon myself to do minor fix 
and rebase to the patch.
1) i removed pylint disable comments from the patch, as they were 
unnecessary (this also solved PEP8 error)

2) I rebased the patch to be applicable for ipa-4-3 branch.
Original functionality of the patch remains unchanged.

Both fixed patches attached.

Lenka




Hello,

1)
This is not needed
+global sn
+
+result = api.Command.cert_show(sn, out=unicode(self.certfile))

you need the global statement only for write access. But sn is not assigned in 
this code block.

2)
I prefer to use instance attributes (self.sn) instead of global variables


As we figured out, pytest creates for each test new instance of 
class, so 2) will not fork.

Please fix only 1), sorry.

Martin^2

Hi,
attached fixed patches for master and 4.3 branches.

Lenka




Martin^2







ACK

Pushed to master: 019f3611c299532cd89321767b0e0e4df0d0db27
Pushed to ipa-4-3: 2a207dd637748a4c05e54755b755986fbed16d55

-- 
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 0155] DNS server upgrade: do not fail when DNS server did not respond

[Petr Spacek]

Hello,

DNS server upgrade: do not fail when DNS server did not respond

Previously, update_dnsforward_emptyzones failed with an exeception if
DNS query failed for some reason. Now the error is logged and upgrade
continues.

I assume that this is okay because the DNS query is used as heuristics
of last resort in the upgrade logic and failure to do so should not have
catastrophics consequences: In the worst case, the admin needs to
manually change forwarding policy from 'first' to 'only'.

In the end I have decided not to auto-start BIND because BIND depends on
GSSAPI for authentication, which in turn depends on KDC ... Alternative
like reconfiguring BIND to use LDAPI+EXTERNAL and reconfiguring DS to
accept LDAP external bind from named user are too complicated.

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

-- 
Petr^2 Spacek
From 145332c9c627594a49e8546c6afb2a7a77dd46b9 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Thu, 11 Aug 2016 13:44:29 +0200
Subject: [PATCH] DNS server upgrade: do not fail when DNS server did not
 respond

Previously, update_dnsforward_emptyzones failed with an exeception if
DNS query failed for some reason. Now the error is logged and upgrade
continues.

I assume that this is okay because the DNS query is used as heuristics
of last resort in the upgrade logic and failure to do so should not have
catastrophics consequences: In the worst case, the admin needs to
manually change forwarding policy from 'first' to 'only'.

In the end I have decided not to auto-start BIND because BIND depends on
GSSAPI for authentication, which in turn depends on KDC ... Alternative
like reconfiguring BIND to use LDAPI+EXTERNAL and reconfiguring DS to
accept LDAP external bind from named user are too complicated.

https://fedorahosted.org/freeipa/ticket/6205
---
 ipaserver/install/plugins/dns.py | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/ipaserver/install/plugins/dns.py b/ipaserver/install/plugins/dns.py
index 873dbd03ed82d904fc712924db1a6bff813af065..6f67f9857778f3018666075c1616dab5d3f4ff11 100644
--- a/ipaserver/install/plugins/dns.py
+++ b/ipaserver/install/plugins/dns.py
@@ -17,6 +17,9 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see .
 
+from __future__ import absolute_import
+
+import dns.exception
 import ldap as _ldap
 import re
 import traceback
@@ -489,8 +492,15 @@ class update_dnsforward_emptyzones(DNSUpdater):
 self.api.Command['dnsconfig_mod'](ipadnsversion=2)
 
 self.update_zones()
-if dnsutil.has_empty_zone_addresses(self.api.env.host):
-self.update_global_ldap_forwarder()
+try:
+if dnsutil.has_empty_zone_addresses(self.api.env.host):
+self.update_global_ldap_forwarder()
+except dns.exception.DNSException as ex:
+self.log.error('Skipping update of global DNS forwarder in LDAP: '
+   'Unable to determine if local server is using an '
+   'IP address belonging to an automatic empty zone. '
+   'Consider changing forwarding policy to "only". '
+   'DNS exception: %s', ex)
 
 return False, []
 
-- 
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 0058] Make get_entries not ignore its size_limit argument

[Stanislav Laznicka]

On 08/11/2016 07:49 AM, Jan Cholasta wrote:

On 2.8.2016 13:47, Stanislav Laznicka wrote:

On 07/19/2016 09:20 AM, Jan Cholasta wrote:

Hi,

On 14.7.2016 14:36, Stanislav Laznicka wrote:

Hello,

This patch fixes https://fedorahosted.org/freeipa/ticket/5640.

With not so much experience with the framework, it raises question 
in my
head whether ipaldap.get_entries is used properly throughout the 
system
- does it always assume that it gets ALL the requested entries or 
just a

few of those as configured by the 'ipaSearchRecordsLimit' attribute of
ipaConfig.etc which it actually gets?


That depends. If you call get_entries() on the ldap2 plugin (which is
usually the case in the framework), then ipaSearchRecordsLimit is
used. If you call it on some arbitrary LDAPClient instance, the
hardcoded default (= unlimited) is used.



One spot that I know the get_entries method was definitely not used
properly before this patch is in the
baseldap.LDAPObject.get_memberindirect() method:

 692 result = self.backend.get_entries(
 693 self.api.env.basedn,
 694 filter=filter,
 695 attrs_list=['member'],
 696 size_limit=-1, # paged search will get everything
anyway
 697 paged_search=True)

which to me seems kind of important if the environment size_limit 
is not

set properly :) The patch does not fix the non-propagation of the
paged_search, though.


Why do you think size_limit is not used properly here?

AFAIU it is desired that the search is unlimited. However, due to the
fact that neither size_limit nor paged_search are passed from
ldap2.get_entries() to ldap2.find_entries() (methods inherited from
LDAPClient), only the number of records specified by
ipaSearchRecordsLimit is returned. That could eventually cause problems
should ipaSearchRecordsLimit be set to a low value as in the ticket.


I see. This is *not* intentional, the **kwargs of get_entries() should 
be passed to find_entries(). This definitely needs to be fixed.




Anyway, this ticket is not really easily fixable without more profound
changes. Often, multiple LDAP searches are done during command
execution. What do you do with the size limit then? Do you pass the
same size limit to all the searches? Do you subtract the result size
from the size limit after each search? Do you do something else with
it? ... The answer is that it depends on the purpose of each
individual LDAP search (like in get_memberindirect() above, we have to
do unlimited search, otherwise the resulting entry would be
incomplete), and fixing this accross the whole framework is a
non-trivial task.


I do realize that the proposed fix for the permission plugin is not
perfect, it would probably be better to subtract the number of currently
loaded records from the sizelimit, although in the end the number of
returned values will not be higher than the given size_limit. However,
it seems reasonable that if get_entries is passed a size limit, it
should apply it over current ipaSearchRecordsLimit rather than ignoring
it. Then, any use of get_entries could be fixed accordingly if someone
sees fit.



Right. Anyway, this is a different issue than above, so please put 
this into a separate commit.



Please see the attached patches, then.

From 75d8cf9c3708b68c9b3a9ba999b3d034b1ddc33a Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Thu, 11 Aug 2016 14:08:33 +0200
Subject: [PATCH 1/2] Make get_entries() not ignore its limit arguments

get_entries() wouldn't pass some arguments deeper to find_entries()
function it wraps. This would cause unexpected behavior in some
cases throughout the framework where specific (non-)limitations
are expected.

https://fedorahosted.org/freeipa/ticket/5640
---
 ipapython/ipaldap.py | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 704e71a9471c27430328a8c7c6a319aa72a9d482..a3f0a5668616f66ba744c336832064269882eb8b 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -1298,7 +1298,8 @@ class LDAPClient(object):
 for their description.
 """
 entries, truncated = self.find_entries(
-base_dn=base_dn, scope=scope, filter=filter, attrs_list=attrs_list)
+base_dn=base_dn, scope=scope, filter=filter, attrs_list=attrs_list,
+**kwargs)
 try:
 self.handle_truncated_result(truncated)
 except errors.LimitsExceeded as e:
@@ -1313,7 +1314,8 @@ class LDAPClient(object):
 
 def find_entries(self, filter=None, attrs_list=None, base_dn=None,
  scope=ldap.SCOPE_SUBTREE, time_limit=None,
- size_limit=None, search_refs=False, paged_search=False):
+ size_limit=None, search_refs=False, paged_search=False,
+ **kwargs):
 """
 Return a list of entries and indication of whether the results were
 truncated ([(dn, entry_attr

Re: [Freeipa-devel] [PATCHES] Coverity fixes

[Martin Basti]

On 05.08.2016 14:13, Lukas Slebodnik wrote:

On (05/08/16 12:43), Petr Vobornik wrote:

On 07/28/2016 01:01 PM, Martin Basti wrote:


On 25.07.2016 11:46, Simo Sorce wrote:

The attached patches fix some minor issues found by coverity, and pull
in other fixes released by the asn1c project.

Simo.




I cannot build RPMS with this patch, is there any missing build dependency?

/bin/sh ./libtool  --tag=CC   --mode=link gcc  -Wall -Wshadow
-Wstrict-prototypes -Wpointer-arith -Wcast-align
-Werror-implicit-function-declaration  -O2 -g -pipe -Wall
-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches
-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -g -O2 -Wall
-Wextra -Wformat-security -Wno-unused-parameter -Wno-sign-compare
-Wno-missing-field-initializers   -Wl,-z,relro
-specs=/usr/lib/rpm/redhat/redhat-hardened-ld  -o ipa-getkeytab ipa-getkeytab.o
ipa-client-common.o ipa_krb5.o ../asn1/libipaasn1.la -lkrb5 -lk5crypto -lcom_err
-llber -lldap -lsasl2 -lpopt  -lini_config -lbasicobjects -lref_array
-lcollection  -lini_config -lini_config
libtool: link: gcc -Wall -Wshadow -Wstrict-prototypes -Wpointer-arith
-Wcast-align -Werror-implicit-function-declaration -O2 -g -pipe -Wall
-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches
-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -g -O2 -Wall
-Wextra -Wformat-security -Wno-unused-parameter -Wno-sign-compare
-Wno-missing-field-initializers -Wl,-z -Wl,relro
-specs=/usr/lib/rpm/redhat/redhat-hardened-ld -o ipa-getkeytab ipa-getkeytab.o
ipa-client-common.o ipa_krb5.o ../asn1/.libs/libipaasn1.a -lkrb5 -lk5crypto
-lcom_err -llber -lldap -lsasl2 -lpopt -lbasicobjects -lref_array -lcollection
-lini_config
../asn1/.libs/libipaasn1.a(constr_CHOICE.o): In function `CHOICE_decode_uper':
/root/freeipa/rpmbuild/BUILD/freeipa-4.4.0/asn1/asn1c/constr_CHOICE.c:897:
undefined reference to `uper_open_type_get'
../asn1/.libs/libipaasn1.a(constr_CHOICE.o): In function `CHOICE_encode_uper':
/root/freeipa/rpmbuild/BUILD/freeipa-4.4.0/asn1/asn1c/constr_CHOICE.c:982:
undefined reference to `uper_open_type_put'
../asn1/.libs/libipaasn1.a(constr_SEQUENCE.o): In function
`SEQUENCE_handle_extensions':
/root/freeipa/rpmbuild/BUILD/freeipa-4.4.0/asn1/asn1c/constr_SEQUENCE.c:1285:
undefined reference to `uper_open_type_put'
../asn1/.libs/libipaasn1.a(constr_SEQUENCE.o): In function 
`SEQUENCE_decode_uper':
/root/freeipa/rpmbuild/BUILD/freeipa-4.4.0/asn1/asn1c/constr_SEQUENCE.c:1187:
undefined reference to `uper_open_type_get'
/root/freeipa/rpmbuild/BUILD/freeipa-4.4.0/asn1/asn1c/constr_SEQUENCE.c:1203:
undefined reference to `uper_open_type_skip'
collect2: error: ld returned 1 exit status

Martin^2


Bumping. Was it temporary issue or issue in the patch?


I could not see such error.
However, these patches would be good to test with coverity.
We need to use fedora rawhide for testing due to BuildRequires
in freeipa.spec. But C-part of freeIPA cannot be compiled on rawhide
due to new samba (4.5). Patches are already on the list.

LS



Lukas helped me to fix error I reported before (missing entries in 
Makefile.am), so I run covscan and I get bunch of new errors.


Question is, do we want push autogenerated code?


Error: FORWARD_NULL (CWE-476): [#def1]
freeipa-4.4.0/asn1/asn1c/INTEGER.c:463: assign_zero: Assigning: 
"dec_value_start" = "NULL".
freeipa-4.4.0/asn1/asn1c/INTEGER.c:488: var_deref_model: Passing null 
pointer "dec_value_start" to "asn_strtol_lim", which dereferences it.
freeipa-4.4.0/asn1/asn1c/INTEGER.c:975:2: deref_parm: Directly 
dereferencing parameter "str".

#  973|   if(str >= *end) return ASN_STRTOL_ERROR_INVAL;
#  974|
#  975|-> switch(*str) {
#  976|   case '-':
#  977|   last_digit_max++;

Error: MISSING_BREAK (CWE-484): [#def2]
freeipa-4.4.0/asn1/asn1c/INTEGER.c:976: unterminated_case: The case for 
value "'-'" is not terminated by a 'break' statement.
freeipa-4.4.0/asn1/asn1c/INTEGER.c:979: fallthrough: The above case 
falls through to this one.

#  977|   last_digit_max++;
#  978|   sign = -1;
#  979|-> case '+':
#  980|   str++;
#  981|   if(str >= *end) {

Error: NEGATIVE_RETURNS (CWE-394): [#def3]
freeipa-4.4.0/asn1/asn1c/OCTET_STRING.c:381: var_tested_neg: Variable 
"tlv_len" tests negative.
freeipa-4.4.0/asn1/asn1c/OCTET_STRING.c:391: var_assign: Assigning: 
signed variable "sel->left" = "tlv_len".
freeipa-4.4.0/asn1/asn1c/OCTET_STRING.c:247: var_assign: Assigning: 
unsigned variable "Left" = "sel->left".
freeipa-4.4.0/asn1/asn1c/OCTET_STRING.c:275: negative_returns: "Left" is 
passed to a parameter that cannot be negative.
freeipa-4.4.0/asn1/asn1c/ber_tlv_tag.c:33:2: parm_loop_bound: Using 
unsigned parameter "size" in a loop exit test.

#  273|   }
#  274|
#  275|-> tl = ber_fetch_tag(buf_ptr, Left,

Re: [Freeipa-devel] [PATCHES 681-682] cert: speed up cert-find, do not crash on invalid data in cert-find

[Martin Basti]

On 01.08.2016 10:27, Jan Cholasta wrote:

On 1.8.2016 10:19, Jan Cholasta wrote:

Hi,

the attached patches fix 
and .


Self-NACK, proper patches attached.

Honza





IMHO this is caused by your patches, test_cert_plugin.py:

[Thu Aug 11 14:12:18.740950 2016] [wsgi:error] [pid 85941] ipa: ERROR: 
non-public: KeyError: 'owner'
[Thu Aug 11 14:12:18.740986 2016] [wsgi:error] [pid 85941] Traceback 
(most recent call last):
[Thu Aug 11 14:12:18.740988 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 359, in 
wsgi_execute
[Thu Aug 11 14:12:18.740990 2016] [wsgi:error] [pid 85941] result = 
self.Command[name](*args, **options)
[Thu Aug 11 14:12:18.740992 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 449, in __call__
[Thu Aug 11 14:12:18.740994 2016] [wsgi:error] [pid 85941] return 
self.__do_call(*args, **options)
[Thu Aug 11 14:12:18.740995 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 477, in 
__do_call
[Thu Aug 11 14:12:18.740997 2016] [wsgi:error] [pid 85941] ret = 
self.run(*args, **options)
[Thu Aug 11 14:12:18.740998 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 799, in run
[Thu Aug 11 14:12:18.741000 2016] [wsgi:error] [pid 85941] return 
self.execute(*args, **options)
[Thu Aug 11 14:12:18.741001 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipaserver/plugins/baseldap.py", line 
1571, in execute
[Thu Aug 11 14:12:18.741003 2016] [wsgi:error] [pid 85941] 
delete_entry(pkey)
[Thu Aug 11 14:12:18.741004 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipaserver/plugins/baseldap.py", line 
1524, in delete_entry
[Thu Aug 11 14:12:18.741014 2016] [wsgi:error] [pid 85941] dn = 
callback(self, ldap, dn, *nkeys, **options)
[Thu Aug 11 14:12:18.741016 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipaserver/plugins/host.py", line 794, 
in pre_callback
[Thu Aug 11 14:12:18.741018 2016] [wsgi:error] [pid 85941] 
api.Command['service_del'](principal)
[Thu Aug 11 14:12:18.741019 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 449, in __call__
[Thu Aug 11 14:12:18.741021 2016] [wsgi:error] [pid 85941] return 
self.__do_call(*args, **options)
[Thu Aug 11 14:12:18.741022 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 477, in 
__do_call
[Thu Aug 11 14:12:18.741024 2016] [wsgi:error] [pid 85941] ret = 
self.run(*args, **options)
[Thu Aug 11 14:12:18.741025 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 799, in run
[Thu Aug 11 14:12:18.741027 2016] [wsgi:error] [pid 85941] return 
self.execute(*args, **options)
[Thu Aug 11 14:12:18.741028 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipaserver/plugins/baseldap.py", line 
1571, in execute
[Thu Aug 11 14:12:18.741029 2016] [wsgi:error] [pid 85941] 
delete_entry(pkey)
[Thu Aug 11 14:12:18.741031 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipaserver/plugins/baseldap.py", line 
1524, in delete_entry
[Thu Aug 11 14:12:18.741032 2016] [wsgi:error] [pid 85941] dn = 
callback(self, ldap, dn, *nkeys, **options)
[Thu Aug 11 14:12:18.741034 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipaserver/plugins/service.py", line 
674, in pre_callback
[Thu Aug 11 14:12:18.741035 2016] [wsgi:error] [pid 85941] 
revoke_certs(entry_attrs.get('usercertificate', []), self.log)
[Thu Aug 11 14:12:18.741037 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipaserver/plugins/service.py", line 
230, in revoke_certs
[Thu Aug 11 14:12:18.741038 2016] [wsgi:error] [pid 85941] result = 
api.Command['cert_show'](unicode(serial))['result']
[Thu Aug 11 14:12:18.741040 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 449, in __call__
[Thu Aug 11 14:12:18.741041 2016] [wsgi:error] [pid 85941] return 
self.__do_call(*args, **options)
[Thu Aug 11 14:12:18.741042 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 477, in 
__do_call
[Thu Aug 11 14:12:18.741044 2016] [wsgi:error] [pid 85941] ret = 
self.run(*args, **options)
[Thu Aug 11 14:12:18.741045 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 799, in run
[Thu Aug 11 14:12:18.741047 2016] [wsgi:error] [pid 85941] return 
self.execute(*args, **options)
[Thu Aug 11 14:12:18.741048 2016] [wsgi:error] [pid 85941]   File 
"/usr/lib/python2.7/site-packages/ipaserver/plugins/cert.py", line 792, 
in execute
[Thu Aug 11 14:12:18.741050 2016] [wsgi:error] [pid 85941] 

Re: [Freeipa-devel] [PATCH] 0001 Added new authentication method

[Petr Vobornik]

On 08/11/2016 10:54 AM, Alexander Bokovoy wrote:
> On Thu, 11 Aug 2016, Jan Cholasta wrote:
>> On 4.8.2016 17:27, Jan Pazdziora wrote:
>>> On Wed, Aug 03, 2016 at 10:29:52AM +0300, Alexander Bokovoy wrote:

 Got it. One thing I would correct, though, -- don't use
 kadmin.local, we
 do support setting ok_as_delegate on the service principals via IPA
 CLI:
 $ ipa service-mod --help |grep -A1 ok-as-delegate
 --ok-as-delegate=BOOL
   Client credentials may be delegated to the
 service
>>>
>>> I've tried
>>>
>>> ipa service-mod --ok-as-delegate=True HTTP/$(hostname)
>>>
>>> but that does not seem to have the same effect as
>>>
>>> modprinc +ok_to_auth_as_delegate HTTP/ipa.example.test
>>>
>>> -- obtaining the delegated certificated fails.
>>
>> That's because ok_as_delegate and ok_to_auth_as_delegate are different
>> flags.
> Right. The following patch adds ok_to_auth_as_delegate to the service
> principal.
> 
> I haven't added any tickets to it yet.
> 
> 

This might deserve also nice Web UI checkbox similar to "Trusted for
delegation". CCing Pavel.

-- 
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] [DESIGN][UPDATE] Time-Based HBAC Policies

[Stanislav Laznicka]

Hello,

I updated the design of the Time-Based HBAC Policies according to the 
discussion we led here earlier. Please check the design page 
http://www.freeipa.org/page/V4/Time-Based_Account_Policies. The biggest 
changes are in the Implementation and Feature Management sections. I 
also added a short How to Use section.


On the link below is a PROTOTYPE-patched FreeIPA that covers most of the 
CLI functionality (except for the creation of iCalendar strings from 
options) for better illustration of the design.


https://github.com/stlaz/freeipa/tree/timerules_2

I will add FreeIPA people that recently had some say about this to CC so 
that we can get the discussion flowing.


Thanks,
Standa

--
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] 0003 Added support for authentication with user certificate

[Jan Cholasta]

Hi,

On 11.8.2016 09:55, Tibor Dudlak wrote:

Hi,

I think this patch is finished. If it does not suits you and it will not
be merged please consider merging PATCH 0001 from
http://www.redhat.com/archives/freeipa-devel/2016-August/msg9.html
at least.

Thank you

On Wed, Aug 10, 2016 at 10:17 AM, Tibor Dudlak mailto:tdud...@redhat.com>> wrote:

Hi,

I have improved my previous patch for authentication with user
certificate/smartcard.
This patch includes patches and plugin and apache configuration
described here:
http://www.freeipa.org/page/V4/External_Authentication/Setup

It also contains steps to configure and test this feature. Once this
patch is merged and released I will simplify this page to not
confuse customers.

On Fri, Aug 5, 2016 at 3:58 PM, Petr Vobornik mailto:pvobo...@redhat.com>> wrote:

On 08/05/2016 02:57 PM, Tibor Dudlak wrote:
>...

Let's assume that we will go with this approach and not separate
RPM.

1. ipa.conf version needs to be bumped


We have found another problem with ipa.conf approach so I have moved
configuration of apache for plugin from ipa.conf into completely
separated file to be not configured in FreeIPA by default. As you
said it may cause some security issues and it definitely causes
errors when plugin dependences are not installed nor configured.

2. Do not put the web ui plugin in src/freeipa/plugins dir. That
is a
dir for core UI plugins. This one is sort of hybrid - basically
a third
party plugin added to core package  but enabled as third party
because
the feature is experimental.

Create rather a new dir for that. E.g. plugins.d as Alexander
suggested
->  freeipa/install/ui/src/plugins.d/cert_auth/cert_auth.js

3. unrelated and "alternative solution"  comments needs to be
removed
from the UI plugin. They were added to the example plugin
https://pvoborni.fedorapeople.org/plugins/loginauth/loginauth.js

mostly
to help you with the development.

4. Add comment to freeipa.spec.in 
describing what the plugin is and why
it is put there this way.

5. The plugin itself deserves better description as well. Right now
there is the general description.

6. I have not tried it, but make sure that it passes jslint
(`jsl -conf
jsl.conf`) Easiest may be to use temp(i.e. do not include it here)
jsl.conf e.g.:
https://pvoborni.fedorapeople.org/plugins/loginauth/jsl.conf


--
Petr Vobornik


I hope result of jsl http://pastebin.test.redhat.com/400076
 means that things passed.
Thanks Petr for review and I hope this patch will cover all concerns
he had.

Addressing ticket: https://fedorahosted.org/freeipa/ticket/5764


Thank you.


+class login_x509(login_kerberos, KerberosSession, HTTP_Status):
+key = '/session/login_x509'

login_kerberos already subclasses KerberosSession and HTTP_Status, no 
need to do it again here. In fact, it would be best to split off the 
bussiness logic from login_kerberos into a separate class and inherit 
both login_kerberos and login_x509 from it:


class KerberosLogin(Backend, KerberosSession, HTTP_Status):
def _on_finalize(self):
...

def __call__(self, ...):
...

class login_kerberos(KerberosLogin):
key = '/session/login_kerberos'

class login_x509(KerberosLogin):
key = '/session/login_x509'

Honza

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH 0003][Tests] Fix for integration tests replication layouts

[Martin Basti]

On 09.08.2016 16:55, Ganna Kaihorodova wrote:

Hello!

Domain level 0 doesn't allow to create replica file on CA master, testcase was 
skipped with Domain level 0

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

Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer





Hello,

Please fix PEP8 error you introduced.
./ipatests/test_integration/test_replication_layouts.py:32:1: E302 
expected 2 blank lines, found 1


IMO this need skip on domain level 0 too
* Test2ConnectedTopologyWithoutCA
* TestDoubleCircleTopologyWithoutCA

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] 0001 Added new authentication method

[Alexander Bokovoy]

On Thu, 11 Aug 2016, Jan Cholasta wrote:

On 4.8.2016 17:27, Jan Pazdziora wrote:

On Wed, Aug 03, 2016 at 10:29:52AM +0300, Alexander Bokovoy wrote:


Got it. One thing I would correct, though, -- don't use kadmin.local, we
do support setting ok_as_delegate on the service principals via IPA CLI:
$ ipa service-mod --help |grep -A1 ok-as-delegate
--ok-as-delegate=BOOL
  Client credentials may be delegated to the service


I've tried

ipa service-mod --ok-as-delegate=True HTTP/$(hostname)

but that does not seem to have the same effect as

modprinc +ok_to_auth_as_delegate HTTP/ipa.example.test

-- obtaining the delegated certificated fails.


That's because ok_as_delegate and ok_to_auth_as_delegate are different 
flags.

Right. The following patch adds ok_to_auth_as_delegate to the service
principal.

I haven't added any tickets to it yet.
--
/ Alexander Bokovoy
From 9af1c479cf8d1862c001fccd5345bd93dd6e54a8 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy 
Date: Thu, 11 Aug 2016 11:52:05 +0300
Subject: [PATCH 6/6] service: add flag to allow S4U2Self

---
 API.txt  | 12 
 ipaserver/plugins/service.py |  7 +++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/API.txt b/API.txt
index 535d8ec..5b83bfb 100644
--- a/API.txt
+++ b/API.txt
@@ -2260,7 +2260,7 @@ output: Output('summary', type=[, ])
 output: Output('value', type=[])
 output: Output('warning', type=[, , ])
 command: host_add/1
-args: 1,24,3
+args: 1,25,3
 arg: Str('fqdn', cli_name='hostname')
 option: Str('addattr*', cli_name='addattr')
 option: Flag('all', autofill=True, cli_name='all', default=False)
@@ -2269,6 +2269,7 @@ option: Flag('force', autofill=True, default=False)
 option: Str('ip_address?')
 option: Str('ipaassignedidview?')
 option: Bool('ipakrbokasdelegate?', cli_name='ok_as_delegate')
+option: Bool('ipakrboktoauthasdelegate?', cli_name='ok_to_auth_as_delegate')
 option: Bool('ipakrbrequirespreauth?', cli_name='requires_pre_auth')
 option: Str('ipasshpubkey*', cli_name='sshpubkey')
 option: Str('krbprincipalauthind*', cli_name='auth_ind')
@@ -2437,7 +2438,7 @@ output: ListOfEntries('result')
 output: Output('summary', type=[, ])
 output: Output('truncated', type=[])
 command: host_mod/1
-args: 1,25,3
+args: 1,26,3
 arg: Str('fqdn', cli_name='hostname')
 option: Str('addattr*', cli_name='addattr')
 option: Flag('all', autofill=True, cli_name='all', default=False)
@@ -2445,6 +2446,7 @@ option: Str('delattr*', cli_name='delattr')
 option: Str('description?', autofill=False, cli_name='desc')
 option: Str('ipaassignedidview?', autofill=False)
 option: Bool('ipakrbokasdelegate?', autofill=False, cli_name='ok_as_delegate')
+option: Bool('ipakrboktoauthasdelegate?', autofill=False, 
cli_name='ok_to_auth_as_delegate')
 option: Bool('ipakrbrequirespreauth?', autofill=False, 
cli_name='requires_pre_auth')
 option: Str('ipasshpubkey*', autofill=False, cli_name='sshpubkey')
 option: Str('krbprincipalauthind*', autofill=False, cli_name='auth_ind')
@@ -4293,13 +4295,14 @@ output: Entry('result')
 output: Output('summary', type=[, ])
 output: PrimaryKey('value')
 command: service_add/1
-args: 1,12,3
+args: 1,13,3
 arg: Principal('krbcanonicalname', cli_name='canonical_principal')
 option: Str('addattr*', cli_name='addattr')
 option: Flag('all', autofill=True, cli_name='all', default=False)
 option: Flag('force', autofill=True, default=False)
 option: StrEnum('ipakrbauthzdata*', cli_name='pac_type', values=[u'MS-PAC', 
u'PAD', u'NONE'])
 option: Bool('ipakrbokasdelegate?', cli_name='ok_as_delegate')
+option: Bool('ipakrboktoauthasdelegate?', cli_name='ok_to_auth_as_delegate')
 option: Bool('ipakrbrequirespreauth?', cli_name='requires_pre_auth')
 option: Str('krbprincipalauthind*', cli_name='auth_ind')
 option: Flag('no_members', autofill=True, default=False)
@@ -4435,13 +4438,14 @@ output: ListOfEntries('result')
 output: Output('summary', type=[, ])
 output: Output('truncated', type=[])
 command: service_mod/1
-args: 1,14,3
+args: 1,15,3
 arg: Principal('krbcanonicalname', cli_name='canonical_principal')
 option: Str('addattr*', cli_name='addattr')
 option: Flag('all', autofill=True, cli_name='all', default=False)
 option: Str('delattr*', cli_name='delattr')
 option: StrEnum('ipakrbauthzdata*', autofill=False, cli_name='pac_type', 
values=[u'MS-PAC', u'PAD', u'NONE'])
 option: Bool('ipakrbokasdelegate?', autofill=False, cli_name='ok_as_delegate')
+option: Bool('ipakrboktoauthasdelegate?', autofill=False, 
cli_name='ok_to_auth_as_delegate')
 option: Bool('ipakrbrequirespreauth?', autofill=False, 
cli_name='requires_pre_auth')
 option: Str('krbprincipalauthind*', autofill=False, cli_name='auth_ind')
 option: Principal('krbprincipalname*', autofill=False, cli_name='principal')
diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py
index a44dcaa..04d1916 100644
--- a/ipaserver/plugins/service.py
+++ b/ipaserver/plugins/service.py
@@ -171,11 +171,18 @@ ti

Re: [Freeipa-devel] [PATCH] 0001 Added new authentication method

[Jan Cholasta]

On 4.8.2016 17:27, Jan Pazdziora wrote:

On Wed, Aug 03, 2016 at 10:29:52AM +0300, Alexander Bokovoy wrote:


Got it. One thing I would correct, though, -- don't use kadmin.local, we
do support setting ok_as_delegate on the service principals via IPA CLI:
$ ipa service-mod --help |grep -A1 ok-as-delegate
 --ok-as-delegate=BOOL
   Client credentials may be delegated to the service


I've tried

ipa service-mod --ok-as-delegate=True HTTP/$(hostname)

but that does not seem to have the same effect as

modprinc +ok_to_auth_as_delegate HTTP/ipa.example.test

-- obtaining the delegated certificated fails.


That's because ok_as_delegate and ok_to_auth_as_delegate are different 
flags.


--
Jan Cholasta

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

Re: [Freeipa-devel] [Test][patch-0058] Fixed topology tests failures in CI

[Martin Basti]

On 10.08.2016 20:32, Oleg Fayans wrote:





Hello,

before we jump into fixing tests, my question is: Was this planned 
change and not reflected by test, or switched values are unwanted side 
effect and thus bug for us?


Ticket contains almost no info, except a traceback and it says nothing. 
Commit message says at least something.


I'm not sure if this patch fixes that ticket, because traceback in test 
shows error message that "removal of segment will disconnect topology", 
but this patch only swap order of replica names in segment name. I would 
expect that you should get different error, something like segment does 
not exist.


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] 0003 Added support for authentication with user certificate

[Tibor Dudlak]

Hi,

I think this patch is finished. If it does not suits you and it will not be
merged please consider merging PATCH 0001 from
http://www.redhat.com/archives/freeipa-devel/2016-August/msg9.html at
least.

Thank you

On Wed, Aug 10, 2016 at 10:17 AM, Tibor Dudlak  wrote:

> Hi,
>
> I have improved my previous patch for authentication with user
> certificate/smartcard.
> This patch includes patches and plugin and apache configuration described
> here: http://www.freeipa.org/page/V4/External_Authentication/Setup
> It also contains steps to configure and test this feature. Once this patch
> is merged and released I will simplify this page to not confuse customers.
>
> On Fri, Aug 5, 2016 at 3:58 PM, Petr Vobornik  wrote:
>
>> On 08/05/2016 02:57 PM, Tibor Dudlak wrote:
>> >...
>>
>> Let's assume that we will go with this approach and not separate RPM.
>>
>> 1. ipa.conf version needs to be bumped
>>
>
> We have found another problem with ipa.conf approach so I have moved
> configuration of apache for plugin from ipa.conf into completely separated
> file to be not configured in FreeIPA by default. As you said it may cause
> some security issues and it definitely causes errors when plugin
> dependences are not installed nor configured.
>
> 2. Do not put the web ui plugin in src/freeipa/plugins dir. That is a
>> dir for core UI plugins. This one is sort of hybrid - basically a third
>> party plugin added to core package  but enabled as third party because
>> the feature is experimental.
>>
>> Create rather a new dir for that. E.g. plugins.d as Alexander suggested
>> ->  freeipa/install/ui/src/plugins.d/cert_auth/cert_auth.js
>>
>> 3. unrelated and "alternative solution"  comments needs to be removed
>> from the UI plugin. They were added to the example plugin
>> https://pvoborni.fedorapeople.org/plugins/loginauth/loginauth.js mostly
>> to help you with the development.
>>
>> 4. Add comment to freeipa.spec.in describing what the plugin is and why
>> it is put there this way.
>>
>> 5. The plugin itself deserves better description as well. Right now
>> there is the general description.
>>
>> 6. I have not tried it, but make sure that it passes jslint (`jsl -conf
>> jsl.conf`) Easiest may be to use temp(i.e. do not include it here)
>> jsl.conf e.g.: https://pvoborni.fedorapeople.
>> org/plugins/loginauth/jsl.conf
>>
>> --
>> Petr Vobornik
>>
>
> I hope result of jsl http://pastebin.test.redhat.com/400076 means that
> things passed.
> Thanks Petr for review and I hope this patch will cover all concerns he
> had.
>
> Addressing ticket: https://fedorahosted.org/freeipa/ticket/5764
>
> Thank you.
>
> --
> Tibor Dudlák
> Intern - Identity management Special Projects
> Red Hat
>



-- 
Tibor Dudlák
Intern - Identity management Special Projects
Red Hat
-- 
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