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

Restrict socket connections #9779

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

varjolintu
Copy link
Member

@varjolintu varjolintu commented Aug 22, 2023

Adds a new Client Restrictions table to Database Settings. When enabled, only the binaries in the table are allowed to connect to the browser extension side.
The binaries are identified by absolute path and MD5 file hash. Currently the feature is supported in Windows, macOS, Linux and FreeBSD.

Requires #9406 and #9485 as the base.

Screenshots

client_restrictions

Testing strategy

Manually.

Type of change

  • ✅ New feature (change that adds functionality)

@varjolintu varjolintu marked this pull request as draft August 22, 2023 18:19
@varjolintu varjolintu changed the title Feature/restrict socket connections Restrict socket connections Aug 22, 2023
@droidmonkey
Copy link
Member

Super cool


// Get process path from PID
char buf[PATH_MAX];
auto procPath = QString("/proc/%1/exe").arg(pid);

Choose a reason for hiding this comment

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

IIRC, FdoSecrets does something similar for its client authorization. You can search its code for "proc" to find where that is. This should probably be moved to a common function in OSutils.

This whole PR is similar in concept to #6458, so there is likely to be some common functionality. Maybe a ClientAunthenticator class?

Copy link
Member Author

Choose a reason for hiding this comment

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

OSUtils is a fine place for this, yes. And I agree common functions should have just one class. Going to look at this again after those two required PR's for this one are merged.

@deathtrip
Copy link

MD5 has been broken for a long time. Why not use a secure hash function?

@varjolintu
Copy link
Member Author

MD5 has been broken for a long time. Why not use a secure hash function?

Is there a need for that? The data is only stored inside your database that is already encrypted.

@droidmonkey
Copy link
Member

droidmonkey commented Feb 4, 2024

MD5 is only broken for cryptographic purposes. It's a perfectly fine option for non-critical, performance sensitive hashing. With that said, there is really no reason not to use SHA-256.

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

Successfully merging this pull request may close these issues.

None yet

4 participants