Project

General

Profile

Bug #1310

Check pakcage .inc files before including to avoid potential breakage

Added by Jim Pingle about 8 years ago. Updated about 3 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Package System
Target version:
Start date:
02/27/2011
Due date:
% Done:

0%

Estimated time:
Affected Version:
All
Affected Architecture:
All

Description

To prevent a broken package from causing even more breakage, we should probably do a check on the package .inc files before including them.

Something like the code dvserg suggests here:
http://forum.pfsense.org/index.php/topic,32867.0.html

if (exec("php -l $pkg_inc") )  {
        require_once($pkg_inc);
        ...

Ideally package authors should be more careful about function name collisions and similar problems but this would at least prevent the breakage from causing too much collateral damage in the process.

History

#1 Updated by Ermal Luçi about 8 years ago

That is too much overhead to be done dynamically.
eval() was supposed to help here but you cannot catch fatal parser errors even there so no escape here really.
Unless we poot hooks on the pacakge tool repo to validate the php before pushing.

#2 Updated by Ermal Luçi about 8 years ago

  • Target version changed from 2.0 to Future

#3 Updated by Stilez y over 6 years ago

Possible solution, though I may be naive about thee details and exact implementation of future package managers. However this looks a good way to go anyway since a package can get corrupted.

PFsense core files have a hash listing already in the installer (MD5 not SHA* but it's there). "Official" packages may or may not have hash entries but as official packages, the hash should be known and standard. So in principle the user can be told if any "official" files PFsense loads and may execute have unexpected hashes, without any significant load or delay.

At best an admin option - "Checksum all PFsense files (including known packages) on startup, first load, and update?"

Unknown or user modded packages can't be tested for validity without running them as the bug notes above say, but that's not an issue. Most packages run will be standard ones, and for any given install or user, a package only needs verifying once for non-breakage; after that the hash can be used to confirm it's a "non-pfsense-breaking" package file in future, and the code doesn't need to be exec()'ed to re-test this. ***

In brief can this bug be solved by something like this:

1) PFsense on startup checks its own (non package) files to their hashes, reports discrepancies.
2) Any package loaded has its files checked to their hashes if they exist, and their "OK-OFFICIAL/OK-TESTED-AUTO/OK-TESTED-MANUAL/FAIL-TESTED-AUTO/FAIL-TESTED-MANUAL/UNCHECKED" status can be found.
3) Any package without prior hashing or status="UNCHECKED" or "FAIL", the user is notified, and asked if they want to ignore, modify the status manually, or have PFsense try to exec the program to check for simple issues.
4) If they set the status manually or have PFsense exec() it, then yes it takes a little time, but it won't need to be done in future or on other installs, it is once only for that package - the info will be held, backed up, and can be re-used if needed again to quickly verify this is a package that's been checked before on this platform.

  • perhaps load order or other packages might interact, some thought needed? But the idea seems valid. Once it's been checked in a given scenario (once-off task per package if the user wishes -> not laborious and optional) it shouldn't need retesting for basic "breaks pfsense" so dynamic testing on load shouldn't be required. Main thing is to emphasise this isn't claiming the package will work, just that it doesn't seem to break pfsense.

#4 Updated by Jim Thompson about 3 years ago

  • Assignee set to Jim Pingle

seems outdated now. Close? assigned to Pingle

#5 Updated by Chris Buechler about 3 years ago

  • Status changed from New to Resolved
  • Target version changed from Future to 2.3

the root issue here is addressed in 2.3 because it runs in a different PHP instance, so the breakage won't spread. That addresses the original problem.

Also available in: Atom PDF