[Solved]: patch for submitqc.tcz

[nick65go]

Right! I am asleep at wheel. The core "problem" for submitqc is that TC community should agree to a common UID:GID for any tcz. Let be this default tc:staff; Or else I, for example, will be uncomfortable for some tcz to modify  my /etc/passwd.

[GNUser]

Hi Rich. Please critique my latest proposal at your convenience:
http://gnuser.ddns.net/public/submitqc.tar.gz.bfe

[GNUser]

I finally understand what this was about:
http://forum.tinycorelinux.net/index.php/topic,26077.msg167960.html#msg167960

I think for vast majority of extensions (including the two affected by the above issue), the following are true:
  - system files (e.g., those in /usr/local/bin and /var) should be owned by root
  - access for non-root users is controlled by group and world permissions

In the case of the two affected extensions, the 700 permissions on the directory was a choice made by the upstream developer. Their assumption was that the relevant directory would be owned by root. Their intention was that only root should have access to the directory. This is a completely reasonable security measure that causes no breakage because every system has a root user.

At some point in the extension creation process, the extension maintainer (me ) made a mistake. System files which were owned by root somehow ended up being owned by the logged-in, non-root user on my machine (user bruno, UID 1000). Now the 700 permissions on the relevant directory is a little weird, even on my system. On someone else's system where a user with UID 1000 doesn't even exist, the 700 permissions on the directory is a disaster.

Long story short, mea culpa. If you guys want to accept the patch so that submitqc puts a bandaid when this kind of mistake is made, please go ahead. If you prefer correctness over bandaids and prefer not to include the patch, I completely understand.

[Rich]

Hi GNUser
I took a look at a few of my extensions (unsquashfs -lls ext.tcz) and most
entries came back as  tc/staff.  The one exception I found was:

Code: [Select]

drwxrwsr-x root/staff               33 2020-04-02 17:05 squashfs-root/usr/local/tce.installedI didn't set those users/permissions, so submitqc did that.

So submitqc  already  does correct some users/permissions.

... Are there rare instances where files/directories in an extension are meant to be owned by a user other than root or tc (e.g., virtual users such as lp)? ...

Maybe a  tce.installed  script should handle that?

Hi Rich. Please critique my latest proposal at your convenience:
http://gnuser.ddns.net/public/submitqc.tar.gz.bfe

It looks OK to me. Maybe some of the smarter kids could give it look
and provide some feedback. The  diff  can be found here:
https://forum.tinycorelinux.net/index.php?action=dlattach;topic=26146.0;attach=6380

The one thing I'm unsure of is after calling  check_non_tc_user()  will the script be able
to run to completion if a non tc user is running it.
I bring this up because initially I tried to use  submitqc  to correct the UIDs in eiwd.tcz. It
spit out lots of permission denied errors instead. Simply unsquashing/resquashing fixed
the UIDs and submitqc ran correctly.

[GNUser]

Hi Rich. Thank you for looking it over.

In the meantime, I found this in the wiki (here):

All extension creators please note
gutmensch provides the following set of rules:
...
All files root:root, 644 for files, 755 for executables, 755 for directories
...

So there it is. Best to just have root (UID 0), the universal user par excellence, own everything inside the squashfs.

For submitqc to change ownership from one non-root user to another non-root user would seem to contradict recommended best practice. Please just ignore my proposed patch. I'm sorry for the noise.

The one good thing from all this is that I updated the eiwd and iwd extensions for TCL14 x86_64, paying attention to follow best practices at every step. The two extensions have been tested and verified to be working. I will PM them to you.

As always, thank you very much for all your help.

[GNUser]

Here are contents of an intentionally flawed extension I created for testing purposes:

Code: [Select]

$ unsquashfs -lls flawed.tcz
Parallel unsquashfs: Using 4 processors
2 inodes (2 blocks) to write

drwxr-xr-x bruno/staff              26 2023-03-07 13:43 squashfs-root
drwxr-xr-x bruno/staff              28 2023-03-07 13:43 squashfs-root/usr
drwxr-xr-x bruno/staff              47 2023-03-07 13:44 squashfs-root/usr/local
drwxr-xr-x bruno/staff              29 2023-03-07 13:45 squashfs-root/usr/local/bin
-rwxr-xr-x bruno/staff              31 2023-03-07 13:45 squashfs-root/usr/local/bin/flawed
drwxr-xr-x bruno/staff              29 2023-03-07 13:45 squashfs-root/usr/local/tce.installed
-rwxr-xr-x bruno/staff              16 2023-03-07 13:45 squashfs-root/usr/local/tce.installed/flawed
Running repo's submitqc with --fix flag results in an extension where some issues have been fixed but not all:

Code: [Select]

$ unsquashfs -lls flawed.tcz
Parallel unsquashfs: Using 4 processors
2 inodes (2 blocks) to write

drwxr-xr-x bruno/staff              26 2023-03-07 13:43 squashfs-root
drwxr-xr-x bruno/staff              28 2023-03-07 13:43 squashfs-root/usr
drwxr-xr-x bruno/staff              47 2023-03-07 13:44 squashfs-root/usr/local
drwxr-xr-x bruno/staff              29 2023-03-07 13:45 squashfs-root/usr/local/bin
-rwxr-xr-x bruno/staff              31 2023-03-07 13:45 squashfs-root/usr/local/bin/flawed
drwxrwxr-x root/staff               29 2023-03-07 13:45 squashfs-root/usr/local/tce.installed
-rwxr-xr-x tc/staff                 16 2023-03-07 13:45 squashfs-root/usr/local/tce.installed/flawed

If I run my patched version of submitqc on the original flawed extension, it produces an extension without any of the issues:

Code: [Select]

$ unsquashfs -lls flawed.tcz
Parallel unsquashfs: Using 4 processors
2 inodes (2 blocks) to write

drwxr-xr-x tc/staff                 26 2023-03-07 13:43 squashfs-root
drwxr-xr-x root/root                28 2023-03-07 13:43 squashfs-root/usr
drwxr-xr-x root/root                47 2023-03-07 13:44 squashfs-root/usr/local
drwxr-xr-x root/root                29 2023-03-07 13:45 squashfs-root/usr/local/bin
-rwxr-xr-x root/root                31 2023-03-07 13:45 squashfs-root/usr/local/bin/flawed
drwxrwxr-x root/staff               29 2023-03-07 13:45 squashfs-root/usr/local/tce.installed
-rwxrwxr-x root/staff               16 2023-03-07 13:45 squashfs-root/usr/local/tce.installed/flawed
The patch is attached, in case it (in whole or in part) is useful to the team.

P.S. Adjusting the ownership of extension's root directory to tc:staff is not strictly necessary but I want it for cosmetic reasons, if nothing else. It seems the majority of extensions in the repo (including juanito's, which serve as a gold standard for me) have it--the mount point ownerships you see in output of ls -l /tmp/tcloop correspond to the ownership of each extension's root directory.

P.P.S. I realize that I'm obviating the need for the CHANGES variable to exist, given that my patched version always rebuilds the extension. I didn't bother to remove the many occurrences of the CHANGES variable because at this point the patch is for demonstration purposes and personal use only.

[GNUser]

One option might be to add the proposed functionality to the --fix flag behavior, while keeping the script's default behavior and CHANGES variable intact.

Rich and curaga, if you like the above idea I can create a patch to achieve it. Please let me know what you think. I would love to have this helpful functionality in the official version of the script.

[Rich]

Hi GNUser
I didn't even know there was a  --fix  flag. If there are known  paths/file types
that should have certain ownerships/permissions I think the default should be
to set them correctly.

[GNUser]

If there are known  paths/file types that should have certain ownerships/permissions I think the default should be to set them correctly.

I agree, but am shy about making big changes to such a venerable script as submitqc.

The patch in Reply #20 can be used as-is or I can polish it further (e.g., get rid of CHANGES variable if we are always going to rebuild the squashfs). I'd be interested in curaga's thoughts as well. Then I can try to create a patch that makes everyone happy.

P.S. Another other odd thing about current submitqc behavior is that it puts tc:staff ownership on startup scripts. The wiki clearly states that root:staff is the preferred ownership for startup scripts (see here).

[GNUser]

Hi Rich. I polished my patch and would like to propose that the attached version be merged.

With patch, submitqc fixes common ownership/permission mistakes by default. Current version in repo fails to fix these issues.

[curaga]

I think the changes are a good idea, but I should mention I don't really deal with this script.

[GNUser]

Thanks for the feedback, curaga.
Since it seems we have consensus, I will submit updated submitqc.tcz extension to Rich via PM.
Thank you both. Topic may be marked as solved.

[Rich]

Hi GNUser

... Topic may be marked as solved.

Done. 

[GNUser]

Thank you

[GNUser]

I finally understand the root issue here (no pun intended).

Code: [Select]

sudo make DESTDIR=/foo/bar installThe above results in root ownership of files inside /foo/bar. All is well because files inside extensions should be owned by root.

Contributor creates baz.tcz from /foo/bar and root ownership is preserved inside the squashfs. All is still well.

But the moment contributor runs unsquashfs baz.tcz (to check something, add a file, or whatever), ownership of everything inside the squashfs-root directory is changed to tc:staff (or bruno:staff in my case). Running 2020 version of submitqc afterwards doesn't fix the non-root ownership once it has crept in.