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

Do not merge: Dummy test change #12315

Closed
wants to merge 4 commits into from
Closed

Do not merge: Dummy test change #12315

wants to merge 4 commits into from

Conversation

OneBlue
Copy link
Collaborator

@OneBlue OneBlue commented Nov 25, 2024

No description provided.

@sbradnick
Copy link
Contributor

Are there any plans to have the Sha256 field be anything (a URL perhaps?) but a string of the form of 0x<sha256 sum of file>?

Part of our build process creates a <file>.tar.xz.sha256 file w/ the content of the file being:

<sha256 sum of file> <file name>

And it would be great to be able to have the modern setup point to:

"Url": "https://<specific something>.tar.xz"
"Sha256": "https://<specific something>.tar.xz.sha256"

@OneBlue
Copy link
Collaborator Author

OneBlue commented Nov 25, 2024

Are there any plans to have the Sha256 field be anything (a URL perhaps?) but a string of the form of 0x<sha256 sum of file>?

Not at this time. The idea behind that hash is to guarantee that the distribution doesn't change after it's been added to the manifest, so having a remote file containing the hash wouldn't have that guarantee unfortunately.

@sbradnick
Copy link
Contributor

Not at this time. The idea behind that hash is to guarantee that the distribution doesn't change after it's been added to the manifest, so having a remote file containing the hash wouldn't have that guarantee unfortunately.

Is that some kind of internal check? Maybe something is auto-checked/validated before a PR would be accepted? Our build service can build the tarball and hash it and even host both files. So it's a bummer if I'd still need to do point-in-time uploads w/ a simple hash; I don't really see the value in that, especially compared to the Store where it felt like it made more sense since we signed the .appx, then Microsoft signed it and there were the other elements related to the Store (and presentation).

@OneBlue
Copy link
Collaborator Author

OneBlue commented Nov 25, 2024

Is that some kind of internal check? Maybe something is auto-checked/validated before a PR would be accepted? Our build service can build the tarball and hash it and even host both files. So it's a bummer if I'd still need to do point-in-time uploads w/ a simple hash; I don't really see the value in that, especially compared to the Store where it felt like it made more sense since we signed the .appx, then Microsoft signed it and there were the other elements related to the Store (and presentation).

Completely understood. This hash is here to prevent potential supply chain attacks so it would need to be directly in the manifest.

Would automating the generation of the pull request be an option for you ? We could potentially have a script hosted in this repo that could do the hashing / generating of the updated manifest and creation of the pull request. If you could call it from your build pipeline that would remove the need for manual work at least

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution to WSL.
The following fatal errors have been found in this pull request:

  • test/test-v1: File "./etc/wsl-distribution.conf" not found in tar

The following suggestions have been found in this pull request:

  • test/test-v1: file: "./etc/wsl-distribution.conf" has unexpected mode: 0o755 (expected: ['0o664', '0o644'])
  • test/test-v1: value for oobe.command is not under /usr/lib/wsl: "/oobe.sh"
  • test/test-v1: file: "./debian.ico" has unexpected mode: 0o755 (expected: ['0o660', '0o640'])
  • test/test-v1: value for shortcut.icon is not under /usr/lib/wsl: "/debian.ico"
  • test/test-v1: Found discouraged system unit: /usr/lib/systemd/system/sysinit.target.wants/systemd-tmpfiles-setup.service
  • test/test-v1: Found discouraged system unit: /usr/lib/systemd/system/sysinit.target.wants/systemd-tmpfiles-setup-dev.service
  • test/test-v1: Found discouraged system unit: /etc/systemd/system/multi-user.target.wants/networking.service
  • test/test-v1: Found discouraged system unit: /usr/lib/systemd/system/sysinit.target.wants/systemd-tmpfiles-setup.service
  • test/test-v1: Found discouraged system unit: /usr/lib/systemd/system/sysinit.target.wants/systemd-tmpfiles-setup-dev.service
  • test/test-v1: Found discouraged system unit: /etc/systemd/system/multi-user.target.wants/systemd-resolved.service
  • test/test-v1: Found discouraged system unit: /etc/systemd/system/multi-user.target.wants/systemd-networkd.service

@OneBlue OneBlue changed the title Dummy test change Do not merge: Dummy test change Nov 25, 2024
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution to WSL.
The following fatal errors have been found in this pull request:

  • test/test-v1: File "./etc/wsl-distribution.conf" not found in tar

The following suggestions have been found in this pull request:

  • test/test-v1: file: "./etc/wsl-distribution.conf" has unexpected mode: 0o755 (expected: ['0o664', '0o644'])
  • test/test-v1: value for oobe.command is not under /usr/lib/wsl: "/oobe.sh"
  • test/test-v1: file: "./debian.ico" has unexpected mode: 0o755 (expected: ['0o660', '0o640'])
  • test/test-v1: value for shortcut.icon is not under /usr/lib/wsl: "/debian.ico"
  • test/test-v1: Found discouraged system unit: /usr/lib/systemd/system/sysinit.target.wants/systemd-tmpfiles-setup.service
  • test/test-v1: Found discouraged system unit: /usr/lib/systemd/system/sysinit.target.wants/systemd-tmpfiles-setup-dev.service
  • test/test-v1: Found discouraged system unit: /etc/systemd/system/multi-user.target.wants/networking.service
  • test/test-v1: Found discouraged system unit: /usr/lib/systemd/system/sysinit.target.wants/systemd-tmpfiles-setup.service
  • test/test-v1: Found discouraged system unit: /usr/lib/systemd/system/sysinit.target.wants/systemd-tmpfiles-setup-dev.service
  • test/test-v1: Found discouraged system unit: /etc/systemd/system/multi-user.target.wants/systemd-resolved.service
  • test/test-v1: Found discouraged system unit: /etc/systemd/system/multi-user.target.wants/systemd-networkd.service

@sbradnick
Copy link
Contributor

Completely understood. This hash is here to prevent potential supply chain attacks so it would need to be directly in the manifest.

I have no problem with it being there, just the fact that the process continues to be manual when it feels like there's a better chance where it could be more automated because the pieces seem like they're present. We'd have the hosting built-in and they're already hashed. I'm not saying this process (w/o it being automated in some sense) is difficult but I'm disappointed if the new way (tarballs <-> .wsl) isn't any more automatic than .appx was.

Would automating the generation of the pull request be an option for you ? We could potentially have a script hosted in this repo that could do the hashing / generating of the updated manifest and creation of the pull request. If you could call it from your build pipeline that would remove the need for manual work at least

The build process is cut off from the internet, so seems very unlikely. I'd also be concerned there'd be some indeterminate amount of time where a PR is in flux and the build process has produced a new build, so that PR's file/hash are no good since that file is gone. Sure, that's alleviated by it being a manual process where I provide a static file to you somehow, so it is what it is. I guess we'd continue what we've been doing, just with a different file type.

Here's 2 URLS, as an example:

  1. https://download.opensuse.org/repositories/Virtualization:/WSL:/instarball/openSUSE_Tumbleweed_images/
  2. https://download.opensuse.org/repositories/Virtualization:/WSL:/instarball/openSUSE_Leap_15.6_images/

Both contain a "BuildX.Y" specific .tar.xz and .sha256 file along with a more generic link to the same files. Right now both Tumbleweed and Leap 15.6 have a '-YYYYMMDD' (-20241124) or '-${OS_VERSION_ID}' (-15.6) tag, which I can remove so they'd be much more static in name (and in the sense of where the link points). So if possible, it'd solve a few issues if the modern setup could do:

"Url": "https://download.opensuse.org/repositories/Virtualization:/WSL:/instarball/openSUSE_Tumbleweed_images/openSUSE-Tumbleweed.x86_64.tar.xz"
"Sha256": "https://download.opensuse.org/repositories/Virtualization:/WSL:/instarball/openSUSE_Tumbleweed_images/openSUSE-Tumbleweed.x86_64.tar.xz.sha256"

Granted, I'd have to make those changes to the build, so those links aren't valid. Also, the .sha256 file isn't ONLY the hash, nor does it have a 0x in it, but I'd assume something could parse a txt file.

I'm not realistically concerned about a supply chain attack in the sense of a static hash for a .tar.xz file being mismatched, since if someone was able to manipulate the build service to do so, we'd have bigger issues than a hash mismatch.

There's also a chance this can't work for SLE as that build service is generally cut-off from the outside world, but I feel there's enough benefit for openSUSE if this could work, I'm throwing it out here.

@OneBlue
Copy link
Collaborator Author

OneBlue commented Nov 27, 2024

The build process is cut off from the internet, so seems very unlikely. I'd also be concerned there'd be some indeterminate amount of time where a PR is in flux and the build process has produced a new build, so that PR's file/hash are no good since that file is gone. Sure, that's alleviated by it being a manual process where I provide a static file to you somehow, so it is what it is. I guess we'd continue what we've been doing, just with a different file type.

Agreed with that. The official recommendation is to keep at least the two most recent builds on unique URL's, so the hash doesn't get invalidated.

Both contain a "BuildX.Y" specific .tar.xz and .sha256 file along with a more generic link to the same files. Right now both Tumbleweed and Leap 15.6 have a '-YYYYMMDD' (-20241124) or '-${OS_VERSION_ID}' (-15.6) tag, which I can remove so they'd be much more static in name (and in the sense of where the link points). So if possible, it'd solve a few issues if the modern setup could do:

"Url": "https://download.opensuse.org/repositories/Virtualization:/WSL:/instarball/openSUSE_Tumbleweed_images/openSUSE-Tumbleweed.x86_64.tar.xz"
"Sha256": "https://download.opensuse.org/repositories/Virtualization:/WSL:/instarball/openSUSE_Tumbleweed_images/openSUSE-Tumbleweed.x86_64.tar.xz.sha256"

Unfortunately we can't easily change the manifest format now that 2.4.4 has been released. That change would be breaking for people that have installed this version.

If the build process itself is disconnected from the internet, there's probably a later step that actually publishes the .tar on a publicly accessible URL right ? Could the pull request step be added there ?

Or maybe if that not an option, could we use a "pull" model, where another script looks for updated tars, and automatically hashes them and creates the pull request ?

@OneBlue OneBlue closed this Nov 27, 2024
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