Browse Source

Reduce the size of the msg chunk to fit header and footer

Before encoding the response into the send buffer space was reserved
for the header, the signature (if applicable) and 2 bytes for the
padding (if applicable). When reserving space for the header, the
end of the buffer was moved out of bounds of the send buffer instead
of reducing the size available for encoded data. 2 bytes padding is
also not enough for worst-case padding scenarios.

With this fix the end buffer position is calculated correctly when
reserving space for the message header and enough space is reserved
for the padding (if the chunk shall be encrypted).
Jonas Green 5 years ago
parent
commit
8f366f15f7
1 changed files with 19 additions and 3 deletions
  1. 19 3
      src/ua_securechannel.c

+ 19 - 3
src/ua_securechannel.c

@@ -576,16 +576,30 @@ setBufPos(UA_MessageContext *mc) {
     /* Forward the data pointer so that the payload is encoded after the
      * message header */
     mc->buf_pos = &mc->messageBuffer.data[UA_SECURE_MESSAGE_HEADER_LENGTH];
-    mc->buf_end = &mc->messageBuffer.data[mc->messageBuffer.length];
+    mc->buf_end = &mc->messageBuffer.data[mc->messageBuffer.length-UA_SECURE_MESSAGE_HEADER_LENGTH-1];
 
+    /* Reserve space for the message footer at the end of the chunk if the chunk
+     * is signed and/or encrypted. The footer includes the fields PaddingSize,
+     * Padding, ExtraPadding and Signature. The padding fields are only present
+     * if the chunk is encrypted. */
     if(channel->securityMode == UA_MESSAGESECURITYMODE_SIGN ||
        channel->securityMode == UA_MESSAGESECURITYMODE_SIGNANDENCRYPT)
         mc->buf_end -= securityPolicy->symmetricModule.cryptoModule.signatureAlgorithm.
             getLocalSignatureSize(securityPolicy, channel->channelContext);
 
-    /* Hide a byte needed for padding */
+    /* The size of the padding depends on the amount of data that shall be sent
+     * and is unknown at this point. Reserve space for the PaddingSize byte,
+     * the maximum amount of Padding which equals the block size of the
+     * symmetric encryption algorithm and last 1 byte for the ExtraPaddingSize
+     * field that is present if the encryption key is larger than 2048 bits.
+     * The actual padding size is later calculated by the function
+     * calculatePaddingSym(). */
     if(channel->securityMode == UA_MESSAGESECURITYMODE_SIGNANDENCRYPT)
-        mc->buf_end -= 2;
+    {
+        size_t encryptionBlockSize = securityPolicy->symmetricModule.cryptoModule.encryptionAlgorithm.
+            getLocalBlockSize(securityPolicy, channel->channelContext );
+        mc->buf_end -= 1 + (UA_Byte)encryptionBlockSize + ((encryptionBlockSize >> 8)?1:0);
+    }
 }
 
 static UA_StatusCode
@@ -725,6 +739,8 @@ sendSymmetricChunk(UA_MessageContext *messageContext) {
        channel->securityMode == UA_MESSAGESECURITYMODE_SIGNANDENCRYPT)
         total_length += securityPolicy->symmetricModule.cryptoModule.signatureAlgorithm.
             getLocalSignatureSize(securityPolicy, channel->channelContext);
+    /* Space for the padding and the signature have been reserved in setBufPos() */
+    UA_assert(total_length<=connection->config.sendBufferSize);
     messageContext->messageBuffer.length = total_length; /* For giving the buffer to the network layer */
 
     UA_assert(res == UA_STATUSCODE_GOOD);