-
Notifications
You must be signed in to change notification settings - Fork 342
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
Add a skeleton kdcpolicy plugin #2147
Conversation
(I don't think the CI failures are caused by this change, but please correct me if that's wrong.) |
CI is failing because named and a DNS helpe are having issues with GSSAPI:
|
2255363
to
6c43ae4
Compare
Fixed CI. Thanks for pointing me at the failure. |
You are using augeas to modify krb5 configuration. AFAIK we decided against augeas because augeas cannot handle all edge cases of krb5.conf. |
This is how the certauth plugin gets set up. I'm happy to move that as well, if you like. It makes sense to me to put this in a snippet in krb5.conf.d. Problem is, there's no hook that I can see right now that upgrades clients. Conceptually, these plugins are "server stuff" (though that lives in krb5.conf because not all plugins are server plugins). While I can work around this for the server plugins, it means that the /etc/krb5.conf.d/freeipa file doesn't get updated (which means SPAKE doesn't get enabled on IPA client upgrade). Suggestions? |
fb34a1b
to
04ddf96
Compare
Answering my own question. First commit makes it so that the snippet is re-written on update. Second commit is the kdcpolicy plugin, now using the snippet. Third commit moves the certauth plugin into the snippet by analogy. @tiran, ready for review (assuming it passes CI). |
04ddf96
to
1cf0bca
Compare
test_forced_client_enrolment appears to be a DNF failure? I guess you already know I have a lot of trouble figuring out what went wrong in the CI. I can't see any logs for test_advise or replica_promotion. |
client/share/freeipa.template
Outdated
certauth = { | ||
module = ipakdb:kdb/ipadb.so | ||
enable_only = ipakdb | ||
} |
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.
Isn't ipadb.so only shipped in servers 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.
Yes, ipadb.so
is only available in ipa-server
package. Please move the snippet to a server specific template.
@@ -1241,10 +1241,12 @@ if [ $1 -gt 1 ] ; then | |||
cp /etc/ipa/ca.crt /var/lib/ipa-client/pki/kdc-ca-bundle.pem | |||
cp /etc/ipa/ca.crt /var/lib/ipa-client/pki/ca-bundle.pem | |||
fi | |||
|
|||
%{python} -c 'from ipaclient.install.client import configure_krb5_snippet; configure_krb5_snippet()' >>/var/log/ipaupgrade.log 2>&1 | |||
fi |
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.
We might want/need an ipa-client-upgrade script to do this instead, if at least for other distros (perhaps outside the scope of this PR).
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.
That was my thought as well. We'd probably want to include the cert stuff above it if we ever make such a tool.
How/where would policy eventually be defined, or is that out-of-scope of this PR? |
1cf0bca
to
fe12c7a
Compare
That's sort of the question I was hoping having code in hand would answer. I assume configuration of this kind has to live in LDAP? I don't know where though. We also will need an interface to modify it. |
Right, it's sort of a chicken-and-egg problem I guess but there seems to be lack of context around this. It should probably be included as part of some initiative to actually include policy in the server. This PR would add a bunch of configuration, running code, etc to a new install that effectively is a no-op (hopefully). I just don't know where/how to land this as-is. |
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.
Please address the comment and rebase the PR.
client/share/freeipa.template
Outdated
certauth = { | ||
module = ipakdb:kdb/ipadb.so | ||
enable_only = ipakdb | ||
} |
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.
Yes, ipadb.so
is only available in ipa-server
package. Please move the snippet to a server specific template.
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
fe12c7a
to
25d6c8e
Compare
Updated and rebased. (Note that it's not technically an issue to have such configuration on the client, but you're right it's cleaner this way.) |
Installation is failing with
|
@@ -0,0 +1,9 @@ | |||
[plugins] |
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.
The build system doesn't pick up new files automatically. You have to add this file to Makefile.am
and maybe freeipa.spec.in
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
25d6c8e
to
cdb8521
Compare
Closing this to avoid confusion since it's been subsumed by #3358. |
Signed-off-by: Robbie Harwood rharwood@redhat.com
Back in krb5-1.16 (and in RHEL-7.5), I added the kdcpolicy plugin to krb5. This interface allows a module to hook all AS and TGS requests, potentially reject them, and manipulate ticket lifetimes. This PR is a basic implementation of the interface, with all the plumbing IPA needs to get it loaded and installed.
There are
twothree use cases I had in mind, though of course many more are possible (this is a very powerful place to have a hook into the KDC):Since presumably we don't want any of that to be hardcoded behavior, the difficult part is now making it all configurable. (As well as figuring out any behavior we want to control at the moment). Per IRC conversation, I'm opening this PR so that we have something to look at while we discuss that.
BACKPORT NOTE: there's a spec file change in this PR.