Refactor XDR code, add support for fixed-length opaque data.

4 files changed, 229 insertions, 150 deletions

diff --git a/tests/Makefile b/tests/Makefile
index 79cb3ff..d64728f 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -45,7 +45,7 @@ CFLAGS		?= -g3 -Wall -fPIC -std=c99 -I${LIBHAL_SRC} -I${LIBTFM_BLD}
 
 CORE_TESTS	= test-aes-key-wrap test-hash test-pbkdf2 test-ecdsa test-bus test-trng test-rsa test-mkmif
 SERVER_TESTS	= test-rpc_server
-CLIENT_TESTS	= test-rpc_hash test-rpc_pkey test-rpc_get_version test-rpc_get_random test-rpc_login test-rpc_bighash
+CLIENT_TESTS	= test-rpc_hash test-rpc_pkey test-rpc_get_version test-rpc_get_random test-rpc_login test-rpc_bighash test-xdr
 
 ALL_TESTS	= ${CORE_TESTS} ${SERVER_TESTS} ${CLIENT_TESTS}
 
diff --git a/tests/test-xdr.c b/tests/test-xdr.c
new file mode 100644
index 0000000..eedf48d
--- /dev/null
+++ b/tests/test-xdr.c
@@ -0,0 +1,111 @@
+/*
+ * xdr.c
+ * -----
+ * Serialization/deserialization routines, using XDR (RFC 4506) encoding.
+ * These functions are not part of the public libhal API.
+ *
+ * Copyright (c) 2016-2018, NORDUnet A/S All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ * - Redistributions of source code must retain the above copyright notice,
+ *   this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in the
+ *   documentation and/or other materials provided with the distribution.
+ *
+ * - Neither the name of the NORDUnet nor the names of its contributors may
+ *   be used to endorse or promote products derived from this software
+ *   without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
+ * IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+ * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED
+ * TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <stdio.h>
+#include <stdint.h>
+#include <stddef.h>		/* ptrdiff_t */
+#include <string.h>             /* memcpy, memset */
+
+#include "hal.h"
+#include "hal_internal.h"	/* htonl/ntohl */
+#include "xdr_internal.h"
+
+static void hexdump(uint8_t *buf, uint32_t len)
+{
+    for (uint32_t i = 0; i < len; ++i)
+        printf("%02x%c", buf[i], ((i & 0x07) == 0x07) ? '\n' : ' ');
+    if ((len & 0x07) != 0)
+        printf("\n");
+}
+
+int main(int argc, char *argv[])
+{
+    uint32_t i;
+    uint8_t buf[256] = {0};
+    uint8_t *bufptr = buf;
+    const uint8_t *readptr;
+    uint8_t *limit = buf + sizeof(buf);
+    hal_error_t ret;
+    uint8_t alphabet[] = "abcdefghijklmnopqrstuvwxyz";
+    uint8_t readbuf[256] = {0};
+
+    printf("hal_xdr_encode_int: work to failure\n");
+    for (i = 1; i < 100; ++i) {
+        if ((ret = hal_xdr_encode_int(&bufptr, limit, i)) != HAL_OK) {
+            printf("%d: %s\n", i, hal_error_string(ret));
+            break;
+        }
+    }
+    hexdump(buf, ((uint8_t *)bufptr - buf));
+
+    printf("\nhal_xdr_decode_int:\n");
+    readptr = buf;
+    while (readptr < bufptr) {
+        if ((ret = hal_xdr_decode_int(&readptr, limit, &i)) != HAL_OK) {
+            printf("%s\n", hal_error_string(ret));
+            break;
+        }
+        printf("%u ", i);
+    }
+    printf("\n");
+
+    printf("\nhal_xdr_encode_variable_opaque: work to failure\n");
+    memset(buf, 0, sizeof(buf));
+    bufptr = buf;
+     for (i = 1; ; ++i) {
+        if ((ret = hal_xdr_encode_variable_opaque(&bufptr, limit, alphabet, i)) != HAL_OK) {
+            printf("%d: %s\n", i, hal_error_string(ret));
+            break;
+        }
+    }
+    hexdump(buf, ((uint8_t *)bufptr - buf));
+
+    printf("\nhal_xdr_decode_variable_opaque:\n");
+    readptr = buf;
+    while (readptr < bufptr) {
+        size_t len = bufptr - readptr;
+        if ((ret = hal_xdr_decode_variable_opaque(&readptr, limit, readbuf, &len)) != HAL_OK) {
+            printf("%s\n", hal_error_string(ret));
+            break;
+        }
+        printf("%lu: ", len);
+        for (size_t j = 0; j < len; ++j)
+            putchar(readbuf[j]);
+        putchar('\n');
+        memset(readbuf, 0, sizeof(readbuf));
+    }
+
+    return 0;
+}
diff --git a/xdr.c b/xdr.c
index e7c81b2..9a958ac 100644
--- a/xdr.c
+++ b/xdr.c
@@ -2,8 +2,9 @@
  * xdr.c
  * -----
  * Serialization/deserialization routines, using XDR (RFC 4506) encoding.
+ * These functions are not part of the public libhal API.
  *
- * Copyright (c) 2016, NORDUnet A/S All rights reserved.
+ * Copyright (c) 2016-2018, NORDUnet A/S All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -38,7 +39,7 @@
 #include <string.h>             /* memcpy, memset */
 
 #include "hal.h"
-#include "hal_internal.h"
+#include "hal_internal.h"	/* htonl/ntohl */
 #include "xdr_internal.h"
 
 /* encode/decode_int. This covers int, unsigned int, enum, and bool types,
@@ -61,7 +62,8 @@ hal_error_t hal_xdr_encode_int(uint8_t ** const outbuf, const uint8_t * const li
     return HAL_OK;
 }
 
-hal_error_t hal_xdr_decode_int(const uint8_t ** const inbuf, const uint8_t * const limit, uint32_t *value)
+/* decode an integer value without advancing the input pointer */
+hal_error_t hal_xdr_decode_int_peek(const uint8_t ** const inbuf, const uint8_t * const limit, uint32_t *value)
 {
     /* arg checks */
     if (inbuf == NULL || *inbuf == NULL || limit == NULL || value == NULL)
@@ -72,44 +74,38 @@ hal_error_t hal_xdr_decode_int(const uint8_t ** const inbuf, const uint8_t * con
         return HAL_ERROR_XDR_BUFFER_OVERFLOW;
 
     *value = ntohl(**(uint32_t **)inbuf);
-    *inbuf += sizeof(*value);
     return HAL_OK;
 }
 
-/* Undo the last decode_int - roll back the input pointer.
- */
-hal_error_t hal_xdr_undecode_int(const uint8_t ** const inbuf)
+/* decode an integer value the regular way */
+hal_error_t hal_xdr_decode_int(const uint8_t ** const inbuf, const uint8_t * const limit, uint32_t *value)
 {
-    if (inbuf == NULL || *inbuf == NULL)
-        return HAL_ERROR_BAD_ARGUMENTS;
+    hal_error_t err;
 
-    *inbuf -= sizeof(uint32_t);
-    return HAL_OK;
+    if ((err = hal_xdr_decode_int_peek(inbuf, limit, value)) == HAL_OK)
+        *inbuf += sizeof(*value);
+
+    return err;
 }
 
-/* encode/decode_buffer. This covers variable-length string and opaque types.
- * The data is preceded by a 4-byte length word (encoded as above), and padded
- * to a multiple of 4 bytes as necessary.
+/* encode/decode_fixed_opaque. This covers fixed-length string and opaque types.
+ * The data is padded to a multiple of 4 bytes as necessary.
  */
 
-hal_error_t hal_xdr_encode_buffer(uint8_t **outbuf, const uint8_t * const limit, const uint8_t *value, const uint32_t len)
+hal_error_t hal_xdr_encode_fixed_opaque(uint8_t ** const outbuf, const uint8_t * const limit, const uint8_t * const value, const size_t len)
 {
-    hal_error_t ret;
+    if (len == 0)
+        return HAL_OK;
 
     /* arg checks */
-    if (outbuf == NULL || *outbuf == NULL || limit == NULL ||
-        (value == NULL && len != 0))
+    if (outbuf == NULL || *outbuf == NULL || limit == NULL || value == NULL)
         return HAL_ERROR_BAD_ARGUMENTS;
 
     /* buffer overflow check */
-    if (limit - *outbuf < (ptrdiff_t)(((len + 3) & ~3) + sizeof(len)))
+    if (limit - *outbuf < (ptrdiff_t)((len + 3) & ~3))
         return HAL_ERROR_XDR_BUFFER_OVERFLOW;
 
-    /* encode length */
-    if ((ret = hal_xdr_encode_int(outbuf, limit, len)) != HAL_OK)
-        return ret;
-
-    /* write the string or opaque data */
+    /* write the data */
     memcpy(*outbuf, value, len);
     *outbuf += len;
 
@@ -123,139 +119,101 @@ hal_error_t hal_xdr_encode_buffer(uint8_t **outbuf, const uint8_t * const limit,
     return HAL_OK;
 }
 
-/* This version returns a pointer to the data in the input buffer.
- * It is used in the rpc server.
- */
-hal_error_t hal_xdr_decode_buffer_in_place(const uint8_t **inbuf, const uint8_t * const limit, const uint8_t ** const value, uint32_t * const len)
+hal_error_t hal_xdr_decode_fixed_opaque_ptr(const uint8_t ** const inbuf, const uint8_t * const limit, const uint8_t ** const value, const size_t len)
 {
-    hal_error_t ret;
-    uint32_t xdr_len;
-    const uint8_t *orig_inbuf = *inbuf;
-
     /* arg checks */
-    if (inbuf == NULL || *inbuf == NULL || limit == NULL || value == NULL || len == NULL)
+    if (inbuf == NULL || *inbuf == NULL || limit == NULL || value == NULL)
         return HAL_ERROR_BAD_ARGUMENTS;
 
-    /* decode the length */
-    if ((ret = hal_xdr_decode_int(inbuf, limit, &xdr_len)) != HAL_OK)
-        return ret;
-
-    /* input and output buffer overflow checks vs decoded length */
-
-    /* decoded length is past the end of the input buffer;
-     * we're probably out of sync, but nothing we can do now
-     */
-    if (limit - *inbuf < (ptrdiff_t)xdr_len) {
-        /* undo read of length */
-        *inbuf = orig_inbuf;
+    /* buffer overflow check */
+    if (limit - *inbuf < (ptrdiff_t)len)
         return HAL_ERROR_XDR_BUFFER_OVERFLOW;
-    }
 
-    /* return a pointer to the string or opaque data */
+    /* return a pointer to the data */
     *value = *inbuf;
-    *len = xdr_len;
 
     /* update the buffer pointer, skipping any padding bytes */
-    *inbuf += (xdr_len + 3) & ~3;
+    *inbuf += ((len + 3) & ~3);
 
     return HAL_OK;
 }
 
-/* This version copies the data to the user-supplied buffer.
- * It is used in the rpc client.
- */
-hal_error_t hal_xdr_decode_buffer(const uint8_t **inbuf, const uint8_t * const limit, uint8_t * const value, uint32_t * const len)
+hal_error_t hal_xdr_decode_fixed_opaque(const uint8_t ** const inbuf, const uint8_t * const limit, uint8_t * const value, const size_t len)
 {
-    if (inbuf == NULL || value == NULL || len == NULL)
-        return HAL_ERROR_BAD_ARGUMENTS;
+    const uint8_t *p;
+    hal_error_t err;
 
-    hal_error_t ret;
-    const uint8_t *vptr;
-    const uint8_t *orig_inbuf = *inbuf;
-    uint32_t xdr_len;
+    /* get and advance the input data pointer */
+    if ((err = hal_xdr_decode_fixed_opaque_ptr(inbuf, limit, &p, len)) == HAL_OK)
+        /* read the data */
+        memcpy(value, p, len);
+
+    return err;
+}
 
-    if ((ret = hal_xdr_decode_buffer_in_place(inbuf, limit, &vptr, &xdr_len)) != HAL_OK)
-        return ret;
+/* encode/decode_variable_opaque. This covers variable-length string and opaque types.
+ * The data is preceded by a 4-byte length word (encoded as above), and padded
+ * to a multiple of 4 bytes as necessary.
+ */
 
-    if (*len < xdr_len) {
-        /* user buffer is too small, undo read of length */
-        *inbuf = orig_inbuf;
-	ret = HAL_ERROR_XDR_BUFFER_OVERFLOW;
-    }
-    else {
-        memcpy(value, vptr, xdr_len);
-    }
+hal_error_t hal_xdr_encode_variable_opaque(uint8_t ** const outbuf, const uint8_t * const limit, const uint8_t * const value, const size_t len)
+{
+    hal_error_t err;
 
-    *len = xdr_len;
+    /* encode length */
+    if ((err = hal_xdr_encode_int(outbuf, limit, (uint32_t)len)) == HAL_OK) {
+        /* encode data */
+        if ((err = hal_xdr_encode_fixed_opaque(outbuf, limit, value, len)) != HAL_OK)
+            /* undo write of length */
+            *outbuf -= 4;
+    }
 
-    return ret;
+    return err;
 }
 
-/* ---------------------------------------------------------------- */
-
-#ifdef TEST
-static void hexdump(uint8_t *buf, uint32_t len)
+/* This version returns a pointer to the data in the input buffer.
+ * It is used in the rpc server.
+ */
+hal_error_t hal_xdr_decode_variable_opaque_ptr(const uint8_t ** const inbuf, const uint8_t * const limit, const uint8_t ** const value, size_t * const len)
 {
-    for (uint32_t i = 0; i < len; ++i)
-        printf("%02x%c", buf[i], ((i & 0x07) == 0x07) ? '\n' : ' ');
-    if ((len & 0x07) != 0)
-        printf("\n");
+    hal_error_t err;
+    uint32_t xdr_len;
+
+    /* arg checks */
+    if (len == NULL)
+        return HAL_ERROR_BAD_ARGUMENTS;
+
+    /* read length */
+    if ((err = hal_xdr_decode_int(inbuf, limit, &xdr_len)) == HAL_OK) {
+        /* get the data pointer */
+        if ((err = hal_xdr_decode_fixed_opaque_ptr(inbuf, limit, value, xdr_len)) == HAL_OK)
+            *len = xdr_len;
+        else
+            /* undo read of length */
+            *inbuf -= 4;
+    }
+
+    return err;
 }
 
-int main(int argc, char *argv[])
+/* This version copies the data to the user-supplied buffer.
+ * It is used in the rpc client.
+ */
+hal_error_t hal_xdr_decode_variable_opaque(const uint8_t ** const inbuf, const uint8_t * const limit, uint8_t * const value, size_t * const len)
 {
-    uint32_t i;
-    uint8_t buf[64] = {0};
-    uint8_t *bufptr = buf, *readptr;
-    uint8_t *limit = buf + sizeof(buf);
-    hal_error_t ret;
-    uint8_t alphabet[] = "abcdefghijklmnopqrstuvwxyz";
-    uint8_t readbuf[64] = {0};
-
-    printf("hal_xdr_encode_int: work to failure\n");
-    for (i = 1; i < 100; ++i) {
-        if ((ret = hal_xdr_encode_int(&bufptr, limit, i)) != HAL_OK) {
-            printf("%d: %s\n", i, hal_error_string(ret));
-            break;
-        }
-    }
-    hexdump(buf, ((uint8_t *)bufptr - buf));
-
-    printf("\nhal_xdr_decode_int:\n");
-    readptr = buf;
-    while (readptr < bufptr) {
-        if ((ret = hal_xdr_decode_int(&readptr, limit, &i)) != HAL_OK) {
-            printf("%s\n", hal_error_string(ret));
-            break;
-        }
-        printf("%u ", i);
-    }
-    printf("\n");
-
-    printf("\nhal_xdr_encode_buffer: work to failure\n");
-    memset(buf, 0, sizeof(buf));
-    bufptr = buf;
-     for (i = 1; i < 10; ++i) {
-        if ((ret = hal_xdr_encode_buffer(&bufptr, limit, alphabet, i)) != HAL_OK) {
-            printf("%d: %s\n", i, hal_error_string(ret));
-            break;
-        }
-    }
-    hexdump(buf, ((uint8_t *)bufptr - buf));
-
-    printf("\nhal_xdr_decode_buffer:\n");
-    readptr = buf;
-    i = sizeof(readbuf);
-    while (readptr < bufptr) {
-        if ((ret = hal_xdr_decode_buffer(&readptr, limit, readbuf, &i)) != HAL_OK) {
-            printf("%s\n", hal_error_string(ret));
-            break;
-        }
-        printf("%u: ", i); for (int j = 0; j < i; ++j) putchar(readbuf[j]); putchar('\n');
-        i = sizeof(readbuf);
-        memset(readbuf, 0, sizeof(readbuf));
+    hal_error_t err;
+    size_t xdr_len;
+    const uint8_t *p;
+
+    /* read data pointer and length */
+    if ((err = hal_xdr_decode_variable_opaque_ptr(inbuf, limit, &p, &xdr_len)) == HAL_OK) {
+        /* user buffer overflow check */
+        if (*len < xdr_len)
+            return HAL_ERROR_XDR_BUFFER_OVERFLOW;
+        /* read the data */
+        memcpy(value, p, xdr_len);
+        *len = xdr_len;
     }
 
-    return 0;
+    return err;
 }
-#endif
diff --git a/xdr_internal.h b/xdr_internal.h
index 9d02fd3..aa3a1e9 100644
--- a/xdr_internal.h
+++ b/xdr_internal.h
@@ -2,8 +2,9 @@
  * xdr_internal.h
  * --------------
  * Serialization/deserialization routines, using XDR (RFC 4506) encoding.
+ * These functions are not part of the public libhal API.
  *
- * Copyright (c) 2016, NORDUnet A/S All rights reserved.
+ * Copyright (c) 2016-2018, NORDUnet A/S All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -35,10 +36,6 @@
 #ifndef _XDR_INTERNAL_H
 #define _XDR_INTERNAL_H
 
- /*
- * RPC serialization/deserialization routines, using XDR (RFC 4506) encoding.
- */
-
 hal_error_t hal_xdr_encode_int(uint8_t ** const outbuf,
                                const uint8_t * const limit,
                                const uint32_t value);
@@ -47,22 +44,35 @@ hal_error_t hal_xdr_decode_int(const uint8_t ** const inbuf,
                                const uint8_t * const limit,
                                uint32_t * const value);
 
-hal_error_t hal_xdr_undecode_int(const uint8_t ** const inbuf);
+hal_error_t hal_xdr_decode_int_peek(const uint8_t ** const inbuf,
+                                    const uint8_t * const limit,
+                                    uint32_t * const value);
+
+hal_error_t hal_xdr_encode_fixed_opaque(uint8_t ** const outbuf,
+                                        const uint8_t * const limit,
+                                        const uint8_t * const value, const size_t len);
 
-hal_error_t hal_xdr_encode_buffer(uint8_t ** const outbuf,
-                                  const uint8_t * const limit,
-                                  const uint8_t * const value,
-                                  const uint32_t len);
+hal_error_t hal_xdr_decode_fixed_opaque(const uint8_t ** const inbuf,
+                                        const uint8_t * const limit,
+                                        uint8_t * const value, const size_t len);
 
-hal_error_t hal_xdr_decode_buffer_in_place(const uint8_t ** const inbuf,
+hal_error_t hal_xdr_decode_fixed_opaque_ptr(const uint8_t ** const inbuf,
+                                            const uint8_t * const limit,
+                                           const  uint8_t ** const vptr, const size_t len);
+
+hal_error_t hal_xdr_encode_variable_opaque(uint8_t ** const outbuf,
                                            const uint8_t * const limit,
-                                           const uint8_t ** const vptr,
-                                           uint32_t * const len);
+                                           const uint8_t * const value,
+                                           const size_t len);
 
-hal_error_t hal_xdr_decode_buffer(const uint8_t ** const inbuf,
-                                  const uint8_t * const limit,
-                                  uint8_t * const value,
-                                  uint32_t * const len);
+hal_error_t hal_xdr_decode_variable_opaque(const uint8_t ** const inbuf,
+                                           const uint8_t * const limit,
+                                           uint8_t * const value,
+                                           size_t * const len);
 
+hal_error_t hal_xdr_decode_variable_opaque_ptr(const uint8_t ** const inbuf,
+                                               const uint8_t * const limit,
+                                               const uint8_t ** const vptr,
+                                               size_t * const len);
 
 #endif /* _XDR_INTERNAL_H*/