Austin Group Defect Tracker

Aardvark Mark IV

Main | My View | View Issues | Change Log | Docs
  

Viewing Issue Simple Details [ Jump to Notes ] [ Issue History ] [ Print ]
ID Category Severity Type Date Submitted Last Update
0000980 [1003.1(2008)/Issue 7] Base Definitions and Headers Editorial Enhancement Request 2015-08-29 08:28 2016-06-09 15:44
Reporter EdSchouten View Status public  
Assigned To ajosey
Priority normal Resolution Rejected  
Status Closed  
Name Ed Schouten
Organization Nuxi
User Reference
Section n/a
Page Number n/a
Line Number n/a
Interp Status ---
Final Accepted Text
Summary 0000980: *at(): could we have bindat() and connectat() as well?
Description Back in 2008 we introduced the *at() filesystem operations to allow for race-free traversal of the filesystem namespace. What's interesting is that these functions now play an important role in Capsicum (https://www.cl.cam.ac.uk/research/security/capsicum/). [^]

Back when Capsicum was implemented, it was observed that we don't provide *at() calls for two rather important operations: binding and connecting to sockets. bindat() and connectat() don't exist.
Desired Action I would hereby propose that we add two new functions to <sys/socket.h>, having the following prototypes:

int bindat(int socket, int directory, const char *filename);
int connectat(int socket, int directory, const char *filename);

Assuming 'directory' is equal to AT_FDCWD, these functions should behave identically to the following:

bind(socket, (struct sockaddr_un){.sun_family = AF_UNIX, .sun_path = filename}, sizeof(struct sockaddr_un));
connect(socket, (struct sockaddr_un){.sun_family = AF_UNIX, .sun_path = filename}, sizeof(struct sockaddr_un));

If 'directory' is not equal to AT_FDCWD, it uses this directory as its working directory instead of the process-wide working directory.

As an added bonus, these functions tackle the long-standing issue that struct sockaddr_un::sun_path is of a fixed size, and typically a lot smaller than PATH_MAX.
Tags No tags attached.
Attached Files

- Relationships

-  Notes
(0002797)
eblake (manager)
2015-08-30 03:09

 If these don't already have an existing implementation, then please make sure to add a flags argument:

int bindat(int socket, int directory, const char *filename, int flags);
int connectat(int socket, int directory, const char *filename, int flags);

We've already regretted that some of the *at() functions (like mkdirat) lacked a flags argument.
(0002798)
EdSchouten (updater)
2015-08-31 11:01

 So right now there are two implementations:

FreeBSD:
https://svnweb.freebsd.org/base/head/sys/sys/socket.h?view=markup#l608 [^]

int bindat(int fd, int s, const struct sockaddr *addr, socklen_t addrlen);
int connectat(int fd, int s, const struct sockaddr *name, socklen_t namelen);

CloudABI:
https://github.com/NuxiNL/cloudlibc/blob/master/src/include/sys/socket.h#L153 [^]

int bindat(int s, int fd, const char *path);
int connectat(int s, int fd, const char *path);

FreeBSD still uses a 'struct sockaddr' instead of a pathname string. CloudABI's variant doesn't have a flags argument. That said, these functions aren't used that often yet. If the Austin Group were to standardize this function, I will see to it that both these platforms have their prototypes synced up to what's agreed upon.

Your proposal with the additional 'flags' parameter makes sense.

A further question: do we want this function to return -1 and set errno, or have it return the error code directly? My suspicion is that the former makes more sense: that's also how bind(), connect() and the other *at() calls work.
(0002799)
EdSchouten (updater)
2015-08-31 13:55

 While we're at it, maybe bindat() should also have a mode_t argument to specify the mode of the newly created socket, like openat()/mkdirat()/mkfifoat()/mknodat().
(0002800)
EdSchouten (updater)
2015-08-31 16:10

 Oh, wait. We can disregard that. There is no need to set file permissions on sockets atomically, as you'd still need to call listen() before you receive incoming connections.
(0002802)
shware_systems (reporter)
2015-08-31 21:46
edited on: 2015-08-31 22:24

 I agree these would be a useful addition, but feel as proposed they are underspecified. The FreeBSD implementation comes closer.

First off, the examples for "work like" should
strcpy(sun_path /* or derived pointer if sun_path not defined */, filename)

and correct size for generic usage is better expressed as
sizeof(struct sockaddr_un)+strlen(filename), minus, if needed, sizeof(sockaddr_un.sun_path);
with a malloc() of this size preceding.

<un.h> should define a constant, imo, e.g. SUN_PATH_SIZE, that when set to 0 indicates sockaddr_un does not include a fixed size field named sun_path, leaving it as implied to follow the sun_family field and any implementation-specific header additions.

There is no inherent limit less than PATH_MAX on AF_UNIX names. Implementations that define a default "usually adequate" size for sun_path to match historical practice, per the Application Usage section of <un.h>, should still operate properly when address_len is larger as a parameter to these interfaces.

As an implementation may map open sockets to be referenced in a directory like /dev/net/eth0/, eth1/, etc., it would be preferable to allow all socket family types, not just AF_UNIX, if filename matches the text version of an IP4 or IP6 address, or a relative path of appropriate format if passed a sockaddr_in or sockaddr_in6 instead of a filepath string. This is implied so stat() may be used by an application to discover which sockets are active without requiring an available per-process file handle for use with fstat(). The prototypes then become:

union sock_at_t {
  struct sockaddr *sockinfo;
  unsigned char *filepath;
};
// This union unifies the two implementation approaches cited

int bindat(int socket, const union sock_at_t *address,
       socklen_t address_len, int directory, int flags);
int connectat(int socket, const union sock_at_t *address,
       socklen_t address_len, int directory, int flags);

#define SOCK_AT_ISPATH // flag value to indicate path part of union should be used;
#define SOCK_AT_NOFOLLOW // flag to abort if filepath has a filename referencing a symbolic link
#define SOCK_AT_NOTRAVERSE // flag to abort if a stat of an address component indicates the target is on another system

The directory and flags arguments are last as bind() and connect() could be extended with a ... parameter to account for those fields, with appropriate AF_* constants defined indicating their presence, as an alternative. SOCK_AT_ISPATH would be implied then, rather than needing to be defined.
(0002803)
EdSchouten (updater)
2015-09-01 07:43

 I think that this is actually a large step in the wrong direction. The question is, why would you want to call the functions that you proposed above without SOCK_AT_ISPATH set? In that case you could just use the existing bind() function, which has been around for decades and available universally.

If you would then consider removing SOCK_AT_ISPATH and use that behaviour by default, you could get rid of the union sock_at_t. By doing that, you end up exactly with what eblake and I proposed.

Furthermore, I truly dislike the overloading of typing. In this case you could easily avoid it by having two separate functions. Doing this in a single function only increases the risk of calling it incorrectly.

Having the directory argument as one of the last arguments is also inconsistent with the rest of all the other *at() functions. They always take the file descriptor and the pathname string it applies to as a consecutive pair of arguments:

int linkat(--> int fd1, const char *name1 <--, --> int fd2, const char *name2 <--, int flag);
int symlinkat(const char *name1, --> int fd, const char *name2 <--);

This is exactly the reason why the bindat() that I propose has the directory descriptor as its second argument. It should not be placed before the socket, nor should it be placed after the pathname string.

Keep in mind that even though the examples that I provided use AF_UNIX explicitly, we could always phrase it in such a way that this function can be used for any address family that uses the filesystem namespace to address instances. In the case of the address families defined by POSIX, that would mean just AF_UNIX.
(0002804)
nick (manager)
2015-09-01 16:38
edited on: 2016-06-09 15:15

 Further details about the BSD implementation can be found at https://www.freebsd.org/cgi/man.cgi?query=bindat&sektion=2&apropos=0&manpath=FreeBSD+10.0-RELEASE [^]
and https://www.freebsd.org/cgi/man.cgi?query=connectat&sektion=2&manpath=FreeBSD+10.0-RELEASE [^]
(0003260)
Don Cragun (manager)
2016-06-09 15:42

 Although we agree that functions like these would be useful, we can't standardize on either interface when there are existing implementations of these function with different function prototypes and suggestions during this discussion that both should be modified to include a flags argument. We are therefore rejecting this bug as currently presented, but strongly encourage the BSD and Nuxi implementors to work together to come up with a unified solution that both groups are willing to implement as a common reference implementation (preferably using the function footprints mentioned in Note: 0002797) and file a new enhancement request with the results of that joint effort.

- Issue History
Date Modified Username Field Change
2015-08-29 08:28 EdSchouten New Issue
2015-08-29 08:28 EdSchouten Status New => Under Review
2015-08-29 08:28 EdSchouten Assigned To => ajosey
2015-08-29 08:28 EdSchouten Name => Ed Schouten
2015-08-29 08:28 EdSchouten Organization => Nuxi
2015-08-29 08:28 EdSchouten Section => n/a
2015-08-29 08:28 EdSchouten Page Number => n/a
2015-08-29 08:28 EdSchouten Line Number => n/a
2015-08-30 03:09 eblake Note Added: 0002797
2015-08-31 11:01 EdSchouten Note Added: 0002798
2015-08-31 13:55 EdSchouten Note Added: 0002799
2015-08-31 16:10 EdSchouten Note Added: 0002800
2015-08-31 21:46 shware_systems Note Added: 0002802
2015-08-31 22:24 shware_systems Note Edited: 0002802
2015-09-01 07:43 EdSchouten Note Added: 0002803
2015-09-01 16:38 nick Note Added: 0002804
2016-06-09 15:15 nick Note Edited: 0002804
2016-06-09 15:42 Don Cragun Note Added: 0003260
2016-06-09 15:44 Don Cragun Interp Status => ---
2016-06-09 15:44 Don Cragun Status Under Review => Closed
2016-06-09 15:44 Don Cragun Resolution Open => Rejected
2016-09-14 14:17 emaste Issue Monitored: emaste

Mantis 1.1.6[^]
Copyright © 2000 - 2008 Mantis Group
Powered by Mantis Bugtracker