[This patch improves the error handling of the CVS server, so that it can log errors rather than just ignoring them. At some point might be nice to add the ability to configure in the ability to use syslog(). For this, setting things like debug levels, and other reasons, probably should be adding an option for a global config file (where the log file is specified), rather than a --log option directly. Ability to write dates to log file? Needs documentation (cvs.texinfo, NEWS). -kingdon] Index: ChangeLog =================================================================== RCS file: /home2/cvsroot/ccvs/src/ChangeLog,v retrieving revision 1.1421 diff -c -r1.1421 ChangeLog *** ChangeLog 1998/01/30 22:41:28 1.1421 --- ChangeLog 1998/02/03 17:03:42 *************** *** 1,3 **** --- 1,11 ---- + Tue Feb 3 11:44:01 1998 Jim Kingdon + + * error.c, error.h: New variable conn_logfile. + * main.c: Set it based on --log option. + * server.c (pserver_authenticate_connection): Check for errors and + log them to conn_logfile. + (getline_checked): New function, helps with this. + Fri Jan 30 11:32:41 1998 Jim Kingdon * sanity.sh: Also test "first-dir" as the regexp in loginfo in Index: error.c =================================================================== RCS file: /home2/cvsroot/ccvs/src/error.c,v retrieving revision 1.19 diff -c -r1.19 error.c *** error.c 1997/11/30 21:51:04 1.19 --- error.c 1998/02/03 17:03:01 *************** *** 253,255 **** --- 253,260 ---- if (status) error_exit (); } + + /* This log file is for connection type problems. FIXME: My thoughts + in terms of what goes in this file are pretty ad hoc. We probably want + to think about this more carefully before we make this official. */ + FILE *conn_logfile; Index: error.h =================================================================== RCS file: /home2/cvsroot/ccvs/src/error.h,v retrieving revision 1.7 diff -c -r1.7 error.h *** error.h 1997/11/30 21:51:05 1.7 --- error.h 1998/02/03 17:03:30 *************** *** 54,57 **** --- 54,60 ---- process and packages up its stderr in the protocol. */ extern int error_use_protocol; + /* See comment in error.c. */ + extern FILE *conn_logfile; + #endif /* _error_h_ */ Index: main.c =================================================================== RCS file: /home2/cvsroot/ccvs/src/main.c,v retrieving revision 1.124 diff -c -r1.124 main.c *** main.c 1998/01/30 16:34:18 1.124 --- main.c 1998/02/03 17:04:38 *************** *** 389,394 **** --- 389,395 ---- {"help-synonyms", 0, NULL, 2}, {"help-options", 0, NULL, 4}, {"allow-root", required_argument, NULL, 3}, + {"log", required_argument, NULL, 5}, {0, 0, 0, 0} }; /* `getopt_long' stores the option index here, but right now we *************** *** 493,498 **** --- 494,505 ---- case 3: /* --allow-root */ root_allow_add (optarg); + break; + case 5: + /* --log */ + conn_logfile = CVS_FOPEN (optarg, "a"); + if (conn_logfile == NULL) + error (1, errno, "cannot open log file %s", optarg); break; case 'Q': really_quiet = 1; Index: server.c =================================================================== RCS file: /home2/cvsroot/ccvs/src/server.c,v retrieving revision 1.180 diff -c -r1.180 server.c *** server.c 1998/01/28 23:17:56 1.180 --- server.c 1998/02/03 17:05:17 *************** *** 4852,4857 **** --- 4852,4884 ---- #if defined (AUTH_SERVER_SUPPORT) || defined (HAVE_GSSAPI) + static void getline_checked PROTO ((char **, size_t *, FILE *)); + + /* Read a line from FP. Returns successfully or not at all. */ + + static void + getline_checked (lineptr, n, fp) + char **lineptr; + size_t *n; + FILE *fp; + { + if (getline (lineptr, n, fp) < 0) + { + if (feof (stdin)) + { + if (conn_logfile != NULL) + fperror (conn_logfile, 0, 0, "end of file reading stdin"); + error (1, 0, "end of file reading stdin"); + } + else + { + if (conn_logfile != NULL) + fperror (conn_logfile, 0, errno, "cannot read stdin"); + error (1, errno, "cannot read stdin"); + } + } + } + /* Read username and password from client (i.e., stdin). If correct, then switch to run as that user and send an ACK to the client via stdout, else send NACK and die. */ *************** *** 4921,4937 **** { int on = 1; ! (void) setsockopt (STDIN_FILENO, SOL_SOCKET, SO_KEEPALIVE, ! (char *) &on, sizeof on); } #endif /* Make sure the protocol starts off on the right foot... */ ! if (getline (&tmp, &tmp_allocated, stdin) < 0) ! /* FIXME: what? We could try writing error/eof, but chances ! are the network connection is dead bidirectionally. log it ! somewhere? */ ! ; if (strcmp (tmp, "BEGIN VERIFICATION REQUEST\n") == 0) verify_and_exit = 1; --- 4948,4968 ---- { int on = 1; ! if (setsockopt (STDIN_FILENO, SOL_SOCKET, SO_KEEPALIVE, ! (char *) &on, sizeof on) < 0) ! { ! if (conn_logfile != NULL) ! fperror (conn_logfile, 0, errno, ! "warning: cannot set keepalive on socket"); ! /* I think that this will very typically get ENOTSOCK if ! people are running "cvs pserver" from the command line ! for debugging. So just keep going. */ ! } } #endif /* Make sure the protocol starts off on the right foot... */ ! getline_checked (&tmp, &tmp_allocated, stdin); if (strcmp (tmp, "BEGIN VERIFICATION REQUEST\n") == 0) verify_and_exit = 1; *************** *** 4958,4966 **** /* Get the three important pieces of information in order. */ /* See above comment about error handling. */ ! getline (&repository, &repository_allocated, stdin); ! getline (&username, &username_allocated, stdin); ! getline (&password, &password_allocated, stdin); /* Make them pure. */ strip_trailing_newlines (repository); --- 4989,4997 ---- /* Get the three important pieces of information in order. */ /* See above comment about error handling. */ ! getline_checked (&repository, &repository_allocated, stdin); ! getline_checked (&username, &username_allocated, stdin); ! getline_checked (&password, &password_allocated, stdin); /* Make them pure. */ strip_trailing_newlines (repository); *************** *** 4969,4975 **** /* ... and make sure the protocol ends on the right foot. */ /* See above comment about error handling. */ ! getline (&tmp, &tmp_allocated, stdin); if (strcmp (tmp, verify_and_exit ? "END VERIFICATION REQUEST\n" : "END AUTH REQUEST\n") --- 5000,5006 ---- /* ... and make sure the protocol ends on the right foot. */ /* See above comment about error handling. */ ! getline_checked (&tmp, &tmp_allocated, stdin); if (strcmp (tmp, verify_and_exit ? "END VERIFICATION REQUEST\n" : "END AUTH REQUEST\n") *************** *** 4978,4990 **** --- 5009,5028 ---- error (1, 0, "bad auth protocol end: %s", tmp); } if (!root_allow_ok (repository)) + { /* Just give a generic I HATE YOU. This is because CVS 1.9.10 and older clients do not support "error". Once more recent clients are more widespread, probably want to fix this (it is a real pain to track down why it isn't letting you in if it won't say why, and I am not convinced that the potential information disclosure to an attacker outweighs this). */ + /* On the other hand, logging it on the server side helps some. */ + if (conn_logfile != NULL) + fperror (conn_logfile, 0, 0, + "Repository %s not specified by --allow-root", + repository); goto i_hate_you; + } /* OK, now parse the config file, so we can use it to control how to check passwords. If there was an error parsing the config