enable-zero-configuration and policy.xml

[philtay]

Is there any way to embed a custom policy.xml at build time? If not, please consider this question as a feature request.

[mikayla-grace]

An interesting conundrum. A zero-configuration build that permits configuration. Why not use the default build that permits configuration if you want to honor the security policy. You can also include your policy in MagickCore/policy.c source module before you build. Not sure it makes sense to have a zero-configuration for all configuration files other than the security policy.

[philtay]

An interesting conundrum. A zero-configuration build that permits configuration. Why not use the default build that permits configuration if you want to honor the security policy.

IMO enable-zero-configuration should mean zero configuration after build. For obvious reasons I must specify a security policy. The default one lacks several things I have to enforce. At the same time I don't want config files floating around after build.

You can also include your policy in MagickCore/policy.c source module before you build.

It sounds brittle. Do you mean I should edit a source file before each build? We're in a CI context and a sed on a semi-random file of a 3rd party project isn't very ideal.

Not sure it makes sense to have a zero-configuration for all configuration files other than the security policy.

It makes sense to give this opportunity for all config files, but policy.xml would be a great start for sure.

[mikayla-grace]

A robust solution eludes us given any solution must work cross-platform from Linux to Windows to an iPhone. To provide a workable solution, we will create a zero-configuration header file with all the configurations including the security policy. You can then copy it, edit it to reflect your local requirements, and replace it in the source tree before you compile IM with zero-configuration enabled. If you have a better suggestion that is cross-platform, let us know.

[philtay]

Sure, the best way would be a ./configure flag (e.g. ./configure --security-policy=/path/to/policy.xml). Embed the specified file at compile-time and use it at run-time. This solution is applicable to all config files.

[philtay]

Another (unrelated) thing: the latest version of ImageMagick available on GitHub is 7.0.5-0. It looks like GitHub is out of sync.

[mikayla-grace]

We had considered your proposal previously but it fails the "robust cross-platform solution test." The zero-configuration header file we proposed works cross platform. However, your proposal is likely the cleanest approach to zero configuration so we will likely implement this solution within a few weeks.

Working on the Github issue, stand by.

[philtay]

Very good. Hope to see it soon. A secure by default ImageMagick would be really great. Also: give to the user the opportunity to override the embedded policy. If a policy.xml can be found at run-time, it will prevail over the embedded one. In this way you shift away from you the security burden. Packagers can define a default secure policy which makes sense for their environment at build-time and, if some user is not happy with it, a standard policy.xml can still be used to replace the default.

[mikayla-grace]

After reconsidering your proposal for zero-configuration, the security policy must be compiled at build time suggesting the zero-configuration header file might be the best solution-- and its cross-platform. We set up a sample policy, you make custom edits, then build, then deploy. We can't readily convert a policy in XML to a "baked in" policy at build time.

[philtay]

I don't understand why you need to "compile" and "convert" a policy. It's going to require a XML parser at build time. You should just embed the config file and use/parse it at run time. There are several ways to accomplish that. A quick search on the subject will reveal them. Two links to start:

http://stackoverflow.com/questions/4864866/c-c-with-gcc-statically-add-resource-files-to-executable-library
https://github.com/graphitemaster/incbin

[mikayla-grace]

The links describe methods that are not robustly cross platform. For example INCBIN says for Windows: "we ship a tool which can process source files containing INCBIN macro usage..." We're not interested in depending on external tools. We could have the XML as string inside a zero-configuration.h header file. but even then some compilers may choke if the XML exceeds certain limits such as 65535 characters. We will continue to research a robust cross-platform solution and try to find an optimal solution.

[philtay]

@mikayla-grace I saw you decided to adopt the header file method. We'd really like to use it but, unfortunately, we can't for the reasons explained above. Considering that we are currently using the C MagickWand API, the best for us would be to have the ability to set the policy at initialization time (i.e. immediately after MagickWandGenesis). Is there any plan to add a public facing method like MagickSetSecurity(char *domain, char *name, char *value) or similar?

[mikayla-grace]

Having a method to set a security policy is problematic because any user could then overwrite the policy defined by the distributor of the package. Many Linux systems, for example, set a security policy. A user can reduce certain policy restrictions but not increase them per the system security policy. Say a Linux instance sets the image width and height policy maximums to 4096. The system administrator would not want users to override the maximums with a call to MagickSetSecurity(). Its the same with the header methods, these are built in and cannot be changed. We recognize a user could download ImageMagick, set their own security policy, and build and execute, however, we cannot control this use case.

[philtay]

MagickSetSecurity should not override policy.xml, just complement it. The most restrictive settings shall always prevail. If policy.xml sets the max height to 4096 then MagickSetSecurity("resource", "height", "8192") should fail. The initial implementation could be even simpler: MagickSetSecurity succeeds only if the limit it tries to set is not enforced anywhere else, regardless of the value. For instance, if max height is set in policy.xml, then you can't alter this value using MagickSetSecurity.

Basically the use case we're trying to address is to let more "advanced" users to fully customize ImageMagick at build-time and/or run-time. To us ImageMagick is a library not a CLI and in our Docker container we don't even have an /etc directory to put configuration files into. It's just a statically linked self-contained binary. However we still need a security policy because it's a public facing microservice. More in general, I think there should be a way to securely ship and execute ImageMagick without depending on external resources.

[mikayla-grace]

Have you looked @ the --disable-installed option? With this option, the user can set the policy in ~/.config/ImageMagick/policy.xml-- and the system policy is not consulted. The downside is the administrator cannot override the users policy.

Your "simple" implementation of MagickSetSecurity() may work. We could implement that but only if the --disable-installed option does not meet your expectations. Let us know.

[philtay]

First of all, thanks for the quick reply and for your help.

In the Docker image there isn't a ~/ directory, not even a filesystem hierarchy. We use scratch as base image. This is a common pattern when you ship a single statically linked executable. The application is written in Go and we use CGO to invoke the MagickWand API.

In our case having something like MagickSetSecurity is a clear win and I think that a lot of MagickWand wrappers out there will end up using it (e.g. PHP, Python, etc.). Many of them are web oriented, a context where security is paramount. The authors of these wrappers will have the opportunity to provide "secure-by-default" libraries to their users. They could also give them the possibility to tweak the security settings directly in their favorite language (Python, Ruby, etc.).

[mikayla-grace]

We propose a simple MagickSetSecurityPolicy() method that accepts a single parameter, an XML string with the security policy per https://www.imagemagick.org/script/security-policy.php. The policy is only applied if the security policy has not already been set and of course if the XML parses. As you know, when it comes to security, simple is desirable. Best practices requires that this method is called right after MagickCoreGenesis().

[philtay]

I like it. +1

Only a question: will MagickSetSecurityPolicy work if the enable-zero-configuration flag is set? It looks like that a default policy is automatically specified in zero-configuration mode.

[mikayla-grace]

Good point. Fixed. Building a Beta release now with the latest patches.

[philtay]

Thank you