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

Reply via email to