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.

Sign up for GitHub

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

Jump to bottom

Ownership of Released binaries is bad in Linux causing phantom "can't read file" errors #158

Open
gdevenyi opened this issue Oct 14, 2024 · 14 comments
Open

Ownership of Released binaries is bad in Linux causing phantom "can't read file" errors #158

gdevenyi opened this issue Oct 14, 2024 · 14 comments

Comments

@gdevenyi
Copy link

Trying to update a drive's firmware:

==========================================================================================
 openSeaChest_Firmware - openSeaChest drive utilities - NVMe Enabled
 Copyright (c) 2014-2024 Seagate Technology LLC and/or its Affiliates, All Rights Reserved
 openSeaChest_Firmware Version: 4.2.0-8_0_1 X86_64
 Build Date: Sep 25 2024
 Today: 20241014T004241 User: root
==========================================================================================
Sending SCSI Test Unit Ready

  CDB:

        0  1  2  3  4  5
  0x00 00 00 00 00 00 00

SG IO Issued as Indirect IO

  Sense Data Buffer:

        0  1  2  3  4  5  6  7
  0x00 00 00 00 00 00 00 00 00


Sense Key: 0h = No Error
ASC & ASCQ: 0h - 0h = No Additional Sense Information
FRU: 0h = No Additional Information
Information: 0000000000000000h
Command Specific Information: 0000000000000000h
Command Time (ms): 196.17

Test Unit Ready returning: SUCCESS

Sending SCSI Inquiry

  CDB:

        0  1  2  3  4  5
  0x00 12 00 00 00 60 00

SG IO Issued as Indirect IO

  Sense Data Buffer:

        0  1  2  3  4  5  6  7
  0x00 00 00 00 00 00 00 00 00


Sense Key: 0h = No Error
ASC & ASCQ: 0h - 0h = No Additional Sense Information
FRU: 0h = No Additional Information
Information: 0000000000000000h
Command Specific Information: 0000000000000000h
Command Time (ms): 51.30

Inquiry returning: SUCCESS

Sending SCSI Report LUNs

  CDB:

        0  1  2  3  4  5  6  7  8  9  A  B
  0x00 A0 00 00 00 00 00 00 00 00 10 00 00

SG IO Issued as Indirect IO

  Sense Data Buffer:

        0  1  2  3  4  5  6  7
  0x00 00 00 00 00 00 00 00 00


Sense Key: 0h = No Error
ASC & ASCQ: 0h - 0h = No Additional Sense Information
FRU: 0h = No Additional Information
Information: 0000000000000000h
Command Specific Information: 0000000000000000h
Command Time (us): 262.39

Report LUNs returning: SUCCESS

Sending SCSI Inquiry, VPD = 00h

  CDB:

        0  1  2  3  4  5
  0x00 12 01 00 00 60 00

SG IO Issued as Indirect IO

  Sense Data Buffer:

        0  1  2  3  4  5  6  7
  0x00 00 00 00 00 00 00 00 00


Sense Key: 0h = No Error
ASC & ASCQ: 0h - 0h = No Additional Sense Information
FRU: 0h = No Additional Information
Information: 0000000000000000h
Command Specific Information: 0000000000000000h
Command Time (us): 286.29

Inquiry returning: SUCCESS

Sending SCSI Inquiry, VPD = 80h

  CDB:

        0  1  2  3  4  5
  0x00 12 01 80 00 18 00

SG IO Issued as Indirect IO

  Sense Data Buffer:

        0  1  2  3  4  5  6  7
  0x00 00 00 00 00 00 00 00 00


Sense Key: 0h = No Error
ASC & ASCQ: 0h - 0h = No Additional Sense Information
FRU: 0h = No Additional Information
Information: 0000000000000000h
Command Specific Information: 0000000000000000h
Command Time (us): 300.72

Inquiry returning: SUCCESS

Sending SCSI Inquiry, VPD = 83h

  CDB:

        0  1  2  3  4  5
  0x00 12 01 83 00 60 00

SG IO Issued as Indirect IO

  Sense Data Buffer:

        0  1  2  3  4  5  6  7
  0x00 00 00 00 00 00 00 00 00


Sense Key: 0h = No Error
ASC & ASCQ: 0h - 0h = No Additional Sense Information
FRU: 0h = No Additional Information
Information: 0000000000000000h
Command Specific Information: 0000000000000000h
Command Time (us): 311.23

Inquiry returning: SUCCESS

Sending SCSI Inquiry, VPD = B1h

  CDB:

        0  1  2  3  4  5
  0x00 12 01 B1 00 40 00

SG IO Issued as Indirect IO

  Sense Data Buffer:

        0  1  2  3  4  5  6  7
  0x00 00 00 00 00 00 00 00 00


Sense Key: 0h = No Error
ASC & ASCQ: 0h - 0h = No Additional Sense Information
FRU: 0h = No Additional Information
Information: 0000000000000000h
Command Specific Information: 0000000000000000h
Command Time (us): 285.18

Inquiry returning: SUCCESS

Sending SCSI Read Capacity 10 command

  CDB:

        0  1  2  3  4  5  6  7  8  9
  0x00 25 00 00 00 00 00 00 00 00 00

SG IO Issued as Indirect IO

  Sense Data Buffer:

        0  1  2  3  4  5  6  7
  0x00 00 00 00 00 00 00 00 00


Sense Key: 0h = No Error
ASC & ASCQ: 0h - 0h = No Additional Sense Information
FRU: 0h = No Additional Information
Information: 0000000000000000h
Command Specific Information: 0000000000000000h
Command Time (us): 247.58

Read Capacity 10 returning: SUCCESS

Sending SCSI Read Capacity 16 command

  CDB:

        0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F
  0x00 9E 10 00 00 00 00 00 00 00 00 00 00 00 20 00 00

SG IO Issued as Indirect IO

  Sense Data Buffer:

        0  1  2  3  4  5  6  7
  0x00 00 00 00 00 00 00 00 00


Sense Key: 0h = No Error
ASC & ASCQ: 0h - 0h = No Additional Sense Information
FRU: 0h = No Additional Information
Information: 0000000000000000h
Command Specific Information: 0000000000000000h
Command Time (us): 254.72

Read Capacity 16 returning: SUCCESS


/dev/sg1 - ST32000444SS - 9WM5QLA3 - 0006 - SCSI
Couldn't open file firmware/MU-SAS-0008.LOD

The file definitely exists at the path and is accessible.
@gdevenyi
Copy link
Author

After running the code through strace, I've found the issue:

readlink("firmware", 0x7fffac7a1f6f, 4080) = -1 EINVAL (Invalid argument)
readlink("firmware/MU-SAS-0008.LOD", 0x7fffac7a1f6f, 4096) = -1 EINVAL (Invalid argument)
getcwd("/root/openSeaChest-v24.08.1-linux-x86_64-portable", 4097) = 50
stat("firmware/MU-SAS-0008.LOD", {st_mode=S_IFREG|0644, st_size=1107456, ...}) = 0
stat("/root/openSeaChest-v24.08.1-linux-x86_64-portable/firmware/MU-SAS-0008.LOD", {st_mode=S_IFREG|0644, st_size=1107456, ...}) = 0
geteuid()                               = 0
lstat("/", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
lstat("/root", {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0
lstat("/root/openSeaChest-v24.08.1-linux-x86_64-portable", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
writev(1, [{"Couldn't open file firmware/MU-S"..., 43}, {"\n", 1}], 2Couldn't open file firmware/MU-SAS-0008.LOD
) = 44
munmap(0x7fa432b16000, 16384)           = 0
close(3)                                = 0
madvise(0x7fa432b1e000, 4096, 0x8 /* MADV_??? */) = 0
munmap(0x7fa432b1d000, 32768)           = 0
writev(1, [{"", 0}, {"\n", 1}], 2
)      = 1
exit_group(3)                           = ?
+++ exited with 3 +++

The code to open the firmware appears to be attempting to compare the effective UID of the runtime to the ownership of all the file paths"

However the ownership of the published files from Releases have weird ownership:

root@server:~# ls -ldn openSeaChest-v24.08.1-linux-x86_64-portable
drwxr-xr-x 5 1001 999 4096 Oct 14 00:53 openSeaChest-v24.08.1-linux-x86_64-portable

After a chown root:root -R openSeaChest-v24.08.1-linux-x86_64-portable the command can read the firmware file.

@gdevenyi gdevenyi changed the title MU-SAS-0008.LOD from ConstellationES1-Muskie-SAS-StdOEM-0008.zip cannot be read Ownership of Released binaries is bad in Linux causing phantom "can't read file" errors Oct 14, 2024
@vonericsen
Copy link
Contributor

Hi @gdevenyi,

Thanks for reporting this issue.

We implemented some Cert-C coding recommendations to check permissions and ownership before opening files. So that is where the checks are coming from. Here is some detail I added to the wiki about it.

The permissions/ownership are possibly like this due to how the Github Actions CI handles builds. I'll see if adding a step to set root:root before zipping it up will help.

@gdevenyi
Copy link
Author

I figured this was intentional, but the release packages start off broken, a user should build software as an unprivileged user, and the error you get isn't very helpful.

All this adds up to a frustrating experience until I worked things out.

@vonericsen
Copy link
Contributor

@gdevenyi,

That is totally fair and I also have on my todo list to expand on the reason for failing to open a file (permissions, ownership, file really not there, etc) as well since we introduced this change.
I'll play with the CI and adding this step to see if that helps.

@vonericsen
Copy link
Contributor

@gdevenyi,

As I'm looking into this, can you share some details about how you downloaded and extracted the release package so I can verify my changes are working as expected?

For example, did you do the download in Windows or Linux, was it moved to a system via USB, etc just so I can attempt to repeat the process and verify my change.

@gdevenyi
Copy link
Author

  • [root] download on linux (wget)
  • [root] tar xavf openSeaChest-v24.08.1-linux-x86_64-portable.tar.xz
  • (ownership of files and resulting directory is not root, but other UID/GID)

@vonericsen
Copy link
Contributor

@gdevenyi,

Thanks! That definitely repeated the issue.

When I tested the tar as my user rather than root, ownership was 1000:1000 (my username in place here), but when I run the tar command as root, I see the same weird ownership from the release.

I'm still testing to see if I can resolve this from the CI, but that is not what I would have expected to happen.

@gdevenyi
Copy link
Author

gdevenyi commented Oct 15, 2024 •
edited
Loading

When I tested the tar as my user rather than root, ownership was 1000:1000 (my username in place here)

That still ends up being a problem of course, because if you were to sudo ./openSeaChest_Firmware <etc> you would run into the same permissions issues having the firmware file locally as a user.

@vonericsen
Copy link
Contributor

That still ends up being a problem of course, because if you were to sudo ./openSeaChest_Firmware you would run into the same permissions issues having the firmware file locally as a user.

I have not had this be an issue when I've tested it this way.
In the permissions evaluation it allows files from the current user or root.
With Sudo it will check if it's owned by root first, but I added a special case to determine if sudo was used to execute the tool, and if it was, to allow anything owned by that user to be trusted, so this should be ok in this case.
The sudo check uses the environment variable SUDO_UID to figure out which user executed the command to determine if it is trusted or not. The idea in this case is the system administrator has already trusted a user with sudo privileges so it should be trusted in this case too....and it makes the software much easier to use in this case.

I'm still debugging to see if I can figure out why un-taring with root causes weird ownership to be set or how to resolve that.

@gdevenyi
Copy link
Author

gdevenyi commented Oct 15, 2024 •
edited
Loading

Unfortunately, the sudo bits don't work.

$ wget https://github.com/Seagate/openSeaChest/releases/download/v24.08.1/openSeaChest-v24.08.1-linux-x86_64-portable.tar.xz
$ tar xaf https://github.com/Seagate/openSeaChest/releases/download/v24.08.1/openSeaChest-v24.08.1-linux-x86_64-portable.tar.xz
$ unzip ConstellationES1-Muskie-SAS-StdOEM-0008.zip
$ sudo ./openSeaChest-v24.08.1-linux-x86_64-portable/openSeaChest_Firmware -d /dev/sg0 --downloadFW firmware/MU-SAS-0008.LOD
==========================================================================================
 openSeaChest_Firmware - openSeaChest drive utilities - NVMe Enabled
 Copyright (c) 2014-2024 Seagate Technology LLC and/or its Affiliates, All Rights Reserved
 openSeaChest_Firmware Version: 4.2.0-8_0_1 X86_64
 Build Date: Sep 25 2024
 Today: 20241015T134312 User: root
==========================================================================================

/dev/sg0 - ST32000444SS - <REDACTED> - 0006 - SCSI
Couldn't open file firmware/MU-SAS-0008.LOD

@vonericsen
Copy link
Contributor

@gdevenyi,

I have now tried setting ownership before tar and after tar and I have not been able to get the ownership to set as root:root.
Here is the before tar step in the CI
and here is the after before it is uploaded

I do not know what else I can do to resolve this particular problem of ownership from the CI. If you have any other ideas for me to try I am happy to test them out as well.
I'm only seeing this when I'm logged in as root for some reason, but my current user is working as expected with ownership being applied to that user.

Unfortunately, the sudo bits don't work

I will debug this some more but I know we have tested this to make sure it can accept user files and root files. It's possible we missed something during testing or someone "fixed" this with a chown or chmod and we didn't log it, but I will check to make sure this is working.
The full error output I mentioned previously will also be helpful so I'll work on adding that in as well.

@vonericsen
Copy link
Contributor

I figured out the tar issue!

I'm cleaning up my CI changes, but I have managed to get it to keep root:root when extracting as root and it keeps the current user when extracting as current user (say UID 1000).

What I was missing is -R on my chown command.
What I learned (and did not know earlier) is that running tar as root by default preserves existing ownership of the contents unless you pass some other flags to tell it otherwise (you can pass --owner:root --group:root when extracting). So the user and group of the CI is what it was spitting out by default.
Since these flags are GNU specific and I don't think many people know about them, I'll change the packaging steps to make sure everything is owned by root first so that this is not an issue for anyone else pulling down the files since I know many people will do this as root and it will throw them off.

vonericsen added a commit that referenced this issue Oct 15, 2024
@vonericsen
bug: Finish set_executable_ownership_in_ci
42bc2c2 
Finishing how the tar files are created in github CI to fix permissions/ownership when extracting as root user.

[#158]
@vonericsen
Copy link
Contributor

@gdevenyi,

Can you please try using wget on this test build to confirm that the CI changes I have made work for you now?
Testing linux tar ownership

vonericsen added a commit to Seagate/opensea-common that referenced this issue Oct 15, 2024
@vonericsen
feat: Adding initial code to set an output error for secure file
65bd2ff 
Adding initial code to handle setting an error message to provide what went wrong when attempting to access a file and finding incorrect ownership or permissions during evaluation.

This will provide a user more information to determine where the security issue is so that they can address it properly and try again once it has been corrected.

[Seagate/openSeaChest#158]

Signed-off-by: Tyler Erickson <[email protected]>
@gdevenyi
Copy link
Author

Running download/extract/commands as root work correctly now, no file access issues.

@gdevenyi gdevenyi mentioned this issue Oct 16, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@gdevenyi @vonericsen