LU-17535 gss: fix lsvcgssd crash in krb lib

This patch fixes some logic around the need to call
gss_delete_sec_context() or not vs kerberos implementations.

snd->ctx address instead of value should be passed to
serialize_context_for_kernel()/serialize_krb5_ctx() to
allow each implementation to clear it with GSS_C_NO_CONTEXT
if it has been destroyed internally, and cases where not
can also be handled in handle_krb() now.

Test-Parameters: trivial
Test-Parameters: kerberos=true testlist=sanity-krb5
Test-Parameters: testgroup=review-dne-selinux-ssk-part-2
Signed-off-by: Bruno Faccini <bfaccini@nvidia.com>
Change-Id: I752712168a2c0f0a5a7a496b851d4cddbb7e4236
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/54023
Reviewed-by: Sebastien Buisson <sbuisson@ddn.com>
Reviewed-by: Aurelien Degremont <adegremont@nvidia.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>

diff --git a/lustre/utils/gss/context.c b/lustre/utils/gss/context.c
index f45caca..e9a067c 100644 (file)
--- a/lustre/utils/gss/context.c
+++ b/lustre/utils/gss/context.c
@@ -44,7 +44,7 @@
 #include "context.h"
 
 int
-serialize_context_for_kernel(gss_ctx_id_t ctx,
+serialize_context_for_kernel(gss_ctx_id_t *ctx,
                             gss_buffer_desc *buf,
                             gss_OID mech)
 {

diff --git a/lustre/utils/gss/context.h b/lustre/utils/gss/context.h
index d19c0fe..5cc7195 100644 (file)
--- a/lustre/utils/gss/context.h
+++ b/lustre/utils/gss/context.h
@@ -78,9 +78,9 @@ extern krb5_error_code krb5_derive_key(const void *enc,
                                       const krb5_data *in_constant);
 #endif
 
-int serialize_context_for_kernel(gss_ctx_id_t ctx, gss_buffer_desc *buf,
+int serialize_context_for_kernel(gss_ctx_id_t *ctx, gss_buffer_desc *buf,
                                 gss_OID mech);
-int serialize_spkm3_ctx(gss_ctx_id_t ctx, gss_buffer_desc *buf);
-int serialize_krb5_ctx(gss_ctx_id_t ctx, gss_buffer_desc *buf);
+int serialize_spkm3_ctx(gss_ctx_id_t *ctx, gss_buffer_desc *buf);
+int serialize_krb5_ctx(gss_ctx_id_t *ctx, gss_buffer_desc *buf);
 
 #endif /* _CONTEXT_H_ */

diff --git a/lustre/utils/gss/context_heimdal.c b/lustre/utils/gss/context_heimdal.c
index 25faa71..201ebf8 100644 (file)
--- a/lustre/utils/gss/context_heimdal.c
+++ b/lustre/utils/gss/context_heimdal.c
@@ -203,7 +203,7 @@ int write_heimdal_seq_key(char **p, char *end, gss_ctx_id_t ctx)
  */
 
 int
-serialize_krb5_ctx(gss_ctx_id_t ctx, gss_buffer_desc *buf)
+serialize_krb5_ctx(gss_ctx_id_t *ctx, gss_buffer_desc *buf)
 {
 
        char *p, *end;
@@ -219,7 +219,7 @@ serialize_krb5_ctx(gss_ctx_id_t ctx, gss_buffer_desc *buf)
 
 
        /* initiate:  1 => initiating 0 => accepting */
-       if (ctx->more_flags & LOCAL) {
+       if ((*ctx)->more_flags & LOCAL) {
                if (WRITE_BYTES(&p, end, constant_one)) goto out_err;
        }
        else {
@@ -242,19 +242,19 @@ serialize_krb5_ctx(gss_ctx_id_t ctx, gss_buffer_desc *buf)
        if (WRITE_BYTES(&p, end, algorithm)) goto out_err;
 
        /* endtime */
-       if (WRITE_BYTES(&p, end, ctx->lifetime)) goto out_err;
+       if (WRITE_BYTES(&p, end, (*ctx)->lifetime)) goto out_err;
 
        /* seq_send */
-       if (WRITE_BYTES(&p, end, ctx->auth_context->local_seqnumber))
+       if (WRITE_BYTES(&p, end, (*ctx)->auth_context->local_seqnumber))
                goto out_err;
        /* mech_used */
        if (write_buffer(&p, end, (gss_buffer_desc*)&krb5oid)) goto out_err;
 
        /* enc: derive the encryption key and copy it into buffer */
-       if (write_heimdal_enc_key(&p, end, ctx)) goto out_err;
+       if (write_heimdal_enc_key(&p, end, *ctx)) goto out_err;
 
        /* seq: get the sequence number key and copy it into buffer */
-       if (write_heimdal_seq_key(&p, end, ctx)) goto out_err;
+       if (write_heimdal_seq_key(&p, end, *ctx)) goto out_err;
 
        buf->length = p - (char *)buf->value;
        printerr(2, "serialize_krb5_ctx: returning buffer "

diff --git a/lustre/utils/gss/context_lucid.c b/lustre/utils/gss/context_lucid.c
index ed33a4f..a49ba61 100644 (file)
--- a/lustre/utils/gss/context_lucid.c
+++ b/lustre/utils/gss/context_lucid.c
@@ -573,7 +573,7 @@ out_err:
        return -1;
 }
 int
-serialize_krb5_ctx(gss_ctx_id_t ctx, gss_buffer_desc *buf)
+serialize_krb5_ctx(gss_ctx_id_t *ctx, gss_buffer_desc *buf)
 {
        OM_uint32 maj_stat, min_stat;
        void *return_ctx = 0;
@@ -582,7 +582,7 @@ serialize_krb5_ctx(gss_ctx_id_t ctx, gss_buffer_desc *buf)
        int retcode = 0;
 
        printerr(3, "lucid version!\n");
-       maj_stat = gss_krb5_export_lucid_sec_context(&min_stat, &ctx,
+       maj_stat = gss_krb5_export_lucid_sec_context(&min_stat, ctx,
                                                1, &return_ctx);
        if (maj_stat != GSS_S_COMPLETE) {
                pgsserr("gss_krb5_export_lucid_sec_context",

diff --git a/lustre/utils/gss/context_mit.c b/lustre/utils/gss/context_mit.c
index f6d0d24..b639689 100644 (file)
--- a/lustre/utils/gss/context_mit.c
+++ b/lustre/utils/gss/context_mit.c
@@ -275,9 +275,9 @@ typedef struct gss_union_ctx_id_t {
 } gss_union_ctx_id_desc, *gss_union_ctx_id_t;
 
 int
-serialize_krb5_ctx(gss_ctx_id_t ctx, gss_buffer_desc *buf)
+serialize_krb5_ctx(gss_ctx_id_t *ctx, gss_buffer_desc *buf)
 {
-       krb5_gss_ctx_id_t kctx = ((gss_union_ctx_id_t)ctx)->internal_ctx_id;
+       krb5_gss_ctx_id_t kctx = ((gss_union_ctx_id_t)*ctx)->internal_ctx_id;
        char *p, *end;
        static int constant_zero = 0;
        static int constant_one = 1;

diff --git a/lustre/utils/gss/context_spkm3.c b/lustre/utils/gss/context_spkm3.c
index 7a77bef..2ef1c1f 100644 (file)
--- a/lustre/utils/gss/context_spkm3.c
+++ b/lustre/utils/gss/context_spkm3.c
@@ -137,7 +137,7 @@ out_err:
  * and only export those fields to the kernel.
  */
 int
-serialize_spkm3_ctx(gss_ctx_id_t ctx, gss_buffer_desc *buf)
+serialize_spkm3_ctx(gss_ctx_id_t *ctx, gss_buffer_desc *buf)
 {
        OM_uint32 vers, ret, maj_stat, min_stat;
        void *ret_ctx = 0;
@@ -146,7 +146,7 @@ serialize_spkm3_ctx(gss_ctx_id_t ctx, gss_buffer_desc *buf)
        printerr(1, "serialize_spkm3_ctx called\n");
 
        printerr(2, "DEBUG: serialize_spkm3_ctx: lucid version!\n");
-       maj_stat = gss_export_lucid_sec_context(&min_stat, &ctx, 1, &ret_ctx);
+       maj_stat = gss_export_lucid_sec_context(&min_stat, ctx, 1, &ret_ctx);
        if (maj_stat != GSS_S_COMPLETE)
                goto out_err;
 
@@ -160,7 +160,7 @@ serialize_spkm3_ctx(gss_ctx_id_t ctx, gss_buffer_desc *buf)
        }
        ret = prepare_spkm3_ctx_buffer(lctx, buf);
 
-       maj_stat = gss_free_lucid_sec_context(&min_stat, ctx, ret_ctx);
+       maj_stat = gss_free_lucid_sec_context(&min_stat, *ctx, ret_ctx);
 
        if (maj_stat != GSS_S_COMPLETE)
                printerr(0, "WARN: failed to free lucid sec context\n");

diff --git a/lustre/utils/gss/lgss_keyring.c b/lustre/utils/gss/lgss_keyring.c
index aef75fc..4df569c 100644 (file)
--- a/lustre/utils/gss/lgss_keyring.c
+++ b/lustre/utils/gss/lgss_keyring.c
@@ -748,7 +748,7 @@ retry_nego:
                goto out;
        }
 
-       rc = serialize_context_for_kernel(lnd.lnd_ctx, &lnd.lnd_ctx_token,
+       rc = serialize_context_for_kernel(&lnd.lnd_ctx, &lnd.lnd_ctx_token,
                                          lnd.lnd_mech);
        if (rc) {
                logmsg(LL_ERR, "key %08x: failed to export context\n", keyid);

diff --git a/lustre/utils/gss/svcgssd_proc.c b/lustre/utils/gss/svcgssd_proc.c
index 456e19d..3e6f001 100644 (file)
--- a/lustre/utils/gss/svcgssd_proc.c
+++ b/lustre/utils/gss/svcgssd_proc.c
@@ -868,15 +868,19 @@ static int handle_krb(struct svc_nego_data *snd)
 
        /* kernel needs ctx to calculate verifier on null response, so
         * must give it context before doing null call: */
-       if (serialize_context_for_kernel(snd->ctx, &snd->ctx_token, mech)) {
+       if (serialize_context_for_kernel(&snd->ctx, &snd->ctx_token, mech)) {
                printerr(LL_ERR,
                         "ERROR: %s: serialize_context_for_kernel failed\n",
                        __func__);
                snd->maj_stat = GSS_S_FAILURE;
                goto out_err;
        }
-       /* We no longer need the gss context */
-       gss_delete_sec_context(&ignore_min_stat, &snd->ctx, &ignore_out_tok);
+
+       /* heimdal/MIT implementations do not delete context at all */
+       if (snd->ctx != GSS_C_NO_CONTEXT)
+               gss_delete_sec_context(&ignore_min_stat, &snd->ctx,
+                                      &ignore_out_tok);
+
        do_svc_downcall(&snd->out_handle, &cred, mech, &snd->ctx_token);
        /* We no longer need the context token */
        if (snd->ctx_token.value) {