Commit Graph

3282 Commits

Author SHA1 Message Date
Fraser Tweedale
4cf9c8689f httpinstance: add fqdn and ipa-ca alias to Certmonger request
BACKGROUND:

We are implementing ACME support in FreeIPA (umbrella ticket:
https://pagure.io/freeipa/issue/4751).  ACME is defined in RFC 8555.
HTTPS is REQUIRED (https://tools.ietf.org/html/rfc8555#section-6.1).
Therefore, every FreeIPA server that provides the ACME service
capability must be reachable by HTTPS.

RFC 8555 does not say anything about which port to use for ACME.
The default HTTPS port of 443 is implied.  Therefore, the FreeIPA
ACME service will be reached via the Apache httpd server, which will
be the TLS server endpoint.

As a usability affordance for ACME clients, and as a maintainability
consideration i.e. to allow the topology to change without having to
reconfigure ACME clients, there should be a a single DNS name used
to reach the IPA ACME service.

The question then, is which DNS name to use.

REQUIREMENTS:

Each FreeIPA server that is also an ACME server must:

1. Be reachable via a common DNS name

2. Have an HTTP service certificate with that DNS name as a SAN
   dNSName value

DESIGN CONSIDERATION - WHAT DNS NAME TO USE?:

Some unrelated FreeIPA ACME design decisions provide important
context for the DNS name decision:

- The ACME service will be automatically and unconditionally
  deployed (but not necessarily *enabled*) on all CA servers.

- Enabling or disabling the ACME service will have topology-wide
  effect, i.e. the ACME service is either enabled on all CA
  servers, or disabled on all CA servers.

In a CA-ful FreeIPA deployment there is already a DNS name that
resolves to all CA servers: ``ipa-ca.$DOMAIN``, e.g.
``ipa-ca.example.com``.  It is expected to point to all CA servers
in the deployment, and *only* to CA servers.  If internal DNS is
deployed, the DNS records for ``ipa-ca.$DOMAIN`` are created and
updated automatically.  If internal DNS is not deployed,
administrators are required to maintain these DNS records
themselves.

The ``ipa-ca.$DOMAIN`` alias is currently used for OCSP and CRL
access.  TLS is not required for these applications (and it can
actually be problematic for OCSP).  Enabling TLS for this name
presents some risk of confusion for operators.  For example, if they
see that TLS is available and alter the certificate profiles to
include an HTTPS OCSP URL in the Authority Information Access (AIA)
extension, OCSP-using clients may fail to validate such
certificates.  But it is possible for administrators to make such a
change to the profile, whether or not HTTPS is available.

One big advantage to using the ``ipa-ca.$DOMAIN`` DNS name is that
there are no new DNS records to manage, either in the FreeIPA
implementation or for administrators in external DNS systems.

The alternative approach is to define a new DNS name, e.g.
``ipa-acme.$DOMAIN``, that ACME clients would use.  For internal
DNS, this means the FreeIPA implementation must manage the DNS
records.  This is straightforward; whenever we add or remove an
``ipa-ca.$DOMAIN`` record, also add/remove the ``ipa-acme.$DOMAIN``
record.  But for CA-ful deployments using external DNS, it is
additional work for adminstrators and, unless automated, additional
room for error.

An advantage of using a different DNS name is ``ipa-ca.$DOMAIN`` can
remain inaccessible over HTTPS.  This possibly reduces the risk of
administrator confusion or creation of invalid AIA configuration in
certificate profiles.

Weighing up the advantages and disadvantages, I decided to use the
``ipa-ca.$DOMAIN`` DNS name.

DESIGN CONSIDERATION - CA SERVERS, OR ALL SERVERS?:

A separate decision from which name to use is whether to include it
on the HTTP service certificate for ACME servers (i.e. CA servers)
only, or on all IPA servers.

Combined with the assumption that the chosen DNS name points to CA
servers *only*, there does not seem to be any harm in adding it to
the certificates on all IPA servers.

The alternative is to only include the chosen DNS name on the HTTP
service certificates of CA servers.  This approach entails some
additional complexity:

- If a non-CA replica gets promoted to CA replica (i.e. via
  ``ipa-ca-install``), its HTTP certificate must be re-issued with
  the relevant name.

- ipa-server-upgrade code must consider whether the server is a CA
  replica when validating (and if necessary re-creating) Certmonger
  tracking requests

- IPA Health Check must be made aware of this factor when checking
  certificates and Certmonger tracking requests.

Weighing up the options, I decided to add the common DNS name to the
HTTP service certificate on all IPA servers.  This avoids the
implementation complexity discussed above.

CHANGES IN THIS COMMIT

When (re-)tracking the HTTP certificate, explicitly add the server
FQDN and ipa-ca.$DOMAIN DNS names to the Certmonger tracking request.

Related changes follow in subsequent commits.

Part of: https://pagure.io/freeipa/issue/8186

Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2020-03-25 11:13:03 +11:00
Fraser Tweedale
18ebd1116d certmonger: support dnsname as request search criterion
We need to be able to filter Certmonger tracking requests by the DNS
names defined for the request.  The goal is to add the
'ipa-ca.$DOMAIN' alias to the HTTP certificate tracking requests, so
we will use that name as a search criterion.  Implement support for
this.

As a result of this commit it will be easy to add support for subset
match of other Certmonger request list properties.  Just add the
property name to the ARRAY_PROPERTIES list (and update the
'criteria' description in the module docstring!)

Part of: https://pagure.io/freeipa/issue/8186

Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2020-03-25 11:13:03 +11:00
Fraser Tweedale
e0fb3816f6 certmonger: move 'criteria' description to module docstring
The 'criteria' parameter is used by several subroutines in the
ipalib.install.certmonger module.  It has incomplete documentation
spread across several of these subroutines.  Move the documentation
to the module docstring and reference it where appropriate.

Part of: https://pagure.io/freeipa/issue/8186

Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2020-03-25 11:13:03 +11:00
Fraser Tweedale
0711c4a0d4 certmonger: avoid mutable default argument
certmonger._get_requests has a mutable default argument.  Although
at the present time it is never modified, this is an antipattern to
be avoided.

In fact, we don't even need the default argument, because it is
always called with a dict() argument.  So just remove it.

Part of: https://pagure.io/freeipa/issue/8186

Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2020-03-25 11:13:03 +11:00
Stanislav Levin
a309de6c08 pylint: Clean up comment
I added a comment in @d0b420f6d, later, on refactoring in
@c6769ad12 I forgot to remove it. So, it is just a clean up.

Related: https://pagure.io/freeipa/issue/8116
Signed-off-by: Stanislav Levin <slev@altlinux.org>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
2020-02-12 18:08:32 +02:00
Fraser Tweedale
769180c2c6 Do not renew externally-signed CA as self-signed
Commit 49cf5ec64b fixed a bug that
prevented migration from externally-signed to self-signed IPA CA.
But it introduced a subtle new issue: certmonger-initiated renewal
renews an externally-signed IPA CA as a self-signed CA.

To resolve this issue, introduce the `--force-self-signed' flag for
the dogtag-ipa-ca-renew-agent script.  Add another certmonger CA
definition that calls this script with the `--force-self-signed'
flag.  Update dogtag-ipa-ca-renew-agent to only issue a self-signed
CA certificate if the existing certificate is self-signed or if
`--force-self-signed' was given.  Update `ipa-cacert-manage renew'
to supply `--force-self-signed' when appropriate.

As a result of these changes, certmonger-initiated renewal of an
externally-signed IPA CA certificate will not issue a self-signed
certificate.

Fixes: https://pagure.io/freeipa/issue/8176
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
2020-01-29 21:47:14 +11:00
Christian Heimes
cf9f9bb326 Fix logic of check_client_configuration
The helper function ipalib.util.check_client_configuration() now
considers a client configured when either:

* confdir is overridden (e.g. with IPA_CONFDIR) and the conf_default
  file exists.
* confdir is /etc/ipa, /etc/ipa/default.conf exists and client
  sysrestore state exists.

The check for sysrestore state is faster than checking for the presence
of the directory and presence of files in the directory. The sysrestore
state is always presence. sysrestore.index may be missing if no files
were backed up.

Fixes: https://pagure.io/freeipa/issue/8133
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2019-12-05 15:09:38 +01:00
Christian Heimes
639bb71940 Don't hard-code client's TLS versions and ciphers
Client connections no longer override TLS version range and ciphers by
default. Instead clients use the default settings from the system's
crypto policy.

Minimum TLS version is now TLS 1.2. The default crypto policy on
RHEL 8 sets TLS 1.2 as minimum version, while Fedora 31 sets TLS 1.0 as
minimum version. The minimum version is configured with OpenSSL 1.1.1
APIs. Python 3.6 lacks the setters to override the system policy.

The effective minimum version is always TLS 1.2, because FreeIPA
reconfigures Apache HTTPd on Fedora.

Fixes: https://pagure.io/freeipa/issue/8125
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Thomas Woerner <twoerner@redhat.com>
2019-12-02 16:48:07 +01:00
Christian Heimes
0451db9d3f Enable TLS 1.3 support on the server
urllib3 now supports post-handshake authentication with TLS 1.3. Enable
TLS 1.3 support for Apache HTTPd.

The update depends on bug fixes for TLS 1.3 PHA support in urllib3 and
Apache HTTPd. New builds are available in freeipa-master COPR and in
F30/F31.

Overwrite crypto-policy on Fedora only. Fedora 31 and earlier have TLS
1.0 and 1.1 still enabled by default.

Fixes: https://pagure.io/freeipa/issue/8125
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Thomas Woerner <twoerner@redhat.com>
2019-12-02 16:48:07 +01:00
Florence Blanc-Renaud
e08a6de6af DNParam: raise Exception when multiple values provided to a 1-val param
When ipa user-add-certmapdata is called with multiple --subject or
multiple --issuer, the DNParam's _convert_scalar method is called with
a tuple containing all the params and should raise an exception as the
--subject and --issuer are single-value params.

The DNParam _convert_scalar method internally calls the DN init method,
and the DN init method is able to create a DN from a tuple of RDNs.
As such, it won't raise exception if a tuple/list is provided.

Check that _convert_scalar is only provided a single element.

Fixes: https://pagure.io/freeipa/issue/8097
Signed-off-by: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2019-11-20 11:15:28 +01:00
Stanislav Levin
c6769ad12f Fix errors found by Pylint-2.4.3
New Pylint (2.4.3) catches several new 'true problems'. At the same
time, it warns about things that are massively and reasonably
employed in FreeIPA.

list of fixed:
- no-else-continue
- redeclared-assigned-name
- no-else-break
- unnecessary-comprehension
- using-constant-test (false positive)

list of ignored (responsibility of contributors and reviewers):
- import-outside-toplevel

Fixes: https://pagure.io/freeipa/issue/8102
Signed-off-by: Stanislav Levin <slev@altlinux.org>
Reviewed-By: Fraser Tweedale <ftweedal@redhat.com>
2019-10-21 18:01:32 +11:00
Rafael Guterres Jeffman
51e0f56487 Removed unnecessary imports after code review.
Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
2019-09-27 09:38:32 +02:00
Rafael Guterres Jeffman
c898be1df9 Removes several pylint warnings.
This patche removes 93 pylint deprecation warnings due to invalid escape
sequences (mostly 'invalid escape sequence \d') on unicode strings.

Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
2019-09-27 09:38:32 +02:00
Stanislav Levin
d0b420f6dd Fixed errors newly exposed by pylint 2.4.0
Newest Pylint introduced additional checks [1]:

- import-outside-toplevel [2]

> This check warns when modules are imported from places other
than a module toplevel, e.g. inside a function or a class.

- no-else-continue [3]

> These checks highlight unnecessary else and elif blocks after
break and continue statements.

- unnecessary-comprehension [4]

> This check is emitted when pylint finds list-, set- or
dict-comprehensions, that are unnecessary and can be rewritten
with the list-, set- or dict-constructors.

[1] https://github.com/PyCQA/pylint/blob/pylint-2.4.0/doc/whatsnew/2.4.rst
[2] https://github.com/PyCQA/pylint/issues/3067
[3] https://github.com/PyCQA/pylint/issues/2327
[4] https://github.com/PyCQA/pylint/issues/2905

Fixes: https://pagure.io/freeipa/issue/8077
Signed-off-by: Stanislav Levin <slev@altlinux.org>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
2019-09-25 20:14:06 +10:00
Rob Crittenden
5b28c458b9 Report if a certmonger CA is missing
If a certmonger CA is not defined but is referenced within
a request (so was removed sometime after a request was
created) then anything that pulls all certmonger requests would
fail with the cryptic error:

"Failed to get request: bus, object_path and dbus_interface
must not be None."

This was often seen during upgrades.

Catch this specific condition and report a more specific error
so the user will have some bread crumb to know how to address
the issue.

https://pagure.io/freeipa/issue/7870

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
2019-09-20 10:16:57 -04:00
Rob Crittenden
faf34fcdfd Replace replication_wait_timeout with certmonger_wait_timeout
The variable is intended to control the timeout for replication
events. If someone had significantly reduced it via configuration
then it could have caused certmogner requests to fail due to timeouts.

Add replication_wait_timeout, certmonger_wait_timeout and
http_timeout to the default.conf man page.

Related: https://pagure.io/freeipa/issue/7971
Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
2019-09-04 14:52:14 +02:00
Christian Heimes
3c82585e52 Update comments to explain caSubsystemCert switch
Related: https://bugzilla.redhat.com/1670239
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Fraser Tweedale <ftweedal@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
2019-08-08 09:46:10 +02:00
Alexander Bokovoy
802a54bfc8 Change RA agent certificate profile to caSubsystemCert
Currently, RA agent certificate is issued using caServerCert profile.
This has unfortunate side effect of asserting id-pk-serverAuth EKU which
is not really needed for RA agent. If IPA CA certificate adds SAN DNS
constraints into issued certificates, presence of id-pk-serverAuth EKU
forces NSS (and other crypto libraries) to validate CN value with
regards to SAN DNS constraints, due to historical use of CN bearing DNS
name.

Since RA agent certificate has 'CN=IPA RA', it is guaranteed to fail
the check.

Default IPA CA configuration does *not* add SAN DNS constraints into RA
agent certificate. However, it is better to be prepared to such
behavior.

Related: https://bugzilla.redhat.com/1670239
Reviewed-By: Fraser Tweedale <ftweedal@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
2019-08-08 09:46:10 +02:00
Fraser Tweedale
1bf008a64f cainstance: add profile to IPA RA tracking request
Profile-based renewal means we should always explicitly specify the
profile in tracking requests that use the dogtag-ipa-ca-renew-agent
renewal helper.  This includes the IPA RA agent certificate.  Update
CAInstance.configure_agent_renewal() to add the profile to the
tracking request.  This also covers the upgrade scenario (because
the same method gets invoked).

Part of: https://pagure.io/freeipa/issue/7991

Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2019-07-22 13:33:24 +10:00
Fraser Tweedale
130e1dc343 move MSCSTemplate classes to ipalib
As we expand the integration tests for external CA functionality, it
is helpful (and avoids duplication) to use the MSCSTemplate*
classes.  These currently live in ipaserver.install.cainstance, but
ipatests is no longer permitted to import from ipaserver (see commit
81714976e5e13131654c78eb734746a20237c933).  So move these classes to
ipalib.

Part of: https://pagure.io/freeipa/issue/7548

Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
2019-07-17 17:58:58 +03:00
Christian Heimes
b553448884 Use system-wide crypto policy for TLS ciphers
IPA now uses the system-wide crypto policy for TLS ciphers on RHEL. It's
also now possible to keep the default policy by setting TLS_HIGH_CIPHERS
to None.

Fixes: https://pagure.io/freeipa/issue/7998
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
2019-07-02 16:38:00 +02:00
Christian Heimes
b57c818fab Use only TLS 1.2 by default
TLS 1.3 is causing some trouble with client cert authentication.
Conditional client cert authentication requires post-handshake
authentication extension on TLS 1.3. The new feature is not fully
implemented yet.

TLS 1.0 and 1.1 are no longer state of the art and now disabled by
default.

TLS 1.2 works everywhere and supports PFS.

Related: https://pagure.io/freeipa/issue/7667

Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2019-07-01 14:55:29 +02:00
Florence Blanc-Renaud
bb91fcabee dnsrecord-mod: allow to modify ttl without passing the record
The command
ipa dnsrecord-mod <zone> <record> --ttl
requires to provide at least one record to modify. When none
is specified, it prompts by proposing each of the existing records,
for instance:
ipa dnsrecord-mod ZZZZZ.org ns11 --ttl=86400
No option to modify specific record provided.
Current DNS record contents:

A record: xxx.xxx.xxx.xxx
AAAA record: xxxx:xx

Modify A record 'xxxx.xxxx.xxxx.xxxx'? Yes/No (default No):
Modify AAAA record 'xxxx:xx'? Yes/No (default No):
ipa: ERROR: No options to modify a specific record provided.

The admin should be able to modify the TTL value without
re-entering the record information. The issue happens because of an
internal check that forgot to consider 'dnsttl' as a valid standalone
modification.

Fixes: https://pagure.io/freeipa/issue/7982
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
2019-07-01 09:16:21 +02:00
Fraser Tweedale
854d3053e2 Handle missing LWCA certificate or chain
If lightweight CA key replication has not completed, requests for
the certificate or chain will return 404**.  This can occur in
normal operation, and should be a temporary condition.  Detect this
case and handle it by simply omitting the 'certificate' and/or
'certificate_out' fields in the response, and add a warning message
to the response.

Also update the client-side plugin that handles the
--certificate-out option.  Because the CLI will automatically print
the warning message, if the expected field is missing from the
response, just ignore it and continue processing.

** after the Dogtag NullPointerException gets fixed!

Part of: https://pagure.io/freeipa/issue/7964

Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Fraser Tweedale <ftweedal@redhat.com>
2019-06-18 10:36:24 +10:00
Fraser Tweedale
a3becc76dd constants: add ca_renewal container
Part of: https://pagure.io/freeipa/issue/7885

Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
2019-05-29 12:49:27 +10:00
Rob Crittenden
6662e99e17 Add knob to limit hostname length
On Linux systems the length limit for hostnames is hardcoded
at 64 in MAXHOSTNAMELEN

Solaris, for example, allows 255 characters, and DNS allows the
total length to be up to 255 (with each label < 64).

Add a knob to allow configuring the maximum hostname length (FQDN)

The same validators are used between hosts and DNS to apply
the knob only when dealing with a FQDN as a hostname.

The maxlen option is included so installers can limit the length
of allowed hostnames when the --hostname option is used.

https://pagure.io/freeipa/issue/2018

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
2019-05-16 14:38:43 -04:00
Christian Heimes
2042b5a0d2 Use PKCS#8 instead of traditional privkey format
The modern PKCS#8 private key format supports better encryption standard
and is preferable over traditional, weak PKCS#1 key format.

Fixes: https://pagure.io/freeipa/issue/7943
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Oleg Kozlov <okozlov@redhat.com>
2019-05-14 17:11:54 +02:00
Alexander Bokovoy
294aa3a333 Revert "Require a minimum SASL security factor of 56"
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>
2019-05-02 11:39:23 +02:00
Christian Heimes
8686cd3b4b Pass token_name to certmonger
For HSM support, IPA has to pass the token name for CA and subsystem
certificates to certmonger. For now, only the default 'internal' token is
supported.

Related: https://pagure.io/freeipa/issue/5608
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2019-04-25 08:57:58 +02:00
Christian Heimes
5452eb6e77 Reduce startup_timeout to 120sec as documented
man(5) default.conf says that startup_timeout has a default value of 120
seconds. Even 120 seconds are not effective unless systemd is also
reconfigured to have a larger DefaultTimeoutStartSec.

Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
2019-04-24 09:09:28 +02:00
Christian Heimes
36c65c4aaa Remove deprecated object logger
The object logger methods been deprecated for about two years since release
4.6.0. The log manager used to moneky-patch additional log methods like
info(), warning(), and error() into API plugin objects. The methods have
been replaced by calls to module logger objects in 4.6.0.

Remove monkey-patch logger methods, log manager, and its root logger from
ipapython.ipa_log_manager.

Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Thomas Woerner <twoerner@redhat.com>
2019-04-23 12:55:35 +02:00
Christian Heimes
00a7868fa4 Reuse key type and size in certmonger resubmit
Certmonger has hard-coded defaults for key size and key type. In case a
request does not contain these values, certmonger uses 2048 RSA keys.
Since the CA now has 3072, it will also rekey the CA to 2048 instead of
resubmitting with the existing 2048 bit key.

Use key-size and key-type from the existing request when resubmitting.

Related: https://pagure.io/freeipa/issue/6790
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
2019-04-16 16:51:40 +02:00
Christian Heimes
3509545897
Require a minimum SASL security factor of 56
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>
2019-04-03 15:16:21 +02:00
Christian Heimes
19b1eb1f5a Use expanduser instead of HOME env var
The HOME directory may not be available in containers. It's also the
wrong variable on some platforms. Use os.path.expanduser() instead of
HOME.

Fixes: https://pagure.io/freeipa/issue/7837
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
2019-01-29 12:44:19 +01:00
Alexander Bokovoy
2aa24eedf2 make sure IPA_CONFDIR is used to check that client is configured
Fixes a test ipatests/test_cmdline/test_cli.py:test_cli_fs_encoding()
which sets IPA_CONFDIR and attempts to interpret the resulting error
message. However, if the test is run on an enrolled machine (a
developer's laptop, for example), check_client_configuration() will
succeed because it ignores IPA_CONFDIR and, as result, api.finalize()
will fail later with a stacktrace.

Pass an environment object and test an overridden config file existence
in this case to fail with a proper and expected message.

Reviewed-By: Christian Heimes <cheimes@redhat.com>
2019-01-10 11:24:08 +01:00
Oleg Kozlov
a0e09526b3 Check pager's executable before subprocess.Popen
Get the value of `PAGER` environment variable in case it's defined, check the executable, if it exists - use a pager, otherwise - print function.

Fixes: https://pagure.io/freeipa/issue/7746
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2018-12-07 14:06:29 +01:00
Christian Heimes
533a5b2633 pylint 2.2: Fix unnecessary pass statement
pylint 2.2.0 has a new checker for unnecessary pass statements. There is
no need to have a pass statement in functions or classes with a doc
string.

Fixes: https://pagure.io/freeipa/issue/7772
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Thomas Woerner <twoerner@redhat.com>
2018-11-26 16:54:43 +01:00
Fraser Tweedale
b8007e14cc rpc: always read response
If the server responds 401 and the response body is empty, the
client raises ResponseNotReady.  This occurs because:

1. For a non-200 response, the response read only if the
   Content-Length header occurs.

2. The response must be read before another request (e.g. the
   follow-up request with WWW-Authenticate header set), and this
   condition was not met.  For details see
   https://github.com/python/cpython/blob/v3.6.7/Lib/http/client.py#L1305-L1321.

This situation should not arise in regular use, because the client
either has a session cookie, or, knowing the details of the server
it is contacting, it establishes the GSS-API context and includes
the WWW-Authenticate header in the initial request.

Nevertheless, this problem has been observed in the wild.  I do not
know its ordinary cause(s), but one can force the issue by removing
an authenticated user's session cache from /run/ipa/ccaches, then
performing a request.

Resolve the issue by always reading the response.  It is safe to
call response.read() regardless of whether the Content-Length header
appears, or whether the body is empty.

Fixes: https://pagure.io/freeipa/issue/7752
Reviewed-By: Christian Heimes <cheimes@redhat.com>
2018-11-07 08:39:42 +01:00
Rob Crittenden
e59ee6099f Fix misleading errors during client install rollback
Some incorrect errors are possible if a client installation
fails and a configuration rollback is required.

These include:

1. Unconfigured automount client failed: CalledProcessError(Command
['/usr/sbin/ipa-client-automount', '--uninstall', '--debug']
returned non-zero exit status 1: '')

Caused by check_client_configuration() not returning the correct
return value (2).

2. WARNING: Unable to revert to the pre-installation state ('authconfig'
tool has been deprecated in favor of 'authselect'). The default sssd
profile will be used instead.
The authconfig arguments would have been: authconfig --disableldap
--disablekrb5 --disablesssdauth --disablemkhomedir

If installation fails before SSSD is configured there is no state
to roll back to. Detect this condition.

3. An error occurred while removing SSSD's cache.Please remove the
cache manually by executing sssctl cache-remove -o.

Again, if SSSD is not configured yet then there is no cache to
remove. Also correct the missing space after the period.

https://pagure.io/freeipa/issue/7729

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
2018-10-26 08:11:03 +02:00
Christian Heimes
15d5e44ee8 Py3: Replace six.moves imports
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>
2018-10-05 12:06:19 +02:00
Christian Heimes
ea396528b7 Py3: Replace six.integer_types with int
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>
2018-09-27 16:11:18 +02:00
Christian Heimes
964a9bdcec Py3: Replace six.string_types with str
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>
2018-09-27 16:11:18 +02:00
Christian Heimes
b431e9b684 Py3: Remove subclassing from object
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>
2018-09-27 11:49:04 +02:00
Christian Heimes
4a58adf79e Sprinkle raw strings across the code base
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>
2018-09-27 10:23:03 +02:00
Christian Heimes
e1a30d3ce3 Workaround for pyasn1 0.4
pyasn1 0.4 changed handling of ANY containers in a backwards
incompatible way. For 0.3.x, keep explicit wrap and unwrap in octet
strings for ANY container members. For >= 0.4, let pyasn1 do the job.

This patch also makes sorting of extended_key_usage_bytes() stable and
adds tests.

Tested with pyasn1 0.3.7 and 0.4.4.

Fixes: https://pagure.io/freeipa/issue/7685
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Fraser Tweedale <ftweedal@redhat.com>
2018-09-27 06:55:34 +02:00
Thomas Woerner
374138d030 Remove replica_file knob from ipalib/install/service.py
The replica_file option is not needed anymore. Threfore the option can
be removed.

See: https://pagure.io/freeipa/issue/7689
Signed-off-by: Thomas Woerner <twoerner@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
2018-09-12 13:11:21 +02:00
Thomas Woerner
0ce79ec6f5 Mark replica_file option as deprecated
The replica_file option is only supported for DL0. The option will be
marked deprecated for now.

See: https://pagure.io/freeipa/issue/7669
Signed-off-by: Thomas Woerner <twoerner@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
2018-08-13 12:35:06 +02:00
Thomas Woerner
38936817d2 Increase MIN_DOMAIN_LEVEL to DOMAIN_LEVEL_1
With increasing the minimal domain level to 1 ipa-replica-install will
refuse to install if the domain has domain level 0.

See: https://pagure.io/freeipa/issue/7669
Signed-off-by: Thomas Woerner <twoerner@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
2018-08-13 12:35:06 +02:00
Armando Neto
d13571942e Fix Pylint 2.0 violations
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>
2018-07-14 12:04:19 +02:00
Christian Heimes
aacf185ff8 Add pylint ignore to magic config.Env attributes
pylinti 2 is having a hard time to handle name mangled, magic attributes
correctly. Double under attributes like __d are internally renamed to
_Env__d. After multiple failed attempts, it was easier to just add more
pylint disable to the implementation.

pylint 2 also thinkgs that Env.server is defined much later or the env
doesn't have that member at all. Ignore the false warnings, too.

Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Armando Neto <abiagion@redhat.com>
2018-07-13 19:56:03 +02:00