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

Executable files are not executable (+x) after upgrade #2619

Closed
pjrobertson opened this issue Mar 31, 2024 · 13 comments · Fixed by #2620
Closed

Executable files are not executable (+x) after upgrade #2619

pjrobertson opened this issue Mar 31, 2024 · 13 comments · Fixed by #2620
Labels

Comments

@pjrobertson
Copy link
Contributor

Steps To Reproduce

Every time I upgrade using sudo bash /var/scripts/menu.sh my executable files (mainly: notify_push and translate/bin/node are not executable which causes errors. If I manually chmod +x these files then it works OK.

  1. Ugrade a major or minor version using sudo bash /var/scripts/menu.sh and going to minor/major upgrade
  2. Make sure during this upgrade that either the translations or notify_push apps are updated
  3. For translations node: go to the admin settings and see an error stating that there was an error initiating the app, due to node not being executable. For notify_push: run the command in debug mode and see:
sudo -u www-data php /var/www/nextcloud/occ notify_push:log debug
sudo journalctl -eu notify_push
...
Mar 30 08:09:41 XX systemd[47543]: notify_push.service: Failed at step EXEC spawning /var/www/nextcloud/apps/notify_push/bin/x86_64/notify_push: Permissio>

This might be an issue with running the upgrade script with sudo, but the script complains if you don't do this.

Expected Result

The apps installed during an update should be executable and continue to run

Actual Result

Files are not executable

Screenshots, Videos, or Pastebins

No response

Additional Context

No response

Build Version

28.0.4

Environment

By downloading the VM

Environment Details

No response

@enoch85
Copy link
Member

enoch85 commented Apr 1, 2024

Thanks for posting your issue!

Exactly where are these files located?

I'm thinking, maybe we should add some more files to setup_secure_nextcloud_permissions...?

@pjrobertson
Copy link
Contributor Author

pjrobertson commented Apr 1, 2024

These are the two apps I have come across so far that have this issue:

  • Translate app: /var/www/nextcloud/apps/translate/bin/node
  • notify push app: /var/www/nextcloud/apps/notify_push/bin/x86_64/notify_push
    • and also these for other archs:
    • /var/www/nextcloud/apps/notify_push/bin/armv7/notify_push
    • /var/www/nextcloud/apps/notify_push/bin/aarch64/notify_push

@enoch85
Copy link
Member

enoch85 commented Apr 1, 2024

Aah, I see what you mean.

For notify push we can implement this:

if nextcloud_occ app:list | grep -q notify_push
then
    chmod +x /var/www/nextcloud/apps/notify_push/bin/x86_64/notify_push
    systemctl restart notify_push.service
fi

But I'm not sure about the translate app, can you do a nextcloud_occ app:list | grep translate?

enoch85 added a commit that referenced this issue Apr 1, 2024
make sure the bin is exceutable

Signed-off-by: Daniel Hansson <mailto@danielhansson.nu>
@enoch85 enoch85 mentioned this issue Apr 1, 2024
@pjrobertson
Copy link
Contributor Author

It's the translate app: https://apps.nextcloud.com/apps/translate

ncadmin@box:/var/www/nextcloud$ sudo -u www-data php occ app:list | grep translate
  - translate: 2.1.0

@enoch85
Copy link
Member

enoch85 commented Apr 2, 2024

Just wrote a fix for all files from the top of my head, need to test before merge.

@enoch85
Copy link
Member

enoch85 commented Apr 2, 2024

Please test this file: https://raw.githubusercontent.com/nextcloud/vm/2619/nextcloud_update.sh

It should work with all executable, no matter the name. :)

@pjrobertson
Copy link
Contributor Author

Great stuff, thanks for the quick fix! I haven't tested the code out, but I've had a quick look through it.

My first concern was: finding all executable files and making them execuable could be a security issue (for example, if someone's managed to get bad files into your nextcloud install). But then I saw you are not making any new files executable, only the ones that were executable before the upgrade - so that looks good.

There is maybe a very small risk of a certain file no longer existing after an upgrade which could lead to a security issue / attack route in, but I think this is very small.

Example:

  1. My app called "great app" has an executable in "great_app/bin/executable"
  2. Before the upgrade, this path gets added to $find_executables
  3. After the upgrade, my app also gets upgraded and in the new 2.0 version of the app, my executable is as "great_app/bin/executable2.0"
  4. The script will now try to make "great_app/bin/executable" executable - if somebody (somehow) managed to get an evil file into that location - then we would make their file executable.

I don't think this is really an issue, and I guess this is what code signing is supposed to solve - not something we can really deal with!

@enoch85
Copy link
Member

enoch85 commented Apr 3, 2024

After the upgrade, my app also gets upgraded and in the new 2.0 version of the app, my executable is as "great_app/bin/executable2.0"

That's not likely, I don't see any reason to change name of the executable, and if that occurs we will handle that then.

I haven't tested this widely, so please run the new script to see if it does what 's expected.

@pjrobertson
Copy link
Contributor Author

That sounds reasonable, yeah. I am probably being overly cautious.

For the script - can I just run it straight off or do I need to wait for an actual upgrade/release?

I found another executable that popped up in my logs just now:

/var/www/nextcloud/apps/recognize/bin/node

@enoch85
Copy link
Member

enoch85 commented Apr 3, 2024

For the script - can I just run it straight off or do I need to wait for an actual upgrade/release?

Yes, just run it, it will be like a standard run with the fix implemented. 👍 It won't actually execute any chmod +x since that only happens after an actual Nextcloud upgrade.

Though, if you run it with bash -x you will be able to see if the variable finds the executables. :)

@enoch85
Copy link
Member

enoch85 commented Apr 5, 2024

Were you able to test? :)

@pjrobertson
Copy link
Contributor Author

pjrobertson commented Apr 6, 2024

No not yet, I've had a busy few days! Will try to look at it in the next 2-3 days and reply here :D

enoch85 added a commit that referenced this issue Apr 15, 2024
* part fix #2619

make sure the bin is exceutable

Signed-off-by: Daniel Hansson <mailto@danielhansson.nu>

* one solution for all files

Signed-off-by: Daniel Hansson <mailto@danielhansson.nu>

* Update nextcloud_update.sh

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: Daniel Hansson <mailto@danielhansson.nu>

* Update nextcloud_update.sh

Signed-off-by: Daniel Hansson <mailto@danielhansson.nu>

* tested and works

Signed-off-by: Daniel Hansson <mailto@danielhansson.nu>

---------

Signed-off-by: Daniel Hansson <mailto@danielhansson.nu>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@Reis-A
Copy link

Reis-A commented Apr 26, 2024

Thank you for this! I had no problems with upgrade to 28.0.5
This finally closes #2578 which was the same issue., pretty much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants