Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[V0.10] Add utmp/wtmp support #3400

Merged
merged 1 commit into from
Jan 23, 2025
Merged

Conversation

mlewissmith
Copy link

@mlewissmith mlewissmith commented Jan 17, 2025

Cherry-pick 2849dce..8cea9b0

See:

Addresses issue #3399

Cherry-pick 28f175b9..2ff3edd0

See:
* PR neutrinolabs#2745 "Add utmp/wtmp support"
  neutrinolabs#2745
* Discussion neutrinolabs#2744 "utmp/wtmp/btmp/lastlog"
  neutrinolabs#2744
@mlewissmith mlewissmith changed the title Add utmp/wtmp support Add utmp/wtmp support (v0.10) Jan 17, 2025
@matt335672
Copy link
Member

matt335672 commented Jan 20, 2025

Thanks for this @mlewissmith

@metalefty - are you happy this to be backported?

If so, I'll retest before merging. The changes look fine to me.

@metalefty
Copy link
Member

@matt335672 The only thing I am concerned about is the known issue, the IP address updating issue. This may confuse users so I have negative (not strong, really) feelings about this option yet. On the other hand, the feature is opt-in so I'm in favor of this feature, as long as it's explicitly stated as experimental.

@matt335672
Copy link
Member

OK - thanks.

To be clear, about the issue, what we're saying is that when the user reconnects, the IP address isn't updated in the relevant files. Is that right?

The feature is opt-in only in that it's a compile-time option. Do you want to add run-time control to that as well? Would that help?

@mlewissmith
Copy link
Author

mlewissmith commented Jan 21, 2025

If it helps, I am aware of the IP updating issue.
As the sysadmin for many machines used for work and research it's important I know that the machines are in use remotely, less so from where.

I don't think you're asking me, but if the feature were compiled in by default and controlled by a flag in xrdp.ini that would be good

@moobyfr
Copy link
Contributor

moobyfr commented Jan 21, 2025

Hello, IIRC, there is another issue on updating the IP: the entry is set when the session start, but a reconnect on xrdp is handled differently, as sesman wasn't aware of the new IP. I didn't look further on the first implementation I've made, but it could be probably improved

@metalefty
Copy link
Member

To be clear, about the issue, what we're saying is that when the user reconnects, the IP address isn't updated in the relevant files. Is that right?

That's right.

The feature is opt-in only in that it's a compile-time option. Do you want to add run-time control to that as well? Would that help?

compile-time opt-in is fine for me.

If the IP updating issue can be resolved, let's improve it in the devel branch and then backport to v0.10.

@matt335672
Copy link
Member

@moobyfr - we're aware of that, and I intend to start updating that area soon. Thanks for the reminder though.

@mlewissmith - I've been a professional sysadmin too in the past, and I agree with your statement above.

@metalefty - suggest we merge this and update NEWS-v0.10 accordingly with the experimental warning, so that it goes in the release note for v0.10. Does that work for you?

@metalefty
Copy link
Member

Yes, that's fair enough.

@matt335672 matt335672 changed the title Add utmp/wtmp support (v0.10) [V0.10] Add utmp/wtmp support Jan 23, 2025
@matt335672 matt335672 merged commit 30a825c into neutrinolabs:v0.10 Jan 23, 2025
13 checks passed
@matt335672
Copy link
Member

Thanks for your contribution @mlewissmith

@matt335672
Copy link
Member

Following text added to the release note for v0.10.3 in the 'General Announcements' section:-

Experimental support for utmp/wtmp file is provided in this release. If you use this, be aware that these files are only updated when an xrdp session is created or destroyed. Disconnections and reconnections to the same session are not tracked. In particular:-

  • the FROM address for a client (as shown by the w command) reflects the IP address of the client at the time of creation, and not the address of the currently connected client.
  • Sessions started by the xrdp-sesrun command do not have a FROM address.

mlewissmith added a commit to mlewissmith/xrdp that referenced this pull request Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants