Simplified the _gnutls13_psk_ext_parser interface and added unit tests

Signed-off-by: Nikos Mavrogiannopoulos <nmav@redhat.com>

diff --git a/lib/auth/psk.h b/lib/auth/psk.h
index 783d4f99adc5df9e7a13dd155e06bc0d9f5cceb1..3d204bc8c8f5d996e4fac3b39b8fec8832821213 100644 (file)
--- a/lib/auth/psk.h
+++ b/lib/auth/psk.h
@@ -63,11 +63,10 @@ typedef struct psk_auth_info_st {
        char hint[MAX_USERNAME_SIZE + 1];
 } *psk_auth_info_t;
 
+typedef struct psk_auth_info_st psk_auth_info_st;
 
 #ifdef ENABLE_PSK
 
-typedef struct psk_auth_info_st psk_auth_info_st;
-
 int
 _gnutls_set_psk_session_key(gnutls_session_t session, gnutls_datum_t * key,
                            gnutls_datum_t * psk2);

diff --git a/lib/ext/pre_shared_key.c b/lib/ext/pre_shared_key.c
index 02c2288528aa969899e1124f82cf8067e4a99457..920ae17398a790af474f181a6bdf8ca4c98f2ad3 100644 (file)
--- a/lib/ext/pre_shared_key.c
+++ b/lib/ext/pre_shared_key.c
@@ -263,10 +263,9 @@ static int server_recv_params(gnutls_session_t session,
        struct psk_st psk;
 
        ret = _gnutls13_psk_ext_parser_init(&psk_parser, data, len);
-       if (ret == 0) {
-               /* No PSKs advertised by client */
-               return 0;
-       } else if (ret < 0) {
+       if (ret < 0) {
+               if (ret == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) /* No PSKs advertised by client */
+                       return 0;
                return gnutls_assert_val(ret);
        }
 
@@ -295,10 +294,8 @@ static int server_recv_params(gnutls_session_t session,
                return gnutls_assert_val(ret);
 
        /* Get full ClientHello */
-       if (!_gnutls_ext_get_full_client_hello(session, &full_client_hello)) {
-               ret = 0;
-               goto cleanup;
-       }
+       if (!_gnutls_ext_get_full_client_hello(session, &full_client_hello))
+               return 0;
 
        /* Compute the binder value for this PSK */
        prf = pskcred->binder_algo;
@@ -306,16 +303,13 @@ static int server_recv_params(gnutls_session_t session,
        ret = compute_psk_binder(GNUTLS_SERVER, prf, hash_size, hash_size, 0, 0,
                                 &key, &full_client_hello,
                                 binder_value);
-       if (ret < 0) {
-               gnutls_assert();
-               goto cleanup;
-       }
+       if (ret < 0)
+               return gnutls_assert_val(ret);
 
        if (_gnutls_mac_get_algo_len(prf) != binder_recvd.size ||
            safe_memcmp(binder_value, binder_recvd.data, binder_recvd.size)) {
                gnutls_free(key.data);
-               ret = gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
-               goto cleanup;
+               return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
        }
 
        if (session->internals.hsk_flags & HSK_PSK_KE_MODE_DHE_PSK)
@@ -335,11 +329,7 @@ static int server_recv_params(gnutls_session_t session,
        session->key.proto.tls13.psk_index = psk_index;
        session->key.proto.tls13.binder_prf = prf;
 
-       ret = 0;
- cleanup:
-       _gnutls_free_datum(&binder_recvd);
-
-       return ret;
+       return 0;
 }
 
 /*

diff --git a/lib/libgnutls.map b/lib/libgnutls.map
index 07aaf714ccb21c669055a1cc2e0b126e015b1aee..45aa9b94312b88c027ed8f46de668543e30494b2 100644 (file)
--- a/lib/libgnutls.map
+++ b/lib/libgnutls.map
@@ -1218,6 +1218,7 @@ GNUTLS_3_6_3
 } GNUTLS_3_6_2;
 
 GNUTLS_FIPS140_3_4 {
+  global:
        gnutls_cipher_self_test;
        gnutls_pk_self_test;
        gnutls_mac_self_test;
@@ -1256,6 +1257,11 @@ GNUTLS_PRIVATE_3_4 {
        _gnutls_digest_exists;
        _gnutls_log;
 
+       # PSK extension parser unit test
+       _gnutls13_psk_ext_parser_init;
+       _gnutls13_psk_ext_parser_next_psk;
+       _gnutls13_psk_ext_parser_find_binder;
+
        # Internal symbols needed by gnutls-cli-debug:
        _gnutls_rsa_pms_set_version;
        _gnutls_record_set_default_version;

diff --git a/lib/tls13/psk_ext_parser.c b/lib/tls13/psk_ext_parser.c
index 7dd1427cf3dbaa751b9f110667f5735585d12c93..9f4087773d1f093a064078fdf2fbcd6029e3964e 100644 (file)
--- a/lib/tls13/psk_ext_parser.c
+++ b/lib/tls13/psk_ext_parser.c
@@ -20,23 +20,13 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>
  *
  */
-#include "tls13/psk_ext_parser.h"
-
-
-static int advance_to_end_of_object(psk_ext_parser_st *p)
-{
-       size_t adv;
 
-       /* Advance the pointer to the end of the current object */
-       if (p->obj_read < p->obj_len) {
-               adv = p->obj_len - p->obj_read;
-               DECR_LEN(p->len, adv);
-               p->data += adv;
-       }
-
-       return 0;
-}
+#include "gnutls_int.h"
+#include "tls13/psk_ext_parser.h"
 
+/* Returns GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE when no identities
+ * are present, or >= 0, on success.
+ */
 int _gnutls13_psk_ext_parser_init(psk_ext_parser_st *p,
                                  const unsigned char *data, size_t _len)
 {
@@ -48,105 +38,106 @@ int _gnutls13_psk_ext_parser_init(psk_ext_parser_st *p,
 
        memset(p, 0, sizeof(*p));
 
+       DECR_LEN(len, 2);
        identities_len = _gnutls_read_uint16(data);
+       data += 2;
 
-       if (identities_len > 0) {
-               DECR_LEN(len, 2);
-               data += 2;
+       if (identities_len == 0)
+               return gnutls_assert_val(GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE);
 
-               p->obj_len = identities_len;
-               p->data = (unsigned char *) data;
-               p->len = len;
-       }
+       p->id_len = identities_len;
+       p->data = (unsigned char *) data;
+       p->len = len;
+
+       DECR_LEN(len, p->id_len);
+       data += p->id_len;
 
-       return identities_len;
+       DECR_LEN(len, 2);
+       p->binder_len = _gnutls_read_uint16(data);
+
+       p->binder_data = p->data + p->id_len + 2;
+       DECR_LEN(len, p->binder_len);
+
+       return 0;
 }
 
 int _gnutls13_psk_ext_parser_next_psk(psk_ext_parser_st *p, struct psk_st *psk)
 {
-       if (p->obj_read >= p->obj_len)
+       if (p->id_read >= p->id_len)
                return GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE;
 
        /* Read a PskIdentity structure */
+       DECR_LEN(p->len, 2);
        psk->identity.size = _gnutls_read_uint16(p->data);
        if (psk->identity.size == 0)
                return GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE;
 
-       DECR_LEN(p->len, 2);
        p->data += 2;
-       p->obj_read += 2;
+       p->id_read += 2;
 
-       psk->identity.data = p->data;
+       psk->identity.data = (void*)p->data;
 
        DECR_LEN(p->len, psk->identity.size);
        p->data += psk->identity.size;
-       p->obj_read += psk->identity.size;
+       p->id_read += psk->identity.size;
 
-       psk->ob_ticket_age = _gnutls_read_uint32(p->data);
        DECR_LEN(p->len, 4);
+       psk->ob_ticket_age = _gnutls_read_uint32(p->data);
+
        p->data += 4;
-       p->obj_read += 4;
+       p->id_read += 4;
 
        return p->next_index++;
 }
 
+/* Output is a pointer to data, which shouldn't be de-allocated. */
 int _gnutls13_psk_ext_parser_find_binder(psk_ext_parser_st *p, int psk_index,
                                         gnutls_datum_t *binder_out)
 {
-       uint16_t binders_len;
        uint8_t binder_len;
        int cur_index = 0, binder_found = 0;
+       ssize_t len;
+       const uint8_t *data;
+       ssize_t read_data = 0;
 
        if (p == NULL || psk_index < 0 || binder_out == NULL)
                return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR);
 
-       if (p->obj_len == 0)
+       if (p->id_len == 0)
                return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR);
 
-       /* Place the pointer at the start of the binders */
-       if (advance_to_end_of_object(p) < 0)
-               return gnutls_assert_val(GNUTLS_E_UNEXPECTED_PACKET_LENGTH);
-
-       DECR_LEN(p->len, 2);
-       binders_len = _gnutls_read_uint16(p->data);
-       if (binders_len > 0) {
-               p->data += 2;
+       len = p->binder_len;
+       data = p->binder_data;
 
-               p->obj_len = binders_len;
-               p->obj_read = 0;
-       } else {
+       if (len == 0)
                return gnutls_assert_val(GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE);
-       }
 
        /* Start traversing the binders */
-       while (!binder_found && p->len > 0) {
-               DECR_LEN(p->len, 1);
-               binder_len = *(p->data);
+       while (!binder_found && len > 0) {
+               DECR_LEN(len, 1);
+               binder_len = *(data);
 
                if (binder_len == 0)
                        return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
 
-               p->data++;
-               p->obj_read++;
+               data++;
+               read_data++;
 
                if (cur_index == psk_index) {
                        /* We found the binder with the supplied index */
-                       binder_out->data = gnutls_malloc(binder_len);
-                       if (!binder_out->data)
-                               return gnutls_assert_val(GNUTLS_E_MEMORY_ERROR);
-
+                       DECR_LEN(len, binder_len);
+                       binder_out->data = (void*)data;
                        binder_out->size = binder_len;
-                       DECR_LEN(p->len, binder_len);
-                       memcpy(binder_out->data, p->data, binder_len);
-                       p->data += binder_len;
-                       p->obj_read += binder_len;
+
+                       data += binder_len;
+                       read_data += binder_len;
 
                        binder_found = 1;
                } else {
                        /* Not our binder - continue to the next one */
-                       DECR_LEN(p->len, binder_len);
-                       p->data += binder_len;
-                       p->obj_read += binder_len;
+                       DECR_LEN(len, binder_len);
+                       data += binder_len;
+                       read_data += binder_len;
 
                        cur_index++;
                }

diff --git a/lib/tls13/psk_ext_parser.h b/lib/tls13/psk_ext_parser.h
index 14f3c89186b4cb8e5272631e72eb5533b707bae7..2c6ab603ae7f363dd04b5be81f11006cbe27c45a 100644 (file)
--- a/lib/tls13/psk_ext_parser.h
+++ b/lib/tls13/psk_ext_parser.h
@@ -22,19 +22,23 @@
 
 #ifndef PSK_PARSER_H
 #define PSK_PARSER_H
-#include "gnutls_int.h"
+#include <gnutls/gnutls.h>
 
 struct psk_ext_parser_st {
-       unsigned char *data;
+       const unsigned char *data;
        ssize_t len;
-       size_t obj_len;
-       size_t obj_read;
+       size_t id_len;
+       size_t id_read;
        int next_index;
+
+       const unsigned char *binder_data;
+       ssize_t binder_len;
 };
 
 typedef struct psk_ext_parser_st psk_ext_parser_st;
 
 struct psk_st {
+       /* constant values */
        gnutls_datum_t identity;
        uint32_t ob_ticket_age;
 };

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5abd976ff87f68e0b86f4fb6dae3ae76dd067bf1..2f66ed5fc74511bcdd4534cb7329c97947eff077 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -108,6 +108,8 @@ ctests = tls13/supported_versions tls13/tls12-no-tls13-exts \
        tls12-rollback-detection tls11-rollback-detection \
        tls12-check-rollback-val tls11-check-rollback-val tls13/hello_random_value
 
+ctests += tls13/psk-ext
+
 ctests += tls13/key_update
 
 ctests += tls13/key_limits

diff --git a/tests/tls13/psk-ext.c b/tests/tls13/psk-ext.c
new file mode 100644 (file)
index 0000000..3d5c595
--- /dev/null
+++ b/tests/tls13/psk-ext.c
@@ -0,0 +1,170 @@
+/*
+ * Copyright (C) 2016, 2017 Red Hat, Inc.
+ *
+ * Author: Nikos Mavrogiannopoulos
+ *
+ * This file is part of GnuTLS.
+ *
+ * GnuTLS is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GnuTLS is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <string.h>
+#include <gnutls/gnutls.h>
+#include <stdint.h>
+#include "../lib/tls13/psk_ext_parser.h"
+
+#include "utils.h"
+
+/* Tests the PSK-extension decoding part */
+
+static void decode(const char *test_name, const gnutls_datum_t *raw, const gnutls_datum_t *id,
+                  const gnutls_datum_t *b, unsigned idx, int res)
+{
+       int ret;
+       psk_ext_parser_st p;
+       struct psk_st psk;
+       gnutls_datum_t binder;
+       unsigned found = 0;
+
+       ret = _gnutls13_psk_ext_parser_init(&p, raw->data, raw->size);
+       if (ret < 0) {
+               if (res == ret) /* expected */
+                       return;
+               fail("%s: _gnutls13_psk_ext_parser_init: %d/%s\n", test_name, ret, gnutls_strerror(ret));
+               exit(1);
+       }
+
+       while ((ret = _gnutls13_psk_ext_parser_next_psk(&p, &psk)) >= 0) {
+               if (ret == (int)idx) {
+                       if (psk.identity.size == id->size && memcmp(psk.identity.data, id->data, id->size) == 0) {
+                               if (debug)
+                                       success("%s: found id\n", test_name);
+                               found = 1;
+                               break;
+                       } else {
+                               fail("%s: did not found identity on index %d\n", test_name, idx);
+                       }
+               }
+       }
+
+       if (found == 0)
+               fail("%s: did not found identity!\n", test_name);
+
+       ret = _gnutls13_psk_ext_parser_find_binder(&p, idx, &binder);
+       if (ret < 0) {
+               fail("%s: could not extract binder: %s\n", test_name, gnutls_strerror(ret));
+       }
+
+       if (binder.size != b->size || memcmp(binder.data, b->data, b->size) != 0) {
+               hexprint(binder.data, binder.size);
+               fail("%s: did not match binder on index %d\n", test_name, idx);
+       }
+
+       return;
+}
+
+struct decode_tests_st {
+       const char *name;
+       gnutls_datum_t psk;
+       unsigned idx; /* the ID index */
+       gnutls_datum_t id;
+       gnutls_datum_t binder;
+       int res;
+};
+
+struct decode_tests_st decode_tests[] = {
+       {
+               .name = "single PSK",
+               .psk = { (unsigned char*)"\x00\x0a\x00\x04\x6e\x6d\x61\x76\x00\x00\x00\x00\x00\x21\x20\xc4\xda\xe5\x7e\x05\x59\xf7\xae\x9b\xba\x90\xd2\x6e\x12\x68\xf6\xc1\xc7\xb9\x7e\xdc\xed\x9e\x67\x4e\xa5\x91\x2d\x7c\xb4\xf0\xab", 47},
+               .id = { (unsigned char*)"nmav", 4 },
+               .binder = { (unsigned char*)"\xc4\xda\xe5\x7e\x05\x59\xf7\xae\x9b\xba\x90\xd2\x6e\x12\x68\xf6\xc1\xc7\xb9\x7e\xdc\xed\x9e\x67\x4e\xa5\x91\x2d\x7c\xb4\xf0\xab", 32 },
+               .idx = 0,
+               .res = 0
+       },
+       {
+               .name = "multiple psks id0",
+               .psk = { (unsigned char*)"\x00\x20\x00\x04\x70\x73\x6b\x31\x00\x00\x00\x00"
+                               "\x00\x06\x70\x73\x6b\x69\x64\x00\x00\x00\x00\x00"
+                               "\x00\x04\x74\x65\x73\x74\x00\x00\x00\x00\x00\x63"
+                               "\x20\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+                               "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+                               "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20\x01\x00"
+                               "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+                               "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+                               "\x00\x00\x00\x00\x00\x00\x20\x71\x83\x89\x3d\xcc"
+                               "\x46\xad\x83\x18\x98\x59\x46\x0b\xb2\x51\x24\x53"
+                               "\x41\xb4\x35\x04\x22\x90\x02\xac\x5e\xc1\xe7\xbc"
+                               "\xca\x52\x16", 135},
+               .id = { (unsigned char*)"psk1", 4 },
+               .binder = { (unsigned char*)"\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00", 32},
+               .idx = 0,
+               .res = 0
+       },
+       {
+               .name = "multiple psks id1",
+               .psk = { (unsigned char*)"\x00\x20\x00\x04\x70\x73\x6b\x31\x00\x00\x00\x00"
+                               "\x00\x06\x70\x73\x6b\x69\x64\x00\x00\x00\x00\x00"
+                               "\x00\x04\x74\x65\x73\x74\x00\x00\x00\x00\x00\x63"
+                               "\x20\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+                               "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+                               "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20\x01\x00"
+                               "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+                               "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+                               "\x00\x00\x00\x00\x00\x00\x20\x71\x83\x89\x3d\xcc"
+                               "\x46\xad\x83\x18\x98\x59\x46\x0b\xb2\x51\x24\x53"
+                               "\x41\xb4\x35\x04\x22\x90\x02\xac\x5e\xc1\xe7\xbc"
+                               "\xca\x52\x16", 135},
+               .id = { (unsigned char*)"pskid", 6 },
+               .binder = { (unsigned char*)"\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00", 32},
+               .idx = 1,
+               .res = 0
+       },
+       {
+               .name = "multiple psks id2",
+               .psk = { (unsigned char*)"\x00\x20\x00\x04\x70\x73\x6b\x31\x00\x00\x00\x00"
+                               "\x00\x06\x70\x73\x6b\x69\x64\x00\x00\x00\x00\x00"
+                               "\x00\x04\x74\x65\x73\x74\x00\x00\x00\x00\x00\x63"
+                               "\x20\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+                               "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+                               "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20\x01\x00"
+                               "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+                               "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+                               "\x00\x00\x00\x00\x00\x00\x20\x71\x83\x89\x3d\xcc"
+                               "\x46\xad\x83\x18\x98\x59\x46\x0b\xb2\x51\x24\x53"
+                               "\x41\xb4\x35\x04\x22\x90\x02\xac\x5e\xc1\xe7\xbc"
+                               "\xca\x52\x16", 135},
+               .id = { (unsigned char*)"test", 4 },
+               .binder = { (unsigned char*)"\x71\x83\x89\x3d\xcc\x46\xad\x83\x18\x98\x59\x46\x0b\xb2\x51\x24\x53\x41\xb4\x35\x04\x22\x90\x02\xac\x5e\xc1\xe7\xbc\xca\x52\x16", 32},
+               .idx = 2,
+               .res = 0
+       }
+};
+
+void doit(void)
+{
+       unsigned i;
+
+       for (i=0;i<sizeof(decode_tests)/sizeof(decode_tests[0]);i++) {
+               decode(decode_tests[i].name, &decode_tests[i].psk, &decode_tests[i].id,
+                      &decode_tests[i].binder, decode_tests[i].idx, decode_tests[i].res);
+       }
+}
+