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.