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

Check internal context in init_context mechglue call on error. #385

Closed
wants to merge 1 commit into from

Conversation

simo5
Copy link
Contributor

@simo5 simo5 commented Jan 5, 2016

Seen crashes in the wild without this check as some code seem to end up
calling gss_inquire_context via the spnego mech before while the internal
context is still NULL.

Signed-off-by: Simo Sorce simo@redhat.com

@greghudson
Copy link
Member

I need a better understanding of the bug here. I didn't think it was possible to create a mechglue context with a NULL internal_ctx_id.

@simo5
Copy link
Contributor Author

simo5 commented Jan 5, 2016

On Tue, 2016-01-05 at 09:36 -0800, Greg Hudson wrote:

I need a better understanding of the bug here. I didn't think it was possible to create a mechglue context with a NULL internal_ctx_id.

This is all I have for now:
https://bugzilla.redhat.com/show_bug.cgi?id=1295893

@greghudson
Copy link
Member

Simo and I believe we worked out what must have happened: on a second call to gss_init_sec_context(), SPNEGO gets an error and deletes its SPNEGO context, but the mechglue preserves the union context with a NULL internal_ctx_id pointer. I would like to see gss_init_sec_context() delete and null out the union context in this circumstance. gss_accept_sec_context() might need a similar change.

If the mechanism clears out the context on error, the mechglue must
do the same to avoid crashes if the application calls other functions
with this invalid union context.

Signed-off-by: Simo Sorce <simo@redhat.com>
@simo5 simo5 changed the title Check internal context in inquire mechglue call. Check internal context in init_context mechglue call on error. Jan 5, 2016
@simo5
Copy link
Contributor Author

simo5 commented Jan 5, 2016

Changed as requested, code now checks for internal_ctx_id on errors and clears the whole union context. Accept_sec_context already clears it all unconditioanlly on errors.

@greghudson
Copy link
Member

Pushed to master as 3beb564

@greghudson greghudson closed this Jan 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants