openssh at p23q.org
2007-Feb-23 18:10 UTC
ssh-agent does not immediately clean timeouted keys from memory
during my seminar of advanced exploitation techniques (SEAT, [1]) i developed some methods to crack into system via DMA (e.g. via firewire). as part of this i developed a program that steals loaded ssh private keys from ssh-agents. i was astonished to find that the keys are not immediately removed from the agent when a timeout occurs, but only the next time the agent is queried via its socket. i have written a __rough__ patch that should fix the problem (a timer checks every 10 seconds). please take a look at it and, if you like it, incorporate it. the patch can be found at [2], more information on other things i developed during SEAT can be found at [3] - once i release the stuff (in a few days, i think). so far losTrace a.k.a. David R. Piegdon [1] seminar of advanced exploitation techniques http://www-i4.informatik.rwth-aachen.de/content/teaching/seminars/sub/2006_2007_seat_seminar.html [2] rough patch that fixes ssh-agent timeout problem http://david.piegdon.de/SEAT/ssh-agent.patch [3] more information on my stuff http://david.piegdon.de/products.html -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 189 bytes Desc: not available Url : http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20070223/e7730d88/attachment-0001.bin
Darren Tucker
2007-Feb-23 23:18 UTC
ssh-agent does not immediately clean timeouted keys from memory
On Fri, Feb 23, 2007 at 06:10:32PM +0000, openssh at p23q.org wrote:> during my seminar of advanced exploitation techniques (SEAT, [1]) i > developed some methods to crack into system via DMA (e.g. via firewire). > as part of this i developed a program that steals loaded ssh private > keys from ssh-agents. i was astonished to find that the keys are not > immediately removed from the agent when a timeout occurs, but only the > next time the agent is queried via its socket. i have written a > __rough__ patch that should fix the problem (a timer checks every 10 > seconds). please take a look at it and, if you like it, incorporate it.Overloading the sigalrm handler seems unnecessarily complex when select(2) has a perfectly good timeout parameter :-) -- Darren Tucker (dtucker at zip.com.au) GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4 37C9 C982 80C7 8FF4 FA69 Good judgement comes with experience. Unfortunately, the experience usually comes from bad judgement. -------------- next part -------------- Index: ssh-agent.c ==================================================================RCS file: /usr/local/src/security/openssh/cvs/openssh/ssh-agent.c,v retrieving revision 1.169 diff -u -p -r1.169 ssh-agent.c --- ssh-agent.c 23 Oct 2006 17:01:16 -0000 1.169 +++ ssh-agent.c 23 Feb 2007 23:01:21 -0000 @@ -434,6 +434,7 @@ reaper(void) for (id = TAILQ_FIRST(&tab->idlist); id; id = nxt) { nxt = TAILQ_NEXT(id, next); if (id->death != 0 && now >= id->death) { + debug("expiring '%s'", id->comment); TAILQ_REMOVE(&tab->idlist, id, next); free_identity(id); tab->nentries--; @@ -698,9 +699,6 @@ process_message(SocketEntry *e) u_int msg_len, type; u_char *cp; - /* kill dead keys */ - reaper(); - if (buffer_len(&e->input) < 5) return; /* Incomplete message. */ cp = buffer_ptr(&e->input); @@ -828,10 +826,14 @@ new_socket(sock_type type, int fd) } static int -prepare_select(fd_set **fdrp, fd_set **fdwp, int *fdl, u_int *nallocp) +prepare_select(fd_set **fdrp, fd_set **fdwp, int *fdl, u_int *nallocp, + struct timeval **tvpp) { - u_int i, sz; - int n = 0; + u_int i, sz, deadline = 0; + int n = 0, version, timeout; + Identity *id, *nxt; + Idtab *tab; + time_t now = time(NULL); for (i = 0; i < sockets_alloc; i++) { switch (sockets[i].type) { @@ -875,6 +877,26 @@ prepare_select(fd_set **fdrp, fd_set **f break; } } + + for (version = 1; version < 3; version++) { + tab = idtab_lookup(version); + for (id = TAILQ_FIRST(&tab->idlist); id; id = nxt) { + nxt = TAILQ_NEXT(id, next); + if (id->death != 0) { + if (deadline == 0) + deadline = id->death; + else + deadline = MIN(deadline, id->death); + } + } + } + if (deadline == 0) { + *tvpp = NULL; + } else { + timeout = deadline - now; + (*tvpp)->tv_sec = timeout < 0 ? 0 : timeout; + (*tvpp)->tv_usec = 0; + } return (1); } @@ -1016,7 +1038,7 @@ int main(int ac, char **av) { int c_flag = 0, d_flag = 0, k_flag = 0, s_flag = 0; - int sock, fd, ch; + int sock, fd, ch, result; u_int nalloc; char *shell, *format, *pidstr, *agentsocket = NULL; fd_set *readsetp = NULL, *writesetp = NULL; @@ -1029,6 +1051,7 @@ main(int ac, char **av) extern char *optarg; pid_t pid; char pidstrbuf[1 + 3 * sizeof pid]; + struct timeval tv, *tvp; /* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */ sanitise_stdfd(); @@ -1242,13 +1265,16 @@ skip: nalloc = 0; while (1) { - prepare_select(&readsetp, &writesetp, &max_fd, &nalloc); - if (select(max_fd + 1, readsetp, writesetp, NULL, NULL) < 0) { + tvp = &tv; + prepare_select(&readsetp, &writesetp, &max_fd, &nalloc, &tvp); + result = select(max_fd + 1, readsetp, writesetp, NULL, tvp); + reaper(); /* remove expired keys */ + if (result < 0) { if (errno == EINTR) continue; fatal("select: %s", strerror(errno)); - } - after_select(readsetp, writesetp); + } else if (result > 0) + after_select(readsetp, writesetp); } /* NOTREACHED */ }