Skip to content

Commit

Permalink
Remove first oNCP negotiation request (only second is necessary)
Browse files Browse the repository at this point in the history
The current oNCP (Juniper) protocol support issues two separate
oNCP negotiation requests.

1) POST /dana/js?prot=1&svc=1 HTTP/1.1
   <ignore response body>
   <teardown and restart TLS connection>

2) POST /dana/js?prot=1&svc=4 HTTP/1.1
   <continue using open TLS connection for oNCP tunnel>

The first of these two requests appears to be totally unnecessary, based on
testing with two different Juniper gateways, one of which returns
"NCP-Version: 2" and one which returns "NCP-Version: 3" in response to the
oNCP negotiation requests.

Removing the first request saves an additional TLS negotiation (2-3
roundtrips with TLS 1.0) and allows the connection to start faster.
  • Loading branch information
dlenski committed Aug 2, 2018
1 parent 46de5ee commit 62c60ba
Showing 1 changed file with 0 additions and 52 deletions.
52 changes: 0 additions & 52 deletions oncp.c
Expand Up @@ -560,57 +560,6 @@ int oncp_connect(struct openconnect_info *vpninfo)

reqbuf = buf_alloc();

buf_append(reqbuf, "POST /dana/js?prot=1&svc=1 HTTP/1.1\r\n");
/* Seems unnecessary because we always ignore the response body,
and close the connection anyway, but retained in case any
server depends on it. (See comments on second negotiation
request below. */
buf_append(reqbuf, "Connection: close\r\n");
oncp_common_headers(vpninfo, reqbuf);
buf_append(reqbuf, "Content-Length: 256\r\n");
buf_append(reqbuf, "\r\n");

if (buf_error(reqbuf)) {
vpn_progress(vpninfo, PRG_ERR,
_("Error creating oNCP negotiation request\n"));
ret = buf_error(reqbuf);
goto out;
}

vpn_progress(vpninfo, PRG_TRACE, _("Sending oNCP negotiation request #1\n"));
ret = vpninfo->ssl_write(vpninfo, reqbuf->data, reqbuf->pos);
if (ret < 0)
goto out;

/* The server is fairly weird. It sends Connection: close which would
* indicate an HTTP 1.0-style body, but doesn't seem to actually close
* the connection. So tell process_http_response() it was a CONNECT
* request, since we don't care about the body anyway, and then close
* the connection for ourselves. */
ret = process_http_response(vpninfo, 1, NULL, reqbuf);
openconnect_close_https(vpninfo, 0);
if (ret < 0) {
/* We'll already have complained about whatever offended us */
goto out;
}
if (ret != 200) {
vpn_progress(vpninfo, PRG_ERR,
_("Unexpected %d result from server\n"),
ret);
ret = -EINVAL;
goto out;
}

/* Now the second request. We should reduce the duplication
here but let's not overthink it for now; we should see what
the authentication requests are going to look like, and make
do_https_request() or a new helper function work for those
too. */
ret = openconnect_open_https(vpninfo);
if (ret)
goto out;

buf_truncate(reqbuf);
buf_append(reqbuf, "POST /dana/js?prot=1&svc=4 HTTP/1.1\r\n");
/* The TLS socket actually remains open for use by the oNCP
tunnel, but the "Connection: close" header is nevertheless
Expand All @@ -629,7 +578,6 @@ int oncp_connect(struct openconnect_info *vpninfo)
ret = buf_error(reqbuf);
goto out;
}
vpn_progress(vpninfo, PRG_TRACE, _("Sending oNCP negotiation request #2\n"));
ret = vpninfo->ssl_write(vpninfo, reqbuf->data, reqbuf->pos);
if (ret < 0)
goto out;
Expand Down

0 comments on commit 62c60ba

Please sign in to comment.