Some plugins, notably dns, modifies a returned entry in order
to compare it to the user-provided info (e.g. dnsrecord-del).
This modification was done on the cached entry directly rather
than a copy which caused unexpected results, mostly
EmptyResult because the cached entry was changed directly so
the next get_entry returned the same modified entry.
In addition, on a hit in the LDAP cache the entire cached entry
was returned regardless of what attributes were requested.
The automember condition add/remove calls only request the
inclusive/exclusive rule attributes and loop over the returned
values to look for duplicates. This was failing because the queried
entry contains attributes that the candidate entry does not contain.
The automember code is:
old_entry = ldap.get_entry(dn, [attr])
for regex in old_entry.keys():
if not isinstance(entry_attrs[regex], (list, tuple)):
old_entry, returned from the cache, contained objectclass, cn,
description, etc. which don't exist in the candidate entry so
entry_attrs[regex] threw a KeyError.
To return a copy of the entry and requested attributes on a
search HIT.
Also be more careful when storing the attributes in the cache entry.
The returned attributes may not match the requested. So store the
attributes we actually have.
This issue was exposed by Ansible which maintains a larger and
longer-lived cache because commands are executed in the server context
one after another, giving the cache a chance to build up.
Adjust the expected test results as well. In test_get_testuser()
the first request asks for all attributes (default) so ensure
that is successful since a user_add gets all attributes in
the post_callback. Next request a subset of the attributes which
is also a hit and confirm that only those requested were returned.
https://pagure.io/freeipa/issue/8897
Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Stanislav Levin <slev@altlinux.org>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Use new nsslapd-ldapiautoauthdnattr feature to switch BIND named from
GSSAPI bind to EXTERNAL LDAPI bind.
Requires 389-DS >= 1.4.4.11 or >= 2.0.2
Fixes: https://pagure.io/freeipa/issue/8544
See: https://github.com/389ds/389-ds-base/issues/4381
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Francois Cami <fcami@redhat.com>
Insert a class before LDAPClient to cache the return value
of get_entry() and certain exceptions (NotFound and
EmptyResult). The cache uses an OrderedDict for the cases
where a large cache might result an LRU model can be used.
The cache be enabled (default) or disabled using
ldap_cache=True/False.
This cache is per-request so is not expected to grow
particularly large except in the case of a large batch
command.
The key to the cache entry is the dn of the object
being requested.
Any write to or referencing a cached dn is evicted from
the cache.
The set of attributes is somewhat taken into consideration.
"*" does not always match everything being asked for by
a plugin so unless the requested set of attributes is a
direct subset of what is cached it will be re-fetched. Err
on the side of safety.
Despite this rather conserative approach to caching 29%
of queries are saved with ipatests/xmlrpc_tests/*
https://pagure.io/freeipa/issue/8798
Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Rafael Guterres Jeffman <rjeffman@redhat.com>
In ipa-healthcheck ds-related tests i.e
test_ipahealthcheck.py::TestIpaHealthCheck::test_ipahealthcheck_ds_configcheck
there exists a scenario of modifying the "nsslapd-logging-hr-timestamps-enabled" attribute to off
The tests are failing with the below error
"ipalib.errors.MidairCollision: change collided with another change"
The test audit log displays that the attribute is deleted first and then added.
changetype: modify
delete: nsslapd-logging-hr-timestamps-enabled
nsslapd-logging-hr-timestamps-enabled: on
-
add: nsslapd-logging-hr-timestamps-enabled
nsslapd-logging-hr-timestamps-enabled: off
Adding the nsslapd-logging-hr-timestamps-enabled attribute in _SINGLE_VALUE_OVERRIDE table
to check if we generate a replace instead of add and delete.
Signed-off-by: Sudhir Menon <sumenon@redhat.com>
Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
nsslapd-db-lock was moved from cn=config,cn=ldbm database,cn=plugins,cn=config
entry to cn=bdb subentry. Manual patching of dse.ldif was no longer
working. Installations with 389-DS 1.4.3 and newer are affected.
Low lock count can affect performance during high load, e.g. mass-import
of users or lots of concurrent connections.
Bump minimal DS version to 1.4.3. Fedora 32 and RHEL 8.3 have 1.4.3.
Fixes: https://pagure.io/freeipa/issue/8515
See: https://pagure.io/freeipa/issue/5914
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Francois Cami <fcami@redhat.com>
OpenLDAP 2.4 sets minimum version with SSL_CTX_set_options(). The
system-wide crypto-policies for TLS minimum version are applied
with SSL_CTX_set_min_proto_version(). The set_option() call cannot
not enable lower versions than allowed by crypto-policy, e.g.
openssl.cnf MinProtocol=TLS1.2 + OPT_X_TLS_PROTOCOL_MIN=TLS1.0
result in TLS 1.2 as minimum protocol version.
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
W1661(exception-escape), RPCClient.forward]
Using an exception object that was bound by an except handler)
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Commit 5be9341fba disallowed simple bind
over an insecure connection. Password logins were only allowed over LDAPS
or LDAP+STARTTLS. The restriction broke 'ipa migrate-ds' in some cases.
This commit lifts the restriction and permits insecure binds over plain
LDAP. It also makes the migrate-ds plugin use STARTTLS when a CA
certificate is configured with a plain LDAP connection.
Fixes: https://pagure.io/freeipa/issue/8040
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Thomas Woerner <twoerner@redhat.com>
This reverts commit 3509545897.
We cannot force increase in minimum SASL security factor until our
consumers are ready to deal with it. Unfortunately, realmd uses
anonymous connection for discovery and validation of IPA LDAP server.
The way it is done is fragile (it doesn't take into account an
advertised IPA version, only checks that 'IPA' string exists in the info
field) but since bumping of minimum SSF prevents reading IPA info field
using anonymous connection, client enrollment fails.
We should get back to bumping minimum SSF after realmd and other
potential consumers are fixed.
Reviewed-By: François Cami <fcami@redhat.com>
SSF_MINX 56 level ensures data integrity and confidentiality for SASL
GSSAPI and SASL GSS SPNEGO connections.
Although at least AES128 is enforced pretty much everywhere, 56 is required
for backwards compatibility with systems that announce wrong SSF.
Related: https://pagure.io/freeipa/issue/7140
Related: https://pagure.io/freeipa/issue/4580
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Tibor Dudlak <tdudlak@redhat.com>
Add LDAPClient.from_realm(), LDAPClient.from_hostname_secure(), and
LDAPClient.from_hostname_plain() constructors.
The simple_bind() method now also refuses to transmit a password over a
plain, unencrypted line.
LDAPClient.from_hostname_secure() uses start_tls and FreeIPA's CA cert
by default. The constructor also automatically disables start_tls for
ldaps and ldapi connections.
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
The helper function realm_to_serverid() and realm_to_ldap_uri() are
useful outside the server installation framework. They are now in
ipapython.ipaldap along other helpers for LDAP handling in FreeIPA.
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
ipa user-find --certificate and ipa host-find --certificate
fail to return matching entries, because the method transforming
the attribute into a LDAP filter does not properly handle
IPACertificate objects.
Directory Server logs show a filter with
(usercertificate=ipalib.x509.IPACertificate object at 0x7fc0a5575b90>)
When the attribute contains a cryptography.x509.Certificate,
the method needs to extract the public bytes instead of calling str(value).
Fixes https://pagure.io/freeipa/issue/7770
Reviewed-By: Christian Heimes <cheimes@redhat.com>
ipaldap does not take into account the possibility of the attribute
encoding returned by python-ldap differing from the attribute
encoding produced by FreeIPA. In particular this can occur with DNs
with special characters that require escaping. For example,
python-ldap (or the underlying LDAP library) escapes special
characters using hex encoding:
CN=Test Sub-CA 201604041620,OU=ftweedal,O=Red Hat\2C Inc.,L=Brisbane,C=AU
Whereas FreeIPA, when encoding the DN, escapes the character
directly:
CN=Test Sub-CA 201604041620,OU=ftweedal,O=Red Hat\, Inc.,L=Brisbane,C=AU
Therefore it is possible to generate an invalid modlist. For
example, during external CA certificate renewal, if the issuer DN
includes a comma in one of the attribute values (as above), an
invalid modlist will be generated:
[ (ldap.MOD_ADD, 'ipacaissuerdn',
[b'CN=Test Sub-CA 201604041620,OU=ftweedal,O=Red Hat\, Inc.,L=Brisbane,C=AU'])
, (ldap.MOD_DELETE, 'ipacaissuerdn',
[b'CN=Test Sub-CA 201604041620,OU=ftweedal,O=Red Hat\2C Inc.,L=Brisbane,C=AU'])
]
Although encoded differently, these are the same value. If this
modification is applied to the object, attributeOrValueExists (error
20) occurs.
To avoid the issue, put deletes before adds in the modlist. If a
value is present (with different encodings) as both an addition and
a deletion, it must be because the original object contained the
value with a different encoding. Therefore it is safe to delete it,
then add it back.
Note that the modlist is not optimal. In the simplest case (like
above example), there should be no modification to perform. It is
considerably more complex (and more computation) to implement this
because the raw attribute values must be decoded before comparison.
Fixes: https://pagure.io/freeipa/issue/7750
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Replace six.moves and six.StringIO/BytesIO imports with cannonical
Python 3 packages.
Note: six.moves.input behaves differently than builtin input function.
Therefore I left six.moves.input for now.
See: https://pagure.io/freeipa/issue/7715
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Fraser Tweedale <ftweedal@redhat.com>
On Python 3, six.text_type (singular) is an alias for str.
See: https://pagure.io/freeipa/issue/7715
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
In Python 3, six.integer_types is (int,). In most places, the alias can
be simply replaced with int. In other places, it was possible to
simplify the code by unpacking the tuple.
See: https://pagure.io/freeipa/issue/7715
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
In Python 3, six.string_types is just an alias for str.
See: https://pagure.io/freeipa/issue/7715
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
Python 2 had old style and new style classes. Python 3 has only new
style classes. There is no point to subclass from object any more.
See: https://pagure.io/freeipa/issue/7715
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
Fix the following violations aiming to support Pylint 2.0
- `unneeded-not` (C0113):
Consider changing "not item in items" to "item not in items" used
when a boolean expression contains an unneeded negation.
- `useless-import-alias` (C0414):
Import alias does not rename original package Used when an import
alias is same as original package.e.g using import numpy as numpy
instead of import numpy as np
- `raising-format-tuple` (W0715):
Exception arguments suggest string formatting might be intended Used
when passing multiple arguments to an exception constructor, the
first of them a string literal containing what appears to be
placeholders intended for formatting
- `bad-continuation` (C0330):
This was already included on the disable list, although with current
version of pylint (2.0.0.dev2) violations at the end of the files
are not being ignored.
See: https://github.com/PyCQA/pylint/issues/2278
- `try-except-raise` (E0705):
The except handler raises immediately Used when an except handler
uses raise as its first or only operator. This is useless because it
raises back the exception immediately. Remove the raise operator or
the entire try-except-raise block!
- `consider-using-set-comprehension` (R1718):
Consider using a set comprehension Although there is nothing
syntactically wrong with this code, it is hard to read and can be
simplified to a set comprehension.Also it is faster since you don't
need to create another transient list
- `dict-keys-not-iterating` (W1655):
dict.keys referenced when not iterating Used when dict.keys is
referenced in a non-iterating context (returns an iterator in
Python 3)
- `comprehension-escape` (W1662):
Using a variable that was bound inside a comprehension Emitted when
using a variable, that was bound in a comprehension handler, outside
of the comprehension itself. On Python 3 these variables will be
deleted outside of the comprehension.
Issue: https://pagure.io/freeipa/issue/7614
Signed-off-by: Armando Neto <abiagion@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
When multiple replicas are installed in parallel, two replicas may try
to create the cn=replica entry at the same time. This leads to a
conflict on one of the replicas. replica_config() and
ensure_replication_managers() now handle conflicts.
ipaldap now maps TYPE_OR_VALUE_EXISTS to DuplicateEntry(). The type or
value exists exception is raised, when an attribute value or type is
already set.
Fixes: https://pagure.io/freeipa/issue/7566
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Thierry Bordaz <tbordaz@redhat.com>
Python 3 has moved all collection abstract base classes to
collections.abc. Python 3.7 started to deprecate the old aliases.
The whole import block needs to be protected with import-error and
no-name-in-module, because Python 2 doesn't have collections.abc module and
collections.abc.Mapping, while Python 3 doesn't have collections.Mapping.
Fixes: https://pagure.io/freeipa/issue/7609
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Fraser Tweedale <ftweedal@redhat.com>
LDAP connections no longer depend on sane settings in global ldap.conf
and use good default settings for cert validation, CA, and SASL canonization.
https://pagure.io/freeipa/issue/7418
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
Pylint3 falsely reports warning W1662: using a variable that was bound
inside a comprehension for the cases where the same name is reused for a
loop after the comprehension in question.
Rename the variable in a loop to avoid it.
If the code looks like the following:
arr = [f for f in filters if callable(f)]
for f in arr:
result = result + f()
pylint3 would consider 'f' used outside of comprehension. Clearly, this
is a false-positive warning as the second 'f' use is completely
independent of the comprehension's use of 'f'.
Reviewed-By: Aleksei Slaikovskii <aslaikov@redhat.com>
Replace all ldap.initialize() calls with a helper function
ldap_initialize(). It handles cacert and cert validation correctly. It
also provides a unique place to handle python-ldap 3.0 bytes warnings in
the future.
Fixes: https://pagure.io/freeipa/issue/7411
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Allow caller to specify that the GetEffectiveRights server control
should be used on a per-operation basis. Also update
ldap2.get_effective_rights to use this new API.
Part of: https://pagure.io/freeipa/issue/6609
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Use new LDAPBytesWarning to ignore python-ldap's bytes warnings. New
build is available in @freeipa/freeipa-master.
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Felipe Volpone <fbarreto@redhat.com>
Replace python3-pyldap with python3-ldap.
Remove some old code for compatibility with very old python-ldap.
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
pylint requires all classes implementing __eq__ to also implement
__hash__. We disable hashing for the classes that miss the ability,
should they ever be required to use it, it can be implemented then.
https://pagure.io/freeipa/issue/6874
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Up until now, Bytes parameter was used for certificate parameters
throughout the framework. However, the Bytes parameter does nothing
special for certificates, like validation, so this had to be done
for each of the parameters which were supposed to represent a
certificate.
This commit introduces a special Certificate parameter which takes
care of certificate validation so this does not have to be done
separately. It also makes sure that the certificates represented by
this parameter are always converted to DER format so that we can work
with them in a unified manner throughout the framework.
This commit also makes it possible to pass bytes directly during
instantiation of the Certificate parameter and they are still
represented correctly after their conversion in the _convert_scalar()
method.
https://pagure.io/freeipa/issue/4985
Reviewed-By: Fraser Tweedale <ftweedal@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Martin Basti <mbasti@redhat.com>
Remove all object-specific loggers, with the exception of `Plugin.log`,
which is now deprecated. Replace affected logger calls with module-level
logger calls.
Deprecate object-specific loggers in `ipa_log_manager.get_logger`.
Reviewed-By: Martin Basti <mbasti@redhat.com>
LDAPClient should be used for ad-hoc connections, so the argument is not
necessary, and currently also unused.
Reviewed-By: Martin Babinsky <mbabinsk@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
in py3 we are receiving ugly tracebacks due __del__ method that
should be used very carefully or never. Due tracebacks, this doesn't
work and context manager should be used for reliable connection
termination.
Exception ignored in: <bound method LDAPClient.__del__ of ipaserver.plugins.ldap2.ldap2()>
Traceback (most recent call last):
File "/usr/lib/python3.5/site-packages/ipapython/ipaldap.py", line 1057, in __del__
File "/usr/lib/python3.5/site-packages/ipaserver/plugins/ldap2.py", line 123, in close
File "/usr/lib/python3.5/site-packages/ipalib/backend.py", line 94, in isconnected
NameError: name 'hasattr' is not defined
Reviewed-By: Stanislav Laznicka <slaznick@redhat.com>
It was hard to detect what is supposed to be in self._orig variable.
Renaming to _orig_raw makes clear for future generations that it
contains bytes.
https://pagure.io/freeipa/issue/4985
Reviewed-By: Stanislav Laznicka <slaznick@redhat.com>
Add new pylint AST checker plugin which implements a check for imports
forbidden in IPA. Which imports are forbidden is configurable in pylintrc.
Provide default forbidden import configuration and disable the check for
existing forbidden imports in our code base.
Reviewed-By: Martin Basti <mbasti@redhat.com>
GSS-SPNEGO allows us to negotiate a SASL bind with less roundtrips
therefore use it when possible.
We only enable it for local connections for now because we only
recently fixed Cyrus SASL to do proper GSS-SPNEGO negotiation. This
change means a newer and an older version are not compatible.
Restricting ourselves to the local host prevents issues with
incompatible services, and it is ok for us as we are only really
looking for speedups for the local short-lived connections performed
by the framework. Most other clients have longer lived connections,
so peformance improvements there are not as important.
Ticket: https://pagure.io/freeipa/issue/6656
Signed-off-by: Simo Sorce <simo@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Tomas Krizek <tkrizek@redhat.com>
In Python 2, the order was preserved by accident.
This change makes sure the order is preserved in both Python 2 and 3.
https://fedorahosted.org/freeipa/ticket/4985
Reviewed-By: Martin Basti <mbasti@redhat.com>
The encode method of LDAPClient didn't return DNSName as bytes but
string in py3. In py2 it returns non-unicode string so it can be encoded
safely by ascii as to_text() method returns only ascii characters.
https://fedorahosted.org/freeipa/ticket/4985
Reviewed-By: Jan Cholasta <jcholast@redhat.com>
Update encoding/decoding accordingly to work under Py3
Removing functions that were used only once in code and give no real
improvements
https://fedorahosted.org/freeipa/ticket/4985
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Method escape_filter_chars() requires string as parameter instead of
bytes. 'value_to_utf8' returns bytes thus this code has to be removed.
https://fedorahosted.org/freeipa/ticket/4985
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Jan Cholasta <jcholast@redhat.com>
Manually escape each byte in the value, do not use
ldap.filter.escape_filter_chars() as it does not work with bytes in
Python 3.
https://fedorahosted.org/freeipa/ticket/4985
Reviewed-By: Martin Basti <mbasti@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
get_entries() wouldn't pass some arguments deeper to find_entries()
function it wraps. This would cause unexpected behavior in some
cases throughout the framework where specific (non-)limitations
are expected.
https://fedorahosted.org/freeipa/ticket/5640
Reviewed-By: Jan Cholasta <jcholast@redhat.com>
In Python 3 exception instances no longer have a message attribute.
For most exceptions, str(e) or string formatting give the same result.
Fix some renamed modules, module members and functions.
https://fedorahosted.org/freeipa/ticket/4985
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Martin Basti <mbasti@redhat.com>
