Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SPNEGO crash #74

Closed
wants to merge 3 commits into from
Closed

Conversation

iboukris
Copy link
Contributor

Hello,

This fix a segfault when trying to init_sec_context with SPNEGO mechanism.

Issue was found at (see there for an example to reproduce):
https://github.com/modauthgssapi/mod_auth_gssapi/pull/49

Thanks!

The actual_mech is not promised till we complet the context.

This fix a segfault when creating a SecurityContext with SPNEGO mech.
Seem trivial, silents compiler warning:
warning: gssapi/raw/sec_contexts.pyx:367:8: Unreachable code
@simo5
Copy link
Contributor

simo5 commented Aug 10, 2015

Thanks!

@@ -204,7 +204,7 @@ flags=None, lifetime=None, channel_bindings=None, input_token=None)
input_token_buffer.value = input_token
input_token_buffer.length = len(input_token)

cdef gss_OID actual_mech_type
cdef gss_OID actual_mech_type = GSS_C_NO_OID;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove the ; from the end of this line

@DirectXMan12
Copy link
Member

a couple of nits inline. Travis had a moment before, but it looks like you're just failing due to flake8 not liking the ;

@DirectXMan12 DirectXMan12 added this to the 1.1.3 milestone Aug 10, 2015
@iboukris
Copy link
Contributor Author

Hi Solly and Simo,

On Mon, Aug 10, 2015 at 11:00 PM, Solly notifications@github.com wrote:

a couple of nits inline. Travis had a moment before, but it looks like
you're just failing due to flake8 not liking the ;

Thanks, I'll fix and add a commit.

One more thing, what do you think - do we want to raise an error if the
context has completed and we got no actual mech or we should trust the
library?

Regards,
Isaac B.

@frozencemetery
Copy link
Member

Isaac Boukris notifications@github.com writes:

One more thing, what do you think - do we want to raise an error if the
context has completed and we got no actual mech or we should trust the
library?

I think we want to follow the behavior of the library, since it's either
a bug in the library (which in general we shouldn't workaround if
possible) or permitted by specification.

@iboukris
Copy link
Contributor Author

On Mon, Aug 10, 2015 at 11:36 PM, Robbie Harwood notifications@github.com
wrote:

Isaac Boukris notifications@github.com writes:

One more thing, what do you think - do we want to raise an error if the
context has completed and we got no actual mech or we should trust the
library?

I think we want to follow the behavior of the library, since it's either
a bug in the library (which in general we shouldn't workaround if
possible) or permitted by specification.

Make sense - thanks.

@frozencemetery
Copy link
Member

Pushed to master as 89197da and 102ea36

simo5 pushed a commit to gssapi/mod_auth_gssapi that referenced this pull request Sep 3, 2015
Add appropairate authorization headers to test with SPNEGO too as
discussed in #48

Requires recent version of python-gssapi module, see:
pythongssapi/python-gssapi#74

Simo: Squashed original patches in one, removed trailing whitespaces
and reworded the commit message.

Reviewed-by: Simo Sorce <simo@redhat.com>
Closes #49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants