Date: Mon, 26 May 2003 06:32:14 -0700 (PDT) From: Dag-Erling Smorgrav <des@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 31890 for review Message-ID: <200305261332.h4QDWE8J081848@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=31890 Change 31890 by des@des.at.des.thinksec.com on 2003/05/26 06:31:28 Continue improving the new configuration parser, particularly error reporting: error messages relating to policy files now include line numbers, and the parser will warn about invalid facility names. Also fix an off-by-one bug in the option handling code. Affected files ... .. //depot/projects/openpam/include/security/openpam.h#23 edit .. //depot/projects/openpam/lib/openpam_configure.c#9 edit .. //depot/projects/openpam/lib/openpam_impl.h#26 edit .. //depot/projects/openpam/lib/openpam_load.c#18 edit .. //depot/projects/openpam/lib/openpam_readline.c#2 edit Differences ... ==== //depot/projects/openpam/include/security/openpam.h#23 (text+ko) ==== @@ -31,7 +31,7 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $P4: //depot/projects/openpam/include/security/openpam.h#22 $ + * $P4: //depot/projects/openpam/include/security/openpam.h#23 $ */ #ifndef _SECURITY_OPENPAM_H_INCLUDED @@ -126,6 +126,7 @@ #ifdef FOPEN_MAX char * openpam_readline(FILE *_f, + int *_lineno, size_t *_lenp); #endif ==== //depot/projects/openpam/lib/openpam_configure.c#9 (text+ko) ==== @@ -31,10 +31,9 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $P4: //depot/projects/openpam/lib/openpam_configure.c#8 $ + * $P4: //depot/projects/openpam/lib/openpam_configure.c#9 $ */ -#include <ctype.h> #include <errno.h> #include <stdio.h> #include <stdlib.h> @@ -44,6 +43,21 @@ #include "openpam_impl.h" +const char *_pam_facility_name[PAM_NUM_FACILITIES] = { + [PAM_ACCOUNT] = "account", + [PAM_AUTH] = "auth", + [PAM_PASSWORD] = "password", + [PAM_SESSION] = "session", +}; + +const char *_pam_control_flag_name[PAM_NUM_CONTROL_FLAGS] = { + [PAM_BINDING] = "binding", + [PAM_OPTIONAL] = "optional", + [PAM_REQUIRED] = "required", + [PAM_REQUISITE] = "requisite", + [PAM_SUFFICIENT] = "sufficient", +}; + static int openpam_load_chain(pam_chain_t **, const char *, const char *); /* @@ -67,10 +81,10 @@ { /* skip current word */ - while (*str && !isspace(*str)) + while (*str && *str != ' ') ++str; /* skip whitespace */ - while (isspace(*str)) + while (*str == ' ') ++str; return (str); } @@ -84,13 +98,26 @@ const char *end; char *word; - for (end = str; *end && !isspace(*end); ++end) + for (end = str; *end && *end != ' '; ++end) /* nothing */ ; if (asprintf(&word, "%.*s", (int)(end - str), str) < 0) return (NULL); return (word); } +/* + * Return the length of the first word in a string. + */ +static int +wordlen(const char *str) +{ + int i; + + for (i = 0; str[i] && str[i] != ' '; ++i) + /* nothing */ ; + return (i); +} + typedef enum { pam_conf_style, pam_d_style } openpam_style_t; /* @@ -105,19 +132,19 @@ { pam_chain_t *this, **next; const char *p, *q; - int count, i, ret; + int count, i, lineno, ret; char *line, *name; FILE *f; if ((f = fopen(filename, "r")) == NULL) { - openpam_log(errno == ENOENT ? PAM_LOG_NOTICE : PAM_LOG_ERROR, + openpam_log(errno == ENOENT ? PAM_LOG_DEBUG : PAM_LOG_NOTICE, "%s: %m", filename); return (0); } next = chain; this = *next = NULL; - count = 0; - while ((line = openpam_readline(f, NULL)) != NULL) { + count = lineno = 0; + while ((line = openpam_readline(f, &lineno, NULL)) != NULL) { p = line; /* match service name */ @@ -130,6 +157,14 @@ } /* match facility name */ + for (i = 0; i < PAM_NUM_FACILITIES; ++i) + if (match_word(p, _pam_facility_name[i])) + break; + if (i == PAM_NUM_FACILITIES) { + openpam_log(PAM_LOG_NOTICE, + "%s(%d): invalid facility '%.*s' (ignored)", + filename, lineno, wordlen(p), p); + } if (!match_word(p, facility)) { FREE(line); continue; @@ -141,8 +176,8 @@ p = next_word(p); if (*next_word(p) != '\0') openpam_log(PAM_LOG_NOTICE, - "%s: garbage at end of 'include' line", - filename); + "%s(%d): garbage at end of 'include' line", + filename, lineno); if ((name = dup_word(p)) == NULL) goto syserr; ret = openpam_load_chain(next, name, facility); @@ -162,30 +197,23 @@ goto syserr; /* control flag */ - if (match_word(p, "required")) { - this->flag = PAM_REQUIRED; - } else if (match_word(p, "requisite")) { - this->flag = PAM_REQUISITE; - } else if (match_word(p, "sufficient")) { - this->flag = PAM_SUFFICIENT; - } else if (match_word(p, "optional")) { - this->flag = PAM_OPTIONAL; - } else if (match_word(p, "binding")) { - this->flag = PAM_BINDING; - } else { - q = next_word(p); + for (i = 0; i < PAM_NUM_CONTROL_FLAGS; ++i) + if (match_word(p, _pam_control_flag_name[i])) + break; + if (i == PAM_NUM_CONTROL_FLAGS) { openpam_log(PAM_LOG_ERROR, - "%s: invalid control flag '%.*s'", - filename, (int)(q - p), p); + "%s(%d): invalid control flag '%.*s'", + filename, lineno, wordlen(p), p); goto fail; } + this->flag = i; /* module name */ p = next_word(p); - q = next_word(p); if (*p == '\0') { openpam_log(PAM_LOG_ERROR, - "%s: missing module name", filename); + "%s(%d): missing module name", + filename, lineno); goto fail; } if ((name = dup_word(p)) == NULL) @@ -196,6 +224,7 @@ goto fail; /* module options */ + p = q = next_word(p); while (*q != '\0') { ++this->optc; q = next_word(q); @@ -204,16 +233,16 @@ if (this->optv == NULL) goto syserr; for (i = 0; i < this->optc; ++i) { - p = next_word(p); if ((this->optv[i] = dup_word(p)) == NULL) goto syserr; + p = next_word(p); } /* hook it up */ *next = this; next = &this->next; this = NULL; - ++count; + ++count; /* next please... */ FREE(line); @@ -273,13 +302,6 @@ return (0); } -const char *_pam_chain_name[PAM_NUM_CHAINS] = { - [PAM_AUTH] = "auth", - [PAM_ACCOUNT] = "account", - [PAM_SESSION] = "session", - [PAM_PASSWORD] = "password" -}; - /* * OpenPAM internal * @@ -292,12 +314,12 @@ { int i, ret; - for (i = 0; i < PAM_NUM_CHAINS; ++i) { + for (i = 0; i < PAM_NUM_FACILITIES; ++i) { ret = openpam_load_chain(&pamh->chains[i], - service, _pam_chain_name[i]); + service, _pam_facility_name[i]); if (ret == 0) ret = openpam_load_chain(&pamh->chains[i], - PAM_OTHER, _pam_chain_name[i]); + PAM_OTHER, _pam_facility_name[i]); if (ret < 0) { openpam_clear_chains(pamh->chains); return (PAM_SYSTEM_ERR); ==== //depot/projects/openpam/lib/openpam_impl.h#26 (text+ko) ==== @@ -31,7 +31,7 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $P4: //depot/projects/openpam/lib/openpam_impl.h#25 $ + * $P4: //depot/projects/openpam/lib/openpam_impl.h#26 $ */ #ifndef _OPENPAM_IMPL_H_INCLUDED @@ -49,21 +49,21 @@ /* * Control flags */ +#define PAM_BINDING 0 #define PAM_REQUIRED 1 #define PAM_REQUISITE 2 #define PAM_SUFFICIENT 3 #define PAM_OPTIONAL 4 -#define PAM_BINDING 5 -#define PAM_NUM_CONTROLFLAGS 6 +#define PAM_NUM_CONTROL_FLAGS 5 /* - * Chains + * Facilities */ #define PAM_AUTH 0 #define PAM_ACCOUNT 1 #define PAM_SESSION 2 #define PAM_PASSWORD 3 -#define PAM_NUM_CHAINS 4 +#define PAM_NUM_FACILITIES 4 typedef struct pam_chain pam_chain_t; struct pam_chain { @@ -86,7 +86,7 @@ char *service; /* chains */ - pam_chain_t *chains[PAM_NUM_CHAINS]; + pam_chain_t *chains[PAM_NUM_FACILITIES]; pam_chain_t *current; int primitive; ==== //depot/projects/openpam/lib/openpam_load.c#18 (text+ko) ==== @@ -31,7 +31,7 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $P4: //depot/projects/openpam/lib/openpam_load.c#17 $ + * $P4: //depot/projects/openpam/lib/openpam_load.c#18 $ */ #include <dlfcn.h> @@ -170,7 +170,7 @@ { int i; - for (i = 0; i < PAM_NUM_CHAINS; ++i) + for (i = 0; i < PAM_NUM_FACILITIES; ++i) openpam_destroy_chain(policy[i]); } ==== //depot/projects/openpam/lib/openpam_readline.c#2 (text+ko) ==== @@ -31,7 +31,7 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $P4: //depot/projects/openpam/lib/openpam_readline.c#1 $ + * $P4: //depot/projects/openpam/lib/openpam_readline.c#2 $ */ #include <ctype.h> @@ -50,7 +50,7 @@ */ char * -openpam_readline(FILE *f, size_t *lenp) +openpam_readline(FILE *f, int *lineno, size_t *lenp) { char *line; size_t len, size; @@ -92,6 +92,9 @@ } /* eol */ if (ch == '\n') { + if (lineno != NULL) + ++*lineno; + /* remove trailing whitespace */ while (len > 0 && isspace(line[len - 1])) --len; @@ -140,6 +143,9 @@ * If a line ends in a backslash, the backslash is stripped and the next * line is appended. * + * If =lineno is not =NULL, the integer variable it points to is + * incremented every time a newline character is read. + * * If =lenp is not =NULL, the length of the line (not including the * terminating NUL character) is stored in the variable it points to. *
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200305261332.h4QDWE8J081848>