chiark / gitweb /
Commit 2.4.5-5 as unpacked
[innduct.git] / doc / compliance-nntp
1 $Id: compliance-nntp 6817 2004-05-18 09:25:55Z rra $
2
3 The following are outstanding issues regarding INN's compliance with the
4 NNTP standard.  The reference documents used in this analysis are the
5 current NNTP IETF Working Group draft (draft-ietf-nntpext-base-15.txt at
6 the time of the last check of this audit) or RFC 2980, not RFC 977 (which
7 is woefully out of date).
8
9 This file documents only compliance issues with the latest version of the
10 standard NNTP protocol.  It does not cover INN's private extensions or
11 INN's implementation of widely available extensions not documented in the
12 NNTP standard.  Specifically, it does not cover the extensions listed in
13 RFC 2980.
14
15 ------------------------------
16
17   Summary: innd doesn't require whitespace between commands and arguments
18  Standard: draft-ietf-nntpext-base-15.txt, section 4
19   Version: 1.0 to CURRENT 2002-12-26
20 Reference: innd/nc.c NCproc() and command handlers
21  Severity: Accepts invalid input
22
23 The standard states:
24
25     Keywords and arguments MUST be each separated by one or more US-ASCII
26     SPACE or US-ASCII TAB characters.
27
28 This is not checked in NCproc or in the individual command handlers in
29 innd.  Commands followed immediately by their argument will be accepted by
30 innd.  For example:
31
32     stat<9k6vjk.hg0@example.com> 
33     223 0 @0301543531000000000000079AAE0000006A@
34
35 Impact:  Should one command be a prefix of another, innd could dispatch
36 the handling of the command to the wrong handler, treating the remainder
37 of the command verb as an argument.  This laxness also encourages sloppy
38 client code.  Internally, the lack of argument parsing in NCproc also
39 results in code duplication in all of the command handlers.
40
41 Suggested fix:  Lift the argument parsing code into a function called from
42 NCproc, breaking the command line into a vector of command and arguments.
43 This will work for all commands implemented by innd and will simplify the
44 implementation of command handlers, as well as fixing this problem.  This
45 is what nnrpd already does.
46
47 Impact of fix:  It's possible that some serving code is relying on this
48 behavior and not sending spaces after commands.  Fixing this problem would
49 break interoperability with that code.
50
51 ------------------------------
52
53   Summary: INN doesn't check argument length
54  Standard: draft-ietf-nntpext-base-15.txt, section 4
55   Version: 1.0 to CURRENT 2002-12-26
56 Reference: innd/nc.c and nnrpd/nnrpd.c
57  Severity: Accepts invalid input
58
59 The standard says:
60
61     Arguments MUST NOT exceed 497 octets.
62
63 This is not checked by either innd or nnrpd, although both do check that
64 the command itself does not exceed 512 octets.
65
66 Impact:  Small.  May accept invalid commands in extremely rare edge cases.
67
68 Suggested fix:  Probably not worth fixing separately, although if standard
69 command parsing code is written to handle both innd and nnrpd, it wouldn't
70 hurt to check this along with everything else.
71
72 ------------------------------
73
74   Summary: Reply codes other than x9x used for private extensions
75  Standard: draft-ietf-nntpext-base-15.txt, section 4.1
76   Version: 1.0 to CURRENT 2002-12-26
77 Reference: include/nntp.h
78  Severity: Violates SHOULD
79
80 The standard says:
81
82     Response codes not specified in this standard MAY be used for any
83     installation-specific additional commands also not specified.  These
84     SHOULD be chosen to fit the pattern of x9x specified above.
85
86 INN uses quite a few response codes that do not fit this pattern for
87 various extensions.  Some of these will likely later be standardized with
88 the response codes that INN uses (the streaming commands, the
89 authentication reply codes, and possibly the STARTTLS reply codes), but
90 the rest (XGTITLE, MODE CANCEL, and XBATCH) should have used response
91 codes in the x9x range.
92
93 Impact:  Additional ambiguity over the meaning of reply codes, as those
94 reply codes could later be standardized as the reply codes for other
95 commands.
96
97 Suggested fix:  For XGTITLE and probably XBATCH, there is no way to fix
98 this now.  Changing the reply codes would break all existing
99 implementations.  It may still be possible to change the reply codes for
100 MODE CANCEL (which should probably be MODE XCANCEL), but it may not be
101 worth the effort.
102
103 ------------------------------
104
105   Summary: innd may return 480 instead of 500 for unrecognized commands
106  Standard: draft-ietf-nntpext-base-15.txt, section 4.1.1
107   Version: 1.0 to CURRENT 2002-12-26
108 Reference: innd/nc.c NCauthinfo()
109  Severity: Violates MUST
110
111 The standard says:
112
113     If the command is not recognized, or it is an optional command or
114     extension that is not implemented by the server, the response code 500
115     MUST be returned.
116
117 In innd, if the connection is determined to need authentication, all
118 incoming commands other than MODE are handed off to NCauthinfo() rather
119 than their normal command handlers.  NCauthinfo() responds with a 480
120 reply code to anything other than AUTHINFO USER, AUTHINFO PASS, or QUIT.
121
122 Impact:  Unlikely to cause problems in practice, but may confuse clients
123 that don't understand the rarely used innd-level authentication
124 mechanisms.
125
126 Suggested fix:  Restructure the command table so that each command also
127 has a flag indicating whether it requires authentication for peers that
128 are required to authenticate.  (Some commands, like HELP and MODE READER,
129 should be allowed without authentication.)  Then eliminate the special
130 casing of the state CSgetauth (it may be better to store whether the peer
131 has authenticated in the channel rather than in the channel state) and the
132 special handling in NCauthinfo.  This should also simplify the code.
133
134 ------------------------------
135
136   Summary: innd always sends 200 for an accepted connection
137  Standard: draft-ietf-nntpext-base-15.txt, section 7.1
138   Version: 1.0 to CURRENT 2002-12-26
139 Reference: innd/nc.c NCsetup() and rc.c RCreader()
140  Severity: Violates MUST
141
142 The standard says:
143
144     If the server will accept further commands from the client including
145     POST, the server MUST present a 200 greeting code.  If the server will
146     accept further commands from the client, but it is not authorized to
147     post articles using the POST command, the server MUST present a 201
148     greeting code.
149
150 The implication is that the greeting code from innd (which doesn't
151 implement POST and therefore is never going to allow it) should always be
152 201, at least for the case where innd never spawns nnrpd.  In the case
153 where innd spawns nnrpd, it's unclear what the greeting code should be.
154
155 The current implementation nevers send 201 unless one knows for certain
156 that the connection will never be allowed to issue a POST command, which
157 means that innd always sends 200.
158
159 It's unknown whether there is any transit news software that would have
160 difficulties with a 201 greeting.  Both innxmit and innfeed handle it
161 correctly in CURRENT 2001-07-04 and NNTPconnect() handles it correctly in
162 INN 1.0, so it seems likely that if any such software exists, it's rare.
163
164 Impact:  It's almost certain that the current innd behavior isn't hurting
165 anything.  Even a confused client that thought 200 meant that it could
166 send a POST command would then try and be rejected with no damage done.
167
168 Suggested fix:  The purpose of this return code is to give a hint to a
169 reading client indicating whether it should even attempt POST, since
170 attempting it may involve a lot of work by the user only to have the post
171 rejected.  It's only relevant to reading connections, not to transit
172 connections.
173
174 It's known that some clients, upon seeing a 201 response, will never
175 attempt POST, even if MODE READER then returns 200.  Therefore innd, when
176 handing off connections to nnrpd, must return 200 to not confuse a client
177 that will later send MODE READER.  For connections where innd won't do
178 that handoff, it makes sense to always send 201 if all transit feeds can
179 handle that and won't interpret it as unwillingness to accept IHAVE or
180 streaming feeds.
181
182 RCreader() should therefore be modified to send 201 if noreader is set,
183 and otherwise send 200.
184
185 Impact of fix:  Any feeding software that didn't consider 201 to be a
186 valid greeting would be unable to feed a fixed innd unless that innd also
187 allowed reading connections.
188
189 ------------------------------
190
191   Summary: innd doesn't support LIST EXTENSIONS
192  Standard: draft-ietf-nntpext-base-15.txt, section 8.1
193   Version: 1.0 to CURRENT 2002-12-26
194 Reference: innd/nc.c NClist()
195  Severity: Not a violation
196
197 Support for LIST EXTENSIONS is optional, and innd's current behavior
198 (returning a 500 response) is permitted by the standard, but it means that
199 innd cannot advertise any of the extensions that it supports.  Since this
200 will eventually include streaming, support should be added.
201
202 Suggested fix:  Add support for LIST EXTENSIONS to NClist() as soon as
203 innd supports a registered extension or as soon as there is documentation
204 for INN's extensions that specify an extension name (beginning with X).
205
206 ------------------------------
207
208   Summary: nnrpd doesn't return 423 errors when there is no overview info
209  Standard: draft-ietf-nntpext-base-17.txt, section 10.5.1.2
210   Version: 1.4 to CURRENT 2003-05-04
211 Reference: nnrpd/article.c CMDxover()
212  Severity: Violates a MUST
213
214 The standard says:
215
216    If there are no articles in the range specified, a 423 response MUST be
217    returned.
218
219 nnrpd (from the beginning of the XOVER command) has always returned a 224
220 response with an empty multiline response instead.  INN doesn't support
221 OVER yet so this isn't actually a bug in INN, but eventually the XOVER
222 implementation will also be used for OVER.
223
224 Impact:  Less information is communicated to the client about why there
225 are no overview records returned.  An error response indicating there are
226 no valid articles in that range is possibly more informative.
227
228 Suggested fix:  Don't print out the initial 224 message until at least one
229 overview entry has been found, so that CMDxover() can print a 420 response
230 instead if no overview records are found.
231
232 Impact of fix:  May confuse some clients that don't expect to get 420
233 errors back from overview queries.  It may be necessary to do something
234 different for OVER (where clients should expect this behavior since OVER
235 is a new command) than for XOVER (where clients may be relying on the
236 existing behavior.
237
238 ------------------------------
239
240   Summary: HDR can return message IDs rather than article numbers
241  Standard: draft-ietf-nntpext-base-17.txt, section 10.6.1.2
242   Version: 1.0 to CURRENT 2003-05-04
243 Reference: nnrpd/article.c CMDpat()
244  Severity: Violates a protocol description
245
246 The standard says:
247
248    The line consists of the article number, a US-ASCII space, and then the
249    contents of the header (without the header name or the colon and space
250    that follow it) or metadata item.  If the article is specified by
251    message-id rather than by article range, the article number is given as
252    "0".
253
254 nnrpd instead returns the message ID as the first word of the line when
255 HDR is given a message ID argument.
256
257 Impact:  A client may not be able to parse the output of HDR correctly,
258 since the first word is not a number.
259
260 Suggested fix:  Change the code to return 0 as the first word instead of
261 the message ID, per the standard.
262
263 Impact of fix:  Clients that are expecting the message ID may be
264 confused.
265
266 ------------------------------
267
268   Summary: innd improperly caches DNS returns
269  Standard: draft-ietf-nntpext-base-15.txt, section 14.4
270   Version: 1.0 to CURRENT 2002-12-26
271 Reference: innd/rc.c RCreadfile() and elsewhere
272  Severity: Violates a MUST
273
274 The standard says:
275
276     If NNTP clients or servers cache the results of host name lookups in
277     order to achieve a performance improvement, they MUST observe the TTL
278     information reported by DNS.
279
280 innd caches DNS lookups when reading incoming.conf and doesn't refresh its
281 knowledge of DNS except when incoming.conf is reloaded.
282
283 Impact:  An explicit reload is required whenever the IP address of any
284 peer changes, and in the presence of network renumbering innd is
285 vulnerable to spoofing if DNS is the only authentication mechanism used.
286
287 Suggested fix:  This is hard to fix without unacceptable performance
288 impact.  The only good fix is to either fork a separate helper process to
289 do DNS lookups (since gethostbyname may block for essentially an
290 arbitrarily long period) or to use the direct resolver library so that one
291 can get access to a file descriptor and throw it into the select loop.
292 Either way, this requires keeping a DNS file descriptor in the main select
293 loop and updating knowledge of DNS periodically, which is a substantial
294 amount of additional complexity.
295
296 ------------------------------
297
298   Summary: innd doesn't diagnose repeated AUTHINFO USER commands
299  Standard: RFC 2980, section 3.1.1
300   Version: 1.0 to CURRENT 2002-12-26
301 Reference: innd/nc.c NCauthinfo()
302  Severity: Violates a protocol description
303
304 RFC 2980 says:
305
306     The 482 code will also be returned when the AUTHINFO commands are not
307     entered in the correct sequence (like two AUTHINFO USERs in a row, or
308     AUTHINFO PASS preceding AUTHINFO USER).
309
310 innd ignores AUTHINFO USER and just always returns a 381 response, however,
311 since it doesn't care about the username.
312
313 Impact:  Probably none.
314
315 Suggested fix:  A long-term solution would be to add real authentication
316 to innd, in which case it would start caring about the authenticated
317 identity (and perhaps use that identity to map to an incoming.conf entry).
318 It's unclear if this would be worthwhile.  Failing that, innd would need
319 to keep internal state to know whether AUTHINFO USER had already been
320 sent.