Handle yet more oNCP framing idiocy

This protocol doesn't use TLS record framing, but *does* have its own
framing, each frame starting with a very minimal 2-byte frame length.

These frames can be larger than 16KiB which is the maximum size of a TLS
record. So ensure that we loop, reading a full frame where necessary.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/oncp.c b/oncp.c
index a81756010249f93375d4835e4beb4e92a087cc76..b9372c28922ca887c6c85c908df64f362157e895 100644 (file)
--- a/oncp.c
+++ b/oncp.c
@@ -467,7 +467,7 @@ int oncp_connect(struct openconnect_info *vpninfo)
 {
        int ret, len, kmp, kmplen, group, check_len;
        struct oc_text_buf *reqbuf;
-       unsigned char bytes[65536];
+       unsigned char bytes[65538];
 
        if (!vpninfo->cookies) {
                /* XX: This will happen if authentication was separate/external */
@@ -602,6 +602,18 @@ int oncp_connect(struct openconnect_info *vpninfo)
        vpn_progress(vpninfo, PRG_TRACE,
                     _("Read %d bytes of SSL record\n"), len);
 
+       /* The NC frame might be split across multiple TLS records. Read the whole frame */
+       while (len < check_len + 2) {
+               int thislen = vpninfo->ssl_read(vpninfo, (void *)(bytes + len), sizeof(bytes) - len);
+               if (len < 0) {
+                       ret = len;
+                       goto out;
+               }
+               len += thislen;
+               vpn_progress(vpninfo, PRG_TRACE,
+                            _("Read additional %d bytes of SSL record\n"), thislen);
+       }
+
        if (len < 0x16 || check_len + 2 != len) {
                vpn_progress(vpninfo, PRG_ERR,
                             _("Invalid packet waiting for KMP 301\n"));
@@ -639,7 +651,7 @@ int oncp_connect(struct openconnect_info *vpninfo)
 
        while (kmplen + 22 > len) {
                char l[2];
-               int thislen;
+               int reclen;
 
                if (vpninfo->ssl_read(vpninfo, (void *)l, 2) != 2) {
                        vpn_progress(vpninfo, PRG_ERR,
@@ -655,13 +667,18 @@ int oncp_connect(struct openconnect_info *vpninfo)
                        goto out;
                }
 
-               thislen = vpninfo->ssl_read(vpninfo, (void *)(bytes + len), load_le16(l));
-               if (thislen != load_le16(l)) {
-                       vpn_progress(vpninfo, PRG_ERR,
-                                    _("Failed to read continuation record of length %d\n"),
-                                    load_le16(l));
-                       ret = -EINVAL;
-                       goto out;
+               /* The NC frame might be split across multiple TLS records. Read the whole frame */
+               reclen = 0;
+               while (reclen < load_le16(l)) {
+                       int thislen = vpninfo->ssl_read(vpninfo, (void *)(bytes + len), load_le16(l) - reclen);
+                       if (thislen < 0) {
+                               vpn_progress(vpninfo, PRG_ERR,
+                                            _("Failed to read continuation record of length %d\n"),
+                                            load_le16(l));
+                               ret = thislen;
+                               goto out;
+                       }
+                       reclen += thislen;
                }
 
                /*
@@ -683,9 +700,9 @@ int oncp_connect(struct openconnect_info *vpninfo)
                 * get a new frame, attempt to find a set of frames which add up to the
                 * correct size and see if they parse sanely. But let's try this for now.
                 */
-               if (thislen >= 21 && !memcmp(bytes + len, kmp_head, sizeof(kmp_head)) &&
+               if (reclen >= 21 && !memcmp(bytes + len, kmp_head, sizeof(kmp_head)) &&
                    !bytes[len + 8] && !memcmp(bytes + len + 9, kmp_tail + 1, sizeof(kmp_tail) - 1) &&
-                   load_be16(bytes + len + 6) == 300 && load_be16(bytes + len + 18) + 20 == thislen &&
+                   load_be16(bytes + len + 6) == 300 && load_be16(bytes + len + 18) + 20 == reclen &&
                    bytes[len + 20] == 0x45 /* Only Legacy IP over oNCP anyway */) {
                        vpn_progress(vpninfo, PRG_INFO,
                                     _("Discarding Legacy IP frame in the middle of oNCP config\n"));
@@ -694,12 +711,12 @@ int oncp_connect(struct openconnect_info *vpninfo)
 
                vpn_progress(vpninfo, PRG_TRACE,
                             _("Read additional %d bytes of KMP 301 message\n"),
-                            thislen);
+                            reclen);
 
                if (vpninfo->dump_http_traffic)
-                       dump_buf_hex(vpninfo, PRG_TRACE, '<', bytes + len, thislen);
+                       dump_buf_hex(vpninfo, PRG_TRACE, '<', bytes + len, reclen);
 
-               len += thislen;
+               len += reclen;
        }
 
        ret = parse_conf_pkt(vpninfo, bytes + 2, len - 2, ret);

diff --git a/www/changelog.xml b/www/changelog.xml
index a148296878a61dad43938a4799ffb8ace2e03fea..f875de63bb4dc7361fa148d01333da3aaeac6e81 100644 (file)
--- a/www/changelog.xml
+++ b/www/changelog.xml
@@ -36,6 +36,7 @@
        <li>Fix bug that caused OpenConnect to incorrectly log the remaining time until a re-key or periodic Trojan (<a href="https://gitlab.com/openconnect/openconnect/-/issues/677">#677</a>, <a href="https://gitlab.com/openconnect/openconnect/-/merge_requests/539">!539</a>)</li>
        <li>Fix bug that prevented GlobalProtect ESP from working correctly when the server sends both Legacy IP and IPv6 versions of the ESP "magic ping" address, but no IPv6 client address (<a href="https://gitlab.com/openconnect/openconnect/-/merge_requests/565">!565</a>)</li>
        <li>Use the full URI (including "usergroup" or path) as specified in <tt>--server</tt> for all requests during authentication instead of only the first one (<a href="https://gitlab.com/openconnect/openconnect/-/merge_requests/560">!560</a>).</li>
+       <li>Deal with more Juniper NC framing idiocy (<a href="https://gitlab.com/openconnect/openconnect/-/issues/779">#779</a>).</li>
      </ul><br/>
   </li>
   <li><b><a href="https://www.infradead.org/openconnect/download/openconnect-9.12.tar.gz">OpenConnect v9.12</a></b>