Bug 2279608 - Nextcloud complains moved license files have not passed integrity check
Summary: Nextcloud complains moved license files have not passed integrity check
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: nextcloud
Version: 40
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Sergio Basto
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2024-05-07 16:59 UTC by Andrew Bauer
Modified: 2024-07-02 20:15 UTC (History)
2 users (show)

Fixed In Version: nextcloud-28.0.6-5.fc41 nextcloud-28.0.6-6.fc40
Clone Of:
Environment:
Last Closed: 2024-06-24 15:53:26 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
list of files not passing integrity check (133.58 KB, text/plain)
2024-05-07 16:59 UTC, Andrew Bauer
no flags Details

Description Andrew Bauer 2024-05-07 16:59:19 UTC
Created attachment 2031968 [details]
list of files not passing integrity check

Description of problem:
From the Administration Overview screen, Nextcloud complains:

>Some files have not passed the integrity check. List of invalid files… Rescan… For more details see the documentation ↗.

This is from a fresh install of Nextcloud. See attached. Full list of files consists of moved License files and deleted .git folders, which was done during the rpm build process.

Version-Release number of selected component (if applicable):
28.0.4-3

How reproducible:
Always

Steps to Reproduce:
1. dnf install nextcloud
2. point browser to http://localhost/nextcloud
3. observe mentioned error from the administration overview screen

Actual results:
Error message persists

Expected results:
No error

Additional info:
While Nextcloud presents this as an error, Nextcloud continues to operate normally, so one could certainly argue this is just a warning, despite the red error text.

From what I have searched, the nextcloud developers, by design, have not included a way to add runtime exceptions to this integrity check.

Before I start looking into how easy (or not) it might be to modify the integrity signature file during build, I thought I'd ask the package maintainer if he is aware of this issue and/or has any ideas on how to address this.

While one could certainly ignore this, the list of files is so large, it really defeats the purpose of having an integrity check... if something nefarious did indeed happen to one's system, the likely hood of noticing the modified file would be small, due to all the noise.

Comment 1 Sergio Basto 2024-05-07 18:54:37 UTC
let me know if you have a fix , you are welcome on co-maintain this package

Comment 2 Andrew Bauer 2024-05-08 19:27:10 UTC
Thanks for the response Sergio. Did you recently take over this package? I seem to recall there was a different maintainer.
If that was you, thanks for cleaning up all the open bug reports. All those bug reports made it difficult to know what was still relevant.

Anyways, I made some headway on fixing the invalid signatures.  The following steps generate new signatures file, thus making the errors go away:

- add the following to config.php: 'integrity.check.disabled' => false
- trigger a rescan from the admin portal (http://localhost/nextcloud/index.php/settings/integrity/rescan)
- remove the integrity check line from config.php or just set it to true

Will add them to %post, once I figure out how to force a rescan from the commandline, rather than the web portal.

Comment 3 Sergio Basto 2024-05-08 20:43:15 UTC
Yes, I recently took over the package because it was orphaned and also cleaned up the bug reports.

ok 

the invalid signature is more missing files ...

Comment 4 Andrew Bauer 2024-05-15 13:33:51 UTC
It seems I spoke too soon. Disabling integrity check, rescanning, and then re-enabling the check seems to work for the current login session only.
Once I log out then back into the Nextcloud portal, the same warning comes back. The Nextcloud forum claimed this method worked, but perhaps that was for a previous version of Nextcloud.

I have not found a simple way to generate new signature files, either. That would require the private key for the core and all the private keys for every app under the 3rdparty folder. Understandably, the private keys don't seem to be available. I'm sure they don't want these keys to get out.

This leaves us with a couple of hard choices:
- Leave the package as-is and recommend to permanently disable integrity check, for those that don't want to see the warnings in the admin portal
- Don't move or delete any license files or .git files or another file during the build process. We of course could symlink the LICENSE files, to make them viewable from a single spot.

Comment 5 Sergio Basto 2024-05-16 11:21:53 UTC
please see if https://src.fedoraproject.org/rpms/nextcloud/pull-requests #5, #9, #10 and #11 have something about this , I will try review them soon

Comment 6 Andrew Bauer 2024-05-16 12:56:09 UTC
Thanks, I'll take a look.

Comment 7 Sergio Basto 2024-05-19 23:43:08 UTC
I reviewed all PR(s) 

We got [1] or [2] , but I don't' think that fixes anything, the problem of integrity is about License files are in list to check and but they were removed by us.  

I merged 3 improvements from PR(s) and updated to 28.0.5 

[3]
https://src.fedoraproject.org/rpms/nextcloud/c/662a74cdbfaa112ae324f8ecceb55f6639aff63f

[1]
https://src.fedoraproject.org/rpms/nextcloud/pull-request/11#request_diff

# Handle license files
mv COPYING LICENSE-nextcloud
find -name '*LICENCE*' -o -name '*LICENSE*' -o -name '*COPYING*' | grep -v LICENSE-nextcloud | while read f; do \
 	nf=`echo "$f" | sed -e 's|^\./||g; s|/$||g; s|THIRD-PARTY-LICENSES||g; s|LICENSE.md||g; s|LICENSE.txt||g; s|LICENSE_||g; s|LICENSES.md||g; s|LICENCE.md||g; s|LICENSES||g; s|LICENSE||g; s|COPYING||g; s|/$||g; s|/|--|g; s| |_|g;'`; \
 	nf="LICENSE---$nf"; \
 	mv "$f" "$nf"; \
done


[2]
https://src.fedoraproject.org/rpms/nextcloud/pull-request/10#request_diff

rename_license_files() {
    trap "$(shopt -p nullglob globstar)" RETURN
    shopt -q -s nullglob globstar
    for file in */**/*LICEN{S,C}E* */**/*COPYING; do
        dst=${file%.txt};
        dst=${dst//\//-};
        if ! [[ "$file" -ef "$dst" ]]; then
            mv "$file" "$dst"
        fi
    done
}

rename_license_files

Comment 8 Andrew Bauer 2024-05-29 16:45:34 UTC
I did a clean install of the latest 25.05-2 package and got the same complaints from the integrity checker.

The signature files for the integrity checker come bundled with the Nextcloud tarball from their download site. Interestingly, the tarball from the Nextcloud github page does not have these signature files. For giggles, I mock built Nextcloud using the github tarball instead, hoping perhaps Nextcloud would autogenerate the signature files, seeing that they were missing. No dice. That just caused the integrity checker to not activate.

Even the signature file itself has been hashed. Thus, modifying it in any way simply generates more complaints from the integrity checker.

I took at a look at what other rpm distros do for this package. Mageia doesn't move or delete any files at all.

Interestingly, OpenSuse used to delete .git files but ran into the same issue with the integrity checker. They have since stopped:
https://src.opensuse.org/rpm/nextcloud/src/branch/factory/nextcloud.spec#L156

I could possibly open a issue on nextcloud github, asking they exclude certain files from the integrity check, but I'm not optimistic I'd get any movement on this. They don't really have any incentive to make this change.

At this stage I'd recommend we stop deleting .git files and folders (as much as it pains me), and symlink LICENSE and README files, rather than move them.

If you agree to this change, I'd be willing to do the work. 

You can make me a co-maintainer if you want. My long term goal would be to build this package (and missing dependencies) for el9, so I can move my work out of Copr.

My FAS: kni

Comment 9 Sergio Basto 2024-05-29 18:28:38 UTC
I added you (kni) as admin

Comment 10 Andrew Bauer 2024-06-06 12:23:09 UTC
I have a working mock build, which eliminates virtually all the integrity check issues. I did this by not deleting any git or other extraneous files, and I also symlinked the documentation and license files back to their original locations, under /usr/share/nextcloud.

However, once that noise was removed, it revealed integrity constraints violations on the following:
- occ
- updater.php
- .htaccess

The first two are because these files get patched during the build, while the last we simply don't include in the rpm. We do this for good reason, however, so I'm not inclined to simply not patch the first two nor include an htaccess file in the package.

It does look like it is possible to call "occ integrity:sign-core" and re-sign the core with certs generated from openssl. I will try to implement that in %post, and see if that can get the integrity check issues to go away completely.

If that does not work, I will fall back to Plan C. Disable integrity check altogether in config.php, and add a note that recommends the end user use nextcloud from another source, if the integrity check is important to them.

Comment 11 Sergio Basto 2024-06-06 12:55:09 UTC
(In reply to Andrew Bauer from comment #10)

> It does look like it is possible to call "occ integrity:sign-core" and
> re-sign the core with certs generated from openssl. I will try to implement
> that in %post, and see if that can get the integrity check issues to go away
> completely.


in integrity check, is it possible not check the removed files ? especially .git and ReadME(s), I remember that I had that warning , because for some reason one README was removed

Comment 12 Andrew Bauer 2024-06-06 14:31:46 UTC
Yeah, good question. Unfortunately, no. The integrity check computes a hash for all files found within the folder structure. If any file is modified or removed, the integrity check complains. 

The hashes are stored in signature.json which comes bundled with the release tarballs on download.nextcloud.com. Unfortunately, a hash is computed against the signature file as well... so any modifications to the signature file also triggers another alert.

I could potentially ask the Nextcloud developers to exclude certain files from their integrity check, but we would still be left with what-to-do about occ, updater.php, and htaccess.  

This is why I am going to explore generating our own signature files. If I can get that to work, then we should be able to make any modifications we want to the core w/o tripping the integrity checker.

Comment 13 Andrew Bauer 2024-06-11 14:14:49 UTC
The occ tool reported the core was successfully re-signed with a new private key and certificate, but then the administration panel reported a signature exception.

I've created an issue on github, to see what kind of feedbacks I might receive:
https://github.com/nextcloud/server/issues/45792

Comment 14 Andrew Bauer 2024-06-23 20:30:37 UTC
Here is a patch, I'm not proud of, but it does silence the integrity checker for specifically named files:
https://github.com/knight-of-ni/server/commit/c7fc5070c84dedc363a476b69b26a4753c02f1e7

Note this permanently silences integrity check alerts for the files we patch during the build process, such as occ and Updater.php. If something malicious were ever to happen to the content of these files, we would never know it.

I suppose this is better than completely turning off the integrity checker.

I'm not ready to commit this just yet, which is why I am putting a reference to it here.

Comment 15 Fedora Update System 2024-06-24 15:48:59 UTC
FEDORA-2024-69e16fcae9 (nextcloud-28.0.6-5.fc41) has been submitted as an update to Fedora 41.
https://bodhi.fedoraproject.org/updates/FEDORA-2024-69e16fcae9

Comment 16 Fedora Update System 2024-06-24 15:53:26 UTC
FEDORA-2024-69e16fcae9 (nextcloud-28.0.6-5.fc41) has been pushed to the Fedora 41 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 17 Fedora Update System 2024-06-24 16:09:17 UTC
FEDORA-2024-aad04ea613 (nextcloud-28.0.6-6.fc40) has been submitted as an update to Fedora 40.
https://bodhi.fedoraproject.org/updates/FEDORA-2024-aad04ea613

Comment 18 Fedora Update System 2024-06-25 02:01:14 UTC
FEDORA-2024-aad04ea613 has been pushed to the Fedora 40 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2024-aad04ea613`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-aad04ea613

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 19 Fedora Update System 2024-07-02 20:15:15 UTC
FEDORA-2024-aad04ea613 (nextcloud-28.0.6-6.fc40) has been pushed to the Fedora 40 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.