Re: Guacamole Prompting, Round 4,963,211
> > I think I've managed to take care of this concern - I implemented a >>> pthread_cond_timedwait() for each of the parameters that get requested, and >>> then a pthread_cond_broadcast() within the argv handler to signal the >>> completion. It's a little hard to tell if it's working properly with the >>> issues I'm having with data not being sent, but during the prompt I see the >>> nop debug messages on the server side, and the connection never times out, >>> so I think it's waiting properly. >>> >> >> > Apparently I haven't quite gotten this nailed down, yet - something with > handling the pthread condition isn't working right... > > Good news - I have prompting working in RDP connections for authentication parameters using the rdp_freerdp_authenticate() callback, along with some pthread_cond_wait() and the "required" and "argv" instructions. Next steps: clean up the code, and then possibly switch SSH and Telnet (and maybe VNC and Kubernetes, depending on how those work) over to use this method rather than the current terminal prompting that they do (assuming that's desired?). Hopefully closing in on having this ready for review, maybe even for whatever release comes after 1.1.0. -Nick
Re: Guacamole Prompting, Round 4,963,211
On Sun, Jul 7, 2019 at 4:46 PM Nick Couchman
wrote:
> Getting back to this after a while...
>
>
>> Right. You cannot have two parsers reading from the same socket, nor
>> should
>> protocol support be attempting to parse the Guacamole protocol manually.
>> To
>> handle argv, you need to provide an argv_handler to the guac_user struct.
>> That handler will be called when argv is received.
>>
>
> So, I've taken care of the issue of two parsers and am now correctly (I
> think) using the argv handler to process received argv instructions. I see
> the argv handler getting called, and it seems to process the correct
> setting name, but the data (argument value) doesn't actually seem to be
> there (empty or null). Not sure if I'm doing this incorrectly, but on the
> client side I'm doing the following:
>
> $log.debug('Sending response for parameter ' + parameter + ': ' +
> JSON.stringify(data[parameter]));
> var stream = client.createArgumentValueStream("string", parameter);
> var writer = new Guacamole.StringWriter(stream);
> writer.sendText(data[parameter]);
> writer.sendEnd();
> $log.debug('Finished sending response.');
>
> on the server side, like I said, I see the call to the argv handler that
> I've written, and it correctly choose the setting, but if I process the
> argv->buffer, it's empty??
>
Alright, scratch this - turns out my debugging was just wrong. Values are
arriving properly, so apparently something wrong with handling them...
>
> My admittedly very messy repos are here:
>
> https://github.com/necouchman/guacamole-client/tree/jira/221
> https://github.com/necouchman/guacamole-server/tree/jira/221
>
>
>
>>
>> This probably means that the use of the FreeRDP authentication callback
>> > will not work, unless
>> > there's some other way to accomplish this. I could still try to block
>> the
>> > pthread until the input is received, but this might cause other
>> problems on
>> > the client side with an unresponsive connection?
>> >
>>
>> If FreeRDP depends on this being synchronous, yes, the handler will need
>> to
>> block on some sort of pthread condition which is signalled when the
>> credentials are set. Reading of the argv instruction should happen
>> magically as long as argv_handler is properly set, as Guacamole protocol
>> parsing is handled for you by guacd and libguac in a separate thread.
>>
>> If blocking will potentially result in the connection going silent,
>> there's
>> a function for that - guac_socket_require_keep_alive():
>>
>>
>> http://guacamole.apache.org/doc/libguac/socket_8h.html#a1d8c5111a8234c7785ae23f379f7db03
>>
>> That will force the guac_socket to send a "nop" instruction roughly every
>> 5
>> seconds if nothing else is sent during that period.
>>
>
> I think I've managed to take care of this concern - I implemented a
> pthread_cond_timedwait() for each of the parameters that get requested, and
> then a pthread_cond_broadcast() within the argv handler to signal the
> completion. It's a little hard to tell if it's working properly with the
> issues I'm having with data not being sent, but during the prompt I see the
> nop debug messages on the server side, and the connection never times out,
> so I think it's waiting properly.
>
>
Apparently I haven't quite gotten this nailed down, yet - something with
handling the pthread condition isn't working right...
-Nick
>
Re: Guacamole Prompting, Round 4,963,211
On Sun, Jul 7, 2019 at 4:46 PM Nick Couchman
wrote:
> Getting back to this after a while...
>
>
>> Right. You cannot have two parsers reading from the same socket, nor
>> should
>> protocol support be attempting to parse the Guacamole protocol manually.
>> To
>> handle argv, you need to provide an argv_handler to the guac_user struct.
>> That handler will be called when argv is received.
>>
>
> So, I've taken care of the issue of two parsers and am now correctly (I
> think) using the argv handler to process received argv instructions. I see
> the argv handler getting called, and it seems to process the correct
> setting name, but the data (argument value) doesn't actually seem to be
> there (empty or null). Not sure if I'm doing this incorrectly, but on the
> client side I'm doing the following:
>
> $log.debug('Sending response for parameter ' + parameter + ': ' +
> JSON.stringify(data[parameter]));
> var stream = client.createArgumentValueStream("string", parameter);
> var writer = new Guacamole.StringWriter(stream);
> writer.sendText(data[parameter]);
> writer.sendEnd();
> $log.debug('Finished sending response.');
>
> on the server side, like I said, I see the call to the argv handler that
> I've written, and it correctly choose the setting, but if I process the
> argv->buffer, it's empty??
>
So, another update on this - first, I've done a packet capture on the
system running guacd, and I see the Base64-encoded value that I entered go
across the wire. However, adding some debugging to my
guac_rdp_argv_blob_handler method, and I'm not seeing any data actually
come into the handler - always seems to be empty? As usual, I'm sure I'm
missing something obvious...
-Nick
>
Re: Guacamole Prompting, Round 4,963,211
Getting back to this after a while...
> Right. You cannot have two parsers reading from the same socket, nor should
> protocol support be attempting to parse the Guacamole protocol manually. To
> handle argv, you need to provide an argv_handler to the guac_user struct.
> That handler will be called when argv is received.
>
So, I've taken care of the issue of two parsers and am now correctly (I
think) using the argv handler to process received argv instructions. I see
the argv handler getting called, and it seems to process the correct
setting name, but the data (argument value) doesn't actually seem to be
there (empty or null). Not sure if I'm doing this incorrectly, but on the
client side I'm doing the following:
$log.debug('Sending response for parameter ' + parameter + ': ' +
JSON.stringify(data[parameter]));
var stream = client.createArgumentValueStream("string", parameter);
var writer = new Guacamole.StringWriter(stream);
writer.sendText(data[parameter]);
writer.sendEnd();
$log.debug('Finished sending response.');
on the server side, like I said, I see the call to the argv handler that
I've written, and it correctly choose the setting, but if I process the
argv->buffer, it's empty??
My admittedly very messy repos are here:
https://github.com/necouchman/guacamole-client/tree/jira/221
https://github.com/necouchman/guacamole-server/tree/jira/221
>
> This probably means that the use of the FreeRDP authentication callback
> > will not work, unless
> > there's some other way to accomplish this. I could still try to block
> the
> > pthread until the input is received, but this might cause other problems
> on
> > the client side with an unresponsive connection?
> >
>
> If FreeRDP depends on this being synchronous, yes, the handler will need to
> block on some sort of pthread condition which is signalled when the
> credentials are set. Reading of the argv instruction should happen
> magically as long as argv_handler is properly set, as Guacamole protocol
> parsing is handled for you by guacd and libguac in a separate thread.
>
> If blocking will potentially result in the connection going silent, there's
> a function for that - guac_socket_require_keep_alive():
>
>
> http://guacamole.apache.org/doc/libguac/socket_8h.html#a1d8c5111a8234c7785ae23f379f7db03
>
> That will force the guac_socket to send a "nop" instruction roughly every 5
> seconds if nothing else is sent during that period.
>
I think I've managed to take care of this concern - I implemented a
pthread_cond_timedwait() for each of the parameters that get requested, and
then a pthread_cond_broadcast() within the argv handler to signal the
completion. It's a little hard to tell if it's working properly with the
issues I'm having with data not being sent, but during the prompt I see the
nop debug messages on the server side, and the connection never times out,
so I think it's waiting properly.
Thanks for the hints so far...getting closer... :-)
-Nick
Re: Guacamole Prompting, Round 4,963,211
On Sat, Apr 6, 2019 at 11:38 PM Nick Couchman wrote: > On Wed, Mar 13, 2019 at 1:18 PM Nick Couchman wrote: > > > > > >> Looks reasonable. Naming convention within libguac for the various > >> instruction sending functions is guac_protocol_send_OPCODE(), though, so > >> this would need to be guac_protocol_send_required(). > >> > > > > I've updated the naming conventions on these... > > > > > >> > >> Perhaps the instruction should accept a list of parameter names rather > >> than > >> just a single name? This is what is done for the related "args" > >> instruction: > >> > >> > https://guacamole.apache.org/doc/1.0.0/libguac/protocol_8h.html#a6047d380b097ebc7d5f35b167e3419e6 > >> > >> > > Yeah, this was my next step, but just thought I should start with getting > > it working, then move on to making the code more efficient. > > > > > >> Client: > >> > > >> > > >> > https://github.com/necouchman/guacamole-client/commit/72de5595880baea46505cf4f9ad49640f16519e7 > >> > >> > >> Same here - convention is "onOPCODE", so this would need to be > >> "onrequired". > >> > > > > Fixed. > > > > > >> > > >> I think you will run into trouble manually parsing the Guacamole > protocol > >> with a guac_parser in a context where inbound instructions are already > >> being parsed received. Is this because the FreeRDP authentication > callback > >> must be synchronous? > >> > >> > > I'm not sure. On the xfreerdp side of things, this code results in a > > Password: prompt on the command line, and the xfreerdp app waits until > you > > enter something before proceeding. > > > > Yeah, looks like this is going to be problematic. Parsing the Guacamole > protocol while it is already running doesn't seem to work - if I use > guac_parser_expect(), it fails because there are other things going on in > the background for the connection and the argv opcode isn't the first thing > through, so it doesn't get what it's expecting. I also tried just looping > on guac_parser_read() and looking at the opcodes as they come across, > ignoring anything but argv, but it still fails. Right. You cannot have two parsers reading from the same socket, nor should protocol support be attempting to parse the Guacamole protocol manually. To handle argv, you need to provide an argv_handler to the guac_user struct. That handler will be called when argv is received. This probably means that the use of the FreeRDP authentication callback > will not work, unless > there's some other way to accomplish this. I could still try to block the > pthread until the input is received, but this might cause other problems on > the client side with an unresponsive connection? > If FreeRDP depends on this being synchronous, yes, the handler will need to block on some sort of pthread condition which is signalled when the credentials are set. Reading of the argv instruction should happen magically as long as argv_handler is properly set, as Guacamole protocol parsing is handled for you by guacd and libguac in a separate thread. If blocking will potentially result in the connection going silent, there's a function for that - guac_socket_require_keep_alive(): http://guacamole.apache.org/doc/libguac/socket_8h.html#a1d8c5111a8234c7785ae23f379f7db03 That will force the guac_socket to send a "nop" instruction roughly every 5 seconds if nothing else is sent during that period. - Mike
Re: Guacamole Prompting, Round 4,963,211
> > >>> > >>> I think you will run into trouble manually parsing the Guacamole protocol >>> with a guac_parser in a context where inbound instructions are already >>> being parsed received. Is this because the FreeRDP authentication >>> callback >>> must be synchronous? >>> >>> >> I'm not sure. On the xfreerdp side of things, this code results in a >> Password: prompt on the command line, and the xfreerdp app waits until you >> enter something before proceeding. >> > > Yeah, looks like this is going to be problematic. Parsing the Guacamole > protocol while it is already running doesn't seem to work - if I use > guac_parser_expect(), it fails because there are other things going on in > the background for the connection and the argv opcode isn't the first thing > through, so it doesn't get what it's expecting. I also tried just looping > on guac_parser_read() and looking at the opcodes as they come across, > ignoring anything but argv, but it still fails. This probably means that > the use of the FreeRDP authentication callback will not work, unless > there's some other way to accomplish this. I could still try to block the > pthread until the input is received, but this might cause other problems on > the client side with an unresponsive connection? > > I could prompt ahead of this, before the RDP connection is actually > established, but the situations where input are going to be required aren't > necessarily predictable. For example, if the server supports either RDP or > TLS security, then the username and password do not have to be entered in > order to establish the connection, so it's possible for the connection to > go to the Windows login screen. If NLA security is in use, then the > credentials must be provided prior to the connection being established. > However, if you have authentication set to "any" (auto-detect), then it's > hard to know before actually connecting to the server what is going to be > required. I could make some assumptions about it (just ask for username > and password if they aren't provided, assuming they'll be used at some > point along the way), but maybe there are situations where people actually > don't want to provide those at all, and don't want to be prompted for it? > > Well, tried moving to the following functions (for RDP) with no joy: - rdp_freerdp_pre_connect() - Behaves identically to using it in the authenticate callback, where the prompt is sent to the client and displayed, but the connection still fails and does not wait for the prompt. - guac_rdp_handle_connection() - This gets me even less far along than putting it in one of the callbacks - now I don't even see the prompt show up on the client side, and it still fails - doesn't wait, and doesn't seem to be getting any opcode. Not sure, but maybe this is too early in the connection? - guac_rdp_client_thread() - Same as in the handle connection function - no required opcode showing up on the client side, and connection just hangs. Sorry, this is probably incredibly dense of me, but right now just trying to get one prompt working... -Nick
Re: Guacamole Prompting, Round 4,963,211
On Wed, Mar 13, 2019 at 1:18 PM Nick Couchman wrote: > > >> Looks reasonable. Naming convention within libguac for the various >> instruction sending functions is guac_protocol_send_OPCODE(), though, so >> this would need to be guac_protocol_send_required(). >> > > I've updated the naming conventions on these... > > >> >> Perhaps the instruction should accept a list of parameter names rather >> than >> just a single name? This is what is done for the related "args" >> instruction: >> >> https://guacamole.apache.org/doc/1.0.0/libguac/protocol_8h.html#a6047d380b097ebc7d5f35b167e3419e6 >> >> > Yeah, this was my next step, but just thought I should start with getting > it working, then move on to making the code more efficient. > > >> Client: >> > >> > >> https://github.com/necouchman/guacamole-client/commit/72de5595880baea46505cf4f9ad49640f16519e7 >> >> >> Same here - convention is "onOPCODE", so this would need to be >> "onrequired". >> > > Fixed. > > >> > >> I think you will run into trouble manually parsing the Guacamole protocol >> with a guac_parser in a context where inbound instructions are already >> being parsed received. Is this because the FreeRDP authentication callback >> must be synchronous? >> >> > I'm not sure. On the xfreerdp side of things, this code results in a > Password: prompt on the command line, and the xfreerdp app waits until you > enter something before proceeding. > Yeah, looks like this is going to be problematic. Parsing the Guacamole protocol while it is already running doesn't seem to work - if I use guac_parser_expect(), it fails because there are other things going on in the background for the connection and the argv opcode isn't the first thing through, so it doesn't get what it's expecting. I also tried just looping on guac_parser_read() and looking at the opcodes as they come across, ignoring anything but argv, but it still fails. This probably means that the use of the FreeRDP authentication callback will not work, unless there's some other way to accomplish this. I could still try to block the pthread until the input is received, but this might cause other problems on the client side with an unresponsive connection? I could prompt ahead of this, before the RDP connection is actually established, but the situations where input are going to be required aren't necessarily predictable. For example, if the server supports either RDP or TLS security, then the username and password do not have to be entered in order to establish the connection, so it's possible for the connection to go to the Windows login screen. If NLA security is in use, then the credentials must be provided prior to the connection being established. However, if you have authentication set to "any" (auto-detect), then it's hard to know before actually connecting to the server what is going to be required. I could make some assumptions about it (just ask for username and password if they aren't provided, assuming they'll be used at some point along the way), but maybe there are situations where people actually don't want to provide those at all, and don't want to be prompted for it? -Nick >
Re: Guacamole Prompting, Round 4,963,211
>
> >
> Looks reasonable. Naming convention within libguac for the various
> instruction sending functions is guac_protocol_send_OPCODE(), though, so
> this would need to be guac_protocol_send_required().
>
I've updated the naming conventions on these...
>
> Perhaps the instruction should accept a list of parameter names rather than
> just a single name? This is what is done for the related "args"
> instruction:
>
> https://guacamole.apache.org/doc/1.0.0/libguac/protocol_8h.html#a6047d380b097ebc7d5f35b167e3419e6
>
>
Yeah, this was my next step, but just thought I should start with getting
it working, then move on to making the code more efficient.
> Client:
> >
> >
> https://github.com/necouchman/guacamole-client/commit/72de5595880baea46505cf4f9ad49640f16519e7
>
>
> Same here - convention is "onOPCODE", so this would need to be
> "onrequired".
>
Fixed.
> >
> I think you will run into trouble manually parsing the Guacamole protocol
> with a guac_parser in a context where inbound instructions are already
> being parsed received. Is this because the FreeRDP authentication callback
> must be synchronous?
>
>
I'm not sure. On the xfreerdp side of things, this code results in a
Password: prompt on the command line, and the xfreerdp app waits until you
enter something before proceeding.
The piece that sends back the requests for information to the client
appears to be working, it's the whole stopping and waiting piece that isn't
quite there. In particular, with my debugging code, on the client-side I
get the following message:
>>>PROMPT<<< Selected the following field: {
"name": "username",
"type": "USERNAME"
}
So, it's seeing the need for a username and sending it back to the client.
Unfortunately, for whatever reason, it doesn't actually wait for the client
to respond - I just immediately get the message:
> The remote desktop server is currently unreachable. If the problem
persists, please notify your system administrator, or check your system
logs.
with the reconnect in 15 seconds dialog, and the option to go home, etc.
> Things will need to be changed to allow normal handling of received
> instructions, with the response to your prompt being received via two argv
> streams, handled with user->argv_handler, etc. This may already be doable
> if the FreeRDP aspects of the connection are in a separate thread (I think
> they are), as this function could (1) send the request for credentials and
> (2) block on a pthread conditional waiting for those credentials to be
> received.
>
Ah, okay, is there anything special to do to block the pthread?
>
> What error are you seeing specifically?
>
>
On the client-side I get the message I posted above - the server is
unreachable, reconnecting, etc. On the server-side, I get the following:
Mar 13 13:15:16 guachost1 guacd[51397]: User
"@75e645b0-6d05-454d-bbdc-e95a772009b6" joined connection
"$93d136f6-c210-443f-aa78-2fd1b92a9832" (1 users now present)
Mar 13 13:15:16 guachost1 guacd[51397]: Loading keymap "base"
Mar 13 13:15:16 guachost1 guacd[51397]: Loading keymap "en-us-qwerty"
Mar 13 13:15:16 guachost1 guacd[51397]: Unable to get username from client.
Mar 13 13:15:16 guachost1 guacd[51397]: Error connecting to RDP server
Mar 13 13:15:16 guachost1 guacd[51397]: User
"@75e645b0-6d05-454d-bbdc-e95a772009b6" disconnected (0 users remain)
Mar 13 13:15:16 guachost1 guacd[51397]: Last user of connection
"$93d136f6-c210-443f-aa78-2fd1b92a9832" disconnected
Mar 13 13:15:16 guachost1 guacd[51397]: Requesting termination of client...
Mar 13 13:15:16 guachost1 guacd[51397]: Client terminated successfully.
Mar 13 13:15:16 guachost1 guacd[46005]: Connection
"$93d136f6-c210-443f-aa78-2fd1b92a9832" removed.
So, you're probably right - it's probably either not blocking, or the other
protocol messages going back-and-forth during this time are triggering it
to continue, and it continues not having received the information it was
expecting??
-Nick
Re: Guacamole Prompting, Round 4,963,211
On Thu, Mar 7, 2019 at 1:13 PM Nick Couchman wrote: > Well, I'm taking my next stab, of who knows how many, at getting prompting > to work in Guacamole, but going the route that Mike suggested on the pull > request and using the protocol to request the parameters rather than > tokens. I'm determined to do this :-). > > Nice! :) > I'm running into some challenges with this that I think may be related to > handling the usually-asynchronous nature of JavaScript and the connection > protocol, but needing to temporarily make the connection pause to wait for > the input. I've implemented the protocol-level changes for this - I'd > appreciate some sanity check on my code, but those items were pretty easy. > See the following commits: > > Server: > > https://github.com/necouchman/guacamole-server/commit/c3bf085076344951621a0170b205d51e387f7cd6 > > Looks reasonable. Naming convention within libguac for the various instruction sending functions is guac_protocol_send_OPCODE(), though, so this would need to be guac_protocol_send_required(). Perhaps the instruction should accept a list of parameter names rather than just a single name? This is what is done for the related "args" instruction: https://guacamole.apache.org/doc/1.0.0/libguac/protocol_8h.html#a6047d380b097ebc7d5f35b167e3419e6 Client: > > https://github.com/necouchman/guacamole-client/commit/72de5595880baea46505cf4f9ad49640f16519e7 Same here - convention is "onOPCODE", so this would need to be "onrequired". Easy enough - hopefully those look sane? I've done a little more building > out for RDP connections, for both server and client. On the server-side, > there's actually a callback function that is invoked by FreeRDP when > authentication fails. I'm using this as a way to send the prompts back to > the client: > > > https://github.com/necouchman/guacamole-server/blob/cfe68a796eb58b1aaecf82bbd01fe49e685f3846/src/protocols/rdp/rdp.c#L445-L501 > > I think you will run into trouble manually parsing the Guacamole protocol with a guac_parser in a context where inbound instructions are already being parsed received. Is this because the FreeRDP authentication callback must be synchronous? Things will need to be changed to allow normal handling of received instructions, with the response to your prompt being received via two argv streams, handled with user->argv_handler, etc. This may already be doable if the FreeRDP aspects of the connection are in a separate thread (I think they are), as this function could (1) send the request for credentials and (2) block on a pthread conditional waiting for those credentials to be received. This seems to be working, fine - I see the prompt come through on the > client-side when implementing a simple GuacClient.onprompt() function that > just logs the request. However, I don't think the connection is pausing > and waiting for the response - rather than getting a prompt on the client > side, I get a notification that the connection failed. Relevant client > code is here: > > > https://github.com/necouchman/guacamole-client/blob/23c784e9d812ba3fd3ee0dd352f3b75e7f422381/guacamole/src/main/webapp/app/client/types/ManagedClient.js#L517-L581 What error are you seeing specifically? - Mike
Guacamole Prompting, Round 4,963,211
Well, I'm taking my next stab, of who knows how many, at getting prompting to work in Guacamole, but going the route that Mike suggested on the pull request and using the protocol to request the parameters rather than tokens. I'm determined to do this :-). I'm running into some challenges with this that I think may be related to handling the usually-asynchronous nature of JavaScript and the connection protocol, but needing to temporarily make the connection pause to wait for the input. I've implemented the protocol-level changes for this - I'd appreciate some sanity check on my code, but those items were pretty easy. See the following commits: Server: https://github.com/necouchman/guacamole-server/commit/c3bf085076344951621a0170b205d51e387f7cd6 Client: https://github.com/necouchman/guacamole-client/commit/72de5595880baea46505cf4f9ad49640f16519e7 Easy enough - hopefully those look sane? I've done a little more building out for RDP connections, for both server and client. On the server-side, there's actually a callback function that is invoked by FreeRDP when authentication fails. I'm using this as a way to send the prompts back to the client: https://github.com/necouchman/guacamole-server/blob/cfe68a796eb58b1aaecf82bbd01fe49e685f3846/src/protocols/rdp/rdp.c#L445-L501 This seems to be working, fine - I see the prompt come through on the client-side when implementing a simple GuacClient.onprompt() function that just logs the request. However, I don't think the connection is pausing and waiting for the response - rather than getting a prompt on the client side, I get a notification that the connection failed. Relevant client code is here: https://github.com/necouchman/guacamole-client/blob/23c784e9d812ba3fd3ee0dd352f3b75e7f422381/guacamole/src/main/webapp/app/client/types/ManagedClient.js#L517-L581 Obviously the code is pretty rough right now, but any feedback would be appreciated, on whether I'm headed the right direction or not, or what I might need to do, either in this onprompt() method, or on the server-side code, to pause the connection while I wait for the prompt. The links to the branches I'm working out of are here - lots of extra commits at the moment while I try things out, so excuse that mess... https://github.com/necouchman/guacamole-server/tree/jira/221 https://github.com/necouchman/guacamole-client/tree/jira/221 Thanks - Nick
