Browse code

Allow trailing \r and \n in control channel message

Writing a reason from a script will easily end up adding extra \r\n characters
at the end of the reason. Our current code pushes this to the peer. So be more
liberal in accepting these message.

Github: closes OpenVPN/openvpn#568

This is the backport of the fix (commit be31325e1dfdffb) to release/2.5.

Change-Id: I47c992b6b73b1475cbff8a28f720cf50dc1fbe3e
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240711113022.52076-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28923.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Arne Schwabe authored on 2024/07/11 20:30:22
Showing 1 changed files
... ...
@@ -221,6 +221,46 @@ parse_incoming_control_channel_command(struct context *c, struct buffer *buf)
221 221
     }
222 222
 }
223 223
 
224
+static struct buffer
225
+extract_command_buffer(struct buffer *buf, struct gc_arena *gc)
226
+{
227
+    /* commands on the control channel are seperated by 0x00 bytes.
228
+     * cmdlen does not include the 0 byte of the string */
229
+    int cmdlen = (int)strnlen(BSTR(buf), BLEN(buf));
230
+
231
+    if (cmdlen >= BLEN(buf))
232
+    {
233
+        buf_advance(buf, cmdlen);
234
+        /* Return empty buffer */
235
+        struct buffer empty = { 0 };
236
+        return empty;
237
+    }
238
+
239
+    /* include the NUL byte and ensure NUL termination */
240
+    cmdlen +=  1;
241
+
242
+    /* Construct a buffer that only holds the current command and
243
+     * its closing NUL byte */
244
+    struct buffer cmdbuf = alloc_buf_gc(cmdlen, gc);
245
+    buf_write(&cmdbuf, BPTR(buf), cmdlen);
246
+
247
+    /* Remove \r and \n at the end of the buffer to avoid
248
+     * problems with scripts and other that add extra \r and \n */
249
+    buf_chomp(&cmdbuf);
250
+
251
+    /* check we have only printable characters or null byte in the
252
+     * command string and no newlines */
253
+    if (!string_check_buf(&cmdbuf, CC_PRINT | CC_NULL, CC_CRLF))
254
+    {
255
+        msg(D_PUSH_ERRORS, "WARNING: Received control with invalid characters: %s",
256
+            format_hex(BPTR(&cmdbuf), BLEN(&cmdbuf), 256, gc));
257
+        cmdbuf.len = 0;
258
+    }
259
+
260
+    buf_advance(buf, cmdlen);
261
+    return cmdbuf;
262
+}
263
+
224 264
 /*
225 265
  * Handle incoming configuration
226 266
  * messages on the control channel.
... ...
@@ -236,41 +276,14 @@ check_incoming_control_channel(struct context *c)
236 236
     struct buffer buf = alloc_buf_gc(len, &gc);
237 237
     if (tls_rec_payload(c->c2.tls_multi, &buf))
238 238
     {
239
-
240 239
         while (BLEN(&buf) > 1)
241 240
         {
242
-            /* commands on the control channel are seperated by 0x00 bytes.
243
-             * cmdlen does not include the 0 byte of the string */
244
-            int cmdlen = (int)strnlen(BSTR(&buf), BLEN(&buf));
245
-
246
-            if (cmdlen < BLEN(&buf))
247
-            {
248
-                /* include the NUL byte and ensure NUL termination */
249
-                int cmdlen = (int)strlen(BSTR(&buf)) + 1;
250
-
251
-                /* Construct a buffer that only holds the current command and
252
-                 * its closing NUL byte */
253
-                struct buffer cmdbuf = alloc_buf_gc(cmdlen, &gc);
254
-                buf_write(&cmdbuf, BPTR(&buf), cmdlen);
241
+            struct buffer cmdbuf = extract_command_buffer(&buf, &gc);
255 242
 
256
-                /* check we have only printable characters or null byte in the
257
-                 * command string and no newlines */
258
-                if (!string_check_buf(&buf, CC_PRINT | CC_NULL, CC_CRLF))
259
-                {
260
-                    msg(D_PUSH_ERRORS, "WARNING: Received control with invalid characters: %s",
261
-                        format_hex(BPTR(&buf), BLEN(&buf), 256, &gc));
262
-                }
263
-                else
264
-                {
265
-                    parse_incoming_control_channel_command(c, &cmdbuf);
266
-                }
267
-            }
268
-            else
243
+            if (cmdbuf.len > 0)
269 244
             {
270
-                msg(D_PUSH_ERRORS, "WARNING: Ignoring control channel "
271
-                    "message command without NUL termination");
245
+                parse_incoming_control_channel_command(c, &cmdbuf);
272 246
             }
273
-            buf_advance(&buf, cmdlen);
274 247
         }
275 248
     }
276 249
     else