scrcpy icon indicating copy to clipboard operation
scrcpy copied to clipboard

install_release.sh wrong SERVER_URL and SHA256

Open pupitetris opened this issue 2 years ago • 13 comments
trafficstars

  • [x] I have read the FAQ.
  • [x] I have searched in existing issues.

Environment

  • OS: Debian bookworm (latest)
  • scrcpy version: 2.0
  • installation method: manual build
  • device model: not relevant
  • Android version: not relevant

Describe the bug After a successful installation, scrcpy won´t run because install_release.sh pulled an old version of the scrcpy-server binary.

On errors, please provide the output of the console (and adb logcat if relevant).

$ scrcpy --no-audio
scrcpy 2.0 <https://github.com/Genymobile/scrcpy>
/usr/local/share/scrcpy/scrcpy-server: 1 file pushed, 0 skipped. 20.2 MB/s (42151 bytes in 0.002s)
[server] ERROR: Exception on thread Thread[main,5,main]
java.lang.IllegalArgumentException: The server version (1.25) does not match the client (2.0)
	at com.genymobile.scrcpy.Server.createOptions(Server.java:166)
	at com.genymobile.scrcpy.Server.main(Server.java:330)
	at com.android.internal.os.RuntimeInit.nativeFinishInit(Native Method)
	at com.android.internal.os.RuntimeInit.main(RuntimeInit.java:355)
ERROR: Server connection failed

The solution was to get the url for the v2.0 binary, calculate the SHA256 hash and replace the values on the script:

PREBUILT_SERVER_URL=https://github.com/Genymobile/scrcpy/releases/download/v1.25/scrcpy-server-v1.25
PREBUILT_SERVER_SHA256=ce0306c7bbd06ae72f6d06f7ec0ee33774995a65de71e0a83813ecb67aec9bdb

to:

PREBUILT_SERVER_URL=https://github.com/Genymobile/scrcpy/releases/download/v2.0/scrcpy-server-v2.0
PREBUILT_SERVER_SHA256=9e241615f578cd690bb43311000debdecf6a9c50a7082b001952f18f6f21ddc2

And run the script again. Now scrcpy runs fine.

I suppose those values on the script should be generated automatically using a template or something when generating the release.

Cheers!

pupitetris avatar Jun 20 '23 22:06 pupitetris

The script install_release.sh is updated once the release binaries are generated (of course, because it includes a release checksum). But the release binaries are generated once the version is tagged (btw the tagname is used for the release name). So the tag may not contain the new install_release.sh script.

Therefore, this script is always updated one commit after the tag (along with the github pages referencing the new version): abc1be4872ee1869b307405b091d736b7cd2e0ff

For install_release.sh, it would be possible to cheat, because the scrcpy-server binary is not impacted by this script, so we could re-tag after the binaries are generated, but it feels wrong.

This script is always intended to be executed on the master branch as a convenience to install the latest release.

rom1v avatar Jun 21 '23 07:06 rom1v

OK, gotcha. Actually, yes: the version of the code I downloaded was from a release tag. Thanks.

pupitetris avatar Jun 23 '23 23:06 pupitetris

Hello @rom1v,

"This script is always intended to be executed on the master branch as a convenience to install the latest release"

please consider tagging it to releases or tags. what we can do here is generate checksums.txt file along with the binary file.

kindly refer to releases here as an example. https://github.com/gohugoio/hugo/releases/tag/v0.131.0

This will ensure the checksums are available to everyone and are not hard-coded into the install script.

We can then modify the install script to verify the hash. Kindly refer this script as an example https://gist.github.com/adityatelange/98a6b8e8cf9551a5eeee129aead01823

Regards.

adityatelange avatar Aug 03 '24 14:08 adityatelange

Reopening issue as the script's existence is still misleading users into using it, and there is now a proposed solution to improve the script and make it work not only for the master branch but for future releases as well.

pupitetris avatar Aug 03 '24 16:08 pupitetris

I totally agree that it is confusing.

In theory, install_release.sh should not be in the same repo, because then it creates a cyclic reference if we want the updated script to be included in the tag (the tag depends on updating the file, but updating the file depends on the release which depends on the tag). But it is still convenient to put it there.

Before that script, the commands to execute were just documented. The script just does it automatically.

what we can do here is generate checksums.txt file along with the binary file.

There is already such a file, but I don't want to use that for install_release.sh. Btw, if the file is at the very same place as the binary file, the checksum is useless to detect tampering.

It is important that the checksum is hardcoded in a commit, because that gives some level of trust (it cannot be maliciously modified without a high probability of being noticed by many git users). By contrast, someone with write access to the repo might change the checksum file in the release, and install_release.sh would accept a malicious replaced file+checksum.

Of course, the solution to this problem is known: public key cryptography (I also publish a signature SHA256SUMS.txt.asc for every release with my gpg key), but I don't want install_release.sh to depend on that. The script is so simple that everyone can read it and understand what it does.

For install_release.sh, it would be possible to cheat, because the scrcpy-server binary is not impacted by this script, so we could re-tag after the binaries are generated, but it feels wrong.

I might consider to move the tag just after the binaries are generated in the future, that would "solve" the problem. But I'm not sure yet.

rom1v avatar Aug 03 '24 18:08 rom1v

I knew you'd come to this as from the way I mentioned, the binary will be safe from tampering but not checksusms.txt is not. And this is a never ending cycle. You have to trust Github here to provide the server package as it is without tampering. With checksums what we are ensuring is users can verify the binary themselves. IMHO we need not check the binary for checksum as a measure of tampering but as a measure that it is downloaded properly and is not corrupt/half finished. In extreme cases where the system itself is compromised, that is not maintainer's job to protect the end user. I hope that makes sense, would like to hear your thoughts.

adityatelange avatar Aug 03 '24 19:08 adityatelange

I am not sure how the release takes place, or maintainers directly drag and drop binaries as assets in a release. Proposing following method, let me know if I can create a PR if you think this way is a good way to proceed.

  1. SHA256SUMS.txt
ca7ab50b2e25a0e5af7599c30383e365983fa5b808e65ce2e1c1bba5bfe8dc3b  scrcpy-server-v2.6.1
17a5d4d17230b4c90fad45af6395efda9aea287a03c04e6b4ecc9ceb8134ea04  scrcpy-win32-v2.6.1.zip
041fc3abf8578ddcead5a8c4a8be8960b7c4d45b21d3370ee2683605e86a728c  scrcpy-win64-v2.6.1.zip
  1. Updating the install instructions to clone the tag (this branch can be updated after each release by maintainer)

https://github.com/Genymobile/scrcpy/blob/44b3fd82b1831f4aa436268870adf32bddb81924/doc/linux.md?plain=1#L34-L38

to

git clone https://github.com/Genymobile/scrcpy --branch v2.6.1
cd scrcpy
./install_release.sh
  1. install_release.sh We use the tag we cloned to download the binary and verify the checksum.
#!/usr/bin/env bash
set -e

GIT_TAG_VERSION=$(git describe --exact-match --tags)

BUILDDIR=build-auto
PREBUILT_SERVER_URL=https://github.com/Genymobile/scrcpy/releases/download/$GIT_TAG_VERSION/scrcpy-server-$GIT_TAG_VERSION
PREBUILT_SERVER_CHECKSUM=https://github.com/Genymobile/scrcpy/releases/download/$GIT_TAG_VERSION/SHA256SUMS.txt

rm -rf scrcpy-server*
rm -rf  SHA256SUMS.*
echo "[scrcpy] Downloading prebuilt server..."
wget "$PREBUILT_SERVER_URL"
echo "[scrcpy] Downloading prebuilt server checksum..."
wget "$PREBUILT_SERVER_CHECKSUM"
echo "[scrcpy] Verifying prebuilt server..."

PREBUILT_SERVER_SHA256=$(grep "scrcpy-server-$GIT_TAG_VERSION" SHA256SUMS.txt)
if echo $PREBUILT_SERVER_SHA256 | sha256sum --check ;then
	echo "[scrcpy] Building client..."
	rm -rf "$BUILDDIR"
	meson setup "$BUILDDIR" --buildtype=release --strip -Db_lto=true \
	    -Dprebuilt_server=scrcpy-server
	cd "$BUILDDIR"
	ninja
	echo "[scrcpy] Installing (sudo)..."
	sudo ninja install
else
	echo "[scrcpy] Checksum verification failed!"
fi

adityatelange avatar Aug 03 '24 19:08 adityatelange

Sorry, there is no way the script will download and accept a file that can be replaced on the remote server in the future.

rom1v avatar Aug 03 '24 20:08 rom1v

@adityatelange

#!/usr/bin/env bash
set -e

GIT_TAG_VERSION=$(git describe --exact-match --tags)

BUILDDIR=build-auto
PREBUILT_SERVER_URL=https://github.com/Genymobile/scrcpy/releases/download/$GIT_TAG_VERSION/scrcpy-server-$GIT_TAG_VERSION
PREBUILT_SERVER_CHECKSUM=https://github.com/Genymobile/scrcpy/releases/download/$GIT_TAG_VERSION/SHA256SUMS.txt

rm -rf scrcpy-server*
rm -rf  SHA256SUMS.*
echo "[scrcpy] Downloading prebuilt server..."
wget "$PREBUILT_SERVER_URL"
echo "[scrcpy] Downloading prebuilt server checksum..."
wget "$PREBUILT_SERVER_CHECKSUM"
echo "[scrcpy] Verifying prebuilt server..."

PREBUILT_SERVER_SHA256=$(grep "scrcpy-server-$GIT_CLONED_VERSION" SHA256SUMS.txt)
if echo $PREBUILT_SERVER_SHA256 | sha256sum --check ;then
	echo "[scrcpy] Building client..."
	rm -rf "$BUILDDIR"
	meson setup "$BUILDDIR" --buildtype=release --strip -Db_lto=true \
	    -Dprebuilt_server=scrcpy-server
	cd "$BUILDDIR"
	ninja
	echo "[scrcpy] Installing (sudo)..."
	sudo ninja install
else
	echo "[scrcpy] Checksum verification failed!"
fi

I'm blind. Can't see where $GIT_CLONED_VERSION variable defined!

Coool avatar Aug 21 '24 16:08 Coool

I'm blind. Can't see where $GIT_CLONED_VERSION variable defined!

Try developing an ability to correlate that it is a typo for GIT_TAG_VERSION

adityatelange avatar Aug 21 '24 16:08 adityatelange

Sorry, there is no way the script will download and accept a file that can be replaced on the remote server in the future.

You can use the git hash then. Since that depends on the previous changes, it cannot be altered: https://stackoverflow.com/questions/460297/git-finding-the-sha1-of-an-individual-file-in-the-index#answer-460422

pupitetris avatar Aug 21 '24 18:08 pupitetris

@pupitetris, strange people are arguing if any of git used command returns valid hash compared to sha command. Haven't used and needed such git function. But if returns wrong hash it's usless.

Coool avatar Aug 22 '24 16:08 Coool

Sorry, there is no way the script will download and accept a file that can be replaced on the remote server in the future.

So you have trust issues on GitHub or the future maintainer of this project. Alright, so if they can replace the remote server file they can ofcourse rewrite the git history as well.

What I can also see is the commits aren't signed by any key so they can do it without anyone even noticing it.

Thoughts?

adityatelange avatar Aug 25 '24 12:08 adityatelange

I might consider to move the tag just after the binaries are generated in the future, that would "solve" the problem. But I'm not sure yet.

Tag v2.7 points to the updated install_release.sh :rocket:

rom1v avatar Sep 15 '24 19:09 rom1v