Changeset 482 in openpam

Timestamp:

Nov 3, 2011, 4:33:02 PM (9 years ago)

Author:

Dag-Erling Smørgrav

Message:

Revert large parts of r478. I had forgotten that the module arguments
are actually passed to each service function in the classic (argc,
argv) form. The only place where the compiler could have caught this
used a type cast, and it did not show up in testing either because all
of the modules I tested use openpam_get_option(3) instead of
manipulating argv directly.

The cleaned-up policy parsing code remains in place, but options are
once more stored as strings, pretty much the way they appear in the
policy file, except that quotes are stripped.

Location:

trunk/lib

Files:

• openpam_configure.c (modified) (6 diffs)
• openpam_get_option.c (modified) (2 diffs)
• openpam_impl.h (modified) (2 diffs)
• openpam_set_option.c (modified) (3 diffs)

trunk/lib/openpam_configure.c

@@ -270 +270 @@
  * Parse an option.
  *
- * Returns a pointer to a dynamically allocated pam_opt_t with the name
- * and value of the next module option, or NULL if the end of the string
- * was reached or a disallowed non-whitespace character was encountered.
- *
- * An option consists of an option name optionally followed by an equal
- * sign and an option value.
- *
- * The structure and strings are allocated as a single object, so a single
- * free(3) call is sufficient to release the allocated memory.
+ * Returns a dynamically allocated string containing the next module
+ * option, or NULL if the end of the string was reached or a disallowed
+ * non-whitespace character was encountered.
  *
  * If parse_option() is successful, it updates *line to point one
@@ -285 +279 @@
  * all other cases, it leaves *line unmodified.
  *
- * If parse_option() fails to allocate memory for the option structure, it
- * will return NULL and set errno to a non-zero value.
+ * If parse_option() fails to allocate memory, it will return NULL and set
+ * errno to a non-zero value.
  *
  * Allowed characters for option names are all characters in the POSIX
@@ -295 +289 @@
  * Note that the entire value must be quoted, not just part of it.
  */
-static pam_opt_t *
+static char *
 parse_option(char **line)
 {
         char *nb, *ne, *vb, *ve;
         unsigned char q = 0;
-        pam_opt_t *opt;
-        size_t size;
+        char *option;
 
         errno = 0;
@@ -332 +325 @@
                 vb = ve = ne;
         }
-        size = sizeof *opt;
-        size += ne - nb + 1;
-        size += ve - vb + 1;
-        if ((opt = malloc(size)) == NULL)
+        if ((option = malloc((ne - nb) + 1 + (ve - vb) + 1)) == NULL)
                 return (NULL);
-        opt->name = (char *)opt + sizeof *opt;
-        strlcpy(opt->name, nb, ne - nb + 1);
-        opt->value = opt->name + (ne - nb) + 1;
-        strlcpy(opt->value, vb, ve - vb + 1);
+        strncpy(option, nb, ne - nb);
+        option[ne - nb] = '=';
+        strncpy(option + (ne - nb), vb, ve - vb);
+        option[(ne - nb) + 1 + (ve - vb) + 1] = '\0';
         *line = q ? ve + 1 : ve;
-        return (opt);
+        return (option);
 }
 
@@ -381 +371 @@
         pam_facility_t fclt;
         pam_control_t ctlf;
-        pam_opt_t *opt;
         char *line, *str, *name;
+        char *option, **optv;
         int len, ret;
         FILE *f;
@@ -471 +461 @@
 
                 /* get module options */
-                /* XXX quick and dirty, may waste a few hundred bytes */
-                if ((this->optv = malloc(sizeof *opt * strlen(line))) == NULL)
-                        goto syserr;
-                while ((opt = parse_option(&line)) != NULL)
-                        this->optv[this->optc++] = opt;
-                this->optv[this->optc] = NULL;
+                this->optv = NULL;
+                this->optc = 0;
+                while ((option = parse_option(&line)) != NULL) {
+                        optv = realloc(this->optv,
+                            (this->optc + 2) * sizeof *optv);
+                        if (optv == NULL)
+                                goto syserr;
+                        this->optv = optv;
+                        this->optv[this->optc++] = option;
+                        this->optv[this->optc] = NULL;
+                }
                 if (*line != '\0') {
                         openpam_log(PAM_LOG_ERROR,

trunk/lib/openpam_get_option.c

@@ -60 +60 @@
 {
         pam_chain_t *cur;
+        size_t len;
         int i;
 
@@ -66 +67 @@
                 RETURNS(NULL);
         cur = pamh->current;
-        for (i = 0; i < cur->optc; ++i)
-                if (strcmp(cur->optv[i]->name, option) == 0)
-                        RETURNS(cur->optv[i]->value);
+        len = strlen(option);
+        for (i = 0; i < cur->optc; ++i) {
+                if (strncmp(cur->optv[i], option, len) == 0) {
+                        if (cur->optv[i][len] == '\0')
+                                RETURNS(&cur->optv[i][len]);
+                        else if (cur->optv[i][len] == '=')
+                                RETURNS(&cur->optv[i][len + 1]);
+                }
+        }
         RETURNS(NULL);
 }

trunk/lib/openpam_impl.h

@@ -72 +72 @@
 } pam_facility_t;
 
-typedef struct pam_opt pam_opt_t;
-struct pam_opt {
-        char            *name;
-        char            *value;
-        char             str[];
-};
-
 typedef struct pam_chain pam_chain_t;
 struct pam_chain {
@@ -84 +77 @@
         int              flag;
         int              optc;
-        pam_opt_t      **optv;
+        char           **optv;
         pam_chain_t     *next;
 };

trunk/lib/openpam_set_option.c

@@ -63 +63 @@
 {
         pam_chain_t *cur;
-        pam_opt_t *opt, **optv;
-        int i, ol, vl;
+        char *opt, **optv;
+        size_t len;
+        int i;
 
         ENTERS(option);
@@ -70 +71 @@
                 RETURNC(PAM_SYSTEM_ERR);
         cur = pamh->current;
-        for (i = 0; i < cur->optc; ++i)
-                if (strcmp(cur->optv[i]->name, option) == 0)
+        for (len = 0; option[len] != '\0'; ++len)
+                if (option[len] == '=')
                         break;
+        for (i = 0; i < cur->optc; ++i) {
+                if (strncmp(cur->optv[i], option, len) == 0 &&
+                    (cur->optv[i][len] == '\0' || cur->optv[i][len] == '='))
+                        break;
+        }
         if (value == NULL) {
                 /* remove */
@@ -80 +86 @@
                         cur->optv[i] = cur->optv[i + 1];
                 cur->optv[i] = NULL;
-                cur->optc--;
                 RETURNC(PAM_SUCCESS);
         }
-        ol = strlen(option) + 1;
-        vl = strlen(value) + 1;
-        if ((opt = malloc(sizeof *opt + ol + vl)) == NULL)
+        if (asprintf(&opt, "%.*s=%s", (int)len, option, value) < 0)
                 RETURNC(PAM_BUF_ERR);
-        opt->name = (char *)opt + sizeof *opt;
-        strlcpy(opt->name, option, ol);
-        opt->value = opt->name + ol;
-        strlcpy(opt->value, value, vl);
         if (i == cur->optc) {
                 /* add */
-                optv = realloc(cur->optv, sizeof *optv * (cur->optc + 2));
+                optv = realloc(cur->optv, sizeof(char *) * (cur->optc + 2));
                 if (optv == NULL) {
                         FREE(opt);