Show port provenance and prevent source name/prefix spoofing#331
Conversation
|
Want to highlight this note: the default theme's info template strings changed, so existing themes.po translations were regenerated in a separate commit. I can easily revert the commit if the diff is undesirable. |
bmdhacks
left a comment
bmdhacks
left a comment
There was a problem hiding this comment.
I'm not sure if this increased verbosity is worth the benefit.
| if port_source in (None, "", "Unknown", "file", "url"): | ||
| port_source = port_info.get('source', {}).get('repo', None) or "" | ||
|
|
||
| description = port_info['attr']['desc'] |
There was a problem hiding this comment.
Why inject the source into the description? Wouldn't it be better to keep these two semantic things distinct and have the source displayed in the layout side of things instead of polluting description?
Ok, I read the submission wall of text, but why would I care about the source of a port if I was running an old version of portmaster that doesn't pull from any different sources? Also I don't think we want to broadcast MV sources.
There was a problem hiding this comment.
The fallback you're referring to targets old theme zips, not old GUI versions. It allows old theme zips to display the new variable. As for description injection, fine to remove if not wanted but the port info list is a small font that can be overlapped by a screenshot. I was going for visibility.
With MV, it's due to the presence of two files. The source comes from the name key. I can add a way to merge the two official sources into a single display.
| "Körtid: {port_info.runtime} ({port_info.runtime_status}){endif}" | ||
|
|
||
| #: default_theme/theme.json:347, default_theme/theme.json:846 | ||
| msgid "" |
There was a problem hiding this comment.
Is this newline correct? This seems like a lot of auto-generated redundancy but perhaps I don't know how .po files work.
There was a problem hiding this comment.
That's standard gettext multi-line syntax, it's correct. But yeah I'm not all too sure about these .po changes; the themes I don't think have been updated in two years, so it could just be an artifact of running the generator script for the first time since new port info was added. Again I can revert the commit that generated them if it's easier to do it with an automation or something.
| source_data['last_checked'] = None | ||
| source_data['data'] = {} | ||
|
|
||
| # A reserved prefix only loads from its bundled file, even if the |
There was a problem hiding this comment.
This is a lot of extra code to protect against a security issue that's not really relevant. "Imposter" source files are fine if somebody wants to do that on their handheld.
There was a problem hiding this comment.
You're looking at user control, and I agree, however my approach has more to do with injection. Not that I would do this, but take Pharos for example. Pharos can already download my gmtoolkit.aarch64 binary and put it in $controlfolder for a user. It is entirely possible for a bad actor to sideload a third-party repo file without the user's consent.
There was a problem hiding this comment.
??? we run ports at root, a bad actor can do whatever tf they want. Unless you can demonstrate a remote vuln without device or access to an installed port (and don't give me DNS poisoning like I mean something real) this is a dead end and I would vote against adding cruft for a non-issue.
This feels to me like an LLM told you that this is a security vulnerability because the LLM doesn't understand the zero-security model we use for handheld devices where we run unaudited chinese firmwares and assume that devices are already rootkitted so our model is to just keep any sensitive info off-device (and frankly suggest partitioning your home network too).
There was a problem hiding this comment.
This is me recognizing JohnnyOnFlame's concerns, finding that when I use portmaster gui's infrastructure as advertised that my port is indistinguishable from a portmaster official port, and promptly fixing it because if I can do it, then so can anyone else.
Ok not to add. Just pointing it out and attempting to contribute a solution.
2 participants
Adds visible provenance for every port and hardens source identity against impersonation.
Changes
Provenance display
port_info.sourcetemplate variable for themes. The default theme rendersSource: Xin the port info panels for all ports (official ones showSource: PortMaster).status.source); theUnknown/file/urlplaceholders used for sideloaded ports fall back to the source currently offering the port, or show nothing if no source does.Spoofing resistance
load_sources()reserves the bundled prefixes: a file claimingpmorpmmvonly loads if it is the bundled source file, independent of filename sort order. Neither a later file (old last-wins bug) nor an earlier-sorting000_spoof.source.jsoncan take the official prefix. Duplicate prefixes between third party files fall back to first-file-wins with a logged error, instead of silently replacing the earlier source.020_portmaster.source.json,021_portmaster.multiverse.source.json). A third party source calling itself "PortMaster" displays asPortMaster (040_spoof). Name collisions between third party sources qualify all parties the same way.status.source, so an installed port keeps honest provenance even after the offering source is removed.Notes
themes.potranslations were regenerated in a separate commit. This can easily be reverted if the diff is undesirable.source.repois attached at runtime only.