You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2017/04/10 23:19:00 UTC

[kudu-CR] [security-faults-itest] added another krb5 error message

Alexey Serbin has uploaded a new change for review.

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

Change subject: [security-faults-itest] added another krb5 error message
......................................................................

[security-faults-itest] added another krb5 error message

Different versions of krb5 library have different error messages for
the ticket/context expiration error.  Recently, just another variation
has been found by Jenkins:

  'GSSAPI Error: The referenced context has expired (Success)'

Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
---
M src/kudu/integration-tests/security-faults-itest.cc
1 file changed, 5 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/6605/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [security-faults-itest] added another krb5 error message

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

Change subject: [security-faults-itest] added another krb5 error message
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6605/1//COMMIT_MSG
Commit Message:

Line 13:   'GSSAPI Error: The referenced context has expired (Success)'
> The 'GSSAPI Error' prefix comes from the cyrus-sasl library: plugins/gssapi
... could get 'minor error status' from the krb5 library using  gss_display_status() function and it was 'Success'.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] [security-faults-itest] added another krb5 error message

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

Change subject: [security-faults-itest] added another krb5 error message
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6605/1//COMMIT_MSG
Commit Message:

Line 13:   'GSSAPI Error: The referenced context has expired (Success)'
hrm, 'Success' here seems to indicate that maybe errno isn't getting set the way we think it is? Is 'Success' coming from our code or from the internals of GSSAPI or SASL? If the former, maybe we should be fixing this?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] [security-faults-itest] added another krb5 error message

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#2).

Change subject: [security-faults-itest] added another krb5 error message
......................................................................

[security-faults-itest] added another krb5 error message

Different versions of krb5 library have different error messages for
the ticket/context expiration error.  Recently, just another variation
has been found by Jenkins:

  'GSSAPI Error: The referenced context has expired'

The 'GSSAPI Error: ' prefix comes from the cyrus-sasl library.

Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
---
M src/kudu/integration-tests/security-faults-itest.cc
1 file changed, 5 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/6605/2
-- 
To view, visit http://gerrit.cloudera.org:8080/6605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [security-faults-itest] added another krb5 error message

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

Change subject: [security-faults-itest] added another krb5 error message
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6605/1//COMMIT_MSG
Commit Message:

Line 13:   'GSSAPI Error: The referenced context has expired (Success)'
> ... could get 'minor error status' from the krb5 library using  gss_display
yea, I think if we don't have any extra detail/message to add, we should just add nothing. Adding (Success) is very confusing to users in the context of an error message, and saying "we couldn't tell you anything more than this!" is not very helpful either.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] [security-faults-itest] added another krb5 error message

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

Change subject: [security-faults-itest] added another krb5 error message
......................................................................


Patch Set 2:

are you working on a separate patch to try to get rid of that '(Success)' thing?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [security-faults-itest] added another krb5 error message

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

Change subject: [security-faults-itest] added another krb5 error message
......................................................................


Patch Set 2:

oh, I misunderstood your earlier comment. I thought _we_ were the ones calling that function.

One more reason we should drop cyrus-sasl at some point and just use GSSAPI directly, I guess.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [security-faults-itest] added another krb5 error message

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

Change subject: [security-faults-itest] added another krb5 error message
......................................................................


Patch Set 2:

> > are you working on a separate patch to try to get rid of that
 > > '(Success)' thing?
 > 
 > I'm not: it seems that that 'Success' comes from the krb5 library
 > as the minor error status message extracted by the
 > gss_display_status() function.  I'm not sure whether we want to fix
 > that.

I just looked at sasl_gss_seterror_() function from cyrus-sasl library in plugins/gssapi.c.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [security-faults-itest] added another krb5 error message

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

Change subject: [security-faults-itest] added another krb5 error message
......................................................................


[security-faults-itest] added another krb5 error message

Different versions of krb5 library have different error messages for
the ticket/context expiration error.  Recently, just another variation
has been found by Jenkins:

  'GSSAPI Error: The referenced context has expired'

The 'GSSAPI Error: ' prefix comes from the cyrus-sasl library.

Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
Reviewed-on: http://gerrit.cloudera.org:8080/6605
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <da...@apache.org>
---
M src/kudu/integration-tests/security-faults-itest.cc
1 file changed, 5 insertions(+), 1 deletion(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [security-faults-itest] added another krb5 error message

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

Change subject: [security-faults-itest] added another krb5 error message
......................................................................


Patch Set 2:

> oh, I misunderstood your earlier comment. I thought _we_ were the
 > ones calling that function.
 > 
 > One more reason we should drop cyrus-sasl at some point and just
 > use GSSAPI directly, I guess.

Sorry for the confusion.

Yep, I think this is just another minor thing after that major performance-related bottleneck.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [security-faults-itest] added another krb5 error message

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

Change subject: [security-faults-itest] added another krb5 error message
......................................................................


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [security-faults-itest] added another krb5 error message

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

Change subject: [security-faults-itest] added another krb5 error message
......................................................................


Patch Set 2:

> are you working on a separate patch to try to get rid of that
 > '(Success)' thing?

I'm not: it seems that that 'Success' comes from the krb5 library as the minor error status message extracted by the gss_display_status() function.  I'm not sure whether we want to fix that.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [security-faults-itest] added another krb5 error message

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

Change subject: [security-faults-itest] added another krb5 error message
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6605/1//COMMIT_MSG
Commit Message:

Line 13:   'GSSAPI Error: The referenced context has expired (Success)'
> hrm, 'Success' here seems to indicate that maybe errno isn't getting set th
The 'GSSAPI Error' prefix comes from the cyrus-sasl library: plugins/gssapi.c (it seems I need to update the commit message to be clear).

The '(Success)' suffix corresponds to the fact the cyrus-sasl library could get 'minor error status' from the krb5 library.  The other alternative would be:

  "GSSAPI Failure: (could not get minor error message)"


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88462a186bd77ff6102a2a07ab3e885d704c3bb3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes