o Log ******** instead of the actual password for "set authkey"

  when command logging is switched on.
o Display ******** for the authkey for "show auth"
o Document how \P should be used, and document the other chat escapes
  while I'm there.
o Make sure the full command is displayed when a compound command
  fails - ie, "set novar rubbish" should say "set novar: Invalid command"
  rather than "novar: Invalid command"

Problem pointed out by: Theo de Raadt <deraadt@cvs.openbsd.org> (among others)

3 files changed, 114 insertions, 29 deletions

diff --git a/usr.sbin/ppp/command.c b/usr.sbin/ppp/command.c
index c167413115b..73cb31ca6c7 100644
--- a/usr.sbin/ppp/command.c
+++ b/usr.sbin/ppp/command.c
@@ -17,7 +17,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: command.c,v 1.6 1997/12/17 00:20:36 brian Exp $
+ * $Id: command.c,v 1.7 1997/12/17 21:17:44 brian Exp $
  *
  */
 #include <sys/param.h>
@@ -75,6 +75,7 @@
 #include "auth.h"
 
 struct in_addr ifnetmask;
+static const char *HIDDEN = "********";
 
 static int ShowCommand(struct cmdargs const *arg);
 static int TerminalCommand(struct cmdargs const *arg);
@@ -487,7 +488,7 @@ ShowAuthKey(struct cmdargs const *arg)
   if (!VarTerm)
     return 0;
   fprintf(VarTerm, "AuthName = %s\n", VarAuthName);
-  fprintf(VarTerm, "AuthKey  = %s\n", VarAuthKey);
+  fprintf(VarTerm, "AuthKey  = %s\n", HIDDEN);
 #ifdef HAVE_DES
   fprintf(VarTerm, "Encrypt  = %s\n", VarMSChap ? "MSChap" : "MD5" );
 #endif
@@ -666,7 +667,8 @@ FindCommand(struct cmdtab const *cmds, const char *str, int *pmatch)
 }
 
 static int
-FindExec(struct cmdtab const *cmds, int argc, char const *const *argv)
+FindExec(struct cmdtab const *cmds, int argc, char const *const *argv,
+         const char *prefix)
 {
   struct cmdtab const *cmd;
   int val = 1;
@@ -675,7 +677,7 @@ FindExec(struct cmdtab const *cmds, int argc, char const *const *argv)
 
   cmd = FindCommand(cmds, *argv, &nmatch);
   if (nmatch > 1)
-    LogPrintf(LogWARN, "%s: Ambiguous command\n", *argv);
+    LogPrintf(LogWARN, "%s%s: Ambiguous command\n", prefix, *argv);
   else if (cmd && (cmd->lauth & VarLocalAuth)) {
     arg.cmd = cmds;
     arg.argc = argc-1;
@@ -683,12 +685,12 @@ FindExec(struct cmdtab const *cmds, int argc, char const *const *argv)
     arg.data = cmd->args;
     val = (cmd->func) (&arg);
   } else
-    LogPrintf(LogWARN, "%s: Invalid command\n", *argv);
+    LogPrintf(LogWARN, "%s%s: Invalid command\n", prefix, *argv);
 
   if (val == -1)
     LogPrintf(LogWARN, "Usage: %s\n", cmd->syntax);
   else if (val)
-    LogPrintf(LogWARN, "%s: Failed %d\n", *argv, val);
+    LogPrintf(LogWARN, "%s%s: Failed %d\n", prefix, *argv, val);
 
   return val;
 }
@@ -736,6 +738,17 @@ InterpretCommand(char *buff, int nb, int *argc, char ***argv)
     *argc = 0;
 }
 
+static int
+arghidden(int argc, char const *const *argv, int n)
+{
+  /* Is arg n of the given command to be hidden from the log ? */
+  if (n == 2 && !strncasecmp(argv[0], "se", 2) &&
+      (!strncasecmp(argv[1], "authk", 5) || !strncasecmp(argv[1], "ke", 2)))
+    return 1;
+
+  return 0;
+}
+
 void
 RunCommand(int argc, char const *const *argv, const char *label)
 {
@@ -754,12 +767,15 @@ RunCommand(int argc, char const *const *argv, const char *label)
       for (f = 0; f < argc; f++) {
         if (n < sizeof(buf)-1 && f)
           buf[n++] = ' ';
-        strncpy(buf+n, argv[f], sizeof(buf)-n-1);
+        if (arghidden(argc, argv, f))
+          strncpy(buf+n, HIDDEN, sizeof(buf)-n-1);
+        else
+          strncpy(buf+n, argv[f], sizeof(buf)-n-1);
         n += strlen(buf+n);
       }
       LogPrintf(LogCOMMAND, "%s\n", buf);
     }
-    FindExec(Commands, argc, argv);
+    FindExec(Commands, argc, argv, "");
   }
 }
 
@@ -777,7 +793,7 @@ static int
 ShowCommand(struct cmdargs const *arg)
 {
   if (arg->argc > 0)
-    FindExec(ShowCommands, arg->argc, arg->argv);
+    FindExec(ShowCommands, arg->argc, arg->argv, "show ");
   else if (VarTerm)
     fprintf(VarTerm, "Use ``show ?'' to get a arg->cmd.\n");
   else
@@ -1459,7 +1475,7 @@ static int
 SetCommand(struct cmdargs const *arg)
 {
   if (arg->argc > 0)
-    FindExec(SetCommands, arg->argc, arg->argv);
+    FindExec(SetCommands, arg->argc, arg->argv, "set ");
   else if (VarTerm)
     fprintf(VarTerm, "Use `set ?' to get a arg->cmd or `set ? <var>' for"
 	    " syntax help.\n");
@@ -1563,7 +1579,7 @@ static int
 AliasCommand(struct cmdargs const *arg)
 {
   if (arg->argc > 0)
-    FindExec(AliasCommands, arg->argc, arg->argv);
+    FindExec(AliasCommands, arg->argc, arg->argv, "alias ");
   else if (VarTerm)
     fprintf(VarTerm, "Use `alias help' to get a arg->cmd or `alias help <option>'"
 	    " for syntax help.\n");
@@ -1634,7 +1650,7 @@ static int
 AllowCommand(struct cmdargs const *arg)
 {
   if (arg->argc > 0)
-    FindExec(AllowCommands, arg->argc, arg->argv);
+    FindExec(AllowCommands, arg->argc, arg->argv, "allow ");
   else if (VarTerm)
     fprintf(VarTerm, "Use `allow ?' to get a arg->cmd or `allow ? <cmd>' for"
 	    " syntax help.\n");
diff --git a/usr.sbin/ppp/ppp.8 b/usr.sbin/ppp/ppp.8
index 2acff991777..a38076cd2f9 100644
--- a/usr.sbin/ppp/ppp.8
+++ b/usr.sbin/ppp/ppp.8
@@ -1,4 +1,4 @@
-.\" $Id: ppp.8,v 1.5 1997/12/16 00:33:08 brian Exp $
+.\" $Id: ppp.8,v 1.6 1997/12/17 21:17:46 brian Exp $
 .Dd 20 September 1995
 .Os OpenBSD
 .Dt PPP 8
@@ -1242,13 +1242,14 @@ This modem "chat" string means:
 .It
 Abort if the string "BUSY" or "NO CARRIER" are received.
 .It
-Set the timeout to 4.
+Set the timeout to 4 seconds.
 .It
 Expect nothing.
 .It
 Send ATZ.
 .It
-Expect OK.  If that's not received, send ATZ and expect OK.
+Expect OK.  If that's not received within the 4 second timeout, send ATZ
+and expect OK.
 .It
 Send ATDTxxxxxxx where xxxxxxx is the next number in the phone list from
 above.
@@ -1259,10 +1260,12 @@ Wait for the CONNECT string.
 .El
 
 Once the connection is established, the login script is executed.  This
-script is written in the same style as the dial script:
+script is written in the same style as the dial script, but care should
+be taken to avoid having your password logged:
 .Bd -literal -offset indent
+set authkey MySecret
 set login "TIMEOUT 15 login:-\\\\r-login: awfulhak \e
-  word: xxx ocol: PPP HELLO"
+  word: \\\\P ocol: PPP HELLO"
 .Ed
 .Pp
 This login "chat" string means:
@@ -1277,7 +1280,9 @@ Send "awfulhak"
 .It
 Expect "word:" (the tail end of a "Password:" prompt).
 .It
-Send "xxx".
+Send whatever our current
+.Ar authkey
+value is set to.
 .It
 Expect "ocol:" (the tail end of a "Protocol:" prompt).
 .It
@@ -1286,6 +1291,17 @@ Send "PPP".
 Expect "HELLO".
 .El
 .Pp
+The
+.Dq set authkey
+command is logged specially (when using
+.Ar command
+logging) so that the actual password is not compromised
+(it is logged as
+.Sq ******** Ns
+), and the '\\P' is logged when
+.Ar chat
+logging is active rather than the actual password.
+.Pp
 Login scripts vary greatly between ISPs.
 
 .It
@@ -2031,8 +2047,14 @@ Refer to the section on PACKET FILTERING above for further details.
 .It set authkey|key value
 This sets the authentication key (or password) used in client mode
 PAP or CHAP negotiation to the given value.  It can also be used to
-specify the password to be used in the dial or login scripts, preventing
-the actual password from being logged.
+specify the password to be used in the dial or login scripts in place
+of the '\\P' sequence, preventing the actual password from being logged.  If
+.Ar command
+logging is in effect,
+.Ar value
+is logged as
+.Ar ********
+for security reasons.
 
 .It set authname id
 This sets the authentication id used in client mode PAP or CHAP negotiation.
@@ -2066,16 +2088,59 @@ above for further details.
 .It set dial chat-script
 This specifies the chat script that will be used to dial the other
 side.  See also the
-.Dv set login
+.Dq set login
 command below.  Refer to
 .Xr chat 8
 and to the example configuration files for details of the chat script
-format.  The string \\\\T will be replaced with the current phone number
-(see
+format.
+It is possible to specify some special
+.Sq values
+in your chat script as follows:
+.Bd -literal -offset indent
+.It \\\\\\\\\\\\\\\\c
+When used as the last character in a
+.Sq send
+string, this indicates that a newline should not be appended.
+.It \\\\\\\\\\\\\\\\d
+When the chat script encounters this sequence, it delays two seconds.
+.It \\\\\\\\\\\\\\\\p
+When the chat script encounters this sequence, it delays for one quarter of
+a second.
+.It \\\\\\\\\\\\\\\\n
+This is replaced with a newline character.
+.It \\\\\\\\\\\\\\\\r
+This is replaced with a carriage return character.
+.It \\\\\\\\\\\\\\\\s
+This is replaced with a space character.
+.It \\\\\\\\\\\\\\\\t
+This is replaced with a tab character.
+.It \\\\\\\\\\\\\\\\T
+This is replaced by the current phone number (see
 .Dq set phone
-below) and the string \\\\P will be replaced with the password (see
-.Dq set key
+below).
+.It \\\\\\\\\\\\\\\\P
+This is replaced by the current
+.Ar authkey
+value (see
+.Dq set authkey
+above).
+.It \\\\\\\\\\\\\\\\U
+This is replaced by the current
+.Ar authname
+value (see
+.Dq set authname
 above).
+.Ed
+.Pp
+Note that two parsers will examine these escape sequences, so in order to
+have the
+.Sq chat parser
+see the escape character, it is necessary to escape it from the
+.Sq command parser .
+This means that in practice you should use two escapes, for example:
+.Bd -literal -offset indent
+set dial "... ATDT\\\\T CONNECT"
+.Ed
 
 .It set hangup chat-script
 This specifies the chat script that will be used to reset the modem
@@ -2365,7 +2430,11 @@ This command allows the user to examine the following:
 List the current rules for the given filter.
 
 .It show auth
-Show the current authname and authkey.
+Show the current authname and encryption values.  If you have built
+.Nm
+without DES support, the encryption value is not displayed as it will
+always be
+.Ar MD5 .
 
 .It show ccp
 Show the current CCP statistics.
diff --git a/usr.sbin/ppp/systems.c b/usr.sbin/ppp/systems.c
index 78c3dcaef5a..8b7b7efdd89 100644
--- a/usr.sbin/ppp/systems.c
+++ b/usr.sbin/ppp/systems.c
@@ -17,7 +17,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: systems.c,v 1.2 1997/12/15 22:50:21 brian Exp $
+ * $Id: systems.c,v 1.3 1997/12/17 21:17:48 brian Exp $
  *
  *  TODO:
  */
@@ -217,7 +217,7 @@ AllowModes(struct cmdargs const *arg)
         break;
       }
     if (modes[m].mode == 0)
-      LogPrintf(LogWARN, "%s: Invalid mode\n", arg->argv[f]);
+      LogPrintf(LogWARN, "allow modes: %s: Invalid mode\n", arg->argv[f]);
   }
 
   modeok = (mode | allowed) == allowed ? 1 : 0;
@@ -353,7 +353,7 @@ LoadCommand(struct cmdargs const *arg)
     LogPrintf(LogERROR, "%s: Label not allowed\n", name);
     return 1;
   } else if (SelectSystem(name, CONFFILE) < 0) {
-    LogPrintf(LogWARN, "%s: not found.\n", name);
+    LogPrintf(LogWARN, "%s: label not found.\n", name);
     return -1;
   } else
     SetLabel(arg->argc ? name : NULL);