From fae7bbe544cba7a9e5e4ab47ff6faa3d978646eb Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Wed, 28 Jan 2015 21:15:47 +0000 Subject: [PATCH] upstream commit avoid fatal() calls in packet code makes ssh-keyscan more reliable against server failures ok dtucker@ markus@ --- opacket.c | 25 +++++++++++++++++++++++-- opacket.h | 3 +-- packet.c | 33 +++++++++++++++------------------ packet.h | 4 ++-- ssh-keyscan.c | 8 +++++--- ssherr.c | 6 +++++- ssherr.h | 4 +++- 7 files changed, 54 insertions(+), 29 deletions(-) diff --git a/opacket.c b/opacket.c index 63b419d5c..a137b5a8a 100644 --- a/opacket.c +++ b/opacket.c @@ -255,8 +255,20 @@ packet_read_seqnr(u_int32_t *seqnr) u_char type; int r; - if ((r = ssh_packet_read_seqnr(active_state, &type, seqnr))) - fatal("%s: %s", __func__, ssh_err(r)); + if ((r = ssh_packet_read_seqnr(active_state, &type, seqnr)) != 0) { + switch (r) { + case SSH_ERR_CONN_CLOSED: + logit("Connection closed by %.200s", + ssh_remote_ipaddr(active_state)); + cleanup_exit(255); + case SSH_ERR_CONN_TIMEOUT: + logit("Connection to %.200s timed out while " + "waiting to read", ssh_remote_ipaddr(active_state)); + cleanup_exit(255); + default: + fatal("%s: %s", __func__, ssh_err(r)); + } + } return type; } @@ -277,3 +289,12 @@ packet_close(void) ssh_packet_close(active_state); active_state = NULL; } + +void +packet_process_incoming(const char *buf, u_int len) +{ + int r; + + if ((r = ssh_packet_process_incoming(active_state, buf, len)) != 0) + fatal("%s: %s", __func__, ssh_err(r)); +} diff --git a/opacket.h b/opacket.h index 1e15626eb..261ed1f81 100644 --- a/opacket.h +++ b/opacket.h @@ -44,6 +44,7 @@ void packet_restore_state(void); void packet_set_connection(int, int); int packet_read_seqnr(u_int32_t *); int packet_read_poll_seqnr(u_int32_t *); +void packet_process_incoming(const char *buf, u_int len); #define packet_set_timeout(timeout, count) \ ssh_packet_set_timeout(active_state, (timeout), (count)) #define packet_connection_is_on_socket() \ @@ -86,8 +87,6 @@ int packet_read_poll_seqnr(u_int32_t *); ssh_packet_read(active_state) #define packet_read_expect(expected_type) \ ssh_packet_read_expect(active_state, (expected_type)) -#define packet_process_incoming(buf, len) \ - ssh_packet_process_incoming(active_state, (buf), (len)) #define packet_get_int64() \ ssh_packet_get_int64(active_state) #define packet_get_bignum(value) \ diff --git a/packet.c b/packet.c index 2c8d8aa9b..eb178f149 100644 --- a/packet.c +++ b/packet.c @@ -1,4 +1,4 @@ -/* $OpenBSD: packet.c,v 1.203 2015/01/20 23:14:00 deraadt Exp $ */ +/* $OpenBSD: packet.c,v 1.204 2015/01/28 21:15:47 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -1314,26 +1314,22 @@ ssh_packet_read_seqnr(struct ssh *ssh, u_char *typep, u_int32_t *seqnr_p) break; } } - if (r == 0) { - logit("Connection to %.200s timed out while " - "waiting to read", ssh_remote_ipaddr(ssh)); - cleanup_exit(255); - } + if (r == 0) + return SSH_ERR_CONN_TIMEOUT; /* Read data from the socket. */ do { cont = 0; len = roaming_read(state->connection_in, buf, sizeof(buf), &cont); } while (len == 0 && cont); - if (len == 0) { - logit("Connection closed by %.200s", - ssh_remote_ipaddr(ssh)); - cleanup_exit(255); - } + if (len == 0) + return SSH_ERR_CONN_CLOSED; if (len < 0) - fatal("Read from socket failed: %.100s", strerror(errno)); + return SSH_ERR_SYSTEM_ERROR; + /* Append it to the buffer. */ - ssh_packet_process_incoming(ssh, buf, len); + if ((r = ssh_packet_process_incoming(ssh, buf, len)) != 0) + return r; } free(setp); return r; @@ -1787,7 +1783,7 @@ ssh_packet_read_poll_seqnr(struct ssh *ssh, u_char *typep, u_int32_t *seqnr_p) * together with packet_read_poll. */ -void +int ssh_packet_process_incoming(struct ssh *ssh, const char *buf, u_int len) { struct session_state *state = ssh->state; @@ -1797,14 +1793,15 @@ ssh_packet_process_incoming(struct ssh *ssh, const char *buf, u_int len) state->keep_alive_timeouts = 0; /* ?? */ if (len >= state->packet_discard) { if ((r = ssh_packet_stop_discard(ssh)) != 0) - fatal("%s: %s", __func__, ssh_err(r)); - cleanup_exit(255); + return r; } state->packet_discard -= len; - return; + return 0; } if ((r = sshbuf_put(ssh->state->input, buf, len)) != 0) - fatal("%s: %s", __func__, ssh_err(r)); + return r; + + return 0; } int diff --git a/packet.h b/packet.h index 069af2eae..8a9d0f6c6 100644 --- a/packet.h +++ b/packet.h @@ -1,4 +1,4 @@ -/* $OpenBSD: packet.h,v 1.64 2015/01/19 20:30:23 markus Exp $ */ +/* $OpenBSD: packet.h,v 1.65 2015/01/28 21:15:47 djm Exp $ */ /* * Author: Tatu Ylonen @@ -94,7 +94,7 @@ void ssh_packet_read_expect(struct ssh *, int type); int ssh_packet_read_poll(struct ssh *); int ssh_packet_read_poll1(struct ssh *, u_char *); int ssh_packet_read_poll2(struct ssh *, u_char *, u_int32_t *seqnr_p); -void ssh_packet_process_incoming(struct ssh *, const char *buf, u_int len); +int ssh_packet_process_incoming(struct ssh *, const char *buf, u_int len); int ssh_packet_read_seqnr(struct ssh *, u_char *, u_int32_t *seqnr_p); int ssh_packet_read_poll_seqnr(struct ssh *, u_char *, u_int32_t *seqnr_p); diff --git a/ssh-keyscan.c b/ssh-keyscan.c index 25a257cc2..e59eacace 100644 --- a/ssh-keyscan.c +++ b/ssh-keyscan.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-keyscan.c,v 1.96 2015/01/20 23:14:00 deraadt Exp $ */ +/* $OpenBSD: ssh-keyscan.c,v 1.97 2015/01/28 21:15:47 djm Exp $ */ /* * Copyright 1995, 1996 by David Mazieres . * @@ -319,8 +319,10 @@ tcpconnect(char *host) memset(&hints, 0, sizeof(hints)); hints.ai_family = IPv4or6; hints.ai_socktype = SOCK_STREAM; - if ((gaierr = getaddrinfo(host, strport, &hints, &aitop)) != 0) - fatal("getaddrinfo %s: %s", host, ssh_gai_strerror(gaierr)); + if ((gaierr = getaddrinfo(host, strport, &hints, &aitop)) != 0) { + error("getaddrinfo %s: %s", host, ssh_gai_strerror(gaierr)); + return -1; + } for (ai = aitop; ai; ai = ai->ai_next) { s = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol); if (s < 0) { diff --git a/ssherr.c b/ssherr.c index 49fbb71de..0b79fbb00 100644 --- a/ssherr.c +++ b/ssherr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssherr.c,v 1.1 2014/04/30 05:29:56 djm Exp $ */ +/* $OpenBSD: ssherr.c,v 1.2 2015/01/28 21:15:47 djm Exp $ */ /* * Copyright (c) 2011 Damien Miller * @@ -125,6 +125,10 @@ ssh_err(int n) return "KRL file has invalid magic number"; case SSH_ERR_KEY_REVOKED: return "Key is revoked"; + case SSH_ERR_CONN_CLOSED: + return "Connection closed"; + case SSH_ERR_CONN_TIMEOUT: + return "Connection timed out"; default: return "unknown error"; } diff --git a/ssherr.h b/ssherr.h index 106f786ea..ac5cd159a 100644 --- a/ssherr.h +++ b/ssherr.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssherr.h,v 1.1 2014/04/30 05:29:56 djm Exp $ */ +/* $OpenBSD: ssherr.h,v 1.2 2015/01/28 21:15:47 djm Exp $ */ /* * Copyright (c) 2011 Damien Miller * @@ -73,6 +73,8 @@ #define SSH_ERR_BUFFER_READ_ONLY -49 #define SSH_ERR_KRL_BAD_MAGIC -50 #define SSH_ERR_KEY_REVOKED -51 +#define SSH_ERR_CONN_CLOSED -52 +#define SSH_ERR_CONN_TIMEOUT -53 /* Translate a numeric error code to a human-readable error string */ const char *ssh_err(int n);