ftp proxy host header
Hi,
Fetching port distfiles with ftp from githup does not work when
using a https proxy. The problem is that the http Host header is
not not set and githup.com needs that.
So remember the host form the url and write it into the http request.
Also write the http request into the debugging output to see what
is going on.
Note that using Proxy-Authorization together with Cookie did not
work. I have fixed the format string.
ok?
bluhm
Index: usr.bin/ftp/fetch.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.135
diff -u -p -r1.135 fetch.c
--- usr.bin/ftp/fetch.c 25 Nov 2014 08:22:09 - 1.135
+++ usr.bin/ftp/fetch.c 10 Jan 2015 01:04:03 -
@@ -188,7 +188,7 @@ url_get(const char *origline, const char
const char *errstr;
ssize_t len, wlen;
#ifndef SMALL
- char *sslpath = NULL, *sslhost = NULL;
+ char *sslpath = NULL, *sslhost = NULL, *proxyhost = NULL;
char *locbase, *full_host = NULL;
const char *scheme;
int ishttpurl = 0, ishttpsurl = 0;
@@ -300,6 +300,9 @@ noslash:
errx(1, "Can't allocate memory for https
path/host.");
}
#endif /* !SMALL */
+ proxyhost = strdup(host);
+ if (proxyhost == NULL)
+ errx(1, "Can't allocate memory for proxy host.");
proxyurl = strdup(proxyenv);
if (proxyurl == NULL)
errx(1, "Can't allocate memory for proxy URL.");
@@ -640,17 +643,18 @@ again:
fprintf(ttyout, " (via %s)\n", proxyurl);
/*
* Host: directive must use the destination host address for
-* the original URI (path). We do not attach it at this moment.
+* the original URI (path).
*/
if (credentials)
ftp_printf(fin, tls, "GET %s HTTP/1.0\r\n"
- "Proxy-Authorization: Basic %s%s\r\n%s\r\n\r\n",
- epath, credentials, buf ? buf : "",
- httpuseragent);
+ "Proxy-Authorization: Basic %s\r\n"
+ "Host: %s\r\n%s%s\r\n\r\n",
+ epath, credentials,
+ proxyhost, buf ? buf : "", httpuseragent);
else
- ftp_printf(fin, tls, "GET %s HTTP/1.0\r\n%s%s\r\n\r\n",
- epath, buf ? buf : "", httpuseragent);
-
+ ftp_printf(fin, tls, "GET %s HTTP/1.0\r\n"
+ "Host: %s\r\n%s%s\r\n\r\n",
+ epath, proxyhost, buf ? buf : "", httpuseragent);
} else {
#ifndef SMALL
if (resume) {
@@ -676,7 +680,10 @@ again:
restart_point ? "HTTP/1.1\r\nConnection: close" :
#endif /* !SMALL */
"HTTP/1.0");
- if (strchr(host, ':')) {
+ if (proxyhost) {
+ ftp_printf(fin, tls, "%s", proxyhost);
+ port = NULL;
+ } else if (strchr(host, ':')) {
/*
* strip off scoped address portion, since it's
* local to node
@@ -991,6 +998,7 @@ cleanup_url_get:
else if (s != -1)
close(s);
free(buf);
+ free(proxyhost);
free(proxyurl);
free(newline);
free(credentials);
@@ -1486,6 +1494,13 @@ ftp_printf(FILE *fp, struct tls *tls, co
ret = 0;
va_end(ap);
+#ifndef SMALL
+ if (debug) {
+ va_start(ap, fmt);
+ ret = vfprintf(ttyout, fmt, ap);
+ va_end(ap);
+ }
+#endif /* !SMALL */
return (ret);
}
openssl c_lient http proxy
Hi,
I have always missed the possiblility to use the openssl c_lient
tool with an http proxy. So I implemented a -proxy feature in the
same hackish way as -starttls.
Do we want that option?
bluhm
Index: usr.bin/openssl/s_client.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.bin/openssl/s_client.c,v
retrieving revision 1.11
diff -u -p -r1.11 s_client.c
--- usr.bin/openssl/s_client.c 14 Dec 2014 14:42:06 - 1.11
+++ usr.bin/openssl/s_client.c 9 Jan 2015 23:44:21 -
@@ -204,6 +204,7 @@ sc_usage(void)
BIO_printf(bio_err, " -host host - use -connect instead\n");
BIO_printf(bio_err, " -port port - use -connect instead\n");
BIO_printf(bio_err, " -connect host:port - who to connect to (default
is %s:%s)\n", SSL_HOST_NAME, PORT_STR);
+ BIO_printf(bio_err, " -proxy host:port - connect to http proxy\n");
BIO_printf(bio_err, " -verify arg - turn on peer certificate
verification\n");
BIO_printf(bio_err, " -cert arg - certificate file to use, PEM
format assumed\n");
@@ -338,6 +339,7 @@ s_client_main(int argc, char **argv)
char *port = PORT_STR;
int full_log = 1;
char *host = SSL_HOST_NAME;
+ char *proxy = NULL, *connect = SSL_HOST_NAME;
char *cert_file = NULL, *key_file = NULL;
int cert_format = FORMAT_PEM, key_format = FORMAT_PEM;
char *passarg = NULL, *pass = NULL;
@@ -412,7 +414,17 @@ s_client_main(int argc, char **argv)
} else if (strcmp(*argv, "-connect") == 0) {
if (--argc < 1)
goto bad;
- if (!extract_host_port(*(++argv), &host, NULL, &port))
+ connect = *(++argv);
+ if (proxy == NULL) {
+ if (!extract_host_port(connect, &host, NULL,
+ &port))
+ goto bad;
+ }
+ } else if (strcmp(*argv, "-proxy") == 0) {
+ if (--argc < 1)
+ goto bad;
+ proxy = *(++argv);
+ if (!extract_host_port(proxy, &host, NULL, &port))
goto bad;
} else if (strcmp(*argv, "-verify") == 0) {
verify = SSL_VERIFY_PEER;
@@ -971,8 +983,7 @@ re_start:
BIO_free(fbio);
BIO_printf(sbio, "AUTH TLS\r\n");
BIO_read(sbio, sbuf, BUFSIZZ);
- }
- if (starttls_proto == PROTO_XMPP) {
+ } else if (starttls_proto == PROTO_XMPP) {
int seen = 0;
BIO_printf(sbio, "
Re: Kill IPv4 list of addresses
On 6 January 2015 at 13:26, Martin Pieuchot wrote: > Diff below remove the last use of the global IPv4 list of addresses. > > The code using it is a hack to move the unique cloning route of a > subnet from one ifa to another. I know a proper fix would be to use > multipath for that, but this is not possible feasible right now > because we cannot select multipath route entries based on a different > ifa. > > In the meantime this allow us to simplify in_ifinit() which still needs > better error handling. > > ok? > Looks good to me.
Re: ftp proxy host header
On 2015/01/12 12:35, Alexander Bluhm wrote: > Hi, > > Fetching port distfiles with ftp from githup does not work when > using a https proxy. The problem is that the http Host header is > not not set and githup.com needs that. > > So remember the host form the url and write it into the http request. > Also write the http request into the debugging output to see what > is going on. > > Note that using Proxy-Authorization together with Cookie did not > work. I have fixed the format string. > > ok? Definitely! This also fixes a long-standing problem using HTTP over proxy servers that cannot reconstruct a Host: header from the supplied URL (IIRC mikeb ran into a problem with this with an old work proxy).
Re: ftp proxy host header
On Mon, Jan 12, 2015 at 1:39 PM, Stuart Henderson wrote: > On 2015/01/12 12:35, Alexander Bluhm wrote: >> Hi, >> >> Fetching port distfiles with ftp from githup does not work when >> using a https proxy. The problem is that the http Host header is >> not not set and githup.com needs that. >> >> So remember the host form the url and write it into the http request. >> Also write the http request into the debugging output to see what >> is going on. >> >> Note that using Proxy-Authorization together with Cookie did not >> work. I have fixed the format string. >> >> ok? > > Definitely! This also fixes a long-standing problem using HTTP over > proxy servers that cannot reconstruct a Host: header from the supplied > URL (IIRC mikeb ran into a problem with this with an old work proxy). > for example when using cntlm (I suspect...) Thanks! David
Re: ftp proxy host header
Stuart Henderson writes: > On 2015/01/12 12:35, Alexander Bluhm wrote: >> Hi, >> >> Fetching port distfiles with ftp from githup does not work when >> using a https proxy. The problem is that the http Host header is >> not not set and githup.com needs that. >> >> So remember the host form the url and write it into the http request. >> Also write the http request into the debugging output to see what >> is going on. >> >> Note that using Proxy-Authorization together with Cookie did not >> work. I have fixed the format string. Heh, nice catch. >> ok? > > Definitely! This also fixes a long-standing problem using HTTP over > proxy servers that cannot reconstruct a Host: header from the supplied > URL (IIRC mikeb ran into a problem with this with an old work proxy). Note that the current diff won't build with SMALL defined. -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: ftp proxy host header
On 2015/01/12 13:51, Jérémie Courrèges-Anglas wrote: > Stuart Henderson writes: > > > On 2015/01/12 12:35, Alexander Bluhm wrote: > >> Hi, > >> > >> Fetching port distfiles with ftp from githup does not work when > >> using a https proxy. The problem is that the http Host header is > >> not not set and githup.com needs that. > >> > >> So remember the host form the url and write it into the http request. > >> Also write the http request into the debugging output to see what > >> is going on. > >> > >> Note that using Proxy-Authorization together with Cookie did not > >> work. I have fixed the format string. > > Heh, nice catch. > > >> ok? > > > > Definitely! This also fixes a long-standing problem using HTTP over > > proxy servers that cannot reconstruct a Host: header from the supplied > > URL (IIRC mikeb ran into a problem with this with an old work proxy). > > Note that the current diff won't build with SMALL defined. Good catch, definition for *proxyhost needs to go outside the #ifndef.
Re: ftp proxy host header
On Mon, Jan 12, 2015 at 01:04:14PM +, Stuart Henderson wrote:
> Good catch, definition for *proxyhost needs to go outside the #ifndef.
Thanks, new diff:
Index: usr.bin/ftp/fetch.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.135
diff -u -p -r1.135 fetch.c
--- usr.bin/ftp/fetch.c 25 Nov 2014 08:22:09 - 1.135
+++ usr.bin/ftp/fetch.c 12 Jan 2015 13:17:38 -
@@ -187,6 +187,7 @@ url_get(const char *origline, const char
off_t hashbytes;
const char *errstr;
ssize_t len, wlen;
+ char *proxyhost = NULL;
#ifndef SMALL
char *sslpath = NULL, *sslhost = NULL;
char *locbase, *full_host = NULL;
@@ -300,6 +301,9 @@ noslash:
errx(1, "Can't allocate memory for https
path/host.");
}
#endif /* !SMALL */
+ proxyhost = strdup(host);
+ if (proxyhost == NULL)
+ errx(1, "Can't allocate memory for proxy host.");
proxyurl = strdup(proxyenv);
if (proxyurl == NULL)
errx(1, "Can't allocate memory for proxy URL.");
@@ -640,17 +644,18 @@ again:
fprintf(ttyout, " (via %s)\n", proxyurl);
/*
* Host: directive must use the destination host address for
-* the original URI (path). We do not attach it at this moment.
+* the original URI (path).
*/
if (credentials)
ftp_printf(fin, tls, "GET %s HTTP/1.0\r\n"
- "Proxy-Authorization: Basic %s%s\r\n%s\r\n\r\n",
- epath, credentials, buf ? buf : "",
- httpuseragent);
+ "Proxy-Authorization: Basic %s\r\n"
+ "Host: %s\r\n%s%s\r\n\r\n",
+ epath, credentials,
+ proxyhost, buf ? buf : "", httpuseragent);
else
- ftp_printf(fin, tls, "GET %s HTTP/1.0\r\n%s%s\r\n\r\n",
- epath, buf ? buf : "", httpuseragent);
-
+ ftp_printf(fin, tls, "GET %s HTTP/1.0\r\n"
+ "Host: %s\r\n%s%s\r\n\r\n",
+ epath, proxyhost, buf ? buf : "", httpuseragent);
} else {
#ifndef SMALL
if (resume) {
@@ -676,7 +681,10 @@ again:
restart_point ? "HTTP/1.1\r\nConnection: close" :
#endif /* !SMALL */
"HTTP/1.0");
- if (strchr(host, ':')) {
+ if (proxyhost) {
+ ftp_printf(fin, tls, "%s", proxyhost);
+ port = NULL;
+ } else if (strchr(host, ':')) {
/*
* strip off scoped address portion, since it's
* local to node
@@ -991,6 +999,7 @@ cleanup_url_get:
else if (s != -1)
close(s);
free(buf);
+ free(proxyhost);
free(proxyurl);
free(newline);
free(credentials);
@@ -1486,6 +1495,13 @@ ftp_printf(FILE *fp, struct tls *tls, co
ret = 0;
va_end(ap);
+#ifndef SMALL
+ if (debug) {
+ va_start(ap, fmt);
+ ret = vfprintf(ttyout, fmt, ap);
+ va_end(ap);
+ }
+#endif /* !SMALL */
return (ret);
}
Re: ftp proxy host header
On 12/01/15(Mon) 14:22, Alexander Bluhm wrote:
> On Mon, Jan 12, 2015 at 01:04:14PM +, Stuart Henderson wrote:
> > Good catch, definition for *proxyhost needs to go outside the #ifndef.
>
> Thanks, new diff:
Awesome, I can now run fw_update(1) behind our broken proxy. ok with
me.
>
> Index: usr.bin/ftp/fetch.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.bin/ftp/fetch.c,v
> retrieving revision 1.135
> diff -u -p -r1.135 fetch.c
> --- usr.bin/ftp/fetch.c 25 Nov 2014 08:22:09 - 1.135
> +++ usr.bin/ftp/fetch.c 12 Jan 2015 13:17:38 -
> @@ -187,6 +187,7 @@ url_get(const char *origline, const char
> off_t hashbytes;
> const char *errstr;
> ssize_t len, wlen;
> + char *proxyhost = NULL;
> #ifndef SMALL
> char *sslpath = NULL, *sslhost = NULL;
> char *locbase, *full_host = NULL;
> @@ -300,6 +301,9 @@ noslash:
> errx(1, "Can't allocate memory for https
> path/host.");
> }
> #endif /* !SMALL */
> + proxyhost = strdup(host);
> + if (proxyhost == NULL)
> + errx(1, "Can't allocate memory for proxy host.");
> proxyurl = strdup(proxyenv);
> if (proxyurl == NULL)
> errx(1, "Can't allocate memory for proxy URL.");
> @@ -640,17 +644,18 @@ again:
> fprintf(ttyout, " (via %s)\n", proxyurl);
> /*
>* Host: directive must use the destination host address for
> - * the original URI (path). We do not attach it at this moment.
> + * the original URI (path).
>*/
> if (credentials)
> ftp_printf(fin, tls, "GET %s HTTP/1.0\r\n"
> - "Proxy-Authorization: Basic %s%s\r\n%s\r\n\r\n",
> - epath, credentials, buf ? buf : "",
> - httpuseragent);
> + "Proxy-Authorization: Basic %s\r\n"
> + "Host: %s\r\n%s%s\r\n\r\n",
> + epath, credentials,
> + proxyhost, buf ? buf : "", httpuseragent);
> else
> - ftp_printf(fin, tls, "GET %s HTTP/1.0\r\n%s%s\r\n\r\n",
> - epath, buf ? buf : "", httpuseragent);
> -
> + ftp_printf(fin, tls, "GET %s HTTP/1.0\r\n"
> + "Host: %s\r\n%s%s\r\n\r\n",
> + epath, proxyhost, buf ? buf : "", httpuseragent);
> } else {
> #ifndef SMALL
> if (resume) {
> @@ -676,7 +681,10 @@ again:
> restart_point ? "HTTP/1.1\r\nConnection: close" :
> #endif /* !SMALL */
> "HTTP/1.0");
> - if (strchr(host, ':')) {
> + if (proxyhost) {
> + ftp_printf(fin, tls, "%s", proxyhost);
> + port = NULL;
> + } else if (strchr(host, ':')) {
> /*
>* strip off scoped address portion, since it's
>* local to node
> @@ -991,6 +999,7 @@ cleanup_url_get:
> else if (s != -1)
> close(s);
> free(buf);
> + free(proxyhost);
> free(proxyurl);
> free(newline);
> free(credentials);
> @@ -1486,6 +1495,13 @@ ftp_printf(FILE *fp, struct tls *tls, co
> ret = 0;
>
> va_end(ap);
> +#ifndef SMALL
> + if (debug) {
> + va_start(ap, fmt);
> + ret = vfprintf(ttyout, fmt, ap);
> + va_end(ap);
> + }
> +#endif /* !SMALL */
> return (ret);
> }
>
>
Re: better implicit warnings in kernel
On Thu, Jan 08, 2015 at 07:00:30PM -0500, Ted Unangst wrote:
> I think -Wimplicit-function-declaration is a better fit for the
> desired warning here. We don't want implicit function declarations.
> This is the same warning we recently added to userland in a few places.
>
> -Wstrict-prototypes was used in the past because I think the above
> warning wasn't available? Anyway, I don't like it, because combined
> with the no static rule, even private functions need prototypes. I
> think it's silly when I see code that does...
>
> int somefun(int); /* declaration */
> int
> somefun(int x)
> {
>/* definition two lines later */
> }
>
> If a function is defined before it's used, it shouldn't be necessary
> to provide a private declaration, too.
The current -Wstrict-prototypes enforces a consistent style at least.
So I would leave it as it is.
bluhm
Re: Sending route messages for local routes or cloning routes
On 07/01/15(Wed) 19:00, Florian Riehm wrote:
> Hi Martin,
>
> Thanks for your diff! Regardless of my problem it makes our code
> more clear. The loop in rt_newaddrmsg() was ugly.
>
> >
> > Here's a diff that should generate a RTM_ADD message for every CLONING
> > route added while keeping the existing RTM_NEWADDR/RTM_DELADDR logic.
> >
> > dhclient(8) is happy with this change, does it fix your use case too?
>
> There is a small bug in your diff of route.c, because rt_ifa_del() is
> never called with flag RTF_CLONING, so ospfd doesn't notice if an address
> gets deleted.
>
> I was thinking about this three options to fix the problem:
> 1.) Check with rt->rt_flags instead of flags for RTF_CLONING flag:
> -if (flags & (RTF_LOCAL|RTF_CLONING))
> +if (rt->rt_flags & (RTF_LOCAL|RTF_CLONING))
>
> 2) Add RTF_CLONING to flags argument at certain calls of rt_ifa_add() /
> rt_ifa_delete()
>
> 3.) Remove the check completely and call always rt_sendmsg()
>
> At the moment I would prefer the 3. solution. After rtrequest1() has been
> called, we check for error and check if rt ist not NULL. If this conditions
> are
> true we have added or deleted a route. Why should we not send a route message
> then?
> Below an updated diff for route.c with my fix.
I agree with you, we should always send RTM_{ADD,DELETE} messages.
Complete diff below, anybody wants to ok it?
Index: net/route.c
===
RCS file: /home/ncvs/src/sys/net/route.c,v
retrieving revision 1.198
diff -u -p -r1.198 route.c
--- net/route.c 8 Jan 2015 15:05:44 - 1.198
+++ net/route.c 12 Jan 2015 13:53:02 -
@@ -382,11 +382,13 @@ void
rt_sendmsg(struct rtentry *rt, int cmd, u_int rtableid)
{
struct rt_addrinfo info;
+ struct sockaddr_rtlabel sa_rl;
- bzero(&info, sizeof(info));
+ memset(&info, 0, sizeof(info));
info.rti_info[RTAX_DST] = rt_key(rt);
info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
info.rti_info[RTAX_NETMASK] = rt_mask(rt);
+ info.rti_info[RTAX_LABEL] = rtlabel_id2sa(rt->rt_labelid, &sa_rl);
if (rt->rt_ifp != NULL) {
info.rti_info[RTAX_IFP] =(struct sockaddr *)rt->rt_ifp->if_sadl;
info.rti_info[RTAX_IFA] = rt->rt_ifa->ifa_addr;
@@ -1138,7 +1140,8 @@ rt_ifa_add(struct ifaddr *ifa, int flags
* userland that a new address has been added.
*/
if (flags & RTF_LOCAL)
- rt_newaddrmsg(RTM_ADD, ifa, error, nrt);
+ rt_sendaddrmsg(nrt, RTM_NEWADDR);
+ rt_sendmsg(nrt, RTM_ADD, rtableid);
}
return (error);
}
@@ -1193,7 +1196,8 @@ rt_ifa_del(struct ifaddr *ifa, int flags
error = rtrequest1(RTM_DELETE, &info, prio, &nrt, rtableid);
if (error == 0 && (rt = nrt) != NULL) {
if (flags & RTF_LOCAL)
- rt_newaddrmsg(RTM_DELETE, ifa, error, nrt);
+ rt_sendaddrmsg(nrt, RTM_DELADDR);
+ rt_sendmsg(nrt, RTM_DELETE, rtableid);
if (rt->rt_refcnt <= 0) {
rt->rt_refcnt++;
rtfree(rt);
Index: net/route.h
===
RCS file: /home/ncvs/src/sys/net/route.h,v
retrieving revision 1.103
diff -u -p -r1.103 route.h
--- net/route.h 8 Jan 2015 15:05:44 - 1.103
+++ net/route.h 12 Jan 2015 13:53:02 -
@@ -357,9 +357,9 @@ void rt_ifannouncemsg(struct ifnet *, i
voidrt_maskedcopy(struct sockaddr *,
struct sockaddr *, struct sockaddr *);
voidrt_sendmsg(struct rtentry *, int, u_int);
+voidrt_sendaddrmsg(struct rtentry *, int);
voidrt_missmsg(int, struct rt_addrinfo *, int, struct ifnet *, int,
u_int);
-voidrt_newaddrmsg(int, struct ifaddr *, int, struct rtentry *);
int rt_setgate(struct rtentry *, struct sockaddr *,
struct sockaddr *, u_int);
int rt_checkgate(struct ifnet *, struct rtentry *, struct sockaddr *,
Index: net/rtsock.c
===
RCS file: /home/ncvs/src/sys/net/rtsock.c,v
retrieving revision 1.155
diff -u -p -r1.155 rtsock.c
--- net/rtsock.c19 Dec 2014 18:57:17 - 1.155
+++ net/rtsock.c12 Jan 2015 13:53:02 -
@@ -1137,70 +1137,36 @@ rt_ifmsg(struct ifnet *ifp)
* copies of it.
*/
void
-rt_newaddrmsg(int cmd, struct ifaddr *ifa, int error, struct rtentry *rt)
+rt_sendaddrmsg(struct rtentry *rt, int cmd)
{
- struct rt_addrinfo info;
- struct sockaddr *sa = NULL;
- int pass;
- struct mbuf *m = NULL;
+ struct ifaddr *ifa = rt->rt_ifa;
struct ifnet*ifp = ifa->ifa_ifp;
+ struct mbuf *m = NULL;
+ struct rt_addrinfo info;
+ struct ifa_msghdr *ifam;
if
Re: operations on nd_prefix list must take rdomain into account
On 26/11/14(Wed) 18:24, Mike Belopuhov wrote:
> More rdomain checks are needed to be able to use the same subnet
> in a back to back connection between IPv6 rdomains as pointed out
> by mpi@.
>
> OK?
ok mpi@
>
> diff --git sys/netinet6/nd6.c sys/netinet6/nd6.c
> index 9616187..d704cd6 100644
> --- sys/netinet6/nd6.c
> +++ sys/netinet6/nd6.c
> @@ -1264,10 +1264,13 @@ nd6_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp)
> s = splsoftnet();
> /* First purge the addresses referenced by a prefix. */
> LIST_FOREACH_SAFE(pr, &nd_prefix, ndpr_entry, npr) {
> struct in6_ifaddr *ia6, *ia6_next;
>
> + if (pr->ndpr_ifp->if_rdomain != ifp->if_rdomain)
> + continue;
> +
> if (IN6_IS_ADDR_LINKLOCAL(&pr->ndpr_prefix.sin6_addr))
> continue; /* XXX */
>
> /* do we really have to remove addresses as well? */
> TAILQ_FOREACH_SAFE(ia6, &in6_ifaddr, ia_list, ia6_next)
> {
> @@ -1282,10 +1285,13 @@ nd6_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp)
>* Purging the addresses might remove the prefix as well.
>* So run the loop again to access only prefixes that have
>* not been freed already.
>*/
> LIST_FOREACH_SAFE(pr, &nd_prefix, ndpr_entry, npr) {
> + if (pr->ndpr_ifp->if_rdomain != ifp->if_rdomain)
> + continue;
> +
> if (IN6_IS_ADDR_LINKLOCAL(&pr->ndpr_prefix.sin6_addr))
> continue; /* XXX */
>
> prelist_remove(pr);
> }
> diff --git sys/netinet6/nd6_rtr.c sys/netinet6/nd6_rtr.c
> index bfc9c9f..e46b3b4 100644
> --- sys/netinet6/nd6_rtr.c
> +++ sys/netinet6/nd6_rtr.c
> @@ -1690,10 +1690,13 @@ nd6_prefix_onlink(struct nd_prefix *pr)
>* interface, and the prefix has already installed the interface route.
>* Although such a configuration is expected to be rare, we explicitly
>* allow it.
>*/
> LIST_FOREACH(opr, &nd_prefix, ndpr_entry) {
> + if (opr->ndpr_ifp->if_rdomain != ifp->if_rdomain)
> + continue;
> +
> if (opr == pr)
> continue;
>
> if ((opr->ndpr_stateflags & NDPRF_ONLINK) == 0)
> continue;
> @@ -1826,10 +1829,13 @@ nd6_prefix_offlink(struct nd_prefix *pr)
>* the interface route (see comments in nd6_prefix_onlink).
>* If there's one, try to make the prefix on-link on the
>* interface.
>*/
> LIST_FOREACH(opr, &nd_prefix, ndpr_entry) {
> + if (opr->ndpr_ifp->if_rdomain != ifp->if_rdomain)
> + continue;
> +
> if (opr == pr)
> continue;
>
> if ((opr->ndpr_stateflags & NDPRF_ONLINK) != 0)
> continue;
>
httpd with fcgi send garbage after non-chunked response
Hi,
In the case where httpds fcgi module handles the end marker, it should
abort if fcgi_chunked is not true. Now it sends 8 bytes of garbage after
each request (it's often NUL terminated so it doesn't seem to show up in
browsers). This patched fixed it for me.
Index: usr.sbin/httpd/server_fcgi.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
retrieving revision 1.44
diff -u -p -u -4 -r1.44 server_fcgi.c
--- usr.sbin/httpd/server_fcgi.c4 Jan 2015 22:23:58 - 1.44
+++ usr.sbin/httpd/server_fcgi.c12 Jan 2015 14:42:27 -
@@ -674,9 +674,9 @@ server_fcgi_writechunk(struct client *cl
} else
len = EVBUFFER_LENGTH(evb);
/* If len is 0, make sure to write the end marker only once */
- if (len == 0 && clt->clt_fcgi_end++)
+ if (len == 0 && (!clt->clt_fcgi_chunked || clt->clt_fcgi_end++))
return (0);
if (clt->clt_fcgi_chunked) {
if (server_bufferevent_printf(clt, "%zx\r\n", len) == -1 ||
Re: ntpd: prefer sizeof(thing) to sizeof(struct type)
On Fri, Jan 09, 2015 at 05:45:17PM -0500, Ted Unangst wrote:
> On Fri, Jan 09, 2015 at 15:45, Brent Cook wrote:
> > From: Brent Cook
> >
> > Yeah yeah, a pointer is a pointer (except when it isn't :). I think this
> > looks nicer, since idx2peer is really the thing we're allocating to.
>
> what about the bzero ten lines later?
>
> I think the sizeof(*idx2peer) idiom is also a little nicer. and you
> can change the other reallocarray too.
>
Good point, might as well fix them all.
This updates all of the applicable sizeof(struct blah) references to use
sizeof(thing). No binary change on 32 or 64-bit.
Index: client.c
===
RCS file: /cvs/src/usr.sbin/ntpd/client.c,v
retrieving revision 1.97
diff -u -p -u -p -r1.97 client.c
--- client.c9 Jan 2015 23:44:07 - 1.97
+++ client.c12 Jan 2015 14:56:01 -
@@ -49,7 +49,7 @@ set_deadline(struct ntp_peer *p, time_t
int
client_peer_init(struct ntp_peer *p)
{
- if ((p->query = calloc(1, sizeof(struct ntp_query))) == NULL)
+ if ((p->query = calloc(1, sizeof(*(p->query == NULL)
fatal("client_peer_init calloc");
p->query->fd = -1;
p->query->msg.status = MODE_CLIENT | (NTP_VERSION << 3);
Index: config.c
===
RCS file: /cvs/src/usr.sbin/ntpd/config.c,v
retrieving revision 1.22
diff -u -p -u -p -r1.22 config.c
--- config.c10 Jan 2015 13:47:05 - 1.22
+++ config.c12 Jan 2015 14:56:01 -
@@ -42,7 +42,7 @@ host(const char *s, struct ntp_addr **hn
struct ntp_addr *h = NULL;
if (!strcmp(s, "*"))
- if ((h = calloc(1, sizeof(struct ntp_addr))) == NULL)
+ if ((h = calloc(1, sizeof(*h))) == NULL)
fatal(NULL);
/* IPv4 address? */
@@ -66,14 +66,14 @@ host_v4(const char *s)
struct sockaddr_in *sa_in;
struct ntp_addr *h;
- bzero(&ina, sizeof(struct in_addr));
+ bzero(&ina, sizeof(ina));
if (inet_pton(AF_INET, s, &ina) != 1)
return (NULL);
- if ((h = calloc(1, sizeof(struct ntp_addr))) == NULL)
+ if ((h = calloc(1, sizeof(*h))) == NULL)
fatal(NULL);
sa_in = (struct sockaddr_in *)&h->ss;
- sa_in->sin_len = sizeof(struct sockaddr_in);
+ sa_in->sin_len = sizeof(*sa_in);
sa_in->sin_family = AF_INET;
sa_in->sin_addr.s_addr = ina.s_addr;
@@ -92,10 +92,10 @@ host_v6(const char *s)
hints.ai_socktype = SOCK_DGRAM; /*dummy*/
hints.ai_flags = AI_NUMERICHOST;
if (getaddrinfo(s, "0", &hints, &res) == 0) {
- if ((h = calloc(1, sizeof(struct ntp_addr))) == NULL)
+ if ((h = calloc(1, sizeof(*h))) == NULL)
fatal(NULL);
sa_in6 = (struct sockaddr_in6 *)&h->ss;
- sa_in6->sin6_len = sizeof(struct sockaddr_in6);
+ sa_in6->sin6_len = sizeof(*sa_in6);
sa_in6->sin6_family = AF_INET6;
memcpy(&sa_in6->sin6_addr,
&((struct sockaddr_in6 *)res->ai_addr)->sin6_addr,
@@ -134,17 +134,17 @@ host_dns(const char *s, struct ntp_addr
if (res->ai_family != AF_INET &&
res->ai_family != AF_INET6)
continue;
- if ((h = calloc(1, sizeof(struct ntp_addr))) == NULL)
+ if ((h = calloc(1, sizeof(*h))) == NULL)
fatal(NULL);
h->ss.ss_family = res->ai_family;
if (res->ai_family == AF_INET) {
sa_in = (struct sockaddr_in *)&h->ss;
- sa_in->sin_len = sizeof(struct sockaddr_in);
+ sa_in->sin_len = sizeof(*sa_in);
sa_in->sin_addr.s_addr = ((struct sockaddr_in *)
res->ai_addr)->sin_addr.s_addr;
} else {
sa_in6 = (struct sockaddr_in6 *)&h->ss;
- sa_in6->sin6_len = sizeof(struct sockaddr_in6);
+ sa_in6->sin6_len = sizeof(*sa_in6);
memcpy(&sa_in6->sin6_addr, &((struct sockaddr_in6 *)
res->ai_addr)->sin6_addr, sizeof(struct in6_addr));
}
@@ -164,7 +164,7 @@ new_peer(void)
{
struct ntp_peer *p;
- if ((p = calloc(1, sizeof(struct ntp_peer))) == NULL)
+ if ((p = calloc(1, sizeof(*p))) == NULL)
fatal("new_peer calloc");
p->id = ++maxid;
@@ -176,7 +176,7 @@ new_sensor(char *device)
{
struct ntp_conf_sensor *s;
- if ((s = calloc(1, sizeof(struct ntp_conf_sensor))) == NULL)
+ if ((s = calloc(1, sizeof(*s))) == NULL)
fatal("new_sensor calloc");
if ((s->device = strdup(device)) == NULL)
fatal("new_sensor strdup");
Index: control.c
==
Re: ntpd: fix some memory leaks in dns handling
Fri, Jan 09, 2015 at 03:32:37PM -0700, Todd C. Miller wrote:
> On Fri, 09 Jan 2015 15:45:51 -0600, Brent Cook wrote:
>
> > - If imsg_add fails, it frees buf. But, so does imsg_close. Punt for
> >now and just die if imsg_add fails (maybe this should be handled more
> >nicely?)
>
> I think it makes sense to treat imsg_add() failure the same as
> imsg_create returning NULL. Perhaps if imsg_add() fails you can
> set buf to NULL since it has been freed?
Thanks, how about this?
- Nothing seems to free the result of host_dns(), so add
host_dns_free() and call after each query.
- If imsg_add fails, it frees buf. Avoid dereferencing the freed buf
afterward in imsg_close().
Granted, imsg_add _shouldn't_ fail today because we preallocated the
whole buf in imsg_create anyway, thus it doesn't have to realloc. But I
guess that's not a guarantee for tomorrow.
---
src/usr.sbin/ntpd/config.c | 11 +++
src/usr.sbin/ntpd/ntp_dns.c | 21 ++---
src/usr.sbin/ntpd/ntpd.c| 21 ++---
src/usr.sbin/ntpd/ntpd.h| 1 +
4 files changed, 40 insertions(+), 14 deletions(-)
diff --git a/src/usr.sbin/ntpd/config.c b/src/usr.sbin/ntpd/config.c
index 31d3a5c..51e4ccc 100644
--- a/src/usr.sbin/ntpd/config.c
+++ b/src/usr.sbin/ntpd/config.c
@@ -109,6 +109,17 @@ host_v6(const char *s)
return (h);
}
+void
+host_dns_free(struct ntp_addr *hn)
+{
+ struct ntp_addr *h = hn, *tmp;
+ while (h) {
+ tmp = h;
+ h = h->next;
+ free(tmp);
+ }
+}
+
int
host_dns(const char *s, struct ntp_addr **hn)
{
diff --git a/src/usr.sbin/ntpd/ntp_dns.c b/src/usr.sbin/ntpd/ntp_dns.c
index f670e90..b8f3b98 100644
--- a/src/usr.sbin/ntpd/ntp_dns.c
+++ b/src/usr.sbin/ntpd/ntp_dns.c
@@ -159,13 +159,20 @@ dns_dispatch_imsg(void)
buf = imsg_create(ibuf_dns, IMSG_HOST_DNS,
imsg.hdr.peerid, 0,
cnt * sizeof(struct sockaddr_storage));
- if (buf == NULL)
- break;
- if (cnt > 0)
- for (h = hn; h != NULL; h = h->next)
- imsg_add(buf, &h->ss, sizeof(h->ss));
-
- imsg_close(ibuf_dns, buf);
+ if (cnt > 0) {
+ if (buf) {
+ for (h = hn; h != NULL; h = h->next)
+ if (imsg_add(buf, &h->ss,
+ sizeof(h->ss)) == -1) {
+ buf = NULL;
+ break;
+ }
+ if (buf)
+ imsg_close(ibuf_dns, buf);
+ }
+ host_dns_free(hn);
+ hn = NULL;
+ }
break;
default:
break;
diff --git a/src/usr.sbin/ntpd/ntpd.c b/src/usr.sbin/ntpd/ntpd.c
index edcf5b2..9943a88 100644
--- a/src/usr.sbin/ntpd/ntpd.c
+++ b/src/usr.sbin/ntpd/ntpd.c
@@ -358,13 +358,20 @@ dispatch_imsg(struct ntpd_conf *lconf)
buf = imsg_create(ibuf, IMSG_HOST_DNS,
imsg.hdr.peerid, 0,
cnt * sizeof(struct sockaddr_storage));
- if (buf == NULL)
- break;
- if (cnt > 0)
- for (h = hn; h != NULL; h = h->next)
- imsg_add(buf, &h->ss, sizeof(h->ss));
-
- imsg_close(ibuf, buf);
+ if (cnt > 0) {
+ if (buf) {
+ for (h = hn; h != NULL; h = h->next)
+ if (imsg_add(buf, &h->ss,
+ sizeof(h->ss)) == -1) {
+ buf = NULL;
+ break;
+ }
+ if (buf)
+ imsg_close(ibuf, buf);
+ }
+ host_dns_free(hn);
+ hn = NULL;
+ }
break;
default:
break;
diff --git a/src/usr.sbin/ntpd/ntpd.h b/src/usr.sbin/ntpd/ntpd.h
index 6e12bed..1040d15 100644
--- a/src/usr.sbin/ntpd/ntpd.h
+++ b/src/usr.sbin/ntpd/ntpd.h
@@ -281,6 +281,7 @@ int parse_config(const char *, struct ntpd_conf *);
/* config.c */
Re: Sending route messages for local routes or cloning routes
On Mon, Jan 12, 2015 at 03:00:41PM +0100, Martin Pieuchot wrote:
> @@ -1138,7 +1140,8 @@ rt_ifa_add(struct ifaddr *ifa, int flags
>* userland that a new address has been added.
>*/
> if (flags & RTF_LOCAL)
> - rt_newaddrmsg(RTM_ADD, ifa, error, nrt);
> + rt_sendaddrmsg(nrt, RTM_NEWADDR);
> + rt_sendmsg(nrt, RTM_ADD, rtableid);
> }
> return (error);
> }
> @@ -1193,7 +1196,8 @@ rt_ifa_del(struct ifaddr *ifa, int flags
> error = rtrequest1(RTM_DELETE, &info, prio, &nrt, rtableid);
> if (error == 0 && (rt = nrt) != NULL) {
> if (flags & RTF_LOCAL)
> - rt_newaddrmsg(RTM_DELETE, ifa, error, nrt);
> + rt_sendaddrmsg(nrt, RTM_DELADDR);
> + rt_sendmsg(nrt, RTM_DELETE, rtableid);
> if (rt->rt_refcnt <= 0) {
> rt->rt_refcnt++;
> rtfree(rt);
The purpose of the pass 1,2 loop was to reverse the order of the
address and route message in the delete case. Although I don't
know wether the order is important to anybody, I would not change
the behavior.
Can we change the second chunk to this?
rt_sendmsg(nrt, RTM_DELETE, rtableid);
if (flags & RTF_LOCAL)
rt_sendaddrmsg(nrt, RTM_DELADDR);
otherwise OK bluhm@
Re: ntpd: fix some memory leaks in dns handling
On Mon, 12 Jan 2015 09:12:02 -0600, Brent Cook wrote: > - Nothing seems to free the result of host_dns(), so add >host_dns_free() and call after each query. > - If imsg_add fails, it frees buf. Avoid dereferencing the freed buf >afterward in imsg_close(). That looks good to me. - todd
Re: ntpd: prefer sizeof(thing) to sizeof(struct type)
> - if ((p->query = calloc(1, sizeof(struct ntp_query))) == NULL) > + if ((p->query = calloc(1, sizeof(*(p->query == NULL) I do not think the replacement pattern is better in any way.
Re: ntpd: prefer sizeof(thing) to sizeof(struct type)
On Mon, Jan 12, 2015 at 09:02:50AM -0600, Brent Cook wrote:
> On Fri, Jan 09, 2015 at 05:45:17PM -0500, Ted Unangst wrote:
> > On Fri, Jan 09, 2015 at 15:45, Brent Cook wrote:
> > > From: Brent Cook
> > >
> > > Yeah yeah, a pointer is a pointer (except when it isn't :). I think this
> > > looks nicer, since idx2peer is really the thing we're allocating to.
> >
> > what about the bzero ten lines later?
> >
> > I think the sizeof(*idx2peer) idiom is also a little nicer. and you
> > can change the other reallocarray too.
> >
>
> Good point, might as well fix them all.
>
> This updates all of the applicable sizeof(struct blah) references to use
> sizeof(thing). No binary change on 32 or 64-bit.
>
> Index: client.c
> ===
> RCS file: /cvs/src/usr.sbin/ntpd/client.c,v
> retrieving revision 1.97
> diff -u -p -u -p -r1.97 client.c
> --- client.c 9 Jan 2015 23:44:07 - 1.97
> +++ client.c 12 Jan 2015 14:56:01 -
> @@ -49,7 +49,7 @@ set_deadline(struct ntp_peer *p, time_t
> int
> client_peer_init(struct ntp_peer *p)
> {
> - if ((p->query = calloc(1, sizeof(struct ntp_query))) == NULL)
> + if ((p->query = calloc(1, sizeof(*(p->query == NULL)
> fatal("client_peer_init calloc");
> p->query->fd = -1;
> p->query->msg.status = MODE_CLIENT | (NTP_VERSION << 3);
> Index: config.c
> ===
> RCS file: /cvs/src/usr.sbin/ntpd/config.c,v
> retrieving revision 1.22
> diff -u -p -u -p -r1.22 config.c
> --- config.c 10 Jan 2015 13:47:05 - 1.22
> +++ config.c 12 Jan 2015 14:56:01 -
> @@ -42,7 +42,7 @@ host(const char *s, struct ntp_addr **hn
> struct ntp_addr *h = NULL;
>
> if (!strcmp(s, "*"))
> - if ((h = calloc(1, sizeof(struct ntp_addr))) == NULL)
> + if ((h = calloc(1, sizeof(*h))) == NULL)
> fatal(NULL);
>
What is the benefit of this mechanical change everywhere?
btw., some of these functions are also used in other daemons in our
tree that and we tried to keep the changes in sync, with minor
differences, including host_* and common code from control.c and
parse.y. I think we can sync this mechanical change (and somebody
will gain many free commits), but it also makes it harder to follow
relevant fixes.
Reyk
> /* IPv4 address? */
> @@ -66,14 +66,14 @@ host_v4(const char *s)
> struct sockaddr_in *sa_in;
> struct ntp_addr *h;
>
> - bzero(&ina, sizeof(struct in_addr));
> + bzero(&ina, sizeof(ina));
> if (inet_pton(AF_INET, s, &ina) != 1)
> return (NULL);
>
> - if ((h = calloc(1, sizeof(struct ntp_addr))) == NULL)
> + if ((h = calloc(1, sizeof(*h))) == NULL)
> fatal(NULL);
> sa_in = (struct sockaddr_in *)&h->ss;
> - sa_in->sin_len = sizeof(struct sockaddr_in);
> + sa_in->sin_len = sizeof(*sa_in);
> sa_in->sin_family = AF_INET;
> sa_in->sin_addr.s_addr = ina.s_addr;
>
> @@ -92,10 +92,10 @@ host_v6(const char *s)
> hints.ai_socktype = SOCK_DGRAM; /*dummy*/
> hints.ai_flags = AI_NUMERICHOST;
> if (getaddrinfo(s, "0", &hints, &res) == 0) {
> - if ((h = calloc(1, sizeof(struct ntp_addr))) == NULL)
> + if ((h = calloc(1, sizeof(*h))) == NULL)
> fatal(NULL);
> sa_in6 = (struct sockaddr_in6 *)&h->ss;
> - sa_in6->sin6_len = sizeof(struct sockaddr_in6);
> + sa_in6->sin6_len = sizeof(*sa_in6);
> sa_in6->sin6_family = AF_INET6;
> memcpy(&sa_in6->sin6_addr,
> &((struct sockaddr_in6 *)res->ai_addr)->sin6_addr,
> @@ -134,17 +134,17 @@ host_dns(const char *s, struct ntp_addr
> if (res->ai_family != AF_INET &&
> res->ai_family != AF_INET6)
> continue;
> - if ((h = calloc(1, sizeof(struct ntp_addr))) == NULL)
> + if ((h = calloc(1, sizeof(*h))) == NULL)
> fatal(NULL);
> h->ss.ss_family = res->ai_family;
> if (res->ai_family == AF_INET) {
> sa_in = (struct sockaddr_in *)&h->ss;
> - sa_in->sin_len = sizeof(struct sockaddr_in);
> + sa_in->sin_len = sizeof(*sa_in);
> sa_in->sin_addr.s_addr = ((struct sockaddr_in *)
> res->ai_addr)->sin_addr.s_addr;
> } else {
> sa_in6 = (struct sockaddr_in6 *)&h->ss;
> - sa_in6->sin6_len = sizeof(struct sockaddr_in6);
> + sa_in6->sin6_len = sizeof(*sa_in6);
> memcpy(&sa_in6->sin6_addr, &((struct sockaddr_in6 *)
> res->ai_addr)->sin6_addr, sizeof(struct in6_addr));
> }
> @@ -164,7 +164,7 @@ new_peer(void)
> {
> struct ntp_
Re: ntpd: prefer sizeof(thing) to sizeof(struct type)
On Mon, Jan 12, 2015 at 05:44:20PM +0100, Reyk Floeter wrote:
> On Mon, Jan 12, 2015 at 09:02:50AM -0600, Brent Cook wrote:
> > On Fri, Jan 09, 2015 at 05:45:17PM -0500, Ted Unangst wrote:
> > > On Fri, Jan 09, 2015 at 15:45, Brent Cook wrote:
> > > > From: Brent Cook
> > > >
> > > > Yeah yeah, a pointer is a pointer (except when it isn't :). I think this
> > > > looks nicer, since idx2peer is really the thing we're allocating to.
> > >
> > > what about the bzero ten lines later?
> > >
> > > I think the sizeof(*idx2peer) idiom is also a little nicer. and you
> > > can change the other reallocarray too.
> > >
> >
> > Good point, might as well fix them all.
> >
> > This updates all of the applicable sizeof(struct blah) references to use
> > sizeof(thing). No binary change on 32 or 64-bit.
> >
> > Index: client.c
> > ===
> > RCS file: /cvs/src/usr.sbin/ntpd/client.c,v
> > retrieving revision 1.97
> > diff -u -p -u -p -r1.97 client.c
> > --- client.c9 Jan 2015 23:44:07 - 1.97
> > +++ client.c12 Jan 2015 14:56:01 -
> > @@ -49,7 +49,7 @@ set_deadline(struct ntp_peer *p, time_t
> > int
> > client_peer_init(struct ntp_peer *p)
> > {
> > - if ((p->query = calloc(1, sizeof(struct ntp_query))) == NULL)
> > + if ((p->query = calloc(1, sizeof(*(p->query == NULL)
> > fatal("client_peer_init calloc");
> > p->query->fd = -1;
> > p->query->msg.status = MODE_CLIENT | (NTP_VERSION << 3);
> > Index: config.c
> > ===
> > RCS file: /cvs/src/usr.sbin/ntpd/config.c,v
> > retrieving revision 1.22
> > diff -u -p -u -p -r1.22 config.c
> > --- config.c10 Jan 2015 13:47:05 - 1.22
> > +++ config.c12 Jan 2015 14:56:01 -
> > @@ -42,7 +42,7 @@ host(const char *s, struct ntp_addr **hn
> > struct ntp_addr *h = NULL;
> >
> > if (!strcmp(s, "*"))
> > - if ((h = calloc(1, sizeof(struct ntp_addr))) == NULL)
> > + if ((h = calloc(1, sizeof(*h))) == NULL)
> > fatal(NULL);
> >
>
> What is the benefit of this mechanical change everywhere?
>
> btw., some of these functions are also used in other daemons in our
> tree that and we tried to keep the changes in sync, with minor
> differences, including host_* and common code from control.c and
> parse.y. I think we can sync this mechanical change (and somebody
> will gain many free commits), but it also makes it harder to follow
> relevant fixes.
>
> Reyk
>
It all started a minimal patch, to made sure we call reallocarray for
idx2peer with sizeof the destination type. Then I fixed pfd and the
bzero calls to match on Ted's suggestion.
Then I noticed inconsistency elsewhere and obviously got carried away.
In other projects, I've usually prefered this notation to avoid type
mismatches. But, it makes sense to avoid needless mutation, especially
since in shared and already correct code.
Would this more appropriately-scoped patch be OK?
Index: ntp.c
===
RCS file: /cvs/src/usr.sbin/ntpd/ntp.c,v
retrieving revision 1.125
diff -u -p -u -p -r1.125 ntp.c
--- ntp.c 9 Jan 2015 07:35:37 - 1.125
+++ ntp.c 12 Jan 2015 14:56:01 -
@@ -200,7 +200,7 @@ ntp_main(int pipe_prnt[2], int fd_ctl, s
while (ntp_quit == 0) {
if (peer_cnt > idx2peer_elms) {
if ((newp = reallocarray(idx2peer, peer_cnt,
- sizeof(void *))) == NULL) {
+ sizeof(*idx2peer))) == NULL) {
/* panic for now */
log_warn("could not resize idx2peer from %u -> "
"%u entries", idx2peer_elms, peer_cnt);
@@ -213,7 +213,7 @@ ntp_main(int pipe_prnt[2], int fd_ctl, s
new_cnt = PFD_MAX + peer_cnt + listener_cnt + ctl_cnt;
if (new_cnt > pfd_elms) {
if ((newp = reallocarray(pfd, new_cnt,
- sizeof(struct pollfd))) == NULL) {
+ sizeof(*pfd))) == NULL) {
/* panic for now */
log_warn("could not resize pfd from %u -> "
"%u entries", pfd_elms, new_cnt);
@@ -223,8 +223,8 @@ ntp_main(int pipe_prnt[2], int fd_ctl, s
pfd_elms = new_cnt;
}
- bzero(pfd, sizeof(struct pollfd) * pfd_elms);
- bzero(idx2peer, sizeof(void *) * idx2peer_elms);
+ bzero(pfd, sizeof(*pfd) * pfd_elms);
+ bzero(idx2peer, sizeof(*idx2peer) * idx2peer_elms);
nextaction = getmonotime() + 3600;
pfd[PFD_PIPE_MAIN].fd = ibuf_main->fd;
pfd[PFD_PIPE_MAIN].events = POLLIN;
axen(4): use %zu modifier for size_t in DPRINTF
Hi,
this fixes the build with AXEN_DEBUG defined for me.
Trailing whitespace removed while here.
Regards,
Fabian
Index: if_axen.c
===
RCS file: /cvs/src/sys/dev/usb/if_axen.c,v
retrieving revision 1.9
diff -u -p -r1.9 if_axen.c
--- if_axen.c 22 Dec 2014 02:28:52 - 1.9
+++ if_axen.c 12 Jan 2015 17:48:50 -
@@ -1032,7 +1032,7 @@ axen_rxeof(struct usbd_xfer *xfer, void
pkt_hdr = letoh32(*hdr_p);
pkt_len = (pkt_hdr >> 16) & 0x1fff;
- DPRINTFN(10,("rxeof: packet#%d, pkt_hdr 0x%08x, pkt_len %d\n",
+ DPRINTFN(10,("rxeof: packet#%d, pkt_hdr 0x%08x, pkt_len %zu\n",
pkt_count, pkt_hdr, pkt_len));
if ((pkt_hdr & AXEN_RXHDR_CRC_ERR) ||
Re: ntpd: prefer sizeof(thing) to sizeof(struct type)
On Mon, Jan 12, 2015 at 11:15, Brent Cook wrote: > > Would this more appropriately-scoped patch be OK? Looks reasonable to me.
Re: axen(4): use %zu modifier for size_t in DPRINTF
On Mon, Jan 12, 2015 at 06:56:37PM +0100, Fabian Raetz wrote:
> Hi,
>
> this fixes the build with AXEN_DEBUG defined for me.
> Trailing whitespace removed while here.
>
> Regards,
> Fabian
>
> Index: if_axen.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_axen.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 if_axen.c
> --- if_axen.c 22 Dec 2014 02:28:52 - 1.9
> +++ if_axen.c 12 Jan 2015 17:48:50 -
> @@ -1032,7 +1032,7 @@ axen_rxeof(struct usbd_xfer *xfer, void
>
> pkt_hdr = letoh32(*hdr_p);
> pkt_len = (pkt_hdr >> 16) & 0x1fff;
> - DPRINTFN(10,("rxeof: packet#%d, pkt_hdr 0x%08x, pkt_len %d\n",
> + DPRINTFN(10,("rxeof: packet#%d, pkt_hdr 0x%08x, pkt_len %zu\n",
> pkt_count, pkt_hdr, pkt_len));
>
> if ((pkt_hdr & AXEN_RXHDR_CRC_ERR) ||
Fixed. Thanks.
spamd-setup bug parsing non-CIDR IPs
Both single IP addresses and ranges suffer from an off-by one error.
The range is inclusive so the end address should not be incremented
by one. Compare how 212.174.194.30/32 is parsed vs. 212.174.194.30
or 212.174.194.30-212.174.194.30.
In cidr2range() we have:
*start = cidr.addr;
*end = cidr.addr + (1 << (32 - cidr.bits)) - 1;
so for a /32 address we get start == end which is as expected.
However, the non-CIDR code sets end = end + 1 (or start + 1 for a
single address).
This can cause extra addrs to be blacklisted and also results in a
lot of excess realloc.
Any one agree or disagree?
- todd
Index: libexec/spamd-setup/spamd-setup.c
===
RCS file: /cvs/src/libexec/spamd-setup/spamd-setup.c,v
retrieving revision 1.39
diff -u -r1.39 spamd-setup.c
--- libexec/spamd-setup/spamd-setup.c 9 Oct 2014 02:43:43 - 1.39
+++ libexec/spamd-setup/spamd-setup.c 12 Jan 2015 18:22:43 -
@@ -95,7 +95,7 @@
{
if (b == 0)
return (0);
- return (0x << (32 - b));
+ return (0xU << (32 - b));
}
u_int8_t
@@ -213,7 +213,7 @@
if (inet_net_pton(AF_INET, astring2, &end->addr,
sizeof(end->addr)) == -1)
return (0);
- end->addr = ntohl(end->addr) + 1;
+ end->addr = ntohl(end->addr);
if (start > end)
return (0);
} else if (sscanf(buf, "%15[0123456789.]", astring) == 1) {
@@ -223,7 +223,7 @@
sizeof(start->addr)) == -1)
return (0);
start->addr = ntohl(start->addr);
- end->addr = start->addr + 1;
+ end->addr = start->addr;
} else
return (0);
Re: env fix
On Sun, Jan 11, 2015 at 23:17, Jérémie Courrèges-Anglas wrote: > Philip Guenther writes: > >> On Sun, 11 Jan 2015, Ted Unangst wrote: >>> Even more awesome. >> >> How about enforcing the full rule? > > IIUC the first diff removed '/' from the characters allowed in an > environment variable, so that one can run env(1) and a program whose > name contains '='... I've never seen such a program name. > > I think this is not env(1)'s job to tell which letters can be put in > a environment variable name; other env(1) implementations don't seem to > care at all. Alas, the posix spec for env doesn't require that arguments be correctly formatted environment variables. What if we just delete the BUGS section from the man page? That's really what annoys me here. I think fixing bugs is much preferable to documenting bugs, but if it's the intended or expected behavior, then it's not a bug. Problem solved.
Re: env fix
> Alas, the posix spec for env doesn't require that arguments be > correctly formatted environment variables. > > What if we just delete the BUGS section from the man page? That's > really what annoys me here. I think fixing bugs is much preferable to > documenting bugs, but if it's the intended or expected behavior, then > it's not a bug. Problem solved. How about this? Index: env.1 === RCS file: /cvs/src/usr.bin/env/env.1,v retrieving revision 1.19 diff -u -p -u -r1.19 env.1 --- env.1 8 Mar 2014 01:42:17 - 1.19 +++ env.1 12 Jan 2015 19:13:37 - @@ -76,6 +76,12 @@ prints out the names and values of the variables in the environment, with one .Ar name Ns = Ns Ar value pair per line. +.Pp +.Nm +doesn't handle commands with equal +.Pq Sq = +signs in their +names, for obvious reasons. .Sh EXIT STATUS The .Nm @@ -119,9 +125,3 @@ specification. The historic .Fl option has been deprecated but is still supported in this implementation. -.Sh BUGS -.Nm -doesn't handle commands with equal -.Pq Sq = -signs in their -names, for obvious reasons.
Re: pax: directory traversal (from CVE request)
2015-01-12 20:48 GMT+01:00 Ted Unangst :
> On Mon, Jan 12, 2015 at 19:58, Daniel Cegiełka wrote:
>> http://www.openwall.com/lists/oss-security/2015/01/07/5
>>
>> Does someone can confirm this vulnerability? It's probably the problem
>> of "OpenBSD-derived (?) pax".
>
> The following is incomplete I think (doesn't handle arcn->ln_name),
> but seems to do the right thing with a few test archives I've created.
>
> Index: pat_rep.c
> ===
> RCS file: /cvs/src/bin/pax/pat_rep.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 pat_rep.c
> --- pat_rep.c 24 May 2014 18:51:00 - 1.34
> +++ pat_rep.c 12 Jan 2015 19:45:17 -
> @@ -632,6 +632,32 @@ mod_name(ARCHD *arcn)
> paxwarn(0, "Removing leading / from absolute path
> names in the archive");
> }
> }
> + if (rmleadslash) {
> + char *p, *prev;
> +
> + while ((p = strstr(arcn->name, "/../"))) {
> + *p = 0; /* overwrite for benefit of strrchr */
> + prev = strrchr(arcn->name, '/');
> + if (prev) {
> + memmove(prev, p + 4, strlen(p + 4) + 1);
> + arcn->nlen -= p + 4 - prev;
> + } else {
> + memmove(arcn->name, p + 4, strlen(p + 4) + 1);
> + }
> + if (rmleadslash < 3) {
> + rmleadslash = 3;
> + paxwarn(0, "Removing /../ from path names in
> the archive");
> + }
> + }
> + while (strstr(arcn->name, "../") == arcn->name) {
> + memmove(arcn->name, arcn->name + 3, strlen(arcn->name
> + 3) + 1);
> + arcn->nlen -= 3;
> + if (rmleadslash < 3) {
> + rmleadslash = 3;
> + paxwarn(0, "Removing leading .. from path
> names in the archive");
> + }
> + }
> + }
>
> /*
> * IMPORTANT: We have a problem. what do we do with symlinks?
>
>
Eh .. I should send it directly to tech, so I removed the misc-list
and addressed to the tech... and once again the link:
http://www.openwall.com/lists/oss-security/2015/01/07/5
Thanks,
Daniel
Re: env fix
On Mon, 12 Jan 2015 14:11:19 -0500, Ted Unangst wrote: > Alas, the posix spec for env doesn't require that arguments be > correctly formatted environment variables. There is no such thing as a correctly formatted environment variable. The restrictions are only for *shell* variables. The shell is not the only consumer of the environment. > What if we just delete the BUGS section from the man page? That's > really what annoys me here. I think fixing bugs is much preferable to > documenting bugs, but if it's the intended or expected behavior, then > it's not a bug. Problem solved. Fine with me; I don't consider that a real bug. - todd
Re: env fix
On Mon, 12 Jan 2015 12:14:51 -0700, Theo de Raadt wrote: > How about this? Makes sense to me. OK millert@ - todd
Only replace /obsd if /bsd changed
Hello,
/bsd currently replaces /obsd upon installation of a new kernel. That's
not a problem under normal circumstances, but if one accidentally issues
make install twice, /obsd will be the same as the new (i.e., potentially
unbootable) /bsd kernel. How about leaving /obsd alone if /bsd didn't
change?
Index: sys/arch/alpha/conf/Makefile.alpha
===
RCS file: /cvs/src/sys/arch/alpha/conf/Makefile.alpha,v
retrieving revision 1.85
diff -u -r1.85 Makefile.alpha
--- sys/arch/alpha/conf/Makefile.alpha 18 Nov 2014 01:17:36 - 1.85
+++ sys/arch/alpha/conf/Makefile.alpha 11 Jan 2015 23:49:14 -
@@ -146,8 +146,7 @@
install: install-kernel-${MACHINE_NAME}
.if !target(install-kernel-${MACHINE_NAME}})
install-kernel-${MACHINE_NAME}:
- rm -f /obsd
- ln /bsd /obsd
+ cmp -s bsd /bsd || ln -f /bsd /obsd
cp bsd /nbsd
mv /nbsd /bsd
.endif
Index: sys/arch/amd64/conf/Makefile.amd64
===
RCS file: /cvs/src/sys/arch/amd64/conf/Makefile.amd64,v
retrieving revision 1.63
diff -u -r1.63 Makefile.amd64
--- sys/arch/amd64/conf/Makefile.amd64 18 Nov 2014 01:11:13 - 1.63
+++ sys/arch/amd64/conf/Makefile.amd64 11 Jan 2015 23:49:14 -
@@ -150,8 +150,7 @@
install: install-kernel-${MACHINE_NAME}
.if !target(install-kernel-${MACHINE_NAME}})
install-kernel-${MACHINE_NAME}:
- rm -f /obsd
- ln /bsd /obsd
+ cmp -s bsd /bsd || ln -f /bsd /obsd
cp bsd /nbsd
mv /nbsd /bsd
.endif
Index: sys/arch/armish/conf/Makefile.armish
===
RCS file: /cvs/src/sys/arch/armish/conf/Makefile.armish,v
retrieving revision 1.49
diff -u -r1.49 Makefile.armish
--- sys/arch/armish/conf/Makefile.armish4 Oct 2014 18:10:14 -
1.49
+++ sys/arch/armish/conf/Makefile.armish11 Jan 2015 23:49:14 -
@@ -157,8 +157,7 @@
install: install-kernel-${MACHINE_NAME}
.if !target(install-kernel-${MACHINE_NAME}})
install-kernel-${MACHINE_NAME}:
- rm -f /obsd
- ln /bsd /obsd
+ cmp -s bsd /bsd || ln -f /bsd /obsd
cp bsd /nbsd
mv /nbsd /bsd
.endif
Index: sys/arch/armv7/conf/Makefile.armv7
===
RCS file: /cvs/src/sys/arch/armv7/conf/Makefile.armv7,v
retrieving revision 1.5
diff -u -r1.5 Makefile.armv7
--- sys/arch/armv7/conf/Makefile.armv7 4 Oct 2014 18:10:14 - 1.5
+++ sys/arch/armv7/conf/Makefile.armv7 11 Jan 2015 23:49:14 -
@@ -154,8 +154,7 @@
install: install-kernel-${MACHINE_NAME}
.if !target(install-kernel-${MACHINE_NAME}})
install-kernel-${MACHINE_NAME}:
- rm -f /obsd
- ln /bsd /obsd
+ cmp -s bsd /bsd || ln -f /bsd /obsd
cp bsd /nbsd
mv /nbsd /bsd
.endif
Index: sys/arch/aviion/conf/Makefile.aviion
===
RCS file: /cvs/src/sys/arch/aviion/conf/Makefile.aviion,v
retrieving revision 1.49
diff -u -r1.49 Makefile.aviion
--- sys/arch/aviion/conf/Makefile.aviion18 Nov 2014 01:17:36 -
1.49
+++ sys/arch/aviion/conf/Makefile.aviion11 Jan 2015 23:49:14 -
@@ -151,8 +151,7 @@
install: install-kernel-${MACHINE_NAME}
.if !target(install-kernel-${MACHINE_NAME}})
install-kernel-${MACHINE_NAME}:
- rm -f /obsd
- ln /bsd /obsd
+ cmp -s bsd /bsd || ln -f /bsd /obsd
cp bsd /nbsd
mv /nbsd /bsd
.endif
Index: sys/arch/hppa/conf/Makefile.hppa
===
RCS file: /cvs/src/sys/arch/hppa/conf/Makefile.hppa,v
retrieving revision 1.78
diff -u -r1.78 Makefile.hppa
--- sys/arch/hppa/conf/Makefile.hppa18 Nov 2014 01:17:36 - 1.78
+++ sys/arch/hppa/conf/Makefile.hppa11 Jan 2015 23:49:14 -
@@ -156,8 +156,7 @@
install: install-kernel-${MACHINE_NAME}
.if !target(install-kernel-${MACHINE_NAME}})
install-kernel-${MACHINE_NAME}:
- rm -f /obsd
- ln /bsd /obsd
+ cmp -s bsd /bsd || ln -f /bsd /obsd
cp bsd /nbsd
mv /nbsd /bsd
.endif
Index: sys/arch/hppa64/conf/Makefile.hppa64
===
RCS file: /cvs/src/sys/arch/hppa64/conf/Makefile.hppa64,v
retrieving revision 1.50
diff -u -r1.50 Makefile.hppa64
--- sys/arch/hppa64/conf/Makefile.hppa6418 Nov 2014 01:17:36 -
1.50
+++ sys/arch/hppa64/conf/Makefile.hppa6411 Jan 2015 23:49:14 -
@@ -148,8 +148,7 @@
install: install-kernel-${MACHINE_NAME}
.if !target(install-kernel-${MACHINE_NAME}})
install-kernel-${MACHINE_NAME}:
- rm -f /obsd
- ln /bsd /obsd
+ cmp -s bsd /bsd || ln -f /bsd /obsd
cp bsd /nbsd
mv /nbsd /bsd
.endif
Index: sys/arch/i386/conf/Makefile.i386
===
RCS file: /cvs/src/sys/arch/i386/c
Re: Only replace /obsd if /bsd changed
Oh so someone actually uses the install target? I've been thinking about deleting it
Re: Only replace /obsd if /bsd changed
On Mon, Jan 12, 2015 at 6:04 PM, Theo de Raadt wrote: > Oh so someone actually uses the install target? I've been thinking > about deleting it > when testing small things I sometimes use make install instead of cp ... . -- - () ascii ribbon campaign - against html e-mail /\
Re: Only replace /obsd if /bsd changed
Theo de Raadt wrote:
> Oh so someone actually uses the install target? I've been thinking
> about deleting it
First the powerdown= feature, now this. I seem to have a hidden talent
for using just the things you're ripping out. Oh well, in that case:
Index: faq/faq5.html
===
RCS file: /cvs/www/faq/faq5.html,v
retrieving revision 1.206
diff -u -r1.206 faq5.html
--- faq/faq5.html 1 Dec 2014 09:49:47 - 1.206
+++ faq/faq5.html 12 Jan 2015 23:34:05 -
@@ -662,7 +662,7 @@
# cd ../compile/GENERIC
# make clean && make
[...lots of output...]
-# make install
+# cp bsd /nbsd && mv /nbsd /bsd
Replace "i386" in the first line with your platform name.
--
Simon Nicolussi
http{s,}://{www.,}sinic.name/
Re: Only replace /obsd if /bsd changed
I would hope that in this particular case, Theo's mail is not to be
taken literally -- especially when the process can easily be made even
more bullet-proof... (and yes, I do "make install" after re-building
the kernel due to errata.)
On 13 Jan 2015 at 0:39, Simon Nicolussi wrote:
> Theo de Raadt wrote:
> > Oh so someone actually uses the install target? I've been thinking
> > about deleting it
>
> First the powerdown= feature, now this. I seem to have a hidden talent
> for using just the things you're ripping out. Oh well, in that case:
>
> Index: faq/faq5.html
> === RCS
> file: /cvs/www/faq/faq5.html,v retrieving revision 1.206 diff -u -r1.206
> faq5.html --- faq/faq5.html 1 Dec 2014 09:49:47 - 1.206 +++
> faq/faq5.html 12 Jan 2015 23:34:05 - @@ -662,7 +662,7 @@
> # cd ../compile/GENERIC
> # make clean && make
> [...lots of output...]
> -# make install
> +# cp bsd /nbsd && mv /nbsd /bsd
>
>
> Replace "i386" in the first line with your platform name.
>
> --
> Simon Nicolussi
> http{s,}://{www.,}sinic.name/
>
>
Re: Only replace /obsd if /bsd changed
On Tue, Jan 13, 2015 at 00:39, Simon Nicolussi wrote: > Theo de Raadt wrote: >> Oh so someone actually uses the install target? I've been thinking >> about deleting it > > First the powerdown= feature, now this. I seem to have a hidden talent > for using just the things you're ripping out. Oh well, in that case: I liked the first diff a lot more. I use make install. > > Index: faq/faq5.html > === > RCS file: /cvs/www/faq/faq5.html,v > retrieving revision 1.206 > diff -u -r1.206 faq5.html > --- faq/faq5.html 1 Dec 2014 09:49:47 - 1.206 > +++ faq/faq5.html 12 Jan 2015 23:34:05 - > @@ -662,7 +662,7 @@ > # cd ../compile/GENERIC > # make clean && make > [...lots of output...] > -# make install > +# cp bsd /nbsd && mv /nbsd /bsd > > > Replace "i386" in the first line with your platform name. >
Re: Only replace /obsd if /bsd changed
On Mon, Jan 12, 2015 at 07:49:32PM -0500, Ted Unangst wrote: > On Tue, Jan 13, 2015 at 00:39, Simon Nicolussi wrote: > > Theo de Raadt wrote: > >> Oh so someone actually uses the install target? I've been thinking > >> about deleting it > > > > First the powerdown= feature, now this. I seem to have a hidden talent > > for using just the things you're ripping out. Oh well, in that case: > > I liked the first diff a lot more. I use make install. Seconded. > > > > > Index: faq/faq5.html > > === > > RCS file: /cvs/www/faq/faq5.html,v > > retrieving revision 1.206 > > diff -u -r1.206 faq5.html > > --- faq/faq5.html 1 Dec 2014 09:49:47 - 1.206 > > +++ faq/faq5.html 12 Jan 2015 23:34:05 - > > @@ -662,7 +662,7 @@ > > # cd ../compile/GENERIC > > # make clean && make > > [...lots of output...] > > -# make install > > +# cp bsd /nbsd && mv /nbsd /bsd > > > > > > Replace "i386" in the first line with your platform name. > > >
mg: remove contp variable from cmode.c
Hi everyone --
In mg's cmode.c getindent() it looks like there's a contp
variable that gets initialized and we set it back and forth
between FALSE and TRUE a few times. But we never actually do
anything with it. This diff removes it.
OK?
~Brian
Index: cmode.c
===
RCS file: /cvs/src/usr.bin/mg/cmode.c,v
retrieving revision 1.11
diff -u -p -r1.11 cmode.c
--- cmode.c 2 Jan 2015 11:43:15 - 1.11
+++ cmode.c 13 Jan 2015 05:14:53 -
@@ -225,7 +225,6 @@ getindent(const struct line *lp, int *cu
int nparen = 0; /* paren count */
int obrace = 0; /* open brace count */
int cbrace = 0; /* close brace count */
- int contp = FALSE; /* Continue? */
int firstnwsp = FALSE; /* First nonspace encountered? */
int colonp = FALSE; /* Did we see a colon? */
int questionp = FALSE; /* Did we see a question mark? */
@@ -260,7 +259,6 @@ getindent(const struct line *lp, int *cu
c = lgetc(lp, co);
/* We have a non-whitespace char */
if (!firstnwsp && !isspace(c)) {
- contp = TRUE;
if (c == '#')
cppp = TRUE;
firstnwsp = TRUE;
@@ -283,7 +281,6 @@ getindent(const struct line *lp, int *cu
} else if (c == '{') {
obrace++;
firstnwsp = FALSE;
- contp = FALSE;
} else if (c == '}') {
cbrace++;
} else if (c == '?') {
@@ -292,9 +289,6 @@ getindent(const struct line *lp, int *cu
/* ignore (foo ? bar : baz) construct */
if (!questionp)
colonp = TRUE;
- } else if (c == ';') {
- if (nparen > 0)
- contp = FALSE;
} else if (c == '/') {
/* first nonwhitespace? -> indent */
if (firstnwsp) {
