This patch adds some paranoia checks for the -c flag.  It also fixes a
bug that could cause sftpd to get into an infinite loop (a possible
DoS).                     

To apply the patch:
  % cd sftpd*
  % patch -p1 < req.enc.paranoia.patch.txt
  % make distclean ; ./configure && make

diff -ub --recursive sftpd-1.45p4/doc/sftpd.html sftpd/doc/sftpd.html
--- sftpd-1.45p4/doc/sftpd.html	Fri Sep 15 15:55:05 2000
+++ sftpd/doc/sftpd.html	Fri Sep 22 03:25:00 2000
@@ -115,7 +115,9 @@

 <dt><b>-c</b>
 <dd>
-Require data encryption to be used for all file transfers.
+Require data encryption to be used for all file transfers.  This option
+implies <b>-3</b> (force data relay) and <b>-9</b> (disallow unencrypted
+connections).
 <p>

 <dt><b>-r</b><i>low</i>-<i>high</i>
diff -ub --recursive sftpd-1.45p4/sc/mkdist sftpd/sc/mkdist
--- sftpd-1.45p4/sc/mkdist	Fri Aug 25 18:02:21 2000
+++ sftpd/sc/mkdist	Fri Sep 22 03:35:46 2000
@@ -2,20 +2,31 @@
 # make a tarball for distribution
 
 if [ "$1" = "" ]; then
-  echo "usage: $0 version-number"
+  echo "usage: $0 version-number [pre-tag]"
+  echo " e.g.: $0 1.45 p1"
   exit
 fi
 
 cd misc/rel || exit 2
 cvs export -D now sftpd || exit 2
-mv sftpd sftpd-$1 || exit 2
-if [ -f sftpd-$1.tar.gz ]; then
+mv sftpd "sftpd-$1" || exit 2
+if [ -f "sftpd-$1.tar.gz" ]; then
   # already exists.. replace it
-  rm -f sftpd-$1.tar.gz
+  rm -f "sftpd-$1.tar.gz"
 fi
-targz sftpd-$1 || exit 2
-rm -rf sftpd-$1 || exit 2
+targz "sftpd-$1" || exit 2
+rm -rf "sftpd-$1" || exit 2
 
-echo "tarball created as misc/rel/sftpd-$1.tar.gz"
+# rename based on pre
+now="sftpd-$1.tar.gz"
+if [ "$2" = "" ]; then
+  fnl="$now"
+else
+  fnl="sftpd-$1$2.tar.gz"
+  mv "$now" "$fnl" || exit 2
+fi
+
+echo "tarball created as misc/rel/$fnl"
+md5 "$fnl"
 exit
 
diff -ub --recursive sftpd-1.45p4/sftpc.cpp sftpd/sftpc.cpp
--- sftpd-1.45p4/sftpc.cpp	Mon Sep 18 12:01:24 2000
+++ sftpd/sftpc.cpp	Fri Sep 22 03:19:28 2000
@@ -213,7 +213,7 @@
     Reply reply = readReply();
 
     // do DIGT and ADAT processing
-    handleReply(reply);
+    handleAdatAndDigt(reply);
 
     // look at reply code
     if (first(reply.getCode()) != RCF_POSITIVE_PRELIMINARY) {
@@ -247,6 +247,16 @@
   Reply reply = readFinalReply();
 
   // confirm is ok
+  checkReplyCode(reply);
+
+  // return the code itself
+  return reply.getCode();
+}
+
+
+// throw xReply if there is a problem
+void SFTPC::checkReplyCode(Reply const &reply) const
+{
   ReplyCodeFirst f = first(reply.getCode());
   if (!( f == RCF_POSITIVE_COMPLETION ||
          f == RCF_POSITIVE_INTERMEDIATE )) {
@@ -258,13 +268,10 @@
   //         doesn't appear to be a possible return from readFinalReply.
   // 6/8/99: doh!  it's "preliminary" which can't be returned;
   //         "intermediate" can be (and is; e.g., "user" response is 331)
-
-  // return the code itself
-  return reply.getCode();
 }
 
 
-void SFTPC::handleReply(Reply &reply)
+void SFTPC::handleAdatAndDigt(Reply &reply)
 {
   // the reply may go into the DIGT calculation
   if (isDigtActive()) {
@@ -620,6 +627,7 @@
 
   // get reply
   Reply reply = readReply();
+  checkReplyCode(reply);       // minor fix: w/o this, errors produce a confusing message
 
   // parse the address and port
   IPAddress addr;
@@ -2228,7 +2236,7 @@
     Reply reply = readReply();
 
     // DIGT and ADAT processing
-    handleReply(reply);
+    handleAdatAndDigt(reply);
 
     // since we don't know what this reply was for,
     // there isn't anything intelligent to do with
@@ -2348,8 +2356,9 @@
 
   // if we're turning on data encryption for the first time,
   // negotiate a PBSZ
-  if (newLevel != DSL_CLEAR &&
-      !negotiatedPBSZ()) {
+  if (!negotiatedPBSZ()) {
+    // fix: was checking that newLevel != DSL_CLEAR, but 2228
+    // specifies that PBSZ must come first even for "PROT C"
     requestPBSZ();
   }
 
diff -ub --recursive sftpd-1.45p4/sftpc.h sftpd/sftpc.h
--- sftpd-1.45p4/sftpc.h	Sat Sep 16 02:27:57 2000
+++ sftpd/sftpc.h	Fri Sep 22 03:19:28 2000
@@ -103,8 +103,11 @@
   // get the reply, throw exception on error reply code
   ReplyCode readAndCheckFinalReplyCode();
 
-  // print, and otherwise respond to reply
-  void handleReply(Reply &reply);
+  // throw xReply if not success
+  void checkReplyCode(Reply const &reply) const;
+
+  // deal with ADATs, possibly adding them to DIGT checksum
+  void handleAdatAndDigt(Reply &reply);
 
 
   // --------- request stuff ----------
diff -ub --recursive sftpd-1.45p4/sftpd.cpp sftpd/sftpd.cpp
--- sftpd-1.45p4/sftpd.cpp	Mon Sep 18 12:01:24 2000
+++ sftpd/sftpd.cpp	Fri Sep 22 03:19:28 2000
@@ -448,7 +448,13 @@
             break;
 
           case 'x':
+            if (requireDataEncryption) {     // handle -c before -x
+              log(LE_ERROR, "-x is incompatible with -c");
+              errors = true;
+            }
+            else {
             allowCleartext = true;
+            }
             break;
 
           case '9':
@@ -491,6 +497,14 @@
 
           case 'c':
             requireDataEncryption = true;
+
+            // 9/22/00 02:07: tightening-down -c even more: make it
+            // imply -3, so the direct-to-ftpd route is completely
+            // closed, and -9 so we never accept unencrypted conns
+            forceDataRelay = true;
+            allowRfc959 = false;
+            allowRfc959_anon = false;
+            allowCleartext = false;      // in case -c after -x
             break;
 
           case 'r': {
@@ -1344,6 +1358,9 @@
     // it worked; no big deal
     diagnostic("port command was accepted");
 
+    // 9/22/00 02:01 bugfix: wasn't resetting this
+    serverPassivePort = NONPASV_PORT;
+
     // but must send ftpd's reply!  this was a bug in 1.10 ... :(
     clientReply(reply);
   }
@@ -1353,6 +1370,10 @@
 // this fn is called in both 959 interop and normal SafeTP mode
 void SFTPD::handlePortCommand(Request const &request)
 {
+  if (failsRequireEncTest()) {
+    return;
+  }
+
   if (doingDataRelay()) {
     // bugfix: clear any prior data-channel socket state
     closeDataSockets();
@@ -1856,6 +1877,7 @@
         if (level == DSL_NONE) {
           clientReply(RC_PARAMETER_SYNTAX_ERROR,
             "Unknown PROT command code.");
+          break;          // 9/22/00 00:28 bugfix: 'break' was missing
         }
 
         // verify we can support it (should never fail, but now is as good
@@ -1909,15 +1931,19 @@
       // with the *attempt*, so at the earliest moment we see that
       // the client sent a data-transmission command, we inc
       if (security && dataChannelProtected()) {
+        if (requireDataEncryption) {          
+          // paranioa rule: never call newFile with anything other
+          // than DSL_PRIVATE when requireDataEncryption==true
+          security->data().newFile(DSL_PRIVATE);
+        }
+        else {
         security->data().newFile(dataSecLevel);
       }
+      }
 
-      // policy option
-      if ( requireDataEncryption &&
-           !(security && (dataSecLevel == DSL_PRIVATE)) ) {
-        clientReply(RC_REQUEST_DENIED,
-          "This server requires data encryption to be enabled -- turn on "
-          "data encryption in your SafeTP software.");
+      // policy option; test this *after* newFile to obey file-number
+      // increment semantics
+      if (failsRequireEncTest()) {
         break;
       }
 
@@ -1929,6 +1955,7 @@
       if (!pasvMode() &&
           doingDataRelay() &&
           server_listen == INVALID_SOCKET) {
+        diagnostic("doing extra PORT for default-client-port transfer");
         listenServerDataPort();
       }
 
@@ -2040,6 +2067,23 @@
 }
 
 
+// if the transfer isn't allowed, reply to the user and return true;
+// otherwise return false
+bool SFTPD::failsRequireEncTest()
+{
+  if ( requireDataEncryption &&
+       !(security && (dataSecLevel == DSL_PRIVATE)) ) {
+    clientReply(RC_REQUEST_DENIED,
+      "This server requires data encryption to be enabled -- turn on "
+      "data encryption in your SafeTP software.");
+    return true;
+  }
+  else {
+    return false;
+  }
+}
+
+
 // check the login name for special processing; return true if
 // action was taken, such that the calling code should not do
 // any more processing
@@ -2140,8 +2184,20 @@
 // the client has just issued a PASV command; we must relay it to the
 // server, but instead of passing on the server's response, substitute
 // our own newly-opened port
+//
+// I do not implement the direct-to-ftpd optimization for PASV
+// transfers..  this originally was simply an oversight, but as I
+// consider it more, the problem is (unlike PORT), I can't detect
+// (before it causes a problem) whether the server is going to balk at
+// the address the client connects from.  So my decision now is if
+// someone really wants the direct-to-ftpd for performance, they can
+// use active mode.  -- sm, 9/22/00 02:07
 void SFTPD::handlePassiveMode()
 {
+  if (failsRequireEncTest()) {
+    return;
+  }
+  
   // bugfix: clear any prior data-channel socket state
   closeDataSockets();
 
@@ -2376,7 +2432,9 @@
 // read from client, decrypt, forward to server
 void SFTPD::decryptDataBlock()
 {
-  // read the block size, 32 bits in network-byte-order
+  // read the block size, 32 bits in network-byte-order; if the
+  // connection has been closed, this will throw xSocket, and
+  // it will be caught and handled in the caller
   unsigned long blockSize = recvNBO32(client_data);
   if (blockSize > maxBlockSize) {
     xsecurity(stringb("Block size was " << blockSize <<
@@ -2441,6 +2499,11 @@
                       maxCleartextBlockSize);
   if (cleartextBlockSize == 0) {
     diagnostic("ftpd just closed the data connection");
+    
+    // 9/22/00 01:06 bugfix: shut down the socket now; if something
+    // fails in the code below, we don't want the conn-closed condition
+    // to keep causing us to come here and fail over and over
+    closeSocket(server_data);
   }
 
   if (printDataChannel) {
@@ -2480,8 +2543,8 @@
     // response to STOR, RETR, etc.)
 
     // shut down data sockets
+    // (update: moved the close(server_data) up)
     closeSocket(client_data);
-    closeSocket(server_data);
   }
 
   doArtificialDelay();
@@ -2490,6 +2553,9 @@
 
 void SFTPD::relaySocketData(SOCKET &dest, SOCKET &source)
 {
+  // paranoia
+  xassert(!requireDataEncryption);
+
   // just use a local buffer for simplicity
   enum { BUFSIZE = 1024 };
   char buf[BUFSIZE];
diff -ub --recursive sftpd-1.45p4/sftpd.h sftpd/sftpd.h
--- sftpd-1.45p4/sftpd.h	Fri Sep 15 15:55:03 2000
+++ sftpd/sftpd.h	Fri Sep 22 03:19:28 2000
@@ -175,6 +175,8 @@
   void encryptedClientReply(Reply const &reply);
   void unencryptedClientReply(Reply const &reply);
 
+  bool failsRequireEncTest();
+
   // automatically determine whether to encrypt the reply or not
   void clientReply(Reply const &reply);
   void clientReply(ReplyCode code, char const *msg);
diff -ub --recursive sftpd-1.45p4/sftpd.todo.txt sftpd/sftpd.todo.txt
--- sftpd-1.45p4/sftpd.todo.txt	Tue Sep 19 02:08:07 2000
+++ sftpd/sftpd.todo.txt	Tue Sep 19 03:29:25 2000
@@ -54,6 +54,8 @@
  - HUP to reopen log file
 
  - actual configuration file instead of endless cmdline options..
+ 
+ - misc/testinstall doesn't work on OSF because of chown thing
 
 
 Can't reproduce
Only in sftpd: sktsrvr
Only in sftpd: socktest