Change dovecot authenticator to use read/write instead of fgets/fprint
authorPhilip Hazel <ph10@hermes.cam.ac.uk>
Thu, 1 Mar 2007 14:06:56 +0000 (14:06 +0000)
committerPhilip Hazel <ph10@hermes.cam.ac.uk>
Thu, 1 Mar 2007 14:06:56 +0000 (14:06 +0000)
to avoid buffering problems that show on Solaris.

doc/doc-txt/ChangeLog
src/ACKNOWLEDGMENTS
src/src/auths/dovecot.c

index 7cc55f84ddd153eae58b6d753b338d88b700b2ee..5aac30f4996354bfa995ce417659f2059f07fb0c 100644 (file)
@@ -1,4 +1,4 @@
-$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.486 2007/03/01 11:17:00 ph10 Exp $
+$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.487 2007/03/01 14:06:56 ph10 Exp $
 
 Change log file for Exim from version 4.21
 -------------------------------------------
 
 Change log file for Exim from version 4.21
 -------------------------------------------
@@ -137,6 +137,10 @@ PH/31 Unlike :fail:, a custom message specified with :defer: was not being
       returned in the SMTP response when smtp_return_error_details was false.
       This has been fixed.
 
       returned in the SMTP response when smtp_return_error_details was false.
       This has been fixed.
 
+PH/32 Change the Dovecot authenticator to use read() and write() on the socket
+      instead of the C I/O that was originally supplied, because problems were
+      reported on Solaris.
+
 
 Exim version 4.66
 -----------------
 
 Exim version 4.66
 -----------------
index 15c1f3fed62006b335267532d971414325246758..99ace355439d7e463dc188a9a592a5c2be360a8f 100644 (file)
@@ -1,4 +1,4 @@
-$Cambridge: exim/src/ACKNOWLEDGMENTS,v 1.73 2007/02/07 12:23:35 ph10 Exp $
+$Cambridge: exim/src/ACKNOWLEDGMENTS,v 1.74 2007/03/01 14:06:56 ph10 Exp $
 
 EXIM ACKNOWLEDGEMENTS
 
 
 EXIM ACKNOWLEDGEMENTS
 
@@ -20,7 +20,7 @@ relatively small patches.
 Philip Hazel
 
 Lists created: 20 November 2002
 Philip Hazel
 
 Lists created: 20 November 2002
-Last updated:  07 February 2007
+Last updated:  01 March 2007
 
 
 THE OLD LIST
 
 
 THE OLD LIST
@@ -261,6 +261,7 @@ Rein Tollevik             Patch to fix search cache missing tidyup
 Stefan Traby              Threaded Perl support
 Samuli Tuomola            OS files for QNX 6.2.0
 Dave Turner               Suggested patch for sender rewriting brokenness
 Stefan Traby              Threaded Perl support
 Samuli Tuomola            OS files for QNX 6.2.0
 Dave Turner               Suggested patch for sender rewriting brokenness
+Steve Usher               Unbuffered I/O patch for Dovecot authentication
 Carlos Villegas           Suggested patch for "headers" in filter files
 Matthias Waffenschmidt    Patch for build-time Perl bug in configure script
                           Queue run abandon log message tidy up
 Carlos Villegas           Suggested patch for "headers" in filter files
 Matthias Waffenschmidt    Patch for build-time Perl bug in configure script
                           Queue run abandon log message tidy up
index c0b4a57cbaf23ec09ebd219a3777d6e6240af41b..c9433741a337fbea9defa20690ed4008fe261b44 100644 (file)
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/src/src/auths/dovecot.c,v 1.6 2007/01/25 16:23:42 ph10 Exp $ */
+/* $Cambridge: exim/src/src/auths/dovecot.c,v 1.7 2007/03/01 14:06:56 ph10 Exp $ */
 
 /*
  * Copyright (c) 2004 Andrey Panin <pazke@donpac.ru>
 
 /*
  * Copyright (c) 2004 Andrey Panin <pazke@donpac.ru>
@@ -9,6 +9,11 @@
  * (at your option) any later version.
  */
 
  * (at your option) any later version.
  */
 
+/* A number of modifications have been made to the original code. Originally I
+commented them specially, but now they are getting quite extensive, so I have
+ceased doing that. The biggest change is to use unbuffered I/O on the socket
+because using C buffered I/O gives problems on some operating systems. PH */
+
 #include "../exim.h"
 #include "dovecot.h"
 
 #include "../exim.h"
 #include "dovecot.h"
 
 /* Options specific to the authentication mechanism. */
 optionlist auth_dovecot_options[] = {
        {
 /* Options specific to the authentication mechanism. */
 optionlist auth_dovecot_options[] = {
        {
-               "server_socket",
-               opt_stringptr,
-               (void *)(offsetof(auth_dovecot_options_block, server_socket))
+       "server_socket",
+       opt_stringptr,
+       (void *)(offsetof(auth_dovecot_options_block, server_socket))
        },
 };
 
 /* Size of the options list. An extern variable has to be used so that its
 address can appear in the tables drtables.c. */
        },
 };
 
 /* Size of the options list. An extern variable has to be used so that its
 address can appear in the tables drtables.c. */
+
 int auth_dovecot_options_count =
        sizeof(auth_dovecot_options) / sizeof(optionlist);
 
 /* Default private options block for the authentication method. */
 int auth_dovecot_options_count =
        sizeof(auth_dovecot_options) / sizeof(optionlist);
 
 /* Default private options block for the authentication method. */
+
 auth_dovecot_options_block auth_dovecot_option_defaults = {
        NULL,                           /* server_socket */
 };
 
 auth_dovecot_options_block auth_dovecot_option_defaults = {
        NULL,                           /* server_socket */
 };
 
+
+/* Static variables for reading from the socket */
+
+static uschar sbuffer[256];
+static int sbp;
+
+
+
 /*************************************************
  *          Initialization entry point           *
  *************************************************/
 /*************************************************
  *          Initialization entry point           *
  *************************************************/
@@ -41,6 +56,7 @@ auth_dovecot_options_block auth_dovecot_option_defaults = {
 /* Called for each instance, after its options have been read, to
 enable consistency checks to be done, or anything else that needs
 to be set up. */
 /* Called for each instance, after its options have been read, to
 enable consistency checks to be done, or anything else that needs
 to be set up. */
+
 void auth_dovecot_init(auth_instance *ablock)
 {
        auth_dovecot_options_block *ob =
 void auth_dovecot_init(auth_instance *ablock)
 {
        auth_dovecot_options_block *ob =
@@ -53,9 +69,9 @@ void auth_dovecot_init(auth_instance *ablock)
        ablock->client = FALSE;
 }
 
        ablock->client = FALSE;
 }
 
-static int strcut(char *str, char **ptrs, int nptrs)
+static int strcut(uschar *str, uschar **ptrs, int nptrs)
 {
 {
-       char *tmp = str;
+       uschar *tmp = str;
        int n;
 
        for (n = 0; n < nptrs; n++)
        int n;
 
        for (n = 0; n < nptrs; n++)
@@ -81,7 +97,7 @@ static int strcut(char *str, char **ptrs, int nptrs)
 }
 
 #define CHECK_COMMAND(str, arg_min, arg_max) do { \
 }
 
 #define CHECK_COMMAND(str, arg_min, arg_max) do { \
-       if (strcasecmp((str), args[0]) != 0) \
+       if (strcmpic(US(str), args[0]) != 0) \
                goto out; \
        if (nargs - 1 < (arg_min)) \
                goto out; \
                goto out; \
        if (nargs - 1 < (arg_min)) \
                goto out; \
@@ -97,21 +113,61 @@ static int strcut(char *str, char **ptrs, int nptrs)
 
 
 /*************************************************
 
 
 /*************************************************
- *             Server entry point                *
- *************************************************/
+*      "fgets" to read directly from socket      *
+*************************************************/
+
+/* Added by PH after a suggestion by Steve Usher because the previous use of
+C-style buffered I/O gave trouble. */
+
+static uschar *
+dc_gets(uschar *s, int n, int fd)
+{
+int p = 0;
+int count = 0;
+
+for (;;)
+  {
+  if (sbp == 0)
+    {
+    sbp = read(fd, sbuffer, sizeof(sbuffer));
+    if (sbp == 0) { if (count == 0) return NULL; else break; }
+    }
+
+  while (p < sbp)
+    {
+    if (count >= n - 1) break;
+    s[count++] = sbuffer[p];
+    if (sbuffer[p++] == '\n') break;
+    }
+
+  memmove(sbuffer, sbuffer + p, sbp - p);
+  sbp -= p;
+
+  if (s[count-1] == '\n' || count >= n - 1) break;
+  }
+
+s[count] = 0;
+return s;
+}
+
+
+
+
+/*************************************************
+*              Server entry point                *
+*************************************************/
 
 int auth_dovecot_server(auth_instance *ablock, uschar *data)
 {
        auth_dovecot_options_block *ob =
                (auth_dovecot_options_block *)(ablock->options_block);
        struct sockaddr_un sa;
 
 int auth_dovecot_server(auth_instance *ablock, uschar *data)
 {
        auth_dovecot_options_block *ob =
                (auth_dovecot_options_block *)(ablock->options_block);
        struct sockaddr_un sa;
-       char buffer[4096];
-       char *args[8];
+       uschar buffer[4096];
+       uschar *args[8];
        uschar *auth_command;
        uschar *auth_extra_data = US"";
        int nargs, tmp;
        int cuid = 0, cont = 1, found = 0, fd, ret = DEFER;
        uschar *auth_command;
        uschar *auth_extra_data = US"";
        int nargs, tmp;
        int cuid = 0, cont = 1, found = 0, fd, ret = DEFER;
-       FILE *f;
 
        HDEBUG(D_auth) debug_printf("dovecot authentication\n");
 
 
        HDEBUG(D_auth) debug_printf("dovecot authentication\n");
 
@@ -141,24 +197,21 @@ int auth_dovecot_server(auth_instance *ablock, uschar *data)
        if (connect(fd, (struct sockaddr *) &sa, sizeof(sa)) < 0)
                goto out;
 
        if (connect(fd, (struct sockaddr *) &sa, sizeof(sa)) < 0)
                goto out;
 
-       f = fdopen(fd, "a+");
-       if (f == NULL)
-               goto out;
-
        auth_defer_msg = US"authentication socket protocol error";
 
        auth_defer_msg = US"authentication socket protocol error";
 
+       sbp = 0;  /* Socket buffer pointer */
        while (cont) {
        while (cont) {
-               if (fgets(buffer, sizeof(buffer), f) == NULL)
+               if (dc_gets(buffer, sizeof(buffer), fd) == NULL)
                        OUT("authentication socket read error or premature eof");
 
                        OUT("authentication socket read error or premature eof");
 
-               buffer[strlen(buffer) - 1] = 0;
+               buffer[Ustrlen(buffer) - 1] = 0;
                HDEBUG(D_auth) debug_printf("received: %s\n", buffer);
                nargs = strcut(buffer, args, sizeof(args) / sizeof(args[0]));
 
                switch (toupper(*args[0])) {
                case 'C':
                        CHECK_COMMAND("CUID", 1, 1);
                HDEBUG(D_auth) debug_printf("received: %s\n", buffer);
                nargs = strcut(buffer, args, sizeof(args) / sizeof(args[0]));
 
                switch (toupper(*args[0])) {
                case 'C':
                        CHECK_COMMAND("CUID", 1, 1);
-                       cuid = atoi(args[1]);
+                       cuid = Uatoi(args[1]);
                        break;
 
                case 'D':
                        break;
 
                case 'D':
@@ -178,7 +231,7 @@ int auth_dovecot_server(auth_instance *ablock, uschar *data)
 
                case 'V':
                        CHECK_COMMAND("VERSION", 2, 2);
 
                case 'V':
                        CHECK_COMMAND("VERSION", 2, 2);
-                       if (atoi(args[1]) != VERSION_MAJOR)
+                       if (Uatoi(args[1]) != VERSION_MAJOR)
                                OUT("authentication socket protocol version mismatch");
                        break;
 
                                OUT("authentication socket protocol version mismatch");
                        break;
 
@@ -232,20 +285,24 @@ int auth_dovecot_server(auth_instance *ablock, uschar *data)
                ablock->public_name, auth_extra_data, sender_host_address,
                interface_address, data ? (char *) data : "");
 
                ablock->public_name, auth_extra_data, sender_host_address,
                interface_address, data ? (char *) data : "");
 
-       fprintf(f, "%s", auth_command);
+       if (write(fd, auth_command, Ustrlen(auth_command)) < 0)
+              HDEBUG(D_auth) debug_printf("error sending auth_command: %s\n",
+                strerror(errno));
+
        HDEBUG(D_auth) debug_printf("sent: %s", auth_command);
 
        while (1) {
        HDEBUG(D_auth) debug_printf("sent: %s", auth_command);
 
        while (1) {
-               if (fgets(buffer, sizeof(buffer), f) == NULL) {
+               uschar *temp;
+               if (dc_gets(buffer, sizeof(buffer), fd) == NULL) {
                        auth_defer_msg = US"authentication socket read error or premature eof";
                        goto out;
                }
 
                        auth_defer_msg = US"authentication socket read error or premature eof";
                        goto out;
                }
 
-               buffer[strlen(buffer) - 1] = 0;
+               buffer[Ustrlen(buffer) - 1] = 0;
                HDEBUG(D_auth) debug_printf("received: %s\n", buffer);
                nargs = strcut(buffer, args, sizeof(args) / sizeof(args[0]));
 
                HDEBUG(D_auth) debug_printf("received: %s\n", buffer);
                nargs = strcut(buffer, args, sizeof(args) / sizeof(args[0]));
 
-               if (atoi(args[1]) != cuid)
+               if (Uatoi(args[1]) != cuid)
                        OUT("authentication socket connection id mismatch");
 
                switch (toupper(*args[0])) {
                        OUT("authentication socket connection id mismatch");
 
                switch (toupper(*args[0])) {
@@ -266,9 +323,9 @@ int auth_dovecot_server(auth_instance *ablock, uschar *data)
                                goto out;
                        }
 
                                goto out;
                        }
 
-                       if (fprintf(f, "CONT\t%d\t%s\r\n", cuid, data) < 0)
+                       temp = string_sprintf("CONT\t%d\t%s\r\n", cuid, data);
+                       if (write(fd, temp, Ustrlen(temp)) < 0)
                                OUT("authentication socket write error");
                                OUT("authentication socket write error");
-
                        break;
 
                case 'F':
                        break;
 
                case 'F':
@@ -276,7 +333,7 @@ int auth_dovecot_server(auth_instance *ablock, uschar *data)
 
                        /* FIXME: add proper response handling */
                        if (args[2]) {
 
                        /* FIXME: add proper response handling */
                        if (args[2]) {
-                               uschar *p = Ustrchr(args[2], '=');
+                               uschar *p = Ustrchr(args[2], '=');
                                if (p) {
                                        ++p;
                                        expand_nstring[1] = auth_vars[0] =
                                if (p) {
                                        ++p;
                                        expand_nstring[1] = auth_vars[0] =
@@ -293,7 +350,7 @@ int auth_dovecot_server(auth_instance *ablock, uschar *data)
                        CHECK_COMMAND("OK", 2, 2);
                        {
                                /* FIXME: add proper response handling */
                        CHECK_COMMAND("OK", 2, 2);
                        {
                                /* FIXME: add proper response handling */
-                               uschar *p = Ustrchr(args[2], '=');
+                               uschar *p = Ustrchr(args[2], '=');
                                if (!p)
                                        OUT("authentication socket protocol error, username missing");
 
                                if (!p)
                                        OUT("authentication socket protocol error, username missing");
 
@@ -311,7 +368,7 @@ int auth_dovecot_server(auth_instance *ablock, uschar *data)
                }
        }
 
                }
        }
 
-out:   close(fd);
+out:
 
        /* Expand server_condition as an authorization check */
        return (ret == OK)? auth_check_serv_cond(ablock) : ret;
 
        /* Expand server_condition as an authorization check */
        return (ret == OK)? auth_check_serv_cond(ablock) : ret;