Bug 884 - DSA keys (id_dsa.pub) with 8192 bits or more aren't correctly recognized
Summary: DSA keys (id_dsa.pub) with 8192 bits or more aren't correctly recognized
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 3.8.1p1
Hardware: All Linux
: P2 enhancement
Assignee: OpenSSH Bugzilla mailing list
URL:
Keywords: openbsd, patch
Depends on:
Blocks: 914
  Show dependency treegraph
 
Reported: 2004-06-24 02:40 AEST by Alessandro Di Marco
Modified: 2005-03-10 09:07 AEDT (History)
0 users

See Also:


Attachments
changes on the fly the line buffer size in key_try_load_public() (1.63 KB, patch)
2004-06-24 02:42 AEST, Alessandro Di Marco
no flags Details | Diff
discard excessively long lines in key_try_load_public (903 bytes, patch)
2004-06-24 15:30 AEST, Darren Tucker
no flags Details | Diff
Allow large keys to work. (1.33 KB, patch)
2004-06-24 17:16 AEST, Darren Tucker
no flags Details | Diff
lets openssh to accept and use very large keys (>8Kbits) (2.91 KB, patch)
2004-06-24 23:40 AEST, Alessandro Di Marco
no flags Details | Diff
Allow arbitary sized rsa1, rsa & dsa keys (7.40 KB, patch)
2004-06-25 17:40 AEST, Darren Tucker
no flags Details | Diff
Update patch #633 to OpenBSD -current (7.43 KB, patch)
2004-10-29 22:03 AEST, Darren Tucker
no flags Details | Diff
Regression test for big key problem. (1.92 KB, patch)
2004-10-29 22:06 AEST, Darren Tucker
no flags Details | Diff
testkeys.tar.gz: 4kbit - 32kbits keys for regression test (41.91 KB, application/octet-stream)
2004-10-29 22:10 AEST, Darren Tucker
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alessandro Di Marco 2004-06-24 02:40:32 AEST
Hello,
this is what I get using ssh with a 8192 bytes DSA key (produced with
'ssh-keygen -t dsa -b 8192'):

dmr:nicetas:0:~/openssh-3.8.1p1$ ./ssh bin
key_read: uudecode
AAAAB3NzaC1kc3MAAAQBAKU+5z15VrrvSBD9q5absauzckc0oinS4DOoOGPHwHoPKdjXU/B2/Z1yxtS7hbafix5gNeHbX5QTPq2f7YVZc/P0x0QRxDAMaADOVymH3DhxOt7pV0KxA8uiiqWNsWrVGFKTfh6NF5Ap8lcrRMu30sZvKUgH+3xaBEUiIXkV7yrWvmdk84llDwLInzxLVXHMWGgvdEPM+t0/bL0JIUe7nXzRO8cdXP2AhvCjAMa+l5GcLH2ZIxaEkDohJG5NgDg5FmJF37bs8vwNnyrrIigxLIwroCfhiirpahRNStdXXV57vYeCOWvqr6yYcXSJD5AuZoHl2MAgje5bwVIP/Va3F3WLBfgQwQVRoyssr8BPACPz8liVgGyZPUsilfF8Jf/BFa9VEPaQNXDN1XunqJPGg35dxMFGhPkjd8DcxKo0jqM+DNI/YbKUe3hQ4P4jDfzE7PcmLWzm80cpZxZO7ODLMQumqiGmo9uTm4dRwI/oxYk8pW7VComq7BTV8roPnw0Q9BwdBWtKXFI3adyq/Z4qghPUTuwPyx1Lgl3Q2k9J+CkHzf/Y9ycAzZUlkLIrsknh1ZVJgiCQIQQhfCrbIM3cdnXtkneJKWsfjyiN2lKNt8tzRjai30zHPWjltc5FiZsHBbwaHWa0uDLyUxtB8ZHCjd1HBafA6isqk8MaJdV47Iw1H1I3ny//5KLTatiIHhpF61wXXRExkWGO9dy635hWCmZiOyFnXpZY6ErdhlHZ5VwSXX0tSG97eS+YG1TMTljaSGD+eKXLwJVTwICC5Hn7qaGwFEY0Zuzo2dnZNVi6EVMtSavC0QZ/rAt2/QtZys8MfDybVRAETfmEcdEY6A2yTk6KlF2wggAySPH7pY6LF2qugrjkVLj/0/cRYv9jCfEzjd8qS3As71U1bO39EtIM/niHS7blF0CN5XsA9x04ZUiW+0y0MUfGYf2CqVLU0fxNR5/wFw8hMDKQvw2J8vhbClq3Last
login: Wed Jun 23 18:26:04 2004 from nicetas.disi.unige.it
root:bin:0:~# 

This seems had to the line buffer size found in key_try_load_public() of
authfile.c that is fixed to 4096 bytes.

Please found here below a patch of mine that _should_ help to solve the problem,
enlarging - on fly - the buffer size.

Regards,
Alessandro
Comment 1 Alessandro Di Marco 2004-06-24 02:42:30 AEST
Created attachment 658 [details]
changes on the fly the line buffer size in key_try_load_public()
Comment 2 Alessandro Di Marco 2004-06-24 10:01:51 AEST
Oops, I've written: 'this is what I get using ssh with a 8192 bytes DSA key...'
but I wanted to say 'this is what I get using ssh with a 8192 bits DSA key...'.
Sorry.

Unfortunately it seems that the key size has been hard-wired in some other
place, because every attempt to raise the keysize over 8192 bits limit won't
work at all.
Comment 3 Darren Tucker 2004-06-24 15:27:38 AEST
> every attempt to raise the keysize over 8192 bits limit won't work

Maybe a limit in OpenSSL's DSA functions?

Your read_whole_line() function seems a lot more complicated than it needs to
be.  xrealloc will leave the file content intact, so instead of ftell/fseek/goto
you can just initialise everthing to zero/NULL then just repeatedly read chunks
until you get a newline, eg,

while (1) {
        buf = xrealloc(buf, size + CHUNKSZ);
        size += CHUNKSZ;
        nextpart = buf + len;
        if (fgets(nextpart, CHUNKSZ, f) == NULL)
                break;
        len += strlen(nextpart);
        if (buf[len - 1] == '\n')
                break;
}

(For bonus points, add some better handling of fgets() == NULL.  Or use
buffer_append instead of xrealloc.)

Anyway, an 8kbit key is just over 4KBytes encoded and I don't think it's worth
the extra complexity of making it dynamic just to save a couple of KB of stack
space.

Either way key_try_load_public shouldn't pass incomplete lines to key_read, though.
Comment 4 Darren Tucker 2004-06-24 15:30:39 AEST
Created attachment 659 [details]
discard excessively long lines in key_try_load_public
Comment 5 Darren Tucker 2004-06-24 17:16:23 AEST
Created attachment 660 [details]
Allow large keys to work.

Try this patch, tested OK with up to 16 kbit keys.
Comment 6 Alessandro Di Marco 2004-06-24 23:36:27 AEST
> > every attempt to raise the keysize over 8192 bits limit won't work
> 
> Maybe a limit in OpenSSL's DSA functions?
> 
> Your read_whole_line() function seems a lot more complicated than it needs to
> be.  xrealloc will leave the file content intact, so instead of ftell/fseek/goto
> you can just initialise everthing to zero/NULL then just repeatedly read chunks
> until you get a newline, eg,
> 
> while (1) {
>         buf = xrealloc(buf, size + CHUNKSZ);
>         size += CHUNKSZ;
>         nextpart = buf + len;
>         if (fgets(nextpart, CHUNKSZ, f) == NULL)
>                 break;
>         len += strlen(nextpart);
>         if (buf[len - 1] == '\n')
>                 break;
> }
> 
> (For bonus points, add some better handling of fgets() == NULL.  Or use
> buffer_append instead of xrealloc.)

You are right... but that patch was only a quick and dirty trick... that
function has been stripped down from another place only for testing purposes.
However now it shoud be ok.
 
> Anyway, an 8kbit key is just over 4KBytes encoded and I don't think it's
> worth the extra complexity of making it dynamic just to save a couple of KB
> of stack space.

IMHO the problem here is that the maximum key size is hard coded in openssh,
while it should be _at most_ hard coded in (e.g.) openssl...
 
> Either way key_try_load_public shouldn't pass incomplete lines to key_read,
  though.

I agree... However, the following patch should fix - it worked pretty fine for
me - any key-size troubles: tested ok with 32K keys, actually in production on
my cluster and several other systems.

Thanks a lot for your support.

Regards,
Alessandro
Comment 7 Alessandro Di Marco 2004-06-24 23:40:31 AEST
Created attachment 661 [details]
lets openssh to accept and use very large keys (>8Kbits)
Comment 8 Darren Tucker 2004-06-25 17:40:06 AEST
Created attachment 663 [details]
Allow arbitary sized rsa1, rsa & dsa keys

Current WIP patch for dynamically allocating memory for keys.
Tests OK for 4kbit -> 16kbit keys (I have a regression test for this which I'll
attach separately).
Comment 9 Darren Tucker 2004-10-29 22:03:59 AEST
Created attachment 733 [details]
Update patch #633 to OpenBSD -current
Comment 10 Darren Tucker 2004-10-29 22:06:30 AEST
Created attachment 734 [details]
Regression test for big key problem.

Tested up to 32 kbit keys.  Needs test keys in next attachment.
$ make t-bigkey
[...]
try proto 1 key type rsa1 size 4096 bits
try proto 1 key type rsa1 size 8192 bits
try proto 1 key type rsa1 size 16384 bits
try proto 1 key type rsa1 size 32768 bits
try proto 2 key type dsa size 4096 bits
try proto 2 key type dsa size 8192 bits
try proto 2 key type dsa size 16384 bits
try proto 2 key type dsa size 32768 bits
try proto 2 key type rsa size 4096 bits
try proto 2 key type rsa size 8192 bits
try proto 2 key type rsa size 16384 bits
try proto 2 key type rsa size 32768 bits
ok big keys
Comment 11 Darren Tucker 2004-10-29 22:10:47 AEST
Created attachment 735 [details]
testkeys.tar.gz: 4kbit - 32kbits keys for regression test

To use for regess tests, untar into /usr/src/regress/usr.bin/ssh.
Comment 12 Darren Tucker 2004-12-06 23:29:01 AEDT
We decided not to allow arbitrary-sized keys.  RSA keys up to 16 kbits and DSA
keys up to 8 kbits now work fine, and the other things have been fixed: the
existing static limits have been moved into a #define and documented, and
over-length lines will be discarded.
Comment 13 Darren Tucker 2005-03-10 09:07:16 AEDT
With the release of OpenSSH 4.0, these bugs are now closed. For details, see:
http://www.openssh.com/txt/release-4.0