On Tue, Aug 19, 2014 at 01:59:42AM +0200, Alexander Bluhm wrote:
> I will split this diff into smaller parts to make review and
> discussion easier.
Replace gethostbyaddr(3) with getnameinfo(3).
Note that I remove the sigprocmask() that was added in rev 1.23
before privsep. It was necessary because gethostbyaddr() is not
signal safe.
ok?
bluhm
Index: usr.sbin/syslogd/privsep.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/privsep.c,v
retrieving revision 1.36
diff -u -p -u -p -r1.36 privsep.c
--- usr.sbin/syslogd/privsep.c 19 Aug 2014 00:53:01 -0000 1.36
+++ usr.sbin/syslogd/privsep.c 19 Aug 2014 20:01:12 -0000
@@ -68,7 +68,7 @@ enum cmd_types {
PRIV_OPEN_CONFIG, /* open config file for reading only */
PRIV_CONFIG_MODIFIED, /* check if config file has been modified */
PRIV_GETHOSTSERV, /* resolve host/service names */
- PRIV_GETHOSTBYADDR, /* resolve numeric address into hostname */
+ PRIV_GETNAMEINFO, /* resolve numeric address into hostname */
PRIV_DONE_CONFIG_PARSE /* signal that the initial config parse is done
*/
};
@@ -76,7 +76,7 @@ static int priv_fd = -1;
static volatile pid_t child_pid = -1;
static char config_file[MAXPATHLEN];
static struct stat cf_info;
-static int allow_gethostbyaddr = 0;
+static int allow_getnameinfo = 0;
static volatile sig_atomic_t cur_state = STATE_INIT;
/* Queue for the allowed logfiles */
@@ -100,12 +100,12 @@ static int may_read(int, void *, size_t
int
priv_init(char *conf, int numeric, int lockfd, int nullfd, char *argv[])
{
- int i, fd, socks[2], cmd, addr_len, addr_af, result, restart;
+ int i, fd, socks[2], cmd, addr_len, result, restart;
size_t path_len, hostname_len, servname_len;
char path[MAXPATHLEN], hostname[MAXHOSTNAMELEN];
char servname[MAXHOSTNAMELEN];
+ struct sockaddr_storage addr;
struct stat cf_stat;
- struct hostent *hp;
struct passwd *pw;
struct addrinfo hints, *res0;
struct sigaction sa;
@@ -191,11 +191,11 @@ priv_init(char *conf, int numeric, int l
if (stat(config_file, &cf_info) < 0)
err(1, "stat config file failed");
- /* Save whether or not the child can have access to gethostbyaddr(3) */
+ /* Save whether or not the child can have access to getnameinfo(3) */
if (numeric > 0)
- allow_gethostbyaddr = 0;
+ allow_getnameinfo = 0;
else
- allow_gethostbyaddr = 1;
+ allow_getnameinfo = 1;
TAILQ_INIT(&lognames);
increase_state(STATE_CONFIG);
@@ -320,24 +320,24 @@ priv_init(char *conf, int numeric, int l
}
break;
- case PRIV_GETHOSTBYADDR:
- dprintf("[priv]: msg PRIV_GETHOSTBYADDR received\n");
- if (!allow_gethostbyaddr)
- errx(1, "rejected attempt to gethostbyaddr");
- /* Expecting: length, address, address family */
+ case PRIV_GETNAMEINFO:
+ dprintf("[priv]: msg PRIV_GETNAMEINFO received\n");
+ if (!allow_getnameinfo)
+ errx(1, "rejected attempt to getnameinfo");
+ /* Expecting: length, sockaddr */
must_read(socks[0], &addr_len, sizeof(int));
- if (addr_len <= 0 || addr_len > sizeof(hostname))
+ if (addr_len <= 0 || addr_len > sizeof(addr))
_exit(1);
- must_read(socks[0], hostname, addr_len);
- must_read(socks[0], &addr_af, sizeof(int));
- hp = gethostbyaddr(hostname, addr_len, addr_af);
- if (hp == NULL) {
+ must_read(socks[0], &addr, addr_len);
+ if (getnameinfo((struct sockaddr *)&addr, addr_len,
+ hostname, sizeof(hostname), NULL, 0,
+ NI_NOFQDN|NI_NAMEREQD|NI_DGRAM) != 0) {
addr_len = 0;
must_write(socks[0], &addr_len, sizeof(int));
} else {
- addr_len = strlen(hp->h_name) + 1;
+ addr_len = strlen(hostname) + 1;
must_write(socks[0], &addr_len, sizeof(int));
- must_write(socks[0], hp->h_name, addr_len);
+ must_write(socks[0], hostname, addr_len);
}
break;
default:
@@ -702,33 +702,33 @@ priv_gethostserv(char *host, char *serv,
/* Reverse address resolution; response is placed into res, and length of
* response is returned (zero on error) */
int
-priv_gethostbyaddr(char *addr, int addr_len, int af, char *res, size_t res_len)
+priv_getnameinfo(struct sockaddr *sa, socklen_t salen, char *host,
+ size_t hostlen)
{
int cmd, ret_len;
if (priv_fd < 0)
errx(1, "%s called from privileged portion", __func__);
- cmd = PRIV_GETHOSTBYADDR;
+ cmd = PRIV_GETNAMEINFO;
must_write(priv_fd, &cmd, sizeof(int));
- must_write(priv_fd, &addr_len, sizeof(int));
- must_write(priv_fd, addr, addr_len);
- must_write(priv_fd, &af, sizeof(int));
+ must_write(priv_fd, &salen, sizeof(int));
+ must_write(priv_fd, sa, salen);
/* Expect back an integer size, and then a string of that length */
must_read(priv_fd, &ret_len, sizeof(int));
/* Check there was no error (indicated by a return of 0) */
if (!ret_len)
- return 0;
+ return (-1);
/* Check we don't overflow the passed in buffer */
- if (res_len < ret_len)
+ if (hostlen < ret_len)
errx(1, "%s: overflow attempt in return", __func__);
/* Read the resolved hostname */
- must_read(priv_fd, res, ret_len);
- return ret_len;
+ must_read(priv_fd, host, ret_len);
+ return (0);
}
/* Pass the signal through to child */
Index: usr.sbin/syslogd/syslogd.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.112
diff -u -p -u -p -r1.112 syslogd.c
--- usr.sbin/syslogd/syslogd.c 19 Aug 2014 00:24:00 -0000 1.112
+++ usr.sbin/syslogd/syslogd.c 19 Aug 2014 20:19:28 -0000
@@ -250,7 +250,7 @@ volatile sig_atomic_t WantDie;
volatile sig_atomic_t DoInit;
struct filed *cfline(char *, char *);
-void cvthname(struct sockaddr_in *, char *, size_t);
+void cvthname(struct sockaddr *, char *, size_t);
int decode(const char *, const CODE *);
void dodie(int);
void doinit(int);
@@ -587,7 +587,7 @@ main(int argc, char *argv[])
(struct sockaddr *)&frominet, &len);
if (i > 0) {
line[i] = '\0';
- cvthname(&frominet, resolve,
+ cvthname((struct sockaddr *)&frominet, resolve,
sizeof resolve);
dprintf("cvthname res: %s\n", resolve);
printline(resolve, line);
@@ -1096,38 +1096,20 @@ reapchild(int signo)
* Return a printable representation of a host address.
*/
void
-cvthname(struct sockaddr_in *f, char *result, size_t res_len)
+cvthname(struct sockaddr *f, char *result, size_t res_len)
{
- sigset_t omask, nmask;
- char *p, *ip;
- int ret_len;
-
- if (f->sin_family != AF_INET) {
+ if (getnameinfo(f, f->sa_len, result, res_len, NULL, 0,
+ NI_NUMERICHOST|NI_NUMERICSERV|NI_DGRAM) != 0) {
dprintf("Malformed from address\n");
strlcpy(result, "???", res_len);
return;
}
-
- ip = inet_ntoa(f->sin_addr);
- dprintf("cvthname(%s)\n", ip);
- if (NoDNS) {
- strlcpy(result, ip, res_len);
+ dprintf("cvthname(%s)\n", result);
+ if (NoDNS)
return;
- }
- sigemptyset(&nmask);
- sigaddset(&nmask, SIGHUP);
- sigprocmask(SIG_BLOCK, &nmask, &omask);
-
- ret_len = priv_gethostbyaddr((char *)&f->sin_addr,
- sizeof(struct in_addr), f->sin_family, result, res_len);
-
- sigprocmask(SIG_SETMASK, &omask, NULL);
- if (ret_len == 0) {
- dprintf("Host name for your address (%s) unknown\n", ip);
- strlcpy(result, ip, res_len);
- } else if ((p = strchr(result, '.')) && strcmp(p + 1, LocalDomain) == 0)
- *p = '\0';
+ if (priv_getnameinfo(f, f->sa_len, result, res_len) != 0)
+ dprintf("Host name for your address (%s) unknown\n", result);
}
void
Index: usr.sbin/syslogd/syslogd.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.h,v
retrieving revision 1.8
diff -u -p -u -p -r1.8 syslogd.h
--- usr.sbin/syslogd/syslogd.h 19 Aug 2013 06:09:23 -0000 1.8
+++ usr.sbin/syslogd/syslogd.h 19 Aug 2014 19:56:24 -0000
@@ -27,7 +27,7 @@ FILE *priv_open_config(void);
void priv_config_parse_done(void);
int priv_config_modified(void);
int priv_gethostserv(char *, char *, struct sockaddr *, size_t);
-int priv_gethostbyaddr(char *, int, int, char *, size_t);
+int priv_getnameinfo(struct sockaddr *, socklen_t, char *, size_t);
/* Terminal message */
char *ttymsg(struct iovec *, int, char *, int);
