the code for accepting delegated credentials is incorrect

[simo5]

Both the low level and the high level API unconditionally try at any step of the context extablishment (on the acceptor side) to return delegated credentials and create the appropriate classes.

Delegated credentials are returned only if the context establishment return GSS_S_COMPLETE and are undefined in any other step.

In the high level API care need to be taken so that if the lower level returns None the delegated_creds should be set straight to None as well, as calling Credentials(None) will istead try to fetch the default credentials for the process, and that is an error.

[DirectXMan12]

Recording conversations for posterity:

RFC 2744 says:

Only valid if deleg_flag in ret_flags is true, in which case an explicit credential handle (i.e. not GSS_C_NO_CREDENTIAL) will be returned; if deleg_flag is false, gss_accept_context() will set this parameter to GSS_C_NO_CREDENTIAL.

RFC 2743 says (in Inquire_context):

GSS_S_COMPLETE indicates that the referenced context is valid and that deleg_state, mutual_state, replay_det_state, sequence_state, anon_state, trans_state, prot_ready_state, conf_avail, integ_avail, locally_initiated, and open return values describe the corresponding characteristics of the context.

And says for init_sec_context (also applying to accept_sec_context):

These state indicators' _[delegstate, etc] values are undefined unless either the routine's major_status indicates GSS_S_COMPLETE, or TRUE prot_ready_state is returned along with GSS_S_CONTINUE_NEEDED major_status; for the latter case, it is possible that additional features, not confirmed or indicated along with TRUE prot_ready_state, will be confirmed and indicated when GSS_S_COMPLETE is subsequently returned.

So it looks like we should check for COMPLETE or (prot_ready and CONTINUE), and then check DELEG_FLAG (the equivalent of deleg_state in 2744).

[DirectXMan12]

So the question is: does "undefined" mean "up to the implementation" or "in an invalid state" here? If it means "up to the implementation", it seems to me according to 2744, we can count on:

• if DELEG_FLAG is false, delegated_creds is NULL
• if DELEG_FLAG is true, delegated_creds is valid
• DELEG_FLAG will always be either true or false

meaning that it's fine to just check NULL. My concern is that if we add more complicated logic than "is it NULL or not" and miss a valid pointer, we'll have a memory leak, since

If a credential handle is returned, the associated resources must be released by the application after use with a call to gss_release_cred()

But if we wrap an invalid pointer, then we'll try to release() an invalid pointer.

(this phrasing seems to support the case that delegated_creds will always be either NULL or valid).

cc @simo5 @kaduk WDYT

[frozencemetery]

(My opinion that I mentioned in our conversation is that it means "up to the implementation" since if it were "invalid" it would be stated as such, but I don't have anything to back that up.)

[simo5]

If GSS_S_COMPLETE is returned then delegated_creds is either NULL (when deleg_state is False) or it is a valid credential (and deleg_state is True).

This is a requirement by 2744: "if deleg_flag is false, gss_accept_context() will set this parameter to GSS_C_NO_CREDENTIAL.

GSS_C_NO_CREDENTIAL == NULL

[DirectXMan12]

GSS_C_NO_CREDENTIAL == NULL

Yes, I know that. NULL is shorter to type.

2743 potentially seems to imply (under one interpretation of "undefined") that there's cases where the value of deleg_flag might not be "valid"

[frozencemetery]

After discussion I believe this has been resolved. If there's more to say we should reopen (and retarget).