bug-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: PATCH - one time password/always prompt for password


From: Derek Robert Price
Subject: Re: PATCH - one time password/always prompt for password
Date: Thu, 31 Jul 2003 12:24:17 -0400
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030624 Netscape/7.1

Brian Murphy wrote:

Hi Derek,
you asked me to impliment something which would not send a password with
a pserver request and in this case the server could prompt the user and
accept  input from the user directly.

Here is my first attempt. It's not beautiful but it works.


Wow!  Thanks!  I didn't expect such a fast response.

If you install the pam opie module and the opie client and server then the following /etc/pam.d/cvs
allows one time password support

auth      sufficient        pam_opie.so
auth      sufficient        pam_unix.so
auth      required        pam_deny.so

account         required        pam_unix.so

All comments/testing welcome.


Good - here it comes.  :)

First off, this needs documentation and ChangeLogs to go with it.

Second, it occurs to me that the -p option itself might be overkill - just like a standard client tries an empty password if it can't find one in ~/.cvspass for this server, it should be able to always prompt the user for a password if the empty pass fails _or_ if it recieves the OTP-needed response from the server. Unless you want to argue about this, please disregard my comments regarding no_password_prompt and documenting -p below as the option and variable should be removed entirely anyhow.

Index: src/client.c

===================================================================
RCS file: /cvs/ccvs/src/client.c,v
retrieving revision 1.347
diff -u -r1.347 client.c
--- src/client.c        23 Jul 2003 21:49:31 -0000      1.347
+++ src/client.c        30 Jul 2003 20:08:17 -0000
@@ -3226,7 +3226,11 @@
        }

        /* Get the password, probably from ~/.cvspass. */
-       password = get_cvs_password ();
+       if (no_password_prompt)


no_password_prompt is a confusing name. What this option actually seems to be doing is stopping the password from being sent and _adding_ a prompt with every CVS command.

+           password = "";
+       else
+           password = get_cvs_password ();
+
        username = root->username ? root->username : getcaller();

        /* Send the empty string by default.  This is so anonymous CVS
@@ -3254,7 +3258,8 @@
        send_to_server_via(to_server, "\012", 1);

        /* Paranoia. */
-        memset (password, 0, strlen (password));
+       if (!no_password_prompt)
+           memset (password, 0, strlen (password));


It is not necessary to skip the memset() here - memset() can handle a string length of 0 just fine.

# else /* ! AUTH_CLIENT_SUPPORT */
        error (1, 0, "INTERNAL ERROR: This client does not support pserver 
authentication");
# endif /* AUTH_CLIENT_SUPPORT */
@@ -3304,6 +3309,12 @@
                fprintf (stderr, "%s\n", read_buf + 2);

                /* Continue with the authentication protocol.  */
+           }
+           else if (strncmp (read_buf, "S ", 2) == 0)
+           {


The "S" response is going to need to be documented in doc/cvsclient.texi. I think I might like another response name, like "OTP-needed", or perhaps send-password, given that the server should have just tried an empty password, as noted below.

+               char *response = getpass(read_buf + 2);
+               send_to_server_via(to_server, response, 0);
+               send_to_server_via(to_server, "\012", 1);
            }
            else if (strncmp (read_buf, "error ", 6) == 0)
            {
Index: src/cvs.h
===================================================================
RCS file: /cvs/ccvs/src/cvs.h,v
retrieving revision 1.269
diff -u -r1.269 cvs.h
--- src/cvs.h   25 Jul 2003 20:17:06 -0000      1.269
+++ src/cvs.h   30 Jul 2003 20:08:18 -0000
@@ -347,6 +347,7 @@
extern char *Tmpdir, *Editor;
extern int cvsadmin_root;
extern char *CurDir;
+extern int no_password_prompt;
extern int really_quiet, quiet;
extern int use_editor;
extern int cvswrite;
Index: src/login.c
===================================================================
RCS file: /cvs/ccvs/src/login.c,v
retrieving revision 1.77
diff -u -r1.77 login.c
--- src/login.c 29 Jul 2003 14:00:10 -0000      1.77
+++ src/login.c 30 Jul 2003 20:08:18 -0000
@@ -508,6 +508,11 @@
    if (argc < 0)
        usage (login_usage);

+    if (no_password_prompt)
+    {
+       error (1, 0, "can't use the `login' command with the -p option (no 
password)");


This will need rephrasing after the name of the no_password_prompt variable is changed.

+    }
+
    if (current_parsed_root->method != pserver_method)
    {
        error (0, 0, "can only use `login' command with the 'pserver' method");
Index: src/main.c
===================================================================
RCS file: /cvs/ccvs/src/main.c,v
retrieving revision 1.189
diff -u -r1.189 main.c
--- src/main.c  24 Jul 2003 13:44:19 -0000      1.189
+++ src/main.c  30 Jul 2003 20:08:19 -0000
@@ -37,6 +37,7 @@
int use_editor = 1;
int use_cvsrc = 1;
int cvswrite = !CVSREAD_DFLT;
+int no_password_prompt = 0;
int really_quiet = 0;
int quiet = 0;
int trace = 0;
@@ -403,7 +404,7 @@
    int help = 0;               /* Has the user asked for help?  This
                                   lets us support the `cvs -H cmd'
                                   convention to give help for cmd. */
-    static const char short_options[] = "+QqrwtnRvb:T:e:d:Hfz:s:xa";
+    static const char short_options[] = "+pQqrwtnRvb:T:e:d:Hfz:s:xa";


This new option will need to be added to the usage message.

    static struct option long_options[] =
    {
        {"help", 0, NULL, 'H'},
@@ -519,6 +520,9 @@
            case 3:
                /* --allow-root */
                root_allow_add (optarg);
+               break;
+           case 'p':
+               no_password_prompt = 1;
                break;
            case 'Q':
                really_quiet = 1;
Index: src/scramble.c
===================================================================
RCS file: /cvs/ccvs/src/scramble.c,v
retrieving revision 1.17
diff -u -r1.17 scramble.c
--- src/scramble.c      23 Jul 2003 20:42:26 -0000      1.17
+++ src/scramble.c      30 Jul 2003 20:08:19 -0000
@@ -114,15 +114,7 @@
    /* For now we can only handle one kind of scrambling.  In the future
       there may be other kinds, and this `if' will become a `switch'.  */
    if (str[0] != 'A')
-#ifndef DIAGNOSTIC
-       error (1, 0, "descramble: unknown scrambling method");
-#else  /* DIAGNOSTIC */
-    {
-       fprintf (stderr, "descramble: unknown scrambling method\n", str);
-       fflush (stderr);
-       exit (EXIT_FAILURE);
-    }
-#endif  /* DIAGNOSTIC */
+       return NULL;


What's all this, then? At the least it looks like this deserves to be in a separate patch?

    /* Method `A' is symmetrical, so scramble again to decrypt. */
    s = scramble (str + 1);
Index: src/server.c
===================================================================
RCS file: /cvs/ccvs/src/server.c,v
retrieving revision 1.309
diff -u -r1.309 server.c
--- src/server.c        25 Jul 2003 16:25:25 -0000      1.309
+++ src/server.c        30 Jul 2003 20:08:22 -0000
@@ -5231,7 +5231,8 @@

        /* Verify blank passwords directly, otherwise use crypt(). */
        if ((found_password == NULL)
-           || ((strcmp (found_password, crypt (password, found_password))
+ || (password && + (strcmp (found_password, crypt (password, found_password))
                 == 0)))


It still isn't possible for the server to get here with a NULL password.

        {
            /* Give host_user_ptr permanent storage. */
@@ -5278,8 +5279,11 @@
    int i;
    struct pam_response *response;
    struct cvs_pam_userinfo *ui = (struct cvs_pam_userinfo *)appdata_ptr;
+    char *tmp = NULL;
+    size_t tmp_allocated = 0;
+    int len;

-    assert (ui && ui->username && ui->password && msg && resp);
+    assert (ui && ui->username && msg && resp);

    response = xmalloc(num_msg * sizeof(struct pam_response));
    memset(response, 0, num_msg * sizeof(struct pam_response));
@@ -5294,11 +5298,21 @@
                break;
            /* PAM wants a password */
            case PAM_PROMPT_ECHO_OFF:
-               response[i].resp = xstrdup(ui->password);
+               if (ui->password)


In the case you are worried about, password will not be NULL, but empty ("").

Also, are you certain you should exclude even an empty password from a test against the PAM database? Is it valid via PAM to try the empty password then come back and try asking the user for a password (probably only when the password is empty)?

+                   response[i].resp = xstrdup(ui->password);
+               else {
+                   printf("S %s\n", msg[i]->msg);
+                   fflush (stdout);
+                   getnline( &tmp, &tmp_allocated, PATH_MAX, stdin );
+                   len = strlen(tmp);
+                   if (len>1 && tmp[strlen(tmp)-1] == '\n')
+                       tmp[strlen(tmp)-1] = '\0';
+                   response[i].resp = tmp;
+               }
                break;
            case PAM_ERROR_MSG:
            case PAM_TEXT_INFO:
-               printf("E %s\n",msg[i]->msg);
+               printf("E %s\n", msg[i]->msg);
                break;
            /* PAM wants something we don't understand - bail out */
            default:
@@ -5372,6 +5386,13 @@
    }
#endif

+    if (password == NULL) {


Again, password can't be NULL here, but it might be empty and an empty password might be valid.

+       printf ("E Fatal error, aborting.\n"
+               "error 0 got NULL password. Try using cvs without the -p 
option\n");
+
+       error_exit ();
+    }
+
    if (found_passwd == NULL && (pw = getpwnam (username)) != NULL)
        found_passwd = pw->pw_passwd;

@@ -5644,13 +5665,15 @@
    /* We need the real cleartext before we hash it. */
    descrambled_password = descramble (password);
    host_user = check_password (username, descrambled_password, repository);
+    if (descrambled_password != NULL) {


descrambled_password can not be NULL either.

+       memset (descrambled_password, 0, strlen (descrambled_password));
+       free (descrambled_password);
+    }
    if (host_user == NULL)
    {
#ifdef HAVE_SYSLOG_H
        syslog (LOG_DAEMON | LOG_NOTICE, "login failure (for %s)", repository);
#endif
-       memset (descrambled_password, 0, strlen (descrambled_password));
-       free (descrambled_password);
    i_hate_you:
        printf ("I HATE YOU\n");
        fflush (stdout);
@@ -5659,8 +5682,6 @@
           yet.  */
        error_exit ();
    }
-    memset (descrambled_password, 0, strlen (descrambled_password));
-    free (descrambled_password);

    /* Don't go any farther if we're just responding to "cvs login". */
    if (verify_and_exit)

Thanks again,

Derek

--
               *8^)

Email: address@hidden

Get CVS support at <http://ximbiot.com>!
--
Boy:  A noise with dirt on it






reply via email to

[Prev in Thread] Current Thread [Next in Thread]