bottlesdevs / Bottles

Run Windows software and games on Linux
https://usebottles.com
GNU General Public License v3.0
6.48k stars 275 forks source link

[Bug]: Dependencies cannot be installed in Bottles due to outdated hashes! #3558

Open Arcitec opened 2 weeks ago

Arcitec commented 2 weeks ago

Describe the bug

Dependencies can never be installed in newly created Bottles without restarting Bottles first.

Update: See first comment below this. I found more details to track down the bug and have clarified.

To Reproduce

  1. Create new Bottle. I use Gaming template.
  2. Open Bottle, click Dependencies.
  3. Try to install vcredist2022. You will ALWAYS get this error. I've seen it for a year across 2 computers: image
  4. Look in the console logs. It says the hash mismatched and that the vcredist file was deleted.
  5. So you try again. It downloads vcredist again. Then the installation says it failed again. Hash mishmatch again.
  6. Restart Bottles.
  7. Install it again. Now it works.

Package

Flatpak from Flathub

Distribution

Fedora Workstation 40

Debugging Information

Official Package: true
Version: '51.15'
DE/WM: gnome-xorg
Display:
    X.org: true
    X.org (port): :0
    Wayland: false
Graphics:
    vendors:
        nvidia:
            vendor: nvidia
            envs:
                __NV_PRIME_RENDER_OFFLOAD: '1'
                __GLX_VENDOR_LIBRARY_NAME: nvidia
                __VK_LAYER_NV_optimus: NVIDIA_only
            icd: /usr/lib/x86_64-linux-gnu/GL/vulkan/icd.d/nvidia_icd.json
            nvngx_path: /usr/lib/x86_64-linux-gnu/GL/nvidia-560-35-03/extra/nvidia/wine
    prime:
        integrated: null
        discrete: null
Kernel:
    Type: Linux
    Version: 6.9.12-200.fc40.x86_64
Disk:
    Total: 33553694720
    Free: 33553510400
RAM:
    MemTotal: 62.5GiB
    MemAvailable: 16.0GiB
Bottles_envs: null

Troubleshooting Logs

Log when it failed:

13:51:32 (INFO) Installing dependency [vcredist2022] in bottle [Ultima Online]. 
13:51:32 (WARNING) File [vcredist2022_x86.exe] already exists in temp, skipping. 
13:51:32 (ERROR) Downloaded file [VC_redist.x86.exe] looks corrupted. 
13:51:32 (ERROR) Source cksum: [822551b098e93c504de2c7865216928f] downloaded: [8457542fd4be74cb2c3a92b3386ae8e9] 
13:51:32 (ERROR) Removing corrupted file [VC_redist.x86.exe].

Log when you retry during the same failed session:

13:53:26 (WARNING) File [vcredist2022_x64.exe] already exists in temp, skipping. 
13:53:26 (ERROR) Downloaded file [VC_redist.x64.exe] looks corrupted. 
13:53:26 (ERROR) Source cksum: [52f4f5a6adc24bce21dbceac2e2ad809] downloaded: [1d545507009cc4ec7409c1bc6e93b17b] 
13:53:26 (ERROR) Removing corrupted file [VC_redist.x64.exe].

Log after Bottles restart, when it always succeeds:

VC_redist.x64.exe (100%) ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ (24.5MiB/24.5MiB - 27.7MiB)

13:55:17 (INFO) Renaming [VC_redist.x64.exe] to [vcredist2022_x64.exe]. 
13:55:17 (INFO) Launching an executable… 
13:55:17 (INFO) Using EasyAntiCheat runtime 
13:55:17 (INFO) Using BattlEye runtime 
fsync: up and running.
wine: RLIMIT_NICE is <= 20, unable to use setpriority safely
0088:err:hid:udev_bus_init UDEV monitor creation failed
13:55:22 (INFO) Adding Key: [HKEY_CURRENT_USER\Software\Wine\DllOverrides] with Value: [concrt140] and Data: [native,builtin] in Ultima Online registry 
13:55:22 (INFO) Using Wine Registry CLI -- add 
fsync: up and running.
wine: RLIMIT_NICE is <= 20, unable to use setpriority safely
0084:err:hid:udev_bus_init UDEV monitor creation failed
13:55:23 (INFO) reg: The operation completed successfully

13:55:23 (INFO) Adding Key: [HKEY_CURRENT_USER\Software\Wine\DllOverrides] with Value: [msvcp140] and Data: [native,builtin] in Ultima Online registry 
13:55:23 (INFO) Using Wine Registry CLI -- add 

etc... it works

Additional context

This is 100% reproducible in all new bottles. Leading me to believe that Bottles is hashing the wrong file or path when you have newly created a bottle.

Edit: Googling the downloaded hashes actually seems to bring up references to VCredist, so it seems more like Bottles is using the wrong hashes when verifying the downloads. But it's weird that it's only a problem for newly created bottles, and that it works after a Bottles restart.

Arcitec commented 2 weeks ago

Okay, I am now sure that the bug is that Bottles doesn't download new dependency hashes until you do the following routine:

  1. Try to download a dependency and fail.
  2. Restart Bottles.
  3. Now the latest dependency hashes will be in effect.

I know this because the following process works. I just verified it:

This means that the real bug is that the hashes for the dependencies are not correctly updated in Bottles during the application lifetime. It must attempt to do an install (which probably fetches the new hashes), and then Bottles must be restarted to load the latest dependency download verification hashes.

Furthermore, you must try to install a dependency that frequently updates the binary on the server, so that the old hashes mismatch. VCRedist is a good candidate.

The reason I associated this bug with "newly created bottles" is that I only try to install dependencies after creating new bottles. ;)

So the fix is: After downloading the latest dependency hashes, LOAD THEM. ;)

commiterate commented 2 weeks ago

Prior hash mismatch issues for vcredist2019 + vcredist2022 were because their dependency manifests used a latest version download link instead of a version-specific download link. That was fixed in this PR last month:

https://github.com/bottlesdevs/dependencies/pull/259

Your hashes for the downloaded files on failed attempts seem to match the prior ones.

Bottles essentially clones https://github.com/bottlesdevs/dependencies on startup and uses that as its source of truth.

_You can override this by setting the LOCAL_DEPENDENCIES environment variable to point to a local clone before starting Bottles in the same process. This is what's used to test dependency manifest changes._

In this case, it seems like Bottles' temp dependency installer cache is populated with an old binary. When Bottles tries to install vcredist2022, it:

  1. Checks the installer cache.
  2. Sees old/stale vcredist2022_x86.exe and vcredist2022_x64.exe files and re-uses them instead of re-downloading.
  3. Compares their md5 checksum against the latest bottlesdevs/dependencies metadata and fails.

So this seems to be a cache invalidation bug.

commiterate commented 2 weeks ago

Relevant lines:

https://github.com/bottlesdevs/Bottles/blob/e93cd189e966ed0daf515f8398f12e2cb6dcf620/bottles/backend/managers/installer.py#L176-L184

https://github.com/bottlesdevs/Bottles/blob/e93cd189e966ed0daf515f8398f12e2cb6dcf620/bottles/backend/managers/component.py#L154-L164

https://github.com/bottlesdevs/Bottles/blob/e93cd189e966ed0daf515f8398f12e2cb6dcf620/bottles/backend/globals.py#L27-L40

This seems to be the state on disk:

~/.local/share/bottles/temp/
├── vcredist2022_x86.exe (md5sum: 8457542fd4be74cb2c3a92b3386ae8e9)
└── vcredist2022_x64.exe (md5sum: 1d545507009cc4ec7409c1bc6e93b17b)

Fix probably requires changing the cache to be a content-addressable store instead where files have their hash (which should be switched from md5 to sha256 or something else to reduce the probability of collisions) somewhere in the path (either a parent directory name or in the file name).

Unsure if the maintainers are interested in reworking this given the current focus on Bottles Next.

Arcitec commented 2 weeks ago

@commiterate Ah thank you for your work investigating and fixing the hash URL issue.

I found something else that's super weird in this behavior.

Look at my "Troubleshooting Logs". I didn't notice it until now:

  1. First attempt: File [vcredist2022_x86.exe] already exists in temp, skipping. Source cksum: [822551b098e93c504de2c7865216928f] downloaded: [8457542fd4be74cb2c3a92b3386ae8e9]. Removing corrupted file [VC_redist.x86.exe].
  2. Second attempt: File [vcredist2022_x64.exe] already exists in temp, skipping. Source cksum: [52f4f5a6adc24bce21dbceac2e2ad809] downloaded: [1d545507009cc4ec7409c1bc6e93b17b]. Removing corrupted file [VC_redist.x64.exe].
  3. I didn't notice until now that each attempt tried a different filename. So I suspect that a 3rd click on the install button would have solved the issue without needing to restart Bottles. Because the first 2 attempts deleted the x86 and x64 exe installers respectively.

Anyway, I disagree that hashes would need to be part of the paths (content-addressable storage). Because Bottles lacks any way to install older versions of dependencies anyway.

All we really need to do to solve the bug is to change this:

File [vcredist2022_x64.exe] already exists in temp, skipping [the download and using local file].

To this:

File [vcredist2022_x64.exe] already exists in temp, but that local bad-boy has the wrong hash, so actually, nah, delete it and download it again!

commiterate commented 2 weeks ago

Switching to a CAS is more to remove the current mechanism of dependency manifests needing to rename files to prevent collisions inside ~/.local/share/bottles/temp.

With the way temp and install_exe/install_msi work today, each dependency manifest needs to make sure its install_exe/install_msi step doesn't create file name collisions inside of temp.

For example, both vcredist2019 and vcredist2022 download binaries that are originally named VC_redist.x86.exe and VC_redist.x64.exe.

https://github.com/bottlesdevs/dependencies/blob/0472592e9b64d52c2b67a91812422c0478acecb6/Essentials/vcredist2019.yml#L8-L24

https://github.com/bottlesdevs/dependencies/blob/0472592e9b64d52c2b67a91812422c0478acecb6/Essentials/vcredist2022.yml#L8-L24

If the manifest maintainer forgets to add a rename directive in their install_exe/install_msi step, they'd clobber each other. That would produce a similar collision issue you're seeing, but instead of stale vcredist2022 installers creating collisions it would be up-to-date vcredist2019 installers creating collisions with vcredist2022.

Invalidating the file in temp and forcing a redownload would fix poisoned cache entries, but it can cause a lot of cache thrashing due to colliding file names. The cost of 100% cache misses isn't too much of an issue today since many installer binaries are only on the order of 10s to 100s of MBs, but it's a broken optimization nonetheless.


There's a slight problem with switching to a CAS since it changes checksums from optional to required in install_exe/install_msi. I wouldn't be surprised if there are installers which don't provide version-specific links and, for better UX at the expense of reproducibility and security, the dependency manifests will leave out a checksum as a result.

Arcitec commented 2 weeks ago

That's true, Bottles could just give each dependency recipe its own subdirectory, like temp/vcredist2022/VC_redist.x86.exe to avoid clashes.