Skip to content

Exclude mimetypelist.js from signing/integrity check #29308

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.

Sign up for GitHub

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

Merged
PVince81 merged 2 commits into from
Oct 20, 2017
Merged

Exclude mimetypelist.js from signing/integrity check #29308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8af5056
Revert "Exclude mimetypelist.js from integrity check"
VicDeo Oct 19, 2017
8c8c354
Add ability to exclude items from signing by path and filename
VicDeo Oct 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions lib/private/IntegrityCheck/Checker.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,22 +179,6 @@ private function generateHashes(\RecursiveIteratorIterator $iterator,
continue;
}

$exclusionList = [
'/core/js/mimetypelist.js'
];

$found = false;
foreach ($exclusionList as $entry) {

@PVince81 PVince81 Oct 20, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you likely still need to keep something here to prevent generating the hashes in the first place ?

@VicDeo VicDeo Oct 20, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@PVince81 generateHashes takes a folder iterator. I excluded this path from the items returned by this iterator.

@VicDeo VicDeo Oct 20, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's reused everywhere, so I basically made core/js/mimetypelist.js completely invisible for signing code. See

core/lib/private/IntegrityCheck/Checker.php

Line 157 in 8c8c354

private function generateHashes(\RecursiveIteratorIterator $iterator,

core/lib/private/IntegrityCheck/Checker.php

Line 386 in 8c8c354

$currentInstanceHashes = $this->generateHashes($this->getFolderIterator($basePath), $basePath);

if($filename === $this->environmentHelper->getServerRoot() . $entry) {
$found = true;
break;
}
}

if ($found) {
continue;
}

// The .user.ini and the .htaccess file of ownCloud can contain some
// custom modifications such as for example the maximum upload size
// to ensure that this will not lead to false positives this will
Expand Down
23 changes: 21 additions & 2 deletions lib/private/IntegrityCheck/Iterator/ExcludeFileByNameFilterIterator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
namespace OC\IntegrityCheck\Iterator;

/**
* Class ExcludeFileByNameFilterIterator provides a custom iterator which excludes
* entries with the specified file name from the file list. These file names are matched exactly.
* Class ExcludeFileByNameFilterIterator provides a custom iterator which excludes
* entries with the specified file name or name part from the file list.
*
* @package OC\Integritycheck\Iterator
*/
Expand Down Expand Up @@ -53,6 +53,18 @@ class ExcludeFileByNameFilterIterator extends \RecursiveFilterIterator {
private $excludedFileNamePatterns = [
'/^\.webapp-owncloud-.*/', // Gentoo/Funtoo & derivatives use a tool known as webapp-config to manage wep-apps.
];

/**
* Array of excluded path and file name parts. Those are not scanned by the integrity checker.
* These strings are regular expressions and any filepath
* matching these expressions are ignored.
*
* @var array
*/
private $excludedFilePathPatterns = [
'|/core/js/mimetypelist.js$|', // this file can be regenerated with additional entries with occ maintenance:mimetype:update-js
];


/**
* @return bool
Expand All @@ -75,6 +87,13 @@ public function accept() {
return false;
}
}

$currentFilePath = $current->getPathname();
foreach ($this->excludedFilePathPatterns as $pattern){
if (preg_match($pattern, $currentFilePath) > 0){
return false;
}
}

return true;
}
Expand Down
1 change: 1 addition & 0 deletions tests/data/integritycheck/app/core/js/mimetypelist.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
27 changes: 19 additions & 8 deletions tests/lib/IntegrityCheck/Iterator/ExcludeFileByNameFilterIteratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,31 @@ public function setUp() {

public function fileNameProvider(){
return [
['a file', true],
['Thumbs.db', false],
['another file', true],
['.directory', false],
['.webapp-owncloud-obee', false],
['wx.webapp-owncloud-obee', true],
['', 'a file', true],
['', 'Thumbs.db', false],
['', 'another file', true],
['', '.directory', false],
['', '.webapp-owncloud-obee', false],
['', 'wx.webapp-owncloud-obee', true],
['/core/js', 'mimetypelist.js', false],
['/core/css', 'mimetypelist.js', true],
['/core/js', 'typelist.js', true],
['/hardcore/js', 'mimetypelist.js', true],
['/js', 'mimetypelist.js', true],

];
}

/**
* @dataProvider fileNameProvider
* @param string $path
* @param string $fileName
* @param bool $expectedResult
*/
public function testAcceptForFiles($fileName, $expectedResult){
public function testAcceptForFiles($path, $fileName, $expectedResult){
$iteratorMock = $this->getMockBuilder(\RecursiveDirectoryIterator::class)
->disableOriginalConstructor()
->setMethods(['getFilename', 'isDir'])
->setMethods(['getPathname', 'getFilename', 'isDir'])
->getMock()
;
$iteratorMock->method('getFilename')
Expand All @@ -66,13 +73,17 @@ public function testAcceptForFiles($fileName, $expectedResult){
$iteratorMock->method('isDir')
->will($this->returnValue(false));

$iteratorMock->method('getPathname')
->will($this->returnValue("$path/$fileName"));

$this->filter->method('current')
->will($this->returnValue($iteratorMock))
;

$actualResult = $this->filter->accept();
$this->assertEquals($expectedResult, $actualResult);
}


/**
* @dataProvider fileNameProvider
Expand Down