Переглянути джерело

SecurityPolicy Interface change (#1613)

* Always use functions to get sizes and lengths.

This makes for better encapsulation and unifies the interface usage for
asymmetric and symmetric modules

* Add nonce length to SecurityPolicy and use it.

Previousely the key length was used as nonce length, which was
incorrect.
This only worked for basic128rsa15.

* Add certificate signing algorithm to interface

* Remove unnecessary functions from SecurityPolicy interface

* allow session to switch over to a new securechannel
Mark Giraud 6 роки тому
батько
коміт
37e8654da8

+ 2 - 2
CMakeLists.txt

@@ -250,13 +250,13 @@ if(NOT UA_COMPILE_AS_CXX AND (CMAKE_COMPILER_IS_GNUCC OR "x${CMAKE_C_COMPILER_ID
         remove_definitions(-Werror -Wpedantic -Wno-static-in-inline -fPIC)
         add_definitions(-D_WRS_KERNEL)
     endif()
-    
+
     if(UA_FREERTOS)
        SET(UA_FREERTOS_INCLUDES "" CACHE STRING "Folders to include from the freeRTOS OS")
        include_directories(${UA_FREERTOS_INCLUDES})
         # Disable flags for freeRTOS
         remove_definitions(-fPIC -Wconversion )
-        add_definitions(-DUA_FREERTOS -DLWIP_TIMEVAL_PRIVATE=0 -DLWIP_COMPAT_MUTEX=0 -DLWIP_POSIX_SOCKETS_IO_NAMES=0 -mcpu=cortex-m3 -mthumb -g -Wall -O0 -specs=nano.specs 
+        add_definitions(-DUA_FREERTOS -DLWIP_TIMEVAL_PRIVATE=0 -DLWIP_COMPAT_MUTEX=0 -DLWIP_POSIX_SOCKETS_IO_NAMES=0 -mcpu=cortex-m3 -mthumb -g -Wall -O0 -specs=nano.specs
                         -ffunction-sections -fdata-sections  -fno-exceptions -fstack-usage -Wno-unused-variable -Wno-format -Wno-format-security -Wno-format-nonliteral)
         list(APPEND open62541_LIBRARIES c m stdc++ supc++)
     endif(UA_FREERTOS)

+ 116 - 47
include/ua_plugin_securitypolicy.h

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. 
  *
- *    Copyright 2017 (c) Mark Giraud, Fraunhofer IOSB
+ *    Copyright 2017-2018 (c) Mark Giraud, Fraunhofer IOSB
  *    Copyright 2017 (c) Julius Pfrommer, Fraunhofer IOSB
  *    Copyright 2017 (c) Stefan Profanter, fortiss GmbH
  */
@@ -25,7 +25,7 @@ struct UA_SecurityPolicy;
 typedef struct UA_SecurityPolicy UA_SecurityPolicy;
 
 typedef struct {
-    UA_String signatureAlgorithmUri;
+    UA_String uri;
 
     /* Verifies the signature of the message using the provided keys in the context.
      *
@@ -72,7 +72,27 @@ typedef struct {
     size_t (*getRemoteSignatureSize)(const UA_SecurityPolicy *securityPolicy,
                                      const void *channelContext);
 
-    UA_String encryptionAlgorithmUri;
+    /* Gets the local signing key length.
+     *
+     * @param securityPolicy the securityPolicy the function is invoked on.
+     * @param channelContext the context to retrieve data from.
+     * @return the length of the signing key in bytes. Returns 0 if no length can be found.
+     */
+    size_t (*getLocalKeyLength)(const UA_SecurityPolicy *securityPolicy,
+                                const void *channelContext);
+
+    /* Gets the local signing key length.
+     *
+     * @param securityPolicy the securityPolicy the function is invoked on.
+     * @param channelContext the context to retrieve data from.
+     * @return the length of the signing key in bytes. Returns 0 if no length can be found.
+     */
+    size_t (*getRemoteKeyLength)(const UA_SecurityPolicy *securityPolicy,
+                                 const void *channelContext);
+} UA_SecurityPolicySignatureAlgorithm;
+
+typedef struct {
+    UA_String uri;
 
     /* Encrypt the given data in place using an asymmetric algorithm and keys.
      *
@@ -81,9 +101,9 @@ typedef struct {
      *                       the keys to encrypt data.
      * @param data the data that is encrypted. The encrypted data will overwrite
      *             the data that was supplied. */
-    UA_StatusCode(*encrypt)(const UA_SecurityPolicy *securityPolicy,
-                            void *channelContext,
-                            UA_ByteString *data) UA_FUNC_ATTR_WARN_UNUSED_RESULT;
+    UA_StatusCode (*encrypt)(const UA_SecurityPolicy *securityPolicy,
+                             void *channelContext,
+                             UA_ByteString *data) UA_FUNC_ATTR_WARN_UNUSED_RESULT;
 
     /* Decrypts the given ciphertext in place using an asymmetric algorithm and
      * key.
@@ -92,9 +112,9 @@ typedef struct {
      * @param channelContext the channelContext which contains information about
      *                       the keys needed to decrypt the message.
      * @param data the data to decrypt. The decryption is done in place. */
-    UA_StatusCode(*decrypt)(const UA_SecurityPolicy *securityPolicy,
-                            void *channelContext,
-                            UA_ByteString *data) UA_FUNC_ATTR_WARN_UNUSED_RESULT;
+    UA_StatusCode (*decrypt)(const UA_SecurityPolicy *securityPolicy,
+                             void *channelContext,
+                             UA_ByteString *data) UA_FUNC_ATTR_WARN_UNUSED_RESULT;
 
     /* Returns the length of the key used locally to encrypt messages in bits
      *
@@ -102,8 +122,8 @@ typedef struct {
      * @param channelContext the context to retrieve data from.
      * @return the length of the local key. Returns 0 if no
      *         key length is known. */
-    size_t (*getLocalEncryptionKeyLength)(const UA_SecurityPolicy *securityPolicy,
-                                          const void *channelContext);
+    size_t (*getLocalKeyLength)(const UA_SecurityPolicy *securityPolicy,
+                                const void *channelContext);
 
     /* Returns the length of the key used remotely to encrypt messages in bits
      *
@@ -111,8 +131,57 @@ typedef struct {
      * @param channelContext the context to retrieve data from.
      * @return the length of the remote key. Returns 0 if no
      *         key length is known. */
-    size_t (*getRemoteEncryptionKeyLength)(const UA_SecurityPolicy *securityPolicy,
-                                           const void *channelContext);
+    size_t (*getRemoteKeyLength)(const UA_SecurityPolicy *securityPolicy,
+                                 const void *channelContext);
+
+    /* Returns the size of encrypted blocks used by the local encryption algorithm.
+     *
+     * @param securityPolicy the securityPolicy the function is invoked on.
+     * @param channelContext the context to retrieve data from.
+     * @return the size of encrypted blocks in bytes. Returns 0 if no key length is known.
+     */
+    size_t (*getLocalBlockSize)(const UA_SecurityPolicy *securityPolicy,
+                                const void *channelContext);
+
+    /* Returns the size of encrypted blocks used by the remote encryption algorithm.
+     *
+     * @param securityPolicy the securityPolicy the function is invoked on.
+     * @param channelContext the context to retrieve data from.
+     * @return the size of encrypted blocks in bytes. Returns 0 if no key length is known.
+     */
+    size_t (*getRemoteBlockSize)(const UA_SecurityPolicy *securityPolicy,
+                                 const void *channelContext);
+
+    /* Returns the size of plaintext blocks used by the local encryption algorithm.
+     *
+     * @param securityPolicy the securityPolicy the function is invoked on.
+     * @param channelContext the context to retrieve data from.
+     * @return the size of plaintext blocks in bytes. Returns 0 if no key length is known.
+     */
+    size_t (*getLocalPlainTextBlockSize)(const UA_SecurityPolicy *securityPolicy,
+                                         const void *channelContext);
+
+    /* Returns the size of plaintext blocks used by the remote encryption algorithm.
+     *
+     * @param securityPolicy the securityPolicy the function is invoked on.
+     * @param channelContext the context to retrieve data from.
+     * @return the size of plaintext blocks in bytes. Returns 0 if no key length is known.
+     */
+    size_t (*getRemotePlainTextBlockSize)(const UA_SecurityPolicy *securityPolicy,
+                                          const void *channelContext);
+} UA_SecurityPolicyEncryptionAlgorithm;
+
+typedef struct {
+    /*
+     * The algorithm used to sign and verify certificates.
+     */
+    UA_SecurityPolicySignatureAlgorithm signatureAlgorithm;
+
+    /*
+     * The algorithm used to encrypt and decrypt messages.
+     */
+    UA_SecurityPolicyEncryptionAlgorithm encryptionAlgorithm;
+
 } UA_SecurityPolicyCryptoModule;
 
 typedef struct {
@@ -126,7 +195,7 @@ typedef struct {
     UA_StatusCode (*makeCertificateThumbprint)(const UA_SecurityPolicy *securityPolicy,
                                                const UA_ByteString *certificate,
                                                UA_ByteString *thumbprint)
-        UA_FUNC_ATTR_WARN_UNUSED_RESULT;
+    UA_FUNC_ATTR_WARN_UNUSED_RESULT;
 
     /* Compares the supplied certificate with the certificate in the endpoit context.
      *
@@ -138,7 +207,7 @@ typedef struct {
      *         don't match or an error occurred an error code is returned. */
     UA_StatusCode (*compareCertificateThumbprint)(const UA_SecurityPolicy *securityPolicy,
                                                   const UA_ByteString *certificateThumbprint)
-        UA_FUNC_ATTR_WARN_UNUSED_RESULT;
+    UA_FUNC_ATTR_WARN_UNUSED_RESULT;
 
     UA_SecurityPolicyCryptoModule cryptoModule;
 } UA_SecurityPolicyAsymmetricModule;
@@ -157,7 +226,8 @@ typedef struct {
     UA_StatusCode (*generateKey)(const UA_SecurityPolicy *securityPolicy,
                                  const UA_ByteString *secret,
                                  const UA_ByteString *seed, UA_ByteString *out)
-        UA_FUNC_ATTR_WARN_UNUSED_RESULT;
+    UA_FUNC_ATTR_WARN_UNUSED_RESULT;
+
     /* Random generator for generating nonces.
      *
      * @param securityPolicy the securityPolicy this function is invoked on.
@@ -168,11 +238,14 @@ typedef struct {
      *            data. */
     UA_StatusCode (*generateNonce)(const UA_SecurityPolicy *securityPolicy,
                                    UA_ByteString *out)
-        UA_FUNC_ATTR_WARN_UNUSED_RESULT;
+    UA_FUNC_ATTR_WARN_UNUSED_RESULT;
+
+    /*
+     * The length of the nonce used in the SecureChannel as specified in the standard.
+     */
+    size_t secureChannelNonceLength;
 
     UA_SecurityPolicyCryptoModule cryptoModule;
-    size_t encryptionBlockSize;
-    size_t signingKeyLength;
 } UA_SecurityPolicySymmetricModule;
 
 typedef struct {
@@ -195,7 +268,7 @@ typedef struct {
     UA_StatusCode (*newContext)(const UA_SecurityPolicy *securityPolicy,
                                 const UA_ByteString *remoteCertificate,
                                 void **channelContext)
-        UA_FUNC_ATTR_WARN_UNUSED_RESULT;
+    UA_FUNC_ATTR_WARN_UNUSED_RESULT;
 
     /* Deletes the the security context. */
     void (*deleteContext)(void *channelContext);
@@ -206,7 +279,7 @@ typedef struct {
      * @param key the local encrypting key to store in the context. */
     UA_StatusCode (*setLocalSymEncryptingKey)(void *channelContext,
                                               const UA_ByteString *key)
-        UA_FUNC_ATTR_WARN_UNUSED_RESULT;
+    UA_FUNC_ATTR_WARN_UNUSED_RESULT;
 
     /* Sets the local signing key in the supplied context.
      *
@@ -214,7 +287,7 @@ typedef struct {
      * @param key the local signing key to store in the context. */
     UA_StatusCode (*setLocalSymSigningKey)(void *channelContext,
                                            const UA_ByteString *key)
-        UA_FUNC_ATTR_WARN_UNUSED_RESULT;
+    UA_FUNC_ATTR_WARN_UNUSED_RESULT;
 
     /* Sets the local initialization vector in the supplied context.
      *
@@ -222,7 +295,7 @@ typedef struct {
      * @param iv the local initialization vector to store in the context. */
     UA_StatusCode (*setLocalSymIv)(void *channelContext,
                                    const UA_ByteString *iv)
-        UA_FUNC_ATTR_WARN_UNUSED_RESULT;
+    UA_FUNC_ATTR_WARN_UNUSED_RESULT;
 
     /* Sets the remote encrypting key in the supplied context.
      *
@@ -230,7 +303,7 @@ typedef struct {
      * @param key the remote encrypting key to store in the context. */
     UA_StatusCode (*setRemoteSymEncryptingKey)(void *channelContext,
                                                const UA_ByteString *key)
-        UA_FUNC_ATTR_WARN_UNUSED_RESULT;
+    UA_FUNC_ATTR_WARN_UNUSED_RESULT;
 
     /* Sets the remote signing key in the supplied context.
      *
@@ -238,7 +311,7 @@ typedef struct {
      * @param key the remote signing key to store in the context. */
     UA_StatusCode (*setRemoteSymSigningKey)(void *channelContext,
                                             const UA_ByteString *key)
-        UA_FUNC_ATTR_WARN_UNUSED_RESULT;
+    UA_FUNC_ATTR_WARN_UNUSED_RESULT;
 
     /* Sets the remote initialization vector in the supplied context.
      *
@@ -246,7 +319,7 @@ typedef struct {
      * @param iv the remote initialization vector to store in the context. */
     UA_StatusCode (*setRemoteSymIv)(void *channelContext,
                                     const UA_ByteString *iv)
-        UA_FUNC_ATTR_WARN_UNUSED_RESULT;
+    UA_FUNC_ATTR_WARN_UNUSED_RESULT;
 
     /* Compares the supplied certificate with the certificate in the channel
      * context.
@@ -258,27 +331,7 @@ typedef struct {
      *         don't match or an errror occurred an error code is returned. */
     UA_StatusCode (*compareCertificate)(const void *channelContext,
                                         const UA_ByteString *certificate)
-        UA_FUNC_ATTR_WARN_UNUSED_RESULT;
-
-    /* Gets the plaintext block size that depends on the remote public key.
-     *
-     * @param channelContext the context to retrieve data from.
-     * @return the size of the plain text block size when encrypting with the
-     *         remote public key. Returns 0 as long as no remote certificate was
-     *         set previousely. */
-    size_t (*getRemoteAsymPlainTextBlockSize)(const void *channelContext);
-
-    /* Gets the number of bytes that are needed by the encryption function in
-     * addition to the length of the plaintext message. This is needed, since
-     * most RSA encryption methods have their own padding mechanism included.
-     * This makes the encrypted message larger than the plainText, so we need to
-     * have enough room in the buffer for the overhead.
-     *
-     * @param channelContext the retrieve data from.
-     * @param maxEncryptionLength the maximum number of bytes that the data to
-     *                            encrypt can be. */
-    size_t (*getRemoteAsymEncryptionBufferLengthOverhead)(const void *channelContext,
-                                                          size_t maxEncryptionLength);
+    UA_FUNC_ATTR_WARN_UNUSED_RESULT;
 } UA_SecurityPolicyChannelModule;
 
 struct UA_SecurityPolicy {
@@ -295,6 +348,7 @@ struct UA_SecurityPolicy {
     /* Function pointers grouped into modules */
     UA_SecurityPolicyAsymmetricModule asymmetricModule;
     UA_SecurityPolicySymmetricModule symmetricModule;
+    UA_SecurityPolicySignatureAlgorithm certificateSigningAlgorithm;
     UA_SecurityPolicyChannelModule channelModule;
     UA_CertificateVerification *certificateVerification;
 
@@ -309,6 +363,21 @@ typedef struct {
     UA_EndpointDescription endpointDescription;
 } UA_Endpoint;
 
+/* Gets the number of bytes that are needed by the encryption function in
+ * addition to the length of the plaintext message. This is needed, since
+ * most RSA encryption methods have their own padding mechanism included.
+ * This makes the encrypted message larger than the plainText, so we need to
+ * have enough room in the buffer for the overhead.
+ *
+ * @param securityPolicy the algorithms to use.
+ * @param channelContext the retrieve data from.
+ * @param maxEncryptionLength the maximum number of bytes that the data to
+ *                            encrypt can be. */
+size_t
+UA_SecurityPolicy_getRemoteAsymEncryptionBufferLengthOverhead(const UA_SecurityPolicy *securityPolicy,
+                                                              const void *channelContext,
+                                                              size_t maxEncryptionLength);
+
 #ifdef __cplusplus
 }
 #endif

+ 123 - 57
plugins/ua_securitypolicy_basic128rsa15.c

@@ -13,8 +13,9 @@
 #include <mbedtls/entropy.h>
 #include <mbedtls/entropy_poll.h>
 #include <mbedtls/error.h>
-#include <ua_plugin_pki.h>
 
+#include "ua_plugin_pki.h"
+#include "ua_plugin_securitypolicy.h"
 #include "ua_securitypolicy_basic128rsa15.h"
 #include "ua_types.h"
 #include "ua_types_generated_handling.h"
@@ -28,6 +29,9 @@
 #define UA_SECURITYPOLICY_BASIC128RSA15_RSAPADDING_LEN 11
 #define UA_SHA1_LENGTH 20
 #define UA_SECURITYPOLICY_BASIC128RSA15_SYM_KEY_LENGTH 16
+#define UA_BASIC128RSA15_SYM_SIGNING_KEY_LENGTH 16
+#define UA_SECURITYPOLICY_BASIC128RSA15_SYM_ENCRYPTION_BLOCK_SIZE 16
+#define UA_SECURITYPOLICY_BASIC128RSA15_SYM_PLAIN_TEXT_BLOCK_SIZE 16
 #define UA_SECURITYPOLICY_BASIC128RSA15_MINASYMKEYLENGTH 128
 #define UA_SECURITYPOLICY_BASIC128RSA15_MAXASYMKEYLENGTH 256
 
@@ -73,6 +77,16 @@ typedef struct {
     mbedtls_x509_crt remoteCertificate;
 } Basic128Rsa15_ChannelContext;
 
+static void
+sha1(const unsigned char *input, size_t ilen, unsigned char output[20] ) {
+    mbedtls_sha1_context sha1Context;
+    mbedtls_sha1_init(&sha1Context);
+    mbedtls_sha1_starts(&sha1Context);
+    mbedtls_sha1_update(&sha1Context, input, ilen);
+    mbedtls_sha1_finish(&sha1Context, output);
+    mbedtls_sha1_free(&sha1Context);
+}
+
 /********************/
 /* AsymmetricModule */
 /********************/
@@ -87,7 +101,7 @@ asym_verify_sp_basic128rsa15(const UA_SecurityPolicy *securityPolicy,
 
     /* Compute the sha1 hash */
     unsigned char hash[UA_SHA1_LENGTH];
-    mbedtls_sha1(message->data, message->length, hash);
+    sha1(message->data, message->length, hash);
 
     /* Set the RSA settings */
     mbedtls_rsa_context *rsaContext = mbedtls_pk_rsa(cc->remoteCertificate.pk);
@@ -110,7 +124,7 @@ asym_sign_sp_basic128rsa15(const UA_SecurityPolicy *securityPolicy,
         return UA_STATUSCODE_BADINTERNALERROR;
 
     unsigned char hash[UA_SHA1_LENGTH];
-    mbedtls_sha1(message->data, message->length, hash);
+    sha1(message->data, message->length, hash);
 
     Basic128Rsa15_PolicyContext *pc = cc->policyContext;
     mbedtls_rsa_context *rsaContext = mbedtls_pk_rsa(pc->localPrivateKey);
@@ -151,8 +165,8 @@ asym_encrypt_sp_basic128rsa15(const UA_SecurityPolicy *securityPolicy,
     if(securityPolicy == NULL || cc == NULL || data == NULL)
         return UA_STATUSCODE_BADINTERNALERROR;
 
-    const size_t plainTextBlockSize = securityPolicy->channelModule.
-        getRemoteAsymPlainTextBlockSize(cc);
+    const size_t plainTextBlockSize = securityPolicy->asymmetricModule.cryptoModule.encryptionAlgorithm.
+        getRemotePlainTextBlockSize(securityPolicy, cc);
 
     if(data->length % plainTextBlockSize != 0)
         return UA_STATUSCODE_BADINTERNALERROR;
@@ -161,8 +175,8 @@ asym_encrypt_sp_basic128rsa15(const UA_SecurityPolicy *securityPolicy,
     mbedtls_rsa_set_padding(remoteRsaContext, MBEDTLS_RSA_PKCS_V15, 0);
 
     UA_ByteString encrypted;
-    const size_t bufferOverhead = securityPolicy->channelModule.
-        getRemoteAsymEncryptionBufferLengthOverhead(cc, data->length);
+    const size_t bufferOverhead =
+        UA_SecurityPolicy_getRemoteAsymEncryptionBufferLengthOverhead(securityPolicy, cc, data->length);
     UA_StatusCode retval = UA_ByteString_allocBuffer(&encrypted, data->length + bufferOverhead);
     if(retval != UA_STATUSCODE_GOOD)
         return retval;
@@ -249,6 +263,20 @@ asym_getRemoteEncryptionKeyLength(const UA_SecurityPolicy *securityPolicy,
     return mbedtls_pk_get_len(&cc->remoteCertificate.pk) * 8;
 }
 
+static size_t
+asym_getRemoteBlockSize_sp_basic128rsa15(const UA_SecurityPolicy *securityPolicy,
+                                         const Basic128Rsa15_ChannelContext *cc) {
+    mbedtls_rsa_context *const rsaContext = mbedtls_pk_rsa(cc->remoteCertificate.pk);
+    return rsaContext->len;
+}
+
+static size_t
+asym_getRemotePlainTextBlockSize(const UA_SecurityPolicy *securityPolicy,
+                                 const Basic128Rsa15_ChannelContext *cc) {
+    mbedtls_rsa_context *const rsaContext = mbedtls_pk_rsa(cc->remoteCertificate.pk);
+    return rsaContext->len - UA_SECURITYPOLICY_BASIC128RSA15_RSAPADDING_LEN;
+}
+
 static UA_StatusCode
 asym_makeThumbprint_sp_basic128rsa15(const UA_SecurityPolicy *securityPolicy,
                                      const UA_ByteString *certificate,
@@ -262,7 +290,7 @@ asym_makeThumbprint_sp_basic128rsa15(const UA_SecurityPolicy *securityPolicy,
     if(thumbprint->length != UA_SHA1_LENGTH)
         return UA_STATUSCODE_BADINTERNALERROR;
 
-    mbedtls_sha1(certificate->data, certificate->length, thumbprint->data);
+    sha1(certificate->data, certificate->length, thumbprint->data);
     return UA_STATUSCODE_GOOD;
 }
 
@@ -337,12 +365,30 @@ sym_getSignatureSize_sp_basic128rsa15(const UA_SecurityPolicy *securityPolicy,
     return UA_SHA1_LENGTH;
 }
 
+static size_t
+sym_getSigningKeyLength_sp_basic128rsa15(const UA_SecurityPolicy *const securityPolicy,
+                                         const void *const channelContext) {
+    return UA_BASIC128RSA15_SYM_SIGNING_KEY_LENGTH;
+}
+
 static size_t
 sym_getEncryptionKeyLength_sp_basic128rsa15(const UA_SecurityPolicy *securityPolicy,
                                             const void *channelContext) {
     return UA_SECURITYPOLICY_BASIC128RSA15_SYM_KEY_LENGTH;
 }
 
+static size_t
+sym_getEncryptionBlockSize_sp_basic128rsa15(const UA_SecurityPolicy *const securityPolicy,
+                                            const void *const channelContext) {
+    return UA_SECURITYPOLICY_BASIC128RSA15_SYM_ENCRYPTION_BLOCK_SIZE;
+}
+
+static size_t
+sym_getPlainTextBlockSize_sp_basic128rsa15(const UA_SecurityPolicy *const securityPolicy,
+                                           const void *const channelContext) {
+    return UA_SECURITYPOLICY_BASIC128RSA15_SYM_PLAIN_TEXT_BLOCK_SIZE;
+}
+
 static UA_StatusCode
 sym_encrypt_sp_basic128rsa15(const UA_SecurityPolicy *securityPolicy,
                              const Basic128Rsa15_ChannelContext *cc,
@@ -350,13 +396,17 @@ sym_encrypt_sp_basic128rsa15(const UA_SecurityPolicy *securityPolicy,
     if(securityPolicy == NULL || cc == NULL || data == NULL)
         return UA_STATUSCODE_BADINTERNALERROR;
 
-    if(cc->localSymIv.length != securityPolicy->symmetricModule.encryptionBlockSize)
+    if(cc->localSymIv.length !=
+       securityPolicy->symmetricModule.cryptoModule.encryptionAlgorithm.getLocalBlockSize(securityPolicy, cc))
         return UA_STATUSCODE_BADINTERNALERROR;
 
-    if(data->length % cc->localSymEncryptingKey.length != 0) {
+    size_t plainTextBlockSize =
+        securityPolicy->symmetricModule.cryptoModule.encryptionAlgorithm.getLocalPlainTextBlockSize(securityPolicy, cc);
+
+    if(data->length % plainTextBlockSize != 0) {
         UA_LOG_ERROR(securityPolicy->logger, UA_LOGCATEGORY_SECURITYPOLICY,
-                     "Length of data to encrypt is not a multiple of the encryptingKey length."
-                         "Padding might not have been calculated appropriatley.");
+                     "Length of data to encrypt is not a multiple of the plain text block size."
+                         "Padding might not have been calculated appropriately.");
         return UA_STATUSCODE_BADINTERNALERROR;
     }
 
@@ -385,10 +435,13 @@ sym_decrypt_sp_basic128rsa15(const UA_SecurityPolicy *securityPolicy,
     if(securityPolicy == NULL || cc == NULL || data == NULL)
         return UA_STATUSCODE_BADINTERNALERROR;
 
-    if(cc->remoteSymIv.length != securityPolicy->symmetricModule.encryptionBlockSize)
+    size_t encryptionBlockSize =
+        securityPolicy->symmetricModule.cryptoModule.encryptionAlgorithm.getLocalBlockSize(securityPolicy, cc);
+
+    if(cc->remoteSymIv.length != encryptionBlockSize)
         return UA_STATUSCODE_BADINTERNALERROR;
 
-    if(data->length % securityPolicy->symmetricModule.encryptionBlockSize != 0) {
+    if(data->length % encryptionBlockSize != 0) {
         UA_LOG_ERROR(securityPolicy->logger, UA_LOGCATEGORY_SECURITYPOLICY,
                      "Length of data to decrypt is not a multiple of the encryptingBlock size.");
         return UA_STATUSCODE_BADINTERNALERROR;
@@ -672,20 +725,6 @@ channelContext_compareCertificate_sp_basic128rsa15(const Basic128Rsa15_ChannelCo
     return UA_STATUSCODE_GOOD;
 }
 
-static size_t
-channelContext_getRemoteAsymPlainTextBlockSize_sp_basic128rsa15(const Basic128Rsa15_ChannelContext *cc) {
-    mbedtls_rsa_context *const rsaContext = mbedtls_pk_rsa(cc->remoteCertificate.pk);
-    return rsaContext->len - UA_SECURITYPOLICY_BASIC128RSA15_RSAPADDING_LEN;
-}
-
-static size_t
-channelContext_getRemoteAsymEncryptionBufferLengthOverhead_sp_basic128rsa15(const Basic128Rsa15_ChannelContext *cc,
-                                                                            size_t maxEncryptionLength) {
-    const size_t maxNumberOfBlocks = maxEncryptionLength /
-                                     channelContext_getRemoteAsymPlainTextBlockSize_sp_basic128rsa15(cc);
-    return maxNumberOfBlocks * UA_SECURITYPOLICY_BASIC128RSA15_RSAPADDING_LEN;
-}
-
 static void
 deleteMembers_sp_basic128rsa15(UA_SecurityPolicy *securityPolicy) {
     if(securityPolicy == NULL)
@@ -812,58 +851,89 @@ UA_SecurityPolicy_Basic128Rsa15(UA_SecurityPolicy *policy, UA_CertificateVerific
     policy->certificateVerification = certificateVerification;
 
     /* AsymmetricModule */
-    asymmetricModule->cryptoModule.signatureAlgorithmUri =
+    UA_SecurityPolicySignatureAlgorithm *asym_signatureAlgorithm =
+        &asymmetricModule->cryptoModule.signatureAlgorithm;
+    asym_signatureAlgorithm->uri =
         UA_STRING("http://www.w3.org/2000/09/xmldsig#rsa-sha1\0");
-    asymmetricModule->cryptoModule.verify =
+    asym_signatureAlgorithm->verify =
         (UA_StatusCode (*)(const UA_SecurityPolicy *, void *,
                            const UA_ByteString *, const UA_ByteString *))asym_verify_sp_basic128rsa15;
-    asymmetricModule->cryptoModule.sign =
+    asym_signatureAlgorithm->sign =
         (UA_StatusCode (*)(const UA_SecurityPolicy *, void *,
                            const UA_ByteString *, UA_ByteString *))asym_sign_sp_basic128rsa15;
-    asymmetricModule->cryptoModule.getLocalSignatureSize =
+    asym_signatureAlgorithm->getLocalSignatureSize =
         (size_t (*)(const UA_SecurityPolicy *, const void *))asym_getLocalSignatureSize_sp_basic128rsa15;
-    asymmetricModule->cryptoModule.getRemoteSignatureSize =
+    asym_signatureAlgorithm->getRemoteSignatureSize =
         (size_t (*)(const UA_SecurityPolicy *, const void *))asym_getRemoteSignatureSize_sp_basic128rsa15;
+    asym_signatureAlgorithm->getLocalKeyLength = NULL; // TODO: Write function
+    asym_signatureAlgorithm->getRemoteKeyLength = NULL; // TODO: Write function
 
-    asymmetricModule->cryptoModule.encryptionAlgorithmUri = UA_STRING("TODO: ALG URI");
-    asymmetricModule->cryptoModule.encrypt =
+    UA_SecurityPolicyEncryptionAlgorithm *asym_encryptionAlgorithm =
+        &asymmetricModule->cryptoModule.encryptionAlgorithm;
+    asym_encryptionAlgorithm->uri = UA_STRING("TODO: ALG URI");
+    asym_encryptionAlgorithm->encrypt =
         (UA_StatusCode(*)(const UA_SecurityPolicy *, void *, UA_ByteString *))asym_encrypt_sp_basic128rsa15;
-    asymmetricModule->cryptoModule.decrypt =
+    asym_encryptionAlgorithm->decrypt =
         (UA_StatusCode(*)(const UA_SecurityPolicy *, void *, UA_ByteString *))
             asym_decrypt_sp_basic128rsa15;
-    asymmetricModule->cryptoModule.getLocalEncryptionKeyLength = NULL; // TODO: Write function
-    asymmetricModule->cryptoModule.getRemoteEncryptionKeyLength =
+    asym_encryptionAlgorithm->getLocalKeyLength = NULL; // TODO: Write function
+    asym_encryptionAlgorithm->getRemoteKeyLength =
         (size_t (*)(const UA_SecurityPolicy *, const void *))asym_getRemoteEncryptionKeyLength;
+    asym_encryptionAlgorithm->getLocalBlockSize = NULL; // TODO: Write function
+    asym_encryptionAlgorithm->getRemoteBlockSize = (size_t (*)(const UA_SecurityPolicy *,
+                                                               const void *))asym_getRemoteBlockSize_sp_basic128rsa15;
+    asym_encryptionAlgorithm->getLocalPlainTextBlockSize = NULL; // TODO: Write function
+    asym_encryptionAlgorithm->getRemotePlainTextBlockSize =
+        (size_t (*)(const UA_SecurityPolicy *, const void *))asym_getRemotePlainTextBlockSize;
 
     asymmetricModule->makeCertificateThumbprint = asym_makeThumbprint_sp_basic128rsa15;
     asymmetricModule->compareCertificateThumbprint =
         asymmetricModule_compareCertificateThumbprint_sp_basic128rsa15;
 
     /* SymmetricModule */
-    symmetricModule->encryptionBlockSize = 16;
-    symmetricModule->signingKeyLength = 16;
     symmetricModule->generateKey = sym_generateKey_sp_basic128rsa15;
     symmetricModule->generateNonce = sym_generateNonce_sp_basic128rsa15;
 
-    symmetricModule->cryptoModule.signatureAlgorithmUri =
+    UA_SecurityPolicySignatureAlgorithm *sym_signatureAlgorithm =
+        &symmetricModule->cryptoModule.signatureAlgorithm;
+    sym_signatureAlgorithm->uri =
         UA_STRING("http://www.w3.org/2000/09/xmldsig#hmac-sha1\0");
-    symmetricModule->cryptoModule.verify =
+    sym_signatureAlgorithm->verify =
         (UA_StatusCode (*)(const UA_SecurityPolicy *, void *, const UA_ByteString *,
                            const UA_ByteString *))sym_verify_sp_basic128rsa15;
-    symmetricModule->cryptoModule.sign =
+    sym_signatureAlgorithm->sign =
         (UA_StatusCode (*)(const UA_SecurityPolicy *, void *,
                            const UA_ByteString *, UA_ByteString *))sym_sign_sp_basic128rsa15;
-    symmetricModule->cryptoModule.getLocalSignatureSize = sym_getSignatureSize_sp_basic128rsa15;
-    symmetricModule->cryptoModule.getRemoteSignatureSize = sym_getSignatureSize_sp_basic128rsa15;
-
-    symmetricModule->cryptoModule.encryptionAlgorithmUri = UA_STRING("TODO: ALG URI");
-    symmetricModule->cryptoModule.encrypt =
+    sym_signatureAlgorithm->getLocalSignatureSize = sym_getSignatureSize_sp_basic128rsa15;
+    sym_signatureAlgorithm->getRemoteSignatureSize = sym_getSignatureSize_sp_basic128rsa15;
+    sym_signatureAlgorithm->getLocalKeyLength =
+        (size_t (*)(const UA_SecurityPolicy *,
+                    const void *))sym_getSigningKeyLength_sp_basic128rsa15;
+    sym_signatureAlgorithm->getRemoteKeyLength =
+        (size_t (*)(const UA_SecurityPolicy *,
+                    const void *))sym_getSigningKeyLength_sp_basic128rsa15;
+
+    UA_SecurityPolicyEncryptionAlgorithm *sym_encryptionAlgorithm =
+        &symmetricModule->cryptoModule.encryptionAlgorithm;
+    sym_encryptionAlgorithm->uri = UA_STRING("http://www.w3.org/2001/04/xmlenc#aes128-cbc");
+    sym_encryptionAlgorithm->encrypt =
         (UA_StatusCode(*)(const UA_SecurityPolicy *, void *, UA_ByteString *))sym_encrypt_sp_basic128rsa15;
-    symmetricModule->cryptoModule.decrypt =
-        (UA_StatusCode(*)(const UA_SecurityPolicy *, void *, UA_ByteString *))
-            sym_decrypt_sp_basic128rsa15;
-    symmetricModule->cryptoModule.getLocalEncryptionKeyLength = sym_getEncryptionKeyLength_sp_basic128rsa15;
-    symmetricModule->cryptoModule.getRemoteEncryptionKeyLength = sym_getEncryptionKeyLength_sp_basic128rsa15;
+    sym_encryptionAlgorithm->decrypt =
+        (UA_StatusCode(*)(const UA_SecurityPolicy *, void *, UA_ByteString *))sym_decrypt_sp_basic128rsa15;
+    sym_encryptionAlgorithm->getLocalKeyLength = sym_getEncryptionKeyLength_sp_basic128rsa15;
+    sym_encryptionAlgorithm->getRemoteKeyLength = sym_getEncryptionKeyLength_sp_basic128rsa15;
+    sym_encryptionAlgorithm->getLocalBlockSize =
+        (size_t (*)(const UA_SecurityPolicy *, const void *))sym_getEncryptionBlockSize_sp_basic128rsa15;
+    sym_encryptionAlgorithm->getRemoteBlockSize =
+        (size_t (*)(const UA_SecurityPolicy *, const void *))sym_getEncryptionBlockSize_sp_basic128rsa15;
+    sym_encryptionAlgorithm->getLocalPlainTextBlockSize =
+        (size_t (*)(const UA_SecurityPolicy *, const void *))sym_getPlainTextBlockSize_sp_basic128rsa15;
+    sym_encryptionAlgorithm->getRemotePlainTextBlockSize =
+        (size_t (*)(const UA_SecurityPolicy *, const void *))sym_getPlainTextBlockSize_sp_basic128rsa15;
+    symmetricModule->secureChannelNonceLength = 16;
+
+    // Use the same signature algorithm as the asymmetric component for certificate signing (see standard)
+    policy->certificateSigningAlgorithm = policy->asymmetricModule.cryptoModule.signatureAlgorithm;
 
     /* ChannelModule */
     channelModule->newContext = channelContext_newContext_sp_basic128rsa15;
@@ -886,10 +956,6 @@ UA_SecurityPolicy_Basic128Rsa15(UA_SecurityPolicy *policy, UA_CertificateVerific
 
     channelModule->compareCertificate = (UA_StatusCode (*)(const void *, const UA_ByteString *))
         channelContext_compareCertificate_sp_basic128rsa15;
-    channelModule->getRemoteAsymPlainTextBlockSize = (size_t (*)(const void *))
-        channelContext_getRemoteAsymPlainTextBlockSize_sp_basic128rsa15;
-    channelModule->getRemoteAsymEncryptionBufferLengthOverhead = (size_t (*)(const void *, size_t))
-        channelContext_getRemoteAsymEncryptionBufferLengthOverhead_sp_basic128rsa15;
 
     policy->deleteMembers = deleteMembers_sp_basic128rsa15;
 

+ 45 - 45
plugins/ua_securitypolicy_none.c

@@ -1,7 +1,7 @@
 /* This work is licensed under a Creative Commons CCZero 1.0 Universal License.
  * See http://creativecommons.org/publicdomain/zero/1.0/ for more information. 
  *
- *    Copyright 2017 (c) Mark Giraud, Fraunhofer IOSB
+ *    Copyright 2017-2018 (c) Mark Giraud, Fraunhofer IOSB
  *    Copyright 2017 (c) Stefan Profanter, fortiss GmbH
  */
 
@@ -66,18 +66,24 @@ generateKey_none(const UA_SecurityPolicy *securityPolicy,
     return UA_STATUSCODE_GOOD;
 }
 
+/* Use the non-cryptographic RNG to set the nonce */
 static UA_StatusCode
-generateNonce_none(const UA_SecurityPolicy *securityPolicy,
-                   UA_ByteString *out) {
+generateNonce_none(const UA_SecurityPolicy *securityPolicy, UA_ByteString *out) {
     if(securityPolicy == NULL || out == NULL)
         return UA_STATUSCODE_BADINTERNALERROR;
-    if(out->length != 0)
-        return UA_STATUSCODE_BADINTERNALERROR;
 
-    if(out->data != NULL)
-        UA_ByteString_deleteMembers(out);
+    /* Fill blocks of four byte */
+    size_t i = 0;
+    while(i + 3 < out->length) {
+        UA_UInt32 rand = UA_UInt32_random();
+        memcpy(&out->data[i], &rand, 4);
+        i = i+4;
+    }
+
+    /* Fill the remaining byte */
+    UA_UInt32 rand = UA_UInt32_random();
+    memcpy(&out->data[i], &rand, out->length % 4);
 
-    out->data = (UA_Byte *) UA_EMPTY_ARRAY_SENTINEL;
     return UA_STATUSCODE_GOOD;
 }
 
@@ -98,17 +104,6 @@ setContextValue_none(void *channelContext,
     return UA_STATUSCODE_GOOD;
 }
 
-static size_t
-getRemoteAsymPlainTextBlockSize_none(const void *channelContext) {
-    return 0;
-}
-
-static size_t
-getRemoteAsymEncryptionBufferLengthOverhead_none(const void *channelContext,
-                                                 size_t maxEncryptionLength) {
-    return 0;
-}
-
 static UA_StatusCode
 compareCertificate_none(const void *channelContext,
                         const UA_ByteString *certificate) {
@@ -123,38 +118,46 @@ policy_deletemembers_none(UA_SecurityPolicy *policy) {
 UA_StatusCode
 UA_SecurityPolicy_None(UA_SecurityPolicy *policy, UA_CertificateVerification *certificateVerification,
                        const UA_ByteString localCertificate, UA_Logger logger) {
-    policy->policyContext = (void *) (uintptr_t) logger;
+    policy->policyContext = (void *)(uintptr_t)logger;
     policy->policyUri = UA_STRING("http://opcfoundation.org/UA/SecurityPolicy#None");
     policy->logger = logger;
     UA_ByteString_copy(&localCertificate, &policy->localCertificate);
 
     policy->certificateVerification = certificateVerification;
 
+    policy->symmetricModule.generateKey = generateKey_none;
+    policy->symmetricModule.generateNonce = generateNonce_none;
+
+    UA_SecurityPolicySignatureAlgorithm *sym_signatureAlgorithm =
+        &policy->symmetricModule.cryptoModule.signatureAlgorithm;
+    sym_signatureAlgorithm->uri = UA_STRING_NULL;
+    sym_signatureAlgorithm->verify = verify_none;
+    sym_signatureAlgorithm->sign = sign_none;
+    sym_signatureAlgorithm->getLocalSignatureSize = length_none;
+    sym_signatureAlgorithm->getRemoteSignatureSize = length_none;
+    sym_signatureAlgorithm->getLocalKeyLength = length_none;
+    sym_signatureAlgorithm->getRemoteKeyLength = length_none;
+
+    UA_SecurityPolicyEncryptionAlgorithm *sym_encryptionAlgorithm =
+        &policy->symmetricModule.cryptoModule.encryptionAlgorithm;
+    sym_encryptionAlgorithm->encrypt = encrypt_none;
+    sym_encryptionAlgorithm->decrypt = decrypt_none;
+    sym_encryptionAlgorithm->getLocalKeyLength = length_none;
+    sym_encryptionAlgorithm->getRemoteKeyLength = length_none;
+    sym_encryptionAlgorithm->getLocalBlockSize = length_none;
+    sym_encryptionAlgorithm->getRemoteBlockSize = length_none;
+    sym_encryptionAlgorithm->getLocalPlainTextBlockSize = length_none;
+    sym_encryptionAlgorithm->getRemotePlainTextBlockSize = length_none;
+    policy->symmetricModule.secureChannelNonceLength = 0;
+
     policy->asymmetricModule.makeCertificateThumbprint = makeThumbprint_none;
     policy->asymmetricModule.compareCertificateThumbprint = compareThumbprint_none;
-    policy->asymmetricModule.cryptoModule.signatureAlgorithmUri = UA_STRING_NULL;
-    policy->asymmetricModule.cryptoModule.verify = verify_none;
-    policy->asymmetricModule.cryptoModule.sign = sign_none;
-    policy->asymmetricModule.cryptoModule.getLocalSignatureSize = length_none;
-    policy->asymmetricModule.cryptoModule.getRemoteSignatureSize = length_none;
-    policy->asymmetricModule.cryptoModule.encrypt = encrypt_none;
-    policy->asymmetricModule.cryptoModule.decrypt = decrypt_none;
-    policy->asymmetricModule.cryptoModule.getLocalEncryptionKeyLength = length_none;
-    policy->asymmetricModule.cryptoModule.getRemoteEncryptionKeyLength = length_none;
 
-    policy->symmetricModule.generateKey = generateKey_none;
-    policy->symmetricModule.generateNonce = generateNonce_none;
-    policy->symmetricModule.cryptoModule.signatureAlgorithmUri = UA_STRING_NULL;
-    policy->symmetricModule.cryptoModule.verify = verify_none;
-    policy->symmetricModule.cryptoModule.sign = sign_none;
-    policy->symmetricModule.cryptoModule.getLocalSignatureSize = length_none;
-    policy->symmetricModule.cryptoModule.getRemoteSignatureSize = length_none;
-    policy->symmetricModule.cryptoModule.encrypt = encrypt_none;
-    policy->symmetricModule.cryptoModule.decrypt = decrypt_none;
-    policy->symmetricModule.cryptoModule.getLocalEncryptionKeyLength = length_none;
-    policy->symmetricModule.cryptoModule.getRemoteEncryptionKeyLength = length_none;
-    policy->symmetricModule.encryptionBlockSize = 0;
-    policy->symmetricModule.signingKeyLength = 0;
+    // This only works for none since symmetric and asymmetric crypto modules do the same i.e. nothing
+    policy->asymmetricModule.cryptoModule = policy->symmetricModule.cryptoModule;
+
+    // Use the same signing algorithm as for asymmetric signing
+    policy->certificateSigningAlgorithm = policy->asymmetricModule.cryptoModule.signatureAlgorithm;
 
     policy->channelModule.newContext = newContext_none;
     policy->channelModule.deleteContext = deleteContext_none;
@@ -165,9 +168,6 @@ UA_SecurityPolicy_None(UA_SecurityPolicy *policy, UA_CertificateVerification *ce
     policy->channelModule.setRemoteSymSigningKey = setContextValue_none;
     policy->channelModule.setRemoteSymIv = setContextValue_none;
     policy->channelModule.compareCertificate = compareCertificate_none;
-    policy->channelModule.getRemoteAsymPlainTextBlockSize = getRemoteAsymPlainTextBlockSize_none;
-    policy->channelModule.getRemoteAsymEncryptionBufferLengthOverhead =
-        getRemoteAsymEncryptionBufferLengthOverhead_none;
     policy->deleteMembers = policy_deletemembers_none;
 
     return UA_STATUSCODE_GOOD;

+ 46 - 41
src/server/ua_securechannel_manager.c

@@ -19,7 +19,7 @@
 #define STARTTOKENID 1
 
 UA_StatusCode
-UA_SecureChannelManager_init(UA_SecureChannelManager* cm, UA_Server* server) {
+UA_SecureChannelManager_init(UA_SecureChannelManager *cm, UA_Server *server) {
     LIST_INIT(&cm->channels);
     // TODO: use an ID that is likely to be unique after a restart
     cm->lastChannelId = STARTCHANNELID;
@@ -29,7 +29,8 @@ UA_SecureChannelManager_init(UA_SecureChannelManager* cm, UA_Server* server) {
     return UA_STATUSCODE_GOOD;
 }
 
-void UA_SecureChannelManager_deleteMembers(UA_SecureChannelManager* cm) {
+void
+UA_SecureChannelManager_deleteMembers(UA_SecureChannelManager *cm) {
     channel_list_entry *entry, *temp;
     LIST_FOREACH_SAFE(entry, &cm->channels, pointers, temp) {
         LIST_REMOVE(entry, pointers);
@@ -40,7 +41,7 @@ void UA_SecureChannelManager_deleteMembers(UA_SecureChannelManager* cm) {
 
 static void
 removeSecureChannelCallback(UA_Server *server, void *entry) {
-    channel_list_entry *centry = (channel_list_entry*)entry;
+    channel_list_entry *centry = (channel_list_entry *)entry;
     UA_SecureChannel_deleteMembersCleanup(&centry->channel);
     UA_free(entry);
 }
@@ -69,7 +70,7 @@ UA_SecureChannelManager_cleanupTimedOut(UA_SecureChannelManager *cm, UA_DateTime
     channel_list_entry *entry, *temp;
     LIST_FOREACH_SAFE(entry, &cm->channels, pointers, temp) {
         UA_DateTime timeout = entry->channel.securityToken.createdAt +
-            (UA_DateTime)(entry->channel.securityToken.revisedLifetime * UA_DATETIME_MSEC);
+                              (UA_DateTime)(entry->channel.securityToken.revisedLifetime * UA_DATETIME_MSEC);
         if(timeout < nowMonotonic || !entry->channel.connection) {
             UA_LOG_INFO_CHANNEL(cm->server->config.logger, &entry->channel,
                                 "SecureChannel has timed out");
@@ -81,8 +82,9 @@ UA_SecureChannelManager_cleanupTimedOut(UA_SecureChannelManager *cm, UA_DateTime
 }
 
 /* remove the first channel that has no session attached */
-static UA_Boolean purgeFirstChannelWithoutSession(UA_SecureChannelManager* cm) {
-    channel_list_entry* entry;
+static UA_Boolean
+purgeFirstChannelWithoutSession(UA_SecureChannelManager *cm) {
+    channel_list_entry *entry;
     LIST_FOREACH(entry, &cm->channels, pointers) {
         if(LIST_EMPTY(&(entry->channel.sessions))) {
             UA_LOG_DEBUG_CHANNEL(cm->server->config.logger, &entry->channel,
@@ -114,7 +116,7 @@ UA_SecureChannelManager_create(UA_SecureChannelManager *const cm, UA_Connection
     UA_LOG_INFO(cm->server->config.logger, UA_LOGCATEGORY_SECURECHANNEL,
                 "Creating a new SecureChannel");
 
-    channel_list_entry* entry = (channel_list_entry*)UA_malloc(sizeof(channel_list_entry));
+    channel_list_entry *entry = (channel_list_entry *)UA_malloc(sizeof(channel_list_entry));
     if(!entry)
         return UA_STATUSCODE_BADOUTOFMEMORY;
 
@@ -140,9 +142,9 @@ UA_SecureChannelManager_create(UA_SecureChannelManager *const cm, UA_Connection
 }
 
 UA_StatusCode
-UA_SecureChannelManager_open(UA_SecureChannelManager* cm, UA_SecureChannel *channel,
-                             const UA_OpenSecureChannelRequest* request,
-                             UA_OpenSecureChannelResponse* response) {
+UA_SecureChannelManager_open(UA_SecureChannelManager *cm, UA_SecureChannel *channel,
+                             const UA_OpenSecureChannelRequest *request,
+                             UA_OpenSecureChannelResponse *response) {
     if(channel->state != UA_SECURECHANNELSTATE_FRESH) {
         UA_LOG_ERROR_CHANNEL(cm->server->config.logger, channel,
                              "Called open on already open or closed channel");
@@ -154,40 +156,43 @@ UA_SecureChannelManager_open(UA_SecureChannelManager* cm, UA_SecureChannel *chan
         return UA_STATUSCODE_BADSECURITYMODEREJECTED;
     }
 
+    channel->securityMode = request->securityMode;
+    channel->securityToken.createdAt = UA_DateTime_nowMonotonic();
     channel->securityToken.channelId = cm->lastChannelId++;
     channel->securityToken.createdAt = UA_DateTime_now();
+
+    /* Set the lifetime. Lifetime 0 -> set the maximum possible */
     channel->securityToken.revisedLifetime =
         (request->requestedLifetime > cm->server->config.maxSecurityTokenLifetime) ?
         cm->server->config.maxSecurityTokenLifetime : request->requestedLifetime;
-    if(channel->securityToken.revisedLifetime == 0) // lifetime 0 -> set the maximum possible
+    if(channel->securityToken.revisedLifetime == 0)
         channel->securityToken.revisedLifetime = cm->server->config.maxSecurityTokenLifetime;
-    UA_ByteString_copy(&request->clientNonce, &channel->remoteNonce);
-    channel->securityMode = request->securityMode;
-    const size_t keyLength = channel->securityPolicy->symmetricModule.cryptoModule.
-        getLocalEncryptionKeyLength(channel->securityPolicy, channel->channelContext);
-    UA_SecureChannel_generateNonce(channel,
-                                   keyLength,
-                                   &channel->localNonce);
 
-    UA_SecureChannel_generateNewKeys(channel);
+    /* Set the nonces and generate the keys */
+    UA_StatusCode retval = UA_ByteString_copy(&request->clientNonce, &channel->remoteNonce);
+    retval |= UA_SecureChannel_generateLocalNonce(channel);
+    retval |= UA_SecureChannel_generateNewKeys(channel);
+    if(retval != UA_STATUSCODE_GOOD)
+        return retval;
 
-    // Set the response
-    UA_ByteString_copy(&channel->localNonce, &response->serverNonce);
-    UA_ChannelSecurityToken_copy(&channel->securityToken, &response->securityToken);
+    /* Set the response */
+    retval = UA_ByteString_copy(&channel->localNonce, &response->serverNonce);
+    retval |= UA_ChannelSecurityToken_copy(&channel->securityToken, &response->securityToken);
     response->responseHeader.timestamp = UA_DateTime_now();
     response->responseHeader.requestHandle = request->requestHeader.requestHandle;
+    if(retval != UA_STATUSCODE_GOOD)
+        return retval;
 
-    // Now overwrite the creation date with the internal monotonic clock
-    channel->securityToken.createdAt = UA_DateTime_nowMonotonic();
-
+    /* The channel is open */
     channel->state = UA_SECURECHANNELSTATE_OPEN;
+
     return UA_STATUSCODE_GOOD;
 }
 
 UA_StatusCode
-UA_SecureChannelManager_renew(UA_SecureChannelManager* cm, UA_SecureChannel *channel,
-                              const UA_OpenSecureChannelRequest* request,
-                              UA_OpenSecureChannelResponse* response) {
+UA_SecureChannelManager_renew(UA_SecureChannelManager *cm, UA_SecureChannel *channel,
+                              const UA_OpenSecureChannelRequest *request,
+                              UA_OpenSecureChannelResponse *response) {
     if(channel->state != UA_SECURECHANNELSTATE_OPEN) {
         UA_LOG_ERROR_CHANNEL(cm->server->config.logger, channel,
                              "Called renew on channel which is not open");
@@ -208,26 +213,26 @@ UA_SecureChannelManager_renew(UA_SecureChannelManager* cm, UA_SecureChannel *cha
 
     /* Replace the nonces */
     UA_ByteString_deleteMembers(&channel->remoteNonce);
-    UA_ByteString_copy(&request->clientNonce, &channel->remoteNonce);
-
-    const size_t keyLength = channel->securityPolicy->symmetricModule.cryptoModule.
-        getLocalEncryptionKeyLength(channel->securityPolicy, channel->channelContext);
-    UA_ByteString_deleteMembers(&channel->localNonce);
-    UA_SecureChannel_generateNonce(channel, keyLength, &channel->localNonce);
+    UA_StatusCode retval = UA_ByteString_copy(&request->clientNonce, &channel->remoteNonce);
+    retval |= UA_SecureChannel_generateLocalNonce(channel);
+    if(retval != UA_STATUSCODE_GOOD)
+        return retval;
 
     /* Set the response */
     response->responseHeader.requestHandle = request->requestHeader.requestHandle;
-    UA_ByteString_copy(&channel->localNonce, &response->serverNonce);
-    UA_ChannelSecurityToken_copy(&channel->nextSecurityToken, &response->securityToken);
+    retval = UA_ByteString_copy(&channel->localNonce, &response->serverNonce);
+    retval |= UA_ChannelSecurityToken_copy(&channel->nextSecurityToken, &response->securityToken);
+    if(retval != UA_STATUSCODE_GOOD)
+        return retval;
 
     /* Reset the internal creation date to the monotonic clock */
     channel->nextSecurityToken.createdAt = UA_DateTime_nowMonotonic();
     return UA_STATUSCODE_GOOD;
 }
 
-UA_SecureChannel*
-UA_SecureChannelManager_get(UA_SecureChannelManager* cm, UA_UInt32 channelId) {
-    channel_list_entry* entry;
+UA_SecureChannel *
+UA_SecureChannelManager_get(UA_SecureChannelManager *cm, UA_UInt32 channelId) {
+    channel_list_entry *entry;
     LIST_FOREACH(entry, &cm->channels, pointers) {
         if(entry->channel.securityToken.channelId == channelId)
             return &entry->channel;
@@ -236,8 +241,8 @@ UA_SecureChannelManager_get(UA_SecureChannelManager* cm, UA_UInt32 channelId) {
 }
 
 UA_StatusCode
-UA_SecureChannelManager_close(UA_SecureChannelManager* cm, UA_UInt32 channelId) {
-    channel_list_entry* entry;
+UA_SecureChannelManager_close(UA_SecureChannelManager *cm, UA_UInt32 channelId) {
+    channel_list_entry *entry;
     LIST_FOREACH(entry, &cm->channels, pointers) {
         if(entry->channel.securityToken.channelId == channelId)
             break;

+ 111 - 125
src/server/ua_services_session.c

@@ -8,7 +8,7 @@
  *    Copyright 2015 (c) Chris Iatrou
  *    Copyright 2015 (c) Oleksiy Vasylyev
  *    Copyright 2017 (c) Stefan Profanter, fortiss GmbH
- *    Copyright 2017 (c) Mark Giraud, Fraunhofer IOSB
+ *    Copyright 2017-2018 (c) Mark Giraud, Fraunhofer IOSB
  */
 
 #include "ua_services.h"
@@ -16,12 +16,10 @@
 #include "ua_session_manager.h"
 #include "ua_types_generated_handling.h"
 
-/* Create a signed nonce */
 static UA_StatusCode
-nonceAndSignCreateSessionResponse(UA_Server *server, UA_SecureChannel *channel,
-                                  UA_Session *session,
-                                  const UA_CreateSessionRequest *request,
-                                  UA_CreateSessionResponse *response) {
+signCreateSessionResponse(UA_Server *server, UA_SecureChannel *channel,
+                          const UA_CreateSessionRequest *request,
+                          UA_CreateSessionResponse *response) {
     if(channel->securityMode != UA_MESSAGESECURITYMODE_SIGN &&
        channel->securityMode != UA_MESSAGESECURITYMODE_SIGNANDENCRYPT)
         return UA_STATUSCODE_GOOD;
@@ -29,63 +27,39 @@ nonceAndSignCreateSessionResponse(UA_Server *server, UA_SecureChannel *channel,
     const UA_SecurityPolicy *const securityPolicy = channel->securityPolicy;
     UA_SignatureData *signatureData = &response->serverSignature;
 
-    const UA_NodeId *authenticationToken = &session->header.authenticationToken;
-
-    /* Generate Nonce
-     * FIXME: remove magic number??? */
-    UA_StatusCode retval = UA_SecureChannel_generateNonce(channel, 32, &response->serverNonce);
-    UA_ByteString_deleteMembers(&session->serverNonce);
-    retval |= UA_ByteString_copy(&response->serverNonce, &session->serverNonce);
-    if(retval != UA_STATUSCODE_GOOD) {
-        UA_SessionManager_removeSession(&server->sessionManager, authenticationToken);
-        return retval;
-    }
-
-    size_t signatureSize = securityPolicy->asymmetricModule.cryptoModule.
+    /* Prepare the signature */
+    size_t signatureSize = securityPolicy->certificateSigningAlgorithm.
         getLocalSignatureSize(securityPolicy, channel->channelContext);
-
+    UA_StatusCode retval = UA_String_copy(&securityPolicy->certificateSigningAlgorithm.uri,
+                                          &signatureData->algorithm);
     retval |= UA_ByteString_allocBuffer(&signatureData->signature, signatureSize);
-    if(retval != UA_STATUSCODE_GOOD) {
-        UA_SessionManager_removeSession(&server->sessionManager, authenticationToken);
+    if(retval != UA_STATUSCODE_GOOD)
         return retval;
-    }
 
-    UA_ByteString dataToSign;
-    retval |= UA_ByteString_allocBuffer(&dataToSign,
-                                        request->clientCertificate.length +
-                                        request->clientNonce.length);
-    if(retval != UA_STATUSCODE_GOOD) {
-        UA_SignatureData_deleteMembers(signatureData);
-        UA_SessionManager_removeSession(&server->sessionManager, authenticationToken);
-        return retval;
-    }
+    /* Sign the signature */
+    size_t dataToSignSize = request->clientCertificate.length + request->clientNonce.length;
+    /* Prevent stack-smashing. TODO: Compute MaxSenderCertificateSize */
+    if(dataToSignSize > 4096)
+        return UA_STATUSCODE_BADINTERNALERROR;
 
+    UA_ByteString dataToSign = {dataToSignSize, (UA_Byte*)UA_alloca(dataToSignSize)};
     memcpy(dataToSign.data, request->clientCertificate.data, request->clientCertificate.length);
     memcpy(dataToSign.data + request->clientCertificate.length,
            request->clientNonce.data, request->clientNonce.length);
-
-    retval |= UA_String_copy(&securityPolicy->asymmetricModule.cryptoModule.
-                             signatureAlgorithmUri, &signatureData->algorithm);
-    retval |= securityPolicy->asymmetricModule.cryptoModule.
+    return securityPolicy->certificateSigningAlgorithm.
         sign(securityPolicy, channel->channelContext, &dataToSign, &signatureData->signature);
-
-    UA_ByteString_deleteMembers(&dataToSign);
-    if(retval != UA_STATUSCODE_GOOD) {
-        UA_SignatureData_deleteMembers(signatureData);
-        UA_SessionManager_removeSession(&server->sessionManager, authenticationToken);
-    }
-    return retval;
 }
 
-void Service_CreateSession(UA_Server *server, UA_SecureChannel *channel,
-                           const UA_CreateSessionRequest *request,
-                           UA_CreateSessionResponse *response) {
-    if(channel == NULL) {
+void
+Service_CreateSession(UA_Server *server, UA_SecureChannel *channel,
+                      const UA_CreateSessionRequest *request,
+                      UA_CreateSessionResponse *response) {
+    if(!channel) {
         response->responseHeader.serviceResult = UA_STATUSCODE_BADINTERNALERROR;
         return;
     }
 
-    if(channel->connection == NULL) {
+    if(!channel->connection) {
         response->responseHeader.serviceResult = UA_STATUSCODE_BADINTERNALERROR;
         return;
     }
@@ -100,9 +74,9 @@ void Service_CreateSession(UA_Server *server, UA_SecureChannel *channel,
             return;
         }
     }
+
     if(channel->securityToken.channelId == 0) {
-        response->responseHeader.serviceResult =
-            UA_STATUSCODE_BADSECURECHANNELIDINVALID;
+        response->responseHeader.serviceResult = UA_STATUSCODE_BADSECURECHANNELIDINVALID;
         return;
     }
 
@@ -113,10 +87,10 @@ void Service_CreateSession(UA_Server *server, UA_SecureChannel *channel,
         return;
     }
 
-    ////////////////////// TODO: Compare application URI with certificate uri (decode certificate)
+    /* TODO: Compare application URI with certificate uri (decode certificate) */
 
     /* Allocate the response */
-    response->serverEndpoints = (UA_EndpointDescription*)
+    response->serverEndpoints = (UA_EndpointDescription *)
         UA_Array_new(server->config.endpointsSize,
                      &UA_TYPES[UA_TYPES_ENDPOINTDESCRIPTION]);
     if(!response->serverEndpoints) {
@@ -140,17 +114,21 @@ void Service_CreateSession(UA_Server *server, UA_SecureChannel *channel,
                        &response->serverEndpoints[i].endpointUrl);
     }
 
-    UA_Session *newSession;
+    UA_Session *newSession = NULL;
     response->responseHeader.serviceResult =
-        UA_SessionManager_createSession(&server->sessionManager,
-                                        channel, request, &newSession);
+        UA_SessionManager_createSession(&server->sessionManager, channel, request, &newSession);
     if(response->responseHeader.serviceResult != UA_STATUSCODE_GOOD) {
         UA_LOG_DEBUG_CHANNEL(server->config.logger, channel,
                              "Processing CreateSessionRequest failed");
         return;
     }
 
-    /* Fill the session with more information */
+    UA_assert(newSession != NULL);
+
+    /* Attach the session to the channel. But don't activate for now. */
+    UA_Session_attachToSecureChannel(newSession, channel);
+
+    /* Fill the session information */
     newSession->maxResponseMessageSize = request->maxResponseMessageSize;
     newSession->maxRequestMessageSize =
         channel->connection->localConf.maxMessageSize;
@@ -166,13 +144,18 @@ void Service_CreateSession(UA_Server *server, UA_SecureChannel *channel,
         UA_String_copy(&request->sessionName, &newSession->sessionName);
 
     if(server->config.endpointsSize > 0)
-         response->responseHeader.serviceResult |=
-         UA_ByteString_copy(&channel->securityPolicy->localCertificate,
-                            &response->serverCertificate);
+        response->responseHeader.serviceResult |=
+            UA_ByteString_copy(&channel->securityPolicy->localCertificate,
+                               &response->serverCertificate);
 
-    /* Create a signed nonce */
-    response->responseHeader.serviceResult =
-        nonceAndSignCreateSessionResponse(server, channel, newSession, request, response);
+    /* Create a session nonce */
+    response->responseHeader.serviceResult = UA_Session_generateNonce(newSession);
+    response->responseHeader.serviceResult |=
+        UA_ByteString_copy(&newSession->serverNonce, &response->serverNonce);
+
+    /* Sign the signature */
+    response->responseHeader.serviceResult |=
+       signCreateSessionResponse(server, channel, request, response);
 
     /* Failure -> remove the session */
     if(response->responseHeader.serviceResult != UA_STATUSCODE_GOOD) {
@@ -182,66 +165,58 @@ void Service_CreateSession(UA_Server *server, UA_SecureChannel *channel,
     }
 
     UA_LOG_DEBUG_CHANNEL(server->config.logger, channel,
-           "Session " UA_PRINTF_GUID_FORMAT " created",
-           UA_PRINTF_GUID_DATA(newSession->sessionId.identifier.guid));
+                         "Session " UA_PRINTF_GUID_FORMAT " created",
+                         UA_PRINTF_GUID_DATA(newSession->sessionId.identifier.guid));
 }
 
-static void
-checkSignature(const UA_Server *server,
-               const UA_SecureChannel *channel,
-               UA_Session *session,
-               const UA_ActivateSessionRequest *request,
-               UA_ActivateSessionResponse *response) {
-    if(channel->securityMode == UA_MESSAGESECURITYMODE_SIGN ||
-       channel->securityMode == UA_MESSAGESECURITYMODE_SIGNANDENCRYPT) {
-        const UA_SecurityPolicy *const securityPolicy = channel->securityPolicy;
-        const UA_ByteString *const localCertificate = &securityPolicy->localCertificate;
+static UA_StatusCode
+checkSignature(const UA_Server *server, const UA_SecureChannel *channel,
+               UA_Session *session, const UA_ActivateSessionRequest *request) {
+    if(channel->securityMode != UA_MESSAGESECURITYMODE_SIGN &&
+       channel->securityMode != UA_MESSAGESECURITYMODE_SIGNANDENCRYPT)
+        return UA_STATUSCODE_GOOD;
 
-        UA_ByteString dataToVerify;
-        UA_StatusCode retval = UA_ByteString_allocBuffer(&dataToVerify,
-                                                         localCertificate->length + session->serverNonce.length);
+    if(!channel->securityPolicy)
+        return UA_STATUSCODE_BADINTERNALERROR;
+    const UA_SecurityPolicy *securityPolicy = channel->securityPolicy;
+    const UA_ByteString *localCertificate = &securityPolicy->localCertificate;
 
-        if(retval != UA_STATUSCODE_GOOD) {
-            response->responseHeader.serviceResult = retval;
-            UA_LOG_DEBUG_SESSION(server->config.logger, session,
-                                 "Failed to allocate buffer for signature verification! %#10x", retval);
-            return;
-        }
+    size_t dataToVerifySize = localCertificate->length + session->serverNonce.length;
 
-        memcpy(dataToVerify.data, localCertificate->data, localCertificate->length);
-        memcpy(dataToVerify.data + localCertificate->length,
-               session->serverNonce.data, session->serverNonce.length);
-
-        retval = securityPolicy->asymmetricModule.cryptoModule.
-            verify(securityPolicy, channel->channelContext, &dataToVerify,
-                   &request->clientSignature.signature);
-        if(retval != UA_STATUSCODE_GOOD) {
-            response->responseHeader.serviceResult = retval;
-            UA_LOG_DEBUG_SESSION(server->config.logger, session,
-                                 "Failed to verify the client signature! %#10x", retval);
-            UA_ByteString_deleteMembers(&dataToVerify);
-            return;
-        }
+    UA_ByteString dataToVerify;
+    UA_StatusCode retval = UA_ByteString_allocBuffer(&dataToVerify, dataToVerifySize);
+    if(retval != UA_STATUSCODE_GOOD)
+        return retval;
 
-        retval  = UA_SecureChannel_generateNonce(channel, 32, &response->serverNonce);
-        UA_ByteString_deleteMembers(&session->serverNonce);
-        retval |= UA_ByteString_copy(&response->serverNonce, &session->serverNonce);
-        if(retval != UA_STATUSCODE_GOOD) {
-            response->responseHeader.serviceResult = retval;
-            UA_LOG_DEBUG_SESSION(server->config.logger, session,
-                                 "Failed to generate a new nonce! %#10x", retval);
-            UA_ByteString_deleteMembers(&dataToVerify);
-            return;
-        }
+    memcpy(dataToVerify.data, localCertificate->data, localCertificate->length);
+    memcpy(dataToVerify.data + localCertificate->length,
+           session->serverNonce.data, session->serverNonce.length);
 
-        UA_ByteString_deleteMembers(&dataToVerify);
-    }
+    retval = securityPolicy->certificateSigningAlgorithm.verify(securityPolicy, channel->channelContext, &dataToVerify,
+                                                                &request->clientSignature.signature);
+    UA_ByteString_deleteMembers(&dataToVerify);
+    return retval;
 }
 
+/* TODO: Check all of the following:
+ *
+ * Part 4, §5.6.3: When the ActivateSession Service is called for the first time
+ * then the Server shall reject the request if the SecureChannel is not same as
+ * the one associated with the CreateSession request. Subsequent calls to
+ * ActivateSession may be associated with different SecureChannels. If this is
+ * the case then the Server shall verify that the Certificate the Client used to
+ * create the new SecureChannel is the same as the Certificate used to create
+ * the original SecureChannel. In addition, the Server shall verify that the
+ * Client supplied a UserIdentityToken that is identical to the token currently
+ * associated with the Session. Once the Server accepts the new SecureChannel it
+ * shall reject requests sent via the old SecureChannel. */
+
 void
 Service_ActivateSession(UA_Server *server, UA_SecureChannel *channel,
                         UA_Session *session, const UA_ActivateSessionRequest *request,
                         UA_ActivateSessionResponse *response) {
+    UA_LOG_DEBUG_SESSION(server->config.logger, session, "Execute ActivateSession");
+
     if(session->validTill < UA_DateTime_nowMonotonic()) {
         UA_LOG_INFO_SESSION(server->config.logger, session,
                             "ActivateSession: SecureChannel %i wants "
@@ -252,40 +227,51 @@ Service_ActivateSession(UA_Server *server, UA_SecureChannel *channel,
         return;
     }
 
-    checkSignature(server, channel, session, request, response);
-    if(response->responseHeader.serviceResult != UA_STATUSCODE_GOOD)
+    /* Check if the signature corresponds to the ServerNonce that was last sent
+     * to the client */
+    response->responseHeader.serviceResult = checkSignature(server, channel, session, request);
+    if(response->responseHeader.serviceResult != UA_STATUSCODE_GOOD) {
+        UA_LOG_INFO_SESSION(server->config.logger, session,
+                            "Signature check failed with status code %s",
+                            UA_StatusCode_name(response->responseHeader.serviceResult));
         return;
+    }
 
     /* Callback into userland access control */
     response->responseHeader.serviceResult =
         server->config.accessControl.activateSession(&session->sessionId,
                                                      &request->userIdentityToken,
                                                      &session->sessionHandle);
-    if(response->responseHeader.serviceResult != UA_STATUSCODE_GOOD)
+    if(response->responseHeader.serviceResult != UA_STATUSCODE_GOOD) {
+        UA_LOG_INFO_SESSION(server->config.logger, session,
+                            "ActivateSession: Could not generate a server nonce");
         return;
+    }
 
-    /* Detach the old SecureChannel */
     if(session->header.channel && session->header.channel != channel) {
         UA_LOG_INFO_SESSION(server->config.logger, session,
                             "ActivateSession: Detach from old channel");
+        /* Detach the old SecureChannel and attach the new */
         UA_Session_detachFromSecureChannel(session);
-        session->activated = false;
+        UA_Session_attachToSecureChannel(session, channel);
     }
 
-    if (session->activated) {
+    /* Activate the session */
+    session->activated = true;
+    UA_Session_updateLifetime(session);
+
+    /* Generate a new session nonce for the next time ActivateSession is called */
+    response->responseHeader.serviceResult = UA_Session_generateNonce(session);
+    response->responseHeader.serviceResult |=
+        UA_ByteString_copy(&session->serverNonce, &response->serverNonce);
+    if(response->responseHeader.serviceResult != UA_STATUSCODE_GOOD) {
+        UA_Session_detachFromSecureChannel(session);
+        session->activated = false;
         UA_LOG_INFO_SESSION(server->config.logger, session,
-                            "ActivateSession: SecureChannel %i wants "
-                                    "to activate, but the session is already activated",
-                            channel->securityToken.channelId);
-        response->responseHeader.serviceResult =
-                UA_STATUSCODE_BADSESSIONIDINVALID;
+                            "ActivateSession: Could not generate a server nonce");
         return;
-
     }
-    /* Attach to the SecureChannel and activate */
-    UA_Session_attachToSecureChannel(session, channel);
-    session->activated = true;
-    UA_Session_updateLifetime(session);
+
     UA_LOG_INFO_SESSION(server->config.logger, session,
                         "ActivateSession: Session activated");
 }

+ 40 - 35
src/server/ua_services_subscription.c

@@ -34,8 +34,8 @@ setSubscriptionSettings(UA_Server *server, UA_Subscription *subscription,
     /* deregister the callback if required */
     UA_StatusCode retval = Subscription_unregisterPublishCallback(server, subscription);
     if(retval != UA_STATUSCODE_GOOD)
-        UA_LOG_DEBUG_SESSION(server->config.logger, subscription->session, "Subscription %u | "
-                             "Could not unregister publish callback with error code %s",
+        UA_LOG_DEBUG_SESSION(server->config.logger, subscription->session,
+                             "Subscription %u | Could not unregister publish callback with error code %s",
                              subscription->subscriptionId, UA_StatusCode_name(retval));
 
     /* re-parameterize the subscription */
@@ -59,8 +59,8 @@ setSubscriptionSettings(UA_Server *server, UA_Subscription *subscription,
 
     retval = Subscription_registerPublishCallback(server, subscription);
     if(retval != UA_STATUSCODE_GOOD)
-        UA_LOG_DEBUG_SESSION(server->config.logger, subscription->session, "Subscription %u | "
-                             "Could not register publish callback with error code %s",
+        UA_LOG_DEBUG_SESSION(server->config.logger, subscription->session,
+                             "Subscription %u | Could not register publish callback with error code %s",
                              subscription->subscriptionId, UA_StatusCode_name(retval));
 }
 
@@ -166,13 +166,13 @@ setMonitoredItemSettings(UA_Server *server, UA_MonitoredItem *mon,
     /* SamplingInterval */
     UA_Double samplingInterval = params->samplingInterval;
     if(mon->attributeId == UA_ATTRIBUTEID_VALUE) {
-        const UA_VariableNode *vn = (const UA_VariableNode*)
+        const UA_VariableNode *vn = (const UA_VariableNode *)
             UA_Nodestore_get(server, &mon->monitoredNodeId);
         if(vn) {
             if(vn->nodeClass == UA_NODECLASS_VARIABLE &&
-               samplingInterval <  vn->minimumSamplingInterval)
+               samplingInterval < vn->minimumSamplingInterval)
                 samplingInterval = vn->minimumSamplingInterval;
-            UA_Nodestore_release(server, (const UA_Node*)vn);
+            UA_Nodestore_release(server, (const UA_Node *)vn);
         }
     } else if(mon->attributeId == UA_ATTRIBUTEID_EVENTNOTIFIER) {
         /* TODO: events should not need a samplinginterval */
@@ -180,7 +180,7 @@ setMonitoredItemSettings(UA_Server *server, UA_MonitoredItem *mon,
     }
     mon->samplingInterval = samplingInterval;
     UA_BOUNDEDVALUE_SETWBOUNDS(server->config.samplingIntervalLimits,
-        samplingInterval, mon->samplingInterval);
+                               samplingInterval, mon->samplingInterval);
     if(samplingInterval != samplingInterval) /* Check for nan */
         mon->samplingInterval = server->config.samplingIntervalLimits.min;
 
@@ -206,7 +206,7 @@ setMonitoredItemSettings(UA_Server *server, UA_MonitoredItem *mon,
         MonitoredItem_registerSampleCallback(server, mon);
 }
 
-static const UA_String binaryEncoding = {sizeof("Default Binary")-1, (UA_Byte*)"Default Binary"};
+static const UA_String binaryEncoding = {sizeof("Default Binary") - 1, (UA_Byte *)"Default Binary"};
 
 /* Thread-local variables to pass additional arguments into the operation */
 struct createMonContext {
@@ -246,7 +246,7 @@ Operation_CreateMonitoredItem(UA_Server *server, UA_Session *session, struct cre
     /* Check if the encoding is supported */
     if(request->itemToMonitor.dataEncoding.name.length > 0 &&
        (!UA_String_equal(&binaryEncoding, &request->itemToMonitor.dataEncoding.name) ||
-       request->itemToMonitor.dataEncoding.namespaceIndex != 0)) {
+        request->itemToMonitor.dataEncoding.namespaceIndex != 0)) {
         result->statusCode = UA_STATUSCODE_BADDATAENCODINGUNSUPPORTED;
         return;
     }
@@ -346,9 +346,10 @@ Operation_ModifyMonitoredItem(UA_Server *server, UA_Session *session, UA_Subscri
     MonitoredItem_ensureQueueSpace(mon);
 }
 
-void Service_ModifyMonitoredItems(UA_Server *server, UA_Session *session,
-                                  const UA_ModifyMonitoredItemsRequest *request,
-                                  UA_ModifyMonitoredItemsResponse *response) {
+void
+Service_ModifyMonitoredItems(UA_Server *server, UA_Session *session,
+                             const UA_ModifyMonitoredItemsRequest *request,
+                             UA_ModifyMonitoredItemsResponse *response) {
     UA_LOG_DEBUG_SESSION(server->config.logger, session, "Processing ModifyMonitoredItemsRequest");
 
     if(server->config.maxMonitoredItemsPerCall != 0 &&
@@ -399,7 +400,7 @@ Operation_SetMonitoringMode(UA_Server *server, UA_Session *session,
         *result = UA_STATUSCODE_BADNOTIMPLEMENTED;
         return;
     }
-  
+
     /* check monitoringMode is valid or not */
     if(smc->monitoringMode > UA_MONITORINGMODE_REPORTING) {
         *result = UA_STATUSCODE_BADMONITORINGMODEINVALID;
@@ -429,9 +430,10 @@ Operation_SetMonitoringMode(UA_Server *server, UA_Session *session,
     }
 }
 
-void Service_SetMonitoringMode(UA_Server *server, UA_Session *session,
-                               const UA_SetMonitoringModeRequest *request,
-                               UA_SetMonitoringModeResponse *response) {
+void
+Service_SetMonitoringMode(UA_Server *server, UA_Session *session,
+                          const UA_SetMonitoringModeRequest *request,
+                          UA_SetMonitoringModeResponse *response) {
     UA_LOG_DEBUG_SESSION(server->config.logger, session,
                          "Processing SetMonitoringMode");
 
@@ -487,9 +489,9 @@ Service_Publish(UA_Server *server, UA_Session *session,
     /* Handle too many subscriptions to free resources before trying to allocate
      * resources for the new publish request. If the limit has been reached the
      * oldest publish request shall be responded */
-    if((server->config.maxPublishReqPerSession != 0 ) &&
+    if((server->config.maxPublishReqPerSession != 0) &&
        (UA_Session_getNumPublishReq(session) >= server->config.maxPublishReqPerSession)) {
-        if(!UA_Subscription_reachedPublishReqLimit(server,session)) {
+        if(!UA_Subscription_reachedPublishReqLimit(server, session)) {
             subscriptionSendError(session->header.channel, requestId,
                                   request->requestHeader.requestHandle,
                                   UA_STATUSCODE_BADINTERNALERROR);
@@ -497,7 +499,7 @@ Service_Publish(UA_Server *server, UA_Session *session,
         }
     }
 
-    UA_PublishResponseEntry *entry = (UA_PublishResponseEntry*)
+    UA_PublishResponseEntry *entry = (UA_PublishResponseEntry *)
         UA_malloc(sizeof(UA_PublishResponseEntry));
     if(!entry) {
         subscriptionSendError(session->header.channel, requestId,
@@ -512,7 +514,7 @@ Service_Publish(UA_Server *server, UA_Session *session,
     UA_PublishResponse_init(response);
     response->responseHeader.requestHandle = request->requestHeader.requestHandle;
     if(request->subscriptionAcknowledgementsSize > 0) {
-        response->results = (UA_StatusCode*)
+        response->results = (UA_StatusCode *)
             UA_Array_new(request->subscriptionAcknowledgementsSize,
                          &UA_TYPES[UA_TYPES_STATUSCODE]);
         if(!response->results) {
@@ -560,8 +562,8 @@ Service_Publish(UA_Server *server, UA_Session *session,
         found = false;
     }
 
-    for(int i=0; i<loopCount; i++) {
-       LIST_FOREACH(immediate, &session->serverSubscriptions, listEntry) {
+    for(int i = 0; i < loopCount; i++) {
+        LIST_FOREACH(immediate, &session->serverSubscriptions, listEntry) {
             if(!found) {
                 if(session->lastSeenSubscriptionId == immediate->subscriptionId) {
                     found = true;
@@ -569,8 +571,9 @@ Service_Publish(UA_Server *server, UA_Session *session,
             } else {
                 if(immediate->state == UA_SUBSCRIPTIONSTATE_LATE) {
                     session->lastSeenSubscriptionId = immediate->subscriptionId;
-                    UA_LOG_DEBUG_SESSION(server->config.logger, session, "Subscription %u | "
-                                         "Response on a late subscription", immediate->subscriptionId);
+                    UA_LOG_DEBUG_SESSION(server->config.logger, session,
+                                         "Subscription %u | Response on a late subscription",
+                                         immediate->subscriptionId);
                     UA_Subscription_publishCallback(server, immediate);
                     return;
                 }
@@ -592,8 +595,8 @@ Operation_DeleteSubscription(UA_Server *server, UA_Session *session, void *_,
                              *subscriptionId);
     } else {
         UA_LOG_DEBUG_SESSION(server->config.logger, session,
-                             "Deleting Subscription with Id %u failed with error "
-                             "code %s", *subscriptionId, UA_StatusCode_name(*result));
+                             "Deleting Subscription with Id %u failed with error code %s",
+                             *subscriptionId, UA_StatusCode_name(*result));
     }
 }
 
@@ -623,9 +626,10 @@ Operation_DeleteMonitoredItem(UA_Server *server, UA_Session *session, UA_Subscri
     *result = UA_Subscription_deleteMonitoredItem(server, sub, *monitoredItemId);
 }
 
-void Service_DeleteMonitoredItems(UA_Server *server, UA_Session *session,
-                                  const UA_DeleteMonitoredItemsRequest *request,
-                                  UA_DeleteMonitoredItemsResponse *response) {
+void
+Service_DeleteMonitoredItems(UA_Server *server, UA_Session *session,
+                             const UA_DeleteMonitoredItemsRequest *request,
+                             UA_DeleteMonitoredItemsResponse *response) {
     UA_LOG_DEBUG_SESSION(server->config.logger, session,
                          "Processing DeleteMonitoredItemsRequest");
 
@@ -651,9 +655,10 @@ void Service_DeleteMonitoredItems(UA_Server *server, UA_Session *session,
                                            &response->resultsSize, &UA_TYPES[UA_TYPES_STATUSCODE]);
 }
 
-void Service_Republish(UA_Server *server, UA_Session *session,
-                       const UA_RepublishRequest *request,
-                       UA_RepublishResponse *response) {
+void
+Service_Republish(UA_Server *server, UA_Session *session,
+                  const UA_RepublishRequest *request,
+                  UA_RepublishResponse *response) {
     UA_LOG_DEBUG_SESSION(server->config.logger, session,
                          "Processing RepublishRequest");
 
@@ -674,8 +679,8 @@ void Service_Republish(UA_Server *server, UA_Session *session,
             break;
     }
     if(!entry) {
-      response->responseHeader.serviceResult = UA_STATUSCODE_BADMESSAGENOTAVAILABLE;
-      return;
+        response->responseHeader.serviceResult = UA_STATUSCODE_BADMESSAGENOTAVAILABLE;
+        return;
     }
 
     response->responseHeader.serviceResult =

+ 21 - 0
src/server/ua_session.c

@@ -12,6 +12,8 @@
 #include "ua_server_internal.h"
 #endif
 
+#define UA_SESSION_NONCELENTH 32
+
 UA_Session adminSession = {
     {{NULL, NULL}, /* .pointers */
      {0,UA_NODEIDTYPE_NUMERIC,{1}}, /* .authenticationToken */
@@ -92,6 +94,25 @@ void UA_Session_detachFromSecureChannel(UA_Session *session) {
     LIST_REMOVE(&session->header, pointers);
 }
 
+UA_StatusCode
+UA_Session_generateNonce(UA_Session *session) {
+    UA_SecureChannel *channel = session->header.channel;
+    if(!channel || !channel->securityPolicy)
+        return UA_STATUSCODE_BADINTERNALERROR;
+
+    /* Is the length of the previous nonce correct? */
+    if(session->serverNonce.length != UA_SESSION_NONCELENTH) {
+        UA_ByteString_deleteMembers(&session->serverNonce);
+        UA_StatusCode retval =
+            UA_ByteString_allocBuffer(&session->serverNonce, UA_SESSION_NONCELENTH);
+        if(retval != UA_STATUSCODE_GOOD)
+            return retval;
+    }
+
+    return channel->securityPolicy->symmetricModule.
+        generateNonce(channel->securityPolicy, &session->serverNonce);
+}
+
 void UA_Session_updateLifetime(UA_Session *session) {
     session->validTill = UA_DateTime_nowMonotonic() +
         (UA_DateTime)(session->timeout * UA_DATETIME_MSEC);

+ 1 - 0
src/server/ua_session.h

@@ -75,6 +75,7 @@ void UA_Session_init(UA_Session *session);
 void UA_Session_deleteMembersCleanup(UA_Session *session, UA_Server *server);
 void UA_Session_attachToSecureChannel(UA_Session *session, UA_SecureChannel *channel);
 void UA_Session_detachFromSecureChannel(UA_Session *session);
+UA_StatusCode UA_Session_generateNonce(UA_Session *session);
 
 /* If any activity on a session happens, the timeout is extended */
 void UA_Session_updateLifetime(UA_Session *session);

+ 131 - 68
src/ua_securechannel.c

@@ -8,7 +8,7 @@
  *    Copyright 2015 (c) Oleksiy Vasylyev
  *    Copyright 2016 (c) TorbenD
  *    Copyright 2017 (c) Stefan Profanter, fortiss GmbH
- *    Copyright 2017 (c) Mark Giraud, Fraunhofer IOSB
+ *    Copyright 2017-2018 (c) Mark Giraud, Fraunhofer IOSB
  */
 
 #include "ua_util.h"
@@ -117,31 +117,70 @@ UA_SecureChannel_deleteMembersCleanup(UA_SecureChannel *channel) {
 }
 
 UA_StatusCode
-UA_SecureChannel_generateNonce(const UA_SecureChannel *channel,
-                               size_t nonceLength,
-                               UA_ByteString *nonce) {
-    UA_ByteString_deleteMembers(nonce);
-    UA_StatusCode retval = UA_ByteString_allocBuffer(nonce, nonceLength);
-    if(retval != UA_STATUSCODE_GOOD)
-        return retval;
+UA_SecureChannel_generateLocalNonce(UA_SecureChannel *channel) {
+    if(!channel->securityPolicy)
+        return UA_STATUSCODE_BADINTERNALERROR;
+
+    /* Is the length of the previous nonce correct? */
+    size_t nonceLength = channel->securityPolicy->symmetricModule.secureChannelNonceLength;
+    if(channel->localNonce.length != nonceLength) {
+        UA_ByteString_deleteMembers(&channel->localNonce);
+        UA_StatusCode retval = UA_ByteString_allocBuffer(&channel->localNonce, nonceLength);
+        if(retval != UA_STATUSCODE_GOOD)
+            return retval;
+    }
 
     return channel->securityPolicy->symmetricModule.
-        generateNonce(channel->securityPolicy, nonce);
+        generateNonce(channel->securityPolicy, &channel->localNonce);
 }
 
-UA_StatusCode
-UA_SecureChannel_generateNewKeys(UA_SecureChannel *channel) {
-    const UA_SecurityPolicy *securityPolicy = channel->securityPolicy;
-    const UA_SecurityPolicyChannelModule *channelModule =
-        &securityPolicy->channelModule;
-    const UA_SecurityPolicySymmetricModule *symmetricModule =
-        &securityPolicy->symmetricModule;
+static UA_StatusCode
+UA_SecureChannel_generateLocalKeys(const UA_SecureChannel *const channel,
+                                   const UA_SecurityPolicy *const securityPolicy) {
+    const UA_SecurityPolicyChannelModule *channelModule = &securityPolicy->channelModule;
+    const UA_SecurityPolicySymmetricModule *symmetricModule = &securityPolicy->symmetricModule;
+    const UA_SecurityPolicyCryptoModule *const cryptoModule = &securityPolicy->symmetricModule.cryptoModule;
+    /* Symmetric key length */
+    size_t encryptionKeyLength =
+        cryptoModule->encryptionAlgorithm.getLocalKeyLength(securityPolicy, channel->channelContext);
+    size_t encryptionBlockSize =
+        cryptoModule->encryptionAlgorithm.getLocalBlockSize(securityPolicy, channel->channelContext);
+    size_t signingKeyLength =
+        cryptoModule->signatureAlgorithm.getLocalKeyLength(securityPolicy, channel->channelContext);
+    const size_t buffSize = encryptionBlockSize + signingKeyLength + encryptionKeyLength;
+    UA_ByteString buffer = {buffSize, (UA_Byte *)UA_alloca(buffSize)};
+
+    /* Local keys */
+    UA_StatusCode retval = symmetricModule->generateKey(securityPolicy, &channel->remoteNonce,
+                                                        &channel->localNonce, &buffer);
+    if(retval != UA_STATUSCODE_GOOD)
+        return retval;
+    const UA_ByteString localSigningKey = {signingKeyLength, buffer.data};
+    const UA_ByteString localEncryptingKey = {encryptionKeyLength,
+                                              buffer.data + signingKeyLength};
+    const UA_ByteString localIv = {encryptionBlockSize,
+                                   buffer.data + signingKeyLength +
+                                   encryptionKeyLength};
+    retval = channelModule->setLocalSymSigningKey(channel->channelContext, &localSigningKey);
+    retval |= channelModule->setLocalSymEncryptingKey(channel->channelContext, &localEncryptingKey);
+    retval |= channelModule->setLocalSymIv(channel->channelContext, &localIv);
+    return retval;
+}
 
+static UA_StatusCode
+UA_SecureChannel_generateRemoteKeys(const UA_SecureChannel *const channel,
+                                    const UA_SecurityPolicy *const securityPolicy) {
+    const UA_SecurityPolicyChannelModule *channelModule = &securityPolicy->channelModule;
+    const UA_SecurityPolicySymmetricModule *symmetricModule = &securityPolicy->symmetricModule;
+    const UA_SecurityPolicyCryptoModule *const cryptoModule = &securityPolicy->symmetricModule.cryptoModule;
     /* Symmetric key length */
-    size_t encryptionKeyLength = symmetricModule->cryptoModule.
-        getLocalEncryptionKeyLength(securityPolicy, channel->channelContext);
-    const size_t buffSize = symmetricModule->encryptionBlockSize +
-                            symmetricModule->signingKeyLength + encryptionKeyLength;
+    size_t encryptionKeyLength =
+        cryptoModule->encryptionAlgorithm.getRemoteKeyLength(securityPolicy, channel->channelContext);
+    size_t encryptionBlockSize =
+        cryptoModule->encryptionAlgorithm.getRemoteBlockSize(securityPolicy, channel->channelContext);
+    size_t signingKeyLength =
+        cryptoModule->signatureAlgorithm.getRemoteKeyLength(securityPolicy, channel->channelContext);
+    const size_t buffSize = encryptionBlockSize + signingKeyLength + encryptionKeyLength;
     UA_ByteString buffer = {buffSize, (UA_Byte *)UA_alloca(buffSize)};
 
     /* Remote keys */
@@ -149,11 +188,11 @@ UA_SecureChannel_generateNewKeys(UA_SecureChannel *channel) {
                                                         &channel->remoteNonce, &buffer);
     if(retval != UA_STATUSCODE_GOOD)
         return retval;
-    const UA_ByteString remoteSigningKey = {symmetricModule->signingKeyLength, buffer.data};
+    const UA_ByteString remoteSigningKey = {signingKeyLength, buffer.data};
     const UA_ByteString remoteEncryptingKey = {encryptionKeyLength,
-                                               buffer.data + symmetricModule->signingKeyLength};
-    const UA_ByteString remoteIv = {symmetricModule->encryptionBlockSize,
-                                    buffer.data + symmetricModule->signingKeyLength +
+                                               buffer.data + signingKeyLength};
+    const UA_ByteString remoteIv = {encryptionBlockSize,
+                                    buffer.data + signingKeyLength +
                                     encryptionKeyLength};
     retval = channelModule->setRemoteSymSigningKey(channel->channelContext, &remoteSigningKey);
     retval |= channelModule->setRemoteSymEncryptingKey(channel->channelContext, &remoteEncryptingKey);
@@ -161,20 +200,19 @@ UA_SecureChannel_generateNewKeys(UA_SecureChannel *channel) {
     if(retval != UA_STATUSCODE_GOOD)
         return retval;
 
-    /* Local keys */
-    retval = symmetricModule->generateKey(securityPolicy, &channel->remoteNonce,
-                                          &channel->localNonce, &buffer);
+    return retval;
+}
+
+UA_StatusCode
+UA_SecureChannel_generateNewKeys(UA_SecureChannel *channel) {
+    UA_StatusCode retval = UA_SecureChannel_generateLocalKeys(channel, channel->securityPolicy);
     if(retval != UA_STATUSCODE_GOOD)
         return retval;
-    const UA_ByteString localSigningKey = {symmetricModule->signingKeyLength, buffer.data};
-    const UA_ByteString localEncryptingKey = {encryptionKeyLength,
-                                              buffer.data + symmetricModule->signingKeyLength};
-    const UA_ByteString localIv = {symmetricModule->encryptionBlockSize,
-                                   buffer.data + symmetricModule->signingKeyLength +
-                                   encryptionKeyLength};
-    retval = channelModule->setLocalSymSigningKey(channel->channelContext, &localSigningKey);
-    retval |= channelModule->setLocalSymEncryptingKey(channel->channelContext, &localEncryptingKey);
-    retval |= channelModule->setLocalSymIv(channel->channelContext, &localIv);
+
+    retval = UA_SecureChannel_generateRemoteKeys(channel, channel->securityPolicy);
+    if(retval != UA_STATUSCODE_GOOD)
+        return retval;
+
     return retval;
 }
 
@@ -207,13 +245,13 @@ UA_SecureChannel_revolveTokens(UA_SecureChannel *channel) {
 static UA_UInt16
 calculatePaddingAsym(const UA_SecurityPolicy *securityPolicy, const void *channelContext,
                      size_t bytesToWrite, UA_Byte *paddingSize, UA_Byte *extraPaddingSize) {
-    size_t plainTextBlockSize = securityPolicy->channelModule.
-        getRemoteAsymPlainTextBlockSize(channelContext);
-    size_t signatureSize = securityPolicy->asymmetricModule.cryptoModule.
+    size_t plainTextBlockSize = securityPolicy->asymmetricModule.cryptoModule.encryptionAlgorithm.
+        getRemotePlainTextBlockSize(securityPolicy, channelContext);
+    size_t signatureSize = securityPolicy->asymmetricModule.cryptoModule.signatureAlgorithm.
         getLocalSignatureSize(securityPolicy, channelContext);
     size_t paddingBytes = 1;
-    if(securityPolicy->asymmetricModule.cryptoModule.
-        getRemoteEncryptionKeyLength(securityPolicy, channelContext) > 2048)
+    if(securityPolicy->asymmetricModule.cryptoModule.encryptionAlgorithm.
+        getRemoteKeyLength(securityPolicy, channelContext) > 2048)
         ++paddingBytes;
     size_t padding = (plainTextBlockSize - ((bytesToWrite + signatureSize + paddingBytes) %
                                             plainTextBlockSize));
@@ -247,14 +285,14 @@ hideBytesAsym(const UA_SecureChannel *channel, UA_Byte **buf_start, const UA_Byt
     /* Hide bytes for signature and padding */
     if(channel->securityMode == UA_MESSAGESECURITYMODE_SIGN ||
        channel->securityMode == UA_MESSAGESECURITYMODE_SIGNANDENCRYPT) {
-        *buf_end -= securityPolicy->asymmetricModule.cryptoModule.
+        *buf_end -= securityPolicy->asymmetricModule.cryptoModule.signatureAlgorithm.
             getLocalSignatureSize(securityPolicy, channel->channelContext);
         *buf_end -= 2; /* padding byte and extraPadding byte */
 
         /* Add some overhead length due to RSA implementations adding a signature themselves */
-        *buf_end -= securityPolicy->channelModule.
-            getRemoteAsymEncryptionBufferLengthOverhead(channel->channelContext,
-                                                        potentialEncryptionMaxSize);
+        *buf_end -= UA_SecurityPolicy_getRemoteAsymEncryptionBufferLengthOverhead(securityPolicy,
+                                                                                  channel->channelContext,
+                                                                                  potentialEncryptionMaxSize);
     }
 }
 
@@ -314,8 +352,8 @@ UA_SecureChannel_sendAsymmetricOPNMessage(UA_SecureChannel *channel, UA_UInt32 r
             *buf_pos = paddingSize;
             ++buf_pos;
         }
-        if(securityPolicy->asymmetricModule.cryptoModule.
-            getRemoteEncryptionKeyLength(securityPolicy, channel->channelContext) > 2048) {
+        if(securityPolicy->asymmetricModule.cryptoModule.encryptionAlgorithm.
+            getRemoteKeyLength(securityPolicy, channel->channelContext) > 2048) {
             *buf_pos = extraPaddingSize;
             ++buf_pos;
         }
@@ -326,7 +364,7 @@ UA_SecureChannel_sendAsymmetricOPNMessage(UA_SecureChannel *channel, UA_UInt32 r
     size_t total_length = pre_sig_length;
     if(channel->securityMode == UA_MESSAGESECURITYMODE_SIGN ||
        channel->securityMode == UA_MESSAGESECURITYMODE_SIGNANDENCRYPT)
-        total_length += securityPolicy->asymmetricModule.cryptoModule.
+        total_length += securityPolicy->asymmetricModule.cryptoModule.signatureAlgorithm.
             getLocalSignatureSize(securityPolicy, channel->channelContext);
 
     /* Encode the headers at the beginning of the message */
@@ -336,8 +374,9 @@ UA_SecureChannel_sendAsymmetricOPNMessage(UA_SecureChannel *channel, UA_UInt32 r
     UA_SecureConversationMessageHeader respHeader;
     respHeader.messageHeader.messageTypeAndChunkType = UA_MESSAGETYPE_OPN + UA_CHUNKTYPE_FINAL;
     respHeader.messageHeader.messageSize = (UA_UInt32)
-        (total_length + securityPolicy->channelModule.
-            getRemoteAsymEncryptionBufferLengthOverhead(channel->channelContext, dataToEncryptLength));
+        (total_length + UA_SecurityPolicy_getRemoteAsymEncryptionBufferLengthOverhead(securityPolicy,
+                                                                                      channel->channelContext,
+                                                                                      dataToEncryptLength));
     respHeader.secureChannelId = channel->securityToken.channelId;
     retval = UA_encodeBinary(&respHeader, &UA_TRANSPORT[UA_TRANSPORT_SECURECONVERSATIONMESSAGEHEADER],
                              &header_pos, &buf_end, NULL, NULL);
@@ -370,10 +409,10 @@ UA_SecureChannel_sendAsymmetricOPNMessage(UA_SecureChannel *channel, UA_UInt32 r
        channel->securityMode == UA_MESSAGESECURITYMODE_SIGNANDENCRYPT) {
         /* Sign message */
         const UA_ByteString dataToSign = {pre_sig_length, buf.data};
-        size_t sigsize = securityPolicy->asymmetricModule.cryptoModule.
+        size_t sigsize = securityPolicy->asymmetricModule.cryptoModule.signatureAlgorithm.
             getLocalSignatureSize(securityPolicy, channel->channelContext);
         UA_ByteString signature = {sigsize, buf.data + pre_sig_length};
-        retval = securityPolicy->asymmetricModule.cryptoModule.
+        retval = securityPolicy->asymmetricModule.cryptoModule.signatureAlgorithm.
             sign(securityPolicy, channel->channelContext, &dataToSign, &signature);
         if(retval != UA_STATUSCODE_GOOD) {
             connection->releaseSendBuffer(connection, &buf);
@@ -387,7 +426,7 @@ UA_SecureChannel_sendAsymmetricOPNMessage(UA_SecureChannel *channel, UA_UInt32 r
             UA_SECURE_CONVERSATION_MESSAGE_HEADER_LENGTH + securityHeaderLength;
         UA_ByteString dataToEncrypt = {total_length - unencrypted_length,
                                        &buf.data[unencrypted_length]};
-        retval = securityPolicy->asymmetricModule.cryptoModule.
+        retval = securityPolicy->asymmetricModule.cryptoModule.encryptionAlgorithm.
             encrypt(securityPolicy, channel->channelContext, &dataToEncrypt);
         if(retval != UA_STATUSCODE_GOOD) {
             connection->releaseSendBuffer(connection, &buf);
@@ -411,10 +450,13 @@ UA_SecureChannel_sendAsymmetricOPNMessage(UA_SecureChannel *channel, UA_UInt32 r
 static UA_UInt16
 calculatePaddingSym(const UA_SecurityPolicy *securityPolicy, const void *channelContext,
                     size_t bytesToWrite, UA_Byte *paddingSize, UA_Byte *extraPaddingSize) {
-    UA_UInt16 padding = (UA_UInt16)(securityPolicy->symmetricModule.encryptionBlockSize -
-                                    ((bytesToWrite + securityPolicy->symmetricModule.cryptoModule.
-                                        getLocalSignatureSize(securityPolicy, channelContext) + 1) %
-                                     securityPolicy->symmetricModule.encryptionBlockSize));
+
+    size_t encryptionBlockSize = securityPolicy->symmetricModule.cryptoModule.encryptionAlgorithm.
+        getLocalBlockSize(securityPolicy, channelContext);
+    size_t signatureSize = securityPolicy->symmetricModule.cryptoModule.signatureAlgorithm.
+        getLocalSignatureSize(securityPolicy, channelContext);
+
+    UA_UInt16 padding = (UA_UInt16)(encryptionBlockSize - ((bytesToWrite + signatureSize + 1) % encryptionBlockSize));
     *paddingSize = (UA_Byte)padding;
     *extraPaddingSize = (UA_Byte)(padding >> 8);
     return padding;
@@ -432,7 +474,7 @@ setBufPos(UA_MessageContext *mc) {
 
     if(channel->securityMode == UA_MESSAGESECURITYMODE_SIGN ||
        channel->securityMode == UA_MESSAGESECURITYMODE_SIGNANDENCRYPT)
-        mc->buf_end -= securityPolicy->symmetricModule.cryptoModule.
+        mc->buf_end -= securityPolicy->symmetricModule.cryptoModule.signatureAlgorithm.
             getLocalSignatureSize(securityPolicy, channel->channelContext);
 
     /* Hide a byte needed for padding */
@@ -492,7 +534,7 @@ sendSymmetricChunk(UA_MessageContext *mc) {
     size_t total_length = pre_sig_length;
     if(channel->securityMode == UA_MESSAGESECURITYMODE_SIGN ||
        channel->securityMode == UA_MESSAGESECURITYMODE_SIGNANDENCRYPT)
-        total_length += securityPolicy->symmetricModule.cryptoModule.
+        total_length += securityPolicy->symmetricModule.cryptoModule.signatureAlgorithm.
             getLocalSignatureSize(securityPolicy, channel->channelContext);
     mc->messageBuffer.length = total_length; /* For giving the buffer to the network layer */
 
@@ -528,10 +570,10 @@ sendSymmetricChunk(UA_MessageContext *mc) {
         UA_ByteString dataToSign = mc->messageBuffer;
         dataToSign.length = pre_sig_length;
         UA_ByteString signature;
-        signature.length = securityPolicy->symmetricModule.cryptoModule.
+        signature.length = securityPolicy->symmetricModule.cryptoModule.signatureAlgorithm.
             getLocalSignatureSize(securityPolicy, channel->channelContext);
         signature.data = mc->buf_pos;
-        res |= securityPolicy->symmetricModule.cryptoModule.
+        res |= securityPolicy->symmetricModule.cryptoModule.signatureAlgorithm.
             sign(securityPolicy, channel->channelContext, &dataToSign, &signature);
     }
 
@@ -540,7 +582,7 @@ sendSymmetricChunk(UA_MessageContext *mc) {
         UA_ByteString dataToEncrypt;
         dataToEncrypt.data = mc->messageBuffer.data + UA_SECUREMH_AND_SYMALGH_LENGTH;
         dataToEncrypt.length = total_length - UA_SECUREMH_AND_SYMALGH_LENGTH;
-        res |= securityPolicy->symmetricModule.cryptoModule.
+        res |= securityPolicy->symmetricModule.cryptoModule.encryptionAlgorithm.
             encrypt(securityPolicy, channel->channelContext, &dataToEncrypt);
     }
 
@@ -644,7 +686,7 @@ UA_SecureChannel_sendSymmetricMessage(UA_SecureChannel *channel, UA_UInt32 reque
                                       UA_MessageType messageType, void *payload,
                                       const UA_DataType *payloadType) {
     if(channel->connection) {
-        if (channel->connection->state == UA_CONNECTION_CLOSED)
+        if(channel->connection->state == UA_CONNECTION_CLOSED)
             return UA_STATUSCODE_BADCONNECTIONCLOSED;
     }
 
@@ -767,7 +809,7 @@ decryptChunk(UA_SecureChannel *channel, const UA_SecurityPolicyCryptoModule *cry
        messageType == UA_MESSAGETYPE_OPN) {
         UA_ByteString cipherText = {chunk->length - offset, chunk->data + offset};
         size_t sizeBeforeDecryption = cipherText.length;
-        retval = cryptoModule->decrypt(securityPolicy, channel->channelContext, &cipherText);
+        retval = cryptoModule->encryptionAlgorithm.decrypt(securityPolicy, channel->channelContext, &cipherText);
         chunkSizeAfterDecryption -= (sizeBeforeDecryption - cipherText.length);
         if(retval != UA_STATUSCODE_GOOD)
             return retval;
@@ -780,14 +822,15 @@ decryptChunk(UA_SecureChannel *channel, const UA_SecurityPolicyCryptoModule *cry
        channel->securityMode == UA_MESSAGESECURITYMODE_SIGNANDENCRYPT ||
        messageType == UA_MESSAGETYPE_OPN) {
         /* Compute the padding size */
-        sigsize = cryptoModule->getRemoteSignatureSize(securityPolicy, channel->channelContext);
+        sigsize = cryptoModule->signatureAlgorithm.getRemoteSignatureSize(securityPolicy, channel->channelContext);
 
         if(channel->securityMode == UA_MESSAGESECURITYMODE_SIGNANDENCRYPT ||
            (messageType == UA_MESSAGETYPE_OPN &&
             channel->securityMode > UA_MESSAGESECURITYMODE_NONE)) {
             paddingSize = (size_t)chunk->data[chunkSizeAfterDecryption - sigsize - 1];
 
-            size_t keyLength = cryptoModule->getRemoteEncryptionKeyLength(securityPolicy, channel->channelContext);
+            size_t keyLength =
+                cryptoModule->encryptionAlgorithm.getRemoteKeyLength(securityPolicy, channel->channelContext);
             if(keyLength > 2048) {
                 paddingSize <<= 8; /* Extra padding size */
                 paddingSize += chunk->data[chunkSizeAfterDecryption - sigsize - 2];
@@ -805,8 +848,8 @@ decryptChunk(UA_SecureChannel *channel, const UA_SecurityPolicyCryptoModule *cry
         /* Verify the signature */
         const UA_ByteString chunkDataToVerify = {chunkSizeAfterDecryption - sigsize, chunk->data};
         const UA_ByteString signature = {sigsize, chunk->data + chunkSizeAfterDecryption - sigsize};
-        retval = cryptoModule->verify(securityPolicy, channel->channelContext,
-                                      &chunkDataToVerify, &signature);
+        retval = cryptoModule->signatureAlgorithm.verify(securityPolicy, channel->channelContext,
+                                                         &chunkDataToVerify, &signature);
 #ifdef UA_ENABLE_UNIT_TEST_FAILURE_HOOKS
         retval |= decrypt_verifySignatureFailure;
 #endif
@@ -1020,3 +1063,23 @@ UA_SecureChannel_processChunk(UA_SecureChannel *channel, UA_ByteString *chunk,
     }
     return retval;
 }
+
+/* Functionality used by both the SecureChannel and the SecurityPolicy */
+
+size_t
+UA_SecurityPolicy_getRemoteAsymEncryptionBufferLengthOverhead(const UA_SecurityPolicy *securityPolicy,
+                                                              const void *channelContext,
+                                                              size_t maxEncryptionLength) {
+    if(maxEncryptionLength == 0)
+        return 0;
+
+    size_t plainTextBlockSize = securityPolicy->asymmetricModule.cryptoModule.encryptionAlgorithm.
+        getRemotePlainTextBlockSize(securityPolicy, channelContext);
+    size_t encryptedBlockSize = securityPolicy->asymmetricModule.cryptoModule.encryptionAlgorithm.
+        getRemoteBlockSize(securityPolicy, channelContext);
+    if(plainTextBlockSize == 0)
+        return 0;
+
+    size_t maxNumberOfBlocks = maxEncryptionLength / plainTextBlockSize;
+    return maxNumberOfBlocks * (encryptedBlockSize - plainTextBlockSize);
+}

+ 4 - 9
src/ua_securechannel.h

@@ -92,16 +92,11 @@ void UA_SecureChannel_deleteMembersCleanup(UA_SecureChannel *channel);
 UA_StatusCode
 UA_SecureChannel_generateNewKeys(UA_SecureChannel* channel);
 
-/* Wrapper function for generating nonces for the supplied channel. Uses the
- * random generator of the channels security policy to allocate and generate a
- * nonce with the specified length.
- *
- * @param channel the channel to use.
- * @param nonceLength the length of the nonce to be generated.
- * @param nonce will contain the nonce after being successfully called. */
+/* Wrapper function for generating a local nonce for the supplied channel. Uses
+ * the random generator of the channels security policy to allocate and generate
+ * a nonce with the specified length. */
 UA_StatusCode
-UA_SecureChannel_generateNonce(const UA_SecureChannel *channel,
-                               size_t nonceLength, UA_ByteString *nonce);
+UA_SecureChannel_generateLocalNonce(UA_SecureChannel *channel);
 
 UA_SessionHeader *
 UA_SecureChannel_getSession(UA_SecureChannel *channel,

+ 2 - 1
tests/CMakeLists.txt

@@ -38,7 +38,8 @@ set(test_plugin_sources ${PROJECT_SOURCE_DIR}/plugins/ua_network_tcp.c
                         ${PROJECT_SOURCE_DIR}/tests/testing-plugins/testing_clock.c
                         ${PROJECT_SOURCE_DIR}/plugins/ua_securitypolicy_none.c
                         ${PROJECT_SOURCE_DIR}/tests/testing-plugins/testing_policy.c
-                        ${PROJECT_SOURCE_DIR}/tests/testing-plugins/testing_networklayers.c)
+                        ${PROJECT_SOURCE_DIR}/tests/testing-plugins/testing_networklayers.c
+)
 
 if(UA_ENABLE_ENCRYPTION)
     set(test_plugin_sources ${test_plugin_sources}

+ 258 - 268
tests/check_securechannel.c

@@ -28,6 +28,7 @@
 #define DEFAULT_ASYM_REMOTE_SIGNATURE_SIZE 7
 #define DEFAULT_ASYM_LOCAL_SIGNATURE_SIZE 11
 #define DEFAULT_ASYM_REMOTE_PLAINTEXT_BLOCKSIZE 256
+#define DEFAULT_ASYM_REMOTE_BLOCKSIZE 256
 
 UA_SecureChannel testChannel;
 UA_ByteString dummyCertificate = UA_BYTESTRING_STATIC("DUMMY CERTIFICATE DUMMY CERTIFICATE DUMMY CERTIFICATE");
@@ -78,6 +79,7 @@ setup_key_sizes(void) {
     keySizes.asym_rmt_sig_size = DEFAULT_ASYM_REMOTE_SIGNATURE_SIZE;
 
     keySizes.asym_rmt_ptext_blocksize = DEFAULT_ASYM_REMOTE_PLAINTEXT_BLOCKSIZE;
+    keySizes.asym_rmt_blocksize = DEFAULT_ASYM_REMOTE_BLOCKSIZE;
     keySizes.asym_rmt_enc_key_size = 2048;
     keySizes.asym_lcl_enc_key_size = 1024;
 }
@@ -87,60 +89,63 @@ teardown_key_sizes(void) {
     memset(&keySizes, 0, sizeof(struct key_sizes));
 }
 
-START_TEST(SecureChannel_initAndDelete) {
-    TestingPolicy(&dummyPolicy, dummyCertificate, &fCalled, &keySizes);
-    UA_StatusCode retval;
-
-    UA_SecureChannel channel;
-    retval = UA_SecureChannel_init(&channel, &dummyPolicy, &dummyCertificate);
-
-    ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected StatusCode to be good");
-    ck_assert_msg(channel.state == UA_SECURECHANNELSTATE_FRESH, "Expected state to be fresh");
-    ck_assert_msg(fCalled.newContext, "Expected newContext to have been called");
-    ck_assert_msg(fCalled.makeCertificateThumbprint, "Expected makeCertificateThumbprint to have been called");
-    ck_assert_msg(channel.securityPolicy == &dummyPolicy, "SecurityPolicy not set correctly");
-
-    UA_SecureChannel_deleteMembersCleanup(&channel);
-    ck_assert_msg(fCalled.deleteContext, "Expected deleteContext to have been called");
-
-    dummyPolicy.deleteMembers(&dummyPolicy);
-} END_TEST
-
-START_TEST(SecureChannel_generateNewKeys) {
-    UA_StatusCode retval = UA_SecureChannel_generateNewKeys(&testChannel);
-
-    ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected Statuscode to be good");
-    ck_assert_msg(fCalled.generateKey, "Expected generateKey to have been called");
-    ck_assert_msg(fCalled.setLocalSymEncryptingKey, "Expected setLocalSymEncryptingKey to have been called");
-    ck_assert_msg(fCalled.setLocalSymSigningKey, "Expected setLocalSymSigningKey to have been called");
-    ck_assert_msg(fCalled.setLocalSymIv, "Expected setLocalSymIv to have been called");
-    ck_assert_msg(fCalled.setRemoteSymEncryptingKey, "Expected setRemoteSymEncryptingKey to have been called");
-    ck_assert_msg(fCalled.setRemoteSymSigningKey, "Expected setRemoteSymSigningKey to have been called");
-    ck_assert_msg(fCalled.setRemoteSymIv, "Expected setRemoteSymIv to have been called");
-} END_TEST
-
-START_TEST(SecureChannel_revolveTokens) {
-    // Fake that no token was issued by setting 0
-    testChannel.nextSecurityToken.tokenId = 0;
-    UA_StatusCode retval = UA_SecureChannel_revolveTokens(&testChannel);
-    ck_assert_msg(retval == UA_STATUSCODE_BADSECURECHANNELTOKENUNKNOWN,
-                  "Expected failure because tokenId 0 signifies that no token was issued");
-
-    // Fake an issued token by setting an id
-    testChannel.nextSecurityToken.tokenId = 10;
-    retval = UA_SecureChannel_revolveTokens(&testChannel);
-    ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected function to return GOOD");
-    ck_assert_msg(fCalled.generateKey,
-                  "Expected generateKey to be called because new keys need to be generated,"
-                  "when switching to the next token.");
-
-    UA_ChannelSecurityToken testToken;
-    UA_ChannelSecurityToken_init(&testToken);
-
-    ck_assert_msg(memcmp(&testChannel.nextSecurityToken, &testToken, sizeof(UA_ChannelSecurityToken)) == 0,
-                  "Expected the next securityToken to be freshly initialized");
-    ck_assert_msg(testChannel.securityToken.tokenId == 10, "Expected token to have been copied");
-} END_TEST
+START_TEST(SecureChannel_initAndDelete)
+    {
+        TestingPolicy(&dummyPolicy, dummyCertificate, &fCalled, &keySizes);
+        UA_StatusCode retval;
+
+        UA_SecureChannel channel;
+        retval = UA_SecureChannel_init(&channel, &dummyPolicy, &dummyCertificate);
+
+        ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected StatusCode to be good");
+        ck_assert_msg(channel.state == UA_SECURECHANNELSTATE_FRESH, "Expected state to be fresh");
+        ck_assert_msg(fCalled.newContext, "Expected newContext to have been called");
+        ck_assert_msg(fCalled.makeCertificateThumbprint, "Expected makeCertificateThumbprint to have been called");
+        ck_assert_msg(channel.securityPolicy == &dummyPolicy, "SecurityPolicy not set correctly");
+
+        UA_SecureChannel_deleteMembersCleanup(&channel);
+        ck_assert_msg(fCalled.deleteContext, "Expected deleteContext to have been called");
+
+        dummyPolicy.deleteMembers(&dummyPolicy);
+    }END_TEST
+
+START_TEST(SecureChannel_generateNewKeys)
+    {
+        UA_StatusCode retval = UA_SecureChannel_generateNewKeys(&testChannel);
+
+        ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected Statuscode to be good");
+        ck_assert_msg(fCalled.generateKey, "Expected generateKey to have been called");
+        ck_assert_msg(fCalled.setLocalSymEncryptingKey, "Expected setLocalSymEncryptingKey to have been called");
+        ck_assert_msg(fCalled.setLocalSymSigningKey, "Expected setLocalSymSigningKey to have been called");
+        ck_assert_msg(fCalled.setLocalSymIv, "Expected setLocalSymIv to have been called");
+        ck_assert_msg(fCalled.setRemoteSymEncryptingKey, "Expected setRemoteSymEncryptingKey to have been called");
+        ck_assert_msg(fCalled.setRemoteSymSigningKey, "Expected setRemoteSymSigningKey to have been called");
+        ck_assert_msg(fCalled.setRemoteSymIv, "Expected setRemoteSymIv to have been called");
+    }END_TEST
+
+START_TEST(SecureChannel_revolveTokens)
+    {
+        // Fake that no token was issued by setting 0
+        testChannel.nextSecurityToken.tokenId = 0;
+        UA_StatusCode retval = UA_SecureChannel_revolveTokens(&testChannel);
+        ck_assert_msg(retval == UA_STATUSCODE_BADSECURECHANNELTOKENUNKNOWN,
+                      "Expected failure because tokenId 0 signifies that no token was issued");
+
+        // Fake an issued token by setting an id
+        testChannel.nextSecurityToken.tokenId = 10;
+        retval = UA_SecureChannel_revolveTokens(&testChannel);
+        ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected function to return GOOD");
+        ck_assert_msg(fCalled.generateKey,
+                      "Expected generateKey to be called because new keys need to be generated,"
+                          "when switching to the next token.");
+
+        UA_ChannelSecurityToken testToken;
+        UA_ChannelSecurityToken_init(&testToken);
+
+        ck_assert_msg(memcmp(&testChannel.nextSecurityToken, &testToken, sizeof(UA_ChannelSecurityToken)) == 0,
+                      "Expected the next securityToken to be freshly initialized");
+        ck_assert_msg(testChannel.securityToken.tokenId == 10, "Expected token to have been copied");
+    }END_TEST
 
 static void
 createDummyResponse(UA_OpenSecureChannelResponse *response) {
@@ -148,22 +153,24 @@ createDummyResponse(UA_OpenSecureChannelResponse *response) {
     memset(response, 0, sizeof(UA_OpenSecureChannelResponse));
 }
 
-START_TEST(SecureChannel_sendAsymmetricOPNMessage_withoutConnection) {
-    UA_OpenSecureChannelResponse dummyResponse;
-    createDummyResponse(&dummyResponse);
-    testChannel.securityMode = UA_MESSAGESECURITYMODE_NONE;
+START_TEST(SecureChannel_sendAsymmetricOPNMessage_withoutConnection)
+    {
+        UA_OpenSecureChannelResponse dummyResponse;
+        createDummyResponse(&dummyResponse);
+        testChannel.securityMode = UA_MESSAGESECURITYMODE_NONE;
 
-    // Remove connection to provoke error
-    UA_Connection_detachSecureChannel(testChannel.connection);
-    testChannel.connection = NULL;
+        // Remove connection to provoke error
+        UA_Connection_detachSecureChannel(testChannel.connection);
+        testChannel.connection = NULL;
 
-    UA_StatusCode retval = UA_SecureChannel_sendAsymmetricOPNMessage(&testChannel, 42, &dummyResponse,
-                                                                     &UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE]);
+        UA_StatusCode retval = UA_SecureChannel_sendAsymmetricOPNMessage(&testChannel, 42, &dummyResponse,
+                                                                         &UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE]);
 
-    ck_assert_msg(retval != UA_STATUSCODE_GOOD, "Expected failure without a connection");
-} END_TEST
+        ck_assert_msg(retval != UA_STATUSCODE_GOOD, "Expected failure without a connection");
+    }END_TEST
 
-START_TEST(SecureChannel_sendAsymmetricOPNMessage_invalidParameters) {
+START_TEST(SecureChannel_sendAsymmetricOPNMessage_invalidParameters)
+    {
         UA_OpenSecureChannelResponse dummyResponse;
         createDummyResponse(&dummyResponse);
 
@@ -174,220 +181,210 @@ START_TEST(SecureChannel_sendAsymmetricOPNMessage_invalidParameters) {
         retval = UA_SecureChannel_sendAsymmetricOPNMessage(&testChannel, 42, &dummyResponse, NULL);
         ck_assert_msg(retval != UA_STATUSCODE_GOOD, "Expected failure");
 
-} END_TEST
+    }END_TEST
 
-START_TEST(SecureChannel_sendAsymmetricOPNMessage_SecurityModeInvalid) {
-    // Configure our channel correctly for OPN messages and setup dummy message
-    UA_OpenSecureChannelResponse dummyResponse;
-    createDummyResponse(&dummyResponse);
+START_TEST(SecureChannel_sendAsymmetricOPNMessage_SecurityModeInvalid)
+    {
+        // Configure our channel correctly for OPN messages and setup dummy message
+        UA_OpenSecureChannelResponse dummyResponse;
+        createDummyResponse(&dummyResponse);
 
-    testChannel.securityMode = UA_MESSAGESECURITYMODE_INVALID;
+        testChannel.securityMode = UA_MESSAGESECURITYMODE_INVALID;
 
-    UA_StatusCode retval = UA_SecureChannel_sendAsymmetricOPNMessage(&testChannel, 42, &dummyResponse,
-                                                                     &UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE]);
-    ck_assert_msg(retval == UA_STATUSCODE_BADSECURITYMODEREJECTED, "Expected SecurityMode rejected error");
-}
+        UA_StatusCode retval = UA_SecureChannel_sendAsymmetricOPNMessage(&testChannel, 42, &dummyResponse,
+                                                                         &UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE]);
+        ck_assert_msg(retval == UA_STATUSCODE_BADSECURITYMODEREJECTED, "Expected SecurityMode rejected error");
+    }
 END_TEST
 
-START_TEST(SecureChannel_sendAsymmetricOPNMessage_SecurityModeNone) {
-    // Configure our channel correctly for OPN messages and setup dummy message
-    UA_OpenSecureChannelResponse dummyResponse;
-    createDummyResponse(&dummyResponse);
-    testChannel.securityMode = UA_MESSAGESECURITYMODE_NONE;
-
-    UA_StatusCode retval = UA_SecureChannel_sendAsymmetricOPNMessage(&testChannel, 42, &dummyResponse,
-                                                                     &UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE]);
-    ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected function to succeed");
-    ck_assert_msg(!fCalled.asym_enc, "Message encryption was called but should not have been");
-    ck_assert_msg(!fCalled.asym_sign, "Message signing was called but should not have been");
-}
-END_TEST
+START_TEST(SecureChannel_sendAsymmetricOPNMessage_SecurityModeNone)
+    {
+        // Configure our channel correctly for OPN messages and setup dummy message
+        UA_OpenSecureChannelResponse dummyResponse;
+        createDummyResponse(&dummyResponse);
+        testChannel.securityMode = UA_MESSAGESECURITYMODE_NONE;
 
-START_TEST(SecureChannel_sendAsymmetricOPNMessage_SecurityModeSign) {
-    // Configure our channel correctly for OPN messages and setup dummy message
-    UA_OpenSecureChannelResponse dummyResponse;
-    createDummyResponse(&dummyResponse);
-    testChannel.securityMode = UA_MESSAGESECURITYMODE_SIGN;
-
-    UA_StatusCode retval = UA_SecureChannel_sendAsymmetricOPNMessage(&testChannel, 42, &dummyResponse,
-                                                                     &UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE]);
-    ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected function to succeed");
-    ck_assert_msg(fCalled.asym_enc, "Expected message to have been encrypted but it was not");
-    ck_assert_msg(fCalled.asym_sign, "Expected message to have been signed but it was not");
-} END_TEST
-
-START_TEST(SecureChannel_sendAsymmetricOPNMessage_SecurityModeSignAndEncrypt) {
-    // Configure our channel correctly for OPN messages and setup dummy message
-    UA_OpenSecureChannelResponse dummyResponse;
-    createDummyResponse(&dummyResponse);
-
-    testChannel.securityMode = UA_MESSAGESECURITYMODE_SIGNANDENCRYPT;
-    UA_StatusCode retval = UA_SecureChannel_sendAsymmetricOPNMessage(&testChannel, 42, &dummyResponse,
-                                                                     &UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE]);
-    ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected function to succeed");
-    ck_assert_msg(fCalled.asym_enc, "Expected message to have been encrypted but it was not");
-    ck_assert_msg(fCalled.asym_sign, "Expected message to have been signed but it was not");
-} END_TEST
-
-START_TEST(SecureChannel_sendAsymmetricOPNMessage_sentDataIsValid) {
-    UA_OpenSecureChannelResponse dummyResponse;
-    createDummyResponse(&dummyResponse);
-
-    testChannel.securityMode = UA_MESSAGESECURITYMODE_SIGNANDENCRYPT;
-    UA_UInt32 requestId = UA_UInt32_random();
-
-    UA_StatusCode retval = UA_SecureChannel_sendAsymmetricOPNMessage(&testChannel, requestId, &dummyResponse,
-                                                                     &UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE]);
-    ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected function to succeed");
-
-    size_t offset = 0;
-    UA_SecureConversationMessageHeader header;
-    UA_SecureConversationMessageHeader_decodeBinary(&sentData, &offset, &header);
-
-    UA_AsymmetricAlgorithmSecurityHeader asymSecurityHeader;
-    UA_AsymmetricAlgorithmSecurityHeader_decodeBinary(&sentData, &offset, &asymSecurityHeader);
-    ck_assert_msg(UA_ByteString_equal(&dummyCertificate, &asymSecurityHeader.senderCertificate),
-                  "Expected the certificate to be equal to the one used  by the secureChannel");
-    ck_assert_msg(UA_ByteString_equal(&testChannel.securityPolicy->policyUri,
-                                      &asymSecurityHeader.securityPolicyUri),
-                  "Expected securityPolicyUri to be equal to the one used by the secureChannel");
-    UA_ByteString thumbPrint = {20, testChannel.remoteCertificateThumbprint};
-    ck_assert_msg(UA_ByteString_equal(&thumbPrint,
-                                      &asymSecurityHeader.receiverCertificateThumbprint),
-                  "Expected receiverCertificateThumbprint to be equal to the one set in the secureChannel");
-
-    for(size_t i = offset; i < header.messageHeader.messageSize; ++i) {
-        sentData.data[i] = (UA_Byte) ((sentData.data[i] - 1) % (UA_BYTE_MAX + 1));
+        UA_StatusCode retval = UA_SecureChannel_sendAsymmetricOPNMessage(&testChannel, 42, &dummyResponse,
+                                                                         &UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE]);
+        ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected function to succeed");
+        ck_assert_msg(!fCalled.asym_enc, "Message encryption was called but should not have been");
+        ck_assert_msg(!fCalled.asym_sign, "Message signing was called but should not have been");
     }
+END_TEST
 
-    UA_SequenceHeader sequenceHeader;
-    UA_SequenceHeader_decodeBinary(&sentData, &offset, &sequenceHeader);
-    ck_assert_msg(sequenceHeader.requestId == requestId, "Expected requestId to be %i but was %i",
-                  requestId,
-                  sequenceHeader.requestId);
+START_TEST(SecureChannel_sendAsymmetricOPNMessage_SecurityModeSign)
+    {
+        // Configure our channel correctly for OPN messages and setup dummy message
+        UA_OpenSecureChannelResponse dummyResponse;
+        createDummyResponse(&dummyResponse);
+        testChannel.securityMode = UA_MESSAGESECURITYMODE_SIGN;
 
-    UA_NodeId original = UA_NODEID_NUMERIC(0, UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE].binaryEncodingId);
-    UA_NodeId requestTypeId;
-    UA_NodeId_decodeBinary(&sentData, &offset, &requestTypeId);
-    ck_assert_msg(UA_NodeId_equal(&original, &requestTypeId), "Expected nodeIds to be equal");
+        UA_StatusCode retval = UA_SecureChannel_sendAsymmetricOPNMessage(&testChannel, 42, &dummyResponse,
+                                                                         &UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE]);
+        ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected function to succeed");
+        ck_assert_msg(fCalled.asym_enc, "Expected message to have been encrypted but it was not");
+        ck_assert_msg(fCalled.asym_sign, "Expected message to have been signed but it was not");
+    }END_TEST
+
+START_TEST(SecureChannel_sendAsymmetricOPNMessage_SecurityModeSignAndEncrypt)
+    {
+        // Configure our channel correctly for OPN messages and setup dummy message
+        UA_OpenSecureChannelResponse dummyResponse;
+        createDummyResponse(&dummyResponse);
 
-    UA_OpenSecureChannelResponse sentResponse;
-    UA_OpenSecureChannelResponse_decodeBinary(&sentData, &offset, &sentResponse);
+        testChannel.securityMode = UA_MESSAGESECURITYMODE_SIGNANDENCRYPT;
+        UA_StatusCode retval = UA_SecureChannel_sendAsymmetricOPNMessage(&testChannel, 42, &dummyResponse,
+                                                                         &UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE]);
+        ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected function to succeed");
+        ck_assert_msg(fCalled.asym_enc, "Expected message to have been encrypted but it was not");
+        ck_assert_msg(fCalled.asym_sign, "Expected message to have been signed but it was not");
+    }END_TEST
 
-    ck_assert_msg(memcmp(&sentResponse, &dummyResponse, sizeof(UA_OpenSecureChannelResponse)) == 0,
-                  "Expected the sent response to be equal to the one supplied to the send function");
+START_TEST(SecureChannel_sendAsymmetricOPNMessage_sentDataIsValid)
+    {
+        UA_OpenSecureChannelResponse dummyResponse;
+        createDummyResponse(&dummyResponse);
 
-    UA_Byte paddingByte = sentData.data[offset];
-    size_t paddingSize = (size_t) paddingByte;
+        testChannel.securityMode = UA_MESSAGESECURITYMODE_SIGNANDENCRYPT;
+        UA_UInt32 requestId = UA_UInt32_random();
 
-    for(size_t i = 0; i <= paddingSize; ++i) {
-        ck_assert_msg(sentData.data[offset + i] == paddingByte,
-                      "Expected padding byte %i to be %i but got value %i",
-                      i, paddingByte, sentData.data[offset + i]);
+        UA_StatusCode retval = UA_SecureChannel_sendAsymmetricOPNMessage(&testChannel, requestId, &dummyResponse,
+                                                                         &UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE]);
+        ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected function to succeed");
+
+        size_t offset = 0;
+        UA_SecureConversationMessageHeader header;
+        UA_SecureConversationMessageHeader_decodeBinary(&sentData, &offset, &header);
+
+        UA_AsymmetricAlgorithmSecurityHeader asymSecurityHeader;
+        UA_AsymmetricAlgorithmSecurityHeader_decodeBinary(&sentData, &offset, &asymSecurityHeader);
+        ck_assert_msg(UA_ByteString_equal(&dummyCertificate, &asymSecurityHeader.senderCertificate),
+                      "Expected the certificate to be equal to the one used  by the secureChannel");
+        ck_assert_msg(UA_ByteString_equal(&testChannel.securityPolicy->policyUri,
+                                          &asymSecurityHeader.securityPolicyUri),
+                      "Expected securityPolicyUri to be equal to the one used by the secureChannel");
+        UA_ByteString thumbPrint = {20, testChannel.remoteCertificateThumbprint};
+        ck_assert_msg(UA_ByteString_equal(&thumbPrint,
+                                          &asymSecurityHeader.receiverCertificateThumbprint),
+                      "Expected receiverCertificateThumbprint to be equal to the one set in the secureChannel");
+
+        for(size_t i = offset; i < header.messageHeader.messageSize; ++i) {
+            sentData.data[i] = (UA_Byte)((sentData.data[i] - 1) % (UA_BYTE_MAX + 1));
+        }
+
+        UA_SequenceHeader sequenceHeader;
+        UA_SequenceHeader_decodeBinary(&sentData, &offset, &sequenceHeader);
+        ck_assert_msg(sequenceHeader.requestId == requestId, "Expected requestId to be %i but was %i",
+                      requestId,
+                      sequenceHeader.requestId);
+
+        UA_NodeId original = UA_NODEID_NUMERIC(0, UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE].binaryEncodingId);
+        UA_NodeId requestTypeId;
+        UA_NodeId_decodeBinary(&sentData, &offset, &requestTypeId);
+        ck_assert_msg(UA_NodeId_equal(&original, &requestTypeId), "Expected nodeIds to be equal");
+
+        UA_OpenSecureChannelResponse sentResponse;
+        UA_OpenSecureChannelResponse_decodeBinary(&sentData, &offset, &sentResponse);
+
+        ck_assert_msg(memcmp(&sentResponse, &dummyResponse, sizeof(UA_OpenSecureChannelResponse)) == 0,
+                      "Expected the sent response to be equal to the one supplied to the send function");
+
+        UA_Byte paddingByte = sentData.data[offset];
+        size_t paddingSize = (size_t)paddingByte;
+
+        for(size_t i = 0; i <= paddingSize; ++i) {
+            ck_assert_msg(sentData.data[offset + i] == paddingByte,
+                          "Expected padding byte %i to be %i but got value %i",
+                          i, paddingByte, sentData.data[offset + i]);
+        }
+
+        ck_assert_msg(sentData.data[offset + paddingSize + 1] == '*', "Expected first byte of signature");
+
+        UA_SecureConversationMessageHeader_deleteMembers(&header);
+        UA_AsymmetricAlgorithmSecurityHeader_deleteMembers(&asymSecurityHeader);
+        UA_SequenceHeader_deleteMembers(&sequenceHeader);
+        UA_OpenSecureChannelResponse_deleteMembers(&sentResponse);
     }
-
-    ck_assert_msg(sentData.data[offset + paddingSize + 1] == '*', "Expected first byte of signature");
-
-    UA_SecureConversationMessageHeader_deleteMembers(&header);
-    UA_AsymmetricAlgorithmSecurityHeader_deleteMembers(&asymSecurityHeader);
-    UA_SequenceHeader_deleteMembers(&sequenceHeader);
-    UA_OpenSecureChannelResponse_deleteMembers(&sentResponse);
-}
 END_TEST
 
-START_TEST(Securechannel_sendAsymmetricOPNMessage_extraPaddingPresentWhenKeyLargerThan2048Bits) {
-    keySizes.asym_rmt_enc_key_size = 4096;
-    keySizes.asym_rmt_ptext_blocksize = 4096;
-
-    UA_OpenSecureChannelResponse dummyResponse;
-    createDummyResponse(&dummyResponse);
-
-    testChannel.securityMode = UA_MESSAGESECURITYMODE_SIGNANDENCRYPT;
-    UA_UInt32 requestId = UA_UInt32_random();
-
-    UA_StatusCode retval = UA_SecureChannel_sendAsymmetricOPNMessage(&testChannel, requestId, &dummyResponse,
-                                                                     &UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE]);
-    ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected function to succeed");
-
-    size_t offset = 0;
-    UA_SecureConversationMessageHeader header;
-    UA_SecureConversationMessageHeader_decodeBinary(&sentData, &offset, &header);
-
-    UA_AsymmetricAlgorithmSecurityHeader asymSecurityHeader;
-    UA_AsymmetricAlgorithmSecurityHeader_decodeBinary(&sentData, &offset, &asymSecurityHeader);
-    ck_assert_msg(UA_ByteString_equal(&dummyCertificate, &asymSecurityHeader.senderCertificate),
-                  "Expected the certificate to be equal to the one used  by the secureChannel");
-    ck_assert_msg(UA_ByteString_equal(&testChannel.securityPolicy->policyUri,
-                                      &asymSecurityHeader.securityPolicyUri),
-                  "Expected securityPolicyUri to be equal to the one used by the secureChannel");
-    UA_ByteString thumbPrint = {20, testChannel.remoteCertificateThumbprint};
-    ck_assert_msg(UA_ByteString_equal(&thumbPrint,
-                                      &asymSecurityHeader.receiverCertificateThumbprint),
-                  "Expected receiverCertificateThumbprint to be equal to the one set in the secureChannel");
-
-    for(size_t i = offset; i < header.messageHeader.messageSize; ++i) {
-        sentData.data[i] = (UA_Byte) ((sentData.data[i] - 1) % (UA_BYTE_MAX + 1));
-    }
-
-    UA_SequenceHeader sequenceHeader;
-    UA_SequenceHeader_decodeBinary(&sentData, &offset, &sequenceHeader);
-    ck_assert_msg(sequenceHeader.requestId == requestId, "Expected requestId to be %i but was %i",
-                  requestId,
-                  sequenceHeader.requestId);
-
-    UA_NodeId original = UA_NODEID_NUMERIC(0, UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE].binaryEncodingId);
-    UA_NodeId requestTypeId;
-    UA_NodeId_decodeBinary(&sentData, &offset, &requestTypeId);
-    ck_assert_msg(UA_NodeId_equal(&original, &requestTypeId), "Expected nodeIds to be equal");
-
-    UA_OpenSecureChannelResponse sentResponse;
-    UA_OpenSecureChannelResponse_decodeBinary(&sentData, &offset, &sentResponse);
-
-    ck_assert_msg(memcmp(&sentResponse, &dummyResponse, sizeof(UA_OpenSecureChannelResponse)) == 0,
-                  "Expected the sent response to be equal to the one supplied to the send function");
-
-    UA_Byte paddingByte = sentData.data[offset];
-    UA_Byte extraPaddingByte = sentData.data[sentData.length - keySizes.asym_lcl_sig_size - 1];
-    size_t paddingSize = (size_t) paddingByte;
-    paddingSize |= extraPaddingByte << 8;
-
-    for(size_t i = 0; i <= paddingSize; ++i) {
-        ck_assert_msg(sentData.data[offset + i] == paddingByte,
-                      "Expected padding byte %i to be %i but got value %i",
-                      i,
-                      paddingByte,
-                      sentData.data[offset + i]);
-    }
-
-    ck_assert_msg(sentData.data[offset + paddingSize + 1] == extraPaddingByte,
-                  "Expected extra padding byte to be %i but got %i",
-                  extraPaddingByte, sentData.data[offset + paddingSize + 1]);
-    ck_assert_msg(sentData.data[offset + paddingSize + 2] == '*',
-                  "Expected first byte 42 of signature but got %i",
-                  sentData.data[offset + paddingSize + 2]);
-
-    UA_SecureConversationMessageHeader_deleteMembers(&header);
-    UA_AsymmetricAlgorithmSecurityHeader_deleteMembers(&asymSecurityHeader);
-    UA_SequenceHeader_deleteMembers(&sequenceHeader);
-    UA_OpenSecureChannelResponse_deleteMembers(&sentResponse);
-} END_TEST
-
-START_TEST(SecureChannel_generateNonce) {
-    UA_ByteString myNonce;
-    UA_ByteString_init(&myNonce);
-
-    for(size_t i = 0; i < 129; ++i) {
-        i = (i == 128) ? 65536 : i; // large edge case
+START_TEST(Securechannel_sendAsymmetricOPNMessage_extraPaddingPresentWhenKeyLargerThan2048Bits)
+    {
+        keySizes.asym_rmt_enc_key_size = 4096;
+        keySizes.asym_rmt_blocksize = 4096;
+        keySizes.asym_rmt_ptext_blocksize = 4096;
 
-        UA_StatusCode retval = UA_SecureChannel_generateNonce(&testChannel, i, &myNonce);
+        UA_OpenSecureChannelResponse dummyResponse;
+        createDummyResponse(&dummyResponse);
 
-        ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected retval to be good");
-        ck_assert_msg(myNonce.length == i, "Expected nonce length to be %i but was %i", i, myNonce.length);
-        ck_assert_msg(fCalled.generateNonce, "Expected generateNonce to have been called");
-    }
+        testChannel.securityMode = UA_MESSAGESECURITYMODE_SIGNANDENCRYPT;
+        UA_UInt32 requestId = UA_UInt32_random();
 
-    UA_ByteString_deleteMembers(&myNonce);
-} END_TEST
+        UA_StatusCode retval = UA_SecureChannel_sendAsymmetricOPNMessage(&testChannel, requestId, &dummyResponse,
+                                                                         &UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE]);
+        ck_assert_msg(retval == UA_STATUSCODE_GOOD, "Expected function to succeed");
+
+        size_t offset = 0;
+        UA_SecureConversationMessageHeader header;
+        UA_SecureConversationMessageHeader_decodeBinary(&sentData, &offset, &header);
+
+        UA_AsymmetricAlgorithmSecurityHeader asymSecurityHeader;
+        UA_AsymmetricAlgorithmSecurityHeader_decodeBinary(&sentData, &offset, &asymSecurityHeader);
+        ck_assert_msg(UA_ByteString_equal(&dummyCertificate, &asymSecurityHeader.senderCertificate),
+                      "Expected the certificate to be equal to the one used  by the secureChannel");
+        ck_assert_msg(UA_ByteString_equal(&testChannel.securityPolicy->policyUri,
+                                          &asymSecurityHeader.securityPolicyUri),
+                      "Expected securityPolicyUri to be equal to the one used by the secureChannel");
+        UA_ByteString thumbPrint = {20, testChannel.remoteCertificateThumbprint};
+        ck_assert_msg(UA_ByteString_equal(&thumbPrint,
+                                          &asymSecurityHeader.receiverCertificateThumbprint),
+                      "Expected receiverCertificateThumbprint to be equal to the one set in the secureChannel");
+
+        for(size_t i = offset; i < header.messageHeader.messageSize; ++i) {
+            sentData.data[i] = (UA_Byte)((sentData.data[i] - 1) % (UA_BYTE_MAX + 1));
+        }
+
+        UA_SequenceHeader sequenceHeader;
+        UA_SequenceHeader_decodeBinary(&sentData, &offset, &sequenceHeader);
+        ck_assert_msg(sequenceHeader.requestId == requestId, "Expected requestId to be %i but was %i",
+                      requestId,
+                      sequenceHeader.requestId);
+
+        UA_NodeId original = UA_NODEID_NUMERIC(0, UA_TYPES[UA_TYPES_OPENSECURECHANNELRESPONSE].binaryEncodingId);
+        UA_NodeId requestTypeId;
+        UA_NodeId_decodeBinary(&sentData, &offset, &requestTypeId);
+        ck_assert_msg(UA_NodeId_equal(&original, &requestTypeId), "Expected nodeIds to be equal");
+
+        UA_OpenSecureChannelResponse sentResponse;
+        UA_OpenSecureChannelResponse_decodeBinary(&sentData, &offset, &sentResponse);
+
+        ck_assert_msg(memcmp(&sentResponse, &dummyResponse, sizeof(UA_OpenSecureChannelResponse)) == 0,
+                      "Expected the sent response to be equal to the one supplied to the send function");
+
+        UA_Byte paddingByte = sentData.data[offset];
+        UA_Byte extraPaddingByte = sentData.data[sentData.length - keySizes.asym_lcl_sig_size - 1];
+        size_t paddingSize = (size_t)paddingByte;
+        paddingSize |= extraPaddingByte << 8;
+
+        for(size_t i = 0; i <= paddingSize; ++i) {
+            ck_assert_msg(sentData.data[offset + i] == paddingByte,
+                          "Expected padding byte %i to be %i but got value %i",
+                          i,
+                          paddingByte,
+                          sentData.data[offset + i]);
+        }
+
+        ck_assert_msg(sentData.data[offset + paddingSize + 1] == extraPaddingByte,
+                      "Expected extra padding byte to be %i but got %i",
+                      extraPaddingByte, sentData.data[offset + paddingSize + 1]);
+        ck_assert_msg(sentData.data[offset + paddingSize + 2] == '*',
+                      "Expected first byte 42 of signature but got %i",
+                      sentData.data[offset + paddingSize + 2]);
+
+        UA_SecureConversationMessageHeader_deleteMembers(&header);
+        UA_AsymmetricAlgorithmSecurityHeader_deleteMembers(&asymSecurityHeader);
+        UA_SequenceHeader_deleteMembers(&sequenceHeader);
+        UA_OpenSecureChannelResponse_deleteMembers(&sentResponse);
+    }END_TEST
 
 static Suite *
 testSuite_SecureChannel(void) {
@@ -428,13 +425,6 @@ testSuite_SecureChannel(void) {
                    Securechannel_sendAsymmetricOPNMessage_extraPaddingPresentWhenKeyLargerThan2048Bits);
     suite_add_tcase(s, tc_sendAsymmetricOPNMessage);
 
-    TCase *tc_generateNonce = tcase_create("Test generateNonce function");
-    tcase_add_checked_fixture(tc_generateNonce, setup_funcs_called, teardown_funcs_called);
-    tcase_add_checked_fixture(tc_generateNonce, setup_key_sizes, teardown_key_sizes);
-    tcase_add_checked_fixture(tc_generateNonce, setup_secureChannel, teardown_secureChannel);
-    tcase_add_test(tc_generateNonce, SecureChannel_generateNonce);
-    suite_add_tcase(s, tc_generateNonce);
-
     return s;
 }
 

+ 2 - 1
tests/fuzz/CMakeLists.txt

@@ -60,7 +60,8 @@ set(fuzzing_plugin_sources ${PROJECT_SOURCE_DIR}/plugins/ua_network_tcp.c
         ${PROJECT_SOURCE_DIR}/plugins/ua_nodestore_default.c
         ${PROJECT_SOURCE_DIR}/plugins/ua_accesscontrol_default.c
         ${PROJECT_SOURCE_DIR}/plugins/ua_pki_certificate.c
-        ${PROJECT_SOURCE_DIR}/plugins/ua_securitypolicy_none.c)
+        ${PROJECT_SOURCE_DIR}/plugins/ua_securitypolicy_none.c
+)
 
 if(UA_ENABLE_ENCRYPTION)
     set(fuzzing_plugin_sources ${fuzzing_plugin_sources}

+ 91 - 34
tests/testing-plugins/testing_policy.c

@@ -4,6 +4,7 @@
 
 #ifndef __clang_analyzer__
 
+#include <ua_plugin_securitypolicy.h>
 #include "ua_types.h"
 #include "ua_plugin_securitypolicy.h"
 #include "ua_log_stdout.h"
@@ -82,27 +83,51 @@ asym_getRemoteSignatureSize_testing(const UA_SecurityPolicy *securityPolicy,
 static size_t
 asym_getLocalEncryptionKeyLength_testing(const UA_SecurityPolicy *securityPolicy,
                                          const void *channelContext) {
+    ck_assert(securityPolicy != NULL);
+    ck_assert(channelContext != NULL);
     return keySizes->asym_lcl_enc_key_size;
 }
 
 static size_t
 asym_getRemoteEncryptionKeyLength_testing(const UA_SecurityPolicy *securityPolicy,
                                           const void *channelContext) {
+    ck_assert(securityPolicy != NULL);
+    ck_assert(channelContext != NULL);
     return keySizes->asym_rmt_enc_key_size;
 }
 
 static size_t
 sym_getLocalSignatureSize_testing(const UA_SecurityPolicy *securityPolicy,
                                   const void *channelContext) {
+    ck_assert(securityPolicy != NULL);
+    ck_assert(channelContext != NULL);
     return 0;
 }
 
 static size_t
 sym_getRemoteSignatureSize_testing(const UA_SecurityPolicy *securityPolicy,
                                    const void *channelContext) {
+    ck_assert(securityPolicy != NULL);
+    ck_assert(channelContext != NULL);
     return 0;
 }
 
+static size_t
+sym_getLocalSigningKeyLength_testing(const UA_SecurityPolicy *securityPolicy,
+                                     const void *channelContext) {
+    ck_assert(securityPolicy != NULL);
+    ck_assert(channelContext != NULL);
+    return keySizes->sym_sig_keyLen;
+}
+
+static size_t
+sym_getRemoteSigningKeyLength_testing(const UA_SecurityPolicy *securityPolicy,
+                                      const void *channelContext) {
+    ck_assert(securityPolicy != NULL);
+    ck_assert(channelContext != NULL);
+    return keySizes->sym_sig_keyLen; // TODO: Remote sig key len
+}
+
 static size_t
 sym_getLocalEncryptionKeyLength_testing(const UA_SecurityPolicy *securityPolicy,
                                         const void *channelContext) {
@@ -114,9 +139,27 @@ sym_getLocalEncryptionKeyLength_testing(const UA_SecurityPolicy *securityPolicy,
 static size_t
 sym_getRemoteEncryptionKeyLength_testing(const UA_SecurityPolicy *securityPolicy,
                                          const void *channelContext) {
+    ck_assert(securityPolicy != NULL);
+    ck_assert(channelContext != NULL);
     return keySizes->sym_enc_keyLen;
 }
 
+static size_t
+sym_getLocalEncryptionBlockSize_testing(const UA_SecurityPolicy *securityPolicy,
+                                        const void *channelContext) {
+    ck_assert(securityPolicy != NULL);
+    ck_assert(channelContext != NULL);
+    return keySizes->sym_enc_blockSize;
+}
+
+static size_t
+sym_getRemoteEncryptionBlockSize_testing(const UA_SecurityPolicy *securityPolicy,
+                                         const void *channelContext) {
+    ck_assert(securityPolicy != NULL);
+    ck_assert(channelContext != NULL);
+    return keySizes->sym_enc_blockSize; // TODO: Different size for remote
+}
+
 static UA_StatusCode
 sym_encrypt_testing(const UA_SecurityPolicy *securityPolicy,
                     void *channelContext,
@@ -137,7 +180,9 @@ asym_encrypt_testing(const UA_SecurityPolicy *securityPolicy,
     ck_assert(channelContext != NULL);
     ck_assert(data != NULL);
 
-    size_t blockSize = securityPolicy->channelModule.getRemoteAsymPlainTextBlockSize(securityPolicy);
+    size_t blockSize =
+        securityPolicy->asymmetricModule.cryptoModule.encryptionAlgorithm.getRemotePlainTextBlockSize(securityPolicy,
+                                                                                                      channelContext);
     ck_assert_msg(data->length % blockSize == 0,
                   "Expected the length of the data to be encrypted to be a multiple of the plaintext block size (%i). "
                       "Remainder was %i",
@@ -145,7 +190,7 @@ asym_encrypt_testing(const UA_SecurityPolicy *securityPolicy,
                   data->length % blockSize);
 
     for(size_t i = 0; i < data->length; ++i) {
-        data->data[i] = (UA_Byte) ((data->data[i] + 1) % (UA_BYTE_MAX + 1));
+        data->data[i] = (UA_Byte)((data->data[i] + 1) % (UA_BYTE_MAX + 1));
     }
 
     return UA_STATUSCODE_GOOD;
@@ -198,8 +243,6 @@ generateNonce_testing(const UA_SecurityPolicy *securityPolicy,
                       UA_ByteString *out) {
     ck_assert(securityPolicy != NULL);
     ck_assert(out != NULL);
-    if(out->length != 0)
-        ck_assert(out->data != NULL);
 
     memset(out->data, 'N', out->length);
 
@@ -217,7 +260,7 @@ newContext_testing(const UA_SecurityPolicy *securityPolicy,
     ck_assert(channelContext != NULL);
 
     ck_assert(funcsCalled != NULL);
-    *channelContext = (void *) funcsCalled;
+    *channelContext = (void *)funcsCalled;
     return UA_STATUSCODE_GOOD;
 }
 
@@ -312,14 +355,15 @@ setRemoteSymIv_testing(void *channelContext,
 }
 
 static size_t
-getRemoteAsymPlainTextBlockSize_testing(const void *channelContext) {
+asym_getRemotePlainTextBlockSize_testing(const UA_SecurityPolicy *securityPolicy,
+                                         const void *channelContext) {
     return keySizes->asym_rmt_ptext_blocksize;
 }
 
 static size_t
-getRemoteAsymEncryptionBufferLengthOverhead_testing(const void *channelContext,
-                                                    size_t maxEncryptionLength) {
-    return 0;
+asym_getRemoteBlockSize_testing(const UA_SecurityPolicy *securityPolicy,
+                                const void *channelContext) {
+    return keySizes->asym_rmt_blocksize;
 }
 
 static UA_StatusCode
@@ -334,11 +378,11 @@ policy_deletemembers_testing(UA_SecurityPolicy *policy) {
 }
 
 UA_StatusCode
-TestingPolicy(UA_SecurityPolicy *policy, const UA_ByteString localCertificate,
+TestingPolicy(UA_SecurityPolicy *policy, UA_ByteString localCertificate,
               funcs_called *fCalled, const key_sizes *kSizes) {
     keySizes = kSizes;
     funcsCalled = fCalled;
-    policy->policyContext = (void *) funcsCalled;
+    policy->policyContext = (void *)funcsCalled;
     policy->policyUri = UA_STRING("http://opcfoundation.org/UA/SecurityPolicy#Testing");
     policy->logger = UA_Log_Stdout;
     policy->certificateVerification = NULL;
@@ -346,29 +390,45 @@ TestingPolicy(UA_SecurityPolicy *policy, const UA_ByteString localCertificate,
 
     policy->asymmetricModule.makeCertificateThumbprint = makeThumbprint_testing;
     policy->asymmetricModule.compareCertificateThumbprint = compareThumbprint_testing;
-    policy->asymmetricModule.cryptoModule.signatureAlgorithmUri = UA_STRING_NULL;
-    policy->asymmetricModule.cryptoModule.verify = verify_testing;
-    policy->asymmetricModule.cryptoModule.sign = asym_sign_testing;
-    policy->asymmetricModule.cryptoModule.getLocalSignatureSize = asym_getLocalSignatureSize_testing;
-    policy->asymmetricModule.cryptoModule.getRemoteSignatureSize = asym_getRemoteSignatureSize_testing;
-    policy->asymmetricModule.cryptoModule.encrypt = asym_encrypt_testing;
-    policy->asymmetricModule.cryptoModule.decrypt = decrypt_testing;
-    policy->asymmetricModule.cryptoModule.getLocalEncryptionKeyLength = asym_getLocalEncryptionKeyLength_testing;
-    policy->asymmetricModule.cryptoModule.getRemoteEncryptionKeyLength = asym_getRemoteEncryptionKeyLength_testing;
+
+    UA_SecurityPolicySignatureAlgorithm *asym_signatureAlgorithm =
+        &policy->asymmetricModule.cryptoModule.signatureAlgorithm;
+    asym_signatureAlgorithm->uri = UA_STRING_NULL;
+    asym_signatureAlgorithm->verify = verify_testing;
+    asym_signatureAlgorithm->sign = asym_sign_testing;
+    asym_signatureAlgorithm->getLocalSignatureSize = asym_getLocalSignatureSize_testing;
+    asym_signatureAlgorithm->getRemoteSignatureSize = asym_getRemoteSignatureSize_testing;
+
+    UA_SecurityPolicyEncryptionAlgorithm *asym_encryptionAlgorithm =
+        &policy->asymmetricModule.cryptoModule.encryptionAlgorithm;
+    asym_encryptionAlgorithm->encrypt = asym_encrypt_testing;
+    asym_encryptionAlgorithm->decrypt = decrypt_testing;
+    asym_encryptionAlgorithm->getLocalKeyLength = asym_getLocalEncryptionKeyLength_testing;
+    asym_encryptionAlgorithm->getRemoteKeyLength = asym_getRemoteEncryptionKeyLength_testing;
+    asym_encryptionAlgorithm->getRemotePlainTextBlockSize = asym_getRemotePlainTextBlockSize_testing;
+    asym_encryptionAlgorithm->getRemoteBlockSize = asym_getRemoteBlockSize_testing;
 
     policy->symmetricModule.generateKey = generateKey_testing;
     policy->symmetricModule.generateNonce = generateNonce_testing;
-    policy->symmetricModule.cryptoModule.signatureAlgorithmUri = UA_STRING_NULL;
-    policy->symmetricModule.cryptoModule.verify = verify_testing;
-    policy->symmetricModule.cryptoModule.sign = sym_sign_testing;
-    policy->symmetricModule.cryptoModule.getLocalSignatureSize = sym_getLocalSignatureSize_testing;
-    policy->symmetricModule.cryptoModule.getRemoteSignatureSize = sym_getRemoteSignatureSize_testing;
-    policy->symmetricModule.cryptoModule.encrypt = sym_encrypt_testing;
-    policy->symmetricModule.cryptoModule.decrypt = decrypt_testing;
-    policy->symmetricModule.cryptoModule.getLocalEncryptionKeyLength = sym_getLocalEncryptionKeyLength_testing;
-    policy->symmetricModule.cryptoModule.getRemoteEncryptionKeyLength = sym_getRemoteEncryptionKeyLength_testing;
-    policy->symmetricModule.encryptionBlockSize = keySizes->sym_enc_blockSize;
-    policy->symmetricModule.signingKeyLength = keySizes->sym_sig_keyLen;
+
+    UA_SecurityPolicySignatureAlgorithm *sym_signatureAlgorithm =
+        &policy->symmetricModule.cryptoModule.signatureAlgorithm;
+    sym_signatureAlgorithm->uri = UA_STRING_NULL;
+    sym_signatureAlgorithm->verify = verify_testing;
+    sym_signatureAlgorithm->sign = sym_sign_testing;
+    sym_signatureAlgorithm->getLocalSignatureSize = sym_getLocalSignatureSize_testing;
+    sym_signatureAlgorithm->getRemoteSignatureSize = sym_getRemoteSignatureSize_testing;
+    sym_signatureAlgorithm->getLocalKeyLength = sym_getLocalSigningKeyLength_testing;
+    sym_signatureAlgorithm->getRemoteKeyLength = sym_getRemoteSigningKeyLength_testing;
+
+    UA_SecurityPolicyEncryptionAlgorithm *sym_encryptionAlgorithm =
+        &policy->symmetricModule.cryptoModule.encryptionAlgorithm;
+    sym_encryptionAlgorithm->encrypt = sym_encrypt_testing;
+    sym_encryptionAlgorithm->decrypt = decrypt_testing;
+    sym_encryptionAlgorithm->getLocalKeyLength = sym_getLocalEncryptionKeyLength_testing;
+    sym_encryptionAlgorithm->getRemoteKeyLength = sym_getRemoteEncryptionKeyLength_testing;
+    sym_encryptionAlgorithm->getLocalBlockSize = sym_getLocalEncryptionBlockSize_testing;
+    sym_encryptionAlgorithm->getRemoteBlockSize = sym_getRemoteEncryptionBlockSize_testing;
 
     policy->channelModule.newContext = newContext_testing;
     policy->channelModule.deleteContext = deleteContext_testing;
@@ -379,9 +439,6 @@ TestingPolicy(UA_SecurityPolicy *policy, const UA_ByteString localCertificate,
     policy->channelModule.setRemoteSymSigningKey = setRemoteSymSigningKey_testing;
     policy->channelModule.setRemoteSymIv = setRemoteSymIv_testing;
     policy->channelModule.compareCertificate = compareCertificate_testing;
-    policy->channelModule.getRemoteAsymPlainTextBlockSize = getRemoteAsymPlainTextBlockSize_testing;
-    policy->channelModule.getRemoteAsymEncryptionBufferLengthOverhead =
-        getRemoteAsymEncryptionBufferLengthOverhead_testing;
     policy->deleteMembers = policy_deletemembers_testing;
 
     return UA_STATUSCODE_GOOD;

+ 2 - 1
tests/testing-plugins/testing_policy.h

@@ -48,12 +48,13 @@ typedef struct key_sizes {
     size_t asym_rmt_sig_size;
     size_t asym_lcl_sig_size;
     size_t asym_rmt_ptext_blocksize;
+    size_t asym_rmt_blocksize;
     size_t asym_rmt_enc_key_size;
     size_t asym_lcl_enc_key_size;
 } key_sizes;
 
 UA_StatusCode UA_EXPORT
-TestingPolicy(UA_SecurityPolicy *policy, const UA_ByteString localCertificate,
+TestingPolicy(UA_SecurityPolicy *policy, UA_ByteString localCertificate,
               funcs_called *fCalled, const key_sizes *kSizes);
 
 #ifdef __cplusplus