Date: Tue, 13 Feb 2001 21:13:58 -0300
From: =?iso-8859-1?Q?Iv=E1n_Arce?= <[email protected]>
To: [email protected]Subject: SSH1 key recovery patch
Hello,
In light of the recent posts to bugtraq concerning the
CORE SDI advisory that describes the SSH1 session
key recovery vulnerability a few things needs to be
noted:
- CORE SDI does not provide support services to
SSH1 and does not maintain its source tree. However,
given the installed base of SSH1 server we considered
convenient to include a patch for the problem found.
Although SSH.com does not support SSH1 anymore,
and it was made clear that their prefered action is to
switch to SSH2 to solve the problem, an untested and
unsupported patch was provided.
The provided patch had two flaws:
- Arguments to the kill function were reversed preventing
it from actually fixing the bug. This problem was spotted
and reported by Matt Power from the Bindview RAZOR
Team.
- Even with the above problem corrected the patch does
prevent exploitation of the vulnerability BUT it opens
the door to a Denial of Service attack against the SSH
server.
I'll comment of the original patch below.
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.
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.
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.
This is not tested other than verifying that it applies correctly
and compiles without errors.
Our advisory will reflect this change, the patch will be available
at ftp://ftp.core-sdi.com/pub/patch/ssh1-keyrecovery.patch
shortly.
- An interesting observation is how several people failed to
see the wrongness of the multiple patches, starting with
the person that produced it, me (ouch!, my bad) failing
to spot 2 different problems in it, guys at the Bindview RAZOR Team
and even Andrew Brown who pointed out that the patch was no good
but for the wrong reasons.
To me this had two outcomes:
. shame (i should have found the problems!)
. a reaffirmation of my belief that peer review and full disclosure
are good things.
- Note also that the path taken by other SSH vendor to address
this problem is different. For example, OpenSSH and others chose
to completly close the information leakage by setting a random
session key when RSA operations fail and by regenerating the key
after a given number of connections to the SSH server.
-ivan
This applies to the source distribution of ssh-1.2.31
decompress and untar the ssh-1.2.31
$ cd ssh-1.2.31
$ patch -p0 < ssh1-keyrecovery.patch
$ ./configure
$ make
----------------------------------- begin ssh1-keyrecovery.patch -------
--- rsaglue.c Wed Jan 17 11:42:52 2001
+++ rsaglue.c Tue Feb 13 16:05:33 2001
@@ -264,8 +264,10 @@
mpz_clear(&aux);
if (value[0] != 0 || value[1] != 2)
+ {
+ kill(getppid(),SIGALRM);
fatal("Bad result from rsa_private_decrypt");
-
+ }
for (i = 2; i < len && value[i]; i++)
;
--- sshd.c Wed Jan 17 11:42:53 2001
+++ sshd.c Tue Feb 13 16:05:15 2001
@@ -757,9 +757,11 @@
RETSIGTYPE key_regeneration_alarm(int sig)
{
+ static time_t last_keygen_time=0;
/* Check if we should generate a new key. */
- if (key_used)
- {
+ if (key_used && (time(NULL) - last_keygen_time > 60))
+ {
+ last_keygen_time = time(NULL);
/* This should really be done in the background. */
log_msg("Generating new %d bit RSA key.", options.server_key_bits);
random_acquire_light_environmental_noise(&sensitive_data.random_state);
----------------------------------- end ssh1-keyrecovery.patch -------
---
"Understanding. A cerebral secretion that enables one having it to know
a house from a horse by the roof on the house,
Its nature and laws have been exhaustively expounded by Locke,
who rode a house, and Kant, who lived in a horse." - Ambrose Bierce
==================[ CORE Seguridad de la Informacion S.A. ]=========
IvАn Arce
Presidente
PGP Fingerprint: C7A8 ED85 8D7B 9ADC 6836 B25D 207B E78E 2AD1 F65A
email : [email protected]http://www.core-sdi.com
Florida 141 2do cuerpo Piso 7
C1005AAG Buenos Aires, Argentina.
Tel/Fax : +(54-11) 4331-5402
--- For a personal reply use [email protected]
