Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upWIP: Channel Binding Extension #123
Conversation
e0737b4
to
005aaf8
| Args: | ||
| context (SecurityContext): Context to set flags on | ||
| req_flags (uint64_t): Request flags for init/accept_sec_context() |
DirectXMan12
Aug 10, 2017
Member
this should be a IntEnumFlagSet like the other flag types (see the main raw SecContext code)
this should be a IntEnumFlagSet like the other flag types (see the main raw SecContext code)
| Args: | ||
| context (SecurityContext): Context to set flags on | ||
| req_flags (uint64_t): Request flags for init/accept_sec_context() | ||
| ret_flags_understood: Return flags that are understood by the |
DirectXMan12
Aug 10, 2017
Member
is this an output parameter? It's not clear what this does. python-gssapi doc should be more-or-less understandable without prior knowledge (it should mostly be usable as GSSAPI documentation).
is this an output parameter? It's not clear what this does. python-gssapi doc should be more-or-less understandable without prior knowledge (it should mostly be usable as GSSAPI documentation).
cipherboy
Aug 11, 2017
•
Author
Contributor
ret_flags_understood is a set of return flags that are understood by the calling application.
In other word, it is a subset of possible ret_flag values from gss_init/accept_sec_context; the only useful ret_flag_understood parameter to set would be the channel bound flag; setting or not setting any other flag does not change the behavior. (This last point about not changing behavior is the subject of a revision in the draft, as if you set req_flags = GSS_C_MUTUAL_FLAG but set ret_flags_understood = 0, gss_init/accept_sec_context will still give a ret_flag with GSS_C_MUTUAL_FLAG on success).
ret_flags was what the original draft called it in the C bindings, hence the discrepancy in the high level API. Suggestions welcome about a more helpful name.
ret_flags_understood is a set of return flags that are understood by the calling application.
In other word, it is a subset of possible ret_flag values from gss_init/accept_sec_context; the only useful ret_flag_understood parameter to set would be the channel bound flag; setting or not setting any other flag does not change the behavior. (This last point about not changing behavior is the subject of a revision in the draft, as if you set req_flags = GSS_C_MUTUAL_FLAG but set ret_flags_understood = 0, gss_init/accept_sec_context will still give a ret_flag with GSS_C_MUTUAL_FLAG on success).
ret_flags was what the original draft called it in the C bindings, hence the discrepancy in the high level API. Suggestions welcome about a more helpful name.
| def __init__(self, base=None, token=None, | ||
| name=None, creds=None, lifetime=None, flags=None, | ||
| def __init__(self, base=None, token=None, name=None, creds=None, | ||
| lifetime=None, flags=None, ret_flags=None, |
DirectXMan12
Aug 10, 2017
Member
this parameter name is not the best...
this parameter name is not the best...
| @@ -104,6 +111,17 @@ def __init__(self, base=None, token=None, | |||
| "argument when creating an accepting " | |||
| "security context") | |||
|
|
|||
| if rchannel_bindings is not None: | |||
| empty_ctx = rchannel_bindings.create_sec_context() | |||
| self.copy_from(empty_ctx) | |||
DirectXMan12
Aug 10, 2017
Member
do this in __new__ so you don't have to call copy_from.
do this in __new__ so you don't have to call copy_from.
| application | ||
| Returns: | ||
| bool: Success |
DirectXMan12
Aug 10, 2017
Member
it's fine not to return anything. This doesn't actually return bool: success because it never returns False.
it's fine not to return anything. This doesn't actually return bool: success because it never returns False.
| bool: Success | ||
| Raises: | ||
| GSSError |
DirectXMan12
Aug 10, 2017
Member
Is there a specific error code for in-process sec contexts? It should be documented if there is.
Is there a specific error code for in-process sec contexts? It should be documented if there is.
cipherboy
Aug 11, 2017
•
Author
Contributor
The only error that is possible are GSS_S_FAILURE, when context.raw_ctx is none/missing for some reason or when context.raw_ctx is malformed. See here.
(This is because context is stored in a gss_union_ctx_id_t until gss_init/accept_sec_context, as no mech is known when gss_set_context_flags is called).
The only error that is possible are GSS_S_FAILURE, when context.raw_ctx is none/missing for some reason or when context.raw_ctx is malformed. See here.
(This is because context is stored in a gss_union_ctx_id_t until gss_init/accept_sec_context, as no mech is known when gss_set_context_flags is called).
This adds support for the channel binding extension from the Kitten WG. This adds two new calls to the raw api: create_sec_context and set_context_flags, allowing ret_flags_understood to be set for mechs that support it. This flag is exposed as the channel_bound requirement flag. Signed-off-by: Alexander Scheel <ascheel@redhat.com>
This is in support of the Kitten WG channel binding draft. A new field is added to the SecurityContext constructor, ret_flags, allowing the new raw methods to be automatically used from the high-level API when either req_flags or ret_flags are specified. Signed-off-by: Alexander Scheel <ascheel@redhat.com>
|
Okie dokie, this has been updated with comments. Thanks @DirectXMan12! :) Please see the comments on some of your changes as well. |
| raise GSSError(maj_stat, min_stat) | ||
|
|
||
|
|
||
| def set_context_flags(SecurityContext context, uint64_t req_flags, |
DirectXMan12
Sep 5, 2017
Member
don't type the flags here (since they can be IntEnumFlagSets), and they should either be not None if non-optional, or =None if optional (then use or like in https://github.com/pythongssapi/python-gssapi/blob/master/gssapi/raw/sec_contexts.pyx#L179)
don't type the flags here (since they can be IntEnumFlagSets), and they should either be not None if non-optional, or =None if optional (then use or like in https://github.com/pythongssapi/python-gssapi/blob/master/gssapi/raw/sec_contexts.pyx#L179)
| @@ -107,6 +107,10 @@ cdef class SecurityContext: | |||
|
|
|||
| self.raw_ctx = NULL | |||
|
|
|||
| def copy_from(self, SecurityContext other): | |||
DirectXMan12
Sep 5, 2017
Member
this isn't used
this isn't used
| def __init__(self, base=None, token=None, | ||
| name=None, creds=None, lifetime=None, flags=None, | ||
| def __init__(self, base=None, token=None, name=None, creds=None, | ||
| lifetime=None, flags=None, ret_flags_understood=None, |
DirectXMan12
Sep 5, 2017
Member
we probably should avoid changing argument order (since this isn't python 3, we can't do keyword-only args).
we probably should avoid changing argument order (since this isn't python 3, we can't do keyword-only args).
| @@ -104,6 +113,14 @@ def __init__(self, base=None, token=None, | |||
| "argument when creating an accepting " | |||
| "security context") | |||
|
|
|||
| if rchannel_bindings is not None: | |||
| i_flags = int(self._desired_flags) | |||
| i_ret_flags = int(self._understood_flags) | |||
DirectXMan12
Sep 5, 2017
Member
shouldn't need a manual cast here -- the low-level code should handle it.
shouldn't need a manual cast here -- the low-level code should handle it.
| if not init_cont_needed: | ||
| break | ||
|
|
||
| if ((int(init_ret_flags) & m_init_ret_flags) != e_init_ret_flags): |
DirectXMan12
Sep 5, 2017
Member
shouldn't need a cast to int here -- IntEnumFlagSet handles that case -- that's why we have it ;-).
shouldn't need a cast to int here -- IntEnumFlagSet handles that case -- that's why we have it ;-).
| [0, f_both, f_cb, f_cb, cb, cb, f_cb, 0, f_cb, f_cb], | ||
| ] | ||
|
|
||
| for test in tests: |
DirectXMan12
Sep 5, 2017
Member
use parameterized?
use parameterized?
This PR implements channel bindings per this draft RFC, and depends on this PR: krb5/krb5#668. In the mean time, patches can be found in this branch.
New raw calls:
create_sec_contextset_context_flagsHigh level changes:
SecurityContextconstructor takes aret_flagsparametercreate_sec_contextcalledset_context_flagscalled