Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

feat(security): init kerberos #585

Merged
merged 37 commits into from
Aug 21, 2020
Merged

Conversation

levy5307
Copy link
Contributor

@levy5307 levy5307 commented Aug 6, 2020

If the config of enable_auth is true, we should init kerberos.
Here is the detail procedure:

  1. set envs which are used in kerberos, like KRB5CCNAME, KRB5_CONFIG and so on.
  2. create a kerberos context.
  3. convert a string principal name to a krb5_principal structure. principal and username that logged in as, this determines "who I am"
  4. get a handle for a key table. key table is the file to store principals and it's correponding key.
  5. acquire credential cache handle and initialize credential cache. The credential cache is used for storing credentials.
  6. allocate a new initial credential options structure, the options are used for getting credentials.
  7. get and schedule to renew credentials from KDC and store it into _ccache

New configuration added

[security]
+ krb5_keytab =
+ krb5_config =
+ krb5_principal =

include/dsn/utility/time_utils.h Show resolved Hide resolved
@@ -30,5 +30,5 @@ add_library(dsn_runtime STATIC
tracer.cpp
zlocks.cpp
)
target_link_libraries(dsn_runtime dsn_utils)
target_link_libraries(dsn_runtime dsn_utils sasl2 gssapi_krb5 krb5)
Copy link
Member

Choose a reason for hiding this comment

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

So don't forget to update docs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which docs do you mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

src/runtime/security/kerberos_utils.cpp Outdated Show resolved Hide resolved
src/runtime/security/kerberos_utils.h Outdated Show resolved Hide resolved
src/runtime/security/kerberos_utils.cpp Outdated Show resolved Hide resolved
src/runtime/security/kerberos_utils.cpp Outdated Show resolved Hide resolved
src/runtime/security/kerberos_utils.cpp Outdated Show resolved Hide resolved
_timer->expires_from_now(boost::posix_time::seconds(renew_gap));
_timer->async_wait([this](const boost::system::error_code &err) {
if (!err.failed()) {
get_credentials();
Copy link
Member

Choose a reason for hiding this comment

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

Will you try to find credential in krb5_ccache in other PRs?

Copy link
Member

@vagetablechicken vagetablechicken Aug 7, 2020

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@levy5307 levy5307 Aug 14, 2020

Choose a reason for hiding this comment

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

I have read some docs of kerberos. It says
The credential cache file holds Kerberos protocol credentials (for example, tickets, session keys, and other identifying information) in semipermanent storage. The Kerberos protocol reads credentials from the cache as they are required and stores new credentials in the cache as they are obtained. This relieves the application of the responsibility for managing the credentials itself. in https://www.ibm.com/support/knowledgecenter/en/SSGSMK_7.1.0/management_sym/sym_kerberos_configuring_credential_cache_file.html

And I have read the corresponding code in kudu, there are no code trying to find credentials in krb5_ccache. It only put credentials into krb5_ccache.

Copy link
Member

Choose a reason for hiding this comment

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

so you mean krb5_cc_ funcs are useless? You should read more. It's not for internal. I didn't say we need to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Kerberos protocol reads credentials from the cache as they are required and stores new credentials in the cache as they are obtained. I didn't say they are useless

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So what the credential cache is used for in your opinion? @vagetablechicken

Copy link
Member

Choose a reason for hiding this comment

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

What I said is the basic conceptive problem. krb5_ccache is not for krb internal. The owner can use it to avoid unnecessary connections.
There's no need to optimize prematurely.

@acelyc111
Copy link
Member

@levy5307 levy5307 marked this pull request as draft August 7, 2020 02:57
@levy5307 levy5307 marked this pull request as ready for review August 14, 2020 02:12
include/dsn/utility/error_code.h Outdated Show resolved Hide resolved
src/runtime/service_api_c.cpp Show resolved Hide resolved
src/runtime/security/kinit_context.cpp Show resolved Hide resolved
src/runtime/security/kinit_context.cpp Outdated Show resolved Hide resolved
src/runtime/security/kinit_context.cpp Show resolved Hide resolved
src/runtime/security/kinit_context.cpp Show resolved Hide resolved
src/runtime/security/kinit_context.cpp Outdated Show resolved Hide resolved
src/runtime/security/kinit_context.cpp Outdated Show resolved Hide resolved
src/runtime/CMakeLists.txt Show resolved Hide resolved
src/runtime/security/init.cpp Outdated Show resolved Hide resolved
src/runtime/security/init.cpp Outdated Show resolved Hide resolved
src/runtime/security/kinit_context.cpp Outdated Show resolved Hide resolved
src/runtime/security/kinit_context.cpp Show resolved Hide resolved
acelyc111
acelyc111 previously approved these changes Aug 19, 2020
acelyc111
acelyc111 previously approved these changes Aug 20, 2020
namespace dsn {
namespace security {
// init security(kerberos and sasl)
bool init(bool is_server);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bool init(bool is_server);
extern bool init_server();
extern bool init_client();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there is no need to split the function into two functions. Because they are almost same

hycdong
hycdong previously approved these changes Aug 21, 2020
@levy5307 levy5307 dismissed stale reviews from hycdong and acelyc111 via 5b96be6 August 21, 2020 06:47
@levy5307 levy5307 merged commit 38095a4 into XiaoMi:master Aug 21, 2020
@levy5307 levy5307 deleted the kerberos-init branch August 21, 2020 07:29
@levy5307 levy5307 added the type/config-change PR that made modification on configs, which should be noted in release note. label Oct 30, 2020
levy5307 added a commit to levy5307/rdsn that referenced this pull request Dec 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component/security type/config-change PR that made modification on configs, which should be noted in release note.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants