Re: Force rdp drive_path to something like /basepath/$user_id via guacd

2020-05-29 Thread Lorenzo Faleschini
At the end we've managed (had to ask to a better-coder-colleague)  to
forcefully create a random path for the drive so the user cannot set
his/hers path in the guacamole WebUI.

for every session a disposable folder is created like
/mnt/drive_path/1ebbeaf-a96f-4677-80

and we clear it from time to time via crontab (example for /etc/crontab to
nuke folders older than 1 week)
4 42 * * * root find /mnt/drive_path/* -type d -ctime +7 |  xargs -I {} rm
-rf {} > /dev/null 2>&1


then edited "src/protocols/rdp/settings.c"
___

guac_rdp_settings* guac_rdp_parse_args(guac_user* user,
int argc, const char** argv) {


stuff

/* Force drive path to avoid filesystem lookups */

   char *usn = (char *)malloc(20);
   memcpy(usn, user->user_id+2, 20);
   char *drvpath=(char *) malloc(1+36);
   strcpy(drvpath, "/mnt/drive_path/");
   strcat(drvpath,usn);

   settings->drive_path =
guac_user_parse_args_string(user, GUAC_RDP_CLIENT_ARGS, argv,
   IDX_DRIVE_PATH, drvpath);

   settings->drive_path = drvpath;


___other_stuff_


 /* Free drive_path string */
free(drvpath);

}
___

Compiled and worked nicely.

Leaving this here since someone may find it useful.
(or some developer can implement "auto-generate-random-drive-path" feature)

cheers!


Il giorno mer 27 mag 2020 alle ore 22:29 Nick Couchman 
ha scritto:

> On Wed, May 27, 2020 at 12:54 PM Lorenzo Faleschini 
> wrote:
>
>> Hi everyone.
>>
>> I've edited the sources of guacamole-server-1.1.0 to fit my needs, for
>> example: enable rdp drive, create drive, use NLA by default.
>> I've edited "src/protocols/rdp/settings.c" file and changed the needed
>> values (as here from 0 to 1)
>> -
>>   /* Drive enable/disable */
>> settings->drive_enabled =
>> guac_user_parse_args_boolean(user, GUAC_RDP_CLIENT_ARGS, argv,
>> IDX_ENABLE_DRIVE, 1);
>> -
>>
>> configured, maked, installed, restarted guacd --> all fine.
>>
>> Then I'm stucked at this point: since I let users create their
>> connections and they need to use drives, I don't want to let them specify
>> the path of the rdp drive. What I want is to have guacd to set the correct
>> path for everyone like if they diligently type in
>> "/correct/base/path/${GUAC_USERNAME}" in the Connection editor.
>>
>>
> Yeah, if you are letting users create their own connections, then they
> will be able to define the parameters however they wish.  If guacd is
> running under a non-root account you should be able to make sure that
> filesystem permissions are set such that, no matter what users define, they
> can only write to a certain set of directories.
>
>
>> I've found a way that works to hardcode a path, but I can't figure out
>> how to dynamically compose the string:
>> 
>> settings->drive_path =
>> guac_user_parse_args_string(user, GUAC_RDP_CLIENT_ARGS, argv,
>>IDX_DRIVE_PATH, "");
>>
>> /* Force drive path to avoid users setting what they like or sneak in
>> other's users dirs*/
>> settings->drive_path = "/mnt/drive_path/${GUAC_USERNAME}";
>> 
>>
>> this works in the sense that whatever a user sets in the connection the
>> path is overwritten, but the variable is not parsed so I get all users in
>> /mnt/drive_path/\$\{GUAC_USERNAME\}/ folder in the filesystem (marked the \
>> escape chars to say that's what the folder is called, it's not
>> myuser@mydomain.whatever subfolder under /mnt/drive_path).
>>
>>
> The problem, here, is the ${GUAC_USERNAME}, the token for the username, is
> evaluated on the Guacamole Client side, by the Java application, and not
> within guacd.  So, if you use that substitution within a connection
> parameter, by the time the parameter gets passed through to guacd the
> substitution has already been made.  guacd has no knowledge of the user
> accounts used to access Guacamole Client, so it has no way of either
> substituting these items in, nor enforcing limits for where users can point
> this directory.
>
> Your best alternative in this case is to define your static top-level
> directory (/mnt/drive_path) and then append the username value
> (settings->username) to this to get the drive path.  This *should* ensure
> that they cannot enter funny values in the username box to try to escape
> the directory or get access to other user's directories, because any
> attempt to do so would also mean they are never logged on to the remote
> system, and thus wouldn't ever gain access.
>
> However, I would caution that the situation you've described is not really
> fundamentally secure - if you don't trust the users to configure any/all
> options, you shouldn't allow them to create connections at all.  If you
> trust the users to create connections, then you should trust them to define
> the correct values for any/all of these options.  At this point in time
> Guacamole does not contain any in-between permissions sets that would a

Re: Force rdp drive_path to something like /basepath/$user_id via guacd

2020-05-27 Thread Nick Couchman
On Wed, May 27, 2020 at 12:54 PM Lorenzo Faleschini 
wrote:

> Hi everyone.
>
> I've edited the sources of guacamole-server-1.1.0 to fit my needs, for
> example: enable rdp drive, create drive, use NLA by default.
> I've edited "src/protocols/rdp/settings.c" file and changed the needed
> values (as here from 0 to 1)
> -
>   /* Drive enable/disable */
> settings->drive_enabled =
> guac_user_parse_args_boolean(user, GUAC_RDP_CLIENT_ARGS, argv,
> IDX_ENABLE_DRIVE, 1);
> -
>
> configured, maked, installed, restarted guacd --> all fine.
>
> Then I'm stucked at this point: since I let users create their connections
> and they need to use drives, I don't want to let them specify the path of
> the rdp drive. What I want is to have guacd to set the correct path for
> everyone like if they diligently type in
> "/correct/base/path/${GUAC_USERNAME}" in the Connection editor.
>
>
Yeah, if you are letting users create their own connections, then they will
be able to define the parameters however they wish.  If guacd is running
under a non-root account you should be able to make sure that filesystem
permissions are set such that, no matter what users define, they can only
write to a certain set of directories.


> I've found a way that works to hardcode a path, but I can't figure out how
> to dynamically compose the string:
> 
> settings->drive_path =
> guac_user_parse_args_string(user, GUAC_RDP_CLIENT_ARGS, argv,
>IDX_DRIVE_PATH, "");
>
> /* Force drive path to avoid users setting what they like or sneak in
> other's users dirs*/
> settings->drive_path = "/mnt/drive_path/${GUAC_USERNAME}";
> 
>
> this works in the sense that whatever a user sets in the connection the
> path is overwritten, but the variable is not parsed so I get all users in
> /mnt/drive_path/\$\{GUAC_USERNAME\}/ folder in the filesystem (marked the \
> escape chars to say that's what the folder is called, it's not
> myuser@mydomain.whatever subfolder under /mnt/drive_path).
>
>
The problem, here, is the ${GUAC_USERNAME}, the token for the username, is
evaluated on the Guacamole Client side, by the Java application, and not
within guacd.  So, if you use that substitution within a connection
parameter, by the time the parameter gets passed through to guacd the
substitution has already been made.  guacd has no knowledge of the user
accounts used to access Guacamole Client, so it has no way of either
substituting these items in, nor enforcing limits for where users can point
this directory.

Your best alternative in this case is to define your static top-level
directory (/mnt/drive_path) and then append the username value
(settings->username) to this to get the drive path.  This *should* ensure
that they cannot enter funny values in the username box to try to escape
the directory or get access to other user's directories, because any
attempt to do so would also mean they are never logged on to the remote
system, and thus wouldn't ever gain access.

However, I would caution that the situation you've described is not really
fundamentally secure - if you don't trust the users to configure any/all
options, you shouldn't allow them to create connections at all.  If you
trust the users to create connections, then you should trust them to define
the correct values for any/all of these options.  At this point in time
Guacamole does not contain any in-between permissions sets that would allow
users to only define certain permissions or have admins lock values for
permissions.

-Nick


Force rdp drive_path to something like /basepath/$user_id via guacd

2020-05-27 Thread Lorenzo Faleschini
Hi everyone.

I've edited the sources of guacamole-server-1.1.0 to fit my needs, for
example: enable rdp drive, create drive, use NLA by default.
I've edited "src/protocols/rdp/settings.c" file and changed the needed
values (as here from 0 to 1)
-
  /* Drive enable/disable */
settings->drive_enabled =
guac_user_parse_args_boolean(user, GUAC_RDP_CLIENT_ARGS, argv,
IDX_ENABLE_DRIVE, 1);
-

configured, maked, installed, restarted guacd --> all fine.

Then I'm stucked at this point: since I let users create their connections
and they need to use drives, I don't want to let them specify the path of
the rdp drive. What I want is to have guacd to set the correct path for
everyone like if they diligently type in
"/correct/base/path/${GUAC_USERNAME}" in the Connection editor.

I've found a way that works to hardcode a path, but I can't figure out how
to dynamically compose the string:

settings->drive_path =
guac_user_parse_args_string(user, GUAC_RDP_CLIENT_ARGS, argv,
   IDX_DRIVE_PATH, "");

/* Force drive path to avoid users setting what they like or sneak in
other's users dirs*/
settings->drive_path = "/mnt/drive_path/${GUAC_USERNAME}";


this works in the sense that whatever a user sets in the connection the
path is overwritten, but the variable is not parsed so I get all users in
/mnt/drive_path/\$\{GUAC_USERNAME\}/ folder in the filesystem (marked the \
escape chars to say that's what the folder is called, it's not
myuser@mydomain.whatever subfolder under /mnt/drive_path).

I know there's a data structure in the libguac stuff that represents the
user id, fine enough to me, but I don't know how to correctly implement
this (C is not my daily bread).
https://guacamole.apache.org/doc/libguac/structguac__user.html#aad1c6a8b56d17c12eb1f1e36c4798c70


what I'd like to have is something like this (but that will compile and
work =)


/* Force drive path to avoid users setting what they like or sneak in
other's users dirs*/
settings->drive_path = "/mnt/drive_path/"guac_user::user_id;
---


Can anyone point me in the right direction please?

thanks!