From e4de1f768e818911e6c035ddcbaf0127e0eccb40 Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Wed, 10 Apr 2013 18:10:52 -0400 Subject: Trace log with a subset of struct conn_state In struct conn_state, collect together the fields for the remote address and put them in a substructure. Pass this substructure to trace logging macros instead of the entire conn_state structure, so that trace.c doesn't have to know about the whole structure. --- src/include/cm.h | 11 +++++---- src/include/k5-trace.h | 54 +++++++++++++++++++++---------------------- src/lib/krb5/os/sendto_kdc.c | 55 +++++++++++++++++++++++--------------------- src/lib/krb5/os/t_trace.c | 34 +++++++++++++-------------- src/lib/krb5/os/t_trace.ref | 8 +++---- src/lib/krb5/os/trace.c | 18 +++++++-------- 6 files changed, 93 insertions(+), 87 deletions(-) diff --git a/src/include/cm.h b/src/include/cm.h index d9c23fce1f..837a549b0c 100644 --- a/src/include/cm.h +++ b/src/include/cm.h @@ -63,6 +63,12 @@ struct incoming_krb5_message { unsigned char bufsizebytes[4]; size_t n_left; }; +struct remote_address { + int family; + int type; + socklen_t len; + struct sockaddr_storage saddr; +}; struct conn_state { SOCKET fd; krb5_error_code err; @@ -70,10 +76,7 @@ struct conn_state { unsigned int is_udp : 1; int (*service)(krb5_context context, struct conn_state *, struct select_state *, int); - int socktype; - int family; - size_t addrlen; - struct sockaddr_storage addr; + struct remote_address addr; struct { struct { sg_buf sgbuf[2]; diff --git a/src/include/k5-trace.h b/src/include/k5-trace.h index 3b539b5154..70151c6866 100644 --- a/src/include/k5-trace.h +++ b/src/include/k5-trace.h @@ -60,7 +60,7 @@ * {lenstr} size_t and const char *, as a counted string * {hexlenstr} size_t and const char *, as hex bytes * {hashlenstr} size_t and const char *, as four-character hex hash - * {connstate} struct conn_state *, show socket type, address, port + * {raddr} struct remote_address *, show socket type, address, port * {data} krb5_data *, display as counted string * {hexdata} krb5_data *, display as hex bytes * {errno} int, display as number/errorstring @@ -312,32 +312,32 @@ void krb5int_trace(krb5_context context, const char *fmt, ...); TRACE(c, "Response was{str} from master KDC", (master) ? "" : " not") #define TRACE_SENDTO_KDC_RESOLVING(c, hostname) \ TRACE(c, "Resolving hostname {str}", hostname) -#define TRACE_SENDTO_KDC_RESPONSE(c, conn) \ - TRACE(c, "Received answer from {connstate}", conn) -#define TRACE_SENDTO_KDC_TCP_CONNECT(c, conn) \ - TRACE(c, "Initiating TCP connection to {connstate}", conn) -#define TRACE_SENDTO_KDC_TCP_DISCONNECT(c, conn) \ - TRACE(c, "Terminating TCP connection to {connstate}", conn) -#define TRACE_SENDTO_KDC_TCP_ERROR_CONNECT(c, conn, err) \ - TRACE(c, "TCP error connecting to {connstate}: {errno}", conn, err) -#define TRACE_SENDTO_KDC_TCP_ERROR_RECV(c, conn, err) \ - TRACE(c, "TCP error receiving from {connstate}: {errno}", conn, err) -#define TRACE_SENDTO_KDC_TCP_ERROR_RECV_LEN(c, conn, err) \ - TRACE(c, "TCP error receiving from {connstate}: {errno}", conn, err) -#define TRACE_SENDTO_KDC_TCP_ERROR_SEND(c, conn, err) \ - TRACE(c, "TCP error sending to {connstate}: {errno}", conn, err) -#define TRACE_SENDTO_KDC_TCP_SEND(c, conn) \ - TRACE(c, "Sending TCP request to {connstate}", conn) -#define TRACE_SENDTO_KDC_UDP_ERROR_RECV(c, conn, err) \ - TRACE(c, "UDP error receiving from {connstate}: {errno}", conn, err) -#define TRACE_SENDTO_KDC_UDP_ERROR_SEND_INITIAL(c, conn, err) \ - TRACE(c, "UDP error sending to {connstate}: {errno}", conn, err) -#define TRACE_SENDTO_KDC_UDP_ERROR_SEND_RETRY(c, conn, err) \ - TRACE(c, "UDP error sending to {connstate}: {errno}", conn, err) -#define TRACE_SENDTO_KDC_UDP_SEND_INITIAL(c, conn) \ - TRACE(c, "Sending initial UDP request to {connstate}", conn) -#define TRACE_SENDTO_KDC_UDP_SEND_RETRY(c, conn) \ - TRACE(c, "Sending retry UDP request to {connstate}", conn) +#define TRACE_SENDTO_KDC_RESPONSE(c, raddr) \ + TRACE(c, "Received answer from {raddr}", raddr) +#define TRACE_SENDTO_KDC_TCP_CONNECT(c, raddr) \ + TRACE(c, "Initiating TCP connection to {raddr}", raddr) +#define TRACE_SENDTO_KDC_TCP_DISCONNECT(c, raddr) \ + TRACE(c, "Terminating TCP connection to {raddr}", raddr) +#define TRACE_SENDTO_KDC_TCP_ERROR_CONNECT(c, raddr, err) \ + TRACE(c, "TCP error connecting to {raddr}: {errno}", raddr, err) +#define TRACE_SENDTO_KDC_TCP_ERROR_RECV(c, raddr, err) \ + TRACE(c, "TCP error receiving from {raddr}: {errno}", raddr, err) +#define TRACE_SENDTO_KDC_TCP_ERROR_RECV_LEN(c, raddr, err) \ + TRACE(c, "TCP error receiving from {raddr}: {errno}", raddr, err) +#define TRACE_SENDTO_KDC_TCP_ERROR_SEND(c, raddr, err) \ + TRACE(c, "TCP error sending to {raddr}: {errno}", raddr, err) +#define TRACE_SENDTO_KDC_TCP_SEND(c, raddr) \ + TRACE(c, "Sending TCP request to {raddr}", raddr) +#define TRACE_SENDTO_KDC_UDP_ERROR_RECV(c, raddr, err) \ + TRACE(c, "UDP error receiving from {raddr}: {errno}", raddr, err) +#define TRACE_SENDTO_KDC_UDP_ERROR_SEND_INITIAL(c, raddr, err) \ + TRACE(c, "UDP error sending to {raddr}: {errno}", raddr, err) +#define TRACE_SENDTO_KDC_UDP_ERROR_SEND_RETRY(c, raddr, err) \ + TRACE(c, "UDP error sending to {raddr}: {errno}", raddr, err) +#define TRACE_SENDTO_KDC_UDP_SEND_INITIAL(c, raddr) \ + TRACE(c, "Sending initial UDP request to {raddr}", raddr) +#define TRACE_SENDTO_KDC_UDP_SEND_RETRY(c, raddr) \ + TRACE(c, "Sending retry UDP request to {raddr}", raddr) #define TRACE_SEND_TGS_ETYPES(c, etypes) \ TRACE(c, "etypes requested in TGS request: {etypes}", etypes) diff --git a/src/lib/krb5/os/sendto_kdc.c b/src/lib/krb5/os/sendto_kdc.c index 040c554e43..04c9a7ae64 100644 --- a/src/lib/krb5/os/sendto_kdc.c +++ b/src/lib/krb5/os/sendto_kdc.c @@ -605,10 +605,10 @@ add_connection(struct conn_state **conns, struct addrinfo *ai, state->state = INITIALIZING; state->err = 0; state->x.out.sgp = state->x.out.sgbuf; - state->socktype = ai->ai_socktype; - state->family = ai->ai_family; - state->addrlen = ai->ai_addrlen; - memcpy(&state->addr, ai->ai_addr, ai->ai_addrlen); + state->addr.type = ai->ai_socktype; + state->addr.family = ai->ai_family; + state->addr.len = ai->ai_addrlen; + memcpy(&state->addr.saddr, ai->ai_addr, ai->ai_addrlen); state->fd = INVALID_SOCKET; state->server_index = server_index; SG_SET(&state->x.out.sgbuf[1], 0, 0); @@ -766,25 +766,27 @@ start_connection(krb5_context context, struct conn_state *state, static const struct linger lopt = { 0, 0 }; dprint("start_connection(@%p)\ngetting %s socket in family %d...", state, - state->socktype == SOCK_STREAM ? "stream" : "dgram", state->family); - fd = socket(state->family, state->socktype, 0); + state->addr.type == SOCK_STREAM ? "stream" : "dgram", + state->addr.family); + fd = socket(state->addr.family, state->addr.type, 0); if (fd == INVALID_SOCKET) { state->err = SOCKET_ERRNO; - dprint("socket: %m creating with af %d\n", state->err, state->family); + dprint("socket: %m creating with af %d\n", state->err, + state->addr.family); return -1; /* try other hosts */ } set_cloexec_fd(fd); /* Make it non-blocking. */ if (ioctlsocket(fd, FIONBIO, (const void *) &one)) dperror("sendto_kdc: ioctl(FIONBIO)"); - if (state->socktype == SOCK_STREAM) { + if (state->addr.type == SOCK_STREAM) { if (setsockopt(fd, SOL_SOCKET, SO_LINGER, &lopt, sizeof(lopt))) dperror("sendto_kdc: setsockopt(SO_LINGER)"); - TRACE_SENDTO_KDC_TCP_CONNECT(context, state); + TRACE_SENDTO_KDC_TCP_CONNECT(context, &state->addr); } /* Start connecting to KDC. */ - e = connect(fd, (struct sockaddr *)&state->addr, state->addrlen); + e = connect(fd, (struct sockaddr *)&state->addr.saddr, state->addr.len); if (e != 0) { /* * This is the path that should be followed for non-blocking @@ -832,16 +834,16 @@ start_connection(krb5_context context, struct conn_state *state, set_conn_state_msg_length(state, &state->callback_buffer); } - if (state->socktype == SOCK_DGRAM) { + if (state->addr.type == SOCK_DGRAM) { /* Send it now. */ ssize_t ret; sg_buf *sg = &state->x.out.sgbuf[0]; - TRACE_SENDTO_KDC_UDP_SEND_INITIAL(context, state); + TRACE_SENDTO_KDC_UDP_SEND_INITIAL(context, &state->addr); dprint("sending %d bytes on fd %d\n", SG_LEN(sg), state->fd); ret = send(state->fd, SG_BUF(sg), SG_LEN(sg), 0); if (ret < 0 || (size_t) ret != SG_LEN(sg)) { - TRACE_SENDTO_KDC_UDP_ERROR_SEND_INITIAL(context, state, + TRACE_SENDTO_KDC_UDP_ERROR_SEND_INITIAL(context, &state->addr, SOCKET_ERRNO); dperror("sendto"); (void) closesocket(state->fd); @@ -889,7 +891,7 @@ maybe_send(krb5_context context, struct conn_state *conn, return -1; } - if (conn->socktype == SOCK_STREAM) { + if (conn->addr.type == SOCK_STREAM) { dprint("skipping stream socket\n"); /* The select callback will handle flushing any data we haven't written yet, and we only write it once. */ @@ -898,11 +900,12 @@ maybe_send(krb5_context context, struct conn_state *conn, /* UDP - retransmit after a previous attempt timed out. */ sg = &conn->x.out.sgbuf[0]; - TRACE_SENDTO_KDC_UDP_SEND_RETRY(context, conn); + TRACE_SENDTO_KDC_UDP_SEND_RETRY(context, &conn->addr); dprint("sending %d bytes on fd %d\n", SG_LEN(sg), conn->fd); ret = send(conn->fd, SG_BUF(sg), SG_LEN(sg), 0); if (ret < 0 || (size_t) ret != SG_LEN(sg)) { - TRACE_SENDTO_KDC_UDP_ERROR_SEND_RETRY(context, conn, SOCKET_ERRNO); + TRACE_SENDTO_KDC_UDP_ERROR_SEND_RETRY(context, &conn->addr, + SOCKET_ERRNO); dperror("send"); /* Keep connection alive, we'll try again next pass. @@ -965,7 +968,7 @@ service_tcp_fd(krb5_context context, struct conn_state *conn, /* Bad -- the KDC shouldn't be sending to us first. */ e = EINVAL /* ?? */; kill_conn: - TRACE_SENDTO_KDC_TCP_DISCONNECT(context, conn); + TRACE_SENDTO_KDC_TCP_DISCONNECT(context, &conn->addr); kill_conn(conn, selstate, e); return e == 0; } @@ -990,7 +993,7 @@ service_tcp_fd(krb5_context context, struct conn_state *conn, */ e = get_so_error(conn->fd); if (e) { - TRACE_SENDTO_KDC_TCP_ERROR_CONNECT(context, conn, e); + TRACE_SENDTO_KDC_TCP_ERROR_CONNECT(context, &conn->addr, e); dprint("socket error on write fd: %m", e); goto kill_conn; } @@ -1012,12 +1015,12 @@ service_tcp_fd(krb5_context context, struct conn_state *conn, ((conn->x.out.sg_count == 2 ? SG_LEN(&conn->x.out.sgp[1]) : 0) + SG_LEN(&conn->x.out.sgp[0])), conn->fd); - TRACE_SENDTO_KDC_TCP_SEND(context, conn); + TRACE_SENDTO_KDC_TCP_SEND(context, &conn->addr); nwritten = SOCKET_WRITEV(conn->fd, conn->x.out.sgp, conn->x.out.sg_count, tmp); if (nwritten < 0) { e = SOCKET_ERRNO; - TRACE_SENDTO_KDC_TCP_ERROR_SEND(context, conn, e); + TRACE_SENDTO_KDC_TCP_ERROR_SEND(context, &conn->addr, e); dprint("failed: %m\n", e); goto kill_conn; } @@ -1072,7 +1075,7 @@ service_tcp_fd(krb5_context context, struct conn_state *conn, e = nread ? SOCKET_ERRNO : ECONNRESET; free(conn->x.in.buf); conn->x.in.buf = 0; - TRACE_SENDTO_KDC_TCP_ERROR_RECV(context, conn, e); + TRACE_SENDTO_KDC_TCP_ERROR_RECV(context, &conn->addr, e); goto kill_conn; } conn->x.in.n_left -= nread; @@ -1087,7 +1090,7 @@ service_tcp_fd(krb5_context context, struct conn_state *conn, conn->x.in.bufsizebytes + conn->x.in.bufsizebytes_read, 4 - conn->x.in.bufsizebytes_read); if (nread < 0) { - TRACE_SENDTO_KDC_TCP_ERROR_RECV_LEN(context, conn, e); + TRACE_SENDTO_KDC_TCP_ERROR_RECV_LEN(context, &conn->addr, e); e = SOCKET_ERRNO; goto kill_conn; } @@ -1132,7 +1135,7 @@ service_udp_fd(krb5_context context, struct conn_state *conn, nread = recv(conn->fd, conn->x.in.buf, conn->x.in.bufsize, 0); if (nread < 0) { - TRACE_SENDTO_KDC_UDP_ERROR_RECV(context, conn, SOCKET_ERRNO); + TRACE_SENDTO_KDC_UDP_ERROR_RECV(context, &conn->addr, SOCKET_ERRNO); kill_conn(conn, selstate, SOCKET_ERRNO); return 0; } @@ -1271,7 +1274,7 @@ k5_sendto(krb5_context context, const krb5_data *message, goto cleanup; for (state = *tailptr; state != NULL && !done; state = state->next) { /* Contact each new connection whose socktype matches socktype1. */ - if (state->socktype != socktype1) + if (state->addr.type != socktype1) continue; if (maybe_send(context, state, sel_state, callback_info)) continue; @@ -1283,7 +1286,7 @@ k5_sendto(krb5_context context, const krb5_data *message, /* Complete the first pass by contacting servers of the non-preferred * socktype (if given), waiting 1s for an answer from each. */ for (state = conns; state != NULL && !done; state = state->next) { - if (state->socktype != socktype2) + if (state->addr.type != socktype2) continue; if (maybe_send(context, state, sel_state, callback_info)) continue; @@ -1323,7 +1326,7 @@ k5_sendto(krb5_context context, const krb5_data *message, goto cleanup; } /* Success! */ - TRACE_SENDTO_KDC_RESPONSE(context, winner); + TRACE_SENDTO_KDC_RESPONSE(context, &winner->addr); reply->data = winner->x.in.buf; reply->length = winner->x.in.pos - winner->x.in.buf; retval = 0; diff --git a/src/lib/krb5/os/t_trace.c b/src/lib/krb5/os/t_trace.c index 746dbea03f..ed53181bae 100644 --- a/src/lib/krb5/os/t_trace.c +++ b/src/lib/krb5/os/t_trace.c @@ -61,7 +61,7 @@ main (int argc, char *argv[]) char *str = "example.data"; krb5_octet *oct = (krb5_octet *) str; unsigned int oct_length = strlen(str); - struct conn_state conn; + struct remote_address ra; struct sockaddr_in *addr_in; krb5_data data; struct krb5_key_st key; @@ -112,26 +112,26 @@ main (int argc, char *argv[]) TRACE(ctx, "size_t and const char *, as four-character hex hash: " "{hashlenstr}", 1, NULL); - conn.socktype = SOCK_STREAM; - addr_in = (struct sockaddr_in *) &conn.addr; + ra.type = SOCK_STREAM; + addr_in = (struct sockaddr_in *)&ra.saddr; addr_in->sin_family = AF_INET; addr_in->sin_addr.s_addr = INADDR_ANY; addr_in->sin_port = htons(88); - conn.addrlen = sizeof(struct sockaddr_in); - conn.family = AF_INET; - TRACE(ctx, "struct conn_state *, show socket type, address, port: " - "{connstate}", &conn); - conn.socktype = SOCK_DGRAM; - TRACE(ctx, "struct conn_state *, show socket type, address, port: " - "{connstate}", &conn); - conn.socktype = 1234; + ra.len = sizeof(struct sockaddr_in); + ra.family = AF_INET; + TRACE(ctx, "struct remote_address *, show socket type, address, port: " + "{raddr}", &ra); + ra.type = SOCK_DGRAM; + TRACE(ctx, "struct remote_address *, show socket type, address, port: " + "{raddr}", &ra); + ra.type = 1234; addr_in->sin_family = AF_UNSPEC; - conn.family = AF_UNSPEC; - TRACE(ctx, "struct conn_state *, show socket type, address, port: " - "{connstate}", &conn); - conn.family = 5678; - TRACE(ctx, "struct conn_state *, show socket type, address, port: " - "{connstate}", &conn); + ra.family = AF_UNSPEC; + TRACE(ctx, "struct remote_address *, show socket type, address, port: " + "{raddr}", &ra); + ra.family = 5678; + TRACE(ctx, "struct remote_address *, show socket type, address, port: " + "{raddr}", &ra); data.magic = 0; data.length = strlen(str); diff --git a/src/lib/krb5/os/t_trace.ref b/src/lib/krb5/os/t_trace.ref index 4922b89e35..749d9c99cd 100644 --- a/src/lib/krb5/os/t_trace.ref +++ b/src/lib/krb5/os/t_trace.ref @@ -8,10 +8,10 @@ size_t and const char *, as hex bytes: 6578616D706C652E64617461 size_t and const char *, as hex bytes: (null) size_t and const char *, as four-character hex hash: 7B9A size_t and const char *, as four-character hex hash: (null) -struct conn_state *, show socket type, address, port: stream 0.0.0.0:88 -struct conn_state *, show socket type, address, port: dgram 0.0.0.0:88 -struct conn_state *, show socket type, address, port: socktype1234 AF_UNSPEC -struct conn_state *, show socket type, address, port: socktype1234 af5678 +struct remote_address *, show socket type, address, port: stream 0.0.0.0:88 +struct remote_address *, show socket type, address, port: dgram 0.0.0.0:88 +struct remote_address *, show socket type, address, port: socktype1234 AF_UNSPEC +struct remote_address *, show socket type, address, port: socktype1234 af5678 krb5_data *, display as counted string: example.data krb5_data *, display as counted string: (null) krb5_data *, display as hex bytes: 6578616D706C652E64617461 diff --git a/src/lib/krb5/os/trace.c b/src/lib/krb5/os/trace.c index 97e1c06608..fcfc70b442 100644 --- a/src/lib/krb5/os/trace.c +++ b/src/lib/krb5/os/trace.c @@ -131,7 +131,7 @@ trace_format(krb5_context context, const char *fmt, va_list ap) krb5_error_code kerr; size_t len, i; int err; - struct conn_state *cs; + struct remote_address *ra; const krb5_data *d; krb5_data data; char addrbuf[NI_MAXHOST], portbuf[NI_MAXSERV], tmpbuf[200], *str; @@ -196,22 +196,22 @@ trace_format(krb5_context context, const char *fmt, va_list ap) k5_buf_add(&buf, str); free(str); } - } else if (strcmp(tmpbuf, "connstate") == 0) { - cs = va_arg(ap, struct conn_state *); - if (cs->socktype == SOCK_DGRAM) + } else if (strcmp(tmpbuf, "raddr") == 0) { + ra = va_arg(ap, struct remote_address *); + if (ra->type == SOCK_DGRAM) k5_buf_add(&buf, "dgram"); - else if (cs->socktype == SOCK_STREAM) + else if (ra->type == SOCK_STREAM) k5_buf_add(&buf, "stream"); else - k5_buf_add_fmt(&buf, "socktype%d", cs->socktype); + k5_buf_add_fmt(&buf, "socktype%d", ra->type); - if (getnameinfo((struct sockaddr *)&cs->addr, cs->addrlen, + if (getnameinfo((struct sockaddr *)&ra->saddr, ra->len, addrbuf, sizeof(addrbuf), portbuf, sizeof(portbuf), NI_NUMERICHOST|NI_NUMERICSERV) != 0) { - if (cs->family == AF_UNSPEC) + if (ra->family == AF_UNSPEC) k5_buf_add(&buf, " AF_UNSPEC"); else - k5_buf_add_fmt(&buf, " af%d", cs->family); + k5_buf_add_fmt(&buf, " af%d", ra->family); } else k5_buf_add_fmt(&buf, " %s:%s", addrbuf, portbuf); } else if (strcmp(tmpbuf, "data") == 0) { -- cgit