[patch] doas(1): use a #define for the environment variable name length limit
No functional or user-visible changes here.
On that note, where did the "1024" (1023-char) come from? Is there
anyone who has environment variables whose name goes near 1023 chars?
Index: usr.bin/doas/env.c
===
RCS file: /cvs/src/usr.bin/doas/env.c,v
retrieving revision 1.5
diff -u -p -u -r1.5 env.c
--- usr.bin/doas/env.c 15 Sep 2016 00:58:23 - 1.5
+++ usr.bin/doas/env.c 5 Apr 2017 05:36:39 -
@@ -27,6 +27,8 @@
#include "doas.h"
+#define VARNAME_MAX 1023
+
struct envnode {
RB_ENTRY(envnode) node;
const char *key;
@@ -87,7 +89,7 @@ createenv(struct rule *rule)
struct envnode *node;
const char *e, *eq;
size_t len;
- char name[1024];
+ char name[VARNAME_MAX + 1];
e = environ[i];
@@ -95,7 +97,7 @@ createenv(struct rule *rule)
if ((eq = strchr(e, '=')) == NULL || eq == e)
continue;
len = eq - e;
- if (len > sizeof(name) - 1)
+ if (len > VARNAME_MAX)
continue;
memcpy(name, e, len);
name[len] = '\0';
@@ -139,7 +141,7 @@ fillenv(struct env *env, const char **en
struct envnode *node, key;
const char *e, *eq;
const char *val;
- char name[1024];
+ char name[VARNAME_MAX + 1];
u_int i;
size_t len;
@@ -151,7 +153,7 @@ fillenv(struct env *env, const char **en
len = strlen(e);
else
len = eq - e;
- if (len > sizeof(name) - 1)
+ if (len > VARNAME_MAX)
continue;
memcpy(name, e, len);
name[len] = '\0';
Re: [patch 1/2] doas(1): Moved some parsing from env.c into parse.y
Hello Philippe,
On Thu, 16 Mar 2017 10:19:12 -0400
Philippe Meunier wrote:
> Ted Unangst wrote:
> >Did I get it backwards? If you have setenv { HERE= there }, your diff
> >changes behavior.
>
> Speaking from the peanut gallery here, but I find this syntax rather
> confusing and error-prone, especially for a security-related file
> such as doas.conf. How about making the list of variables
> comma-separated instead of space-separated, so that the intent is
> clearer from the syntax?
Whilst I agree with this, I presume this would depend on the popular use
case. If Ted and co found the major use case to be just to pass
variables along like this:
setenv { FOO BAR }
then a space-delimited list is fine, and the parser is simpler. As I
see it, this is also acceptable:
setenv { "FOO=" "BAR" }
Adding a comma-separated list will be similar to what I did to move the
'=' processing into the lexer. It's possible, but it does add some code
to the lexer and doesn't allow for much refactoring, unless you also
have command line args like this:
... cmd hello args foo, bar
It also means that command-line arguments with commas in them will need
to be enclosed in quote marks.
I have had a lot of time to think about this. Since equal sign
processing is needed anyway in env.c to process the strings in the
"environ" variable (filled in by the OS), might as well keep it all in
one place.
Re: __pure __dead
On Wed, 5 Apr 2017, Mark Kettenis wrote: > Our only defines these if when __STRICT_ANSI__ isn't > defined. As a result, functions like exit(3) are not properly marked > as "noreturn". This in turn makes compilers complain that other > "noreturn" functions actually may return. ok guenther@ on your existing diff, but... > Alternatively we could gut the pre-gcc-2.5 case completely and simply > change this into: > > #if __GNUC_PREREQ__(2, 5) > #define __dead __attribute__((__noreturn__)) > #define __pure __attribute__((__const__)) > #else > #define __dead > #define __pure > #endif > > Thoughts? ok? Should this wait a bit to get better testing in ports? *If* we keep support for pre-2.5 gcc (?! why?) then the antique branch of the #if would need to preserve the #define __attribute__(x) line. But I'm with millert@ in thinking that we shouldn't keep that support. Philip
Re: __pure __dead
OK millert@ We should probably consider pruning out all the gcc < 3.x bits from sys/cdefs.h. - todd
Re: syslogd log_debug
On Fri, Mar 17, 2017 at 02:09:35AM +0100, Alexander Bluhm wrote:
> This is the next step for refactoring internal syslogd(8) logging.
>
> Replace logdebug() with generic log_debug() from log.c. Implement
> log_debugadd() to construct debug message incrementally.
Updated diff after unlock, ok?
bluhm
Index: usr.sbin/syslogd/log.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/log.c,v
retrieving revision 1.1
diff -u -p -r1.1 log.c
--- usr.sbin/syslogd/log.c 16 Mar 2017 23:55:19 - 1.1
+++ usr.sbin/syslogd/log.c 4 Apr 2017 23:51:59 -
@@ -17,8 +17,10 @@
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
+#include
#include
#include
+#include
#include
#include
#include
@@ -30,6 +32,8 @@ static int debug;
static int verbose;
static int facility;
static const char *log_procname;
+static char*debug_ebuf;
+static size_t debug_length;
void
log_init(int n_debug, int fac)
@@ -41,6 +45,12 @@ log_init(int n_debug, int fac)
facility = fac;
log_procinit(__progname);
+ if (debug_ebuf == NULL)
+ if ((debug_ebuf = malloc(ERRBUFSIZE)) == NULL)
+ err(1, "allocate debug buffer");
+ debug_ebuf[0] = '\0';
+ debug_length = 0;
+
tzset();
}
@@ -150,16 +160,42 @@ log_info(int pri, const char *emsg, ...)
void
log_debug(const char *emsg, ...)
{
- char ebuf[ERRBUFSIZE];
va_list ap;
int saved_errno;
if (verbose) {
saved_errno = errno;
va_start(ap, emsg);
- vsnprintf(ebuf, sizeof(ebuf), emsg, ap);
- fprintf(stderr, "%s\n", ebuf);
+ if (debug_length < ERRBUFSIZE - 1)
+ vsnprintf(debug_ebuf + debug_length,
+ ERRBUFSIZE - debug_length, emsg, ap);
+ fprintf(stderr, "%s\n", debug_ebuf);
fflush(stderr);
+ va_end(ap);
+ errno = saved_errno;
+ }
+ debug_ebuf[0] = '\0';
+ debug_length = 0;
+}
+
+void
+log_debugadd(const char *emsg, ...)
+{
+ size_t l;
+ va_list ap;
+ int saved_errno;
+
+ if (verbose) {
+ saved_errno = errno;
+ va_start(ap, emsg);
+ if (debug_length < ERRBUFSIZE - 1) {
+ l = vsnprintf(debug_ebuf + debug_length,
+ ERRBUFSIZE - debug_length, emsg, ap);
+ if (l < ERRBUFSIZE - debug_length)
+ debug_length += l;
+ else
+ debug_length = ERRBUFSIZE - 1;
+ }
va_end(ap);
errno = saved_errno;
}
Index: usr.sbin/syslogd/log.h
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/log.h,v
retrieving revision 1.1
diff -u -p -r1.1 log.h
--- usr.sbin/syslogd/log.h 16 Mar 2017 23:55:19 - 1.1
+++ usr.sbin/syslogd/log.h 4 Apr 2017 23:51:59 -
@@ -38,6 +38,8 @@ void log_info(int, const char *, ...)
__attribute__((__format__ (printf, 2, 3)));
void log_debug(const char *, ...)
__attribute__((__format__ (printf, 1, 2)));
+void log_debugadd(const char *, ...)
+ __attribute__((__format__ (printf, 1, 2)));
void logit(int, const char *, ...)
__attribute__((__format__ (printf, 2, 3)));
void vlog(int, const char *, va_list)
Index: usr.sbin/syslogd/privsep.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/privsep.c,v
retrieving revision 1.66
diff -u -p -r1.66 privsep.c
--- usr.sbin/syslogd/privsep.c 30 Dec 2016 23:21:26 - 1.66
+++ usr.sbin/syslogd/privsep.c 4 Apr 2017 23:51:59 -
@@ -35,6 +35,7 @@
#include
#include
+#include "log.h"
#include "syslogd.h"
/*
@@ -208,7 +209,7 @@ priv_exec(char *conf, int numeric, int c
sigaction(SIGCHLD, &sa, NULL);
setproctitle("[priv]");
- logdebug("[priv]: fork+exec done\n");
+ log_debug("[priv]: fork+exec done");
sigemptyset(&sigmask);
if (sigprocmask(SIG_SETMASK, &sigmask, NULL) == -1)
@@ -226,7 +227,7 @@ priv_exec(char *conf, int numeric, int c
break;
switch (cmd) {
case PRIV_OPEN_TTY:
- logdebug("[priv]: msg PRIV_OPEN_TTY received\n");
+ log_debug("[priv]: msg PRIV_OPEN_TTY received");
/* Expecting: length, path */
must_read(sock, &path_len, sizeof(size_t));
if (path_len == 0 || path_len > sizeof(path))
@@ -244,7 +245,7 @@ priv_exec(char *conf, int numeric, int c
Re: Better error output for readlink(1)
> On Apr 4, 2017, at 4:46 PM, Nicholas Marriott
> wrote:
>
> readlink is explicitly documented to silently exit 1 if run without -f,
> and GNU readlink behaves the same way. I doubt that should change.
Yeah, I saw that. I (incorrectly, I guess?) interpreted it to mean
"does not print anything on the standard output."
My think is -- and maybe I'm being nitpicky, but -- the documentation
says:
> If the -f option is not specified and readlink is invoked with an
> argument other than the pathname of a symbolic link, it exits with
> a nonzero exit code without printing anything.
In the current code, however, you could have insufficient permissions
for a part of the path (EPERM), or an I/O failure (EIO), but otherwise
specify a valid symbolic link, and still get the described behavior,
which seems wrong to me.
If the documented "say nothing and exit 1" behavior when the target
is not a symbolic link is sacred, then maybe this snippet is better:
if (fflag) {
if (realpath(argv[0], buf) == NULL)
err(1, "%s", argv[0]);
} else {
- if ((n = readlink(argv[0], buf, sizeof buf-1)) < 0)
- exit(1);
+ if ((n = readlink(argv[0], buf, sizeof(buf) - 1)) == -1) {
+ if (errno != EINVAL)
+ warn("%s", argv[0]);
+ return 1;
+ }
buf[n] = '\0';
}
That way you still say nothing if the issue is, in fact, that your target
isn't a symbolic link, but you point out any other errors.
--
Scott Cheloha
__pure __dead
Our only defines these if when __STRICT_ANSI__ isn't defined. As a result, functions like exit(3) are not properly marked as "noreturn". This in turn makes compilers complain that other "noreturn" functions actually may return. This is most visible in various llvm projects that tend to be compiled with -std=c++11 or -std=c++14 and therefore are compiled with __STRICT_ANSI__ defined. It looks like the !defined(__STRICT_ANSI__) condition was inherited from the pre-gcc-2.5 case, where warnings would be generated in "strict" mode. But that doesn't happen with the post-gcc-2.5 case, so the __STRICT_ANSI__ check isn't necessary. Alternatively we could gut the pre-gcc-2.5 case completely and simply change this into: #if __GNUC_PREREQ__(2, 5) #define __dead __attribute__((__noreturn__)) #define __pure __attribute__((__const__)) #else #define __dead #define __pure #endif Thoughts? ok? Should this wait a bit to get better testing in ports? Index: sys/cdefs.h === RCS file: /cvs/src/sys/sys/cdefs.h,v retrieving revision 1.40 diff -u -p -r1.40 cdefs.h --- sys/cdefs.h 6 Jan 2017 14:22:30 - 1.40 +++ sys/cdefs.h 4 Apr 2017 21:47:13 - @@ -102,7 +102,7 @@ #define__dead __volatile #define__pure __const #endif -#elif !defined(__STRICT_ANSI__) +#else #define __dead __attribute__((__noreturn__)) #define __pure __attribute__((__const__)) #endif
Re: Better error output for readlink(1)
Hi
readlink is explicitly documented to silently exit 1 if run without -f,
and GNU readlink behaves the same way. I doubt that should change.
On Tue, Apr 04, 2017 at 04:40:19PM -0500, Scott Cheloha wrote:
> This patch replaces a custom error message in readlink.c with err(3).
> The custom message and call to strlen(3) aren't needed because
> readlink(2) checks the length of the argument string and sets errno
> to ENAMETOOLONG if it is too long.
>
> Dropping strlen(3) lets us drop string.h.
>
> Using err(3) also lets us trivially report the myriad ways readlink(2)
> can fail. At the moment we just exit 1, which can be misleading during
> interactive use.
>
> While here, do other miscellaneous style(9)-type changes.
>
> Any takers?
>
> --
> Scott Cheloha
>
> P.S. returning from main() is preferred over exit(3), right?
>
> Index: readlink.c
> ===
> RCS file: /cvs/src/usr.bin/readlink/readlink.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 readlink.c
> --- readlink.c9 Oct 2015 01:37:08 - 1.27
> +++ readlink.c4 Apr 2017 21:10:41 -
> @@ -32,17 +32,17 @@
> #include
> #include
> #include
> -#include
> #include
>
> -static void usage(void);
> +static __dead void usage(void);
>
> int
> main(int argc, char *argv[])
> {
> char buf[PATH_MAX];
> - int n, ch, nflag = 0, fflag = 0;
> - extern int optind;
> + int ch, fflag, n, nflag;
> +
> + fflag = nflag = 0;
>
> if (pledge("stdio rpath", NULL) == -1)
> err(1, "pledge");
> @@ -64,30 +64,26 @@ main(int argc, char *argv[])
> if (argc != 1)
> usage();
>
> - n = strlen(argv[0]);
> - if (n > PATH_MAX - 1) {
> - fprintf(stderr,
> - "readlink: filename longer than PATH_MAX-1 (%d)\n",
> - PATH_MAX - 1);
> - exit(1);
> - }
> -
> if (fflag) {
> if (realpath(argv[0], buf) == NULL)
> err(1, "%s", argv[0]);
> } else {
> - if ((n = readlink(argv[0], buf, sizeof buf-1)) < 0)
> - exit(1);
> + if ((n = readlink(argv[0], buf, sizeof(buf) - 1)) == -1) {
> + if (errno == EINVAL)
> + errx(1, "%s: Not a symbolic link", argv[0]);
> + else
> + err(1, "%s", argv[0]);
> + }
> buf[n] = '\0';
> }
>
> printf("%s", buf);
> if (!nflag)
> putchar('\n');
> - exit(0);
> + return 0;
> }
>
> -static void
> +static __dead void
> usage(void)
> {
> (void)fprintf(stderr, "usage: readlink [-fn] file\n");
>
Better error output for readlink(1)
This patch replaces a custom error message in readlink.c with err(3).
The custom message and call to strlen(3) aren't needed because
readlink(2) checks the length of the argument string and sets errno
to ENAMETOOLONG if it is too long.
Dropping strlen(3) lets us drop string.h.
Using err(3) also lets us trivially report the myriad ways readlink(2)
can fail. At the moment we just exit 1, which can be misleading during
interactive use.
While here, do other miscellaneous style(9)-type changes.
Any takers?
--
Scott Cheloha
P.S. returning from main() is preferred over exit(3), right?
Index: readlink.c
===
RCS file: /cvs/src/usr.bin/readlink/readlink.c,v
retrieving revision 1.27
diff -u -p -r1.27 readlink.c
--- readlink.c 9 Oct 2015 01:37:08 - 1.27
+++ readlink.c 4 Apr 2017 21:10:41 -
@@ -32,17 +32,17 @@
#include
#include
#include
-#include
#include
-static voidusage(void);
+static __dead void usage(void);
int
main(int argc, char *argv[])
{
char buf[PATH_MAX];
- int n, ch, nflag = 0, fflag = 0;
- extern int optind;
+ int ch, fflag, n, nflag;
+
+ fflag = nflag = 0;
if (pledge("stdio rpath", NULL) == -1)
err(1, "pledge");
@@ -64,30 +64,26 @@ main(int argc, char *argv[])
if (argc != 1)
usage();
- n = strlen(argv[0]);
- if (n > PATH_MAX - 1) {
- fprintf(stderr,
- "readlink: filename longer than PATH_MAX-1 (%d)\n",
- PATH_MAX - 1);
- exit(1);
- }
-
if (fflag) {
if (realpath(argv[0], buf) == NULL)
err(1, "%s", argv[0]);
} else {
- if ((n = readlink(argv[0], buf, sizeof buf-1)) < 0)
- exit(1);
+ if ((n = readlink(argv[0], buf, sizeof(buf) - 1)) == -1) {
+ if (errno == EINVAL)
+ errx(1, "%s: Not a symbolic link", argv[0]);
+ else
+ err(1, "%s", argv[0]);
+ }
buf[n] = '\0';
}
printf("%s", buf);
if (!nflag)
putchar('\n');
- exit(0);
+ return 0;
}
-static void
+static __dead void
usage(void)
{
(void)fprintf(stderr, "usage: readlink [-fn] file\n");
Re: rtadvd: asprintf
> > The last commit to this file fixed two (void)asprintf calls. Fix the > > two remaining calls: depend on the ret buffer being set to NULL on > > failure. > > Personally, I would leave vltimexpire and pltimexpire initialized > to NULL and avoid those else clauses. Anything which makes the next review pass easier. Correct idioms should be easy to spot, incorrect ones also. It's the middle ground where the idiom isn't clear, that I despise.
Re: rtadvd: asprintf
On Tue, 04 Apr 2017 19:08:13 +0200, Jeremie Courreges-Anglas wrote: > The last commit to this file fixed two (void)asprintf calls. Fix the > two remaining calls: depend on the ret buffer being set to NULL on > failure. Personally, I would leave vltimexpire and pltimexpire initialized to NULL and avoid those else clauses. Either way OK millert@ - todd
rtadvd: asprintf
The last commit to this file fixed two (void)asprintf calls. Fix the
two remaining calls: depend on the ret buffer being set to NULL on
failure.
ok?
Index: dump.c
===
RCS file: /d/cvs/src/usr.sbin/rtadvd/dump.c,v
retrieving revision 1.22
diff -u -p -p -u -r1.22 dump.c
--- dump.c 2 Apr 2017 22:57:20 - 1.22
+++ dump.c 4 Apr 2017 16:53:22 -
@@ -113,7 +113,7 @@ rtadvd_dump(void)
int first;
struct timeval now, next;
char *origin, *vltime, *pltime, *flags;
- char *vltimexpire=NULL, *pltimexpire=NULL;
+ char *vltimexpire, *pltimexpire;
char ctimebuf[26];
gettimeofday(&now, NULL);
@@ -186,16 +186,23 @@ rtadvd_dump(void)
default:
origin = "";
}
- if (pfx->vltimeexpire != 0)
+ if (pfx->vltimeexpire != 0) {
/* truncate to onwire value */
- asprintf(&vltimexpire, "(decr,expire %u)",
+ if (asprintf(&vltimexpire, "(decr,expire %u)",
(u_int32_t)(pfx->vltimeexpire > now.tv_sec ?
- pfx->vltimeexpire - now.tv_sec : 0));
- if (pfx->pltimeexpire != 0)
+ pfx->vltimeexpire - now.tv_sec : 0)) == -1)
+ vltimexpire = NULL;
+ } else
+ vltimexpire = NULL;
+
+ if (pfx->pltimeexpire != 0) {
/* truncate to onwire value */
- asprintf(&pltimexpire, "(decr,expire %u)",
+ if (asprintf(&pltimexpire, "(decr,expire %u)",
(u_int32_t)(pfx->pltimeexpire > now.tv_sec ?
- pfx->pltimeexpire - now.tv_sec : 0));
+ pfx->pltimeexpire - now.tv_sec : 0)) == -1)
+ pltimexpire = NULL;
+ } else
+ pltimexpire = NULL;
vltime = lifetime(pfx->validlifetime);
pltime = lifetime(pfx->preflifetime);
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
rtadvd: recvmsg/sendmsg return value
Said functions return an ssize_t. Comparing (size_t < 0) isn't going to
give good results. The type change in ra_output() deserves its own
commit.
Also, use the same type and the same name for both return values; "i" is
too generic.
ra_input()/rs_input() take an int, but I don't think we need to change
their signatures (no actual risk of overflow).
Last hunk: I don't think we want a failed send to be counted as
a successful one.
Compile-tested only for now. Thoughts? ok?
Index: rtadvd.c
===
RCS file: /d/cvs/src/usr.sbin/rtadvd/rtadvd.c,v
retrieving revision 1.83
diff -u -p -p -u -r1.83 rtadvd.c
--- rtadvd.c20 Jan 2017 23:29:58 - 1.83
+++ rtadvd.c4 Apr 2017 07:56:09 -
@@ -463,7 +463,7 @@ rtsock_cb(int fd, short event, void *arg
void
sock_cb(int fd, short event, void *arg)
{
- int i;
+ ssize_t len;
int *hlimp = NULL;
struct icmp6_hdr *icp;
int ifindex = 0;
@@ -478,7 +478,7 @@ sock_cb(int fd, short event, void *arg)
* receive options.
*/
rcvmhdr.msg_controllen = rcvcmsgbuflen;
- if ((i = recvmsg(sock, &rcvmhdr, 0)) < 0)
+ if ((len = recvmsg(sock, &rcvmhdr, 0)) < 0)
return;
/* extract optional information via Advanced API */
@@ -516,8 +516,8 @@ sock_cb(int fd, short event, void *arg)
return;
}
- if (i < sizeof(struct icmp6_hdr)) {
- log_warnx("packet size(%d) is too short", i);
+ if (len < sizeof(struct icmp6_hdr)) {
+ log_warnx("packet size(%zd) is too short", len);
return;
}
@@ -548,15 +548,14 @@ sock_cb(int fd, short event, void *arg)
if_indextoname(pi->ipi6_ifindex, ifnamebuf));
return;
}
- if (i < sizeof(struct nd_router_solicit)) {
- log_info("RS from %s on %s does not have enough "
- "length (len = %d)",
+ if (len < sizeof(struct nd_router_solicit)) {
+ log_info("RS from %s on %s too short (len = %zd)",
inet_ntop(AF_INET6, &from.sin6_addr, ntopbuf,
INET6_ADDRSTRLEN),
- if_indextoname(pi->ipi6_ifindex, ifnamebuf), i);
+ if_indextoname(pi->ipi6_ifindex, ifnamebuf), len);
return;
}
- rs_input(i, (struct nd_router_solicit *)icp, pi, &from);
+ rs_input(len, (struct nd_router_solicit *)icp, pi, &from);
break;
case ND_ROUTER_ADVERT:
/*
@@ -581,15 +580,14 @@ sock_cb(int fd, short event, void *arg)
if_indextoname(pi->ipi6_ifindex, ifnamebuf));
return;
}
- if (i < sizeof(struct nd_router_advert)) {
- log_info("RA from %s on %s does not have enough "
- "length (len = %d)",
+ if (len < sizeof(struct nd_router_advert)) {
+ log_info("RA from %s on %s too short (len = %zd)",
inet_ntop(AF_INET6, &from.sin6_addr, ntopbuf,
INET6_ADDRSTRLEN),
- if_indextoname(pi->ipi6_ifindex, ifnamebuf), i);
+ if_indextoname(pi->ipi6_ifindex, ifnamebuf), len);
return;
}
- ra_input(i, (struct nd_router_advert *)icp, pi, &from);
+ ra_input(len, (struct nd_router_advert *)icp, pi, &from);
break;
default:
/*
@@ -1218,7 +1216,7 @@ ra_output(struct rainfo *rainfo)
{
struct cmsghdr *cm;
struct in6_pktinfo *pi;
- size_t len;
+ ssize_t len;
if ((iflist[rainfo->ifindex]->ifm_flags & IFF_UP) == 0) {
log_debug("%s is not up, skip sending RA", rainfo->ifname);
@@ -1255,9 +1253,10 @@ ra_output(struct rainfo *rainfo)
rainfo->ifname, rainfo->waiting);
len = sendmsg(sock, &sndmhdr, 0);
-
- if (len < 0)
+ if (len < 0) {
log_warn("sendmsg on %s", rainfo->ifname);
+ return;
+ }
/* update counter */
if (rainfo->initcounter < MAX_INITIAL_RTR_ADVERTISEMENTS)
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: ifconfig.8: Small improvement, typofix
On Tue, Apr 04, 2017 at 04:12:16PM +0200, Mike Belopuhov wrote: > On Tue, Apr 04, 2017 at 15:32 +0200, Klemens Nanni wrote: > > Hey, > > > > blocknonip's description is a tad clearer that way, the rest is mostly > > cosmetical but still. > > > Index: sbin/ifconfig/ifconfig.8 > > === > > RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v > > retrieving revision 1.280 > > diff -u -p -r1.280 ifconfig.8 > > --- sbin/ifconfig/ifconfig.820 Mar 2017 14:06:26 - 1.280 > > +++ sbin/ifconfig/ifconfig.84 Apr 2017 09:33:37 - > > @@ -400,7 +400,7 @@ output from > > .Cm hwfeatures > > shows the maximum supported MTU. > > .It Cm netmask Ar mask > > -(inet and inet6) > > +(inet and inet6 only) > > Specify how much of the address to reserve for subdividing > > networks into subnetworks. > > The mask includes the network part of the local address > > @@ -508,7 +508,7 @@ Disable automatic point-to-point link de > > .It Cm blocknonip Ar interface > > Mark > > .Ar interface > > -so that no non-IPv4, IPv6, ARP, or Reverse > > +so that only IPv4, IPv6, ARP, and Reverse > > ARP packets are accepted from it or forwarded to it from other > > bridge member interfaces. > > .It Cm -blocknonip Ar interface > > @@ -824,7 +824,7 @@ The formula is > > + > > .Pf ( Cm advskew > > / 256). > > -If the master does not advertise within three times this interval, this > > host > > +If the master does not advertise this interval within three times, this > > host > > will begin advertising as master. > > .Sh IEEE 802.11 (WIRELESS DEVICES) > > .nr nS 1 > > @@ -1520,7 +1520,7 @@ Remove the trunk port > > Set the trunk protocol. > > Refer to > > .Xr trunk 4 > > -for a complete list of the available protocols, > > +for a complete list of the available protocols. > > .El > > .Sh TUNNEL > > .nr nS 1 > > I agree that this reads better. OK mikeb > please commit this with my ok too. but one adjustment is incorrect: > > -If the master does not advertise within three times this interval, this > > host > > +If the master does not advertise this interval within three times, this > > host the change is wrong, because "within three times what"? the interval of course. it's just the wording that's a bit off. try: ...does not advertise within thrice this interval, ... that's how it should read, but i'm aware some people will faint if you use the word "thrice". if you want to come up with something else feel free, but don;t commit that change as-is. jmc
Re: ifconfig.8: Small improvement, typofix
> -If the master does not advertise within three times this interval, this host > +If the master does not advertise this interval within three times, this host > I think the old phrasing is easier to understand.
Re: ifconfig.8: Small improvement, typofix
On Tue, Apr 04, 2017 at 15:32 +0200, Klemens Nanni wrote: > Hey, > > blocknonip's description is a tad clearer that way, the rest is mostly > cosmetical but still. > Index: sbin/ifconfig/ifconfig.8 > === > RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v > retrieving revision 1.280 > diff -u -p -r1.280 ifconfig.8 > --- sbin/ifconfig/ifconfig.8 20 Mar 2017 14:06:26 - 1.280 > +++ sbin/ifconfig/ifconfig.8 4 Apr 2017 09:33:37 - > @@ -400,7 +400,7 @@ output from > .Cm hwfeatures > shows the maximum supported MTU. > .It Cm netmask Ar mask > -(inet and inet6) > +(inet and inet6 only) > Specify how much of the address to reserve for subdividing > networks into subnetworks. > The mask includes the network part of the local address > @@ -508,7 +508,7 @@ Disable automatic point-to-point link de > .It Cm blocknonip Ar interface > Mark > .Ar interface > -so that no non-IPv4, IPv6, ARP, or Reverse > +so that only IPv4, IPv6, ARP, and Reverse > ARP packets are accepted from it or forwarded to it from other > bridge member interfaces. > .It Cm -blocknonip Ar interface > @@ -824,7 +824,7 @@ The formula is > + > .Pf ( Cm advskew > / 256). > -If the master does not advertise within three times this interval, this host > +If the master does not advertise this interval within three times, this host > will begin advertising as master. > .Sh IEEE 802.11 (WIRELESS DEVICES) > .nr nS 1 > @@ -1520,7 +1520,7 @@ Remove the trunk port > Set the trunk protocol. > Refer to > .Xr trunk 4 > -for a complete list of the available protocols, > +for a complete list of the available protocols. > .El > .Sh TUNNEL > .nr nS 1 I agree that this reads better. OK mikeb
ifconfig.8: Small improvement, typofix
Hey, blocknonip's description is a tad clearer that way, the rest is mostly cosmetical but still. Index: sbin/ifconfig/ifconfig.8 === RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v retrieving revision 1.280 diff -u -p -r1.280 ifconfig.8 --- sbin/ifconfig/ifconfig.820 Mar 2017 14:06:26 - 1.280 +++ sbin/ifconfig/ifconfig.84 Apr 2017 09:33:37 - @@ -400,7 +400,7 @@ output from .Cm hwfeatures shows the maximum supported MTU. .It Cm netmask Ar mask -(inet and inet6) +(inet and inet6 only) Specify how much of the address to reserve for subdividing networks into subnetworks. The mask includes the network part of the local address @@ -508,7 +508,7 @@ Disable automatic point-to-point link de .It Cm blocknonip Ar interface Mark .Ar interface -so that no non-IPv4, IPv6, ARP, or Reverse +so that only IPv4, IPv6, ARP, and Reverse ARP packets are accepted from it or forwarded to it from other bridge member interfaces. .It Cm -blocknonip Ar interface @@ -824,7 +824,7 @@ The formula is + .Pf ( Cm advskew / 256). -If the master does not advertise within three times this interval, this host +If the master does not advertise this interval within three times, this host will begin advertising as master. .Sh IEEE 802.11 (WIRELESS DEVICES) .nr nS 1 @@ -1520,7 +1520,7 @@ Remove the trunk port Set the trunk protocol. Refer to .Xr trunk 4 -for a complete list of the available protocols, +for a complete list of the available protocols. .El .Sh TUNNEL .nr nS 1
Re: httpd/libtls: TLS client certificate revocation checking
On Sat, Apr 01, 2017 at 07:10:35PM +1030, Jack Burton wrote: > On Fri, 31 Mar 2017 13:03:44 -0700 > William Ahern wrote: > > Basically, anything short of passing through the entire certificate > > is going to be severely limiting and frustrating, to the point of > > uselessness. And as a practical matter, parsing out and passing > > through a subset of certificate data is likely going to require more > > complex code than just passing the entire certificate to the > > application. > > Thanks William. That's an interesting idea. > > Yes, I can see how having the raw client certificate available to > fastcgi responders would be useful. That would solve a few more > problems for my use case too and would provide the ultimate in > flexibility. And it's not difficult to implement. > > My initial gut feeling was that adding a tls_peer_serial() function to > libtls would be less intrusive (and more in keeping with the rest of > the tls_peer_* functions) than adding a tls_peer_cert() along the lines > you're suggesting. Converting a cert to a PEM string should be as simple as: BIO *bio = BIO_new(BIO_s_mem()); /* See NOTE 1 */ if (!bio) goto sslerror; if (!PEM_write_bio_X509(bio, crt)) goto sslerror; char *pem; long pemlen = BIO_get_mem_data(bio, &pem); assert(pemlen >= 0); /* < 0 shouldn't be possible */ ... /* copy pem string */ BIO_free(bio); /* See NOTE 1 */ compare that to getting the serial: BIGNUM *bn = BN_new(); if (!bn) goto sslerror; ASN1_INTEGER *i; if (!(i = X509_get_serialNumber(crt))) /* See NOTE 2 */ goto noserial; if (!ASN1_INTEGER_to_BN(i, bn)) goto sslerror; char *serial = BN_bn2dec(serial); if (!serial) goto sslerror; ... /* copy serial string */ BN_free(bn); NOTE 1: I usually keep a BIO buffer around as a singleton, using BIO_reset instead of BIO_free. NOTE 2: I'd try X509_get0_serialNumber unless the const-ness is troublesome. I'm cribbing from my own code, originally written for OpenSSL 0.9.8, which lacks X509_get0_serialNumber.
