- dtucker@cvs.openbsd.org 2013/06/01 13:15:52

[ssh-agent.c clientloop.c misc.h packet.c progressmeter.c misc.c channels.c sandbox-systrace.c] Use clock_gettime(CLOCK_MONOTONIC ...) for ssh timers so that things like keepalives and rekeying will work properly over clock steps. Suggested by markus@, "looks good" djm@.
2013-06-02 07:46:16 +10:00 · 2013-06-02 07:46:16 +10:00 · b759c9c2ef
parent 55119253c6
commit b759c9c2ef
9 changed files with 49 additions and 30 deletions
--- a/6
+++ b/6
@ -38,6 +38,12 @@
   - dtucker@cvs.openbsd.org 2013/05/31 12:28:10
     [ssh-agent.c]
     Use time_t where appropriate.  ok djm
+   - dtucker@cvs.openbsd.org 2013/06/01 13:15:52
+     [ssh-agent.c clientloop.c misc.h packet.c progressmeter.c misc.c
+     channels.c sandbox-systrace.c]
+     Use clock_gettime(CLOCK_MONOTONIC ...) for ssh timers so that things like
+     keepalives and rekeying will work properly over clock steps.  Suggested by
+     markus@, "looks good" djm@.

 20130529
  - (dtucker) [configure.ac openbsd-compat/bsd-misc.h] bz#2087: Add a null
--- a/channels.c
+++ b/channels.c
@ -1,4 +1,4 @@
-/* $OpenBSD: channels.c,v 1.321 2013/05/17 00:13:13 djm Exp $ */
+/* $OpenBSD: channels.c,v 1.322 2013/06/01 13:15:51 dtucker Exp $ */
 /*
 * Author: Tatu Ylonen <ylo@cs.hut.fi>
 * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -1335,7 +1335,7 @@ channel_post_x11_listener(Channel *c, fd_set *readset, fd_set *writeset)
 			    errno != ECONNABORTED)
 				error("accept: %.100s", strerror(errno));
 			if (errno == EMFILE || errno == ENFILE)
-				c->notbefore = time(NULL) + 1;
+				c->notbefore = monotime() + 1;
 			return;
 		}
 		set_nodelay(newsock);
@ -1482,7 +1482,7 @@ channel_post_port_listener(Channel *c, fd_set *readset, fd_set *writeset)
 			    errno != ECONNABORTED)
 				error("accept: %.100s", strerror(errno));
 			if (errno == EMFILE || errno == ENFILE)
-				c->notbefore = time(NULL) + 1;
+				c->notbefore = monotime() + 1;
 			return;
 		}
 		set_nodelay(newsock);
@ -1518,7 +1518,7 @@ channel_post_auth_listener(Channel *c, fd_set *readset, fd_set *writeset)
 			error("accept from auth socket: %.100s",
 			    strerror(errno));
 			if (errno == EMFILE || errno == ENFILE)
-				c->notbefore = time(NULL) + 1;
+				c->notbefore = monotime() + 1;
 			return;
 		}
 		nc = channel_new("accepted auth socket",
@ -1922,7 +1922,7 @@ channel_post_mux_listener(Channel *c, fd_set *readset, fd_set *writeset)
 	    &addrlen)) == -1) {
 		error("%s accept: %s", __func__, strerror(errno));
 		if (errno == EMFILE || errno == ENFILE)
-			c->notbefore = time(NULL) + 1;
+			c->notbefore = monotime() + 1;
 		return;
 	}

@ -2085,7 +2085,7 @@ channel_handler(chan_fn *ftab[], fd_set *readset, fd_set *writeset,
 		channel_handler_init();
 		did_init = 1;
 	}
-	now = time(NULL);
+	now = monotime();
 	if (unpause_secs != NULL)
 		*unpause_secs = 0;
 	for (i = 0, oalloc = channels_alloc; i < oalloc; i++) {
--- a/clientloop.c
+++ b/clientloop.c
@ -1,4 +1,4 @@
-/* $OpenBSD: clientloop.c,v 1.250 2013/05/17 00:13:13 djm Exp $ */
+/* $OpenBSD: clientloop.c,v 1.251 2013/06/01 13:15:51 dtucker Exp $ */
 /*
 * Author: Tatu Ylonen <ylo@cs.hut.fi>
 * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -273,7 +273,7 @@ set_control_persist_exit_time(void)
 		control_persist_exit_time = 0;
 	} else if (control_persist_exit_time <= 0) {
 		/* a client connection has recently closed */
-		control_persist_exit_time = time(NULL) +
+		control_persist_exit_time = monotime() +
 			(time_t)options.control_persist_timeout;
 		debug2("%s: schedule exit in %d seconds", __func__,
 		    options.control_persist_timeout);
@ -356,7 +356,7 @@ client_x11_get_proto(const char *display, const char *xauth_path,
 				if (system(cmd) == 0)
 					generated = 1;
 				if (x11_refuse_time == 0) {
-					now = time(NULL) + 1;
+					now = monotime() + 1;
 					if (UINT_MAX - timeout < now)
 						x11_refuse_time = UINT_MAX;
 					else
@ -581,7 +581,7 @@ client_wait_until_can_do_something(fd_set **readsetp, fd_set **writesetp,
 {
 	struct timeval tv, *tvp;
 	int timeout_secs;
-	time_t minwait_secs = 0, server_alive_time = 0, now = time(NULL);
+	time_t minwait_secs = 0, server_alive_time = 0, now = monotime();
 	int ret;

 	/* Add any selections by the channel mechanism. */
@ -676,7 +676,7 @@ client_wait_until_can_do_something(fd_set **readsetp, fd_set **writesetp,
 		 * Timeout.  Could have been either keepalive or rekeying.
 		 * Keepalive we check here, rekeying is checked in clientloop.
 		 */
-		if (server_alive_time != 0 && server_alive_time <= time(NULL))
+		if (server_alive_time != 0 && server_alive_time <= monotime())
 			server_alive_check();
 	}

@ -1650,7 +1650,7 @@ client_loop(int have_pty, int escape_char_arg, int ssh2_chan_id)
 		 * connections, then quit.
 		 */
 		if (control_persist_exit_time > 0) {
-			if (time(NULL) >= control_persist_exit_time) {
+			if (monotime() >= control_persist_exit_time) {
 				debug("ControlPersist timeout expired");
 				break;
 			}
@ -1868,7 +1868,7 @@ client_request_x11(const char *request_type, int rchan)
 		    "malicious server.");
 		return NULL;
 	}
-	if (x11_refuse_time != 0 && time(NULL) >= x11_refuse_time) {
+	if (x11_refuse_time != 0 && monotime() >= x11_refuse_time) {
 		verbose("Rejected X11 connection after ForwardX11Timeout "
 		    "expired");
 		return NULL;
--- a/misc.c
+++ b/misc.c
@ -1,4 +1,4 @@
-/* $OpenBSD: misc.c,v 1.89 2013/05/17 00:13:13 djm Exp $ */
+/* $OpenBSD: misc.c,v 1.90 2013/06/01 13:15:52 dtucker Exp $ */
 /*
 * Copyright (c) 2000 Markus Friedl.  All rights reserved.
 * Copyright (c) 2005,2006 Damien Miller.  All rights reserved.
@ -854,6 +854,17 @@ ms_to_timeval(struct timeval *tv, int ms)
 	tv->tv_usec = (ms % 1000) * 1000;
 }

+time_t
+monotime(void)
+{
+	struct timespec ts;
+
+	if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0)
+		fatal("clock_gettime: %s", strerror(errno));
+
+	return (ts.tv_sec);
+}
+
 void
 bandwidth_limit_init(struct bwlimit *bw, u_int64_t kbps, size_t buflen)
 {
--- a/misc.h
+++ b/misc.h
@ -1,4 +1,4 @@
-/* $OpenBSD: misc.h,v 1.48 2011/03/29 18:54:17 stevesk Exp $ */
+/* $OpenBSD: misc.h,v 1.49 2013/06/01 13:15:52 dtucker Exp $ */

 /*
 * Author: Tatu Ylonen <ylo@cs.hut.fi>
@ -35,6 +35,7 @@ char	*tohex(const void *, size_t);
 void	 sanitise_stdfd(void);
 void	 ms_subtract_diff(struct timeval *, int *);
 void	 ms_to_timeval(struct timeval *, int);
+time_t	 monotime(void);
 void	 sock_set_v6only(int);

 struct passwd *pwcopy(struct passwd *);
--- a/packet.c
+++ b/packet.c
@ -1,4 +1,4 @@
-/* $OpenBSD: packet.c,v 1.186 2013/05/17 00:13:13 djm Exp $ */
+/* $OpenBSD: packet.c,v 1.187 2013/06/01 13:15:52 dtucker Exp $ */
 /*
 * Author: Tatu Ylonen <ylo@cs.hut.fi>
 * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -1015,7 +1015,7 @@ packet_send2(void)
 	/* after a NEWKEYS message we can send the complete queue */
 	if (type == SSH2_MSG_NEWKEYS) {
 		active_state->rekeying = 0;
-		active_state->rekey_time = time(NULL);
+		active_state->rekey_time = monotime();
 		while ((p = TAILQ_FIRST(&active_state->outgoing))) {
 			type = p->type;
 			debug("dequeue packet: %u", type);
@ -1942,7 +1942,7 @@ packet_need_rekeying(void)
 	    (active_state->max_blocks_in &&
 	        (active_state->p_read.blocks > active_state->max_blocks_in)) ||
 	    (active_state->rekey_interval != 0 && active_state->rekey_time +
-		 active_state->rekey_interval <= time(NULL));
+		 active_state->rekey_interval <= monotime());
 }

 void
@ -1956,7 +1956,7 @@ packet_set_rekey_limits(u_int32_t bytes, time_t seconds)
 	 * We set the time here so that in post-auth privsep slave we count
 	 * from the completion of the authentication.
 	 */
-	active_state->rekey_time = time(NULL);
+	active_state->rekey_time = monotime();
 }

 time_t
@ -1965,7 +1965,7 @@ packet_get_rekey_timeout(void)
 	time_t seconds;

 	seconds = active_state->rekey_time + active_state->rekey_interval -
-	    time(NULL);
+	    monotime();
 	return (seconds <= 0 ? 1 : seconds);
 }

--- a/progressmeter.c
+++ b/progressmeter.c
@ -1,4 +1,4 @@
-/* $OpenBSD: progressmeter.c,v 1.37 2006/08/03 03:34:42 deraadt Exp $ */
+/* $OpenBSD: progressmeter.c,v 1.38 2013/06/01 13:15:52 dtucker Exp $ */
 /*
 * Copyright (c) 2003 Nils Nordman.  All rights reserved.
 *
@ -131,7 +131,7 @@ refresh_progress_meter(void)

 	transferred = *counter - cur_pos;
 	cur_pos = *counter;
-	now = time(NULL);
+	now = monotime();
 	bytes_left = end_pos - cur_pos;

 	if (bytes_left > 0)
@ -249,7 +249,7 @@ update_progress_meter(int ignore)
 void
 start_progress_meter(char *f, off_t filesize, off_t *ctr)
 {
-	start = last_update = time(NULL);
+	start = last_update = monotime();
 	file = f;
 	end_pos = filesize;
 	cur_pos = 0;
--- a/sandbox-systrace.c
+++ b/sandbox-systrace.c
@ -1,4 +1,4 @@
-/* $OpenBSD: sandbox-systrace.c,v 1.6 2012/06/30 14:35:09 markus Exp $ */
+/* $OpenBSD: sandbox-systrace.c,v 1.7 2013/06/01 13:15:52 dtucker Exp $ */
 /*
 * Copyright (c) 2011 Damien Miller <djm@mindrot.org>
 *
@ -57,6 +57,7 @@ static const struct sandbox_policy preauth_policy[] = {
 	{ SYS_exit, SYSTR_POLICY_PERMIT },
 	{ SYS_getpid, SYSTR_POLICY_PERMIT },
 	{ SYS_gettimeofday, SYSTR_POLICY_PERMIT },
+	{ SYS_clock_gettime, SYSTR_POLICY_PERMIT },
 	{ SYS_madvise, SYSTR_POLICY_PERMIT },
 	{ SYS_mmap, SYSTR_POLICY_PERMIT },
 	{ SYS_mprotect, SYSTR_POLICY_PERMIT },
--- a/ssh-agent.c
+++ b/ssh-agent.c
@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-agent.c,v 1.174 2013/05/31 12:28:10 dtucker Exp $ */
+/* $OpenBSD: ssh-agent.c,v 1.175 2013/06/01 13:15:52 dtucker Exp $ */
 /*
 * Author: Tatu Ylonen <ylo@cs.hut.fi>
 * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -431,7 +431,7 @@ process_remove_all_identities(SocketEntry *e, int version)
 static time_t
 reaper(void)
 {
-	time_t deadline = 0, now = time(NULL);
+	time_t deadline = 0, now = monotime();
 	Identity *id, *nxt;
 	int version;
 	Idtab *tab;
@ -618,7 +618,7 @@ process_add_identity(SocketEntry *e, int version)
 	while (buffer_len(&e->request)) {
 		switch ((type = buffer_get_char(&e->request))) {
 		case SSH_AGENT_CONSTRAIN_LIFETIME:
-			death = time(NULL) + buffer_get_int(&e->request);
+			death = monotime() + buffer_get_int(&e->request);
 			break;
 		case SSH_AGENT_CONSTRAIN_CONFIRM:
 			confirm = 1;
@ -633,7 +633,7 @@ process_add_identity(SocketEntry *e, int version)
 	}
 	success = 1;
 	if (lifetime && !death)
-		death = time(NULL) + lifetime;
+		death = monotime() + lifetime;
 	if ((id = lookup_identity(k, version)) == NULL) {
 		id = xcalloc(1, sizeof(Identity));
 		id->key = k;
@ -712,7 +712,7 @@ process_add_smartcard_key(SocketEntry *e)
 	while (buffer_len(&e->request)) {
 		switch ((type = buffer_get_char(&e->request))) {
 		case SSH_AGENT_CONSTRAIN_LIFETIME:
-			death = time(NULL) + buffer_get_int(&e->request);
+			death = monotime() + buffer_get_int(&e->request);
 			break;
 		case SSH_AGENT_CONSTRAIN_CONFIRM:
 			confirm = 1;
@ -724,7 +724,7 @@ process_add_smartcard_key(SocketEntry *e)
 		}
 	}
 	if (lifetime && !death)
-		death = time(NULL) + lifetime;
+		death = monotime() + lifetime;

 	count = pkcs11_add_provider(provider, pin, &keys);
 	for (i = 0; i < count; i++) {