date:20160721

[Freeipa-devel] [PATCH] 0095 cert-request: allow directoryName in SAN extension

2016-07-21 Thread Fraser Tweedale

While I was poking around SAN-processing code, I decided to
implement a small enhancement: allowing the subject principal's DN
to appear in SAN.

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

Patch depends on my other patches 0090, 0092, 0093, 0094.

Thanks,
Fraser
From 6a2ab7165c0ae600402c1c2794f2b10c9e38da05 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Fri, 22 Jul 2016 13:07:09 +1000
Subject: [PATCH] cert-request: allow directoryName in SAN extension

Allow directoryName in SAN extension if the value matches the
subject principal's DN in the IPA directory.

Fixes: https://fedorahosted.org/freeipa/ticket/6112
---
 ipaserver/plugins/cert.py | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 
606d6cdbc28d30892ab60ad4aeb41ecbbd646589..605fd321f00304f69347aae633f935dde8e59bdc
 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -667,6 +667,12 @@ class cert_request(Create, BaseCertMethod, VirtualCommand):
 error=_("subject alt name type %s is forbidden "
 "for non-user principals") % desc
 )
+elif name_type == nss.certDirectoryName:
+if DN(name) != principal_obj['dn']:
+raise errors.ValidationError(
+name='csr',
+error=_("Directory Name does not match principal's DN")
+)
 else:
 raise errors.ACIError(
 info=_("Subject alt name type %s is forbidden") % desc)
-- 
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] 0090, 0092..0094 cert-show: show subject alternative names

2016-07-21 Thread Fraser Tweedale

On Tue, Jul 19, 2016 at 08:50:34AM +0200, Jan Cholasta wrote:
> Hi,
> 
> On 14.7.2016 13:44, Fraser Tweedale wrote:
> > Hi all,
> > 
> > The attached patch includes SANs in cert-show output.  If you have
> > certs with esoteric altnames (especially any that are more than just
> > ASN.1 string types), please test with those certs.
> > 
> > https://fedorahosted.org/freeipa/ticket/6022
> 
> I think it would be better to have a separate attribute for each supported
> SAN type rather than cramming everything into subject_alt_name. That way if
> you care only about a single specific type you won't have to go through all
> the values and parse them. Also it would allow you to use param types
> appropriate to the SAN types (DNSNameParam for DNS names, Principal for
> principal names, etc.)
> 
> Nitpick: please don't mix moving existing stuff and adding new stuff in a
> single patch.
> 
Updated patches attached.

Patches 0092..0094 are refactors and bugfixes.
Patch 0090-2 is the main feature (depends on 0092..0094).

Thanks,
Fraser
From 0f85eba4efdd9281725c54268e8d213c412edebf Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Thu, 21 Jul 2016 16:27:49 +1000
Subject: [PATCH 92/94] Move GeneralName parsing code to ipalib.x509

GeneralName parsing code is primarily relevant to X.509.  An
upcoming change will add SAN parsing to the cert-show command, so
first move the GeneralName parsing code from ipalib.pkcs10 to
ipalib.x509.

Part of: https://fedorahosted.org/freeipa/ticket/6022
---
 ipalib/pkcs10.py  |  93 ++---
 ipalib/x509.py| 114 +-
 ipaserver/plugins/cert.py |   8 ++--
 3 files changed, 120 insertions(+), 95 deletions(-)

diff --git a/ipalib/pkcs10.py b/ipalib/pkcs10.py
index 
e340c1a2005ad781542a32a0a76753e80364dacf..158ebb3a25be2bd292f3883545fe8afe49b7be8c
 100644
--- a/ipalib/pkcs10.py
+++ b/ipalib/pkcs10.py
@@ -22,9 +22,10 @@ from __future__ import print_function
 import sys
 import base64
 import nss.nss as nss
-from pyasn1.type import univ, char, namedtype, tag
+from pyasn1.type import univ, namedtype, tag
 from pyasn1.codec.der import decoder
 import six
+from ipalib import x509
 
 if six.PY3:
 unicode = str
@@ -32,11 +33,6 @@ if six.PY3:
 PEM = 0
 DER = 1
 
-SAN_DNSNAME = 'DNS name'
-SAN_RFC822NAME = 'RFC822 Name'
-SAN_OTHERNAME_UPN = 'Other Name (OID.1.3.6.1.4.1.311.20.2.3)'
-SAN_OTHERNAME_KRB5PRINCIPALNAME = 'Other Name (OID.1.3.6.1.5.2.2)'
-
 def get_subject(csr, datatype=PEM):
 """
 Given a CSR return the subject value.
@@ -72,78 +68,6 @@ def get_extensions(csr, datatype=PEM):
 return tuple(get_prefixed_oid_str(ext)[4:]
  for ext in request.extensions)
 
-class _PrincipalName(univ.Sequence):
-componentType = namedtype.NamedTypes(
-namedtype.NamedType('name-type', univ.Integer().subtype(
-explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0))
-),
-namedtype.NamedType('name-string', 
univ.SequenceOf(char.GeneralString()).subtype(
-explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 1))
-),
-)
-
-class _KRB5PrincipalName(univ.Sequence):
-componentType = namedtype.NamedTypes(
-namedtype.NamedType('realm', char.GeneralString().subtype(
-explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0))
-),
-namedtype.NamedType('principalName', _PrincipalName().subtype(
-explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 1))
-),
-)
-
-def _decode_krb5principalname(data):
-principal = decoder.decode(data, asn1Spec=_KRB5PrincipalName())[0]
-realm = (str(principal['realm']).replace('\\', '')
-.replace('@', '\\@'))
-name = principal['principalName']['name-string']
-name = '/'.join(str(n).replace('\\', '')
-  .replace('/', '\\/')
-  .replace('@', '\\@') for n in name)
-name = '%s@%s' % (name, realm)
-return name
-
-class _AnotherName(univ.Sequence):
-componentType = namedtype.NamedTypes(
-namedtype.NamedType('type-id', univ.ObjectIdentifier()),
-namedtype.NamedType('value', univ.Any().subtype(
-explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0))
-),
-)
-
-class _GeneralName(univ.Choice):
-componentType = namedtype.NamedTypes(
-namedtype.NamedType('otherName', _AnotherName().subtype(
-implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0))
-),
-namedtype.NamedType('rfc822Name', char.IA5String().subtype(
-implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 1))
-),
-namedtype.NamedType('dNSName', char.IA5String().subtype(
-implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 2))
-),
-

Re: [Freeipa-devel] [PATCH 0002][Tests] Small fix for dns_plugin tests

2016-07-21 Thread Martin Basti




On 20.07.2016 18:17, Ganna Kaihorodova wrote:

Hello!

Thank you for review.
I attached patch with fixed commit message


Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer


- Original Message -
From: "Martin Basti" 
To: "Ganna Kaihorodova" , freeipa-devel@redhat.com
Sent: Wednesday, July 20, 2016 5:04:47 PM
Subject: Re: [Freeipa-devel] [PATCH 0002][Tests] Small fix for dns_plugin tests



On 20.07.2016 17:02, Ganna Kaihorodova wrote:

Greetings!

Fix for ipatests/test_xmlrpc/test_dns_plugin.py

Fix conflict between “got” and “expected” values when testing "dnsconfig_mod: Update 
global DNS settings"

Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer





LGTM, but can you fix commit message?

This looks very suspicious

Subject: [PATCH 2/2] =?UTF-8?q?Fix=20conflict=20between=20=E2=80=9Cgot?=
   =?UTF-8?q?=E2=80=9D=20and=20=E2=80=9Cexpected=E2=80=9D=20values=20when=20?=
   =?UTF-8?q?testing=20"dnsconfig=5Fmod:=20Update=20global=20DNS=20settings"?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


regards,
Martin^2

ACK

I just replaced some fancy unicode quotation marks with ASCII in commit 
message before push


Pushed to master: 359cfeb7c6798038f5638f9d0977dda351f21431

--
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 0556] host-del: fix behavior of --updatedns and PTR records

2016-07-21 Thread Martin Basti


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


Patch attached.

From e358291a66827518c29250fce303fc00db7bcec4 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 21 Jul 2016 13:18:34 +0200
Subject: [PATCH] Host-del: fix behavior of --updatedns and PTR records

* target for ptr record must be absolute domain name
* zone is detected using DNS system instead of random splitting of
hostname

https://fedorahosted.org/freeipa/ticket/6060
---
 ipaserver/plugins/host.py | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py
index f342b05c87b936ab7b99009cfb0f6d3acde4ef93..413dcf15e0423170d8334902b9dcf8fb5aa14de6 100644
--- a/ipaserver/plugins/host.py
+++ b/ipaserver/plugins/host.py
@@ -18,6 +18,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.resolver
 import string
 
 import six
@@ -134,7 +137,7 @@ register = Registry()
 host_pwd_chars = string.digits + string.ascii_letters + '_,.@+-='
 
 
-def remove_ptr_rec(ipaddr, host, domain):
+def remove_ptr_rec(ipaddr, fqdn):
 """
 Remove PTR record of IP address (ipaddr)
 :return: True if PTR record was removed, False if record was not found
@@ -143,13 +146,12 @@ def remove_ptr_rec(ipaddr, host, domain):
 try:
 revzone, revname = get_reverse_zone(ipaddr)
 
-# in case domain is in FQDN form with a trailing dot, we needn't add
-# another one, in case it has no trailing dot, dnsrecord-del will
-# normalize the entry
-delkw = {'ptrrecord': "%s.%s" % (host, domain)}
+# assume that target in PTR record is absolute name (otherwise it is
+# non-standard configuration)
+delkw = {'ptrrecord': u"%s" % fqdn.make_absolute()}
 
 api.Command['dnsrecord_del'](revzone, revname, **delkw)
-except errors.NotFound:
+except (errors.NotFound, errors.AttrValueNotFound):
 api.log.debug('PTR record of ipaddr %s not found', ipaddr)
 return False
 
@@ -794,13 +796,15 @@ class host_del(LDAPDelete):
 
 if updatedns:
 # Remove A, , SSHFP and PTR records of the host
-parts = fqdn.split('.')
-domain = unicode('.'.join(parts[1:]))
+fqdn_dnsname = DNSName(fqdn).make_absolute()
+zone = DNSName(dns.resolver.zone_for_name(fqdn_dnsname))
+relative_hostname = fqdn_dnsname.relativize(zone)
+
 # Get all resources for this host
 rec_removed = False
 try:
 record = api.Command['dnsrecord_show'](
-domain, parts[0])['result']
+zone, relative_hostname)['result']
 except errors.NotFound:
 pass
 else:
@@ -808,13 +812,13 @@ class host_del(LDAPDelete):
 for attr in ('arecord', 'record'):
 for val in record.get(attr, []):
 rec_removed = (
-remove_ptr_rec(val, parts[0], domain) or
+remove_ptr_rec(val, fqdn_dnsname) or
 rec_removed
 )
 try:
 # remove all A, , SSHFP records of the host
 api.Command['dnsrecord_mod'](
-domain,
+zone,
 record['idnsname'][0],
 arecord=[],
 record=[],
-- 
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 0555] AVC: use copy during instalation to keep SELinux context valid

2016-07-21 Thread Martin Basti


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

I was able to reproduce this locally with vagrant, but I haven't been 
able to reproduce this in LAB, I don't know where differences are (cloud 
vs desktop fedora?)



Patch attached.

From 80e95343c0ff3c8ee1bb8628507a31499e5a96f5 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 21 Jul 2016 18:49:57 +0200
Subject: [PATCH] Use copy when replacing files to keep SELinux context

When installer replaces any file with newer, it must use 'copy' instead of
'mv' to keep SELinux context valid.

https://fedorahosted.org/freeipa/ticket/6111
---
 ipapython/ipautil.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 763a99c117e22a4ac49d8d34b38230f3da7c8435..9964fba4f694b57242b3bd3065a418917d977533 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -528,10 +528,14 @@ def dir_exists(filename):
 except Exception:
 return False
 
+
 def install_file(fname, dest):
+# SELinux: use copy to keep the right context
 if file_exists(dest):
 os.rename(dest, dest + ".orig")
-shutil.move(fname, dest)
+shutil.copy(fname, dest)
+os.remove(fname)
+
 
 def backup_file(fname):
 if file_exists(fname):
-- 
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] [DRAFT] FreeIPA 4.3.2 release notes

2016-07-21 Thread Petr Vobornik

Hi all,

this is a draft of release notes for upcoming 4.3.2 release
- http://www.freeipa.org/page/Releases/4.3.2

Comments/updates welcome!

Regards,
-- 
Petr Vobornik

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

Re: [Freeipa-devel] [PATCH] 963 unite log file name of ipa-ca-install

2016-07-21 Thread Martin Babinsky


On 07/21/2016 05:49 PM, Petr Vobornik wrote:

On 07/21/2016 05:47 PM, Martin Babinsky wrote:

On 07/21/2016 05:22 PM, Petr Vobornik wrote:

On 07/19/2016 09:27 AM, Petr Vobornik wrote:

On 07/19/2016 08:01 AM, Jan Cholasta wrote:

Hi,

On 18.7.2016 18:50, Florence Blanc-Renaud wrote:

On 07/15/2016 04:29 PM, Petr Vobornik wrote:

ipa-ca-install said that it used
  /var/log/ipareplica-ca-install.log
but in fact it used
  /var/log/ipaserver-ca-install.log

This patch unites it to ipaserver-ca-install.log

It was chosen because ipa-ca-install can be also used on
master on CA-less -> CA conversion.

Term "server" is valid for both master and replica.

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





Looks good to me.
Ack


Does not look so good to me, "ipareplica-ca-install.log" is in fact the
original file name used since ipa-ca-install was introduced (in commit
8a32bb3746802a29b2655e4ad2cbbba8481e1eaf), so why the switch to
"ipaserver-ca-install.log"?

Honza



Ideally it would be ipa-ca-install.log but for backwards compatibility,
let's stick with one which we have. AFAIK the framework(run_script
methodú doesn't support switching the log name which is printed in error
message depending on usage. Therefore the universal was chosen -
ipaserver-ca-install.log. It was introduced by your commit
d27e77adc56f5a04f3bdd1aaed5440a89ed3acad

And I see, that I used wrong ticket number in the commit. Correct is
https://fedorahosted.org/freeipa/ticket/6086

Proper solution might be to rework main and __main__ function in
ipa-ca-install but IMO we spent too much time on this already.



Updated patch attach - it uses the other log.





Correct me if I'm wrong but the ticket URL should be
https://fedorahosted.org/freeipa/ticket/6086



you are absolutely right. Fixed.


ACK

Pushed to master: 1b8a36d134dd320896e05809cc6b49f725eadda7

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

[Freeipa-devel] [PATCH] 0083: webui: remove full name column from user to user group adder dialog

2016-07-21 Thread Pavel Vomacka


Remove full name from adding user to user group dialog

As the 'cn' is not in the response of user-show there is empty column in 
adder dialog.

Therefore the column was removed.

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

--
Pavel^3 Vomacka

From 336a37f1c5d1ed34c7abbd75dd7dd63a1f274d9d Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Thu, 21 Jul 2016 17:44:50 +0200
Subject: [PATCH] Remove full name from adding user to user group dialog

As the 'cn' is not in the response of user-show there is empty column in adder dialog.
Therefore the column was removed.

https://fedorahosted.org/freeipa/ticket/6055
---
 install/ui/src/freeipa/group.js | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/install/ui/src/freeipa/group.js b/install/ui/src/freeipa/group.js
index 6f6f13e74737de9c79daa9d7d0ad156365a08214..b26560c0acb4fe2be057a87514534fca422cbee7 100644
--- a/install/ui/src/freeipa/group.js
+++ b/install/ui/src/freeipa/group.js
@@ -105,13 +105,8 @@ return {
 ],
 adder_columns:[
 {
-name: 'cn',
-width: '100px'
-},
-{
 name: 'uid',
-primary_key: true,
-width: '100px'
+primary_key: true
 }
 ]
 },
-- 
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] 963 unite log file name of ipa-ca-install

2016-07-21 Thread Petr Vobornik

On 07/21/2016 05:47 PM, Martin Babinsky wrote:
> On 07/21/2016 05:22 PM, Petr Vobornik wrote:
>> On 07/19/2016 09:27 AM, Petr Vobornik wrote:
>>> On 07/19/2016 08:01 AM, Jan Cholasta wrote:
 Hi,

 On 18.7.2016 18:50, Florence Blanc-Renaud wrote:
> On 07/15/2016 04:29 PM, Petr Vobornik wrote:
>> ipa-ca-install said that it used
>>   /var/log/ipareplica-ca-install.log
>> but in fact it used
>>   /var/log/ipaserver-ca-install.log
>>
>> This patch unites it to ipaserver-ca-install.log
>>
>> It was chosen because ipa-ca-install can be also used on
>> master on CA-less -> CA conversion.
>>
>> Term "server" is valid for both master and replica.
>>
>> https://fedorahosted.org/freeipa/ticket/6088
>>
>>
>>
>
> Looks good to me.
> Ack

 Does not look so good to me, "ipareplica-ca-install.log" is in fact the
 original file name used since ipa-ca-install was introduced (in commit
 8a32bb3746802a29b2655e4ad2cbbba8481e1eaf), so why the switch to
 "ipaserver-ca-install.log"?

 Honza

>>>
>>> Ideally it would be ipa-ca-install.log but for backwards compatibility,
>>> let's stick with one which we have. AFAIK the framework(run_script
>>> methodú doesn't support switching the log name which is printed in error
>>> message depending on usage. Therefore the universal was chosen -
>>> ipaserver-ca-install.log. It was introduced by your commit
>>> d27e77adc56f5a04f3bdd1aaed5440a89ed3acad
>>>
>>> And I see, that I used wrong ticket number in the commit. Correct is
>>> https://fedorahosted.org/freeipa/ticket/6086
>>>
>>> Proper solution might be to rework main and __main__ function in
>>> ipa-ca-install but IMO we spent too much time on this already.
>>>
>>
>> Updated patch attach - it uses the other log.
>>
>>
>>
> 
> Correct me if I'm wrong but the ticket URL should be
> https://fedorahosted.org/freeipa/ticket/6086
> 

you are absolutely right. Fixed.

-- 
Petr Vobornik
From 08b2ba61e2bc390f3c8a94dc88c5d1dc0ef19288 Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Fri, 15 Jul 2016 16:25:36 +0200
Subject: [PATCH] unite log file name of ipa-ca-install

ipa-ca-install said that it used
  /var/log/ipareplica-ca-install.log
but in fact it used
  /var/log/ipaserver-ca-install.log

This patch unites it to ipareplica-ca-install.log

It was chosen because of backwards compatibility - ipareplica-ca-install
was more commonly used. ipaserver-ca-install.log was used only in rare
CA less -> CA installation.

https://fedorahosted.org/freeipa/ticket/6086
---
 install/tools/ipa-ca-install | 2 +-
 ipaplatform/base/paths.py| 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index ed685920cbadb9cd3fc80865afb1610ca42f8b13..985e7413aa06900976934c329757ce45da5ff12d 100755
--- a/install/tools/ipa-ca-install
+++ b/install/tools/ipa-ca-install
@@ -285,7 +285,7 @@ def main():
 cainstance.is_ca_installed_locally()):
 sys.exit("CA is already installed on this host.")
 
-standard_logging_setup(paths.IPASERVER_CA_INSTALL_LOG, debug=options.debug)
+standard_logging_setup(log_file_name, debug=options.debug)
 root_logger.debug("%s was invoked with options: %s,%s",
   sys.argv[0], safe_options, filename)
 root_logger.debug("IPA version %s", version.VENDOR_VERSION)
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index d6fbe32f6839a5db40148777132ba1454cbc3382..1507ac36da5b40447c951ee608053a09b2db2fc3 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -307,7 +307,6 @@ class BasePathNamespace(object):
 IPAREPLICA_CONNCHECK_LOG = "/var/log/ipareplica-conncheck.log"
 IPAREPLICA_INSTALL_LOG = "/var/log/ipareplica-install.log"
 IPARESTORE_LOG = "/var/log/iparestore.log"
-IPASERVER_CA_INSTALL_LOG = "/var/log/ipaserver-ca-install.log"
 IPASERVER_INSTALL_LOG = "/var/log/ipaserver-install.log"
 IPASERVER_KRA_INSTALL_LOG = "/var/log/ipaserver-kra-install.log"
 IPASERVER_KRA_UNINSTALL_LOG = "/var/log/ipaserver-kra-uninstall.log"
-- 
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] 963 unite log file name of ipa-ca-install

2016-07-21 Thread Martin Babinsky


On 07/21/2016 05:22 PM, Petr Vobornik wrote:

On 07/19/2016 09:27 AM, Petr Vobornik wrote:

On 07/19/2016 08:01 AM, Jan Cholasta wrote:

Hi,

On 18.7.2016 18:50, Florence Blanc-Renaud wrote:

On 07/15/2016 04:29 PM, Petr Vobornik wrote:

ipa-ca-install said that it used
  /var/log/ipareplica-ca-install.log
but in fact it used
  /var/log/ipaserver-ca-install.log

This patch unites it to ipaserver-ca-install.log

It was chosen because ipa-ca-install can be also used on
master on CA-less -> CA conversion.

Term "server" is valid for both master and replica.

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





Looks good to me.
Ack


Does not look so good to me, "ipareplica-ca-install.log" is in fact the
original file name used since ipa-ca-install was introduced (in commit
8a32bb3746802a29b2655e4ad2cbbba8481e1eaf), so why the switch to
"ipaserver-ca-install.log"?

Honza



Ideally it would be ipa-ca-install.log but for backwards compatibility,
let's stick with one which we have. AFAIK the framework(run_script
methodú doesn't support switching the log name which is printed in error
message depending on usage. Therefore the universal was chosen -
ipaserver-ca-install.log. It was introduced by your commit
d27e77adc56f5a04f3bdd1aaed5440a89ed3acad

And I see, that I used wrong ticket number in the commit. Correct is
https://fedorahosted.org/freeipa/ticket/6086

Proper solution might be to rework main and __main__ function in
ipa-ca-install but IMO we spent too much time on this already.



Updated patch attach - it uses the other log.





Correct me if I'm wrong but the ticket URL should be 
https://fedorahosted.org/freeipa/ticket/6086


--
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] [DESIGN] Text-based rules for CSR autogeneration using Jinja2

2016-07-21 Thread Petr Spacek

On 20.7.2016 19:25, Ben Lipton wrote:
> On 07/20/2016 12:21 PM, Simo Sorce wrote:
>> On Wed, 2016-07-20 at 12:14 -0400, Ben Lipton wrote:
>>> On 07/20/2016 10:37 AM, Simo Sorce wrote:
 On Wed, 2016-07-20 at 10:17 -0400, Ben Lipton wrote:
> On 07/20/2016 06:27 AM, Simo Sorce wrote:
>> On Tue, 2016-07-19 at 16:20 -0400, Ben Lipton wrote:
>>> Hi,
>>>
>>> I have updated the design page
>>> http://www.freeipa.org/page/V4/Automatic_Certificate_Request_
>>> Gene
>>> rati
>>> on/Mapping_Rules
>>> with my plan for implementing user-configurable rules for
>>> mapping
>>> IPA
>>> data into certificate requests. In brief: we will use Jinja2
>>> for
>>> templating. Data rules (which map individual data items) and
>>> syntax
>>> rules (which group them into certificate fields) will both be
>>> snippets
>>> of Jinja2 markup. The formatting process will be as follows:

I've finally found some time to read the sub-page Mapping_Rules and for me it
is kind of hard to follow. It would not be understandable without this e-mail
and the blog posts (BTW the blog articles are among best I have seen!).

Most importantly, the explanations in brackets above ["Data rules (which map
individual data items) and (which group them into certificate fields)"] are
missing in the wiki page itself :-)

Could you fold relevant parts of the e-mails and blogs back into the wiki page
so it is self-contained?

Besides this nit,
http://www.freeipa.org/page/V4/Automatic_Certificate_Request_Generation/Mapping_Rules#Planned_implementation
sounds reasonable. I like how it prevents bad data from template-injection.

Regarding
http://www.freeipa.org/page/V4/Automatic_Certificate_Request_Generation/Schema#Option_A
, I prefer Option A with separate object for each helper. It is somehow
cleaner and it might be useful to use distinct object classes for each helper 
etc.


API for ipa cert-get-requestdata sounds good.
API for ipa cert-request makes sense to me as well.

In any case I would recommend you to consult API design with Jan Cholasta
 - he is our API custodian.


BTW I very much like "Alternatives considered" sections, we should have this
for each design!

Good work, I really like the dutiful analysis!



>>> 1. Syntax rules will be rendered using Jinja2. Data rules
>>> (rule
>>> text,
>>> not rendered) will be passed as the datarules attribute.
>>> 2. Rendered syntax rules will be processed by the Formatter
>>> class
>>> for
>>> the selected CSR generation helper (e.g. openssl or
>>> certutil).
>>> The
>>> formatter combines these partial rules into a full template
>>> for
>>> the
>>> config.
>>> 3. The template will be rendered using Jinja2. Relevant data
>>> from
>>> the
>>> IPA database will be available in the context for this
>>> rendering.
>>> 4. The final rendered template will be returned to the
>>> caller,
>>> labeled
>>> with its function (e.g. a command line or a config file).
>>>
>>> Are there any comments or objections to this approach? Here's
>>> an
>>> example
>>> to show what it might look like in practice.
>>>
>>> Example data rules:
>>> email={{subject.email}}
>>> O={{config.ipacertificatesubjectbase}}\nCN={{subject.username
>>> }}
>>>
>>> Example syntax rule:
>>> subjectAltName=@{% section %}{{datarules|join('\n')}}{%
>>> endsection %}
>>>
>>> Example composed config template:
>>> [ req ]
>>> prompt = no
>>> encrypt_key = no
>>>
>>> distinguished_name = {% section
>>> %}O={{config.ipacertificatesubjectbase}}
>>> CN={{subject.username}}{% endsection %}
>>>
>>> req_extensions = exts
>>>
>>> [ exts ]
>>> subjectAltName=@{% section %}email={{subject.email}}{%
>>> endsection
>>> %}
>>>
>>> There's a lot more information about the thinking behind this
>>> at
>>> http://blog.benjaminlipton.com/2016/07/19/csr-generation-temp
>>> lati
>>> ng.h
>>> tml
>>> if you're interested, as well.
>> Nice work Ben,
>> it's been really nice to be able to follow your notes on the
>> blog
>> post,
>> one question remains lingering in my head, why jinja2 ?
>> I know that engine relatively well as I used it in ipsilon, so
>> I am
>> not
>> questioning the choice just asking why specifically jinja2 and
>> not
>> something else, potentially language agnostic.
>>
>> Simo.
> Honestly, my reasoning didn't go very far beyond that it seems to
> be
> widely used and is compatible with python, which is the language
> where
> the implementation is taking place (in the IPA RPC server). I
> thought
> about using the built-in python format strings or creating a
> simple
> domain-specific language, but the likelihood of wanting the
> built-in

Re: [Freeipa-devel] [PATCH] 963 unite log file name of ipa-ca-install

2016-07-21 Thread Petr Vobornik

On 07/19/2016 09:27 AM, Petr Vobornik wrote:
> On 07/19/2016 08:01 AM, Jan Cholasta wrote:
>> Hi,
>>
>> On 18.7.2016 18:50, Florence Blanc-Renaud wrote:
>>> On 07/15/2016 04:29 PM, Petr Vobornik wrote:
 ipa-ca-install said that it used
   /var/log/ipareplica-ca-install.log
 but in fact it used
   /var/log/ipaserver-ca-install.log

 This patch unites it to ipaserver-ca-install.log

 It was chosen because ipa-ca-install can be also used on
 master on CA-less -> CA conversion.

 Term "server" is valid for both master and replica.

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



>>>
>>> Looks good to me.
>>> Ack
>>
>> Does not look so good to me, "ipareplica-ca-install.log" is in fact the
>> original file name used since ipa-ca-install was introduced (in commit
>> 8a32bb3746802a29b2655e4ad2cbbba8481e1eaf), so why the switch to
>> "ipaserver-ca-install.log"?
>>
>> Honza
>>
> 
> Ideally it would be ipa-ca-install.log but for backwards compatibility,
> let's stick with one which we have. AFAIK the framework(run_script
> methodú doesn't support switching the log name which is printed in error
> message depending on usage. Therefore the universal was chosen -
> ipaserver-ca-install.log. It was introduced by your commit
> d27e77adc56f5a04f3bdd1aaed5440a89ed3acad
> 
> And I see, that I used wrong ticket number in the commit. Correct is
> https://fedorahosted.org/freeipa/ticket/6086
> 
> Proper solution might be to rework main and __main__ function in
> ipa-ca-install but IMO we spent too much time on this already.
> 

Updated patch attach - it uses the other log.

-- 
Petr Vobornik
From 4a5904b726c11c7e3323de142e5d47a5b51cff88 Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Fri, 15 Jul 2016 16:25:36 +0200
Subject: [PATCH] unite log file name of ipa-ca-install

ipa-ca-install said that it used
  /var/log/ipareplica-ca-install.log
but in fact it used
  /var/log/ipaserver-ca-install.log

This patch unites it to ipareplica-ca-install.log

It was chosen because of backwards compatibility - ipareplica-ca-install
was more commonly used. ipaserver-ca-install.log was used only in rare
CA less -> CA installation.

https://fedorahosted.org/freeipa/ticket/6088
---
 install/tools/ipa-ca-install | 2 +-
 ipaplatform/base/paths.py| 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index ed685920cbadb9cd3fc80865afb1610ca42f8b13..985e7413aa06900976934c329757ce45da5ff12d 100755
--- a/install/tools/ipa-ca-install
+++ b/install/tools/ipa-ca-install
@@ -285,7 +285,7 @@ def main():
 cainstance.is_ca_installed_locally()):
 sys.exit("CA is already installed on this host.")
 
-standard_logging_setup(paths.IPASERVER_CA_INSTALL_LOG, debug=options.debug)
+standard_logging_setup(log_file_name, debug=options.debug)
 root_logger.debug("%s was invoked with options: %s,%s",
   sys.argv[0], safe_options, filename)
 root_logger.debug("IPA version %s", version.VENDOR_VERSION)
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index d6fbe32f6839a5db40148777132ba1454cbc3382..1507ac36da5b40447c951ee608053a09b2db2fc3 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -307,7 +307,6 @@ class BasePathNamespace(object):
 IPAREPLICA_CONNCHECK_LOG = "/var/log/ipareplica-conncheck.log"
 IPAREPLICA_INSTALL_LOG = "/var/log/ipareplica-install.log"
 IPARESTORE_LOG = "/var/log/iparestore.log"
-IPASERVER_CA_INSTALL_LOG = "/var/log/ipaserver-ca-install.log"
 IPASERVER_INSTALL_LOG = "/var/log/ipaserver-install.log"
 IPASERVER_KRA_INSTALL_LOG = "/var/log/ipaserver-kra-install.log"
 IPASERVER_KRA_UNINSTALL_LOG = "/var/log/ipaserver-kra-uninstall.log"
-- 
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 0001][Tests] Fix for dns_plugin tests

2016-07-21 Thread Martin Basti




On 21.07.2016 12:24, Ganna Kaihorodova wrote:

Hello!

Thank you for review and notes about patch format. I will keep it in mind.

I agree that this is not the best way to fix it and I was about to bring up 
topic of strange behavior of named/bind-dyndb-ldap.
Because without restart named service I have one number of failing tests and 
after restart different number. This is happening for me randomly, I never know 
when I need to restart service.



I really don't like using 'ipa-ca.' domain name as server name, this 
is not right.

Can you please, explain me why?
ipa-ca contains A records of IPA servers that are installed with CA 
subsystem. There is no guarantee that server is installed with CA, so 
ipa-ca record may not be there at all.

Also it does not look nice, because it is unrelated to CA.




So if it's not too much trouble for you, maybe you can give me some advice's 
how to investigate such kind of issues, at least some basic stuff, like what to 
check, where to look and etc.
It can be a letter or in a format of 1 hour meeting or presentation (I think it 
will be good to all members of our team to understand better how our dns plugin 
works and how to troubleshoot it)
And it seems like I will spend some time with tests for our dns-plugin, and It 
will be really great to know more about it.

Thank you and have a nice day!
It depends from case to case, I don't have any 'works for everything' 
solution. Usually, I run tests with --pdb option, that pause testing 
after each fail and I connect to machine and I try to find why.


Feel free to ask if you need help with something particular.

Martin^2
  


Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer


- Original Message -
From: "Martin Basti" 
To: "Ganna Kaihorodova" , freeipa-devel@redhat.com
Sent: Wednesday, July 20, 2016 4:11:08 PM
Subject: Re: [Freeipa-devel] [PATCH 0001][Tests] Fix for dns_plugin tests

Hello,


On 19.07.2016 17:45, Ganna Kaihorodova wrote:

Greetings!

Fix for ipatests/test_xmlrpc/test_dns_plugin.py 
(test_forwardzone_delegation_warnings.test)

You can't have a DNS zone with the authoritative nameserver that does not have 
a A or  record in the local DNS.

Not true


   Since in some test environments primary hostname of the master is managed by 
an external DNS, this hostname can not be used as a NS-record for IPA-managed 
zones. Therefore another existing fqdn corresponding to the master's ip and 
managed by IPA DNS was used.

I really don't like using 'ipa-ca.' domain name as server
name, this is not right.

I was digging deeper today to find what is the root case why it doesn't
work, and it looks that after some tests, named is not able to resolve
hostname, even if global forwarder is specified.
So this looks like bug on named/bind-dyndb-ldap side, because when I
restarted named-pkcs11 before the first test that is failing and all
forwardzone tests passed.

So I think this patch just hides the real issue and we should discard
it. I and pspacek will be continue investigating this tomorrow.

Notes to the patch format:

Please split that huge text in commit message to:

short summary (max 72 chars)
empty line
longer description
empty line
[ticket (if available)]

regars
Martin^2


Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer






--
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] 0211-0212 Make sure --raw option works for trust-add

2016-07-21 Thread Jan Cholasta


On 19.7.2016 11:32, Alexander Bokovoy wrote:

On Tue, 19 Jul 2016, Jan Cholasta wrote:

On 18.7.2016 12:06, Martin Babinsky wrote:

On 07/16/2016 12:50 PM, Alexander Bokovoy wrote:

Hi,


I had some time and was blocked by these bugs to do my tickets so I
actually fixed these three problems that are assigned to Martin
Babinsky. Hopefully, Martin wouldn't be offended by that. :)

Note that this fix (patch 0211) has potential for a break but also
introduces a correct behavior in my view as we should not really have
non-lower cased keys in LDAP dictionaries in entry_to_dict() for both
normal and --raw modes.

- 0211

Since commit a8dd7aa337f25abd938a582d0fcba51d3b356410 if IPA command
is called with --raw option, a retrieved LDAP entry's attribute
names aren't normalized to lower case when converting the entry
to a dictionary. This breaks overall assumption that dictionary
keys are in lower case.

Partially fixes 'ipa trust-add --raw' issues.

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

- 0212

Make sure we display raw values for 'trust-add --raw' case.

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






Hi Alexander,

I am CC'ing Jan since I hope he knows why
a8dd7aa337f25abd938a582d0fcba51d3b356410 was implemented in this way. I
think there was a reason behind this decision and we should not revert
it without further discussion.


I think it was just because with --raw, the raw LDAP entry should be
returned, letter case and all. I don't really care if the names are
lowercased or not, but you should never assume anything about raw
results in your code anyway, so I think the revert would be pointless.
There were a few bugs with --raw unrelated to letter case in the past
and most of the offending code was fixed, the same should be done here
IMO.

This is not about LDAP entry, this is about Python dictionary from
entry_to_dict(). LDAP attribute name is case-insensitive.

We call entry_to_dict() in multiple places and we expect that
dictionary keys are lowcased in Python code. I don't think it is fair to
have this assumption broken when --raw is used -- after all, we are
dealing with Python dictionary and LDAP attributes are case insensitive.


We don't expect anything about raw results anywhere in our code base, 
except for buggy code, which needs be fixed anyway.


Martin's patch fixes the issue without any intrusive changes, so ACK. We 
can talk bigger changes later for 4.5.


Pushed to master: 2234a774416309a44aecb84f27e6cf4c6a1a990f

--
Jan Cholasta

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

[Freeipa-devel] [PATCH 0194] harden the check for trust namespace overlap in new principals

2016-07-21 Thread Martin Babinsky

'*-add-principal' would crash with error if the trusted domains did not 
have any UPN suffixes or NETBIOS name associated with them. This patch 
fixes that.


Big thanks to Milan who found and reported the issue during writing 
tests for the feature.


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

--
Martin^3 Babinsky
From bb1b54a1d7432af719c6051b79b9afdef8e87c96 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Wed, 20 Jul 2016 15:46:22 +0200
Subject: [PATCH] harden the check for trust namespace overlap in new
 principals

This check must handle the possibility of optional attributes
(ipantadditionalsuffixes and ipantflatname) missing in the trusted domain
entry.

https://fedorahosted.org/freeipa/ticket/6099
---
 ipalib/util.py | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/ipalib/util.py b/ipalib/util.py
index 0cd5c091ec576e02e477f661bab981d12e01f1eb..805774006312e82c7acd4a46b8c9df2895a94ffe 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -975,11 +975,15 @@ def check_principal_realm_in_trust_namespace(api_instance, *keys):
 trust_suffix_namespace = set()
 
 for obj in trust_objects:
-trust_suffix_namespace.update(
-set(upn.lower() for upn in obj['ipantadditionalsuffixes']))
+nt_suffixes = obj.get('ipantadditionalsuffixes', [])
 
 trust_suffix_namespace.update(
-set((obj['cn'][0].lower(), obj['ipantflatname'][0].lower(
+set(upn.lower() for upn in nt_suffixes))
+
+if 'ipantflatname' in obj:
+trust_suffix_namespace.add(obj['ipantflatname'][0].lower())
+
+trust_suffix_namespace.add(obj['cn'][0].lower())
 
 for principal in keys[-1]:
 realm = principal.realm
-- 
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 0001][Tests] Fix for dns_plugin tests

2016-07-21 Thread Ganna Kaihorodova

Hello!

Thank you for review and notes about patch format. I will keep it in mind.

I agree that this is not the best way to fix it and I was about to bring up 
topic of strange behavior of named/bind-dyndb-ldap.
Because without restart named service I have one number of failing tests and 
after restart different number. This is happening for me randomly, I never know 
when I need to restart service.

> I really don't like using 'ipa-ca.' domain name as server name, 
> this is not right.
Can you please, explain me why?

So if it's not too much trouble for you, maybe you can give me some advice's 
how to investigate such kind of issues, at least some basic stuff, like what to 
check, where to look and etc. 
It can be a letter or in a format of 1 hour meeting or presentation (I think it 
will be good to all members of our team to understand better how our dns plugin 
works and how to troubleshoot it)
And it seems like I will spend some time with tests for our dns-plugin, and It 
will be really great to know more about it.

Thank you and have a nice day!

Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer

- Original Message -
From: "Martin Basti" 
To: "Ganna Kaihorodova" , freeipa-devel@redhat.com
Sent: Wednesday, July 20, 2016 4:11:08 PM
Subject: Re: [Freeipa-devel] [PATCH 0001][Tests] Fix for dns_plugin tests

Hello,

On 19.07.2016 17:45, Ganna Kaihorodova wrote:
> Greetings!
>
> Fix for ipatests/test_xmlrpc/test_dns_plugin.py 
> (test_forwardzone_delegation_warnings.test)
>
> You can't have a DNS zone with the authoritative nameserver that does not 
> have a A or  record in the local DNS.
Not true

>   Since in some test environments primary hostname of the master is managed 
> by an external DNS, this hostname can not be used as a NS-record for 
> IPA-managed zones. Therefore another existing fqdn corresponding to the 
> master's ip and managed by IPA DNS was used.
I really don't like using 'ipa-ca.' domain name as server 
name, this is not right.

I was digging deeper today to find what is the root case why it doesn't 
work, and it looks that after some tests, named is not able to resolve 
hostname, even if global forwarder is specified.
So this looks like bug on named/bind-dyndb-ldap side, because when I 
restarted named-pkcs11 before the first test that is failing and all 
forwardzone tests passed.

So I think this patch just hides the real issue and we should discard 
it. I and pspacek will be continue investigating this tomorrow.

Notes to the patch format:

Please split that huge text in commit message to:

short summary (max 72 chars)
empty line
longer description
empty line
[ticket (if available)]

regars
Martin^2
>
>
> Best regards,
> Ganna Kaihorodova
> Associate Software Quality Engineer
>
>
>
>

-- 
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 190] expose `--secret` option in radiusproxy-* commands

2016-07-21 Thread Martin Babinsky


On 07/20/2016 12:10 PM, Martin Babinsky wrote:

On 07/19/2016 12:32 PM, Jan Cholasta wrote:

Hi,

On 18.7.2016 13:51, Martin Babinsky wrote:

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


I don't think we want the secret searchable. Add a 'no_search' flag to
the param to fix that.

Honza



'no_search' flag breaks the API backwards compatibility, so I am sending
another two patches which fix handling of deprecated options in the
framework and deprecate `--secret` in radiusproxy-find command.

I hope this solution is the best.



After discussion with Jan we realized that it is enough to hide the 
'--secret' option from CLI, not deprecate it.


Re-sending patch 190 and updated 193.1. Patch 192 will be send in 
separate thread since the actual issue it fixes is orthogonal to this 
one and requires a separate ticket.


--
Martin^3 Babinsky
From 645b7ece72e902c9b108d41a5e71d7e88a48720f Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 18 Jul 2016 10:45:48 +0200
Subject: [PATCH] expose `--secret` option in radiusproxy-* commands

Option `--secret` was hidden from radiusproxy CLI preventing setting a secret
on existing server or searching by secret. Since thin client implementation it
was also not recognized by the interactive prompt code in CLI frontend since
it never got there.

https://fedorahosted.org/freeipa/ticket/6078
---
 ipaserver/plugins/radiusproxy.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/ipaserver/plugins/radiusproxy.py b/ipaserver/plugins/radiusproxy.py
index 44d87b9ae1337278bb6237d471f64693b0eac3db..5657e002c1ce66335b7697b98f95a49207c61d87 100644
--- a/ipaserver/plugins/radiusproxy.py
+++ b/ipaserver/plugins/radiusproxy.py
@@ -126,7 +126,6 @@ class radiusproxy(LDAPObject):
 label=_('Secret'),
 doc=_('The secret used to encrypt data'),
 confirm=True,
-flags=['no_option'],
 ),
 Int('ipatokenradiustimeout?',
 cli_name='timeout',
-- 
2.7.4

From 5542508919a0615b4088329ba80eb92002d45f0f Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Thu, 21 Jul 2016 09:42:01 +0200
Subject: [PATCH] prevent search for RADIUS proxy servers by secret

radiusproxy-find should not allow search by proxy secret even for privileged
users so we should hide it from CLI.

https://fedorahosted.org/freeipa/ticket/6078
---
 ipaserver/plugins/radiusproxy.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/ipaserver/plugins/radiusproxy.py b/ipaserver/plugins/radiusproxy.py
index 5657e002c1ce66335b7697b98f95a49207c61d87..3391b8aed77205fb1a586d5472d8cfdbc9fd1cd5 100644
--- a/ipaserver/plugins/radiusproxy.py
+++ b/ipaserver/plugins/radiusproxy.py
@@ -169,6 +169,14 @@ class radiusproxy_find(LDAPSearch):
 '%(count)d RADIUS proxy server matched', '%(count)d RADIUS proxy servers matched', 0
 )
 
+def get_options(self):
+for option in super(radiusproxy_find, self).get_options():
+if option.name == 'ipatokenradiussecret':
+option = option.clone(flags={'no_option'})
+
+yield option
+
+
 @register()
 class radiusproxy_show(LDAPRetrieve):
 __doc__ = _('Display information about a RADIUS proxy server.')
-- 
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 0112-7] Speeding up cli help

2016-07-21 Thread Jan Cholasta


Hi,

On 20.7.2016 14:32, David Kupka wrote:

On 15/07/16 12:53, David Kupka wrote:

Hello!

After Honza introduced thin client that builds plugins and commands
dynamically from schema client became much slower. This is only logical,
instead of importing a module client now must fetch the schema from
server, parse it and instantiate the commands using the data.

First step to speed it up was addition of schema cache to client. That
removed the RTT and download time of fetching schema every time.

Now the most time consuming task became displaying help for lists of
topics and command and displaying individual topics. This is simply
because of the need to instantiate all the commands to find the
relations between topics and commands.

All the necessary bits for server commands and topics are already in the
schema cache so we can skip this part and generate help from it, right?
Not so fast!

There are client plugins with commands and topics. So we can generate
basic bits (list of all topics, list of all commands, list of commands
for each topic) from schema and store it in cache. Then we need to go
through all client plugins and get similar bits for client plugins. Then
we can merge and print.

Still the client response is not as fast as before and I this it even
can't be. Also first time you display particular topic or list takes
longer because it must be freshly generated and stored in cache for next
use. And this is what the attached patches do.

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


Reimplemented so there is no need to distinguish client plugins and
remote plugins.
The main idea of this approach is to avoid creating instances of the
commands just to get the information about topic, name and summary
needed for displaying help. Instead class properties are used to access
the information directly in schema.


Patch 0112:

I think this would better be done in Schema.read_namespace_member, 
because Schema is where all the state is.


(BTW does _SchemaNameSpace.__getitem__ raise KeyError for non-existent 
keys? It looks like it doesn't.)



Patch 0113:

How about setting _schema_cached to False in Schema.__init__() rather 
that getattr()-ing it in _ensure_cached()?



Patch 0116:

ClientCommand.doc should be a class property as well, otherwise .summary 
won't work on it correctly.


_SchemaCommand.doc should not be a property, as it's not needed for 
.summary to work on it correctly.



Otherwise works fine for me.

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

[Freeipa-devel] [PATCH] 0095 cert-request: allow directoryName in SAN extension

Re: [Freeipa-devel] [PATCH] 0090, 0092..0094 cert-show: show subject alternative names

Re: [Freeipa-devel] [PATCH 0002][Tests] Small fix for dns_plugin tests

[Freeipa-devel] [PATCH 0556] host-del: fix behavior of --updatedns and PTR records

[Freeipa-devel] [PATCH 0555] AVC: use copy during instalation to keep SELinux context valid

[Freeipa-devel] [DRAFT] FreeIPA 4.3.2 release notes

Re: [Freeipa-devel] [PATCH] 963 unite log file name of ipa-ca-install

[Freeipa-devel] [PATCH] 0083: webui: remove full name column from user to user group adder dialog

Re: [Freeipa-devel] [PATCH] 963 unite log file name of ipa-ca-install

Re: [Freeipa-devel] [PATCH] 963 unite log file name of ipa-ca-install

Re: [Freeipa-devel] [DESIGN] Text-based rules for CSR autogeneration using Jinja2

Re: [Freeipa-devel] [PATCH] 963 unite log file name of ipa-ca-install

Re: [Freeipa-devel] [PATCH 0001][Tests] Fix for dns_plugin tests

Re: [Freeipa-devel] [PATCH] 0211-0212 Make sure --raw option works for trust-add

[Freeipa-devel] [PATCH 0194] harden the check for trust namespace overlap in new principals

Re: [Freeipa-devel] [PATCH 0001][Tests] Fix for dns_plugin tests

Re: [Freeipa-devel] [PATCH 190] expose `--secret` option in radiusproxy-* commands

Re: [Freeipa-devel] [PATCH 0112-7] Speeding up cli help

18 matches

Site Navigation

Mail list logo

Footer information