Re: SSH1 key recovery patch

[Andrew Brown <atatat () ATATDOT NET>]

> 1)    {
> 2)      static time_t last_kill_time = 0;
> 3)      if (time(NULL) - last_kill_time > 60 && getppid() != 1)
> 4)        {
> 5)         last_kill_time = time(NULL);
> 6)         kill(SIGALRM, getppid());
> 7)       }
> 8)      fatal("Bad result from rsa_private_decrypt");
> 9)    }
>
> The rationale for the above fix is to regenerate the key whenever
> a RSA operation failed - a symptom of an attacker trying to
> execute the key recovery attack- this does not close the information
> leakage in the ssh server (namely the existence of an oracle that can
> be used to infer a session key) but it does make IMPOSSIBLE to
> exploit it.

that much was clear.  i just saw that it didn't actually work.

> The patch intends to limit the key regeneration rate to
> at most one regeneration per minute, in order to prevent
> a DoS.
> The problem here is that the code is executed in the context
> of an sshd *child* process and therefore after the first
> key regeneration the child process exits (in the fatal() function)
> loosing all notion of the last time the key was regenerated.
> This was spotted and reported to bugtraq by Andrew Brown, altho
> the explaination of why it didnt work was not correct.

i think my explanation was correct (how was it not?) even though it
might perhaps have depended a little too much on people knowing the c
language a little more in depth.

i received no small number of personal emails explaining to me that my
analysis was incorrect based on my "misunderstanding" of the keyword
static.  some people though i misread it as "auto" and some others
believed i thought it meant "const".  i just didn't feel that quoting
chapter and verse from the c standard would benefit the list at all.

> Below, is a new patch that does what it was originally intended.
> The rate limit for key regeneration is now put in the alarm signal
> handler and gets executed in the context of the sshd daemon
> responsible for doing it.

much better.

>       /* This should really be done in the background. */

aside: fork, and write the new key back up a pipe after regeneration
is finished.  ssh already depends on fork and pipe semantics, so this
ought to be trivial.  sure, it's brutal and ugly, but then you don't
end up delaying connections because the server is busy.

--
|-----< "CODE WARRIOR" >-----|
codewarrior () daemon org             * "ah!  i see you have the internet
twofsonet () graffiti com (Andrew Brown)                that goes *ping*!"
andrew () crossbar com       * "information is power -- share the wealth."