Re: tftp-proxy(8) with nat-to

2013-12-20 Thread Florian Obser
On Fri, Dec 20, 2013 at 01:17:08PM +1000, David Gwynne wrote:
 im glad you wrote a diff rather than simply complain that nat and tftp doesnt 
 work. the moving parts generally look good to me apart from the struct 
 src_addr and getopt chunks.
 
 please use sockaddr_storage instead of sockaddr in the src_addr struct.
 
 could you split the resolution of the argument to -a out into a separate 
 function and call it after the getopt loop, and pass the value of the family 
 variable to it too?
 
 you might want to use sockaddr_storage in unprivproc_pop too.
 
 cheers,
 dlg
 

done, I also nuked this from the man page, that was a brain-fart:
+This has to be the IP to which an accompanying nat-to
+.Xr pf 4
+rule translates outgoing packets.

Additionally a pass out rule is generated, so it actually does work.

( I was wondering why the return traffic was passed and I thought it was
the nat-to rule. Turns out I had a stall pass out rule from a previous
tftp-proxy run where it created pass rules instead of rdr rules and
then crashed and thus the anchor was never cleared. Since I never
restarted the tftp client those rules still matched. )

Thanks,
Florian

diff --git tftp-proxy.8 tftp-proxy.8
index f0dc9a4..fb06ba04 100644
--- tftp-proxy.8
+++ tftp-proxy.8
@@ -34,6 +34,7 @@
 .Sh SYNOPSIS
 .Nm tftp-proxy
 .Op Fl 46dv
+.Op Fl a Ar address
 .Op Fl l Ar address
 .Op Fl p Ar port
 .Op Fl w Ar transwait
@@ -51,7 +52,7 @@ a rule with divert-reply set.
 .Pp
 The proxy inserts
 .Xr pf 4
-pass rules using the
+pass and / or rdr rules using the
 .Ar anchor
 facility to allow payload packets between the client and the server.
 Once the rules are inserted,
@@ -76,6 +77,10 @@ to use IPv4 addresses only.
 Forces
 .Nm
 to use IPv6 addresses only.
+.It Fl a Ar address
+The proxy will use this as the source address for the initial request from
+the client to the server for nat traversal.
+Instead of a pass in rule a rdr rule will be generated.
 .It Fl d
 Do not daemonize.
 If this option is specified,
diff --git tftp-proxy.c tftp-proxy.c
index bcbecd7..7a85646 100644
--- tftp-proxy.c
+++ tftp-proxy.c
@@ -141,7 +141,7 @@ __dead void
 usage(void)
 {
extern char *__progname;
-   fprintf(stderr, usage: %s [-46dv] [-l address] [-p port]
+   fprintf(stderr, usage: %s [-46dv] [-a address] [-l address] [-p port]
 [-w transwait]\n, __progname);
exit(1);
 }
@@ -179,6 +179,15 @@ struct proxy_child {
 struct proxy_child *child = NULL;
 TAILQ_HEAD(, proxy_listener) proxy_listeners;
 
+struct src_addr {
+   TAILQ_ENTRY(src_addr)   entry;
+   struct sockaddr_storage addr;
+   socklen_t   addrlen;
+};
+TAILQ_HEAD(, src_addr) src_addrs;
+
+void   source_addresses(const char*, int);
+
 int
 main(int argc, char *argv[])
 {
@@ -190,12 +199,15 @@ main(int argc, char *argv[])
struct passwd *pw;
 
char *addr = localhost;
+   char *saddr = NULL;
char *port = 6969;
int family = AF_UNSPEC;
 
int pair[2];
 
-   while ((c = getopt(argc, argv, 46dvl:p:w:)) != -1) {
+   TAILQ_INIT(src_addrs);
+
+   while ((c = getopt(argc, argv, 46a:dvl:p:w:)) != -1) {
switch (c) {
case '4':
family = AF_INET;
@@ -203,6 +215,9 @@ main(int argc, char *argv[])
case '6':
family = AF_INET6;
break;
+   case 'a':
+   saddr = optarg;
+   break;
case 'd':
verbose = debug = 1;
break;
@@ -239,6 +254,9 @@ main(int argc, char *argv[])
if (pw == NULL)
lerrx(1, no %s user, NOPRIV_USER);
 
+   if (saddr != NULL)
+   source_addresses(saddr, family);
+
switch (fork()) {
case -1:
lerr(1, fork);
@@ -312,6 +330,30 @@ main(int argc, char *argv[])
return(0);
 }
 
+void
+source_addresses(const char* name, int family)
+{
+   struct addrinfo hints, *res, *res0;
+   struct src_addr *saddr;
+   int error;
+
+   memset(hints, 0, sizeof(hints));
+   hints.ai_family = family;
+   hints.ai_socktype = SOCK_DGRAM;
+   hints.ai_flags = AI_PASSIVE;
+   error = getaddrinfo(name, 0, hints, res0);
+   if (error)
+   lerrx(1, %s:%s: %s, name, 0, gai_strerror(error));
+   for (res = res0; res != NULL; res = res-ai_next) {
+   if ((saddr = calloc(1, sizeof(struct src_addr))) == NULL)
+   lerrx(1, calloc);
+   memcpy((saddr-addr), res-ai_addr,
+   sizeof(struct sockaddr));
+   saddr-addrlen = res-ai_addrlen;
+   TAILQ_INSERT_TAIL(src_addrs, saddr, entry);
+   }
+   freeaddrinfo(res0);
+}
 
 void
 proxy_privproc(int s, struct passwd *pw)
@@ -360,7 +402,8 @@ privproc_pop(int fd, short events, void *arg)
struct addr_pair req;
struct 

Re: security(8) check maildir as well as mailbox permissions

2013-12-20 Thread Joerg Jung

Am 20.12.2013 um 08:48 schrieb David Gwynne da...@gwynne.id.au:

 On 20 Dec 2013, at 2:56 am, Alexander Hall alexan...@beard.se wrote:
 
 Henning Brauer lists-openbsdt...@bsws.de wrote:
 * Craig R. Skinner skin...@britvault.co.uk [2013-12-19 10:18]:
 On 2013-12-18 Wed 20:48 PM |, J??r??mie Courr??ges-Anglas wrote:
 skin...@britvault.co.uk (Craig R. Skinner) writes:
 On 2013-12-18 Wed 15:54 PM |, Stuart Henderson wrote:
 Check the security of /var/mail/dirs similar to
 /var/mail/boxes:
 
 
 Indeed, but security(8) really reflects things in the base OS,
 
 
 smtpd.conf(8)
  deliver to maildir path
  Mail is added to a maildir.  Its location, path, may
  contain format specifiers that are expanded before use
 
 
 Therefore: ... deliver to maildir /var/mail/%{user.username}
 Therefore?  How so?  What's the logic, here?
 THEREFORE software in base can deliver to maildir in /var/mail
 
 THEREFORE software in base can also deliver mail to
 /omgohmymail/pr0n/$uid - does that mean we check it in security?
 
 The question is rather wether Maildirs in /var/mail are a common
 enough setup to warrant a check in security.
 
 I totally agree with Henning here.
 
 That said, I ended up putting my Maildirs in /var/maildir because of this, 
 so I for one wouldn't object.
 
 i also put maildirs in /var/maildir...

Similar discussion, pops up from time to time:
http://marc.info/?l=openbsd-miscm=133422769629575w=2

Quoting sthen@ in the old thread:
/var/mail is intended for user-owned mbox files, I would think
moving your maildirs elsewhere is more sane. I tend to use /mail
for virtual user mailboxes but each to their own :)

IMHO, some standard/best practice directory for maildirs is 
missing in hier(7).

FWIIW, I put mine in /var/vmail but I would move mine to anything 
else to fulfill standard/best practices. 



Re: tftp-proxy(8) with nat-to

2013-12-20 Thread David Gwynne
you need to change len to be sizeof(saddr) instead of sizeof(struct sockaddr) 
in unprivproc_pop. likewise, the memcpy in source_addresses needs to use 
res-ai_addrlen instead of sizeof(struct sockaddr).

with those fixes its ok by me.

On 20 Dec 2013, at 9:12 pm, Florian Obser flor...@openbsd.org wrote:

 On Fri, Dec 20, 2013 at 01:17:08PM +1000, David Gwynne wrote:
 im glad you wrote a diff rather than simply complain that nat and tftp 
 doesnt work. the moving parts generally look good to me apart from the 
 struct src_addr and getopt chunks.
 
 please use sockaddr_storage instead of sockaddr in the src_addr struct.
 
 could you split the resolution of the argument to -a out into a separate 
 function and call it after the getopt loop, and pass the value of the family 
 variable to it too?
 
 you might want to use sockaddr_storage in unprivproc_pop too.
 
 cheers,
 dlg
 
 
 done, I also nuked this from the man page, that was a brain-fart:
 +This has to be the IP to which an accompanying nat-to
 +.Xr pf 4
 +rule translates outgoing packets.
 
 Additionally a pass out rule is generated, so it actually does work.
 
 ( I was wondering why the return traffic was passed and I thought it was
 the nat-to rule. Turns out I had a stall pass out rule from a previous
 tftp-proxy run where it created pass rules instead of rdr rules and
 then crashed and thus the anchor was never cleared. Since I never
 restarted the tftp client those rules still matched. )
 
 Thanks,
 Florian
 
 diff --git tftp-proxy.8 tftp-proxy.8
 index f0dc9a4..fb06ba04 100644
 --- tftp-proxy.8
 +++ tftp-proxy.8
 @@ -34,6 +34,7 @@
 .Sh SYNOPSIS
 .Nm tftp-proxy
 .Op Fl 46dv
 +.Op Fl a Ar address
 .Op Fl l Ar address
 .Op Fl p Ar port
 .Op Fl w Ar transwait
 @@ -51,7 +52,7 @@ a rule with divert-reply set.
 .Pp
 The proxy inserts
 .Xr pf 4
 -pass rules using the
 +pass and / or rdr rules using the
 .Ar anchor
 facility to allow payload packets between the client and the server.
 Once the rules are inserted,
 @@ -76,6 +77,10 @@ to use IPv4 addresses only.
 Forces
 .Nm
 to use IPv6 addresses only.
 +.It Fl a Ar address
 +The proxy will use this as the source address for the initial request from
 +the client to the server for nat traversal.
 +Instead of a pass in rule a rdr rule will be generated.
 .It Fl d
 Do not daemonize.
 If this option is specified,
 diff --git tftp-proxy.c tftp-proxy.c
 index bcbecd7..7a85646 100644
 --- tftp-proxy.c
 +++ tftp-proxy.c
 @@ -141,7 +141,7 @@ __dead void
 usage(void)
 {
   extern char *__progname;
 - fprintf(stderr, usage: %s [-46dv] [-l address] [-p port]
 + fprintf(stderr, usage: %s [-46dv] [-a address] [-l address] [-p port]
[-w transwait]\n, __progname);
   exit(1);
 }
 @@ -179,6 +179,15 @@ struct proxy_child {
 struct proxy_child *child = NULL;
 TAILQ_HEAD(, proxy_listener) proxy_listeners;
 
 +struct src_addr {
 + TAILQ_ENTRY(src_addr)   entry;
 + struct sockaddr_storage addr;
 + socklen_t   addrlen;
 +};
 +TAILQ_HEAD(, src_addr) src_addrs;
 +
 +void source_addresses(const char*, int);
 +
 int
 main(int argc, char *argv[])
 {
 @@ -190,12 +199,15 @@ main(int argc, char *argv[])
   struct passwd *pw;
 
   char *addr = localhost;
 + char *saddr = NULL;
   char *port = 6969;
   int family = AF_UNSPEC;
 
   int pair[2];
 
 - while ((c = getopt(argc, argv, 46dvl:p:w:)) != -1) {
 + TAILQ_INIT(src_addrs);
 +
 + while ((c = getopt(argc, argv, 46a:dvl:p:w:)) != -1) {
   switch (c) {
   case '4':
   family = AF_INET;
 @@ -203,6 +215,9 @@ main(int argc, char *argv[])
   case '6':
   family = AF_INET6;
   break;
 + case 'a':
 + saddr = optarg;
 + break;
   case 'd':
   verbose = debug = 1;
   break;
 @@ -239,6 +254,9 @@ main(int argc, char *argv[])
   if (pw == NULL)
   lerrx(1, no %s user, NOPRIV_USER);
 
 + if (saddr != NULL)
 + source_addresses(saddr, family);
 +
   switch (fork()) {
   case -1:
   lerr(1, fork);
 @@ -312,6 +330,30 @@ main(int argc, char *argv[])
   return(0);
 }
 
 +void
 +source_addresses(const char* name, int family)
 +{
 + struct addrinfo hints, *res, *res0;
 + struct src_addr *saddr;
 + int error;
 +
 + memset(hints, 0, sizeof(hints));
 + hints.ai_family = family;
 + hints.ai_socktype = SOCK_DGRAM;
 + hints.ai_flags = AI_PASSIVE;
 + error = getaddrinfo(name, 0, hints, res0);
 + if (error)
 + lerrx(1, %s:%s: %s, name, 0, gai_strerror(error));
 + for (res = res0; res != NULL; res = res-ai_next) {
 + if ((saddr = calloc(1, sizeof(struct src_addr))) == NULL)
 + lerrx(1, calloc);
 + memcpy((saddr-addr), res-ai_addr,
 + sizeof(struct sockaddr));
 + 

Re: PATCH: Allow shared semaphores to be really shared

2013-12-20 Thread Ted Unangst
On Sat, Dec 07, 2013 at 14:11, Vadim Zhukov wrote:
 This patch fixes problems in KDE4, that relies on sharing (process-shared)
 semaphores via mmap(). This feature is used in the wild, so if we claim
 that we support process-shared semaphores, we have to implement it, too.

Haven't forgotten about this.

 Dirty games with lock member required to avoid exposing extra structure
 (struct _spinlock) in public header. On the bright side, this diff removes
 more stuff than it adds, so it have to be a good diff. :)

This is really the troubling part for me. In particular, our spinlock
implementation isn't all that great. I was poking at it earlier this
year, but didn't make any progress except to convince myself more work
needs to be done. Exposing spinlock internals makes experimentation in
this area more difficult.

I realized my previous replies had only vague promises of pain and
misery resulting from changing sem_t :), but I've been thinking about
it a bit more and wanted to make clear that there's a concrete reason
behind my concern. Unfortunately, there's no timeframe for when I
expect this to happen, other than I consider spinlock/mutex
performance a serious issue.

So yes, I'm stalling/slacking, but I hope you'll find the patience to
wait it out. And if not, let's talk.