Merge pull request #294 from Castaglia/single-payload-len-check

Attempting to keep the SSH packet reading code in `mod_proxy` more in…

Author: Castaglia

lib/proxy/ssh/packet.c

@@ -732,7 +732,7 @@ static int read_packet_payload(conn_t *conn, struct proxy_ssh_packet *pkt,
  return -1;
  }
 
- pkt->payload = pcalloc(pkt->pool, payload_len);
+ pkt->payload = palloc(pkt->pool, payload_len);
  }
 
  /* If there's data in the buffer we received, it's probably already part
@@ -762,7 +762,7 @@ static int read_packet_payload(conn_t *conn, struct proxy_ssh_packet *pkt,
  * modes.
  */
  if (padding_len > 0) {
- pkt->padding = pcalloc(pkt->pool, padding_len);
+ pkt->padding = palloc(pkt->pool, padding_len);
  }
 
  /* If there's data in the buffer we received, it's probably already part
@@ -1082,62 +1082,6 @@ static void reset_timers(void) {
  }
 }
 
-static int check_packet_lengths(conn_t *conn, struct proxy_ssh_packet *pkt) {
- if (pkt->packet_len < 5) {
- (void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
- "packet length too short (%lu), less than minimum packet length (5)",
- (unsigned long) pkt->packet_len);
- read_packet_discard(conn);
- return -1;
- }
-
- if (pkt->packet_len > PROXY_SSH_MAX_PACKET_LEN) {
- (void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
- "packet length too long (%lu), exceeds maximum packet length (%lu)",
- (unsigned long) pkt->packet_len,
- (unsigned long) PROXY_SSH_MAX_PACKET_LEN);
- read_packet_discard(conn);
- return -1;
- }
-
- /* Per Section 6 of RFC4253, the minimum padding length is 4, the
- * maximum padding length is 255.
- */
-
- if (pkt->padding_len < PROXY_SSH_MIN_PADDING_LEN) {
- (void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
- "padding length too short (%u), less than minimum padding length (%u)",
- (unsigned int) pkt->padding_len,
- (unsigned int) PROXY_SSH_MIN_PADDING_LEN);
- read_packet_discard(conn);
- return -1;
- }
-
- if (pkt->padding_len > pkt->packet_len) {
- (void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
- "padding length too long (%u), exceeds packet length (%lu)",
- (unsigned int) pkt->padding_len, (unsigned long) pkt->packet_len);
- read_packet_discard(conn);
- return -1;
- }
-
- /* XXX I'm not so sure about this check; we SHOULD have a maximum payload
- * check, but using the max packet length check for the payload length seems
- * awkward. Still, better than nothing.
- */
- if (pkt->payload_len > PROXY_SSH_MAX_PACKET_LEN) {
- (void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
- "payload length too long (%lu), exceeds maximum payload length (%lu) "
- "(packet len %lu, padding len %u)", (unsigned long) pkt->payload_len,
- (unsigned long) PROXY_SSH_MAX_PACKET_LEN, (unsigned long) pkt->packet_len,
- (unsigned int) pkt->padding_len);
- read_packet_discard(conn);
- return -1;
- }
-
- return 0;
-}
-
 int proxy_ssh_packet_read(conn_t *conn, struct proxy_ssh_packet *pkt) {
  unsigned char buf[PROXY_SSH_MAX_PACKET_LEN];
  size_t buflen, bufsz = PROXY_SSH_MAX_PACKET_LEN, offset = 0, auth_len = 0;
@@ -1211,15 +1155,10 @@ int proxy_ssh_packet_read(conn_t *conn, struct proxy_ssh_packet *pkt) {
  (unsigned int) pkt->padding_len);
 
  pkt->payload_len = (pkt->packet_len - pkt->padding_len - 1);
-
- if (check_packet_lengths(conn, pkt) < 0) {
- return -1;
- }
+ pr_trace_msg(trace_channel, 20, "SSH2 packet payload len = %u bytes",
+ (unsigned int) pkt->payload_len);
  }
 
- pr_trace_msg(trace_channel, 20, "SSH2 packet payload len = %lu bytes",
- (unsigned long) pkt->payload_len);
-
  /* Read both payload and padding, since we may need to have both before
  * decrypting the data.
  */
@@ -1237,7 +1176,7 @@ int proxy_ssh_packet_read(conn_t *conn, struct proxy_ssh_packet *pkt) {
 
  if (etm_mac == TRUE) {
  bufsz2 = buflen2 = pkt->mac_len;
- buf2 = pcalloc(pkt->pool, bufsz2);
+ buf2 = palloc(pkt->pool, bufsz2);
 
  /* The MAC routines assume the presence of the necessary data in
  * pkt->payload, so we temporarily put our encrypted packet data there.
@@ -1270,7 +1209,7 @@ int proxy_ssh_packet_read(conn_t *conn, struct proxy_ssh_packet *pkt) {
  * packet from read_packet_payload().
  */
  bufsz2 = buflen2 = PROXY_SSH_MAX_PACKET_LEN;
- buf2 = pcalloc(pkt->pool, bufsz2);
+ buf2 = palloc(pkt->pool, bufsz2);
 
  if (proxy_ssh_cipher_read_data(pkt, buf, buflen, &buf2,
  (uint32_t *) &buflen2) < 0) {
@@ -1290,9 +1229,19 @@ int proxy_ssh_packet_read(conn_t *conn, struct proxy_ssh_packet *pkt) {
  pr_trace_msg(trace_channel, 20, "SSH2 packet padding len = %u bytes",
  (unsigned int) pkt->padding_len);
 
- } else {
- memset(buf, 0, sizeof(buf));
+ pkt->payload_len = (pkt->packet_len - pkt->padding_len - 1);
+ pr_trace_msg(trace_channel, 20, "SSH2 packet payload len = %lu bytes",
+ (unsigned long) pkt->payload_len);
 
+ if (pkt->payload_len > 0) {
+ pkt->payload = palloc(pkt->pool, pkt->payload_len);
+ memmove(pkt->payload, buf2 + offset, pkt->payload_len);
+ }
+
+ pkt->padding = palloc(pkt->pool, pkt->padding_len);
+ memmove(pkt->padding, buf2 + offset + pkt->payload_len, pkt->padding_len);
+
+ } else {
  if (read_packet_mac(conn, pkt, buf) < 0) {
  (void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
  "unable to read MAC from socket %d", conn->rfd);
@@ -1319,20 +1268,42 @@ int proxy_ssh_packet_read(conn_t *conn, struct proxy_ssh_packet *pkt) {
  * correct.
  */
 
- if (etm_mac == TRUE) {
- pkt->payload_len = (pkt->packet_len - pkt->padding_len - 1);
+ if (pkt->packet_len < 5) {
+ (void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
+ "packet length too short (%lu), less than minimum packet length (5)",
+ (unsigned long) pkt->packet_len);
+ read_packet_discard(conn);
+ return -1;
+ }
 
- if (check_packet_lengths(conn, pkt) < 0) {
- return -1;
- }
+ if (pkt->packet_len > PROXY_SSH_MAX_PACKET_LEN) {
+ (void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
+ "packet length too long (%lu), exceeds maximum packet length (%lu)",
+ (unsigned long) pkt->packet_len,
+ (unsigned long) PROXY_SSH_MAX_PACKET_LEN);
+ read_packet_discard(conn);
+ return -1;
+ }
 
- if (pkt->payload_len > 0) {
- pkt->payload = pcalloc(pkt->pool, pkt->payload_len);
- memmove(pkt->payload, buf2 + offset, pkt->payload_len);
- }
+ /* Per Section 6 of RFC4253, the minimum padding length is 4, the
+ * maximum padding length is 255.
+ */
 
- pkt->padding = pcalloc(pkt->pool, pkt->padding_len);
- memmove(pkt->padding, buf2 + offset + pkt->payload_len, pkt->padding_len);
+ if (pkt->padding_len < PROXY_SSH_MIN_PADDING_LEN) {
+ (void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
+ "padding length too short (%u), less than minimum padding length (%u)",
+ (unsigned int) pkt->padding_len,
+ (unsigned int) PROXY_SSH_MIN_PADDING_LEN);
+ read_packet_discard(conn);
+ return -1;
+ }
+
+ if (pkt->padding_len > pkt->packet_len) {
+ (void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
+ "padding length too long (%u), exceeds packet length (%lu)",
+ (unsigned int) pkt->padding_len, (unsigned long) pkt->packet_len);
+ read_packet_discard(conn);
+ return -1;
  }
 
  /* From RFC4253, Section 6: