sven falempin(sven.falem...@gmail.com) on 2016.06.20 17:38:40 -0400:
> Dear Tech Readers,
>
> in a pf.conf file one can do
> "silly things" = egress
Thanks for your diff, but
one, i dont think spaces in macros are useful in pf.conf.
second, we want to keep this consistent across all the parse.y in our code,
so we have to think how this affects these(*)
Below is a diff that disallows "silly things".
I thinks it's easier to check that spaces in macros can be done without.
diff --git sbin/pfctl/parse.y sbin/pfctl/parse.y
index 934438c..341d1af 100644
--- sbin/pfctl/parse.y
+++ sbin/pfctl/parse.y
@@ -94,6 +94,7 @@ struct sym {
char *nam;
char *val;
};
+int has_whitespace(const char *);
int symset(const char *, const char *, int);
char *symget(const char *);
@@ -714,6 +715,10 @@ numberstring : NUMBER
{
varset : STRING '=' varstring {
if (pf->opts & PF_OPT_VERBOSE)
printf("%s = \"%s\"\n", $1, $3);
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
err(1, "cannot store variable %s", $1);
free($1);
@@ -5455,6 +5460,16 @@ parse_config(char *filename, struct pfctl *xpf)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace(*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
symset(const char *nam, const char *val, int persist)
{
struct sym *sym;
(*)
./bin/chio/parse.y
./sbin/iked/parse.y
./sbin/ipsecctl/parse.y
./sbin/pfctl/parse.y
./usr.bin/doas/parse.y
./usr.sbin/bgpd/parse.y
./usr.sbin/dvmrpd/parse.y
./usr.sbin/eigrpd/parse.y
./usr.sbin/hostapd/parse.y
./usr.sbin/httpd/parse.y
./usr.sbin/ifstated/parse.y
./usr.sbin/iscsictl/parse.y
./usr.sbin/ldapd/parse.y
./usr.sbin/ldomctl/parse.y
./usr.sbin/ldpd/parse.y
./usr.sbin/npppd/npppd/parse.y
./usr.sbin/ntpd/parse.y
./usr.sbin/ospf6d/parse.y
./usr.sbin/ospfd/parse.y
./usr.sbin/radiusd/parse.y
./usr.sbin/relayd/parse.y
./usr.sbin/ripd/parse.y
./usr.sbin/smtpd/parse.y
./usr.sbin/snmpd/parse.y
./usr.sbin/vmd/parse.y
./usr.sbin/ypldap/parse.y
> as defined in parse.y like
>
> varset : STRING '=' varstring {
> if (pf->opts & PF_OPT_VERBOSE)
> printf("%s = \"%s\"\n", $1, $3);
> if (symset($1, $3, 0) == -1)
> err(1, "cannot store variable %s", $1);
> free($1);
> free($3);
> }
>
> and because it s better to triple check
> $ cat /tmp/pf.lol.conf
> karate = egress
> "kar ra tei" = egress
> pass on $kar\ ra\ tei
> $ pfctl -nf /tmp/pf.lol.conf
> /tmp/pf.lol.conf:3: macro 'kar' not defined
> /tmp/pf.lol.conf:3: syntax error
>
> i also tried the bash ${hope} or makefile $(madness) and even $"sillyness"
>
> I also remember that being able to read a config file with ease can save a
> lot of time
>
> "Third Floor Network Switch" = em8
> pass quick on $"Third Floor Network Switch" from www.openbsd.org to
> ($"Third Floor Network Switch":network) set prio (7,7)
>
> I was wondering why it refused such and read the code,
> fount out a lgetc function is used to read string, and first argument is
> explicit and is a switch to manage quoted string,
> so why not using it after the $macro ?
>
> --- ./parse.y Tue Apr 21 12:34:59 2015
> +++ /tmp/1 Mon Jun 20 17:04:08 2016
> @@ -5179,7 +5179,7 @@
> ; /* nothing */
> if (c == '$' && parsebuf == NULL) {
> while (1) {
> - if ((c = lgetc(0)) == EOF)
> + if ((c = lgetc(1)) == EOF)
> return (0);
>
> if (p + 1 >= buf + sizeof(buf) - 1) {
>
> of course it s not that simple as the code below show, this one works,
> the previous does not.
> :
>
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/sbin/pfctl/parse.y,v
> retrieving revision 1.648
> diff -u -r1.648 parse.y
> --- parse.y 21 Apr 2015 16:34:59 -0000 1.648
> +++ parse.y 20 Jun 2016 21:36:29 -0000
> @@ -5178,22 +5178,55 @@
> while ((c = lgetc(0)) != '\n' && c != EOF)
> ; /* nothing */
> if (c == '$' && parsebuf == NULL) {
> - while (1) {
> - if ((c = lgetc(0)) == EOF)
> - return (0);
> -
> - if (p + 1 >= buf + sizeof(buf) - 1) {
> - yyerror("string too long");
> - return (findeol());
> - }
> - if (isalnum(c) || c == '_') {
> + if ((c = lgetc(0)) == '"') {
> + quotec = c;
> + while (1) {
> + if ((c = lgetc(quotec)) == EOF)
> + return (0);
> + if (c == '\n') {
> + file->lineno++;
> + continue;
> + } else if (c == '\\') {
> + if ((next = lgetc(quotec)) == EOF)
> + return (0);
> + if (next == quotec || c == ' ' || c
> == '\t')
> + c = next;
> + else if (next == '\n') {
> + file->lineno++;
> + continue;
> + } else
> + lungetc(next);
> + } else if (c == quotec) {
> + *p = '\0';
> + break;
> + } else if (c == '\0') {
> + yyerror("syntax error");
> + return (findeol());
> + }
> + if (p + 1 >= buf + sizeof(buf) - 1) {
> + yyerror("string too long");
> + return (findeol());
> + }
> *p++ = c;
> - continue;
> }
> - *p = '\0';
> - lungetc(c);
> - break;
> - }
> + } else
> + while (1) {
> + if ((c = lgetc(0)) == EOF)
> + return (0);
> +
> + if (p + 1 >= buf + sizeof(buf) - 1) {
> + yyerror("string too long");
> + return (findeol());
> + }
> +
> + if (isalnum(c) || c == '_') {
> + *p++ = c;
> + continue;
> + }
> + *p = '\0';
> + lungetc(c);
> + break;
> + }
> val = symget(buf);
> if (val == NULL) {
> yyerror("macro '%s' not defined", buf);
>
> I do not have current build right now, but this is not something that
> change a lot.
>
> --
> ---------------------------------------------------------------------------------------------------------------------
> () ascii ribbon campaign - against html e-mail
> /\
>
--
