Damien, thanks for your answer. any comments on ancillary issues?
* should the limit be documented, or removed? or should the limit
just be increased?
* is there any mechanism to keep the code in libopenssh in sync with
the corresponding code in openssh-portable? i would think that as a
philosophical matter, openssh-portable should be using the same code
as libopenssh where possible. particularly for things like readconf.c .
(in fact, as a newcomer to this project, i am wondering why
openssh-portable couldn't simply link in libopenssh as a library?
why are they two separate github repos, instead of one repo that
can build both the executable and the library?)
* is there a missing assignment to active (*activep) in the
openssh version of readconf.c, is the comment incorrect, or?
On 12/18/16 19:52, Damien Miller wrote:
> Hi,
>
> This does look like a bug, so it would be great if you could file it on
> bugzilla.
>
> Thanks,
> Damien
>
> On Sun, 18 Dec 2016, Don Fong wrote:
>
>> To all,
>>
>> i think i've found a minor bug in openssh. i'm writing to the list
>> toget input on whether it's really a bug, or an undocumented limit,
>> or maybe it's even documented somewhere (although i didn't see
>> it documented in ssh_config(5)). if there is a consensus that this
>> is indeed a bug, i'll file it in bugzilla. i would also like to
>> submit the fix.
>>
>> the bug is that ssh barfs if my .ssh/config file contains a very
>> long comment line. basically it tries to parse anything beyond
>> the 1022-th char as regular input, not a comment.
>>
>> i discovered this behavior by accident. i have an ansible script
>> that spins up new AWS instances. it creates an ssh config stanza
>> for eachnew instance. at the start of the stanza, the script puts
>> a commentline containing amazon's JSON description of the instance.
>> dueto recent changes made by amazon, the description is longer than
>> it used to be, making the comment longer. then ssh started failing.
>>
>> example (using the attached config file verylong.config):
>>
>> $ ssh -F verylong.config whatever
>> verylong.config: line 9: Bad configuration option: ABCDEFG
>> verylong.config: terminating, 1 bad configuration options
>>
>> i decided to take a look at the ssh source code. i think it is
>> pretty clear what is going wrong.
>>
>> ----- readconf.c:
>> 1703 char line[1024];
>> ...
>> 1730 while (fgets(line, sizeof(line), f)) {
>> 1731 /* Update line number counter. */
>> 1732 linenum++;
>> 1733 if (process_config_line_depth(options, pw, host,
>> original_host,
>> 1734 line, filename, linenum, activep, flags, depth)
>> != 0)
>> 1735 bad_options++;
>> 1736 }
>>
>> if fgets() runs across a very long input line, whatever won't fit in
>> the given buffer (sizeof line) is left unread on the input stream,
>> to be picked up by later reads. this is the documented behavior of
>> fgets(). how long is too long? the buffer is sized at 1024 chars.
>> one char is needed for the null terminator, another one for the
>> newline. so anything longer than 1024-2 = 1022 bytes is too big.
>>
>> unf readconf.c as written just naively assumes that fgets() returns the
>> entire line. it makes no attempt to deal with the case where the line
>> didn't entirely fit. so basically, a long line is treated as multiple
>> lines. this is true whether the line was a comment or something else.
>> it's just that the behavior stands out more for a comment line, which
>> "should" be completely ignored. besides, there's not much reason for
>> any other type of line to be that long, in the config file.
>>
>> i see basically the same problem in the libopenssh version of readconf.c.
>>
>> IMHO this is a bug. some might consider it to be a reasonable limit
>> on the line length, but in that case it should be documented in
>> ssh_config(5). and in either case, i think line[] should be declared
>> using a symbolic constant for the length.
>>
>> or, get rid of the fixed-length buffer, and implement a dynamically
>> sized buffer instead. since i've only begun to look at this code,
>> i'm not sure this would be a safe thing to do. is there any other
>> code that implicitly assumes that the line length is less than 1024?
>>
>> incidentally, i see a strange discrepancy between the openssh-portable
>> version and the libopenssh version. before the while-fgets loop,
>> there is this comment (both versions):
>>
>> 1095 /*
>> 1096 * Mark that we are now processing the options. This flag
>> is turned
>> 1097 * on/off by Host specifications.
>> 1098 */
>> 1099 active = 1;
>>
>> but the "active=1" (line 1099) appears only in the libopenssh version,
>> not the openssh-portable version.
>>
>>
>>
>>
>>
>>
>>
>>
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
