Date: Tue, 14 Jul 2009 06:40:50 GMT From: Jonathan Anderson <jona@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 166062 for review Message-ID: <200907140640.n6E6eotr033203@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=166062 Change 166062 by jona@jona-trustedbsd-belle-vmware on 2009/07/14 06:40:41 Much more sensible error handling for libuserangel Affected files ... .. //depot/projects/trustedbsd/capabilities/src/lib/libuserangel/libuserangel.c#8 edit .. //depot/projects/trustedbsd/capabilities/src/lib/libuserangel/libuserangel.h#10 edit Differences ... ==== //depot/projects/trustedbsd/capabilities/src/lib/libuserangel/libuserangel.c#8 (text+ko) ==== @@ -39,6 +39,7 @@ #include <errno.h> #include <fcntl.h> +#include <stdarg.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -57,18 +58,14 @@ const char *VERSION = "UA/0.1"; const char *ua_protocol_version() { return VERSION; } -char errmsg[256]; -const char* ua_protocol_error(void) { return errmsg; } - int ua_find(void) { char *homedir = getenv("HOME"); if(strlen(homedir) > 200) { - sprintf(errmsg, "Obscenely long $HOME variable (%i chars)", - strlen(homedir)); + errno = ENAMETOOLONG; return -1; } @@ -81,27 +78,20 @@ strcpy(addr.sun_path, control_socket_name); angel = socket(AF_UNIX, SOCK_STREAM, 0); - if(connect(angel, (struct sockaddr*) &addr, sizeof(addr))) - { - sprintf(errmsg, "Error connecting to angel at '%s'", addr.sun_path); - return -1; - } + if(connect(angel, (struct sockaddr*) &addr, sizeof(addr))) return -1; - if(lc_limitfd(angel, CAP_READ | CAP_WRITE) < 0) - { - sprintf(errmsg, "Error creating user angel capability: %i (%s)", - errno, strerror(errno)); - return -1; - } + int on = 1; + if(setsockopt(angel, SOL_SOCKET, SO_NOSIGPIPE, &on, sizeof(on))) return -1; + if(lc_limitfd(angel, CAP_READ | CAP_WRITE) < 0) return -1; // receive server 'hello' struct ua_datum *hello_datum = ua_recv(angel, NULL, NULL); if(!hello_datum) { - sprintf(errmsg, "Error receiving server 'hello': %i (%s)", - errno, strerror(errno)); + int olderrno = errno; close(angel); + errno = olderrno; angel = -1; return -1; } @@ -110,10 +100,9 @@ char hello[hellolen]; if(ua_unmarshall_string(hello_datum, hello, &hellolen) < 0) { - sprintf(errmsg, "Error unmarshalling server 'hello': %i (%s)", - errno, strerror(errno)); - + int olderrno = errno; close(angel); + errno = olderrno; angel = -1; return -1; } @@ -122,10 +111,8 @@ // validate server 'hello' message if(strncmp(hello, "user_angel", 10)) { - sprintf(errmsg, "Server handshake didn't start with user_angel: %s", - hello); - close(angel); + errno = EFTYPE; angel = -1; return -1; } @@ -137,10 +124,8 @@ unsigned int len = strlen(version); if((len != strlen(VERSION)) || strncmp(version, VERSION, len)) { - sprintf(errmsg, "Invalid UA protocol version: %s (expected %s)", - version, VERSION); - close(angel); + errno = ERPCMISMATCH; angel = -1; return -1; } @@ -156,17 +141,24 @@ int ua_open(const char *path, int flags) { - if(angel < 0) angel = ua_find(); - if(angel < 0) return -1; - cap_rights_t rights = CAP_SEEK | CAP_FSYNC; if(flags & O_WRONLY) rights |= CAP_WRITE | CAP_FTRUNCATE; else if(flags & O_RDWR) rights |= CAP_READ | CAP_WRITE | CAP_FTRUNCATE; else rights |= CAP_READ; - if(flags & O_EXEC) rights |= CAP_FEXECVE; + if(flags & O_EXEC) rights |= CAP_FSTAT | CAP_FEXECVE; + + return ua_ropen(path, flags, rights); +} + + +int ua_ropen(const char *path, int flags, cap_rights_t rights) +{ + if(angel < 0) angel = ua_find(); + if(angel < 0) return -1; + printf("ua_ropen('%s', %i, %016llx)\n", path, flags, rights); struct ua_datum *data[4]; data[0] = ua_marshall_int(UA_OPEN_PATH); @@ -175,13 +167,7 @@ data[3] = ua_marshall_int(rights); - for(int i = 0; i < 4; i++) - if(ua_send(angel, data[i], NULL, 0) < 0) - { - sprintf(errmsg, "Error sending request message: %s", - ua_protocol_error()); - return -1; - } + for(int i = 0; i < 4; i++) if(ua_send(angel, data[i], NULL, 0) < 0) return -1; free(data[0]); free(data[1]); @@ -192,45 +178,25 @@ // retrieve the file descriptor(s) struct ua_datum *fdcountd = ua_recv(angel, NULL, NULL); - if(!fdcountd) - { - sprintf(errmsg, "Error receiving FD count: %s", - ua_protocol_error()); - return -1; - } + if(!fdcountd) return -1; int fdcount; - if(ua_unmarshall_int(fdcountd, &fdcount) < 0) - { - fprintf(stderr, "Error unmarshalling FD count: %s\n", - ua_protocol_error()); - return -1; - } + if(ua_unmarshall_int(fdcountd, &fdcount) < 0) return -1; if(fdcount != 1) { - sprintf(errmsg, "Receiving %i FDs, only asked for 1", fdcount); + errno = EOVERFLOW; return -1; } int32_t fd; unsigned int fdlen = 1; struct ua_datum *fd_datum = ua_recv(angel, &fd, &fdlen); - if(!fd_datum) - { - sprintf(errmsg, "Error receiving FD: %s", - ua_protocol_error()); - return -1; - } + if(!fd_datum) return -1; unsigned int namelen = 80; char name[namelen]; - if(ua_unmarshall_string(fd_datum, name, &namelen) < 0) - { - sprintf(errmsg, "Error unmarshalling FD name: %s", - ua_protocol_error()); - return -1; - } + if(ua_unmarshall_string(fd_datum, name, &namelen) < 0) return -1; return fd; } @@ -280,9 +246,9 @@ if(flags & O_APPEND) if(lseek(fd, 0, SEEK_END) < 0) { - sprintf(errmsg, "Error seeking to end of %s: %i (%s)", - path, errno, strerror(errno)); + int olderrno = errno; close(fd); + errno = olderrno; return NULL; } @@ -305,12 +271,7 @@ { int cmsghdrlen = sizeof(struct cmsghdr) + fdlen * sizeof(int32_t); anc_hdr = (struct cmsghdr*) malloc(cmsghdrlen); - if(!anc_hdr) - { - sprintf(errmsg, "Error creating ancilliary header: %i (%s)", - errno, strerror(errno)); - return -1; - } + if(!anc_hdr) return -1; anc_hdr->cmsg_len = cmsghdrlen; anc_hdr->cmsg_level = SOL_SOCKET; @@ -334,13 +295,7 @@ // send! int bytes_sent = sendmsg(sock, &header, 0); - if(bytes_sent < 0) - { - sprintf(errmsg, "Error sending data and file descriptors: %i (%s)", - errno, strerror(errno)); - free(anc_hdr); - return -1; - } + if(bytes_sent < 0) return -1; free(anc_hdr); @@ -353,18 +308,7 @@ // how much data is there to receive? datum peek; int bytes = recv(sock, &peek, sizeof(datum), MSG_PEEK); - - if(bytes == 0) - { - sprintf(errmsg, "Socket closed: %s", strerror(errno)); - return NULL; - } - else if(bytes < 0) - { - sprintf(errmsg, "Error peeking at socket: %i (%s)", - errno, strerror(errno)); - return NULL; - } + if(bytes <= 0) return NULL; int to_receive = sizeof(datum) + peek.length; @@ -382,11 +326,7 @@ struct cmsghdr *anc_hdr = (struct cmsghdr*) malloc(size); bzero(anc_hdr, size); - if(!anc_hdr) - { - sprintf(errmsg, "Error creating ancilliary data header: %i (%s)", - errno, strerror(errno)); - } + if(!anc_hdr) return NULL; anc_hdr->cmsg_len = size; anc_hdr->cmsg_level = SOL_SOCKET; anc_hdr->cmsg_type = (maxfds ? SCM_RIGHTS : 0); @@ -404,21 +344,14 @@ bytes = recvmsg(sock, &header, MSG_WAITALL); - if(bytes < 0) + if(bytes <= 0) { - sprintf(errmsg, "Error receiving message: %i (%s)", - errno, strerror(errno)); + int olderrno = errno; free(anc_hdr); free(d); + errno = olderrno; return NULL; } - else if(bytes == 0) - { - sprintf(errmsg, "Socket closed: %i (%s)", errno, strerror(errno)); - free(anc_hdr); - free(d); - return NULL; - } if(maxfds > 0) @@ -461,24 +394,23 @@ { if(d == NULL) { - sprintf(errmsg, "Datum is NULL"); + errno = EINVAL; return -1; } if(!(d->type & INTEGER)) { if(d->type & ERROR) handle_error(d); - else sprintf(errmsg, "Datum's type is %i, not INTEGER (%i)", - d->type, INTEGER); + else errno = EINVAL; return -1; } - if(d->length != sizeof(int32_t)) + if(d->length > sizeof(int32_t)) { - sprintf(errmsg, "An 32-bit integer should be 4B long, not %i", - d->length); + errno = EOVERFLOW; return -1; } + else bzero(value, sizeof(int32_t)); memcpy(value, ((const char*) d) + sizeof(datum), 4); return d->length; @@ -504,19 +436,21 @@ { if(d == NULL) { - sprintf(errmsg, "NULL datum"); + errno = EINVAL; return -1; } else if(d->type != STRING) { if(d->type & ERROR) handle_error(d); - else sprintf(errmsg, "Datum's type is %i, not STRING (%i)", - d->type, STRING); + else + { + errno = EINVAL; + return -1; + } } else if(d->length >= *len) { - sprintf(errmsg, "String in datum is too long (%iB) to fit in buffer (%iB)", - d->length, *len); + errno = EOVERFLOW; return -1; } @@ -552,18 +486,17 @@ { if(d == NULL) { - sprintf(errmsg, "NULL datum"); + errno = EINVAL; return -1; } else if(d->type != ERROR) { - sprintf(errmsg, "Datum's type is %i, not ERROR (%i)", - d->type, ERROR); + errno = EINVAL; + return -1; } else if(d->length >= (*msglen - sizeof(int32_t))) { - sprintf(errmsg, "Message is too long (%iB) to fit in buffer (%iB)", - d->length - sizeof(int32_t), *msglen); + errno = EOVERFLOW; return -1; } @@ -596,7 +529,7 @@ for(int i = 0; i < 8; i++) if(data[i] == NULL) { - sprintf(errmsg, "Capbox datum %i is NULL", i); + errno = EINVAL; return NULL; } else total_size += (sizeof(datum) + data[i]->length); @@ -623,37 +556,24 @@ { if(d == NULL) { - sprintf(errmsg, "NULL datum"); + errno = EINVAL; return -1; } else if(d->type != CAPBOX_OPTIONS) { if(d->type & ERROR) handle_error(d); - else sprintf(errmsg, "Datum's type is %i, not CAPBOX_OPTIONS (%i)", - d->type, CAPBOX_OPTIONS); + else errno = EINVAL; return -1; } int32_t tmp_int; const datum *head = (const datum*) (((const char*) d) + sizeof(datum)); + if(ua_unmarshall_int(head, &tmp_int) < 0) return -1; - if(ua_unmarshall_int(head, &tmp_int) < 0) - { - char error[128]; - sprintf(error, "Error unmarshalling UI type: %s", ua_protocol_error()); - strcpy(errmsg, error); - return -1; - } options->ui = tmp_int; head = (const datum*) (((const char*) head) + sizeof(datum) + head->length); + if(ua_unmarshall_int(head, &tmp_int) < 0) return -1; - if(ua_unmarshall_int(head, &tmp_int) < 0) - { - char error[128]; - sprintf(error, "Error unmarshalling operation: %s", ua_protocol_error()); - strcpy(errmsg, error); - return -1; - } options->operation = tmp_int; head = (const datum*) (((const char*) head) + sizeof(datum) + head->length); @@ -661,25 +581,13 @@ // window title is handled elsewhere - if(ua_unmarshall_int(head, &tmp_int) < 0) - { - char error[128]; - sprintf(error, "Error unmarshalling parent: %s", ua_protocol_error()); - strcpy(errmsg, error); - return -1; - } + if(ua_unmarshall_int(head, &tmp_int) < 0) return -1; options->parent_window = tmp_int; head = (const datum*) (((const char*) head) + sizeof(datum) + head->length); options->pathlen = d->length + 1; char *str = (char*) malloc(options->pathlen); - if(ua_unmarshall_string(head, str, &options->pathlen) < 0) - { - char error[128]; - sprintf(error, "Error unmarshalling path: %s", ua_protocol_error()); - strcpy(errmsg, error); - return -1; - } + if(ua_unmarshall_string(head, str, &options->pathlen) < 0) return -1; options->start_path = str; head = (const datum*) (((const char*) head) + sizeof(datum) + head->length); @@ -688,45 +596,21 @@ // that's handled at the recvmsg() level - if(ua_unmarshall_int(head, &tmp_int) < 0) - { - char error[128]; - sprintf(error, "Error unmarshalling 'mult': %s", ua_protocol_error()); - strcpy(errmsg, error); - return -1; - } + if(ua_unmarshall_int(head, &tmp_int) < 0) return -1; options->mult = tmp_int; head = (const datum*) (((const char*) head) + sizeof(datum) + head->length); options->filterlen = d->length + 1; str = (char*) malloc(options->filterlen); - if(ua_unmarshall_string(head, str, &options->filterlen) < 0) - { - char error[128]; - sprintf(error, "Error unmarshalling filter: %s", ua_protocol_error()); - strcpy(errmsg, error); - return -1; - } + if(ua_unmarshall_string(head, str, &options->filterlen) < 0) return -1; options->filter = str; head = (const datum*) (((const char*) head) + sizeof(datum) + head->length); - if(ua_unmarshall_int(head, &tmp_int) < 0) - { - char error[128]; - sprintf(error, "Error unmarshalling 'flags': %s", ua_protocol_error()); - strcpy(errmsg, error); - return -1; - } + if(ua_unmarshall_int(head, &tmp_int) < 0) return -1; options->flags = tmp_int; head = (const datum*) (((const char*) head) + sizeof(datum) + head->length); - if(ua_unmarshall_int(head, &tmp_int) < 0) - { - char error[128]; - sprintf(error, "Error unmarshalling 'rights': %s", ua_protocol_error()); - strcpy(errmsg, error); - return -1; - } + if(ua_unmarshall_int(head, &tmp_int) < 0) return -1; options->rights = tmp_int; @@ -740,13 +624,13 @@ char msg[200]; int msglen = 200; - if(ua_unmarshall_error(d, &errnum, msg, &msglen) < 0) - { - sprintf(errmsg, "Error unmarshalling error message\n"); - return; - } + if(ua_unmarshall_error(d, &errnum, msg, &msglen) < 0) return; + + errno = errnum; - sprintf(errmsg, "user_angel error: %s (%i: %s)", - msg, errnum, strerror(errnum)); + /* TODO: do something! + fprintf(stderr, "user_angel error: %s (%i: %s)\n", + msg, errnum, strerror(errnum)); + */ } ==== //depot/projects/trustedbsd/capabilities/src/lib/libuserangel/libuserangel.h#10 (text+ko) ==== @@ -47,9 +47,6 @@ /** Version of the User Angel protocol */ const char* ua_protocol_version(void); -/** The last angel/sandbox protocol error */ -const char* ua_protocol_error(void); - /** Find the user angel (at $HOME/.user-angel or the like) */ int ua_find(void); @@ -59,6 +56,9 @@ /** Open a file via the User Angel */ int ua_open(const char *path, int flags); +/** Open a file via the User Angel, specifying rights the capability should have */ +int ua_ropen(const char *path, int flags, cap_rights_t rights); + /** Open a file stream via the User Angel */ FILE* ua_fopen(const char *path, const char *mode);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200907140640.n6E6eotr033203>