LU-12992 gss: retry in case of short computed shared key

Lustre uses OpenSSL's DH_compute_key() to compute shared secret key.
There is around 1 chance out of 256 that the returned shared key
is shorter than expected.
https://www.qacafe.com/articles/router-vpn-implementation-pitfalls/
If the returned key is short by 1 or 2 bytes, we prepend it with 0s.
Otherwise, we retry twice as it should finally be the expected length.

Test-Parameters: envdefinitions=SHARED_KEY=true testlist=sanity,recovery-small
Test-Parameters: envdefinitions=SHARED_KEY=true testlist=sanity,recovery-small
Test-Parameters: envdefinitions=SHARED_KEY=true testlist=sanity,recovery-small
Test-Parameters: envdefinitions=SHARED_KEY=true testlist=sanity,recovery-small
Test-Parameters: envdefinitions=SHARED_KEY=true testlist=sanity,recovery-small
Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
Change-Id: I519194ef52539ca9a6305120fbf00b60ca985b1f
Reviewed-on: https://review.whamcloud.com/37064
Tested-by: jenkins <devops@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Jeremy Filizetti <jeremy.filizetti@gmail.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/utils/gss/lgss_sk_utils.c b/lustre/utils/gss/lgss_sk_utils.c
index 04133f1..3d12921 100644 (file)
--- a/lustre/utils/gss/lgss_sk_utils.c
+++ b/lustre/utils/gss/lgss_sk_utils.c
@@ -176,7 +176,7 @@ static int lgss_sk_validate_cred(struct lgss_cred *cred, gss_buffer_desc *token,
        }
 
        rc = sk_compute_dh_key(skc, &bufs[SK_RESP_PUB_KEY]);
-       if (rc == GSS_S_DEFECTIVE_TOKEN) {
+       if (rc == GSS_S_BAD_QOP) {
                /* Defective token for short key means we need to retry
                 * because there is a chance that the parameters generated
                 * resulted in a key that is 1 byte short */

diff --git a/lustre/utils/gss/sk_utils.c b/lustre/utils/gss/sk_utils.c
index a5c09df..d3cf5ff 100644 (file)
--- a/lustre/utils/gss/sk_utils.c
+++ b/lustre/utils/gss/sk_utils.c
@@ -1162,10 +1162,26 @@ uint32_t sk_compute_dh_key(struct sk_cred *skc, const gss_buffer_desc *pub_key)
                         ERR_error_string(ERR_get_error(), NULL));
                goto out_err;
        } else if (status < dh_shared->length) {
-               printerr(0, "DH_compute_key() returned a short key of %d "
-                        "bytes, expected: %zu\n", status, dh_shared->length);
-               rc = GSS_S_DEFECTIVE_TOKEN;
-               goto out_err;
+               /* there is around 1 chance out of 256 that the returned
+                * shared key is shorter than expected
+                */
+               if (status >= dh_shared->length - 2) {
+                       int shift = dh_shared->length - status;
+                       /* if the key is short by only 1 or 2 bytes, just
+                        * prepend it with 0s
+                        */
+                       memmove((void *)(dh_shared->value + shift),
+                               dh_shared->value, status);
+                       memset(dh_shared->value, 0, shift);
+               } else {
+                       /* if the key is really too short, return GSS_S_BAD_QOP
+                        * so that the caller can retry to generate
+                        */
+                       printerr(0, "DH_compute_key() returned a short key of %d bytes, expected: %zu\n",
+                                status, dh_shared->length);
+                       rc = GSS_S_BAD_QOP;
+                       goto out_err;
+               }
        }
 
        rc = GSS_S_COMPLETE;

diff --git a/lustre/utils/gss/svcgssd_proc.c b/lustre/utils/gss/svcgssd_proc.c
index f2e94fc..d3e1b04 100644 (file)
--- a/lustre/utils/gss/svcgssd_proc.c
+++ b/lustre/utils/gss/svcgssd_proc.c
@@ -364,6 +364,7 @@ int handle_sk(struct svc_nego_data *snd)
        uint32_t version;
        uint32_t flags;
        int i;
+       int attempts = 0;
 
        printerr(3, "Handling sk request\n");
        memset(bufs, 0, sizeof(gss_buffer_desc) * SK_INIT_BUFFERS);
@@ -458,12 +459,34 @@ int handle_sk(struct svc_nego_data *snd)
                goto cleanup_partial;
        }
 
+redo:
        rc = sk_gen_params(skc);
        if (rc != GSS_S_COMPLETE) {
                printerr(0, "Failed to generate DH params for responder\n");
                goto cleanup_partial;
        }
-       if (sk_compute_dh_key(skc, &remote_pub_key)) {
+       rc = sk_compute_dh_key(skc, &remote_pub_key);
+       if (rc == GSS_S_BAD_QOP && attempts < 2) {
+               /* GSS_S_BAD_QOP means the generated shared key was shorter
+                * than expected. Just retry twice before giving up.
+                */
+               attempts++;
+               if (skc->sc_params) {
+                       DH_free(skc->sc_params);
+                       skc->sc_params = NULL;
+               }
+               if (skc->sc_pub_key.value) {
+                       free(skc->sc_pub_key.value);
+                       skc->sc_pub_key.value = NULL;
+               }
+               skc->sc_pub_key.length = 0;
+               if (skc->sc_dh_shared_key.value) {
+                       free(skc->sc_dh_shared_key.value);
+                       skc->sc_dh_shared_key.value = NULL;
+               }
+               skc->sc_dh_shared_key.length = 0;
+               goto redo;
+       } else if (rc != GSS_S_COMPLETE) {
                printerr(0, "Failed to compute session key from DH params\n");
                goto cleanup_partial;
        }