Skip Menu |
 

In gss_import_sec_context there is a mismatch between the context
created and the mechanism assigned to the union context when an
interposer plugin is involved.

The context mech_type should always reflect the actual context type
stored in internal_ctx_id, however currently the wrong mech_type is saved.
When an interposer context is returned the mechanism type is set to that
of the real mechanism, and conversely when a real context is returned
the interposer mechanism type is stored.

The net effect is that the wrong mechanism context is cast and is passed
to subsequent functions.
Preliminary patch to fix the issue.
From 0ca98948097b51166017758ee39781d8e44ba520 Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Sat, 16 Mar 2013 19:23:03 +0000
Subject: Fix import_sec_context with interposers

The code was correctly selecting the mechanism to execute, however it was
improperly setting the mechanism type of the intenal context when a the
selected mechanism was that of an interposer and vice versa.

When an interposer is involved the internal context is that of the interposer
so the mechanism type of the context needs to be the interposer oid.
Conversely when an interposer re-enters gssapi and presents a token with a
special oid, the mechanism called is the real mechanism and the context
returned is a real mechanism context. In this case the mechanism type of
the context needs to be that of the real mechanism.
---
diff --git a/src/lib/gssapi/mechglue/g_imp_sec_context.c b/src/lib/gssapi/mechglue/g_imp_sec_context.c
index 53310dd..31cb30b 100644
--- a/src/lib/gssapi/mechglue/g_imp_sec_context.c
+++ b/src/lib/gssapi/mechglue/g_imp_sec_context.c
@@ -84,6 +84,7 @@ gss_ctx_id_t * context_handle;
gss_union_ctx_id_t ctx;
gss_ctx_id_t mctx;
gss_buffer_desc token;
+ gss_OID token_mech = GSS_C_NO_OID;
gss_OID selected_mech = GSS_C_NO_OID;
gss_OID public_mech;
gss_mechanism mech;
@@ -96,15 +97,13 @@ gss_ctx_id_t * context_handle;
/* Initial value needed below. */
status = GSS_S_FAILURE;

- ctx = (gss_union_ctx_id_t) malloc(sizeof(gss_union_ctx_id_desc));
+ ctx = (gss_union_ctx_id_t) calloc(sizeof(gss_union_ctx_id_desc), 1);
if (!ctx)
return (GSS_S_FAILURE);

- ctx->mech_type = (gss_OID) malloc(sizeof(gss_OID_desc));
- if (!ctx->mech_type) {
- free(ctx);
- return (GSS_S_FAILURE);
- }
+ token_mech = (gss_OID) calloc(sizeof(gss_OID_desc), 1);
+ if (!token_mech)
+ goto cleanup;

if (interprocess_token->length >= sizeof (OM_uint32)) {
p = interprocess_token->value;
@@ -116,16 +115,16 @@ gss_ctx_id_t * context_handle;

if (length == 0 ||
length > (interprocess_token->length - sizeof (OM_uint32))) {
- free(ctx);
- return (GSS_S_CALL_BAD_STRUCTURE | GSS_S_DEFECTIVE_TOKEN);
+ status = GSS_S_CALL_BAD_STRUCTURE | GSS_S_DEFECTIVE_TOKEN;
+ goto cleanup;
}

- ctx->mech_type->length = length;
- ctx->mech_type->elements = malloc(length);
- if (!ctx->mech_type->elements) {
- goto error_out;
- }
- memcpy(ctx->mech_type->elements, p, length);
+ token_mech->length = length;
+ token_mech->elements = malloc(length);
+ if (!token_mech->elements)
+ goto cleanup;
+
+ memcpy(token_mech->elements, p, length);
p += length;

token.length = interprocess_token->length - sizeof (OM_uint32) - length;
@@ -136,20 +135,26 @@ gss_ctx_id_t * context_handle;
* call it.
*/

- status = gssint_select_mech_type(minor_status, ctx->mech_type,
+ status = gssint_select_mech_type(minor_status, token_mech,
&selected_mech);
if (status != GSS_S_COMPLETE)
- goto error_out;
+ goto cleanup;

mech = gssint_get_mechanism(selected_mech);
if (!mech) {
status = GSS_S_BAD_MECH;
- goto error_out;
+ goto cleanup;
}
if (!mech->gssspi_import_sec_context_by_mech &&
!mech->gss_import_sec_context) {
status = GSS_S_UNAVAILABLE;
- goto error_out;
+ goto cleanup;
+ }
+
+ if (generic_gss_copy_oid(minor_status, selected_mech,
+ &ctx->mech_type) != GSS_S_COMPLETE) {
+ status = GSS_S_FAILURE;
+ goto cleanup;
}

if (mech->gssspi_import_sec_context_by_mech) {
@@ -164,18 +169,21 @@ gss_ctx_id_t * context_handle;
ctx->internal_ctx_id = mctx;
ctx->loopback = ctx;
*context_handle = (gss_ctx_id_t)ctx;
- return (GSS_S_COMPLETE);
+ status = GSS_S_COMPLETE;
+ } else
+ map_error(minor_status, mech);
+
+cleanup:
+ if (token_mech != GSS_C_NO_OID) {
+ free(token_mech->elements);
+ free(token_mech);
}
- map_error(minor_status, mech);
-
-error_out:
- if (ctx) {
- if (ctx->mech_type) {
- if (ctx->mech_type->elements)
- free(ctx->mech_type->elements);
- free(ctx->mech_type);
- }
- free(ctx);
+ if (status != GSS_S_COMPLETE) {
+ if (ctx->mech_type) {
+ free(ctx->mech_type->elements);
+ free(ctx->mech_type);
+ }
+ free(ctx);
}
return status;
}
--
cgit v0.9.1
Revised patch attached.

No real behaviour change, just style changes as requested by Greg on IRC.

The code is tested and confirmed working with gssproxy tests.
From 2ce34d51d2f69af3663d8a4e48ecf959ee592f14 Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Sat, 16 Mar 2013 19:23:03 +0000
Subject: Fix import_sec_context with interposers

The code was correctly selecting the mechanism to execute, however it was
improperly setting the mechanism type of the intenal context when a the
selected mechanism was that of an interposer and vice versa.

When an interposer is involved the internal context is that of the interposer
so the mechanism type of the context needs to be the interposer oid.
Conversely when an interposer re-enters gssapi and presents a token with a
special oid, the mechanism called is the real mechanism and the context
returned is a real mechanism context. In this case the mechanism type of
the context needs to be that of the real mechanism.
---
diff --git a/src/lib/gssapi/mechglue/g_imp_sec_context.c b/src/lib/gssapi/mechglue/g_imp_sec_context.c
index 53310dd..a0e2d71 100644
--- a/src/lib/gssapi/mechglue/g_imp_sec_context.c
+++ b/src/lib/gssapi/mechglue/g_imp_sec_context.c
@@ -84,6 +84,7 @@ gss_ctx_id_t * context_handle;
gss_union_ctx_id_t ctx;
gss_ctx_id_t mctx;
gss_buffer_desc token;
+ gss_OID_desc token_mech;
gss_OID selected_mech = GSS_C_NO_OID;
gss_OID public_mech;
gss_mechanism mech;
@@ -100,12 +101,6 @@ gss_ctx_id_t * context_handle;
if (!ctx)
return (GSS_S_FAILURE);

- ctx->mech_type = (gss_OID) malloc(sizeof(gss_OID_desc));
- if (!ctx->mech_type) {
- free(ctx);
- return (GSS_S_FAILURE);
- }
-
if (interprocess_token->length >= sizeof (OM_uint32)) {
p = interprocess_token->value;
length = (OM_uint32)*p++;
@@ -120,12 +115,9 @@ gss_ctx_id_t * context_handle;
return (GSS_S_CALL_BAD_STRUCTURE | GSS_S_DEFECTIVE_TOKEN);
}

- ctx->mech_type->length = length;
- ctx->mech_type->elements = malloc(length);
- if (!ctx->mech_type->elements) {
- goto error_out;
- }
- memcpy(ctx->mech_type->elements, p, length);
+ token_mech.length = length;
+ token_mech.elements = p;
+
p += length;

token.length = interprocess_token->length - sizeof (OM_uint32) - length;
@@ -136,7 +128,7 @@ gss_ctx_id_t * context_handle;
* call it.
*/

- status = gssint_select_mech_type(minor_status, ctx->mech_type,
+ status = gssint_select_mech_type(minor_status, &token_mech,
&selected_mech);
if (status != GSS_S_COMPLETE)
goto error_out;
@@ -152,6 +144,12 @@ gss_ctx_id_t * context_handle;
goto error_out;
}

+ if (generic_gss_copy_oid(minor_status, selected_mech,
+ &ctx->mech_type) != GSS_S_COMPLETE) {
+ status = GSS_S_FAILURE;
+ goto error_out;
+ }
+
if (mech->gssspi_import_sec_context_by_mech) {
public_mech = gssint_get_public_oid(selected_mech);
status = mech->gssspi_import_sec_context_by_mech(minor_status,
@@ -167,16 +165,11 @@ gss_ctx_id_t * context_handle;
return (GSS_S_COMPLETE);
}
map_error(minor_status, mech);
+ free(ctx->mech_type->elements);
+ free(ctx->mech_type);

error_out:
- if (ctx) {
- if (ctx->mech_type) {
- if (ctx->mech_type->elements)
- free(ctx->mech_type->elements);
- free(ctx->mech_type);
- }
- free(ctx);
- }
+ free(ctx);
return status;
}
#endif /* LEAN_CLIENT */
--
cgit v0.9.1
From: ghudson@mit.edu
Subject: git commit

Fix import_sec_context with interposers

The code was correctly selecting the mechanism to execute, but it was
improperly setting the mechanism type of the internal context when the
selected mechanism was that of an interposer and vice versa.

When an interposer is involved the internal context is that of the
interposer, so the mechanism type of the context needs to be the
interposer oid. Conversely, when an interposer re-enters gssapi and
presents a token with a special oid, the mechanism called is the real
mechanism, and the context returned is a real mechanism context. In
this case the mechanism type of the context needs to be that of the
real mechanism.

https://github.com/krb5/krb5/commit/36c76aa3c625afc9291b9e1df071db51ccf37dab
Author: Simo Sorce <simo@redhat.com>
Committer: Greg Hudson <ghudson@mit.edu>
Commit: 36c76aa3c625afc9291b9e1df071db51ccf37dab
Branch: master
src/lib/gssapi/mechglue/g_imp_sec_context.c | 35 ++++++++++----------------
1 files changed, 14 insertions(+), 21 deletions(-)
From: tlyu@mit.edu
Subject: git commit

Fix import_sec_context with interposers

The code was correctly selecting the mechanism to execute, but it was
improperly setting the mechanism type of the internal context when the
selected mechanism was that of an interposer and vice versa.

When an interposer is involved the internal context is that of the
interposer, so the mechanism type of the context needs to be the
interposer oid. Conversely, when an interposer re-enters gssapi and
presents a token with a special oid, the mechanism called is the real
mechanism, and the context returned is a real mechanism context. In
this case the mechanism type of the context needs to be that of the
real mechanism.

(cherry picked from commit 36c76aa3c625afc9291b9e1df071db51ccf37dab)

https://github.com/krb5/krb5/commit/db203a153fbe2b776210e966bf198c40f796d535
Author: Simo Sorce <simo@redhat.com>
Committer: Tom Yu <tlyu@mit.edu>
Commit: db203a153fbe2b776210e966bf198c40f796d535
Branch: krb5-1.11
src/lib/gssapi/mechglue/g_imp_sec_context.c | 35 ++++++++++----------------
1 files changed, 14 insertions(+), 21 deletions(-)