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.
-------------- next part --------------
# 1 this is a very long comment line xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxx
# 2 this is a very long comment line xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxx
# 3 this is a very long comment line xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxx
# 4 this is a very long comment line xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxx
# 5 this is a longer comment line xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxABCDEFG
Host whatever
Hostname localhost
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. > > > > > > > >
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. >> >> >> >> >> >> >> >>