Installation or IPA CA renewal with externally-signed CA accepts an
IPA CA certificate with empty Subject Key Identifier. This is
technically legal in X.509, but is an operational issue.
Furthermore, due to an extant bug in Dogtag
(https://pagure.io/dogtagpki/issue/3079) it will cause Dogtag
startup failure.
Reject CA certificates with empty Subject Key Identifier.
Fixes: https://pagure.io/freeipa/issue/7762
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>
ipa-server-install or ipa-replica-install do not provide
a meaningful error message in CA-less mode when the install
fails because of a wrong PIN.
Update the err msg so that it provides a hint to the user.
Fixes https://pagure.io/freeipa/issue/5378
Reviewed-By: Fraser Tweedale <ftweedal@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>
tox / pytest is complaining about lots and lots of invalid escape
sequences in our code base. Sprinkle raw strings or backslash escapes
across the code base to fix most occurences of:
DeprecationWarning: invalid escape sequence
There is still one warning that keeps repeating, though:
source:264: DeprecationWarning: invalid escape sequence \d
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
Remove Python 2 related code and configuration from spec file, autoconf
and CI infrastructure. From now on, FreeIPA 4.8 requires at least Python
3.6. Python 2 packages like python2-ipaserver or python2-ipaclient are
no longer available. PR-CI, lint, and tox aren't testing Python 2
compatibility either.
See: https://fedoraproject.org/wiki/Changes/FreeIPA_Python_2_Removal
Fixes: https://pagure.io/freeipa/issue/7568
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Tibor Dudlak <tdudlak@redhat.com>
ipa-server-install --uninstall -v -U prints Traceback in its log file.
This issue happens because it calls subprocess.Popen with close_fds=True
(which closes all file descriptors in the child process)
but it is trying to use the file logger in the child process
(preexec_fn is called in the child just before the child is executed).
The fix is using the logger only in the parent process.
Fixes: https://pagure.io/freeipa/issue/7681
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Globally disabling the following violations:
- `assignment-from-no-return` (E1111):
Assigning to function call which doesn't return. Used when an
assignment is done on a function call but the inferred function
doesn't return anything.
- `keyword-arg-before-vararg` (W1113):
Keyword argument before variable positional arguments list in the
definition of %s function When defining a keyword argument before
variable positional arguments, one can end up in having multiple
values passed for the aforementioned parameter in case the method is
called with keyword arguments.
Locally disabling the following:
- `subprocess-popen-preexec-fn` (W1509):
Using preexec_fn keyword which may be unsafe in the presence of
threads The preexec_fn parameter is not safe to use in the presence
of threads in your application. The child process could deadlock
before exec is called. If you must use it, keep it trivial! Minimize
the number of libraries you call into.
https://docs.python.org/3/library/subprocess.html#popen-constructor
Fixed violations:
- `bad-mcs-classmethod-argument` (C0204):
Metaclass class method %s should have %s as first argument Used when
a metaclass class method has a first argument named differently than
the value specified in valid-metaclass-classmethod-first-arg option
(default to "mcs"), recommended to easily differentiate them from
regular instance methods.
- Note: Actually `cls` is the default first arg for `__new__`.
- `consider-using-get` (R1715):
Consider using dict.get for getting values from a dict if a key is
present or a default if not Using the builtin dict.get for getting a
value from a dictionary if a key is present or a default if not, is
simpler and considered more idiomatic, although sometimes a bit slower
Issue: https://pagure.io/freeipa/issue/7614
Signed-off-by: Armando Neto <abiagion@redhat.com>
Reviewed-By: Christian Heimes <cheimes@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>
In order to support pylint 2.0 the following violations must be fixed:
- `chained-comparison` (R1716):
Simplify chained comparison between the operands This message is
emitted when pylint encounters boolean operation like
"a < b and b < c", suggesting instead to refactor it to "a < b < c".
- `consider-using-in` (R1714):
Consider merging these comparisons with "in" to %r To check if a
variable is equal to one of many values,combine the values into a
tuple and check if the variable is contained "in" it instead of
checking for equality against each of the values.This is faster
and less verbose.
Issue: https://pagure.io/freeipa/issue/7614
Signed-off-by: Armando Neto <abiagion@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Aiming to support pylint 2.0 some functions and methods must have their
return statements updated in order to fix two new violations:
- `useless-return` (R1711):
Useless return at end of function or method Emitted when a single
"return" or "return None" statement is found at the end of function
or method definition. This statement can safely be removed because
Python will implicitly return None
- `inconsistent-return-statements` (R1710):
Either all return statements in a function should return an
expression, or none of them should. According to PEP8, if any return
statement returns an expression, any return statements where no value
is returned should explicitly state this as return None, and an
explicit return statement should be present at the end of the
function (if reachable)
Issue: https://pagure.io/freeipa/issue/7614
Signed-off-by: Armando Neto <abiagion@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Calls to `os.fsync(f.fileno())` need to be accompained by `f.flush()`.
Commit 8bbeedc93f introduces the helper
`ipapython.ipautil.flush_sync()`, which handles all calls in the right
order.
However, `flush_sync()` takes as parameter a file object with fileno
and name, where name must be a path to the file, this isn't possible
in some cases where file descriptors are used.
Issue: https://pagure.io/freeipa/issue/7251
Signed-off-by: Armando Neto <abiagion@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
- Add missing executable bits to all scripts
- Remove executable bits from all files that are not scripts,
e.g. js, html, and Python libraries.
- Remove Python shebang from all Python library files.
It's frown upon to have executable library files in site-packages.
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Armando Neto <abiagion@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>
Test more scenarios like replication replica -> master. Verify that master
and replica have all expected certs with correct trust flags and all keys.
See: https://pagure.io/freeipa/issue/7590
See: https://pagure.io/freeipa/issue/7589
Fixes: https://pagure.io/freeipa/issue/7611
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Stanislav Laznicka <slaznick@redhat.com>
The server installation and uninstallation overlaps both the
server and client installers. The output could be confusing
with a server uninstall finishing with the message:
The ipa-client-install command was successful
This was in part due to the fact that the server was not
configured with a console format and verbose was False which
meant that no logger messages were displayed at all.
In order to suppress client installation errors and avoid
confusion add a list of errors to ignore. If a server install
was not successful and hadn't gotten far enough to do the
client install then we shouldn't complain loudly about it.
https://pagure.io/freeipa/issue/6760
Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Stanislav Laznicka <slaznick@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
On multiple occasions, SRV query answers were not properly sorted by
priority. Records with same priority weren't randomized and shuffled.
This caused FreeIPA to contact the same remote peer instead of
distributing the load across all available servers.
Two new helper functions now take care of SRV queries. sort_prio_weight()
sorts SRV and URI records. query_srv() combines SRV lookup with
sort_prio_weight().
Fixes: https://pagure.io/freeipa/issue/7475
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Python 2 doesn't have gzip.decompress(data: bytes) -> bytes function.
Backport the two line function from Python 3.6.
Fixes: https://pagure.io/freeipa/issue/7563
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
We currently accept compressed responses for some Dogtag resources,
via an 'Accept: gzip, deflate' header. But we don't decompress the
received data. Inspect the response Content-Encoding header and
decompress the response body according to its value.
The `gzip.decompress` function is only available on Python 3.2 or
later. In earlier versions, it is necessary to use StringIO and
treat the compressed data as a file. This commit avoids this
complexity. Therefore it should only be included in Python 3 based
releases.
Fixes: https://pagure.io/freeipa/issue/7563
Reviewed-By: Christian Heimes <cheimes@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>
Introduce server installation constants similar to the client
but only tie in SERVER_NOT_CONFIGURED right now.
For the case of not configured don't spit out the "See <some log>
for more information" because no logging was actually done.
In the case of ipa-backup this could also be confusing if the
--log-file option was also passed in because it would not be
used.
https://pagure.io/freeipa/issue/6843
Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Only certutil creates files in the local directory. Changing the
directory for pk12util breaks ipa-server-certinstall if the
PKCS#12 file is not passed in as an absolute path.
https://pagure.io/freeipa/issue/7489
Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexey Slaykovsky <alexey@slaykovsky.com>
Add absolute_import from __future__ so that pylint
does not fail and to achieve python3 behavior in
python2.
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
According to a comment, certutil may create files in the current working
directory. Rather than changing the cwd of the current process,
FreeIPA's certutil wrapper now changes cwd for the subprocess only.
See: https://pagure.io/freeipa/issue/7416
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
The installer now checks that port 8080 is available and not in use by
any other application.
The port checker has been rewritten to use bind() rather than just
checking if a server responds on localhost. It's much more reliable and
detects more problems.
Original patch by m3gat0nn4ge.
Co-authored-by: Mega Tonnage <m3gat0nn4ge@gmail.com>
Fixes: https://pagure.io/freeipa/issue/7415
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
The AdminTool class purports to "call sys.exit() with the return
value" but most of the run implementations returned no value, or
the methods they called returned nothing so there was nothing to
return, so this was a no-op.
The fix is to capture and bubble up the return values which will
return 1 if any exceptions are caught.
This potentially affects other users in that when executing the
steps of an installer or uninstaller the highest return code
will be the exit value of that installer.
Don't use the Continuous class because it doesn't add any
value and makes catching the exceptions more difficult.
https://pagure.io/freeipa/issue/7330
Signed-off-by: Rob Crittenden rcritten@redhat.com
Reviewed-By: Stanislav Laznicka <slaznick@redhat.com>
The disable system trust feature is no longer used.
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Stanislav Laznicka <slaznick@redhat.com>
CertDB no longer makes any assumptions about the default db type of a NSS
DB. Instead it let's certutil decide when dbtype is set to 'auto'. This
makes it much easier to support F27 and F28 from a single code base.
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Stanislav Laznicka <slaznick@redhat.com>
- Refactor CertDB to look up values from its NSSDatabase.
- Add run_modutil() helpers to support sql format. modutil does not
auto-detect the NSSDB format.
- Add migration helpers to CertDB.
- Add explicit DB format to NSSCertificateDatabase stanza
- Restore SELinux context when migrating NSSDB.
- Add some debugging and sanity checks to httpinstance.
The actual database format is still dbm. Certmonger on Fedora 27 does
neither auto-detect DB format nor support SQL out of the box.
https://pagure.io/freeipa/issue/7354
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Stanislav Laznicka <slaznick@redhat.com>
In case of an error, ipapython.ipautil.run() now raises an exception that
contains the error message of the failed command. Before the exception
only contained the command and error code.
The command is no longer collapsed into one string. The error message
and logging output contains the actual command and arguments with intact
quoting.
Example:
CalledProcessError(Command ['/usr/bin/python3', '-c', 'import sys; sys.exit(" ".join(("error", "XXXXXXXX")))'] returned non-zero exit status 1: 'error XXXXXXXX\n')
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Tibor Dudlak <tdudlak@redhat.com>
The Python 3 refactoring effort is finishing, it should be safe
to turn all scripts to run in Python 3 by default.
https://pagure.io/freeipa/issue/4985
Reviewed-By: Christian Heimes <cheimes@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>
Several run() calls used hard-coded paths rather than pre-defined paths
from ipaplatform.paths. The patch fixes all places that I was able to
find with a simple search.
The fix simplifies Darix's port of freeIPA on openSuSE.
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
ipapython/dn.py:1324: [R1710(inconsistent-return-statements), DN.__contains__]
Either all return statements in a function should return an expression, or none of them should.)
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
For loop variable '_nothing' is not used in the loop body. The name
'unused' is used to indicate that a variable is unused.
https://pagure.io/freeipa/issue/7344
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Fraser Tweedale <ftweedal@redhat.com>
