Browse code

Cleanup tls_pre_decrypt_lite and tls_pre_encrypt

Mostly C90 -> C99 cleanups and "return immediately" instead of
wrapping function body into if.

(Review with ignore whitespace)

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200810143707.5834-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20676.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Arne Schwabe authored on 2020/08/10 23:36:52
Showing 1 changed files
... ...
@@ -3424,7 +3424,7 @@ tls_pre_decrypt(struct tls_multi *multi,
3424 3424
     }
3425 3425
 
3426 3426
     /*
3427
-     * If we detected new session in the last if block, i has
3427
+     * If we detected new session in the last if block, variable i has
3428 3428
      * changed to TM_ACTIVE, so check the condition again.
3429 3429
      */
3430 3430
     if (i == TM_SIZE && is_hard_reset_method2(op))
... ...
@@ -3437,7 +3437,7 @@ tls_pre_decrypt(struct tls_multi *multi,
3437 3437
 
3438 3438
         /*
3439 3439
          * If --single-session, don't allow any hard-reset connection request
3440
-         * unless it the the first packet of the session.
3440
+         * unless it the first packet of the session.
3441 3441
          */
3442 3442
         if (multi->opt.single_session)
3443 3443
         {
... ...
@@ -3665,100 +3665,91 @@ tls_pre_decrypt_lite(const struct tls_auth_standalone *tas,
3665 3665
                      const struct buffer *buf)
3666 3666
 
3667 3667
 {
3668
-    struct gc_arena gc = gc_new();
3669
-    bool ret = false;
3670
-
3671
-    if (buf->len > 0)
3668
+    if (buf->len <= 0)
3672 3669
     {
3673
-        int op;
3674
-        int key_id;
3670
+        return false;
3671
+    }
3672
+    struct gc_arena gc = gc_new();
3675 3673
 
3676
-        /* get opcode and key ID */
3677
-        {
3678
-            uint8_t c = *BPTR(buf);
3679
-            op = c >> P_OPCODE_SHIFT;
3680
-            key_id = c & P_KEY_ID_MASK;
3681
-        }
3674
+    /* get opcode and key ID */
3675
+    uint8_t pkt_firstbyte = *BPTR(buf);
3676
+    int op = pkt_firstbyte >> P_OPCODE_SHIFT;
3677
+    int key_id = pkt_firstbyte & P_KEY_ID_MASK;
3682 3678
 
3683
-        /* this packet is from an as-yet untrusted source, so
3684
-         * scrutinize carefully */
3679
+    /* this packet is from an as-yet untrusted source, so
3680
+     * scrutinize carefully */
3685 3681
 
3686
-        if (op != P_CONTROL_HARD_RESET_CLIENT_V2
3687
-            && op != P_CONTROL_HARD_RESET_CLIENT_V3)
3688
-        {
3689
-            /*
3690
-             * This can occur due to bogus data or DoS packets.
3691
-             */
3692
-            dmsg(D_TLS_STATE_ERRORS,
3693
-                 "TLS State Error: No TLS state for client %s, opcode=%d",
3694
-                 print_link_socket_actual(from, &gc),
3695
-                 op);
3696
-            goto error;
3697
-        }
3682
+    if (op != P_CONTROL_HARD_RESET_CLIENT_V2
3683
+        && op != P_CONTROL_HARD_RESET_CLIENT_V3)
3684
+    {
3685
+        /*
3686
+         * This can occur due to bogus data or DoS packets.
3687
+         */
3688
+        dmsg(D_TLS_STATE_ERRORS,
3689
+             "TLS State Error: No TLS state for client %s, opcode=%d",
3690
+             print_link_socket_actual(from, &gc),
3691
+             op);
3692
+        goto error;
3693
+    }
3698 3694
 
3699
-        if (key_id != 0)
3700
-        {
3701
-            dmsg(D_TLS_STATE_ERRORS,
3702
-                 "TLS State Error: Unknown key ID (%d) received from %s -- 0 was expected",
3703
-                 key_id,
3704
-                 print_link_socket_actual(from, &gc));
3705
-            goto error;
3706
-        }
3695
+    if (key_id != 0)
3696
+    {
3697
+        dmsg(D_TLS_STATE_ERRORS,
3698
+             "TLS State Error: Unknown key ID (%d) received from %s -- 0 was expected",
3699
+             key_id,
3700
+             print_link_socket_actual(from, &gc));
3701
+        goto error;
3702
+    }
3707 3703
 
3708
-        if (buf->len > EXPANDED_SIZE_DYNAMIC(&tas->frame))
3709
-        {
3710
-            dmsg(D_TLS_STATE_ERRORS,
3711
-                 "TLS State Error: Large packet (size %d) received from %s -- a packet no larger than %d bytes was expected",
3712
-                 buf->len,
3713
-                 print_link_socket_actual(from, &gc),
3714
-                 EXPANDED_SIZE_DYNAMIC(&tas->frame));
3715
-            goto error;
3716
-        }
3704
+    if (buf->len > EXPANDED_SIZE_DYNAMIC(&tas->frame))
3705
+    {
3706
+        dmsg(D_TLS_STATE_ERRORS,
3707
+             "TLS State Error: Large packet (size %d) received from %s -- a packet no larger than %d bytes was expected",
3708
+             buf->len,
3709
+             print_link_socket_actual(from, &gc),
3710
+             EXPANDED_SIZE_DYNAMIC(&tas->frame));
3711
+        goto error;
3712
+    }
3717 3713
 
3718
-        {
3719
-            struct buffer newbuf = clone_buf(buf);
3720
-            struct tls_wrap_ctx tls_wrap_tmp = tas->tls_wrap;
3721
-            bool status;
3722
-
3723
-            /* HMAC test, if --tls-auth was specified */
3724
-            status = read_control_auth(&newbuf, &tls_wrap_tmp, from, NULL);
3725
-            free_buf(&newbuf);
3726
-            free_buf(&tls_wrap_tmp.tls_crypt_v2_metadata);
3727
-            if (tls_wrap_tmp.cleanup_key_ctx)
3728
-            {
3729
-                free_key_ctx_bi(&tls_wrap_tmp.opt.key_ctx_bi);
3730
-            }
3731
-            if (!status)
3732
-            {
3733
-                goto error;
3734
-            }
3735 3714
 
3736
-            /*
3737
-             * At this point, if --tls-auth is being used, we know that
3738
-             * the packet has passed the HMAC test, but we don't know if
3739
-             * it is a replay yet.  We will attempt to defeat replays
3740
-             * by not advancing to the S_START state until we
3741
-             * receive an ACK from our first reply to the client
3742
-             * that includes an HMAC of our randomly generated 64 bit
3743
-             * session ID.
3744
-             *
3745
-             * On the other hand if --tls-auth is not being used, we
3746
-             * will proceed to begin the TLS authentication
3747
-             * handshake with only cursory integrity checks having
3748
-             * been performed, since we will be leaving the task
3749
-             * of authentication solely up to TLS.
3750
-             */
3715
+    struct buffer newbuf = clone_buf(buf);
3716
+    struct tls_wrap_ctx tls_wrap_tmp = tas->tls_wrap;
3751 3717
 
3752
-            ret = true;
3753
-        }
3718
+    /* HMAC test, if --tls-auth was specified */
3719
+    bool status = read_control_auth(&newbuf, &tls_wrap_tmp, from, NULL);
3720
+    free_buf(&newbuf);
3721
+    free_buf(&tls_wrap_tmp.tls_crypt_v2_metadata);
3722
+    if (tls_wrap_tmp.cleanup_key_ctx)
3723
+    {
3724
+        free_key_ctx_bi(&tls_wrap_tmp.opt.key_ctx_bi);
3725
+    }
3726
+    if (!status)
3727
+    {
3728
+        goto error;
3754 3729
     }
3730
+
3731
+    /*
3732
+     * At this point, if --tls-auth is being used, we know that
3733
+     * the packet has passed the HMAC test, but we don't know if
3734
+     * it is a replay yet.  We will attempt to defeat replays
3735
+     * by not advancing to the S_START state until we
3736
+     * receive an ACK from our first reply to the client
3737
+     * that includes an HMAC of our randomly generated 64 bit
3738
+     * session ID.
3739
+     *
3740
+     * On the other hand if --tls-auth is not being used, we
3741
+     * will proceed to begin the TLS authentication
3742
+     * handshake with only cursory integrity checks having
3743
+     * been performed, since we will be leaving the task
3744
+     * of authentication solely up to TLS.
3745
+     */
3755 3746
     gc_free(&gc);
3756
-    return ret;
3747
+    return true;
3757 3748
 
3758 3749
 error:
3759 3750
     tls_clear_error();
3760 3751
     gc_free(&gc);
3761
-    return ret;
3752
+    return false;
3762 3753
 }
3763 3754
 
3764 3755
 /* Choose the key with which to encrypt a data packet */
... ...
@@ -3767,48 +3758,51 @@ tls_pre_encrypt(struct tls_multi *multi,
3767 3767
                 struct buffer *buf, struct crypto_options **opt)
3768 3768
 {
3769 3769
     multi->save_ks = NULL;
3770
-    if (buf->len > 0)
3770
+    if (buf->len <= 0)
3771
+    {
3772
+        buf->len = 0;
3773
+        *opt = NULL;
3774
+        return;
3775
+    }
3776
+
3777
+    struct key_state *ks_select = NULL;
3778
+    for (int i = 0; i < KEY_SCAN_SIZE; ++i)
3771 3779
     {
3772
-        int i;
3773
-        struct key_state *ks_select = NULL;
3774
-        for (i = 0; i < KEY_SCAN_SIZE; ++i)
3780
+        struct key_state *ks = multi->key_scan[i];
3781
+        if (ks->state >= S_ACTIVE
3782
+            && (ks->authenticated == KS_AUTH_TRUE)
3783
+            && ks->crypto_options.key_ctx_bi.initialized
3784
+            )
3775 3785
         {
3776
-            struct key_state *ks = multi->key_scan[i];
3777
-            if (ks->state >= S_ACTIVE
3778
-                && (ks->authenticated == KS_AUTH_TRUE)
3779
-                && ks->crypto_options.key_ctx_bi.initialized
3780
-                )
3786
+            if (!ks_select)
3781 3787
             {
3782
-                if (!ks_select)
3783
-                {
3784
-                    ks_select = ks;
3785
-                }
3786
-                if (now >= ks->auth_deferred_expire)
3787
-                {
3788
-                    ks_select = ks;
3789
-                    break;
3790
-                }
3788
+                ks_select = ks;
3789
+            }
3790
+            if (now >= ks->auth_deferred_expire)
3791
+            {
3792
+                ks_select = ks;
3793
+                break;
3791 3794
             }
3792 3795
         }
3796
+    }
3793 3797
 
3794
-        if (ks_select)
3795
-        {
3796
-            *opt = &ks_select->crypto_options;
3797
-            multi->save_ks = ks_select;
3798
-            dmsg(D_TLS_KEYSELECT, "TLS: tls_pre_encrypt: key_id=%d", ks_select->key_id);
3799
-            return;
3800
-        }
3801
-        else
3802
-        {
3803
-            struct gc_arena gc = gc_new();
3804
-            dmsg(D_TLS_KEYSELECT, "TLS Warning: no data channel send key available: %s",
3805
-                 print_key_id(multi, &gc));
3806
-            gc_free(&gc);
3807
-        }
3798
+    if (ks_select)
3799
+    {
3800
+        *opt = &ks_select->crypto_options;
3801
+        multi->save_ks = ks_select;
3802
+        dmsg(D_TLS_KEYSELECT, "TLS: tls_pre_encrypt: key_id=%d", ks_select->key_id);
3803
+        return;
3808 3804
     }
3805
+    else
3806
+    {
3807
+        struct gc_arena gc = gc_new();
3808
+        dmsg(D_TLS_KEYSELECT, "TLS Warning: no data channel send key available: %s",
3809
+             print_key_id(multi, &gc));
3810
+        gc_free(&gc);
3809 3811
 
3810
-    buf->len = 0;
3811
-    *opt = NULL;
3812
+        *opt = NULL;
3813
+        buf->len = 0;
3814
+    }
3812 3815
 }
3813 3816
 
3814 3817
 void