-
Notifications
You must be signed in to change notification settings - Fork 21
Kerberos-based authentication #312
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
base: master
Are you sure you want to change the base?
Conversation
| ipa_manage_sssd: | ||
| help: Whether to manage SSSD service and configuration for IPA authentication | ||
| action: store_true | ||
| pam_service: | ||
| help: Name of the PAM service to use for IPA authentication | ||
| gssapi_local_name: | ||
| help: Whether to use the local name for GSSAPI authentication | ||
| action: store_true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to explicitly expose those or leave EXTRA_VARS as the only option for overriding those?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's currently exposed in the Puppet-based installer, but not documented in our docs, so I'd err on the side of leaving it out, but I also don't recall when this is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but I also don't recall when this is needed
The original redmine which introduced it says it might be useful to workaround bugs like https://bugzilla.redhat.com/show_bug.cgi?id=1787630 . Let's not expose it for the time being
src/roles/httpd/tasks/main.yml
Outdated
| cmd: | | ||
| KRB5CCNAME=KEYRING:session:get-http-service-keytab kinit -k || true | ||
| KRB5CCNAME=KEYRING:session:get-http-service-keytab /usr/sbin/ipa-getkeytab -k {{ httpd_ipa_keytab }} -p HTTP/{{ ansible_facts['fqdn'] }} | ||
| kdestroy -c KEYRING:session:get-http-service-keytab || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be probably broken into several individual single-command tasks, but I'm not sure if it's worth it
265bc56 to
80a3bcd
Compare
| - ipa | ||
| - ipa_with_api |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the difference between the two? ipa is UI only, while ipa_with_api does UI and API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And to extend that, later we can add other methods here, like oidc etc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ipa is UI only, while ipa_with_api does UI and API?
exactly
later we can add other methods here, like oidc etc?
Yes, that was the intent here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and those are mutually exclusive, right? as in, there can be only one external auth?
(LDAP probably always works, but that's not terminated on the httpd level)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. I wanted to avoid the situation we had in the old installer where all the methods could be enabled individually, but some conflicted with others.
| pam_service: | ||
| help: Name of the PAM service to use for IPA authentication |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as there is no foreman user on the new host, one needs to pass something else here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no foreman user, but as part of this we deploy a configuration for a foreman pam service, which seems to be enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting!
|
While I am not a fan of adding another non-containerized service (sssd), I guess that's the only way to make it work right now. As such this looks like the right way to me |
| - name: Create PAM service file for IPA authentication | ||
| ansible.builtin.template: | ||
| src: pam_service.j2 | ||
| dest: "/etc/pam.d/{{ httpd_ipa_pam_service }}" | ||
| mode: "0644" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory this is not needed if httpd_ipa_pam_service != foreman
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But when is it not foreman?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then the service either needs to be defined manually by the user as a yet another pam service or it has to be the name of a user-created HBAC service in ipa
Edit: in either case, deploying our custom service config shouldn't be needed, but also doesn't really hurt anything
docs/parameters.md
Outdated
| | `--foreman-ipa-authentication` | Enable configuration for external authentication via IPA for web UI | `--external-authentication=ipa` | | ||
| | `--foreman-ipa-authentication-api` | Enable configuration for external authentication via IPA for web UI and API | `--external-authentication=ipa_with_api` | | ||
| | `--pam-service` | PAM service used for host-based access control in IPA | `--foreman-pam-service` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you have the columns the wrong way round, the last column is the "old version", the first is "this is how it's done now"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd also probably fold the two "authentication" ones into one line, given the difference is minimal (and just explain it in the description)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
Ansible/structure wise this is fine. I think it should get a short reference to https://docs.theforeman.org/3.16/Configuring_User_Authentication/index-katello.html#enrolling-foreman-server-in-freeipa-domain as in "this is how you need to prepare you machine for this to work". And then someone who knows Krb should verify this actually works fine (which I did not) |
bdf0d0f to
9da9011
Compare
ec99bdc to
bcddb3a
Compare
|
The correct workflow here is to do exactly what documentation says currently (Chapter 3. Configuring Kerberos SSO with Identity Management in Satellite) but in the |
|
Yes, although the options don't map one to one between the installer and foremanctl. See docs/parameters.md for details on how they translate. |
|
Thanks, @adamruzicka , I have successfully verified that this basically works with IDM in UI, using both user/pass and kinit. There's more to do but it looks good. |
|
|
|
Then it should be documented that this is irreversible and the reset operation leaves the machine in an undefined state. Although I understand it "works" and AFAIK is the same as in classical installer. |
|
Update, it also works with IDM using API. Just AD to do. |
|
When I read https://docs.theforeman.org/3.17/Configuring_User_Authentication/index-katello.html we do document the reset "softly". Not an explicit procedure. IMHO this is something we should change for foremanctl: either have an explicit disable procedure or not at all. In the short term I'm OK with no documented/supported procedure and we can re-evaluate this when there are questions about it. @aneta-petrova now that we have the new foremanctl docs structure, do you think we could start updating https://docs.theforeman.org/nightly/Configuring_User_Authentication/index-foremanctl-katello.html#configuring-kerberos-sso-with-FreeIPA-in-foreman and prepare the docs PR? It'll be a good way to verify we've covered all the user stories. Ideally I'd like to merge them in tandem. |
Sure. Can you create a ticket that we could refine and plan for a future sprint? |
|
Verified with UPstream + packit on RHEL9. According to documentation, set up {ipa, ipa with api, ad} and tested using {webui, hammer, curl}. In WebUI, I tested both using kinit + browser negotiate and inserting credentials directly to the form. In all cases, I've been able to login except for cases where I used API or Hammer and ipa_with_api wasn't configured - which is correct. Steps:
I've hit an unrelated issue when setting up AD and reported it as [0], it's not a blocker for this PR. |
Uhh, what? |
dccf29c to
77db6fc
Compare
d19e648 to
4979f6c
Compare
|
Now based on top of #362 |
|
QA ACK |
5e2d958 to
9b015c6
Compare
|
theforeman/foreman-documentation#4543 has received all the necessary acks and is ready to be merged after this PR is merged. |
Very rough draft at the moment, but the happy path seems to work.
Borrows ideas from theforeman/puppet-foreman#1172 and theforeman/puppet-foreman#1203
TODOs: