Added support for GGF inquire cred and sec context #141

jborean93 · 2018-01-11T03:18:40Z

Implements gss_inquire_cred_by_oid and gss_inquire_sec_context_by_oid. This partially completes #51.

I was trying to also implement the other 2 methods and wasn't able to get a working implementation so have decided to ignore it for now.

jborean93 · 2018-01-11T03:19:25Z

This is also a superset of #128 but also includes some documentation and tests as well as the inquire_cred_by_oid method

frozencemetery

Thanks, the code looks good to me (will wait for review from @DirectXMan12). Also appreciate you adding tests.

In the commit message, it would be good if you fleshed out what the GGF extensions are a bit more - maybe with a link to the IETF draft as well. Also, it would be nice to give the @vm86 an Also-authored-by (or otherwise mention their contributions) in the commit message.

jborean93 · 2018-01-11T22:04:39Z

Thanks @frozencemetery will do

jborean93 · 2018-01-11T22:24:38Z

@frozencemetery let me know if the latest commit message is what you are looking for.

frozencemetery

It is, thanks!

vm86 · 2018-01-13T10:44:36Z

Thanks! I unfortunately did not have time.

DirectXMan12

minor nits inline, mostly looks good

DirectXMan12 · 2018-01-16T18:24:54Z

gssapi/raw/ext_ggf.pyx

+        mech (OID): the desired mechanism
+
+    Returns:
+        list: A list of zero or more pieces of data corresponding to the


nit: a list of zero or more bytes objects? Can you be more specific about what a piece of data is?

I'm honestly not really sure, I've only used this to retrieve GSS_C_INQ_SSPI_SESSION_KEY and the end result is a list with a single entry of a byte string. I'm happy to say the "piece of data" is a byte string but not sure what else it could be.

@DirectXMan12 no you can't be more precise, because the data is a bunch of bytes, and how you interpret them depends on the desired oid used.

I mean, if the actual return type is just multiple chunks of bytes, then it's fine to say

a list of zero or more pieces of data (as bytes objects)

or something of the sort

DirectXMan12 · 2018-01-16T18:26:49Z

gssapi/raw/ext_ggf.pyx

+
+    Args:
+        cred_handle (Creds): the security context to query
+        mech (OID): the desired mechanism


nit: is this really a mechanism OID, or is it just some other type of OID?

Not sure what the terms actually mean but this is probably more desired_object (OID): the OID of the desired object or something like that?

yeah, that's fine (alternatively, something like desired_aspect (OID): the desired aspect of the Credentials to inquire about (also, looks like you have have a copy-paste error above with creds vs contexts)

DirectXMan12 · 2018-01-16T18:34:15Z

Also, could you put that link and info from the commit message into a module-level doc string?
All extensions should have some module-level docs, even if just a title (like https://github.com/jborean93/python-gssapi/blob/a50b089aa9e7caa70254d64171bad22b59a7d563/gssapi/raw/ext_s4u.pyx#L1)

jborean93 · 2018-01-16T20:18:16Z

@DirectXMan12 so just add the commit message to a docstring at the top of the pyx file?

DirectXMan12 · 2018-01-16T20:22:54Z

Something like that should work, yes. When people look at the GGF extension module and they're wondering what it's all about, they'll see that documentation, so it should serve that purpose, more or less.

jborean93 · 2018-01-16T20:23:58Z

No worries, will update the PR now. Thanks for the review.

DirectXMan12

generally looks good. a couple small nits inline, then please to a git rebase -i and mark the second commit as a fixup (so you're left with one commit). We tend towards a policy of each commit being a clean, distinct piece of history/functionality.

Thanks!

DirectXMan12 · 2018-01-16T21:09:20Z

gssapi/raw/ext_ggf.pyx

+
+GGF provides extended credential and security context inquiry that allows
+application to retrieve more information about the client's credentials and
+security context. One common use case is to use gss_inquire_sec_context_by_oid


should just be inquire_sec_context_by_oid, and you should be able to make it a link to the actual function documentation below.

Do I link it by enclosing it with `? Sorry just can't find an example.

Edit: I think I found one, I will do ":meth:`inquire_sec_context_by_oid'"

DirectXMan12 · 2018-01-16T21:10:05Z

gssapi/raw/ext_ggf.pyx

+security context. One common use case is to use gss_inquire_sec_context_by_oid
+to retrieve the "session" key that is required by the SMB protocol for signing
+and encrypting a message. These calls are provided as a part of the raw
+interface and are not exposed in the high-level interface.


no need for the sentence starting with "These calls". It just means if we ever expose it at the high level, we'll have to remember to remove that sentence.

jborean93 · 2018-01-16T22:27:50Z

Thanks @DirectXMan12 hopefully the latest changes are what you are looking for.

jborean93 · 2018-01-17T19:52:43Z

sigh Travis-CI looks like it is still having issues, trying to run the tests again.

@vm86

GGF provides extended credential and security context inquiry that allows application to retrieve more information about the client's credentials and security context. One common use case is to use gss_inquire_sec_context_by_oid to retrieve the "session" key that is required by the SMB protocol for signing and encrypting a message. These calls are provided as a part of the raw interface and are not exposed in the high-level interface. Thanks to @vm86 for his work on the gss_inquire_sec_context_by_oid. Draft IETF document for these extensions can be found at https://tools.ietf.org/html/draft-engert-ggf-gss-extensions-00

DirectXMan12

LGTM 👍

jborean93 · 2018-01-17T22:42:17Z

Thanks @DirectXMan12 @frozencemetery for the reviews and help.

jborean93 force-pushed the ext_ggf branch from 05beb4a to 4b68958 Compare January 11, 2018 04:14

jborean93 closed this Jan 11, 2018

jborean93 reopened this Jan 11, 2018

frozencemetery requested a review from DirectXMan12 January 11, 2018 21:30

frozencemetery suggested changes Jan 11, 2018

View reviewed changes

jborean93 force-pushed the ext_ggf branch from 4b68958 to a50b089 Compare January 11, 2018 22:24

frozencemetery approved these changes Jan 12, 2018

View reviewed changes

vm86 mentioned this pull request Jan 13, 2018

Implement support for GSSAPI extension gss_inquire_sec_context_by_oid #128

Closed

DirectXMan12 suggested changes Jan 16, 2018

View reviewed changes

jborean93 force-pushed the ext_ggf branch from 4f4c569 to e24b0b3 Compare January 16, 2018 21:22

jborean93 mentioned this pull request Jan 16, 2018

Implement Global Grid Forum Extensions #51

Closed

jborean93 closed this Jan 17, 2018

jborean93 reopened this Jan 17, 2018

jborean93 closed this Jan 17, 2018

jborean93 reopened this Jan 17, 2018

jborean93 force-pushed the ext_ggf branch from e24b0b3 to 36c0da8 Compare January 17, 2018 20:00

DirectXMan12 approved these changes Jan 17, 2018

View reviewed changes

DirectXMan12 merged commit a14d750 into pythongssapi:master Jan 17, 2018

jborean93 deleted the ext_ggf branch January 17, 2018 22:41

frozencemetery added this to the 1.4.0 milestone Feb 16, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for GGF inquire cred and sec context #141

Added support for GGF inquire cred and sec context #141

jborean93 commented Jan 11, 2018

jborean93 commented Jan 11, 2018

frozencemetery left a comment

jborean93 commented Jan 11, 2018

jborean93 commented Jan 11, 2018

frozencemetery left a comment

vm86 commented Jan 13, 2018

DirectXMan12 left a comment •

edited

Loading

DirectXMan12 Jan 16, 2018

jborean93 Jan 16, 2018 •

edited

Loading

simo5 Jan 16, 2018

DirectXMan12 Jan 16, 2018

DirectXMan12 Jan 16, 2018

jborean93 Jan 16, 2018

DirectXMan12 Jan 16, 2018

DirectXMan12 commented Jan 16, 2018 •

edited

Loading

jborean93 commented Jan 16, 2018

DirectXMan12 commented Jan 16, 2018

jborean93 commented Jan 16, 2018

DirectXMan12 left a comment •

edited

Loading

DirectXMan12 Jan 16, 2018

jborean93 Jan 16, 2018 •

edited

Loading

DirectXMan12 Jan 16, 2018

jborean93 commented Jan 16, 2018

jborean93 commented Jan 17, 2018

DirectXMan12 left a comment

jborean93 commented Jan 17, 2018

Added support for GGF inquire cred and sec context #141

Added support for GGF inquire cred and sec context #141

Conversation

jborean93 commented Jan 11, 2018

jborean93 commented Jan 11, 2018

frozencemetery left a comment

Choose a reason for hiding this comment

jborean93 commented Jan 11, 2018

jborean93 commented Jan 11, 2018

frozencemetery left a comment

Choose a reason for hiding this comment

vm86 commented Jan 13, 2018

DirectXMan12 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jborean93 Jan 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DirectXMan12 commented Jan 16, 2018 • edited Loading

jborean93 commented Jan 16, 2018

DirectXMan12 commented Jan 16, 2018

jborean93 commented Jan 16, 2018

DirectXMan12 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jborean93 Jan 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jborean93 commented Jan 16, 2018

jborean93 commented Jan 17, 2018

DirectXMan12 left a comment

Choose a reason for hiding this comment

jborean93 commented Jan 17, 2018

DirectXMan12 left a comment •

edited

Loading

jborean93 Jan 16, 2018 •

edited

Loading

DirectXMan12 commented Jan 16, 2018 •

edited

Loading

DirectXMan12 left a comment •

edited

Loading

jborean93 Jan 16, 2018 •

edited

Loading