On 1/21/20 1:53 PM, Fabian Grünbichler wrote: > Signed-off-by: Fabian Grünbichler <f.gruenbich...@proxmox.com> > --- > > Notes: > v2->v3: > - fix out-of-bounds write > - drop null-termination, instead only compare lines that have the proper > length > - don't include terminating \0 in tokenlen >
applied, with resolving the trivial merge due to Stefans new cpu-model.conf which is also observed, I moved your up to the other "priv/" ones. Also squashed 2/2 into this one, as it belongs to the pmxcfs perl binding part it does not warrant its own patch, IMO. Did also a small followup to mark two pointer variables pointed data as const (see inline). Thanks! > data/src/cfs-ipc-ops.h | 2 ++ > data/src/server.c | 55 ++++++++++++++++++++++++++++++++++++++++++ > data/src/status.c | 1 + > data/PVE/Cluster.pm | 18 ++++++++++++++ > 4 files changed, 76 insertions(+) > > diff --git a/data/src/cfs-ipc-ops.h b/data/src/cfs-ipc-ops.h > index e4026ad..9d788bc 100644 > --- a/data/src/cfs-ipc-ops.h > +++ b/data/src/cfs-ipc-ops.h > @@ -41,4 +41,6 @@ > > #define CFS_IPC_GET_GUEST_CONFIG_PROPERTY 11 > > +#define CFS_IPC_VERIFY_TOKEN 12 > + > #endif > diff --git a/data/src/server.c b/data/src/server.c > index 36acc1d..8519001 100644 > --- a/data/src/server.c > +++ b/data/src/server.c > @@ -89,6 +89,11 @@ typedef struct { > char property[]; > } cfs_guest_config_propery_get_request_header_t; > > +typedef struct { > + struct qb_ipc_request_header req_header; > + char token[]; > +} cfs_verify_token_request_header_t; > + > struct s1_context { > int32_t client_pid; > uid_t uid; > @@ -343,6 +348,56 @@ static int32_t s1_msg_process_fn( > > result = cfs_create_guest_conf_property_msg(outbuf, > memdb, rh->property, rh->vmid); > } > + } else if (request_id == CFS_IPC_VERIFY_TOKEN) { > + > + cfs_verify_token_request_header_t *rh = > (cfs_verify_token_request_header_t *) data; > + int tokenlen = request_size - > G_STRUCT_OFFSET(cfs_verify_token_request_header_t, token) - 1; > + > + if (tokenlen <= 0) { > + cfs_debug("tokenlen <= 0, %d", tokenlen); > + result = -EINVAL; > + } else if (memchr(rh->token, '\n', tokenlen) != NULL) { > + cfs_debug("token contains newline"); > + result = -EINVAL; > + } else if (rh->token[tokenlen] != '\0') { > + cfs_debug("token not NULL-terminated"); > + result = -EINVAL; > + } else if (strnlen(rh->token, tokenlen) != tokenlen) { > + cfs_debug("token contains NULL-byte"); > + result = -EINVAL; > + } else { > + cfs_debug("cfs_verify_token: basic validity checked, > reading token.cfg"); > + gpointer tmp = NULL; > + int bytes_read = memdb_read(memdb, "priv/token.cfg", > &tmp); > + size_t remaining = bytes_read > 0 ? bytes_read : 0; > + if (tmp != NULL && remaining >= tokenlen) { > + char *line = (char *) tmp; const char *line ... > + char *next_line; const char *next_line .. Both are only for pointer tracking, but should not allow to change the data they point to, makes things slightly easier to reason about (for compiler and humans :) > + const char *const end = line + remaining; > + size_t linelen; > + > + while (line != NULL) { > + next_line = memchr(line, '\n', > remaining); > + linelen = next_line == NULL ? remaining > : next_line - line; > + if (linelen == tokenlen && > strncmp(line, rh->token, linelen) == 0) { > + result = 0; > + break; > + } > + line = next_line; > + if (line != NULL) { > + line += 1; > + remaining = end - line; > + } > + } > + if (line == NULL) { > + result = -ENOENT; > + } > + g_free(tmp); > + } else { > + cfs_debug("token: token.cfg does not exist - > ENOENT"); > + result = -ENOENT; > + } > + } > } > > cfs_debug("process result %d", result); > diff --git a/data/src/status.c b/data/src/status.c > index d6dccc1..2dde874 100644 > --- a/data/src/status.c > +++ b/data/src/status.c > @@ -100,6 +100,7 @@ static memdb_change_t memdb_change_array[] = { > { .path = "sdn/zones.cfg.new" }, > { .path = "sdn/controllers.cfg" }, > { .path = "sdn/controllers.cfg.new" }, > + { .path = "priv/token.cfg" }, > }; > > static GMutex mutex; > diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm > index b4fac03..b329b23 100644 > --- a/data/PVE/Cluster.pm > +++ b/data/PVE/Cluster.pm > @@ -187,6 +187,18 @@ my $ipcc_get_cluster_log = sub { > return &$ipcc_send_rec(CFS_IPC_GET_CLUSTER_LOG, $bindata); > }; > > +my $ipcc_verify_token = sub { > + my ($full_token) = @_; > + > + my $bindata = pack "Z*", $full_token; > + my $res = PVE::IPCC::ipcc_send_rec(CFS_IPC_VERIFY_TOKEN, $bindata); > + > + return 1 if $! == 0; > + return 0 if $! == ENOENT; > + > + die "$!\n"; > +}; > + > my $ccache = {}; > > sub cfs_update { > @@ -447,6 +459,12 @@ sub get_cluster_log { > return &$ipcc_get_cluster_log($user, $max); > } > > +sub verify_token { > + my ($userid, $token) = @_; > + > + return &$ipcc_verify_token("$userid $token"); > +} > + > my $file_info = {}; > > sub cfs_register_file { > _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel