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

[Kerberos] Add bootstrap checks for kerberos realm #31548

Merged
merged 4 commits into from
Jun 27, 2018

Conversation

bizybot
Copy link
Contributor

@bizybot bizybot commented Jun 25, 2018

As there are some system properties like java.security.krb5.kdc
, java.security.krb5.realm, java.security.krb5.conf which can
specify values that are applicable to whole JVM. This is the reason
for having only one instance of Kerberos realm.
Each ES node will have a Kerberos keytab with credentials. This
keytab must exist for Kerberos authentication to work.
KerberosRealmBootstrapCheck performs these checks for given
configuration.

As there are some system properties like `java.security.krb5.kdc`
, `java.security.krb5.realm` which can specify values that are
applicable to whole JVM. This is the reason for having only one
instance of KerberosRealm.
Each ES node will have a kerberos keytab with credentials. This
keytab must exist for kerberos authentication to work.
`KerberosRealmBootstrapCheck` performs these checks for given
configuration.
@bizybot bizybot requested review from tvernum and jaymode June 25, 2018 05:40
@bizybot bizybot added the :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) label Jun 25, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

set bootstrap failure when `type` not specified for realm

@Override
public BootstrapCheckResult check(final BootstrapContext context) {
final Settings realmsSettings = RealmSettings.get(context.settings);
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 recommend using RealmSettings.getRealmSettings instead.
It slightly reduces your reliance on the underlying realm config model.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, Tim. Addressed this.

@bizybot
Copy link
Contributor Author

bizybot commented Jun 26, 2018

Hi, @tvernum I have addressed your review comment. Please take another look when you get some time. Thank you.

Copy link
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

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

I left a request for improved javadocs on the class level. Otherwise LGTM

import java.util.Map.Entry;

/**
* This class is used to perform bootstrap checks for kerberos realm.
Copy link
Member

Choose a reason for hiding this comment

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

can you be more detailed here about the checks and why we have these checks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, addressed the javadocs.

@bizybot bizybot merged commit 8899920 into elastic:feature/kerberos Jun 27, 2018
@bizybot bizybot deleted the kerberos/bootstrapcheck branch June 27, 2018 00:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>feature :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants