Bug 884

Created attachment 658 [details]
changes on the fly the line buffer size in key_try_load_public()

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.

> 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.

Created attachment 659 [details]
discard excessively long lines in key_try_load_public

Created attachment 660 [details]
Allow large keys to work.

Try this patch, tested OK with up to 16 kbit keys.

> > 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

Created attachment 661 [details]
lets openssh to accept and use very large keys (>8Kbits)

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).

Created attachment 733 [details]
Update patch #633 to OpenBSD -current

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

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.

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.

With the release of OpenSSH 4.0, these bugs are now closed. For details, see:
http://www.openssh.com/txt/release-4.0