neutrinordp: Log the IP address, port, and user name of the NeutrioRDP Proxy connection.

[TOMATO-ONE]

#1873
I made sure to log the IP address, port, and user name of the NeutrioRDP Proxy connection.

The following will be recorded in xrdp.log.
[20210503-17:07:11] [INFO ] NeutrinoRDP: connection [Success], destination [192.168.102.101:3389], username [user], source [192.168.0.212:62118], pamusername [xrdp], pid [7443]

I don't know the C language very well.
I made the patch as an example.
If you have better code, please let me know.

I would like to log the disconnect as well.
Where should I add the code in xrdp-neutrinordp.c?
Please advise.

Best regards

[matt335672]

Hi @TOMATO-ONE

Firstly, thanks very much for taking the time to contribute to xrdp.

I'll add some review comments to your code shortly. Here are a couple of other things:-

• We have a coding style on the wiki. Please have a look at it. In particular, we use spaces for indents rather than tabs.
• You can log a disconnect in the mod_exit() function. This will be called whether or not the connect was successful. I suggest you add some logging there and see what happens. Come back to me if you have any questions.

[matt335672]

Comments below.

At the moment logging the pid doesn't gain much, but if you get the disconnect logging added, the two messages can be linked by the administrator

[aquesnel]

Hi @TOMATO-ONE

Thanks for contributing this patch.

For your question about where to log the disconnection, you can try adding it to lxrdp_end which is always called or you can see if there is a disconnect event that is passed to lxrdp_event.

Also, since the point of this log message is to be able to correlate the connection and disconnection events, I'd suggest extracting the log message to a helper function to guarantee that all of the places (connect, failure, disconnect) that log this message log it consistently.

[matt335672]

I think either lxrdp_end() or mod_exit() can be used for logging the disconnect. @aquesnel - can you see any reason to prefer one or the other?

I think it makes sense to log pamusername here. All the information is available from other logs, but this gathers it all in one place for the proxy case. I can't see a better way to do this with the current architecture.

[aquesnel]

@matt335672 to me it seems like the intent of the function lxrdp_end() is for cleaning up at the end of the connection and mod_exit() is when the module is released. That's the only reason I can see for preferring lxrdp_end()

[matt335672]

@aquesnel - I'll go with that, particularly as the connect happens in lxrdp_connect() rather than mod_init().

[TOMATO-ONE]

@aquesnel @matt335672
It is true that pamusername is not described in man xrdp.ini(5).
I am not good at English, so I am not confident to change the document.
Instead, I suggest adding a comment to xrdp.ini.in.
86c5674
If you have a better text, please let me know.

[matt335672]

Hi @TOMATO-ONE - some more comments for you

[matt335672]

Remove the delay_ms line from here as it only applies to VNC connections (see b66aafc)

[TOMATO-ONE]

Fixed in 1f09ad9.

[TOMATO-ONE]

The reason why the LOG in neutorinordp.c was not shown in DEBUG in the disconnect log was simple: it was a binary that had not been built with the configure option --enable-xrdpdebug.

Sorry about that.

I'm checking the log again.
I have removed the wrong information as it would have caused confusion in hindsight.
I will update the PR again when I make a proposal to output the disconnection log.

[matt335672]

Hi @TOMATO-ONE

The --enable-xrdpdebug option is meant for developers only, so it won't be enabled by any distros. Your logging is intended for sysadmins rather than developers, and so I think it should work whether or not --enable-xrdpdebug is specified.

Thanks for your efforts so far.

[TOMATO-ONE]

Thank you. @matt335672

I was confused.
LOG() is the function that logs even a normal binary, and
I see that LOG_DEBUG() is the function that requires --enable-xrdpdebug.

Since the lxrdp_end: by LOG_DEBUG() in lxrdp_end() is not logged in normal binary, the
I misunderstood that lxrdp_end() is not being used.

When I used the --enable-xrdpdebug binary, lxrdp_end: was logged when the RDP connection was closed.
The misunderstanding was cleared up.

I followed @aquesnel's advice and used the LOG() function right after lxrdp_end: in lxrdp_end() to log.
5af1373

The following log will be recorded.

[20210510-15:26:50] [INFO ] [pid:27043 tid:139924764699072] NeutrinoRDP proxy connection: status [Disconnect], RDP client [192.168.0.212:65032], RDP server [192.168.102.101:3389], RDP server username [user], xrdp pamusername [xrdp], xrdp process id [27043]

[matt335672]

Hi @TOMATO-ONE

That all looks fine to me now - I take it the logging as-is meets your needs?

Do you understand what 'squashing commits' is in git? It would be good to get a single commit for the above in the main-line repository.

[TOMATO-ONE]

Hi @matt335672
Thank you.
That satisfies my request.

I didn't know about 'squashing commits', so I learned.
I also practiced in my local environment.

Am I right in thinking that I can run the following command and squash my miscellaneous pushes together?

git rebase -i a4c7ee077c049202d73f82f518b2de958d722890

git push -f

I'm sorry to ask you such a basic question.

[matt335672]

Hi @TOMATO-ONE

That's fine - we've all been there with git! Speaking personally, I found it quite hard to get started coming from a background including Subversion and Sun Teamware but I think it's worth the trouble.

I'm assuming you've got a local repository on your machine which you're working on which you've cloned from your github copy.

That command you've quoted will work fine in this case. Because you're working on your devel branch I can't think of a better one.

As you get more familiar with git, you'll find it's normally better to work on a change in a separate branch. The reason is that then it makes merging, squashing, etc a lot easier as your changes can be identified as belonging together. Here are some references for you:-

• https://guides.github.com/introduction/flow/
• git-scm.com has a good chapter on branching and merging which is available in several languages.

Hope that helps.

[TOMATO-ONE]

Hi @matt335672

Thank you very much for your kind help.
I have done the 'squashing commits'. c6fcb16

I'm slowly learning more about git.
I will continue to use it in the future.

Thank you for teaching me about branch separation.
I'll try it in the next time I raise another PR.

[matt335672]

@TOMATO-ONE - thanks for your contribution.

[TOMATO-ONE]

Thank you very much for your help.

If you all had worked on this PR, you probably could have completed it in less than an hour.
However, as a beginner in git, English, and C programming, you patiently stuck with me for 10 days.
This was a very valuable experience for me.
Once again, thank you very much.

As an administrator of an RDP Proxy server, I am looking forward to #1338.
I will help with testing, etc.

Best regards.