Re: [Freeipa-devel] [PATCH] 369 Added CLI param and ACL for vault service operations.
On 08/03/2015 10:37 PM, Endi Sukma Dewata wrote:
> On 8/3/2015 2:47 PM, Martin Kosek wrote:
>> On 08/03/2015 05:36 PM, Endi Sukma Dewata wrote:
>>> On 8/3/2015 2:31 AM, Martin Kosek wrote:
On 07/31/2015 05:07 PM, Endi Sukma Dewata wrote:
> The CLIs to manage vault owners and members have been modified
> to accept services with a new parameter. Due to name conflict,
> the existing 'service' parameter has been renamed to
> 'servicename'.
>
> A new ACL has been added to allow a service to create its own
> service container.
>
> https://fedorahosted.org/freeipa/ticket/5172
I did not do a full review, I just saw some of the changes that made
me worry -
like renaming API command attribute. Wouldn't that make the older
clients fail?
See for example permission-* commands, we faced similar situation
there and had
to rename command attributes potentially used by old clients in the
new format.
>>>
>>> Yes, it will break older clients. The problem is I cannot keep the legacy
>>> 'service' parameter for backward compatibility:
>>>
>>> Str(
>>> 'service?',
>>> cli_name='deprecated_service',
>>> doc=_('DEPRECATED: Service name of the service vault'),
>>> ),
>>> Str(
>>> 'servicename?',
>>> cli_name='service',
>>> doc=_('Service name of the service vault'),
>>> ),
>>>
>>> because it will conflict with a new 'service' parameter:
>>>
>>> [Mon Aug 03 17:19:00.197040 2015] [wsgi:error] [pid 9409] ipa: ERROR:
>>> Failed to
>>> start IPA: cannot override NameSpace.service value Str('service?',
>>> cli_name='deprecated_service', doc=Gettext('DEPRECATED: Service name
>>> of the
>>> service vault', domain='ipa', localedir=None)) with Str('service*',
>>> alwaysask=True, cli_name='services', csv=True, doc=u'services to add',
>>> label=u'member service')
>>>
>>> that was added automatically when I added the 'service' attribute member:
>>>
>>> attribute_members = {
>>> 'owner': ['user', 'group', 'service'],
>>> 'member': ['user', 'group', 'service'],
>>> }
>>>
>>> If there's a way to use a different parameter name for the 'service'
>>> attribute
>>> member to avoid conflict with the legacy 'service' parameter please
>>> let me know.
>>>
>>> The other option is to force the client to upgrade to the same version.
>>>
>>> Please let me know. Thanks.
>>>
>>
>> Honza, do we have any other options than to break API between 4.2.0 and
>> 4.2.1?
>
> Another option is to keep 2 vault plugins. The old plugin (1.0) will handle
> old
> IPA 4.2.0 clients. The new plugin (1.1) will handle the new IPA 4.2.1 clients.
> For this to work the plugins need to have different API URLs so they won't
> conflict/confuse the clients.
We do not have that versions in FreeIPA (yet?).
> Please also note that my next patch that adds the ability to change vault
> type,
> password, and keys will also require a client upgrade because the
> functionality
> is mainly implemented on the client side. In this case API URL versioning will
> be necessary.
Adding new commands and/or attributes is a common thing in FreeIPA. We just do
the work, bump the minor API version and that's it. We planned having better
version support in FreeIPA 4.4, we will see how it goes.
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] 0030 Add permission for bypassing CA ACL enforcement
The attached patch fixes
https://fedorahosted.org/freeipa/ticket/5099.
Thanks,
Fraser
From 294205795f595095f14eecb451f974cbf867ebe3 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale
Date: Tue, 4 Aug 2015 01:13:09 -0400
Subject: [PATCH] Add permission for bypassing CA ACL enforcement
Add the "Request Certificate ignoring CA ACLs" permission and
associated ACI, initially assigned to "Certificate Administrators"
privilege.
Update cert-request command to skip CA ACL enforcement when the bind
principal has this permission.
Fixes: https://fedorahosted.org/freeipa/ticket/5099
---
install/updates/40-delegation.update | 15 +++
ipalib/plugins/cert.py | 13 ++---
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/install/updates/40-delegation.update
b/install/updates/40-delegation.update
index
bc0736c5b6c07747586a56c2cbde9596c7522d1c..8d4f6296cbed7fcc968c2193022cb50b488c8561
100644
--- a/install/updates/40-delegation.update
+++ b/install/updates/40-delegation.update
@@ -144,6 +144,21 @@ default:member: cn=Certificate
Administrators,cn=privileges,cn=pbac,$SUFFIX
dn: $SUFFIX
add:aci:(targetattr = "objectclass")(target = "ldap:///cn=request certificate
with subjectaltname,cn=virtual operations,cn=etc,$SUFFIX" )(version 3.0; acl
"permission:Request Certificate with SubjectAltName"; allow (write) groupdn =
"ldap:///cn=Request Certificate with
SubjectAltName,cn=permissions,cn=pbac,$SUFFIX";)
+dn: cn=request certificate ignore caacl,cn=virtual operations,cn=etc,$SUFFIX
+default:objectClass: top
+default:objectClass: nsContainer
+default:cn: request certificate ignore caacl
+
+dn: cn=Request Certificate ignoring CA ACLs,cn=permissions,cn=pbac,$SUFFIX
+default:objectClass: top
+default:objectClass: groupofnames
+default:objectClass: ipapermission
+default:cn: Request Certificate ignoring CA ACLs
+default:member: cn=Certificate Administrators,cn=privileges,cn=pbac,$SUFFIX
+
+dn: $SUFFIX
+add:aci:(targetattr = "objectclass")(target = "ldap:///cn=request certificate
ignore caacl,cn=virtual operations,cn=etc,$SUFFIX" )(version 3.0; acl
"permission:Request Certificate ignoring CA ACLs"; allow (write) groupdn =
"ldap:///cn=Request Certificate ignoring CA
ACLs,cn=permissions,cn=pbac,$SUFFIX";)
+
# Read privileges
dn: cn=RBAC Readers,cn=privileges,cn=pbac,$SUFFIX
diff --git a/ipalib/plugins/cert.py b/ipalib/plugins/cert.py
index
341bdd01766d50ba18ce7147d4408851e6f95487..8c06a9269d00d9bb4095944f965f942b8384aa0f
100644
--- a/ipalib/plugins/cert.py
+++ b/ipalib/plugins/cert.py
@@ -344,8 +344,6 @@ class cert_request(VirtualCommand):
else:
principal_type = SERVICE
-caacl_check(principal_type, principal_string, ca, profile_id)
-
bind_principal = split_any_principal(getattr(context, 'principal'))
bind_service, bind_name, bind_realm = bind_principal
@@ -361,6 +359,15 @@ class cert_request(VirtualCommand):
self.check_access()
try:
+self.check_access("request certificate ignore caacl")
+bypass_caacl = True
+except errors.ACIError:
+bypass_caacl = False
+
+if not bypass_caacl:
+caacl_check(principal_type, principal_string, ca, profile_id)
+
+try:
subject = pkcs10.get_subject(csr)
extensions = pkcs10.get_extensions(csr)
subjectaltname = pkcs10.get_subjectaltname(csr) or ()
@@ -468,7 +475,7 @@ class cert_request(VirtualCommand):
raise errors.ACIError(info=_(
"Insufficient privilege to create a certificate "
"with subject alt name '%s'.") % name)
-if alt_principal_string is not None:
+if alt_principal_string is not None and not bypass_caacl:
caacl_check(
principal_type, alt_principal_string, ca, profile_id)
elif name_type in (pkcs10.SAN_OTHERNAME_KRB5PRINCIPALNAME,
--
2.4.3
--
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] 371 Added support for changing vault encryption.
The vault-mod command has been modified to support changing vault
encryption attributes (i.e. type, password, public/private keys)
in addition to normal attributes (i.e. description). Changing the
encryption requires retrieving the stored secret with the old
attributes and rearchieving it with the new attributes.
https://fedorahosted.org/freeipa/ticket/5176
--
Endi S. Dewata
From e80928fa8e8a099576fcdbff08fd90a634600825 Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata"
Date: Fri, 31 Jul 2015 07:53:15 +0200
Subject: [PATCH] Added support for changing vault encryption.
The vault-mod command has been modified to support changing vault
encryption attributes (i.e. type, password, public/private keys)
in addition to normal attributes (i.e. description). Changing the
encryption requires retrieving the stored secret with the old
attributes and rearchieving it with the new attributes.
https://fedorahosted.org/freeipa/ticket/5176
---
API.txt | 27 +-
VERSION | 4 +-
ipalib/plugins/vault.py | 239
3 files changed, 248 insertions(+), 22 deletions(-)
diff --git a/API.txt b/API.txt
index
00b47b8709c81217d7f2a69e719093f4a04f1734..bd5c48998056ab94729cb1b475bf444707a883cb
100644
--- a/API.txt
+++ b/API.txt
@@ -5466,11 +5466,12 @@ output: Output('completed', , None)
output: Output('failed', , None)
output: Entry('result', , Gettext('A dictionary representing an
LDAP entry', domain='ipa', localedir=None))
command: vault_archive
-args: 1,10,3
+args: 1,11,3
arg: Str('cn', attribute=True, cli_name='name', maxlength=255,
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True,
required=True)
option: Flag('all', autofill=True, cli_name='all', default=False,
exclude='webui')
option: Bytes('data?')
option: Str('in?')
+option: Flag('override_password?', autofill=True, default=False)
option: Str('password?', cli_name='password')
option: Str('password_file?', cli_name='password_file')
option: Flag('raw', autofill=True, cli_name='raw', default=False,
exclude='webui')
@@ -5528,6 +5529,30 @@ output: ListOfEntries('result', (, ), Gettext('A list
output: Output('summary', (, ), None)
output: Output('truncated', , None)
command: vault_mod
+args: 1,18,3
+arg: Str('cn', attribute=True, cli_name='name', maxlength=255,
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True,
required=True)
+option: Flag('all', autofill=True, cli_name='all', default=False,
exclude='webui')
+option: Flag('change_password?', autofill=True, default=False)
+option: Str('description?', cli_name='desc')
+option: Bytes('ipavaultsalt?', cli_name='salt')
+option: Str('ipavaulttype?', cli_name='type')
+option: Str('new_password?', cli_name='new_password')
+option: Str('new_password_file?', cli_name='new_password_file')
+option: Str('old_password?', cli_name='old_password')
+option: Str('old_password_file?', cli_name='old_password_file')
+option: Bytes('private_key?', cli_name='private_key')
+option: Str('private_key_file?', cli_name='private_key_file')
+option: Bytes('public_key?', cli_name='public_key')
+option: Str('public_key_file?', cli_name='public_key_file')
+option: Flag('raw', autofill=True, cli_name='raw', default=False,
exclude='webui')
+option: Str('servicename?', cli_name='service')
+option: Flag('shared?', autofill=True, default=False)
+option: Str('username?', cli_name='user')
+option: Str('version?', exclude='webui')
+output: Entry('result', , Gettext('A dictionary representing an
LDAP entry', domain='ipa', localedir=None))
+output: Output('summary', (, ), None)
+output: PrimaryKey('value', None, None)
+command: vault_mod_internal
args: 1,15,3
arg: Str('cn', attribute=True, cli_name='name', maxlength=255,
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True,
required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
diff --git a/VERSION b/VERSION
index
c42bea06522dae55e1a89ff94ae394594086b467..feb9f4db92c7c7b95e9e5d5907b1f97e96b26886
100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=149
-# Last change: edewata - Added CLI param and ACL for vault service operations
+IPA_API_VERSION_MINOR=150
+# Last change: edewata - Added support for changing vault encryption.
diff --git a/ipalib/plugins/vault.py b/ipalib/plugins/vault.py
index
3b62822366a62c90f843a6293589c28383e782ef..5bee6aaae8ddd306d4ee0c273143c9d0ffc913d5
100644
--- a/ipalib/plugins/vault.py
+++ b/ipalib/plugins/vault.py
@@ -133,19 +133,36 @@ EXAMPLES:
ipa vault-show --shared
""") + _("""
Show a user vault:
- ipa vault-show --user
+ ipa vault-show
""") + _("""
- Modify a private vault:
- ipa vault-mod --desc
+ Modify vault description:
+ ipa vault-mod
+ [--user |--service |--
[Freeipa-devel] [PATCHES] Replica Promotion #2888
Hello freeipa-devel,
this patcheset implement the main piece of the replica promotion
feature.
It first introduces the custodia modules, custodia is a service that
allows to securely transfer secrets between FreeIPA instances, using
asymetric crypto and LDAP published keys to insure confidentiality.
These patches intentionally duplicate some code in the installer in
order to avoid regression in the "classic" installer code path, in the
hope that the promotion functionality will not unintentionally break the
classic prepare/install code paths.
To use test this patchset you need the jwcrypto and custodia python
packages. Jwcrypto ins in fedora rawhide already (built today for f22
too) and Custodia is under review. I prepared two copr repositories for
now so people can build.
Use dnf copr enable simo/jwcrypto and dnf copr enable simo/custodia on
your devel VMs to get the proper packages (dnf install custodia will
suffice to drag in all dependencies).
To test do NOT follow the usual path of creating a replica file on the
master server with the ipa-replica-prepare tool.
Instead prepare a machine and run:
ipa-client-install
ipa-replica-install --promote
That should be it.
You can optionally test the --setup-dns install option, but --setup-ca
and --seyup-kra do not work yet.
If you kinit admin right after the client install, you'll be asked no
passwords.
Note that you need to raise the domain level to 1 before you can use the
replica promotion code as it is intended to be used with the topology
plugin activated.
This patchset depends on the previous one sent last week.
Cheers,
Simo.
--
Simo Sorce * Red Hat, Inc * New York
>From 5087b3dc3b3cf09613e5c516561b800cd1f73f76 Mon Sep 17 00:00:00 2001
From: Simo Sorce
Date: Fri, 8 May 2015 13:39:29 -0400
Subject: [PATCH 1/6] IPA Custodia Daemon
IPA specific plugins and configuration for using Custodia as a key
transfer mechanism.
Signed-off-by: Simo Sorce
---
daemons/ipa-custodia/Makefile| 27
daemons/ipa-custodia/ipakeys/__init__.py | 0
daemons/ipa-custodia/ipakeys/common.py | 45 +++
daemons/ipa-custodia/ipakeys/kem.py | 203 +++
daemons/ipa-custodia/ipakeys/store.py| 197 ++
daemons/ipa-custodia/ipakeys/tests.py| 55 +
daemons/ipa-custodia/setup.py| 16 +++
install/updates/73-custodia.update | 4 +
ipaplatform/base/paths.py| 1 +
9 files changed, 548 insertions(+)
create mode 100644 daemons/ipa-custodia/Makefile
create mode 100644 daemons/ipa-custodia/ipakeys/__init__.py
create mode 100644 daemons/ipa-custodia/ipakeys/common.py
create mode 100644 daemons/ipa-custodia/ipakeys/kem.py
create mode 100644 daemons/ipa-custodia/ipakeys/store.py
create mode 100644 daemons/ipa-custodia/ipakeys/tests.py
create mode 100755 daemons/ipa-custodia/setup.py
create mode 100644 install/updates/73-custodia.update
diff --git a/daemons/ipa-custodia/Makefile b/daemons/ipa-custodia/Makefile
new file mode 100644
index ..fbe148103f0731ee40717b71b32be6dd074289df
--- /dev/null
+++ b/daemons/ipa-custodia/Makefile
@@ -0,0 +1,27 @@
+all: lint pep8 test
+ echo "All tests passed"
+
+lint:
+ # Analyze code
+ # don't show recommendations, info, comments, report
+ # W0613 - unused argument
+ # Ignore cherrypy class members as they are dynamically added
+ pylint -d c,r,i,W0613 -r n -f colorized \
+ --notes= \
+ --disable=star-args \
+ ./ipakeys
+
+pep8:
+ # Check style consistency
+ pep8 ipakeys
+
+clean:
+ rm -fr build dist *.egg-info
+ find ./ -name '*.pyc' -exec rm -f {} \;
+
+cscope:
+ git ls-files | xargs pycscope
+
+test:
+ rm -f .coverage
+ nosetests -s
diff --git a/daemons/ipa-custodia/ipakeys/__init__.py b/daemons/ipa-custodia/ipakeys/__init__.py
new file mode 100644
index ..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
diff --git a/daemons/ipa-custodia/ipakeys/common.py b/daemons/ipa-custodia/ipakeys/common.py
new file mode 100644
index ..caed9a5e7481e561b8032e65981a242d84b57c1a
--- /dev/null
+++ b/daemons/ipa-custodia/ipakeys/common.py
@@ -0,0 +1,45 @@
+# Copyright (C) 2015 IPA Project Contributors, see COPYING for license
+from __future__ import print_function
+import ldap
+import ldap.sasl
+import ldap.filter
+
+
+class IKLdap(object):
+
+def __init__(self, uri, auth_type=None):
+self.uri = uri
+if auth_type is not None:
+self.auth_type = auth_type
+else:
+if uri.startswith('ldapi'):
+self.auth_type = 'EXTERNAL'
+else:
+self.auth_type = 'GSSAPI'
+self._basedn = None
+
+@property
+def basedn(self):
+if self._basedn is None:
+conn = self.connect()
+r = conn.search_s('', ldap.SCOPE_BASE)
+self._basedn = r[0][1]['defaultnamingcontext'][0]
+
Re: [Freeipa-devel] [PATCH] Port from python-kerberos library to python-gssapi
On Mon, 2015-08-03 at 23:56 +0200, Michael Šimáček wrote: > On 2015-07-27 11:38, Simo Sorce wrote: > > On Sun, 2015-07-26 at 21:51 +0200, Michael Šimáček wrote: > >> It would probably be nicer to do the full cycle, but I'd like to > >> avoid > >> changes in behavior when porting from one library to another. And the > >> code above doesn't actually hold any connection, so it would require > >> more refactoring to make that happen. For now I would follow what the > >> original code was doing. As for the exceptions, I think it would > >> actually be justifiable to use the raw api's init_sec_context, > >> because > >> the high level api would just do the same call + the exception > >> handling > >> magic, which we want to avoid for now. Please let me know what do you > >> think. > >> Attaching updated patch that uses 'unicode' instead of > >> raw.display_name > >> and reverts back to using init_sec_context. > > > > Sorry, > > but we should really not use the raw API here. > > If it means more changes to the code, so be it, please us the high level > > API as recommended by Robbie, we wrote a better API so that people would > > use it, and we want to apply best practices when changing code in IPA. > > > > Attaching new revision of the patch that performs the full negotiation > cycle. > > Michael LGTM Thanks! -- 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] Port from python-kerberos library to python-gssapi
On 2015-07-27 11:38, Simo Sorce wrote: On Sun, 2015-07-26 at 21:51 +0200, Michael Šimáček wrote: It would probably be nicer to do the full cycle, but I'd like to avoid changes in behavior when porting from one library to another. And the code above doesn't actually hold any connection, so it would require more refactoring to make that happen. For now I would follow what the original code was doing. As for the exceptions, I think it would actually be justifiable to use the raw api's init_sec_context, because the high level api would just do the same call + the exception handling magic, which we want to avoid for now. Please let me know what do you think. Attaching updated patch that uses 'unicode' instead of raw.display_name and reverts back to using init_sec_context. Sorry, but we should really not use the raw API here. If it means more changes to the code, so be it, please us the high level API as recommended by Robbie, we wrote a better API so that people would use it, and we want to apply best practices when changing code in IPA. Attaching new revision of the patch that performs the full negotiation cycle. Michael From 9cd3f604ba4c2a8ccc116296ed9c4a5b4b2075fe Mon Sep 17 00:00:00 2001 From: Michael Simacek Date: Thu, 16 Jul 2015 18:22:00 +0200 Subject: [PATCH] Port from python-kerberos to python-gssapi kerberos library doesn't support Python 3 and probably never will. python-gssapi library is Python 3 compatible. https://fedorahosted.org/freeipa/ticket/5147 --- BUILD.txt| 2 +- freeipa.spec.in | 4 +- ipalib/rpc.py| 112 +++ ipalib/util.py | 13 +++--- ipapython/ipautil.py | 17 5 files changed, 95 insertions(+), 53 deletions(-) diff --git a/BUILD.txt b/BUILD.txt index 6a28beba1e0844971fb5625c0e1adf3f0c0fc0e3..53012b14d05673d4fbc4d0567e877348d5e78444 100644 --- a/BUILD.txt +++ b/BUILD.txt @@ -20,7 +20,7 @@ systemd-units samba-devel samba-python libwbclient-devel libtalloc-devel \ libtevent-devel nspr-devel nss-devel openssl-devel openldap-devel krb5-devel \ krb5-workstation libuuid-devel libcurl-devel xmlrpc-c-devel popt-devel \ autoconf automake m4 libtool gettext python-devel python-ldap \ -python-setuptools python-krbV python-nss python-netaddr python-kerberos \ +python-setuptools python-krbV python-nss python-netaddr python-gssapi \ python-rhsm pyOpenSSL pylint python-polib libipa_hbac-python python-memcached \ sssd python-lxml python-pyasn1 python-qrcode-core python-dns m2crypto \ check libsss_idmap-devel libsss_nss_idmap-devel java-headless rhino \ diff --git a/freeipa.spec.in b/freeipa.spec.in index 0351952c692eb0cee2148053462c50b6d9073b5d..57d3d26e94aab6267143793943268175ed440586 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -72,7 +72,7 @@ BuildRequires: python-krbV BuildRequires: python-nss BuildRequires: python-cryptography BuildRequires: python-netaddr -BuildRequires: python-kerberos >= 1.1-14 +BuildRequires: python-gssapi >= 1.1.1 BuildRequires: python-rhsm BuildRequires: pyOpenSSL BuildRequires: pylint >= 1.0 @@ -303,7 +303,7 @@ IPA administrators. %package python Summary: Python libraries used by IPA Group: System Environment/Libraries -Requires: python-kerberos >= 1.1-14 +Requires: python-gssapi >= 1.1.1 Requires: gnupg Requires: iproute Requires: keyutils diff --git a/ipalib/rpc.py b/ipalib/rpc.py index 466b49a6dd60370db4d588389acba8dcaa493aa1..4176bbd283da709b60844bdb38651af97ea8f48f 100644 --- a/ipalib/rpc.py +++ b/ipalib/rpc.py @@ -44,7 +44,7 @@ from urllib2 import urlparse from xmlrpclib import (Binary, Fault, DateTime, dumps, loads, ServerProxy, Transport, ProtocolError, MININT, MAXINT) -import kerberos +import gssapi from dns import resolver, rdatatype from dns.exception import DNSException from nss.error import NSPRError @@ -510,24 +510,32 @@ class KerbTransport(SSLTransport): """ Handles Kerberos Negotiation authentication to an XML-RPC server. """ -flags = kerberos.GSS_C_MUTUAL_FLAG | kerberos.GSS_C_SEQUENCE_FLAG +flags = [gssapi.RequirementFlag.mutual_authentication, + gssapi.RequirementFlag.out_of_sequence_detection] + +def __init__(self, *args, **kwargs): +SSLTransport.__init__(self, *args, **kwargs) +self._sec_context = None def _handle_exception(self, e, service=None): -(major, minor) = ipautil.get_gsserror(e) -if minor[1] == KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN: +# kerberos library coerced error codes to signed, gssapi uses unsigned +minor = e.min_code +if minor & (1 << 31): +minor -= 1 << 32 +if minor == KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN: raise errors.ServiceError(service=service) -elif minor[1] == KRB5_FCC_NOFILE: +elif minor == KRB5_FCC_NOFILE: raise errors.NoCCacheError() -elif minor[1] == KRB5KRB_AP_ERR_TKT_EXPIRED: +elif minor == KRB5KRB_AP_
Re: [Freeipa-devel] [PATCH] 369 Added CLI param and ACL for vault service operations.
On 8/3/2015 2:47 PM, Martin Kosek wrote:
On 08/03/2015 05:36 PM, Endi Sukma Dewata wrote:
On 8/3/2015 2:31 AM, Martin Kosek wrote:
On 07/31/2015 05:07 PM, Endi Sukma Dewata wrote:
The CLIs to manage vault owners and members have been modified
to accept services with a new parameter. Due to name conflict,
the existing 'service' parameter has been renamed to
'servicename'.
A new ACL has been added to allow a service to create its own
service container.
https://fedorahosted.org/freeipa/ticket/5172
I did not do a full review, I just saw some of the changes that made
me worry -
like renaming API command attribute. Wouldn't that make the older
clients fail?
See for example permission-* commands, we faced similar situation
there and had
to rename command attributes potentially used by old clients in the
new format.
Yes, it will break older clients. The problem is I cannot keep the legacy
'service' parameter for backward compatibility:
Str(
'service?',
cli_name='deprecated_service',
doc=_('DEPRECATED: Service name of the service vault'),
),
Str(
'servicename?',
cli_name='service',
doc=_('Service name of the service vault'),
),
because it will conflict with a new 'service' parameter:
[Mon Aug 03 17:19:00.197040 2015] [wsgi:error] [pid 9409] ipa: ERROR:
Failed to
start IPA: cannot override NameSpace.service value Str('service?',
cli_name='deprecated_service', doc=Gettext('DEPRECATED: Service name
of the
service vault', domain='ipa', localedir=None)) with Str('service*',
alwaysask=True, cli_name='services', csv=True, doc=u'services to add',
label=u'member service')
that was added automatically when I added the 'service' attribute member:
attribute_members = {
'owner': ['user', 'group', 'service'],
'member': ['user', 'group', 'service'],
}
If there's a way to use a different parameter name for the 'service'
attribute
member to avoid conflict with the legacy 'service' parameter please
let me know.
The other option is to force the client to upgrade to the same version.
Please let me know. Thanks.
Honza, do we have any other options than to break API between 4.2.0 and
4.2.1?
Another option is to keep 2 vault plugins. The old plugin (1.0) will
handle old IPA 4.2.0 clients. The new plugin (1.1) will handle the new
IPA 4.2.1 clients. For this to work the plugins need to have different
API URLs so they won't conflict/confuse the clients.
Please also note that my next patch that adds the ability to change
vault type, password, and keys will also require a client upgrade
because the functionality is mainly implemented on the client side. In
this case API URL versioning will be necessary.
--
Endi S. Dewata
--
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] 369 Added CLI param and ACL for vault service operations.
On 08/03/2015 05:36 PM, Endi Sukma Dewata wrote:
On 8/3/2015 2:31 AM, Martin Kosek wrote:
On 07/31/2015 05:07 PM, Endi Sukma Dewata wrote:
The CLIs to manage vault owners and members have been modified
to accept services with a new parameter. Due to name conflict,
the existing 'service' parameter has been renamed to
'servicename'.
A new ACL has been added to allow a service to create its own
service container.
https://fedorahosted.org/freeipa/ticket/5172
I did not do a full review, I just saw some of the changes that made me worry -
like renaming API command attribute. Wouldn't that make the older clients fail?
See for example permission-* commands, we faced similar situation there and had
to rename command attributes potentially used by old clients in the new format.
Yes, it will break older clients. The problem is I cannot keep the legacy
'service' parameter for backward compatibility:
Str(
'service?',
cli_name='deprecated_service',
doc=_('DEPRECATED: Service name of the service vault'),
),
Str(
'servicename?',
cli_name='service',
doc=_('Service name of the service vault'),
),
because it will conflict with a new 'service' parameter:
[Mon Aug 03 17:19:00.197040 2015] [wsgi:error] [pid 9409] ipa: ERROR: Failed to
start IPA: cannot override NameSpace.service value Str('service?',
cli_name='deprecated_service', doc=Gettext('DEPRECATED: Service name of the
service vault', domain='ipa', localedir=None)) with Str('service*',
alwaysask=True, cli_name='services', csv=True, doc=u'services to add',
label=u'member service')
that was added automatically when I added the 'service' attribute member:
attribute_members = {
'owner': ['user', 'group', 'service'],
'member': ['user', 'group', 'service'],
}
If there's a way to use a different parameter name for the 'service' attribute
member to avoid conflict with the legacy 'service' parameter please let me know.
The other option is to force the client to upgrade to the same version.
Please let me know. Thanks.
Honza, do we have any other options than to break API between 4.2.0 and 4.2.1?
--
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 0052] store user certificates in 'userCertificate; binary' attributes
On 08/03/2015 03:39 PM, Jan Cholasta wrote:
Dne 3.8.2015 v 14:58 Martin Babinsky napsal(a):
On 08/03/2015 02:46 PM, Jan Cholasta wrote:
Dne 3.8.2015 v 14:14 Jan Cholasta napsal(a):
Hi,
Dne 3.8.2015 v 14:00 Martin Babinsky napsal(a):
This patch fixes the inconsistency between storing certificates in
'userCertificate'/'userCertificate;binary' attribute for the user
entries: the certificate must be stored in the latter attribute only.
Since a more general fix is out of 4.2.1 scope, I have implemented
some
workarounds in pre/post callbacks of user-* commands in order to
enforce
this behavior.
1)
+def convert_usercertificate_pre(self, entry_attrs, **options):
+if options.get('all', False):
+return
We don't want to do any renaming when --raw is specified, not --all.
Same for convert_usercertificate_post.
Actually, the attribute should be always renamed in
convert_usercertificate_pre, otherwise we would modify the wrong
attribute. In convert_usercertificate_post, it should actually be
renamed only when --raw is specified.
If you do the rename in `convert_usercertificate_post` only when '--raw'
is specified, then you get no certificate displayed when you do `ipa
user-show` on user with userCertificate;binary attribute. Is this
intended? (Keep in mind that `convert_usercertificate_post` should be
called in post-callback when returning results back to user/client).
Oops, I meant "rename only when --raw is *not* specified".
2)
+self.obj.convert_usercertificate_pre(entry_attrs, **options)
Rather than calling this directly from user_add, this should be called
from baseuser.pre_common_callback(), which should be called from
user_add.post_callback().
3) IMO you should change user_{add,remove}_cert to call
baseuser.convert_usercertificate_{pre,post} as well, to avoid code
duplication.
Honza
Attaching updated patch.
--
Martin^3 Babinsky
From 76b3dc3fcd213ff8b6e16122f5c9220d52bbbd11 Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Mon, 3 Aug 2015 13:36:29 +0200
Subject: [PATCH] store certificates issued for user entries as
userCertificate;binary
This patch forces the user management CLI command to store certificates as
userCertificate;binary attribute. The code to retrieve of user information was
modified to enable outputting of userCertificate;binary attribute to the
command line.
---
ipalib/plugins/baseuser.py | 24 +++-
ipalib/plugins/user.py | 21 +
2 files changed, 32 insertions(+), 13 deletions(-)
diff --git a/ipalib/plugins/baseuser.py b/ipalib/plugins/baseuser.py
index bd66cf5a3e3a4e6c18d1a54408f969668c834fab..bce427c02774f9dde2fe7d0925d04124add0daa4 100644
--- a/ipalib/plugins/baseuser.py
+++ b/ipalib/plugins/baseuser.py
@@ -187,7 +187,7 @@ class baseuser(LDAPObject):
'telephonenumber', 'title', 'memberof', 'nsaccountlock',
'memberofindirect', 'ipauserauthtype', 'userclass',
'ipatokenradiusconfiglink', 'ipatokenradiususername',
-'krbprincipalexpiration', 'usercertificate',
+'krbprincipalexpiration', 'usercertificate;binary',
]
search_display_attributes = [
'uid', 'givenname', 'sn', 'homedirectory', 'loginshell',
@@ -465,10 +465,28 @@ class baseuser(LDAPObject):
assert isinstance(user, DN)
return self._user_status(user, DN(self.delete_container_dn, api.env.basedn))
+def convert_usercertificate_pre(self, entry_attrs):
+if 'usercertificate' in entry_attrs:
+entry_attrs['usercertificate;binary'] = entry_attrs.pop(
+'usercertificate')
+
+def convert_usercertificate_post(self, entry_attrs, **options):
+if ('usercertificate;binary' in entry_attrs
+and not options.get('raw', False)):
+entry_attrs['usercertificate'] = entry_attrs.pop(
+'usercertificate;binary')
+
class baseuser_add(LDAPCreate):
"""
Prototype command plugin to be implemented by real plugin
"""
+def pre_common_callback(self, ldap, dn, entry_attrs, **options):
+assert isinstance(dn, DN)
+self.obj.convert_usercertificate_pre(entry_attrs)
+
+def post_common_callback(self, ldap, dn, entry_attrs, **options):
+assert isinstance(dn, DN)
+self.obj.convert_usercertificate_post(entry_attrs, **options)
class baseuser_del(LDAPDelete):
"""
@@ -542,6 +560,7 @@ class baseuser_mod(LDAPUpdate):
self.check_userpassword(entry_attrs, **options)
self.check_objectclass(ldap, dn, entry_attrs)
+self.obj.convert_usercertificate_pre(entry_attrs)
def post_common_callback(self, ldap, dn, entry_attrs, **options):
assert isinstance(dn, DN)
@@ -554,6 +573,7 @@ class baseuser_mod(LDAPUpdate):
convert_nsaccountlock(entry_attrs)
self.obj.convert_manager(entry_attrs, **options)
self.obj.get_password_attributes(ldap, dn, entry_attrs)
+self.obj.convert_use
Re: [Freeipa-devel] [PATCH] 369 Added CLI param and ACL for vault service operations.
On 8/3/2015 2:31 AM, Martin Kosek wrote:
On 07/31/2015 05:07 PM, Endi Sukma Dewata wrote:
The CLIs to manage vault owners and members have been modified
to accept services with a new parameter. Due to name conflict,
the existing 'service' parameter has been renamed to
'servicename'.
A new ACL has been added to allow a service to create its own
service container.
https://fedorahosted.org/freeipa/ticket/5172
I did not do a full review, I just saw some of the changes that made me worry -
like renaming API command attribute. Wouldn't that make the older clients fail?
See for example permission-* commands, we faced similar situation there and had
to rename command attributes potentially used by old clients in the new format.
Yes, it will break older clients. The problem is I cannot keep the
legacy 'service' parameter for backward compatibility:
Str(
'service?',
cli_name='deprecated_service',
doc=_('DEPRECATED: Service name of the service vault'),
),
Str(
'servicename?',
cli_name='service',
doc=_('Service name of the service vault'),
),
because it will conflict with a new 'service' parameter:
[Mon Aug 03 17:19:00.197040 2015] [wsgi:error] [pid 9409] ipa: ERROR:
Failed to start IPA: cannot override NameSpace.service value
Str('service?', cli_name='deprecated_service', doc=Gettext('DEPRECATED:
Service name of the service vault', domain='ipa', localedir=None)) with
Str('service*', alwaysask=True, cli_name='services', csv=True,
doc=u'services to add', label=u'member service')
that was added automatically when I added the 'service' attribute member:
attribute_members = {
'owner': ['user', 'group', 'service'],
'member': ['user', 'group', 'service'],
}
If there's a way to use a different parameter name for the 'service'
attribute member to avoid conflict with the legacy 'service' parameter
please let me know.
The other option is to force the client to upgrade to the same version.
Please let me know. Thanks.
--
Endi S. Dewata
--
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] Replace stageuser-add --from-delete with user-undel --to-staged
On 07/28/2015 12:34 PM, Jan Cholasta wrote: Dne 28.7.2015 v 11:36 Lenka Doudova napsal(a): Dne 28.7.2015 v 11:27 Jan Cholasta napsal(a): Dne 27.7.2015 v 17:59 Martin Basti napsal(a): On 23/07/15 14:43, Martin Basti wrote: Hello, I tried to fix #5145 and I partially succeeded. However, I cannot fix this part of ticket, where user is prompted to write name and surname. $ ipa stageuser-add tuser --from-delete First name: this will be ignored Last name: this will be also ignored Added stage user "tuser" As the first name and last name are mandatory attributes of stageuser-add command, but they are not needed by when the --from-delete option is used. I would like to ask how to fix this issue, IMO this will be huge hack in internal API. Or should we just document this bug as known issue (thierry wrote that this is not use case that should be used often)? The best solution would be separate command, but this idea was rejected in thread "[Freeipa-devel] User life cycle: question regarding the design" Regards Martin^2 Hello, as was mentioned before, we have issue with current internal API and the stageuser-add --from-delete command. We discussed this today, and we did not find a nice way how to fix it, so we propose this (which is IMO the best solution): * stageuser-add --from-delete should be deprecated +1 * create new option for user-undel: used-undel --to-staged (or create new command) that will handle moving deleted users to staged area as --from-delete did. Make it new command please. Instead of stageuser-add and option --from-delete, which work totally different, the command user-undel does similar operation than stage-user --from-delete, it just uses different container. NACK on stuffing everything into a single command just because it does something similar. How about making it a 'stageuser-undel'? The 'user-undel' moves preserved user to active, so the 'stageuser-undel' would move preserved to staged. The action is similar, but has slightly different specifics (which attributes are preserved etc.), and for me the 'stageuser-undel' feels more natural than 'user-undel --to-staged' since it's basically the same as there is 'stageuser-add' for creating a staged user, not 'user-add --to-staged'. It would be in the same style as all the other commands concerning operations with users in staged container. Well, user-undel is the opposite of user-del, and stageuser-undel should be the opposite of stageuser-del. The stageuser-undel you are suggesting is not. Also I'm not sure if we want to (always) remove the deleted user once a staged user is created from it, but -undel behaves like that. I don't think the command should be limited to deleted users only. Active and deleted users share the same namespace, so it is an arbitrary limitation. preserved users has been valid active user. In that sense active/preserved are managed by a same set of CLI (user-find,user-del,user-show) because a preserved user is a 'user'. So I would vote for continuing with a 'user-*' commands and use 'user-undel --to-stage'. I think that what we are looking for is the opposite of stageuser-activate. So maybe user-stage? -- 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] Time-Based Account Policies
On Mon, 03 Aug 2015, Stanislav Laznicka wrote: dragons may appear, although with a tiny tiny possibility of a golden treasure in the end. Yes, I think intervals are required. Alright. I gave it a little thought considering the current state of the language for time rules and considering where I should go with these intervals. The thing about the 'interval' in iCalendar is that to use it it is necessary to also add at least the 'frequency' and 'startdate' elements to the language. With that, adding 'enddate' and 'count' optional elements should not be that bad. What I am hoping for is that the same functionality as in iCalendar can be achieved here and so far I do not see why not. Yep. 3. The mockups for HBAC time policies show quite a wizard-like UI. While I might be very wrong here, I was thinking of rather a simple UI where user would be able to set the values for each of the rule keywords (timeofday, dayofweek, ...) directly in some text input fields with possible help of JavaScript, that would add text description to the user input (e.g. "Monday to Friday" with user input "1-5" at the dayofweek input field). With JavaScript support your wizard-like approach would still work within the same page -- instead of moving 'next', you would need to modify a number of available input fields based on selected items. That's possible and I don't see much of trouble with it. 4. Do we want some special settings for "absolute" time policies (policies that start and end at certain time in year)? The issue now would be that some of such rules would have to be broken down in more than one time rule (e.g. rule starting at a certain time of a day in a month in one year and ending at a certain time, day and month of a different year might get broken down to up to 6 rules if I count right). This could actually be solved by a UI wizard-like setting where the user gets to pick the dates and times of the rule, a conversion method would need to be created and such a thing would then work for the CLI, too. Still, usually more than one time rule would be created for such cases. Same here. I might not have expressed myself clearly there, for which I am sorry. I was rather thinking that instead of different 'screens' for different time scenarios, like "Yearly", "Daily", etc., user should be able to set all the values in one UI pop-up screen directly as number values in text fields (with the exception of "absolute" time policies, perhaps). While the user experience may suffer a bit, to me it seems more clear, readable and flexible than to have some elements such as checkboxes and selects take care of that (although the values around the "interval" from iCalendar discussed here earlier would probably need just that). Hopefully, I made myself clearer here :) If you are able to structure types of scenarios clearly, use accordion pattern to present them at the top level, like here: https://www.patternfly.org/widgets/#accordion (we are using PatternFly in our UI). Do per-scenario options in each panel as needed. -- / 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] Time-Based Account Policies
On 08/03/2015 11:45 AM, Alexander Bokovoy wrote: On Mon, 03 Aug 2015, Stanislav Laznicka wrote: Hi, I have made some changes to the structure of the HBAC time rules extension, namely the code that validates the time rules' strings was moved from the ipalib/parameters to the hbacrule module itself, and a more "fresh" approach was used in code for methods of adding/removing time policies to HBAC rules. Thanks for the advances. :) A slight change was made to understanding a week in a month. The change follows the Java implementation of a week in a month as suggested by Petr V., given a week starts on Monday (=1; iso 8601). More on that on the previously mentioned link https://docs.oracle.com/javase/8/docs/api/java/time/temporal/WeekFields.html What this change means is that a first week in a month is a week that contains at least 4 days. If it has less days, it's 0-th week (probably better than having it belong to the previous month as some sources also suggest - iso 8601 does not have a definition for a week in month but it has a definition for a week in a year). Sounds fine. I had Jan C. check the current implementation of the FreeIPA side for the time-based policies and it seems to work as is. He created official number identifiers for the 2 new LDAP attributeTypes, too. \o/ I was also going through some old mockups for the WebUI Petr V. sent me earlier last month. It brought some questions worth sharing here. 1. Do we need time rules based on the day and week of the year? Currently, there is no such option as dayofyear or weekofyear in the rules language, although adding it should not be that much of a problem. I did not include them as it seemed more convenient to set the data as combinations of dayofmonth and monthofyear values. In business circles it is increasingly common to refer to events by their week numbers, especially in logistics and factory delivery. See http://www.cl.cam.ac.uk/~mgk25/iso-time.html for some details at the end of 'Date' section. It should go right in, then. 2. Should we add dayofyear and weekofyear, a possible need for "intervals" might be required. An "interval" is a behavior from the iCalendar format. It basically functions as range() in Python, although with possible 'infinite' end. Example: should you have a recurrence rule on daily basis with interval=2, a rule would apply on every other day. This is kind of a question of keeping it easy and light or heading a way of robust implementation during which dragons may appear, although with a tiny tiny possibility of a golden treasure in the end. Yes, I think intervals are required. Alright. I gave it a little thought considering the current state of the language for time rules and considering where I should go with these intervals. The thing about the 'interval' in iCalendar is that to use it it is necessary to also add at least the 'frequency' and 'startdate' elements to the language. With that, adding 'enddate' and 'count' optional elements should not be that bad. What I am hoping for is that the same functionality as in iCalendar can be achieved here and so far I do not see why not. 3. The mockups for HBAC time policies show quite a wizard-like UI. While I might be very wrong here, I was thinking of rather a simple UI where user would be able to set the values for each of the rule keywords (timeofday, dayofweek, ...) directly in some text input fields with possible help of JavaScript, that would add text description to the user input (e.g. "Monday to Friday" with user input "1-5" at the dayofweek input field). With JavaScript support your wizard-like approach would still work within the same page -- instead of moving 'next', you would need to modify a number of available input fields based on selected items. That's possible and I don't see much of trouble with it. 4. Do we want some special settings for "absolute" time policies (policies that start and end at certain time in year)? The issue now would be that some of such rules would have to be broken down in more than one time rule (e.g. rule starting at a certain time of a day in a month in one year and ending at a certain time, day and month of a different year might get broken down to up to 6 rules if I count right). This could actually be solved by a UI wizard-like setting where the user gets to pick the dates and times of the rule, a conversion method would need to be created and such a thing would then work for the CLI, too. Still, usually more than one time rule would be created for such cases. Same here. I might not have expressed myself clearly there, for which I am sorry. I was rather thinking that instead of different 'screens' for different time scenarios, like "Yearly", "Daily", etc., user should be able to set all the values in one UI pop-up screen directly as number values in text fields (with the exception of "absolute" time policies, perhaps). While the user experience may s
Re: [Freeipa-devel] [PATCH 0052] store user certificates in 'userCertificate; binary' attributes
Dne 3.8.2015 v 14:58 Martin Babinsky napsal(a):
On 08/03/2015 02:46 PM, Jan Cholasta wrote:
Dne 3.8.2015 v 14:14 Jan Cholasta napsal(a):
Hi,
Dne 3.8.2015 v 14:00 Martin Babinsky napsal(a):
This patch fixes the inconsistency between storing certificates in
'userCertificate'/'userCertificate;binary' attribute for the user
entries: the certificate must be stored in the latter attribute only.
Since a more general fix is out of 4.2.1 scope, I have implemented some
workarounds in pre/post callbacks of user-* commands in order to
enforce
this behavior.
1)
+def convert_usercertificate_pre(self, entry_attrs, **options):
+if options.get('all', False):
+return
We don't want to do any renaming when --raw is specified, not --all.
Same for convert_usercertificate_post.
Actually, the attribute should be always renamed in
convert_usercertificate_pre, otherwise we would modify the wrong
attribute. In convert_usercertificate_post, it should actually be
renamed only when --raw is specified.
If you do the rename in `convert_usercertificate_post` only when '--raw'
is specified, then you get no certificate displayed when you do `ipa
user-show` on user with userCertificate;binary attribute. Is this
intended? (Keep in mind that `convert_usercertificate_post` should be
called in post-callback when returning results back to user/client).
Oops, I meant "rename only when --raw is *not* specified".
2)
+self.obj.convert_usercertificate_pre(entry_attrs, **options)
Rather than calling this directly from user_add, this should be called
from baseuser.pre_common_callback(), which should be called from
user_add.post_callback().
3) IMO you should change user_{add,remove}_cert to call
baseuser.convert_usercertificate_{pre,post} as well, to avoid code
duplication.
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 0052] store user certificates in 'userCertificate; binary' attributes
On 08/03/2015 02:46 PM, Jan Cholasta wrote:
Dne 3.8.2015 v 14:14 Jan Cholasta napsal(a):
Hi,
Dne 3.8.2015 v 14:00 Martin Babinsky napsal(a):
This patch fixes the inconsistency between storing certificates in
'userCertificate'/'userCertificate;binary' attribute for the user
entries: the certificate must be stored in the latter attribute only.
Since a more general fix is out of 4.2.1 scope, I have implemented some
workarounds in pre/post callbacks of user-* commands in order to enforce
this behavior.
1)
+def convert_usercertificate_pre(self, entry_attrs, **options):
+if options.get('all', False):
+return
We don't want to do any renaming when --raw is specified, not --all.
Same for convert_usercertificate_post.
Actually, the attribute should be always renamed in
convert_usercertificate_pre, otherwise we would modify the wrong
attribute. In convert_usercertificate_post, it should actually be
renamed only when --raw is specified.
If you do the rename in `convert_usercertificate_post` only when '--raw'
is specified, then you get no certificate displayed when you do `ipa
user-show` on user with userCertificate;binary attribute. Is this
intended? (Keep in mind that `convert_usercertificate_post` should be
called in post-callback when returning results back to user/client).
2)
+self.obj.convert_usercertificate_pre(entry_attrs, **options)
Rather than calling this directly from user_add, this should be called
from baseuser.pre_common_callback(), which should be called from
user_add.post_callback().
3) IMO you should change user_{add,remove}_cert to call
baseuser.convert_usercertificate_{pre,post} as well, to avoid code
duplication.
Honza
--
Martin^3 Babinsky
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0052] store user certificates in 'userCertificate; binary' attributes
Dne 3.8.2015 v 14:14 Jan Cholasta napsal(a):
Hi,
Dne 3.8.2015 v 14:00 Martin Babinsky napsal(a):
This patch fixes the inconsistency between storing certificates in
'userCertificate'/'userCertificate;binary' attribute for the user
entries: the certificate must be stored in the latter attribute only.
Since a more general fix is out of 4.2.1 scope, I have implemented some
workarounds in pre/post callbacks of user-* commands in order to enforce
this behavior.
1)
+def convert_usercertificate_pre(self, entry_attrs, **options):
+if options.get('all', False):
+return
We don't want to do any renaming when --raw is specified, not --all.
Same for convert_usercertificate_post.
Actually, the attribute should be always renamed in
convert_usercertificate_pre, otherwise we would modify the wrong
attribute. In convert_usercertificate_post, it should actually be
renamed only when --raw is specified.
2)
+self.obj.convert_usercertificate_pre(entry_attrs, **options)
Rather than calling this directly from user_add, this should be called
from baseuser.pre_common_callback(), which should be called from
user_add.post_callback().
3) IMO you should change user_{add,remove}_cert to call
baseuser.convert_usercertificate_{pre,post} as well, to avoid code
duplication.
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] [PATCHES 0042-45] new commands for adding/removing certificates from entries
Dne 3.8.2015 v 14:34 Martin Babinsky napsal(a): On 07/14/2015 03:22 PM, Milan Kubík wrote: On 07/02/2015 04:44 PM, Jan Cholasta wrote: Dne 2.7.2015 v 16:36 Martin Babinsky napsal(a): On 07/02/2015 02:37 PM, Martin Babinsky wrote: On 07/02/2015 11:28 AM, Martin Babinsky wrote: On 07/02/2015 11:12 AM, Martin Babinsky wrote: On 07/01/2015 03:05 PM, Martin Babinsky wrote: On 06/30/2015 02:45 PM, Martin Babinsky wrote: On 06/30/2015 01:11 PM, Martin Babinsky wrote: On 06/30/2015 12:04 PM, Jan Cholasta wrote: Dne 29.6.2015 v 10:36 Martin Babinsky napsal(a): On 06/23/2015 01:49 PM, Martin Babinsky wrote: This patchset implements new API commands for manipulating user/host/service userCertificate attribute alongside some underlying plumbing. PATCH 0045 is a small test suite that I slapped together since manual testing of this stuff is very cumbersome. It requires my PATCH 0040 to apply and work which was pushed to master recently (commit 74883bbc959058c8bfafd9f63e8fad0e3792ac28). The work is related to http://www.freeipa.org/page/V4/User_Certificates and https://fedorahosted.org/freeipa/ticket/4238 Attaching updated patches. Here are some notes for Jan because I did some things differently than we agreed on during review: 1.) I chose not to rename 'usercertificate' to 'usercertificate;binary' and back in pre/post callbacks. Despite the fact that the correct way to name the certificate attribute is 'usercertificate;binary', I feel that suddenly renaming it in the new code is asking for trouble. New code is new, there is no renaming, there is naming, and that naming should follow standards, and the standard is userCertificate;binary. (For the record I did not ask for any renaming in *old* host and service code.) OK I will then use 'usercertificate;binary' and try to not break things. I'm all for changing the mapping between CLI options and actual attribute names but it should be done in a systematic fashion. +1, shall I post a patch? That would be great, but I'm not sure if there is time for it. Maybe we can create a ticket for tracking? 2.) I have kept the `normalize_certs` function. It has the potential to catch incorrectly formatted/encoded certificates and in a way circumvents the slightly demented way the framework deals with supposedly binary data. One sentence above you asked for doing things in systematic fashion. This is exactly what it isn't. A systematic solution would be a new parameter type for certificates. Ha I didn't notice that incorrect encoding is caught by validator. But I think that we still need to catch malformed certificates that can not be decoded to DER and AFAIK we don't do that anywhere (failing tests when adding a random Base64-encoded string confirm this). All this probably stems from my confusion about the way IPA framework guesses binary data. For example, if I call `api.Command.user_add_cert` and fill 'certificate' option with Base64 blob reencoded to Unicode, everything works as expected. However, filling this option with 'str' leads to another round of Base64 encoding in the framework, leading to 'userCertificate;binary' which is filled by original Base64 blob instead of DER encoded cert. I have also added two negative test cases which deal with incorrectly encoded and formatted certificates. Attaching updated patches (actually only 44 is updated, I added the rename to/from 'usercertificate;binary' to user pre/post callbacks). Another patch update attached (mainly fixing pep8 complaints and reworking certificate validation). Updated patches attached. I left a a bug in PATCH 0043. Attaching updated version. Attaching updated patches. Attaching revised patchset. Thanks, ACK on patch 42-44. Pushed to master: 76eea85701af80dc972c47e14aecc7a688b9c846 It would be nice if Milan could comment on PATCH 0045. (I did not push this patch.) Hi, sorry for the delay. The test looks good to me. Though we'll better rewrite the test after the Trackers for the plugins involved in certificate signing and CA ACL enforcement will be available. It won't be necessary right away, though. ACK Thanks, Milan So what about PATCH 45 (tests). Milan ACKed it, can we push it? I don't see why not. Pushed to: master: 555229e33e44a200a4035d21da326f568b25946c ipa-4-2: d0db86f9b5ec33b9015163035ff84da20d859a2a -- 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] [PATCHES 0042-45] new commands for adding/removing certificates from entries
On 07/14/2015 03:22 PM, Milan Kubík wrote: On 07/02/2015 04:44 PM, Jan Cholasta wrote: Dne 2.7.2015 v 16:36 Martin Babinsky napsal(a): On 07/02/2015 02:37 PM, Martin Babinsky wrote: On 07/02/2015 11:28 AM, Martin Babinsky wrote: On 07/02/2015 11:12 AM, Martin Babinsky wrote: On 07/01/2015 03:05 PM, Martin Babinsky wrote: On 06/30/2015 02:45 PM, Martin Babinsky wrote: On 06/30/2015 01:11 PM, Martin Babinsky wrote: On 06/30/2015 12:04 PM, Jan Cholasta wrote: Dne 29.6.2015 v 10:36 Martin Babinsky napsal(a): On 06/23/2015 01:49 PM, Martin Babinsky wrote: This patchset implements new API commands for manipulating user/host/service userCertificate attribute alongside some underlying plumbing. PATCH 0045 is a small test suite that I slapped together since manual testing of this stuff is very cumbersome. It requires my PATCH 0040 to apply and work which was pushed to master recently (commit 74883bbc959058c8bfafd9f63e8fad0e3792ac28). The work is related to http://www.freeipa.org/page/V4/User_Certificates and https://fedorahosted.org/freeipa/ticket/4238 Attaching updated patches. Here are some notes for Jan because I did some things differently than we agreed on during review: 1.) I chose not to rename 'usercertificate' to 'usercertificate;binary' and back in pre/post callbacks. Despite the fact that the correct way to name the certificate attribute is 'usercertificate;binary', I feel that suddenly renaming it in the new code is asking for trouble. New code is new, there is no renaming, there is naming, and that naming should follow standards, and the standard is userCertificate;binary. (For the record I did not ask for any renaming in *old* host and service code.) OK I will then use 'usercertificate;binary' and try to not break things. I'm all for changing the mapping between CLI options and actual attribute names but it should be done in a systematic fashion. +1, shall I post a patch? That would be great, but I'm not sure if there is time for it. Maybe we can create a ticket for tracking? 2.) I have kept the `normalize_certs` function. It has the potential to catch incorrectly formatted/encoded certificates and in a way circumvents the slightly demented way the framework deals with supposedly binary data. One sentence above you asked for doing things in systematic fashion. This is exactly what it isn't. A systematic solution would be a new parameter type for certificates. Ha I didn't notice that incorrect encoding is caught by validator. But I think that we still need to catch malformed certificates that can not be decoded to DER and AFAIK we don't do that anywhere (failing tests when adding a random Base64-encoded string confirm this). All this probably stems from my confusion about the way IPA framework guesses binary data. For example, if I call `api.Command.user_add_cert` and fill 'certificate' option with Base64 blob reencoded to Unicode, everything works as expected. However, filling this option with 'str' leads to another round of Base64 encoding in the framework, leading to 'userCertificate;binary' which is filled by original Base64 blob instead of DER encoded cert. I have also added two negative test cases which deal with incorrectly encoded and formatted certificates. Attaching updated patches (actually only 44 is updated, I added the rename to/from 'usercertificate;binary' to user pre/post callbacks). Another patch update attached (mainly fixing pep8 complaints and reworking certificate validation). Updated patches attached. I left a a bug in PATCH 0043. Attaching updated version. Attaching updated patches. Attaching revised patchset. Thanks, ACK on patch 42-44. Pushed to master: 76eea85701af80dc972c47e14aecc7a688b9c846 It would be nice if Milan could comment on PATCH 0045. (I did not push this patch.) Hi, sorry for the delay. The test looks good to me. Though we'll better rewrite the test after the Trackers for the plugins involved in certificate signing and CA ACL enforcement will be available. It won't be necessary right away, though. ACK Thanks, Milan So what about PATCH 45 (tests). Milan ACKed it, can we push it? -- Martin^3 Babinsky -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0052] store user certificates in 'userCertificate; binary' attributes
Hi,
Dne 3.8.2015 v 14:00 Martin Babinsky napsal(a):
This patch fixes the inconsistency between storing certificates in
'userCertificate'/'userCertificate;binary' attribute for the user
entries: the certificate must be stored in the latter attribute only.
Since a more general fix is out of 4.2.1 scope, I have implemented some
workarounds in pre/post callbacks of user-* commands in order to enforce
this behavior.
1)
+def convert_usercertificate_pre(self, entry_attrs, **options):
+if options.get('all', False):
+return
We don't want to do any renaming when --raw is specified, not --all.
Same for convert_usercertificate_post.
2)
+self.obj.convert_usercertificate_pre(entry_attrs, **options)
Rather than calling this directly from user_add, this should be called
from baseuser.pre_common_callback(), which should be called from
user_add.post_callback().
3) IMO you should change user_{add,remove}_cert to call
baseuser.convert_usercertificate_{pre,post} as well, to avoid code
duplication.
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 0052] store user certificates in 'userCertificate; binary' attributes
This patch fixes the inconsistency between storing certificates in
'userCertificate'/'userCertificate;binary' attribute for the user
entries: the certificate must be stored in the latter attribute only.
Since a more general fix is out of 4.2.1 scope, I have implemented some
workarounds in pre/post callbacks of user-* commands in order to enforce
this behavior.
--
Martin^3 Babinsky
From f3a35458271c3fd149eed752fe0815e61edf0cb4 Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Mon, 3 Aug 2015 13:36:29 +0200
Subject: [PATCH] store certificates issued for user entries as
userCertificate;binary
This patch forces the user management CLI command to store certificates as
userCertificate;binary attribute. The code to retrieve of user information was
modified to enable outputting of userCertificate;binary attribute to the
command line.
---
ipalib/plugins/baseuser.py | 26 +-
ipalib/plugins/user.py | 4
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/ipalib/plugins/baseuser.py b/ipalib/plugins/baseuser.py
index bd66cf5a3e3a4e6c18d1a54408f969668c834fab..36ad0590833ab1279920d7e1c19851832266a57d 100644
--- a/ipalib/plugins/baseuser.py
+++ b/ipalib/plugins/baseuser.py
@@ -187,7 +187,7 @@ class baseuser(LDAPObject):
'telephonenumber', 'title', 'memberof', 'nsaccountlock',
'memberofindirect', 'ipauserauthtype', 'userclass',
'ipatokenradiusconfiglink', 'ipatokenradiususername',
-'krbprincipalexpiration', 'usercertificate',
+'krbprincipalexpiration', 'usercertificate;binary',
]
search_display_attributes = [
'uid', 'givenname', 'sn', 'homedirectory', 'loginshell',
@@ -465,11 +465,31 @@ class baseuser(LDAPObject):
assert isinstance(user, DN)
return self._user_status(user, DN(self.delete_container_dn, api.env.basedn))
+def convert_usercertificate_pre(self, entry_attrs, **options):
+if options.get('all', False):
+return
+
+if 'usercertificate' in entry_attrs:
+entry_attrs['usercertificate;binary'] = entry_attrs.pop(
+'usercertificate')
+
+def convert_usercertificate_post(self, entry_attrs, **options):
+if options.get('all', False):
+return
+
+if 'usercertificate;binary' in entry_attrs:
+entry_attrs['usercertificate'] = entry_attrs.pop(
+'usercertificate;binary')
+
class baseuser_add(LDAPCreate):
"""
Prototype command plugin to be implemented by real plugin
"""
+def post_common_callback(self, ldap, dn, entry_attrs, **options):
+assert isinstance(dn, DN)
+self.obj.convert_usercertificate_post(entry_attrs, **options)
+
class baseuser_del(LDAPDelete):
"""
Prototype command plugin to be implemented by real plugin
@@ -542,6 +562,7 @@ class baseuser_mod(LDAPUpdate):
self.check_userpassword(entry_attrs, **options)
self.check_objectclass(ldap, dn, entry_attrs)
+self.obj.convert_usercertificate_pre(entry_attrs, **options)
def post_common_callback(self, ldap, dn, entry_attrs, **options):
assert isinstance(dn, DN)
@@ -554,6 +575,7 @@ class baseuser_mod(LDAPUpdate):
convert_nsaccountlock(entry_attrs)
self.obj.convert_manager(entry_attrs, **options)
self.obj.get_password_attributes(ldap, dn, entry_attrs)
+self.obj.convert_usercertificate_post(entry_attrs, **options)
convert_sshpubkey_post(ldap, dn, entry_attrs)
radius_dn2pk(self.api, entry_attrs)
@@ -584,6 +606,7 @@ class baseuser_find(LDAPSearch):
for attrs in entries:
self.obj.convert_manager(attrs, **options)
self.obj.get_password_attributes(ldap, attrs.dn, attrs)
+self.obj.convert_usercertificate_post(attrs, **options)
if (lockout):
attrs['nsaccountlock'] = True
else:
@@ -598,5 +621,6 @@ class baseuser_show(LDAPRetrieve):
assert isinstance(dn, DN)
self.obj.convert_manager(entry_attrs, **options)
self.obj.get_password_attributes(ldap, dn, entry_attrs)
+self.obj.convert_usercertificate_post(entry_attrs, **options)
convert_sshpubkey_post(ldap, dn, entry_attrs)
radius_dn2pk(self.api, entry_attrs)
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 0209b29b130f2377c04f497f95c8ad39e98f2587..61d174d82c077a5c9f945ebafa123569df75fea3 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -510,6 +510,7 @@ class user_add(baseuser_add):
answer = self.api.Object['radiusproxy'].get_dn_if_exists(rcl)
entry_attrs['ipatokenradiusconfiglink'] = answer
+self.obj.convert_usercertificate_pre(entry_attrs, **options)
return dn
def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
@@ -557,6 +558,9 @@ class user_add(baseuser_add):
convert_sshpubkey_post(ldap, dn, entr
Re: [Freeipa-devel] [PATCH 0051] IPA server and replica installers can accept options from config file
On 07/30/2015 08:55 AM, Jan Cholasta wrote:
Dne 29.7.2015 v 17:43 Petr Vobornik napsal(a):
On 07/29/2015 05:13 PM, Martin Babinsky wrote:
On 07/29/2015 01:25 PM, Jan Cholasta wrote:
Dne 29.7.2015 v 12:20 Martin Babinsky napsal(a):
Initial attempt to implement
https://fedorahosted.org/freeipa/ticket/4517
Some points to discuss:
1.) name of the config entries: currently the option names are derived
from CLI options but have underscores in them instead of dashes. Maybe
keeping the CLI option names also for config entries will make it
easier
for the user to transfer their CLI options from scripts to config
files.
NACK. There is no point in generating config names from CLI names,
which
are generated from knob names - use knob names directly.
The problem is that in some cases the cli_name does not map directly to
knob name, leading in different naming of CLI options and config
entries, confusion and mayhem.
What works for CLI may not work for config files and vice versa. For
example, this works for CLI:
--no-ntp
--no-forwarders
--forwarder 1.2.3.4 --forwarder 5.6.7.8
but this works better in config file:
ntp = False
forwarders =
forwarders = 1.2.3.4, 5.6.7.8
These are some offenders from `ipaserver/install/server.py`:
http://fpaste.org/249424/18226114/
On the other hand, this can be an incentive to finally put an end to
inconsistent option/knob naming across server/replica/etc. installers.
Yes please.
If the names are different than cli names, then they should be made
discoverable somehow or be documented.
IMHO documenting them is easy.
2.) Config sections: there is currently only one valid section named
'[global]' in accordance with the format of 'default.conf'. Should we
have separate sections equivalent to option groups in CLI (e.g.
[basic],
[certificate system], [dns])?
No, because they would have to be maintained forever. For example, some
options are in wrong sections and we wouldn't be able to move them.
I'm also more inclined to a single section, at least for now since we
are pressed for time with this RFE.
That's not to say that we should ditch Alexander's idea about separate
sections with overrides for different hosts. We should consider it as a
future enhancement to this feature once the basic plumbing is in place.
Right.
3.) Handling of unattended mode when specifying a config file:
Currently there is no connection between --config-file and unattended
mode. So when you run ipa-server-install using config file, you still
get asked for missing stuff. Should '--config-file' automatically
imply
'--unattended'?
The behavior should be the same as if you specified the options on the
command line. So no, --config-file should not imply --unattended.
That sound reasonable. the code behaves this way already so no changes
here.
There are probably other issues to discuss. Feel free to write
email/ping me on IRC.
(I haven't looked at the patch yet.)
Please take a look at it ASAP. I am on PTO tomorrow and on Friday, but I
will find time to work at it in the evening if you send me you comments.
1) IMO the option should be in the top-level option section, not in a
separate group (use "parser.add_option()").
Also maybe rename it to --config, AFAIK that's what is usually used.
A short name ("-c"?) would be nice too.
Nitpick: if the option is named --config-file, dest should be
"config_file", to make it easier to look it up in the code.
2) Please don't duplicate the knob retrieval code, store knobs in a list
and pass that as an argument to parse_config_file.
3) I'm not sure about using newline as a list separator. I don't know
about other IPA components, but SSSD in particular uses commas, maybe we
should be consistent with that?
4) Booleans should be assignable either True or False, i.e. do not use
_parse_knob to parse them.
Honza
Attaching updated patch.
--
Martin^3 Babinsky
From c787830e833c96d522b5dfe22bb0a054857a901d Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Wed, 22 Jul 2015 13:55:26 +0200
Subject: [PATCH] IPA server and replica installers can accept options from
config file
New option '-c'/'--config' enables ipa-server-install and ipa-replica-install
to obtain parameters from supplied configuration file in INI format.
The file syntax is as follows:
* all options are listed in a single [global] section
* the option name can be derived from long CLI option name by replacing
dashes with underscores
* to specify multivalued parameter, assign a list of comma separated values
to a single option. Whitespace around commas is permitted.
Parameters specified explicitly through CLI options take precedence over the
values contained in config file.
In the case of unknown options present in the config file, the installer will
raise an error listing them.
https://fedorahosted.org/freeipa/ticket/4517
---
ipapython/install/cli.py | 83 +++-
1 f
Re: [Freeipa-devel] New freeipa-tools repo
On 07/30/2015 01:58 PM, Martin Kosek wrote: > On 07/30/2015 01:51 PM, Alexander Bokovoy wrote: >> On Thu, 30 Jul 2015, Martin Kosek wrote: >>> Hello all, >>> >>> Right now, the people pushing patches to FreeIPA use Petr's great "ipatool" >>> that is part of Petr's (CCed) ipa-tools repo forked from my old "ipa-tools" >>> repository. >>> >>> Recently, we have found that "ipatool" needs update due to 4.2 being >>> released, >>> so I think this is a great moment to make this tool more official and use a >>> shared team repository so that people on the team can contribute and freely >>> improve it. >>> >>> As we have our shiny organization on github, I simply created new >>> "freeipa-tools" repo and with Petr's permission, moved "ipa-tools" content >>> there: >>> >>> https://github.com/freeipa/freeipa-tools >>> >>> If there are no objections, I would use that as the authoritative version of >>> the tools repo for the team. >>> >>> FreeIPA developers, please feel free to request access to >>> https://github.com/orgs/freeipa/teams/freeipa >>> if you want to commit to this repo. >> The link does not exist, you'd get 404 trying to access it. >> I don't see any way to add myself or request addition at >> https://github.com/freeipa > > Ah, looks like the github teams work differently then I though. You may need > to > send me your github logins so that I can add people. > > If there is better way, please let me know. > As a heads up - the freeipa-ci repository containing the FreeIPA integration tests definitions under jenkins-job-builder is now available under freeipa organization as well. -- 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] Time-Based Account Policies
On Mon, 03 Aug 2015, Stanislav Laznicka wrote: Hi, I have made some changes to the structure of the HBAC time rules extension, namely the code that validates the time rules' strings was moved from the ipalib/parameters to the hbacrule module itself, and a more "fresh" approach was used in code for methods of adding/removing time policies to HBAC rules. Thanks for the advances. :) A slight change was made to understanding a week in a month. The change follows the Java implementation of a week in a month as suggested by Petr V., given a week starts on Monday (=1; iso 8601). More on that on the previously mentioned link https://docs.oracle.com/javase/8/docs/api/java/time/temporal/WeekFields.html What this change means is that a first week in a month is a week that contains at least 4 days. If it has less days, it's 0-th week (probably better than having it belong to the previous month as some sources also suggest - iso 8601 does not have a definition for a week in month but it has a definition for a week in a year). Sounds fine. I had Jan C. check the current implementation of the FreeIPA side for the time-based policies and it seems to work as is. He created official number identifiers for the 2 new LDAP attributeTypes, too. \o/ I was also going through some old mockups for the WebUI Petr V. sent me earlier last month. It brought some questions worth sharing here. 1. Do we need time rules based on the day and week of the year? Currently, there is no such option as dayofyear or weekofyear in the rules language, although adding it should not be that much of a problem. I did not include them as it seemed more convenient to set the data as combinations of dayofmonth and monthofyear values. In business circles it is increasingly common to refer to events by their week numbers, especially in logistics and factory delivery. See http://www.cl.cam.ac.uk/~mgk25/iso-time.html for some details at the end of 'Date' section. 2. Should we add dayofyear and weekofyear, a possible need for "intervals" might be required. An "interval" is a behavior from the iCalendar format. It basically functions as range() in Python, although with possible 'infinite' end. Example: should you have a recurrence rule on daily basis with interval=2, a rule would apply on every other day. This is kind of a question of keeping it easy and light or heading a way of robust implementation during which dragons may appear, although with a tiny tiny possibility of a golden treasure in the end. Yes, I think intervals are required. 3. The mockups for HBAC time policies show quite a wizard-like UI. While I might be very wrong here, I was thinking of rather a simple UI where user would be able to set the values for each of the rule keywords (timeofday, dayofweek, ...) directly in some text input fields with possible help of JavaScript, that would add text description to the user input (e.g. "Monday to Friday" with user input "1-5" at the dayofweek input field). With JavaScript support your wizard-like approach would still work within the same page -- instead of moving 'next', you would need to modify a number of available input fields based on selected items. That's possible and I don't see much of trouble with it. 4. Do we want some special settings for "absolute" time policies (policies that start and end at certain time in year)? The issue now would be that some of such rules would have to be broken down in more than one time rule (e.g. rule starting at a certain time of a day in a month in one year and ending at a certain time, day and month of a different year might get broken down to up to 6 rules if I count right). This could actually be solved by a UI wizard-like setting where the user gets to pick the dates and times of the rule, a conversion method would need to be created and such a thing would then work for the CLI, too. Still, usually more than one time rule would be created for such cases. Same here. -- / 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 022] make-lint Python 3 porting mode
On 2015-08-03 11:30, Jan Cholasta wrote: > Hi, > > Dne 3.8.2015 v 11:22 Christian Heimes napsal(a): >> Python 3 porting mode for make-lint >> >> http://docs.pylint.org/features.html#general-options > > I would rather wait until all the modernization patches are pulled in > and then make the porting mode enabled by default. If it's optional, no > one will use it. In porting mode the normal checkers aren't executed. In order to enable the porting mode by default, make-lint has to run two passes: one linter instance with and one linter instance without the porting mode. signature.asc Description: OpenPGP digital signature -- 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 022] make-lint Python 3 porting mode
Hi, Dne 3.8.2015 v 11:22 Christian Heimes napsal(a): Python 3 porting mode for make-lint http://docs.pylint.org/features.html#general-options I would rather wait until all the modernization patches are pulled in and then make the porting mode enabled by default. If it's optional, no one will use it. 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 022] make-lint Python 3 porting mode
Python 3 porting mode for make-lint
http://docs.pylint.org/features.html#general-options
From eb0565a16934a85df5075a6389dc49239e08f699 Mon Sep 17 00:00:00 2001
From: Christian Heimes
Date: Mon, 3 Aug 2015 11:18:03 +0200
Subject: [PATCH] make-lint Python 3 porting mode
pylint can check code for Python 3 portability. The new option --py3k
enables the Python 3 porting mode of pylint in make-lint.
---
make-lint | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/make-lint b/make-lint
index 0447985303f485a014fecf7d17d0b1c7eb6137bd..04d7f3644bef7fccba1ce37b9d92e2e1405ffd08 100755
--- a/make-lint
+++ b/make-lint
@@ -220,6 +220,8 @@ def main():
dest='fail', default=True, action='store_false')
optparser.add_option('--enable-noerror', help='enable warnings and other non-error messages',
dest='errors_only', default=True, action='store_false')
+optparser.add_option('--py3k', help='Python 3 porting mode',
+dest='py3k', default=False, action='store_true')
options, args = optparser.parse_args()
cwd = os.getcwd()
@@ -246,7 +248,10 @@ def main():
'{path}:{line}: [{msg_id}({symbol}), {obj}] {msg})')
linter.set_option('reports', False)
linter.set_option('persistent', False)
-linter.set_option('disable', 'python3')
+if options.py3k:
+linter.python3_porting_mode()
+else:
+linter.set_option('disable', 'python3')
linter.check(files)
--
2.4.3
signature.asc
Description: OpenPGP digital signature
--
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] 0691-0695 Modernization
On 2015-07-31 23:14, Simo Sorce wrote: > On Fri, 2015-07-31 at 19:14 +0200, Petr Viktorin wrote: >> Hello, >> Here is a batch of mostly mechanical changes: removing deprecated >> features to prepare for Python 3. >> > > Do we have accompanying lint (or similar) tests that will prevent new > patches from reintroducing py3 incompatible syntax ? pylint has a Python 3 porting mode. That should help, see patch 022. Christian signature.asc Description: OpenPGP digital signature -- 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] 0691-0695 Modernization
On 2015-07-31 19:14, Petr Viktorin wrote: > Hello, > Here is a batch of mostly mechanical changes: removing deprecated > features to prepare for Python 3. Out of curiosity, what tool did you use for patch 695-absolute-imports? Python-modernize adds from __future__ import absolute_imports and changes imports to explicit relative imports. In patch 693 you have removed test cases for CIDict.has_key(), but CIDict still provides the function. You should either keep the tests around or remove has_key() from CIDict. The rest looks good to me, but I haven't studied every change thoroughly. It's just too much. Christian signature.asc Description: OpenPGP digital signature -- 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] Time-Based Account Policies
Hi, I have made some changes to the structure of the HBAC time rules extension, namely the code that validates the time rules' strings was moved from the ipalib/parameters to the hbacrule module itself, and a more "fresh" approach was used in code for methods of adding/removing time policies to HBAC rules. A slight change was made to understanding a week in a month. The change follows the Java implementation of a week in a month as suggested by Petr V., given a week starts on Monday (=1; iso 8601). More on that on the previously mentioned link https://docs.oracle.com/javase/8/docs/api/java/time/temporal/WeekFields.html What this change means is that a first week in a month is a week that contains at least 4 days. If it has less days, it's 0-th week (probably better than having it belong to the previous month as some sources also suggest - iso 8601 does not have a definition for a week in month but it has a definition for a week in a year). I had Jan C. check the current implementation of the FreeIPA side for the time-based policies and it seems to work as is. He created official number identifiers for the 2 new LDAP attributeTypes, too. \o/ I was also going through some old mockups for the WebUI Petr V. sent me earlier last month. It brought some questions worth sharing here. 1. Do we need time rules based on the day and week of the year? Currently, there is no such option as dayofyear or weekofyear in the rules language, although adding it should not be that much of a problem. I did not include them as it seemed more convenient to set the data as combinations of dayofmonth and monthofyear values. 2. Should we add dayofyear and weekofyear, a possible need for "intervals" might be required. An "interval" is a behavior from the iCalendar format. It basically functions as range() in Python, although with possible 'infinite' end. Example: should you have a recurrence rule on daily basis with interval=2, a rule would apply on every other day. This is kind of a question of keeping it easy and light or heading a way of robust implementation during which dragons may appear, although with a tiny tiny possibility of a golden treasure in the end. 3. The mockups for HBAC time policies show quite a wizard-like UI. While I might be very wrong here, I was thinking of rather a simple UI where user would be able to set the values for each of the rule keywords (timeofday, dayofweek, ...) directly in some text input fields with possible help of JavaScript, that would add text description to the user input (e.g. "Monday to Friday" with user input "1-5" at the dayofweek input field). 4. Do we want some special settings for "absolute" time policies (policies that start and end at certain time in year)? The issue now would be that some of such rules would have to be broken down in more than one time rule (e.g. rule starting at a certain time of a day in a month in one year and ending at a certain time, day and month of a different year might get broken down to up to 6 rules if I count right). This could actually be solved by a UI wizard-like setting where the user gets to pick the dates and times of the rule, a conversion method would need to be created and such a thing would then work for the CLI, too. Still, usually more than one time rule would be created for such cases. Thanks for keeping up with me and my long emails. I am a terrible person for that and I hope I will be able to cut them short in the future. Cheers, Standa From 4f25d320d9a31707bf7d0b55ea28b7e223e09f70 Mon Sep 17 00:00:00 2001 From: Stanislav Laznicka Date: Tue, 7 Jul 2015 09:44:23 +0200 Subject: [PATCH 1/3] Added time-based policies types to LDAP schema. https://fedorahosted.org/freeipa/ticket/547 https://fedorahosted.org/freeipa/ticket/548 --- install/share/60basev2.ldif | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/install/share/60basev2.ldif b/install/share/60basev2.ldif index 00712ddda2c548b7f7924a012f3f68499f2f01da..c3251a4331005ade1333f9e64b57a62a89706ce9 100644 --- a/install/share/60basev2.ldif +++ b/install/share/60basev2.ldif @@ -37,7 +37,9 @@ attributeTypes: (2.16.840.1.113730.3.8.3.11 NAME 'externalHost' DESC 'Multivalue attributeTypes: (2.16.840.1.113730.3.8.3.12 NAME 'sourceHostCategory' DESC 'Additional classification for hosts' EQUALITY caseIgnoreMatch ORDERING caseIgnoreOrderingMatch SUBSTR caseIgnoreSubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v2' ) attributeTypes: (2.16.840.1.113730.3.8.3.13 NAME 'accessRuleType' DESC 'The flag to represent if it is allow or deny rule.' EQUALITY caseIgnoreMatch ORDERING caseIgnoreOrderingMatch SUBSTR caseIgnoreSubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v2' ) attributeTypes: (2.16.840.1.113730.3.8.3.14 NAME 'accessTime' DESC 'Access time' EQUALITY caseIgnoreMatch ORDERING caseIgnoreOrderingMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v2' ) -objectClasses: (2
Re: [Freeipa-devel] [PATCH] 369 Added CLI param and ACL for vault service operations.
On 07/31/2015 05:07 PM, Endi Sukma Dewata wrote: > The CLIs to manage vault owners and members have been modified > to accept services with a new parameter. Due to name conflict, > the existing 'service' parameter has been renamed to > 'servicename'. > > A new ACL has been added to allow a service to create its own > service container. > > https://fedorahosted.org/freeipa/ticket/5172 I did not do a full review, I just saw some of the changes that made me worry - like renaming API command attribute. Wouldn't that make the older clients fail? See for example permission-* commands, we faced similar situation there and had to rename command attributes potentially used by old clients in the new format. -- 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 0002] Port from python-krbV to python-gssapi
Dne 31.7.2015 v 20:20 Simo Sorce napsal(a): On Fri, 2015-07-31 at 16:41 +0200, Michael Šimáček wrote: On 2015-07-31 07:52, Jan Cholasta wrote: Hi Michael, Dne 29.7.2015 v 10:09 Michael Šimáček napsal(a): Hi, this is the first attempt to port FreeIPA from deprecated python3-incompatible python-krbV library to python-gssapi. The patch depends on python-kerberos->python-gssapi patch [1] to apply cleanly, but the overlap is small, so I think it can be at least partially reviewed without it. Comments: I removed Backend.krb and KRB5_CCache classes as they were wrappers around krbV classes. I added few utility functions to krb_utils module that perform part of its functionality (no need for classes, because gssapi acquire calls don't pass any context objects, they wouldn't have any state). I merged the two different kinit_keytab functions. GSSAPI doesn't provide any method (that I'm aware of) to get default ccache name. In most cases this is not needed as we can simply not pass any name and it will use the default. The ldap plugin had to be adjusted for this - the connect method now takes new use_gssapi argument, which can turn on gssapi support without the need to supply explicit ccache name. The only place where the ccache name is really needed is the test server, where I use system klist command to obtain it. I would prefer if the semantics were the same as in IPAdmin, i.e. GSSAPI is used by default if bind password is not specified, see IPAdmin.do_bind() in ipapython.ipaldap. Just to clarify, the current flow in ldap module is: if ccache: # I added "or use_gssapi" here in this patch gssapi_bind elif autobind: external_bind else: simple_bind I had to make this change as well for my replica promotion code, and incidentally used the same indicator "use_gssapi". and you would like it to be changed into: if bind_pw: simple_bind elif autobind: external_bind else: gssapi_bind Is that correct? Actually this is what IPAdmin does: def do_bind(self, dm_password="", autobind=AUTOBIND_AUTO, timeout=DEFAULT_TIMEOUT): if dm_password: self.do_simple_bind(bindpw=dm_password, timeout=timeout) return if autobind != AUTOBIND_DISABLED and os.getegid() == 0 and self.ldapi: try: # autobind pw_name = pwd.getpwuid(os.geteuid()).pw_name self.do_external_bind(pw_name, timeout=timeout) return except errors.NotFound, e: if autobind == AUTOBIND_ENABLED: # autobind was required and failed, raise # exception that it failed raise #fall back self.do_sasl_gssapi_bind(timeout=timeout) I think this is what Jan wanted, but I am wondering if it is the right thing to do. In ipa we have basically 2 possible default approaches. One is to use GSSAPI, and one is to use LDAPI with external bind. The latter makes sense mostly only when running as root, so I am wondering, should the default change depending on whether we are root and we are connecting to the local LDAP server ? If this is a sensible option it means we have to preserver use_gssapi as we may need to force use of gssapi in some case even when we are root and connectiong to the local server (for example to test that the local ccache can successfully be used). Jan, what do you think ? I think GSSAPI should be the default and EXTERNAL should be opt-in, like in IPAdmin, see above. It's also not possible to directly get default realm name, what I do is importing nonexistent name, cannonicalizing it and extracting the realm from it. Which should work but is ugly. It would be better if we could modify the places that use it to not need it at all, but it's mostly used in ldap code and I don't understand that part of FreeIPA. Alternative would be parsing /etc/krb.conf. You should use api.env.realm where possible. I think this should be most of the places where default realm is currently used, if not all of them. That would be great if all the usages could be replaced. How can I determine where api.env.realm can be used? In particular, I'm unsure about ipapython/config.py/__discover_config and ipaserver/plugins/join.py. I would just remove the code from __discover_config. It is used to get realm name in case it is not configured in /etc/ipa/default.conf, but it is called only from ipa-compat-manage and ipa-nis-manage, which can be run only on IPA server, and IPA server won't work if realm is not configured. As for join.py, you can just return api.env.realm in get_realm(). try: realm = api.env.realm except: realm = dirty gssapi trick ? Please don't, you should always be able to choose the correct one instead of guessing. -- Jan Cholasta -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to F
