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

Introduce user-defined wrapdb mirror #13953

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

klokik
Copy link
Contributor

@klokik klokik commented Nov 24, 2024

This consolidates all queries to the wrapdb to go through the open_wrapdburl() function. The function handles a domain-specific URL scheme, wrapdb. When encountered, it substitutes the scheme and an authority(net loc) with either the upstream wrapdb address or a user-defined one stored in the subprojects/wrap-sources.json file. The file may be checked into the project's version control system for persistent use.

The user should use meson wrap set-sources <wrapdb-address> command to create the file.

Justification:

  • wrapdb servers may not always be available
  • internal organization policies prohibit using external sources during the build process
  • wraps for non-public proprietary subprojects

@klokik klokik requested a review from jpakkane as a code owner November 24, 2024 11:33
@klokik klokik force-pushed the wrapdb-mirror branch 2 times, most recently from 1182455 to 840f286 Compare November 24, 2024 12:27
@jpakkane
Copy link
Member

user-defined one from the MESON_WRAPDB_MIRROR environment variable.

This has the potential to be a really nasty way of hijacking user queries for malware injection. Not to mention all the other ways in which environment variables are terrible for storing persistent configuration.

@klokik
Copy link
Contributor Author

klokik commented Nov 25, 2024

@jpakkane you mean adding more malware, once attacker already gained control over the users shell environment? Or getting the first handful of malware by user getting source code from the untrustworthy mirror?

I'd be glad to hear for alternative more secure options. Providing the mirror address as a meson wrap command argument on each invocation would be acceptable for me, but less convenient in day-to-day use.
Other than that meson does not seem to use any persistent configuration. This does not belong to the machine files either.
How about a meson wrap set-mirror (address) that will be stored in the subprojects dir, along the local version of db?

Getting code from untrusted sources IMO is a user problem. I honestly don't expect anyone to use random third party mirror found on the search engine.

@klokik
Copy link
Contributor Author

klokik commented Nov 25, 2024

Ok, I can see this being abused on a build machine, as a stepping ladder of a more complex attack.What's worse - it will affect users that are not aware of this variable.

@klokik klokik force-pushed the wrapdb-mirror branch 4 times, most recently from babf222 to 51a23d2 Compare November 25, 2024 22:17
@klokik
Copy link
Contributor Author

klokik commented Nov 25, 2024

@jpakkane, I changed it to use an explicit user-provided address, unaffected by environment variables, limited to a single workspace.

It looks tempting to store the address inside subprojects/wrap.json along with the revisions it provides, but this will require breaking file layout and annoyance for the end-users.

```

You will be limited to the wraps available on the mirror, only one source is used at a time.
The address is stored in `subprojects/wrapdb-mirrors.json`, remove the file to use upstream address.
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure I like the UI for this feature. I think that the use case you have in mind for it is very different from what I have in mind. Why isn't a command line argument to meson wrap enough? If this is a functionality intended mostly for corporate environment where access to the "upstream" wrap database is somehow forbidden, doesn't a network level redirect provide a transparent solution for the same problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Command-line argument will have to be set for each of the update and install sub-commands, and transparent fallback can not happen unless the address is stored somewhere.

May I propose an optional positional argument to the update-db command that will create the "persistent" file?

Actually, you caught my mental confusion on what what it is for. I'm calling it a mirror, but it is not what I want it for - I want a different wrap-db-source.

This is indeed for a corporate environment, but a simple redirect is not acceptable. All sources should be stored on the company-controlled machines, wraps and source updates have to be audited to an extent. Wraps for the subprojects developed by other teams, that have no interest in using meson yet are to be served from this source as well.

Copy link
Member

Choose a reason for hiding this comment

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

This is indeed for a corporate environment, but a simple redirect is not acceptable. All sources should be stored on the company-controlled machines, wraps and source updates have to be audited to an extent. Wraps for the subprojects developed by other teams, that have no interest in using meson yet are to be served from this source as well.

I don't understand why a redirect is not acceptable. If that is your policy, most likely you have measures in place that forbid access to https://wrapdb.mesonbuild.com. You just need to change those measures to redirect access to that resource to https://wrapdb.localnet or whatever you like. Then you just serve your own wrap-db from that URL. Your wrap-db may contain your hand-picked selection of wraps. Why would this not work?

Copy link
Member

Choose a reason for hiding this comment

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

May I propose an optional positional argument to the update-db command that will create the "persistent" file?

AFAIK running meson wrap update-db is not required to use other meson wrap commands. Therefore, making the configuration of the wrap database URL dependent on running it seems at least counterintuitive.

One thing I haven't understood yet is whether you want a setting that is per system, per user, per project, or per specific wrap. Depending on what is the use case the configuration would need to be stored in a different location.

I am also not convinced that there is the need of a meson command to set the WrapDB URL instead that a simple configuration file format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You just need to change those measures to redirect access to that resource to https://wrapdb.localnet or whatever you like. Then you just serve your own wrap-db from that URL. Your wrap-db may contain your hand-picked selection of wraps. Why would this not work?

  1. It will break TLS
  2. Let's say that policy is only enforced on the CI machines
  3. I have no control over this redirect
  4. meson is overly attached to this specific address hardcoded in a few places

Copy link
Contributor Author

@klokik klokik Nov 27, 2024

Choose a reason for hiding this comment

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

One thing I haven't understood yet is whether you want a setting that is per system, per user, per project, or per specific wrap. Depending on what is the use case the configuration would need to be stored in a different location.

Ideally setting wrapdb source per project with all of its subprojects. + A single instance of wrap server on the network.

AFAIK running meson wrap update-db is not required to use other meson wrap commands. Therefore, making the configuration of the wrap database URL dependent on running it seems at least counterintuitive.

Certainly, it will not require update-db with the changes proposed in this MR. Only commands that query wrapdb need the update command whether with the source set or not. I don't think that one is supposed to have wrapdb.json file checked-in, and without it status/info/search/list won't work. I see what you mean now: other commands do query release data directly from the server bypassing local wrapdb.json file. I don't like it.

Copy link
Contributor Author

@klokik klokik Nov 28, 2024

Choose a reason for hiding this comment

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

May I propose an optional positional argument to the update-db command that will create the "persistent" file?

I did it, but I don't like it. Will revert to a separate wrap subcommand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So far I find a file stored in subprojects to be the most convenient:

  • does not depend on environment variables, thus cannot be easily hijacked by using environment variables
  • has to be set once per project, as opposed to a wrap subcommands argument that will have to be set repeatedly when installing each wrap
  • may be checked-in to the version control system, if project maintainer chooses to, eliminating the need for other developers to set it manually

Cons:

  • it adds steps to the usual flow of setup + compile. Some build systems, that have wrappers around configuration scripts may need custom handling. Buildroot and Yocto come to mind, but these are bad examples, as both of them explicitly disable fallback and provide own mechanisms to deal with dependencies.

mesonbuild/wrap/wrap.py Outdated Show resolved Hide resolved
This consolidates all queries to the wrapdb to go through the open_wrapdburl() function.
The function handles a domain-specific URL scheme, wrapdb. When encountered,
it substitutes the scheme and an authority(net loc) with either the upstream wrapdb address
or a user-defined one stored in the subprojects/wrap-sources.json file.
The file may be checked into the project's version control system for persistent use.

User is expected to use `meson wrap set-sources <wrapdb-url>` command to create the file.

<wrapdb-url> may be any valid url that urllib can do urlopen() for:
http, https, ftp, file protocols should work. Note that some of these are insecure.
Address may contain a path prefix, wrapdb path component will be
appended to it.
@klokik
Copy link
Contributor Author

klokik commented Dec 23, 2024

Squashed commits

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

Successfully merging this pull request may close these issues.

3 participants