Bug 1565 - ssh-keyscan doesn't like comment-lines
Summary: ssh-keyscan doesn't like comment-lines
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: Miscellaneous (show other bugs)
Version: 5.1p1
Hardware: All Linux
: P2 minor
Assignee: Assigned to nobody
URL:
Keywords: low-hanging-fruit
Depends on:
Blocks: V_5_6
  Show dependency tree / graph
 
Reported: 2009-03-04 20:00 AEDT by Martin Schuster
Modified: 2011-01-24 12:33 AEDT (History)
3 users (show)

See Also:

Attachments
Patch to fix integer overflow in fgets() wrapper (4.60 KB, patch)
2010-03-07 08:56 AEDT, Joachim Schipper 		no flags Details | Diff
/home/djm/keyscan-uncrazy.diff (4.24 KB, patch)
2010-06-18 13:40 AEST, Damien Miller 		no flags Details | Diff
/home/djm/keyscan-uncrazy.diff (4.24 KB, patch)
2010-06-18 13:41 AEST, Damien Miller 		no flags Details | Diff
/home/djm/keyscan-uncrazy.diff (4.18 KB, patch)
2010-06-18 13:44 AEST, Damien Miller 		dtucker: ok+
Details | Diff
Patch to fix ssh-keyscan (4.33 KB, patch)
2010-06-18 19:56 AEST, Joachim Schipper 		no flags Details | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Schuster 2009-03-04 20:00:19 AEDT 
ssh-keyscan can take an existing known_hosts file as input, but only if it contains no comment-lines.

To reproduce:
echo '#' > khtest
ssh-keyscan -f khtest

Result:
getaddrinfo #: Name or service not known

Expected:
Nothing happens
Comment 1 Joachim Schipper 2010-03-07 08:56:52 AEDT 
Created attachment 1806 [details]
Patch to fix integer overflow in fgets() wrapper
Comment 2 Joachim Schipper 2010-03-07 08:58:07 AEDT 
As described in http://mid.gmane.org/20100306210548.GA32662@polymnia.sshunet.nl, ssh-keyscan may suffer an integer overflow when run on a file with ridiculously (> 2GB) long lines. The attached patch fixes this and also allows comments.
Comment 3 Damien Miller 2010-06-18 13:40:03 AEST 
Created attachment 1868 [details]
/home/djm/keyscan-uncrazy.diff

use read_keyfile_line()\n\nWe already have a fgets() wrapper, let's use it.
Comment 4 Damien Miller 2010-06-18 13:41:55 AEST 
Created attachment 1869 [details]
/home/djm/keyscan-uncrazy.diff

revised diff
Comment 5 Damien Miller 2010-06-18 13:42:58 AEST 
Comment on attachment 1869 [details]
/home/djm/keyscan-uncrazy.diff

ugh, attached the wrong diff twice :(
Comment 6 Damien Miller 2010-06-18 13:44:02 AEST 
Created attachment 1870 [details]
/home/djm/keyscan-uncrazy.diff

The original diff didn't correctly handle the case of "ssh-keyscan -f -" (it would SEGV or EINVAL on fopen). This one uses our existing wrapper for fgets().
Comment 7 Joachim Schipper 2010-06-18 19:56:01 AEST 
Created attachment 1875 [details]
Patch to fix ssh-keyscan

The attached patch is a slight alteration of your (Damien's) patch.

- these lines are not related to SSH_MAX_PUBKEY_BYTES, so just hardcode some reasonable value;
- linenum should be per-file, not over all files;
- fatal() on long lines instead of silently ignoring them.
Comment 8 Darren Tucker 2010-06-22 14:54:29 AEST 
(In reply to comment #7)
> Created attachment 1875 [details]
> Patch to fix ssh-keyscan
> 
> The attached patch is a slight alteration of your (Damien's) patch.
> 
> - these lines are not related to SSH_MAX_PUBKEY_BYTES, so just hardcode
> some reasonable value;
> - linenum should be per-file, not over all files;

These are both valid points.

> - fatal() on long lines instead of silently ignoring them.

This one I don't care so much about.
Comment 9 Damien Miller 2010-06-22 14:55:43 AEST 
I agree with Darren. The corresponding patch has been committed and will be in OpenSSH-5.6. Thanks!
Comment 10 Damien Miller 2011-01-24 12:33:55 AEDT 
Move resolved bugs to CLOSED after 5.7 release