From owner-p4-projects@FreeBSD.ORG Thu Jun 18 13:02:32 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 6E8BD1065674; Thu, 18 Jun 2009 13:02:32 +0000 (UTC) Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 2DA6D106566B for ; Thu, 18 Jun 2009 13:02:32 +0000 (UTC) (envelope-from jona@FreeBSD.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 12F938FC22 for ; Thu, 18 Jun 2009 13:02:32 +0000 (UTC) (envelope-from jona@FreeBSD.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id n5ID2WFD021449 for ; Thu, 18 Jun 2009 13:02:32 GMT (envelope-from jona@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id n5ID2WOF021447 for perforce@freebsd.org; Thu, 18 Jun 2009 13:02:32 GMT (envelope-from jona@FreeBSD.org) Date: Thu, 18 Jun 2009 13:02:32 GMT Message-Id: <200906181302.n5ID2WOF021447@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to jona@FreeBSD.org using -f From: Jonathan Anderson To: Perforce Change Reviews Cc: Subject: PERFORCE change 164664 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Jun 2009 13:02:33 -0000 http://perforce.freebsd.org/chv.cgi?CH=164664 Change 164664 by jona@jona-trustedbsd-kentvm on 2009/06/18 13:01:59 Error message passing and handling Affected files ... .. //depot/projects/trustedbsd/capabilities/src/tools/cap/user_angel/cap.c#6 edit .. //depot/projects/trustedbsd/capabilities/src/tools/cap/user_angel/cap.h#3 edit .. //depot/projects/trustedbsd/capabilities/src/tools/cap/user_angel/protocol.c#11 edit .. //depot/projects/trustedbsd/capabilities/src/tools/cap/user_angel/protocol.h#10 edit .. //depot/projects/trustedbsd/capabilities/src/tools/cap/user_angel/server.c#7 edit .. //depot/projects/trustedbsd/capabilities/src/tools/cap/user_angel/test_client.c#8 edit Differences ... ==== //depot/projects/trustedbsd/capabilities/src/tools/cap/user_angel/cap.c#6 (text+ko) ==== @@ -31,7 +31,7 @@ * SUCH DAMAGE. */ -#include +#include #include #include #include @@ -41,21 +41,27 @@ #include "cap.h" +char errstr[512]; +const char *cap_error() { return errstr; } + + int cap_open(const char *path, int flags, cap_rights_t rights) { int fd = open(path, flags); if(fd < 0) { - char error[80 + strlen(path)]; - sprintf(error, "failed to open() path '%s'", path); - perror(error); + if(strlen(path) > 256) path = ""; + sprintf(errstr, "Failed to open() path '%s': %i (%s)", + path, errno, strerror(errno)); return -1; } int cap = cap_new(fd, rights); if(cap < 0) { - perror("Failed to create new capability"); + if(strlen(path) > 256) path = ""; + sprintf(errstr, "Failed to create new capability: %i (%s)", + errno, strerror(errno)); return -1; } ==== //depot/projects/trustedbsd/capabilities/src/tools/cap/user_angel/cap.h#3 (text+ko) ==== @@ -41,6 +41,9 @@ #define CAP_SET_FILE_RW (CAP_SET_FILE_READ | CAP_SET_FILE_WRITE) +/** The last capability-related error */ +const char *cap_error(); + /** Open a file in capability mode with specified rights */ int cap_open(const char *path, int flags, cap_rights_t rights); ==== //depot/projects/trustedbsd/capabilities/src/tools/cap/user_angel/protocol.c#11 (text+ko) ==== @@ -44,9 +44,30 @@ #include "protocol.h" +char errmsg[256]; +const char* cap_protocol_error(void) { return errmsg; } + +typedef struct cap_wire_datum wire_datum; +void handle_error(const wire_datum *d) +{ + int32_t errnum; + char msg[200]; + int msglen = 200; + + if(cap_unmarshall_error(d, &errnum, msg, &msglen) < 0) + { + sprintf(errmsg, "Error unmarshalling error message\n"); + return; + } + + sprintf(errmsg, "user_angel error: %s (%i: %s)", + msg, errnum, strerror(errnum)); +} + + void print_datum(const struct cap_wire_datum *d) { printf("Datum @ 0x%8x:\n", (unsigned int) d); @@ -65,14 +86,6 @@ - -typedef struct cap_wire_datum wire_datum; - - -char errmsg[256]; -const char* cap_error(void) { return errmsg; } - - wire_datum* cap_marshall_int(int32_t value) { int size = sizeof(wire_datum) + sizeof(int32_t); @@ -99,8 +112,9 @@ if(!(datum->type & INTEGER)) { - sprintf(errmsg, "Datum's type is %i, not INTEGER (%i)", - datum->type, INTEGER); + if(datum->type & ERROR) handle_error(datum); + else sprintf(errmsg, "Datum's type is %i, not INTEGER (%i)", + datum->type, INTEGER); return -1; } @@ -138,8 +152,9 @@ } else if(datum->type != STRING) { - sprintf(errmsg, "Datum's type is %i, not STRING (%i)", - datum->type, STRING); + if(datum->type & ERROR) handle_error(datum); + else sprintf(errmsg, "Datum's type is %i, not STRING (%i)", + datum->type, STRING); } else if(datum->length < 0) { @@ -163,40 +178,25 @@ -wire_datum* cap_marshall_error(int errno, const char *msg, int msglen) +wire_datum* cap_marshall_error(int errnum, const char *msg, int msglen) { - wire_datum *data[2]; - data[0] = cap_marshall_int(errno); - data[1] = cap_marshall_string(msg, msglen); - - int total_size = 0; - for(int i = 0; i < 2; i++) - if(data[i] == NULL) - { - sprintf(errmsg, "Error datum %i is NULL", i); - return NULL; - } - else total_size += (sizeof(wire_datum) + data[i]->length); - - wire_datum *d = (wire_datum*) malloc(sizeof(wire_datum) + total_size); + int size = sizeof(wire_datum) + sizeof(int32_t) + msglen; + wire_datum *d = (wire_datum*) malloc(size); + d->type = ERROR; - d->length = total_size; + d->length = sizeof(int32_t) + msglen; - char *buffer = ((char*) d) + sizeof(wire_datum); - char *head = buffer; - for(int i = 0; i < 2; i++) - { - memcpy(head, data[i], sizeof(wire_datum) + data[i]->length); - head += sizeof(wire_datum) + data[i]->length; + char *address = ((char*) d) + sizeof(wire_datum); + *((int32_t*) address) = errnum; - free(data[i]); - } + address += sizeof(int32_t); + memcpy((char*) address, msg, msglen); return d; } -int cap_unmarshall_error(const wire_datum *d, int *errno, const char *msg, int *msglen) +int cap_unmarshall_error(const wire_datum *datum, int *errnum, char *msg, int *msglen) { if(datum == NULL) { @@ -214,30 +214,22 @@ datum->length); return -1; } - - int32_t tmp_int; - wire_datum *d = (wire_datum*) (((char*) datum) + sizeof(wire_datum)); - - if(cap_unmarshall_int(d, &tmp_int) < 0) + else if(datum->length >= (*msglen - sizeof(int32_t))) { - char error[128]; - sprintf(error, "Error unmarshalling errno: %s", cap_error()); - strcpy(errmsg, error); + sprintf(errmsg, "Message is too long (%iB) to fit in buffer (%iB)", + datum->length - sizeof(int32_t), *msglen); return -1; } - *errno = tmp_int; - d = (wire_datum*) (((char*) d) + sizeof(wire_datum) + d->length); + char *address = ((char*) datum) + sizeof(wire_datum); + *errnum = *((int32_t*) address); + address += sizeof(int32_t); - if(cap_unmarshall_string(d, msg, msglen) < 0) - { - char error[128]; - sprintf(error, "Error unmarshalling path: %s", cap_error()); - strcpy(errmsg, error); - return -1; - } + *msglen = datum->length - sizeof(int32_t); + memcpy(msg, address, *msglen); + msg[*msglen] = '\0'; - return sizeof(wire_datum) + datum->length; + return datum->length; } @@ -290,8 +282,10 @@ } else if(datum->type != CAPBOX_OPTIONS) { - sprintf(errmsg, "Datum's type is %i, not CAPBOX_OPTIONS (%i)", - datum->type, CAPBOX_OPTIONS); + if(datum->type & ERROR) handle_error(datum); + else sprintf(errmsg, "Datum's type is %i, not CAPBOX_OPTIONS (%i)", + datum->type, CAPBOX_OPTIONS); + return -1; } else if(datum->length < 0) { @@ -306,7 +300,7 @@ if(cap_unmarshall_int(d, &tmp_int) < 0) { char error[128]; - sprintf(error, "Error unmarshalling UI type: %s", cap_error()); + sprintf(error, "Error unmarshalling UI type: %s", cap_protocol_error()); strcpy(errmsg, error); return -1; } @@ -316,7 +310,7 @@ if(cap_unmarshall_int(d, &tmp_int) < 0) { char error[128]; - sprintf(error, "Error unmarshalling operation: %s", cap_error()); + sprintf(error, "Error unmarshalling operation: %s", cap_protocol_error()); strcpy(errmsg, error); return -1; } @@ -330,7 +324,7 @@ if(cap_unmarshall_int(d, &tmp_int) < 0) { char error[128]; - sprintf(error, "Error unmarshalling parent: %s", cap_error()); + sprintf(error, "Error unmarshalling parent: %s", cap_protocol_error()); strcpy(errmsg, error); return -1; } @@ -342,7 +336,7 @@ if(cap_unmarshall_string(d, (char*) options->start_path, &options->pathlen) < 0) { char error[128]; - sprintf(error, "Error unmarshalling path: %s", cap_error()); + sprintf(error, "Error unmarshalling path: %s", cap_protocol_error()); strcpy(errmsg, error); return -1; } @@ -356,7 +350,7 @@ if(cap_unmarshall_int(d, &tmp_int) < 0) { char error[128]; - sprintf(error, "Error unmarshalling 'mult': %s", cap_error()); + sprintf(error, "Error unmarshalling 'mult': %s", cap_protocol_error()); strcpy(errmsg, error); return -1; } @@ -368,7 +362,7 @@ if(cap_unmarshall_string(d, (char*) options->filter, &options->filterlen) < 0) { char error[128]; - sprintf(error, "Error unmarshalling filter: %s", cap_error()); + sprintf(error, "Error unmarshalling filter: %s", cap_protocol_error()); strcpy(errmsg, error); return -1; } @@ -377,7 +371,7 @@ if(cap_unmarshall_int(d, &tmp_int) < 0) { char error[128]; - sprintf(error, "Error unmarshalling 'flags': %s", cap_error()); + sprintf(error, "Error unmarshalling 'flags': %s", cap_protocol_error()); strcpy(errmsg, error); return -1; } @@ -387,7 +381,7 @@ if(cap_unmarshall_int(d, &tmp_int) < 0) { char error[128]; - sprintf(error, "Error unmarshalling 'rights': %s", cap_error()); + sprintf(error, "Error unmarshalling 'rights': %s", cap_protocol_error()); strcpy(errmsg, error); return -1; } ==== //depot/projects/trustedbsd/capabilities/src/tools/cap/user_angel/protocol.h#10 (text+ko) ==== @@ -46,7 +46,7 @@ /** The last protocol error */ -const char* cap_error(void); +const char* cap_protocol_error(void); @@ -76,14 +76,14 @@ /* Unmarshalling functions; calling programs should free the result */ struct cap_wire_datum* cap_marshall_int(int32_t value); struct cap_wire_datum* cap_marshall_string(const char *value, int len); -struct cap_wire_datum* cap_marshall_error(int errno, const char *msg, int msglen); +struct cap_wire_datum* cap_marshall_error(int errnum, const char *msg, int msglen); struct cap_wire_datum* cap_marshall_capbox(const struct capbox_options *options); /* Unmarshalling functions; return the number of bytes unmarshalled (or -1) */ int cap_unmarshall_int(const struct cap_wire_datum *d, int32_t *value); int cap_unmarshall_string(const struct cap_wire_datum *d, char *value, int *len); -int cap_unmarshall_error(const struct cap_wire_datum *d, int *errno, const char *msg, int *msglen); +int cap_unmarshall_error(const struct cap_wire_datum *d, int *errnum, char *msg, int *msglen); int cap_unmarshall_capbox(const struct cap_wire_datum *d, struct capbox_options *options); ==== //depot/projects/trustedbsd/capabilities/src/tools/cap/user_angel/server.c#7 (text+ko) ==== @@ -53,6 +53,8 @@ int shutting_down = 0; char control_socket_name[256] = ""; +char current_error[512]; + struct fd_set clients; int highest_fd; @@ -76,7 +78,7 @@ int handle_request(int client, enum capangel_req_t req); int handle_path_request(int client); int handle_powerbox_request(int client); -void client_closed(int client); +void close_client(int client, int errnum, const char *msg); @@ -242,7 +244,7 @@ if(!d) { if((errno == ENOENT) || (errno == ECONNRESET)) - client_closed(client); + close_client(client, errno, "Client socket closed"); else perror("Error receiving from client"); @@ -256,20 +258,20 @@ else { - fprintf(stderr, "enum size is %iB\n", sizeof(enum capangel_req_t)); + sprintf(current_error, "enum size is %iB", + sizeof(enum capangel_req_t)); return -1; } if(bytes < 0) { - fprintf(stderr, "Error unmarshalling request: %s\n", cap_error()); + strcpy(current_error, cap_protocol_error()); return -1; } if(handle_request(client, req)) { - perror("Error handling client request"); - client_closed(client); + close_client(client, errno, current_error); return 0; } @@ -292,7 +294,7 @@ return handle_powerbox_request(client); default: - fprintf(stderr, "Unknown request %i\n", req); + sprintf(current_error, "Unknown request %i", req); return -1; } @@ -303,37 +305,48 @@ int handle_path_request(int client) { int fdlen = 0; + char path[256] = ""; + int pathlen = 256; + struct cap_wire_datum *d = cap_recv_fds(client, NULL, &fdlen); + if(cap_unmarshall_string(d, path, &pathlen) < 0) + { + strcpy(current_error, cap_protocol_error()); + return -1; + } + free(d); - if(!d) + int32_t flags, rights; + if(cap_unmarshall_int(cap_recv(client), &flags) < 0) { - perror("Error receiving path from client"); + fprintf(stderr, "Error unmarshalling flags: %s\n", cap_protocol_error()); return -1; } - char path[256] = ""; - int pathlen = 256; - - if(cap_unmarshall_string(d, path, &pathlen) < 0) + if(cap_unmarshall_int(cap_recv(client), &rights) < 0) { - fprintf(stderr, "Error unmarshalling path: %s\n", cap_error()); + fprintf(stderr, "Error unmarshalling rights: %s\n", cap_protocol_error()); return -1; } - free(d); - int cap = cap_open(path, O_RDONLY, CAP_SET_FILE_READ); + int cap = cap_open(path, flags, rights); + if(cap < 0) + { + strcpy(current_error, cap_error()); + return -1; + } d = cap_marshall_int(1); if(!d) { - fprintf(stderr, "Error marshalling FD count: %s\n", cap_error()); + strcpy(current_error, cap_protocol_error()); return -1; } if(cap_send(client, d) < 0) { - perror("Error sending FD count"); + sprintf(current_error, "Error sending FD count: %s", strerror(errno)); return -1; } free(d); @@ -341,7 +354,7 @@ d = cap_marshall_string(path, pathlen); if(!d) { - fprintf(stderr, "Error marshalling FD path: %s\n", cap_error()); + strcpy(current_error, cap_protocol_error()); return -1; } @@ -371,8 +384,7 @@ if(cap_unmarshall_capbox(d, &options) < 0) { - fprintf(stderr, "Error unmarshalling powerbox options: %s", - cap_error()); + strcpy(current_error, cap_protocol_error()); return -1; } @@ -388,8 +400,8 @@ int len = 32; if(capbox_display(&options, fds, names, &len)) { - fprintf(stderr, "Error in powerbox\n"); - return 0; + strcpy(current_error, "Unknown error in powerbox"); + return -1; } free(options.window_title); @@ -419,9 +431,13 @@ } -void client_closed(int client) +void close_client(int client, int errnum, const char *reason) { - printf("Client %4i: Closed\n", client); + printf("Client %4i: Closing (errno: %i/'%s', reason: '%s')\n", + client, errnum, strerror(errnum), reason); + + cap_send(client, cap_marshall_error(errnum, reason, strlen(reason))); + close(client); FD_CLR(client, &clients); ==== //depot/projects/trustedbsd/capabilities/src/tools/cap/user_angel/test_client.c#8 (text+ko) ==== @@ -15,7 +15,7 @@ int connect_to_user_angel(void); -void open_file(int fd_angel, const char *path); +void open_file(int fd_angel, const char *path, int flags, cap_rights_t rights); void open_powerbox(int fd_angel, const char *path, const char *filter, int parent); void test_fd(int fd, char *name); @@ -61,8 +61,8 @@ - open_file(fd_angel, "/etc/group"); - open_file(fd_angel, "/etc/passwd"); + open_file(fd_angel, "/etc/group", O_RDONLY, CAP_FSTAT | CAP_READ | CAP_SEEK); + open_file(fd_angel, "/etc/passwd", O_RDONLY, CAP_FSTAT | CAP_READ | CAP_WRITE | CAP_SEEK); open_powerbox(fd_angel, "~/Desktop/", "*.gz", 0x2a00003); return 0; @@ -94,19 +94,23 @@ -void open_file(int fd_angel, const char *path) +void open_file(int fd_angel, const char *path, int flags, cap_rights_t rights) { // get the user angel to open the file for us - struct cap_wire_datum *data[2]; + struct cap_wire_datum *data[4]; data[0] = cap_marshall_int(FD_FROM_PATH); data[1] = cap_marshall_string(path, strlen(path)); + data[2] = cap_marshall_int(flags); + data[3] = cap_marshall_int(rights); - if(cap_send_message(fd_angel, data, 2) < 0) + if(cap_send_message(fd_angel, data, 4) < 0) err(EX_IOERR, "Error sending request message"); free(data[0]); free(data[1]); + free(data[2]); + free(data[3]); @@ -116,7 +120,10 @@ int fdcount; if(cap_unmarshall_int(fdcountd, &fdcount) < 0) - err(EX_SOFTWARE, "Error unmarshalling FD count"); + { + fprintf(stderr, "Error unmarshalling FD count: %s\n", cap_protocol_error()); + return; + } for(int i = 0; i < fdcount; i++) {