Todd Lipcon has posted comments on this change. Change subject: KUDU-1845: Kerberos client keytab should be periodically renewed ......................................................................
Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/5820/4/src/kudu/security/init.cc File src/kudu/security/init.cc: PS4, Line 125: strcmp(creds.server->data[1].data, principal_->realm.data) != 0 || : strcmp(creds.server->data[0].data, KRB5_TGS_NAME) != 0 || : strcmp(creds.server->realm.data, principal_->realm.data) != 0) { : the kerberos code uses a 'data_eq' helper here which also uses the length field of the data (and probably avoids the heap buffer overflow that ASAN is hitting) Line 134: time_t renew_deadline = renew_till - 30; we may want some jitter on this. As is, it seems like if you started up 100 servers, they'd all get their initial ticket at the same time. Then, they'd all hit this 30-second-before-expiration window at the same moment and cause a coordinated flood of load at the KDC at that moment. Some randomization like between 30 seconds and 5 minutes would probably be a good idea (potentailly also a random sleep before the first renewal attempt so that the checks themselves are offset) PS4, Line 144: // Acquire a new ticket using the keytab. This ticket will automatically be put into the : // credential cache. : this probably needs the same OSX workaround performed below in the initial kinit PS4, Line 157: // Clear existing credentials in cache. : KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_initialize(krb5_ctx_, ccache_, principal_), : "Failed to re-initialize ccache"); : // Store the new credentials in the cache. : KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_store_cred(krb5_ctx_, ccache_, &new_creds), : "Failed to store credentials in ccache"); : does this leave open a race where a connection might be attempted in between the two calls, see no TGT, and fail? is there a way to atomically replace the cred? (what happens if you don't use krb5_cc_initialize to reset it first?) -- To view, visit http://gerrit.cloudera.org:8080/5820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic4c072c1210216369e60eac88be4a20d9b166b2d Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
