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

upstream fix for LP #2040924 #198

Closed
wants to merge 2 commits into from
Closed

upstream fix for LP #2040924 #198

wants to merge 2 commits into from

Conversation

Perfect5th
Copy link
Contributor

@Perfect5th Perfect5th commented Nov 25, 2023

/var/lib/landscape-sysinfo.cache to prevent permissions errors when
executing update-motd.
@Perfect5th Perfect5th changed the title upstream fix for 2040924 upstream fix for LP #2040924 Nov 25, 2023

touch $STAMP_LOCATION
chown landscape:root $STAMP_LOCATION
chmod 666 $STAMP_LOCATION
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why perms 666? That's world writable. Didn't you mean 0644?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file is written (or really, replaced) when motd is executed. I'd assumed it would need to be world writable for that to happen. Is that assumption incorrect?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The /etc/update-motd.d/50-landscape-sysinfo script runs as root at login time. Maybe the original bug was misdiagnosed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original bug was related to the show-motd package, which I think is intended to be executed by non-root users as well, but that's just my interpretation based on the bug report and the description of the package (sic):

 This package installs a script in /etc/profile.d that dynamically
 generates and shows a message-of-the-day in inteactive shells by
 running scripts installed in /etc/update-motd.d.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then why did it start happening just now? Something else changed. But creating a world writable file in /var/lib is not the solution. The way it is now, any user can write to it, and that will be displayed on login.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why it would suddenly have changed, but I agree with you that permissions would be better off as 0644 here. Changing it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to understand what is going on, and why this fix or change is even needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a comment to the original bug: https://bugs.launchpad.net/ubuntu/+source/landscape-client/+bug/2040924/comments/2
If I had the time, I'd go digging myself for the root cause.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a revert of this change and a fix to https://code.launchpad.net/~mitchburton/ubuntu/+source/landscape-client/+git/landscape-client/+merge/454434

Once that's past review, I will contribute the resulting fix here as well.

@Perfect5th
Copy link
Contributor Author

Closing as this was fixed in #201 and #219

@Perfect5th Perfect5th closed this Mar 1, 2024
@Perfect5th Perfect5th deleted the 2040924-sysinfo-cache-perms branch March 1, 2024 20:56
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.

2 participants