You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Sailesh Mukil (Code Review)" <ge...@cloudera.org> on 2017/08/30 17:58:23 UTC

[Impala-ASF-CR] [security] avoid kerberos ticket renewal and only reacquire

Hello Todd Lipcon, Kudu Jenkins,

I'd like you to do a code review.  Please visit

    http://gerrit.cloudera.org:8080/7898

to review the following change.

Change subject: [security] avoid kerberos ticket renewal and only reacquire
......................................................................

[security] avoid kerberos ticket renewal and only reacquire

It was found that if we use a file based credential cache that is
shared between the C++ side and the java side of a process, and we
encounter the specific edge case where we renew a ticket that has
less than 'ticket_lifetime' left before its 'renew_lifetime' expires,
the ticket is set to have a NULL 'renew_till' timestamp.

Eg:
ticket_lifetime = 10m
renew_lifetime = 100m

[current ticket being renewed] at '15:30:00'
endtime = '15:30:30'
renew_till = '15:31:00'

This ticket will be renewed and the renewed ticket will have the
following values:
endtime = '15:31:00'
renew_till = null

The Java krb5 library refuses to read these kinds of tickets which
have the RENEWABLE flag set but no 'renew_till' set, causing
unexpected failures.

Currently, the only way to work around this is to not renew tickets
at all and only always reacquire them. The reason for this is that
the Java side of a process or even another process may be running
its own renewal thread on the same credential cache for the same
principal(s). So even if we were to avoid renewing in this window,
the Java side could renew in this window, causing the above problem.
If we always reacquire the tickets, we're forcefully reseting this
window for that principal, thereby not allowing the Java side to hit
this bug.
The scenario where this bug played out is when using the kudu renewal
code in tandem with a hadoop process that use the same principals.

Also, currently there is no advantage we gain from just renewing the
tickets vs. reacquiring them, either in terms of security or
performance, since we login from a keytab.

Tracked on the Java side by:
http://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8186576

Change-Id: I8e5225de332ba785e3a73014b8418cfd4059fe07
Reviewed-on: http://gerrit.cloudera.org:8080/7810
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Kudu Jenkins
---
M be/src/kudu/security/init.cc
M be/src/kudu/security/init.h
2 files changed, 29 insertions(+), 57 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/7898/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7898
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8e5225de332ba785e3a73014b8418cfd4059fe07
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] [security] avoid kerberos ticket renewal and only reacquire

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change.

Change subject: [security] avoid kerberos ticket renewal and only reacquire
......................................................................


Patch Set 1:

Cherry-pick wasn't clean, but the resolution was trivial.

Kudu uses #ifdef __APPLE__, whereas we use #ifdef HAVE_KRB5_GET_INIT_CREDS_OPT_SET_OUT_CCACHE, which was the only cause of the conflict.

-- 
To view, visit http://gerrit.cloudera.org:8080/7898
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e5225de332ba785e3a73014b8418cfd4059fe07
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[Impala-ASF-CR] [security] avoid kerberos ticket renewal and only reacquire

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: [security] avoid kerberos ticket renewal and only reacquire
......................................................................


Patch Set 5: Verified+1

-- 
To view, visit http://gerrit.cloudera.org:8080/7898
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e5225de332ba785e3a73014b8418cfd4059fe07
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[Impala-ASF-CR] [security] avoid kerberos ticket renewal and only reacquire

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change.

Change subject: [security] avoid kerberos ticket renewal and only reacquire
......................................................................


Patch Set 4: Code-Review+2

(1 comment)

I didn't look too deeply into the change given it's a mostly clean backport. The conflict is expected and the resolution looks reasonable to me.

http://gerrit.cloudera.org:8080/#/c/7898/4/be/src/kudu/security/init.cc
File be/src/kudu/security/init.cc:

PS4, Line 187: reacqusition
nit: typo: reacquisition. Feel free to ignore and merge as-is. Fixing it may cause merge conflict in the future.


-- 
To view, visit http://gerrit.cloudera.org:8080/7898
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e5225de332ba785e3a73014b8418cfd4059fe07
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[Impala-ASF-CR] [security] avoid kerberos ticket renewal and only reacquire

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: [security] avoid kerberos ticket renewal and only reacquire
......................................................................


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1189/

-- 
To view, visit http://gerrit.cloudera.org:8080/7898
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e5225de332ba785e3a73014b8418cfd4059fe07
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[Impala-ASF-CR] [security] avoid kerberos ticket renewal and only reacquire

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.

Change subject: [security] avoid kerberos ticket renewal and only reacquire
......................................................................


[security] avoid kerberos ticket renewal and only reacquire

It was found that if we use a file based credential cache that is
shared between the C++ side and the java side of a process, and we
encounter the specific edge case where we renew a ticket that has
less than 'ticket_lifetime' left before its 'renew_lifetime' expires,
the ticket is set to have a NULL 'renew_till' timestamp.

Eg:
ticket_lifetime = 10m
renew_lifetime = 100m

[current ticket being renewed] at '15:30:00'
endtime = '15:30:30'
renew_till = '15:31:00'

This ticket will be renewed and the renewed ticket will have the
following values:
endtime = '15:31:00'
renew_till = null

The Java krb5 library refuses to read these kinds of tickets which
have the RENEWABLE flag set but no 'renew_till' set, causing
unexpected failures.

Currently, the only way to work around this is to not renew tickets
at all and only always reacquire them. The reason for this is that
the Java side of a process or even another process may be running
its own renewal thread on the same credential cache for the same
principal(s). So even if we were to avoid renewing in this window,
the Java side could renew in this window, causing the above problem.
If we always reacquire the tickets, we're forcefully reseting this
window for that principal, thereby not allowing the Java side to hit
this bug.
The scenario where this bug played out is when using the kudu renewal
code in tandem with a hadoop process that use the same principals.

Also, currently there is no advantage we gain from just renewing the
tickets vs. reacquiring them, either in terms of security or
performance, since we login from a keytab.

Tracked on the Java side by:
http://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8186576

Change-Id: I8e5225de332ba785e3a73014b8418cfd4059fe07
Reviewed-on: http://gerrit.cloudera.org:8080/7810
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Kudu Jenkins
Reviewed-on: http://gerrit.cloudera.org:8080/7898
Reviewed-by: Sailesh Mukil <sa...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/kudu/security/init.cc
M be/src/kudu/security/init.h
2 files changed, 29 insertions(+), 57 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Sailesh Mukil: Looks good to me, approved



-- 
To view, visit http://gerrit.cloudera.org:8080/7898
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I8e5225de332ba785e3a73014b8418cfd4059fe07
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] [security] avoid kerberos ticket renewal and only reacquire

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change.

Change subject: [security] avoid kerberos ticket renewal and only reacquire
......................................................................


Patch Set 5: Code-Review+2

(1 comment)

Rebase, carry +2.

http://gerrit.cloudera.org:8080/#/c/7898/4/be/src/kudu/security/init.cc
File be/src/kudu/security/init.cc:

PS4, Line 187: reacqusition
> nit: typo: reacquisition. Feel free to ignore and merge as-is. Fixing it ma
Ah yea, I'll fix that in Kudu and it'll come in later at some point when we rebase.


-- 
To view, visit http://gerrit.cloudera.org:8080/7898
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e5225de332ba785e3a73014b8418cfd4059fe07
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes