Decouple marshalling/un-marshalling from socket I/O

configure.ac

@@ -48,6 +48,7 @@ dnl --------------------------------------------------------------------
 dnl Checks for header files.
 AC_HEADER_STDC
 AC_CHECK_HEADERS([arpa/inet.h fcntl.h netdb.h netinet/in.h stdlib.h string.h strings.h sys/socket.h sys/time.h ])
+AC_CHECK_HEADERS([stdbool.h limits.h assert.h])
 AC_CHECK_HEADERS([syslog.h unistd.h openssl/md5.h openssl/rand.h linux/random.h sys/random.h])
 AC_CHECK_HEADER(security/pam_appl.h, [], [AC_MSG_ERROR([PAM libraries missing. Install with "yum install pam-devel" or "apt-get install libpam-dev".])] )
 AM_CONDITIONAL(MY_MD5, [test "$ac_cv_header_openssl_md5_h" = "no" ])

libtac/include/libtac.h

@@ -42,6 +42,9 @@ extern "C" {
 #else
 #include "cdefs.h"
 #endif
+#include <assert.h>
+#include <stdbool.h>
+#include <limits.h>
 #include "tacplus.h"
 
 #if defined(DEBUGTAC) && !defined(TACDEBUG)
@@ -87,6 +90,7 @@ struct tac_attrib {
 struct areply {
 	struct tac_attrib *attr;
 	char *msg;
+	char *data;
 	int status :8;
 	int flags :8;
 	int seq_no :8;
@@ -144,6 +148,8 @@ extern int tac_authen_service;
 extern int tac_debug_enable;
 extern int tac_readtimeout_enable;
 
+HDR *_tac_req_header(u_char, int);
+
 /* connect.c */
 extern int tac_timeout;
 
@@ -152,30 +158,65 @@ int tac_connect_single(const struct addrinfo *, const char *, struct addrinfo *,
 		int);
 char *tac_ntop(const struct sockaddr *);
 
-int tac_authen_send(int, const char *, const char *, const char *, const char *,
-		u_char);
+/* authen_s.c */
+u_char tac_get_authen_type(const char *);
+void tac_authen_send_pkt(const char *, const char *, const char *,
+    const char *, u_char, u_char **, unsigned *);
+int tac_authen_send(int, const char *, const char *, const char *,
+    const char *, u_char);
+
+/* authen_r.c */
+int tac_authen_parse(struct areply *, u_char *, unsigned);
 int tac_authen_read(int, struct areply *);
-int tac_cont_send_seq(int, const char *, int);
+
+/* cont_s.c */
+void tac_cont_send_pkt(const char *, uint8_t, u_char **, unsigned *);
+int tac_cont_send_seq(int, const char *, uint8_t);
 #define tac_cont_send(fd, pass) tac_cont_send_seq((fd), (pass), 3)
-HDR *_tac_req_header(u_char, int);
+
+/* crypt.c */
 void _tac_crypt(u_char *, const HDR *);
+
+/* author_r.c */
+int tac_author_parse(u_char *, unsigned, struct areply *);
+int tac_author_read(int, struct areply *);
+
+/* author_s.c */
+void tac_author_send_pkt(const char *, const char *, const char *,
+    struct tac_attrib *, u_char **, unsigned *);
+int tac_author_send(int, const char *, const char *, const char *,
+    struct tac_attrib *);
+
+/* attrib.c */
 void tac_add_attrib(struct tac_attrib **, char *, char *);
+void tac_add_attrib_pair(struct tac_attrib **, char *, char, char *);
 void tac_free_attrib(struct tac_attrib **);
-char *tac_acct_flag2str(int);
-int tac_acct_send(int, int, const char *, char *, char *, struct tac_attrib *);
+
+/* acct_s.c */
+char *tac_acct_flag2str(u_char);
+void tac_acct_send_pkt(u_char, const char *, const char *, const char *,
+    struct tac_attrib *, u_char **, unsigned *);
+int tac_acct_send(int, u_char, const char *, const char *, const char *,
+    struct tac_attrib *);
+
+/* acct_r.c */
+int tac_acct_parse(u_char *, unsigned, struct areply *);
 int tac_acct_read(int, struct areply *);
+
+/* xalloc.c */
 void *xcalloc(size_t, size_t);
 void *xrealloc(void *, size_t);
 char *xstrcpy(char *, const char *, size_t);
-char *_tac_check_header(HDR *, int);
-int tac_author_send(int, const char *, char *, char *, struct tac_attrib *);
-int tac_author_read(int, struct areply *);
-void tac_add_attrib_pair(struct tac_attrib **, char *, char, char *);
-int tac_read_wait(int, int, int, int *);
+
+/* hdr_check.c */
+char *_tac_check_header(HDR *, uint8_t);
 
 /* magic.c */
 u_int32_t magic(void);
 
+/* read_wait.c */
+int tac_read_wait(int, int, int, int *);
+
 #ifdef __cplusplus
 }
 #endif

libtac/include/tacplus.h

@@ -107,6 +107,7 @@ struct authen_cont {
     u_short user_msg_len;
     u_short user_data_len;
     u_char flags;
+    u_char msg[0];
 
 #define TAC_PLUS_CONTINUE_FLAG_ABORT 0x01
 
@@ -133,6 +134,7 @@ struct authen_reply {
 
     u_short msg_len;
     u_short data_len;
+    u_char  msg[0];
 };
 
 #define TAC_AUTHEN_REPLY_FIXED_FIELDS_SIZE 6
@@ -173,6 +175,7 @@ struct acct {
     u_char port_len;
     u_char r_addr_len;
     u_char arg_cnt;    /* the number of cmd args */
+    u_char arg_len[0];
 };
 
 #define TAC_ACCT_REQ_FIXED_FIELDS_SIZE 9
@@ -201,6 +204,7 @@ struct author {
     u_char port_len;
     u_char r_addr_len;
     u_char arg_cnt;    /* the number of args */
+    u_char arg_len[0];
 };
 
 #define TAC_AUTHOR_REQ_FIXED_FIELDS_SIZE 8
@@ -211,6 +215,7 @@ struct author_reply {
     u_char arg_cnt;
     u_short msg_len;
     u_short data_len;
+    u_char  arg_len[0];
 
 #define TAC_PLUS_AUTHOR_STATUS_PASS_ADD  0x01
 #define TAC_PLUS_AUTHOR_STATUS_PASS_REPL 0x02

libtac/lib/acct_r.c

@@ -32,118 +32,80 @@
  *             LIBTAC_STATUS_PROTOCOL_ERR
  *   >= 0 : server response, see TAC_PLUS_AUTHEN_STATUS_...
  */
-int tac_acct_read(int fd, struct areply *re) {
-    HDR th;
+int tac_acct_parse(u_char *pkt, unsigned pkt_total, struct areply *re) {
+    HDR *th = (HDR *)pkt;
     struct acct_reply *tb = NULL;
     size_t ulen_from_header, len_from_body;
-    ssize_t spacket_read;
     char *msg = NULL;
-	int timeleft = 0;
-    re->attr = NULL; /* unused */
-    re->msg = NULL;
-
-    if (tac_readtimeout_enable &&
-        tac_read_wait(fd,tac_timeout*1000, TAC_PLUS_HDR_SIZE,&timeleft) < 0 ) {
-        TACSYSLOG(LOG_ERR,\
-            "%s: reply timeout after %u secs", __FUNCTION__, tac_timeout);
-        re->msg = xstrdup(acct_syserr_msg);
-        re->status = LIBTAC_STATUS_READ_TIMEOUT;
-        free(tb);
-        return re->status;
-    }
 
-    spacket_read = read(fd, &th, TAC_PLUS_HDR_SIZE);
-    if(spacket_read  < TAC_PLUS_HDR_SIZE) {
-        TACSYSLOG(LOG_ERR,\
-            "%s: short reply header, read %zd of %u expected: %m", __FUNCTION__,\
-            spacket_read, TAC_PLUS_HDR_SIZE);
-        re->msg = xstrdup(acct_syserr_msg);
-        re->status = LIBTAC_STATUS_SHORT_HDR;
-        free(tb);
-        return re->status;
-    }
+    re->attr = NULL; /* unused */
+    re->msg = re->data = NULL;
 
     /* check the reply fields in header */
-    msg = _tac_check_header(&th, TAC_PLUS_ACCT);
+    msg = _tac_check_header(th, TAC_PLUS_ACCT);
     if(msg != NULL) {
         re->msg = xstrdup(msg);
         re->status = LIBTAC_STATUS_PROTOCOL_ERR;
-        free(tb);
         TACDEBUG(LOG_DEBUG, "%s: exit status=%d, status message \"%s\"",\
             __FUNCTION__, re->status, re->msg != NULL ? re->msg : "");
         return re->status;
     }
 
-    ulen_from_header = ntohl(th.datalength);
-    if (ulen_from_header > TAC_PLUS_MAX_PACKET_SIZE) {
-        TACSYSLOG(LOG_ERR,\
-            "%s: length declared in the packet %zu exceeds max allowed packet size %d",\
-            __FUNCTION__,\
-            ulen_from_header, TAC_PLUS_MAX_PACKET_SIZE);
-        re->status=LIBTAC_STATUS_SHORT_HDR;
-        free(tb);
-        return re->status;
-    }
-    tb=(struct acct_reply *) xcalloc(1, ulen_from_header);
+    ulen_from_header = ntohl(th->datalength);
 
-    /* read reply packet body */
-    if (tac_readtimeout_enable &&
-        tac_read_wait(fd,timeleft,ulen_from_header,NULL) < 0 ) {
-        TACSYSLOG(LOG_ERR,\
-            "%s: reply timeout after %u secs", __FUNCTION__, tac_timeout);
-        re->msg = xstrdup(acct_syserr_msg);
-        re->status = LIBTAC_STATUS_READ_TIMEOUT;
-        free(tb);
-        return re->status;
-    }
+    tb = (struct acct_reply *)(pkt + TAC_PLUS_HDR_SIZE);
 
-    spacket_read = read(fd, tb, ulen_from_header);
-    if(spacket_read < (ssize_t) ulen_from_header) {
+    if (pkt_total != ulen_from_header) {
         TACSYSLOG(LOG_ERR,\
-            "%s: short reply body, read %zd of %zu: %m",\
+            "%s: short packet, got %u expected %zu: %m",\
             __FUNCTION__,\
-			spacket_read, ulen_from_header);
+	    pkt_total, TAC_PLUS_HDR_SIZE + ulen_from_header);
         re->msg = xstrdup(acct_syserr_msg);
         re->status = LIBTAC_STATUS_SHORT_BODY;
-        free(tb);
         return re->status;
     }
 
     /* decrypt the body */
-    _tac_crypt((u_char *) tb, &th);
+    _tac_crypt((u_char *) tb, th);
 
     /* Convert network byte order to host byte order */
     tb->msg_len  = ntohs(tb->msg_len);
     tb->data_len = ntohs(tb->data_len);
 
     /* check the length fields */
-    len_from_body=sizeof(tb->msg_len) + sizeof(tb->data_len) +
-        sizeof(tb->status) + tb->msg_len + tb->data_len;
+    len_from_body = TAC_ACCT_REPLY_FIXED_FIELDS_SIZE + \
+        tb->msg_len + tb->data_len;
 
     if(ulen_from_header != len_from_body) {
         TACSYSLOG(LOG_ERR,\
-            "%s: inconsistent reply body, incorrect key?",\
-            __FUNCTION__);
+            "%s: inconsistent reply body, header len %zu versus parsed len %zu",\
+            __FUNCTION__, ulen_from_header, len_from_body);
         re->msg = xstrdup(acct_syserr_msg);
         re->status = LIBTAC_STATUS_PROTOCOL_ERR;
-        free(tb);
         return re->status;
     }
 
     /* save status and clean up */
     if(tb->msg_len) {
         msg=(char *) xcalloc(1, tb->msg_len+1);
         bcopy((u_char *) tb+TAC_ACCT_REPLY_FIXED_FIELDS_SIZE, msg, tb->msg_len);
-        msg[(int)tb->msg_len] = '\0';
+        msg[tb->msg_len] = '\0';
         re->msg = msg;      /* Freed by caller */
     }
 
+    if(tb->data_len) {
+        msg=(char *) xcalloc(1, tb->data_len+1);
+        bcopy((u_char *) tb+TAC_ACCT_REPLY_FIXED_FIELDS_SIZE+tb->data_len,
+            msg, tb->data_len);
+        msg[tb->data_len] = '\0';
+        re->data = msg;     /* Freed by caller */
+    }
+
     /* server logged our request successfully */
     if (tb->status == TAC_PLUS_ACCT_STATUS_SUCCESS) {
         TACDEBUG(LOG_DEBUG, "%s: accounted ok", __FUNCTION__);
         if (!re->msg) re->msg = xstrdup(acct_ok_msg);
         re->status = tb->status;
-        free(tb);
         return re->status;
     }
 
@@ -162,6 +124,94 @@ int tac_acct_read(int fd, struct areply *re) {
             break;
     }
 
-    free(tb);
     return re->status;
-}
+}    /* tac_acct_parse */
+
+/*
+ * return value:
+ *   <  0 : error status code, see LIBTAC_STATUS_...
+ *             LIBTAC_STATUS_READ_TIMEOUT
+ *             LIBTAC_STATUS_SHORT_HDR
+ *             LIBTAC_STATUS_SHORT_BODY
+ *             LIBTAC_STATUS_PROTOCOL_ERR
+ *   >= 0 : server response, see TAC_PLUS_AUTHEN_STATUS_...
+ */
+int tac_acct_read(int fd, struct areply *re) {
+    HDR *th;
+    struct acct_reply *tb = NULL;
+    size_t ulen_from_header;
+    ssize_t spacket_read;
+    int status, timeleft = 0;
+
+    re->attr = NULL; /* unused */
+    re->msg = re->data = NULL;
+
+    if (tac_readtimeout_enable &&
+        tac_read_wait(fd, tac_timeout * 1000, TAC_PLUS_HDR_SIZE, &timeleft) < 0 ) {
+        TACSYSLOG(LOG_ERR,\
+            "%s: reply timeout after %u secs", __FUNCTION__, tac_timeout);
+        re->msg = xstrdup(acct_syserr_msg);
+        re->status = LIBTAC_STATUS_READ_TIMEOUT;
+        return re->status;
+    }
+
+    th = (HDR *)xcalloc(1, TAC_PLUS_HDR_SIZE);
+
+    spacket_read = read(fd, (char *)th, TAC_PLUS_HDR_SIZE);
+    if(spacket_read < TAC_PLUS_HDR_SIZE) {
+        TACSYSLOG(LOG_ERR,\
+            "%s: short reply header, read %zd of %u expected: %m", __FUNCTION__,\
+            ((spacket_read >= 0) ? spacket_read : 0), TAC_PLUS_HDR_SIZE);
+        re->msg = xstrdup(acct_syserr_msg);
+        re->status = LIBTAC_STATUS_SHORT_HDR;
+        free(th);
+        return re->status;
+    }
+
+    ulen_from_header = ntohl(th->datalength);
+    if (ulen_from_header > TAC_PLUS_MAX_PACKET_SIZE) {
+        TACSYSLOG(LOG_ERR,\
+            "%s: length declared in the packet %zu exceeds max allowed packet size %u", __FUNCTION__,\
+            ulen_from_header, TAC_PLUS_MAX_PACKET_SIZE);
+        re->msg = xstrdup(acct_syserr_msg);
+        re->status = LIBTAC_STATUS_PROTOCOL_ERR;
+        free(th);
+        return re->status;
+    }
+
+    /* now make room for entire contiguous packet */
+    th = (HDR *)xrealloc(th, TAC_PLUS_HDR_SIZE + ulen_from_header);
+    tb = (struct acct_reply *)((u_char *)th + TAC_PLUS_HDR_SIZE);
+
+    /* read reply packet body */
+    if (tac_readtimeout_enable &&
+        tac_read_wait(fd, timeleft, ulen_from_header, NULL) < 0 ) {
+        TACSYSLOG(LOG_ERR,\
+            "%s: reply timeout after %u secs", __FUNCTION__, tac_timeout);
+        re->msg = xstrdup(acct_syserr_msg);
+        re->status = LIBTAC_STATUS_READ_TIMEOUT;
+        free(th);
+        return re->status;
+    }
+
+    spacket_read = read(fd, (char *)tb, ulen_from_header);
+    if(spacket_read < 0 || (size_t) spacket_read < ulen_from_header) {
+        TACSYSLOG(LOG_ERR,\
+            "%s: short reply body, read %zd of %zu: %m",\
+            __FUNCTION__,\
+            ((spacket_read >= 0) ? spacket_read : 0), ulen_from_header);
+        re->msg = xstrdup(acct_syserr_msg);
+        re->status = LIBTAC_STATUS_SHORT_BODY;
+        free(th);
+        return re->status;
+    }
+
+    /* now parse remaining packet fields */
+    status = tac_acct_parse((u_char *)th, TAC_PLUS_HDR_SIZE + ulen_from_header, re);
+
+    /* all useful data has been copied out */
+    free(th);
+
+    return status;
+}    /* tac_acct_read */
+