Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

install: fix CA-less PKINIT #758

Closed
wants to merge 14 commits into from
Closed

install: fix CA-less PKINIT #758

wants to merge 14 commits into from

Conversation

HonzaCholasta
Copy link
Contributor

@HonzaCholasta HonzaCholasta commented May 3, 2017

certdb: add named trust flag constants

Add named constants for common trust flag combinations.

Use the named constants instead of trust flags strings in the code.

certdb, certs: make trust flags argument mandatory

Make the trust flags argument mandatory in all functions in certdb and
certs.

certdb: use custom object for trust flags

Replace trust flag strings with TrustFlags objects. The TrustFlags
class encapsulates certstore key policy and has an additional flag
indicating the presence of a private key.

install: trust IPA CA for PKINIT

Trust IPA CA to issue PKINIT KDC and client authentication certificates in
the IPA certificate store.

client install: fix client PKINIT configuration

Set pkinit_anchors in krb5.conf to a CA certificate bundle of CAs
trusted to issue KDC certificates rather than /etc/ipa/ca.crt.

Set pkinit_pool in krb5.conf to a CA certificate bundle of all CAs
known to IPA.

Make sure both bundles are exported in all installation code paths.

server install: fix KDC PKINIT configuration

Make sure cacert.pem contains only certificates of CAs trusted to issue
PKINIT client certificates and is exported in all installation code paths.

Set pkinit_pool in kdc.conf to a CA certificate bundle of all CAs known
to IPA.

Use the KDC certificate itself as a PKINIT anchor in login_password.

certs: do not export CA certs in install_pem_from_p12

This fixes kdc.crt containing the full chain rather than just the KDC
certificate in CA-less server install.

server install: fix KDC certificate validation in CA-less

Verify that the provided certificate has the extended key usage and subject
alternative name required for KDC.

cacert manage: support PKINIT

Allow installing 3rd party CA certificates trusted to issue PKINIT KDC
and/or client certificates.

server certinstall: support PKINIT

Allow replacing the KDC certificate.

https://pagure.io/freeipa/issue/6831
https://pagure.io/freeipa/issue/6869

@HonzaCholasta HonzaCholasta requested review from martbab and a user May 3, 2017 13:26
@stlaz stlaz self-assigned this May 4, 2017
@martbab martbab self-assigned this May 4, 2017
trust_flags[i] += 'u'

trust_flags = ','.join(trust_flags)
return trust_flags
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                      this would be perfect for unit testing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I second that, a unittest for flag parsing/unparsing would be welcome.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please file a ticket, this is old code moved here from certstore.py, adding unit tests for it is out of the scope of this PR.

Copy link
Contributor

@martbab martbab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only a few nitpicks for now.

trust_flags[i] += 'u'

trust_flags = ','.join(trust_flags)
return trust_flags
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I second that, a unittest for flag parsing/unparsing would be welcome.

raise admintool.ScriptError(
"Peer's certificate issuer is not trusted (%s). "
"Please run ipa-cacert-manage install and ipa-certupdate "
"to install the CA certificate." % str(e))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer new-style string interpolation. Also the str(e) is rather redundant here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is consistent with the code in ServerCertInstall.check_chain() and the lack of refactoring is intentional.

@stlaz
Copy link
Contributor

stlaz commented May 9, 2017

External CA (rebased on current master to be able to install):

$ kinit -n
kinit: Invalid certificate while getting initial credentials
$ /usr/bin/kinit -n -c /var/run/ipa/ccaches/armor_9588 -X X509_anchors=FILE:/var/kerberos/krb5kdc/kdc.crt -X X509_anchors=FILE:/var/kerberos/krb5kdc/cacert.pem
kinit: Invalid certificate while getting initial credentials

and on replica:

$ kinit -n
kinit: Preauthentication failed while getting initial credentials

=> this breaks WebUI on external CA installations.

=================================
CA-less with --no-pkinit:

$ kinit -n
kinit: Preauthentication failed while getting initial credentials

but I guess that's expected, WebUI works since the following does work as well:

$ /usr/bin/kinit -n -X X509_anchors=FILE:/var/kerberos/krb5kdc/kdc.crt -X X509_anchors=FILE:/var/kerberos/krb5kdc/cacert.pem

=================================
In CA-less with PKINIT options, kinit -n works fine, although replica installation will produce:

Configuring Kerberos KDC (krb5kdc)
  [1/1]: installing X509 Certificate for PKINIT
ipa         : ERROR    PKINIT certificate request failed: Certificate issuance failed (CA_UNREACHABLE)
ipa         : ERROR    Failed to configure PKINIT
Done configuring Kerberos KDC (krb5kdc).

when run with own PKINIT certificate from --pkinit-cert-file option. I don't think it should be asking any CA for a certificate if we already have the certificate.

@HonzaCholasta
Copy link
Contributor Author

@stlaz, FTFY. Also fixed wrong permissions on the CA-less KDC key file (props to @dkupka).

The "preauthentication failed" with --no-pkinit is expected indeed.

@stlaz
Copy link
Contributor

stlaz commented May 15, 2017

kinit -n still fails with my external CA setup. I found out the reason is that I have a self-sign certificate in the trust chain:

[36993] 1494834859.113259: PKINIT client could not verify DH reply
[36993] 1494834859.113276: Preauth module pkinit (17) (real) returned: -1765328313/Failed to verify received certificate (depth 2): self signed certificate in certificate chain
kinit: Invalid certificate while getting initial credentials

This does not happen without this patchset so the question is whether it is OK that this is happening or not. If so, we should add a check which would prevent this + probably warn our QA team because I guess this is just the way they are testing this.

@HonzaCholasta
Copy link
Contributor Author

@stlaz, this seems to be a bug in kinit. When you have a certificate chain root CA -> intermediate CA -> KDC and want to trust the intermediate CA, but not the root CA, the validation will always fail. This is the case in external CA setup (the external CA is the root CA, IPA CA is the intermediate CA), but I haven't confirmed it without IPA yet.

Without this patchset, both the CA certificates are trusted, which is a bug, but makes kinit work.

@tkrizek tkrizek added the prioritized Pull Request has higher priority for PR-CI label May 17, 2017
@HonzaCholasta
Copy link
Contributor Author

Fixed kdc.conf upgrade.

@stlaz
Copy link
Contributor

stlaz commented May 19, 2017

Upgrade from 4.4 to 4.5 during external-CA installation prints error messages, related log:

2017-05-19T07:08:04Z INFO [Setup PKINIT]
2017-05-19T07:08:04Z DEBUG raw: ca_is_enabled(version=u'2.225')
2017-05-19T07:08:04Z DEBUG ca_is_enabled(version=u'2.225')
2017-05-19T07:08:04Z DEBUG certmonger request is in state dbus.String(u'GENERATING_KEY_PAIR', variant_level=1)
2017-05-19T07:08:09Z DEBUG certmonger request is in state dbus.String(u'CA_UNREACHABLE', variant_level=1)
2017-05-19T07:08:09Z ERROR PKINIT certificate request failed: Certificate issuance failed (CA_UNREACHABLE)
2017-05-19T07:08:09Z ERROR Failed to configure PKINIT
2017-05-19T07:08:09Z DEBUG certmonger request is in state dbus.String(u'GENERATING_CSR', variant_level=1)
2017-05-19T07:08:14Z DEBUG certmonger request is in state dbus.String(u'MONITORING', variant_level=1)
2017-05-19T07:08:14Z DEBUG Starting external process

but as you can see, CA is enabled and running.

@stlaz
Copy link
Contributor

stlaz commented May 19, 2017

Did not realize this was unrelated to your patches. Please rebase.

Jan Cholasta and others added 14 commits May 19, 2017 09:36
Add named constants for common trust flag combinations.

Use the named constants instead of trust flags strings in the code.

https://pagure.io/freeipa/issue/6831
Make the trust flags argument mandatory in all functions in `certdb` and
`certs`.

https://pagure.io/freeipa/issue/6831
Replace trust flag strings with `TrustFlags` objects. The `TrustFlags`
class encapsulates `certstore` key policy and has an additional flag
indicating the presence of a private key.

https://pagure.io/freeipa/issue/6831
Trust IPA CA to issue PKINIT KDC and client authentication certificates in
the IPA certificate store.

https://pagure.io/freeipa/issue/6831
Set `pkinit_anchors` in `krb5.conf` to a CA certificate bundle of CAs
trusted to issue KDC certificates rather than `/etc/ipa/ca.crt`.

Set `pkinit_pool` in `krb5.conf` to a CA certificate bundle of all CAs
known to IPA.

Make sure both bundles are exported in all installation code paths.

https://pagure.io/freeipa/issue/6831
Introduce new IPAKrb5 lens to handle krb5.conf and kdc.conf changes using
Augeas. The stock Krb5 lens does not work on our krb5.conf and kdc.conf.

https://pagure.io/freeipa/issue/6831
Set `pkinit_pool` in `kdc.conf` to a CA certificate bundle of all CAs known
to IPA.

Make sure `cacert.pem` is exported in all installation code paths.

Use the KDC certificate itself as a PKINIT anchor in `login_password`.

https://pagure.io/freeipa/issue/6831
Make sure the exported private key files are readable only by the owner.

https://pagure.io/freeipa/issue/6831
This fixes `kdc.crt` containing the full chain rather than just the KDC
certificate in CA-less server install.

https://pagure.io/freeipa/issue/6831
https://pagure.io/freeipa/issue/6869
Verify that the provided certificate has the extended key usage and subject
alternative name required for KDC.

https://pagure.io/freeipa/issue/6831
https://pagure.io/freeipa/issue/6869
When --pkinit-cert-file is used, make sure the certificate and key is
actually passed to `KrbInstance`.

https://pagure.io/freeipa/issue/6831
Allow installing 3rd party CA certificates trusted to issue PKINIT KDC
and/or client certificates.

https://pagure.io/freeipa/issue/6831
@stlaz stlaz added the ack Pull Request approved, can be merged label May 19, 2017
@MartinBasti
Copy link
Contributor

master:

  • 235265a certdb: add named trust flag constants
  • f0442a2 certdb, certs: make trust flags argument mandatory
  • 52730c7 certdb: use custom object for trust flags
  • 01a7416 install: trust IPA CA for PKINIT
  • 11b8a34 client install: fix client PKINIT configuration
  • 4d36cbf install: introduce generic Kerberos Augeas lens
  • f769045 server install: fix KDC PKINIT configuration
  • b9fd123 ipapython.ipautil.run: Add option to set umask before executing command
  • 0c5b2c4 certs: do not export keys world-readable in install_key_from_p12
  • cc57237 certs: do not export CA certs in install_pem_from_p12
  • 3b5dbf7 server install: fix KDC certificate validation in CA-less
  • b385570 replica install: respect --pkinit-cert-file
  • 9ea764e cacert manage: support PKINIT
  • 96ca62f server certinstall: support PKINIT

@MartinBasti MartinBasti added the pushed Pull Request has already been pushed label May 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged prioritized Pull Request has higher priority for PR-CI pushed Pull Request has already been pushed
Projects
None yet
5 participants