| Message ID | 20220417184538.1044417-5-rostedt@goodmis.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | trace-cmd: Allow agent to use networking | expand |
On Mon, Apr 18, 2022 at 11:28 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > Add a -N option to trace-cmd agent to listen on a network socket that > can be used by trace-cmd record -A to connect with. > I have concerns about exposing the agent over the network, without any client authentication. It runs with root privileges, the kernel tracing data will be exposed to anyone over the network. At least, this should be written clearly in the documentation and even should be printed by "trace-cmd agent -N ...". I saw your patch that adds a name or IP address of the client as parameter, it is some security, but the tracing data itself still flows unencrypted over the network and could be visible to anyone. > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > tracecmd/include/trace-local.h | 6 +++ > tracecmd/trace-agent.c | 89 ++++++++++++++++++++++++++-------- > tracecmd/trace-listen.c | 58 ++++++++++++++-------- > 3 files changed, 113 insertions(+), 40 deletions(-) > > diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h > index afbcab422c2f..fdb0d887557f 100644 > --- a/tracecmd/include/trace-local.h > +++ b/tracecmd/include/trace-local.h > @@ -306,6 +306,12 @@ extern struct buffer_instance *first_instance; > #define is_guest(instance) ((instance)->flags & BUFFER_FL_GUEST) > #define is_network(instance) ((instance)->flags & BUFFER_FL_NETWORK) > > +#define START_PORT_SEARCH 1500 > +#define MAX_PORT_SEARCH 6000 > + > +int trace_net_make(int port, enum port_type type); > +int trace_net_search(int start_port, int *sfd, enum port_type type); > + > struct buffer_instance *allocate_instance(const char *name); > void add_instance(struct buffer_instance *instance, int cpu_count); > void update_first_instance(struct buffer_instance *instance, int topt); > diff --git a/tracecmd/trace-agent.c b/tracecmd/trace-agent.c > index 719d7126ca67..cd201ad948ad 100644 > --- a/tracecmd/trace-agent.c > +++ b/tracecmd/trace-agent.c > @@ -41,6 +41,32 @@ static void make_vsocks(int nr, int *fds, unsigned int *ports) > } > } > > +static void make_net(int nr, int *fds, unsigned int *ports) > +{ > + int port; > + int i, fd; > + int start_port = START_PORT_SEARCH; > + > + for (i = 0; i < nr; i++) { > + port = trace_net_search(start_port, &fd, USE_TCP); > + if (port < 0) > + die("Failed to open socket"); > + if (listen(fd, 5) < 0) > + die("Failed to listen on port %d\n", port); > + fds[i] = fd; > + ports[i] = port; > + start_port = port + 1; > + } > +} > + > +static void make_sockets(int nr, int *fds, unsigned int *ports, bool network) > +{ > + if (network) > + return make_net(nr, fds, ports); > + else > + return make_vsocks(nr, fds, ports); > +} > + > static int open_agent_fifos(int nr_cpus, int *fds) > { > char path[PATH_MAX]; > @@ -80,7 +106,7 @@ static char *get_clock(int argc, char **argv) > return NULL; > } > > -static void agent_handle(int sd, int nr_cpus, int page_size) > +static void agent_handle(int sd, int nr_cpus, int page_size, bool network) > { > struct tracecmd_tsync_protos *tsync_protos = NULL; > struct tracecmd_time_sync *tsync = NULL; > @@ -117,17 +143,31 @@ static void agent_handle(int sd, int nr_cpus, int page_size) > use_fifos = false; > > if (!use_fifos) > - make_vsocks(nr_cpus, fds, ports); > + make_sockets(nr_cpus, fds, ports, network); > if (tsync_protos && tsync_protos->names) { > - if (get_vsocket_params(msg_handle->fd, &local_id, > - &remote_id)) { > - warning("Failed to get local and remote ids"); > - /* Just make something up */ > - remote_id = -1; > - local_id = -2; > + if (network) { > + /* For now just use something */ > + remote_id = 2; > + local_id = 1; > + tsync_port = trace_net_search(START_PORT_SEARCH, &fd, USE_TCP); > + if (listen(fd, 5) < 0) > + die("Failed to listen on %d\n", tsync_port); > + } else { > + if (get_vsocket_params(msg_handle->fd, &local_id, > + &remote_id)) { > + warning("Failed to get local and remote ids"); > + /* Just make something up */ > + remote_id = -1; > + local_id = -2; > + } > + fd = trace_vsock_make_any(); > + if (fd >= 0 && > + trace_vsock_get_port(fd, &tsync_port) < 0) { > + close(fd); > + fd = -1; > + } > } > - fd = trace_vsock_make_any(); > - if (fd >= 0 && trace_vsock_get_port(fd, &tsync_port) >= 0) { > + if (fd >= 0) { > tsync = tracecmd_tsync_with_host(fd, tsync_protos, > get_clock(argc, argv), > remote_id, local_id); > @@ -193,7 +233,7 @@ static pid_t do_fork() > return fork(); > } > > -static void agent_serve(unsigned int port, bool do_daemon) > +static void agent_serve(unsigned int port, bool do_daemon, bool network) > { > int sd, cd, nr_cpus; > unsigned int cid; > @@ -204,14 +244,21 @@ static void agent_serve(unsigned int port, bool do_daemon) > nr_cpus = tracecmd_count_cpus(); > page_size = getpagesize(); > > - sd = trace_vsock_make(port); > + if (network) { > + sd = trace_net_make(port, USE_TCP); > + if (listen(sd, 5) < 0) > + die("Failed to listen on %d\n", port); > + } else > + sd = trace_vsock_make(port); > if (sd < 0) > - die("Failed to open vsocket"); > + die("Failed to open socket"); > tracecmd_tsync_init(); > > - cid = trace_vsock_local_cid(); > - if (cid >= 0) > - printf("listening on @%u:%u\n", cid, port); > + if (!network) { > + cid = trace_vsock_local_cid(); > + if (cid >= 0) > + printf("listening on @%u:%u\n", cid, port); > + } > > if (do_daemon && daemon(1, 0)) > die("daemon"); > @@ -231,7 +278,7 @@ static void agent_serve(unsigned int port, bool do_daemon) > if (pid == 0) { > close(sd); > signal(SIGCHLD, SIG_DFL); > - agent_handle(cd, nr_cpus, page_size); > + agent_handle(cd, nr_cpus, page_size, network); > } > if (pid > 0) > handler_pid = pid; > @@ -250,6 +297,7 @@ void trace_agent(int argc, char **argv) > { > bool do_daemon = false; > unsigned int port = TRACE_AGENT_DEFAULT_PORT; > + bool network = false; > > if (argc < 2) > usage(argv); > @@ -267,7 +315,7 @@ void trace_agent(int argc, char **argv) > {NULL, 0, NULL, 0} > }; > > - c = getopt_long(argc-1, argv+1, "+hp:D", > + c = getopt_long(argc-1, argv+1, "+hp:DN", > long_options, &option_index); > if (c == -1) > break; > @@ -275,6 +323,9 @@ void trace_agent(int argc, char **argv) > case 'h': > usage(argv); > break; > + case 'N': > + network = true; > + break; > case 'p': > port = atoi(optarg); > break; > @@ -296,5 +347,5 @@ void trace_agent(int argc, char **argv) > if (optind < argc-1) > usage(argv); > > - agent_serve(port, do_daemon); > + agent_serve(port, do_daemon, network); > } > diff --git a/tracecmd/trace-listen.c b/tracecmd/trace-listen.c > index 2338173fc8a6..8476fa51dadd 100644 > --- a/tracecmd/trace-listen.c > +++ b/tracecmd/trace-listen.c > @@ -234,52 +234,68 @@ static int setup_vsock_port(int start_port, int *sfd) > return start_port; > } > > -#define START_PORT_SEARCH 1500 > -#define MAX_PORT_SEARCH 6000 > - > -static int bind_a_port(int start_port, int *sfd, enum port_type type) > +int trace_net_make(int port, enum port_type type) > { > struct addrinfo hints; > struct addrinfo *result, *rp; > char buf[BUFSIZ]; > + int sd; > int s; > - int num_port = start_port; > > - if (type == USE_VSOCK) > - return setup_vsock_port(start_port, sfd); > - again: > - snprintf(buf, BUFSIZ, "%d", num_port); > + snprintf(buf, BUFSIZ, "%d", port); > > memset(&hints, 0, sizeof(hints)); > hints.ai_family = AF_UNSPEC; > - hints.ai_socktype = type == USE_TCP ? SOCK_STREAM : SOCK_DGRAM; > hints.ai_flags = AI_PASSIVE; > > + switch (type) { > + case USE_TCP: > + hints.ai_socktype = SOCK_STREAM; > + break; > + case USE_UDP: > + hints.ai_socktype = SOCK_DGRAM; > + break; > + default: > + return -1; > + } > + > s = getaddrinfo(NULL, buf, &hints, &result); > if (s != 0) > pdie("getaddrinfo: error opening socket"); > > for (rp = result; rp != NULL; rp = rp->ai_next) { > - *sfd = socket(rp->ai_family, rp->ai_socktype, > - rp->ai_protocol); > - if (*sfd < 0) > + sd = socket(rp->ai_family, rp->ai_socktype, > + rp->ai_protocol); > + if (sd < 0) > continue; > > - if (bind(*sfd, rp->ai_addr, rp->ai_addrlen) == 0) > + if (bind(sd, rp->ai_addr, rp->ai_addrlen) == 0) > break; > > - close(*sfd); > + close(sd); > } > + freeaddrinfo(result); > + > + if (rp == NULL) > + return -1; > + > + return sd; > +} > + > +int trace_net_search(int start_port, int *sfd, enum port_type type) > +{ > + int num_port = start_port; > > - if (rp == NULL) { > - freeaddrinfo(result); > + if (type == USE_VSOCK) > + return setup_vsock_port(start_port, sfd); > + again: > + *sfd = trace_net_make(num_port, type); > + if (*sfd < 0) { > if (++num_port > MAX_PORT_SEARCH) > pdie("No available ports to bind"); > goto again; > } > > - freeaddrinfo(result); > - > return num_port; > } > > @@ -309,10 +325,10 @@ static int open_port(const char *node, const char *port, int *pid, > int num_port; > > /* > - * bind_a_port() currently does not return an error, but if that > + * trace_net_search() currently does not return an error, but if that > * changes in the future, we have a check for it now. > */ > - num_port = bind_a_port(start_port, &sfd, type); > + num_port = trace_net_search(start_port, &sfd, type); > if (num_port < 0) > return num_port; > > -- > 2.35.1 > -- Tzvetomir (Ceco) Stoyanov VMware Open Source Technology Center
On Mon, 18 Apr 2022 13:22:51 +0300 Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote: > On Mon, Apr 18, 2022 at 11:28 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > > > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > > > Add a -N option to trace-cmd agent to listen on a network socket that > > can be used by trace-cmd record -A to connect with. > > > > I have concerns about exposing the agent over the network, without any > client authentication. It runs with root privileges, the kernel > tracing data will be exposed to anyone over the network. At least, > this should be written clearly in the documentation and even should be > printed by "trace-cmd agent -N ...". I saw your patch that adds a name > or IP address of the client as parameter, it is some security, but the > tracing data itself still flows unencrypted over the network and could > be visible to anyone. True. But it's similar to telnet. The use case for this option is for local networks where you expect to "trust" the network. I'm fine with adding more warnings and stating that this is "NOT SECURE" in big letters to let people know the dangers of it. I'm hoping to have more authentication in the future. I could even have it print a warning "UNSECURE CONNECTION" or something to enforce that this is not something you want to use in any scenario. -- Steve
diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h index afbcab422c2f..fdb0d887557f 100644 --- a/tracecmd/include/trace-local.h +++ b/tracecmd/include/trace-local.h @@ -306,6 +306,12 @@ extern struct buffer_instance *first_instance; #define is_guest(instance) ((instance)->flags & BUFFER_FL_GUEST) #define is_network(instance) ((instance)->flags & BUFFER_FL_NETWORK) +#define START_PORT_SEARCH 1500 +#define MAX_PORT_SEARCH 6000 + +int trace_net_make(int port, enum port_type type); +int trace_net_search(int start_port, int *sfd, enum port_type type); + struct buffer_instance *allocate_instance(const char *name); void add_instance(struct buffer_instance *instance, int cpu_count); void update_first_instance(struct buffer_instance *instance, int topt); diff --git a/tracecmd/trace-agent.c b/tracecmd/trace-agent.c index 719d7126ca67..cd201ad948ad 100644 --- a/tracecmd/trace-agent.c +++ b/tracecmd/trace-agent.c @@ -41,6 +41,32 @@ static void make_vsocks(int nr, int *fds, unsigned int *ports) } } +static void make_net(int nr, int *fds, unsigned int *ports) +{ + int port; + int i, fd; + int start_port = START_PORT_SEARCH; + + for (i = 0; i < nr; i++) { + port = trace_net_search(start_port, &fd, USE_TCP); + if (port < 0) + die("Failed to open socket"); + if (listen(fd, 5) < 0) + die("Failed to listen on port %d\n", port); + fds[i] = fd; + ports[i] = port; + start_port = port + 1; + } +} + +static void make_sockets(int nr, int *fds, unsigned int *ports, bool network) +{ + if (network) + return make_net(nr, fds, ports); + else + return make_vsocks(nr, fds, ports); +} + static int open_agent_fifos(int nr_cpus, int *fds) { char path[PATH_MAX]; @@ -80,7 +106,7 @@ static char *get_clock(int argc, char **argv) return NULL; } -static void agent_handle(int sd, int nr_cpus, int page_size) +static void agent_handle(int sd, int nr_cpus, int page_size, bool network) { struct tracecmd_tsync_protos *tsync_protos = NULL; struct tracecmd_time_sync *tsync = NULL; @@ -117,17 +143,31 @@ static void agent_handle(int sd, int nr_cpus, int page_size) use_fifos = false; if (!use_fifos) - make_vsocks(nr_cpus, fds, ports); + make_sockets(nr_cpus, fds, ports, network); if (tsync_protos && tsync_protos->names) { - if (get_vsocket_params(msg_handle->fd, &local_id, - &remote_id)) { - warning("Failed to get local and remote ids"); - /* Just make something up */ - remote_id = -1; - local_id = -2; + if (network) { + /* For now just use something */ + remote_id = 2; + local_id = 1; + tsync_port = trace_net_search(START_PORT_SEARCH, &fd, USE_TCP); + if (listen(fd, 5) < 0) + die("Failed to listen on %d\n", tsync_port); + } else { + if (get_vsocket_params(msg_handle->fd, &local_id, + &remote_id)) { + warning("Failed to get local and remote ids"); + /* Just make something up */ + remote_id = -1; + local_id = -2; + } + fd = trace_vsock_make_any(); + if (fd >= 0 && + trace_vsock_get_port(fd, &tsync_port) < 0) { + close(fd); + fd = -1; + } } - fd = trace_vsock_make_any(); - if (fd >= 0 && trace_vsock_get_port(fd, &tsync_port) >= 0) { + if (fd >= 0) { tsync = tracecmd_tsync_with_host(fd, tsync_protos, get_clock(argc, argv), remote_id, local_id); @@ -193,7 +233,7 @@ static pid_t do_fork() return fork(); } -static void agent_serve(unsigned int port, bool do_daemon) +static void agent_serve(unsigned int port, bool do_daemon, bool network) { int sd, cd, nr_cpus; unsigned int cid; @@ -204,14 +244,21 @@ static void agent_serve(unsigned int port, bool do_daemon) nr_cpus = tracecmd_count_cpus(); page_size = getpagesize(); - sd = trace_vsock_make(port); + if (network) { + sd = trace_net_make(port, USE_TCP); + if (listen(sd, 5) < 0) + die("Failed to listen on %d\n", port); + } else + sd = trace_vsock_make(port); if (sd < 0) - die("Failed to open vsocket"); + die("Failed to open socket"); tracecmd_tsync_init(); - cid = trace_vsock_local_cid(); - if (cid >= 0) - printf("listening on @%u:%u\n", cid, port); + if (!network) { + cid = trace_vsock_local_cid(); + if (cid >= 0) + printf("listening on @%u:%u\n", cid, port); + } if (do_daemon && daemon(1, 0)) die("daemon"); @@ -231,7 +278,7 @@ static void agent_serve(unsigned int port, bool do_daemon) if (pid == 0) { close(sd); signal(SIGCHLD, SIG_DFL); - agent_handle(cd, nr_cpus, page_size); + agent_handle(cd, nr_cpus, page_size, network); } if (pid > 0) handler_pid = pid; @@ -250,6 +297,7 @@ void trace_agent(int argc, char **argv) { bool do_daemon = false; unsigned int port = TRACE_AGENT_DEFAULT_PORT; + bool network = false; if (argc < 2) usage(argv); @@ -267,7 +315,7 @@ void trace_agent(int argc, char **argv) {NULL, 0, NULL, 0} }; - c = getopt_long(argc-1, argv+1, "+hp:D", + c = getopt_long(argc-1, argv+1, "+hp:DN", long_options, &option_index); if (c == -1) break; @@ -275,6 +323,9 @@ void trace_agent(int argc, char **argv) case 'h': usage(argv); break; + case 'N': + network = true; + break; case 'p': port = atoi(optarg); break; @@ -296,5 +347,5 @@ void trace_agent(int argc, char **argv) if (optind < argc-1) usage(argv); - agent_serve(port, do_daemon); + agent_serve(port, do_daemon, network); } diff --git a/tracecmd/trace-listen.c b/tracecmd/trace-listen.c index 2338173fc8a6..8476fa51dadd 100644 --- a/tracecmd/trace-listen.c +++ b/tracecmd/trace-listen.c @@ -234,52 +234,68 @@ static int setup_vsock_port(int start_port, int *sfd) return start_port; } -#define START_PORT_SEARCH 1500 -#define MAX_PORT_SEARCH 6000 - -static int bind_a_port(int start_port, int *sfd, enum port_type type) +int trace_net_make(int port, enum port_type type) { struct addrinfo hints; struct addrinfo *result, *rp; char buf[BUFSIZ]; + int sd; int s; - int num_port = start_port; - if (type == USE_VSOCK) - return setup_vsock_port(start_port, sfd); - again: - snprintf(buf, BUFSIZ, "%d", num_port); + snprintf(buf, BUFSIZ, "%d", port); memset(&hints, 0, sizeof(hints)); hints.ai_family = AF_UNSPEC; - hints.ai_socktype = type == USE_TCP ? SOCK_STREAM : SOCK_DGRAM; hints.ai_flags = AI_PASSIVE; + switch (type) { + case USE_TCP: + hints.ai_socktype = SOCK_STREAM; + break; + case USE_UDP: + hints.ai_socktype = SOCK_DGRAM; + break; + default: + return -1; + } + s = getaddrinfo(NULL, buf, &hints, &result); if (s != 0) pdie("getaddrinfo: error opening socket"); for (rp = result; rp != NULL; rp = rp->ai_next) { - *sfd = socket(rp->ai_family, rp->ai_socktype, - rp->ai_protocol); - if (*sfd < 0) + sd = socket(rp->ai_family, rp->ai_socktype, + rp->ai_protocol); + if (sd < 0) continue; - if (bind(*sfd, rp->ai_addr, rp->ai_addrlen) == 0) + if (bind(sd, rp->ai_addr, rp->ai_addrlen) == 0) break; - close(*sfd); + close(sd); } + freeaddrinfo(result); + + if (rp == NULL) + return -1; + + return sd; +} + +int trace_net_search(int start_port, int *sfd, enum port_type type) +{ + int num_port = start_port; - if (rp == NULL) { - freeaddrinfo(result); + if (type == USE_VSOCK) + return setup_vsock_port(start_port, sfd); + again: + *sfd = trace_net_make(num_port, type); + if (*sfd < 0) { if (++num_port > MAX_PORT_SEARCH) pdie("No available ports to bind"); goto again; } - freeaddrinfo(result); - return num_port; } @@ -309,10 +325,10 @@ static int open_port(const char *node, const char *port, int *pid, int num_port; /* - * bind_a_port() currently does not return an error, but if that + * trace_net_search() currently does not return an error, but if that * changes in the future, we have a check for it now. */ - num_port = bind_a_port(start_port, &sfd, type); + num_port = trace_net_search(start_port, &sfd, type); if (num_port < 0) return num_port;