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