Skip to content

Commit

Permalink
security: change command line embedding for oiiotool & maketx output
Browse files Browse the repository at this point in the history
It was pointed out that oiiotool and maketx's habit of embedding their
exact command line arguments in the Software and ImageHistory metadata
of output images was potentially problematic in that it might
inadvertently leak important internal data (for example, file paths
may reveal secret projects, etc.).

This patch changes the behavior of oiiotool and maketx as follows:

* The default behavior is now to use `"OpenImageIO <version> <hash>"`
  where `<hash>` is the SHA-1 hash of the command line arguments (which
  allows you to tell if two files use the same command line args, but
  doesn't let you know what they were).

* A new `--history` argument (or setting OIIOTOOL_METADATA_HISTORY env
  variable to a nonzero integer) instead uses the full command line
  arguments with no obfuscation. (This used to be the default behavior
  but is now deemed unsafe.) When the env variable is in effect, a
  `--no-history` argument explicitly disables it.

* For oiiotool, the existing `--nosoftwareattrib`, as before, causes
  the Software and ImageHistory metadata to not be modified at all.

Here is an example of the behavior:

```
$ oiiotool tmp.tif -o tmp.exr && iinfo -v tmp.exr
tmp.exr :  640 x  480, 3 channel, half openexr
    channel list: R, G, B
    Software: "OpenImageIO 2.6.2.0dev : B897973B86EC13E3F1858AAE5E057EFCCF99889A"
    ...

$ oiiotool --history tmp.tif -o tmp.exr && iinfo -v tmp.exr
tmp.exr :  640 x  480, 3 channel, half openexr
    channel list: R, G, B
    Software: "OpenImageIO 2.6.2.0dev : oiiotool --history tmp.tif -o tmp.exr"
    Exif:ImageHistory: "oiiotool --history tmp.tif -o tmp.exr"
    ...
```

Signed-off-by: Larry Gritz <lg@larrygritz.com>
  • Loading branch information
lgritz committed Apr 19, 2024
1 parent f481262 commit 751ce36
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 28 deletions.
9 changes: 9 additions & 0 deletions src/doc/imageioapi.rst
Original file line number Diff line number Diff line change
Expand Up @@ -357,3 +357,12 @@ inside the source code.
use for its thread pool. If both are set, ``OPENIMAGEIO_THREADS`` will
take precedence. If neither is set, the default will be 0, which means
to use as many threads as there are physical cores on the machine.

``OIIOTOOL_METADATA_HISTORY``

If set to a nonzero integer value, `oiiotool` will by default write the
command line into the ImageHistory and Software metadata fields of any
images it outputs. The default if this is not set is to only write the
name and version of the software and an indecipherable hash of the command
line, but not the full human-readable command line. (This was added in
OpenImageIO 2.5.0.)
29 changes: 26 additions & 3 deletions src/doc/oiiotool.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1952,10 +1952,33 @@ current top image.
are not set, only the first subimage will be changed, or all subimages
if the `-a` command line flag was used.

.. option:: --nosoftwareattrib
.. option:: --history
--no-history
--nosoftwareattrib

When set, this prevents the normal adjustment of "Software" and
"ImageHistory" metadata to reflect what :program:`oiiotool` is doing.
By default, oiiotool writes "OpenImageIO <version>" and a SHA-1 hash of
the command line as the "Software" metadata in output images.

The `--history` option appends the full command line arguments and appends
that information to the "ImageHistory" metadata as well. This behavior is
"opt-in" because some users may find it undesirable for metadata in the
image to potentially reveal any proprietary information that might have
been present in the command line arguments.

If the `OIIOTOOL_METADATA_HISTORY` environment variable is set to a
nonzero integer value, the `--history` option will be enabled by default,
but can be disabled on the command line with `--no-history`.

The `--nosoftwareattrib` option prevents even the minimal default information
from being written, so that no information about the software is written
to any metadata field.

Prior to OpenImageIO 2.5.11, the full information was always written, but
could be overridden with `--nosoftwareattrib`. Beginning with 2.5.11, the
default changed only say the software name and version (unless the
`OIIOTOOL_METADATA_HISTORY` environment variable is set), and require the
new `--history` option to cause the command line arguments to be saved as
meetadata.

.. option:: --sansattrib

Expand Down
7 changes: 6 additions & 1 deletion src/include/OpenImageIO/hash.h
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,11 @@ class OIIO_API SHA1 {
SHA1 (const void *data=NULL, size_t size=0);
~SHA1 ();

SHA1 (string_view str) : SHA1(str.data(), str.size()) { }

template<typename T>
SHA1 (span<T> v) : SHA1(v.data(), v.size()) { }

/// Append more data
void append (const void *data, size_t size);

Expand All @@ -587,7 +592,7 @@ class OIIO_API SHA1 {
}

/// Append more data from a span, without thinking about sizes.
template<class T> void append (span<T> v) {
template<typename T> void append (span<T> v) {
append (v.data(), v.size()*sizeof(T));
}

Expand Down
22 changes: 19 additions & 3 deletions src/maketx/maketx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@

using namespace OIIO;

#ifndef OIIOTOOL_METADATA_HISTORY_DEFAULT
# define OIIOTOOL_METADATA_HISTORY_DEFAULT 0
#endif


// # FIXME: Refactor all statics into a struct

Expand Down Expand Up @@ -181,6 +185,13 @@ getargs(int argc, char* argv[], ImageSpec& configspec)
bool cdf = false;
float cdfsigma = 1.0f / 6;
int cdfbits = 8;
#if OIIOTOOL_METADATA_HISTORY_DEFAULT
bool metadata_history = Strutil::from_string<int>(
getenv("OIIOTOOL_METADATA_HISTORY", "1"));
#else
bool metadata_history = Strutil::from_string<int>(
getenv("OIIOTOOL_METADATA_HISTORY"));
#endif
std::string incolorspace;
std::string outcolorspace;
std::string colorconfigname;
Expand Down Expand Up @@ -275,6 +286,10 @@ getargs(int argc, char* argv[], ImageSpec& configspec)
.help("Sets string metadata attribute (name, value)");
ap.arg("--sansattrib", &sansattrib)
.help("Write command line into Software & ImageHistory but remove --sattrib and --attrib options");
ap.arg("--history", &metadata_history)
.help("Write full command line into Exif:ImageHistory, Software metadata attributes");
ap.arg("--no-history %!", &metadata_history)
.help("Do not write full command line into Exif:ImageHistory, Software metadata attributes");
ap.arg("--constant-color-detect", &constant_color_detect)
.help("Create 1-tile textures from constant color inputs");
ap.arg("--monochrome-detect", &monochrome_detect)
Expand Down Expand Up @@ -447,9 +462,10 @@ getargs(int argc, char* argv[], ImageSpec& configspec)
configspec.attribute("maketx:cdfsigma", cdfsigma);
configspec.attribute("maketx:cdfbits", cdfbits);

std::string cmdline
= Strutil::fmt::format("OpenImageIO {} : {}", OIIO_VERSION_STRING,
command_line_string(argc, argv, sansattrib));
std::string cmdline = command_line_string(argc, argv, sansattrib);
cmdline = Strutil::fmt::format("OpenImageIO {} : {}", OIIO_VERSION_STRING,
metadata_history ? cmdline
: SHA1(cmdline).digest());
configspec.attribute("Software", cmdline);
configspec.attribute("maketx:full_command_line", cmdline);

Expand Down
45 changes: 32 additions & 13 deletions src/oiiotool/oiiotool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ using pvt::print_info_options;
# define OIIO_UNIT_TESTS 1
#endif

#ifndef OIIOTOOL_METADATA_HISTORY_DEFAULT
# define OIIOTOOL_METADATA_HISTORY_DEFAULT 0
#endif



// Macro to fully set up the "action" function that straightforwardly calls
Expand Down Expand Up @@ -170,13 +174,20 @@ Oiiotool::clear_options()
output_dither = false;
output_force_tiles = false;
metadata_nosoftwareattrib = false;
diff_warnthresh = 1.0e-6f;
diff_warnpercent = 0;
diff_hardwarn = std::numeric_limits<float>::max();
diff_failthresh = 1.0e-6f;
diff_failpercent = 0;
diff_hardfail = std::numeric_limits<float>::max();
m_pending_callback = nullptr;
#if OIIOTOOL_METADATA_HISTORY_DEFAULT
metadata_history = Strutil::from_string<int>(
getenv("OIIOTOOL_METADATA_HISTORY", "1"));
#else
metadata_history = Strutil::from_string<int>(
getenv("OIIOTOOL_METADATA_HISTORY"));
#endif
diff_warnthresh = 1.0e-6f;
diff_warnpercent = 0;
diff_hardwarn = std::numeric_limits<float>::max();
diff_failthresh = 1.0e-6f;
diff_failpercent = 0;
diff_hardfail = std::numeric_limits<float>::max();
m_pending_callback = nullptr;
m_pending_argv.clear();
frame_number = 0;
frame_padding = 0;
Expand Down Expand Up @@ -946,18 +957,22 @@ adjust_output_options(string_view filename, ImageSpec& spec,
// entire command line (eg. when we have loaded it up with metadata attributes
// that will make it into the header anyway).
if (!ot.metadata_nosoftwareattrib) {
std::string cmdline;
if (ot.metadata_history) {
cmdline = ot.full_command_line;
} else {
cmdline = SHA1(ot.full_command_line).digest();
}
std::string history = spec.get_string_attribute("Exif:ImageHistory");
if (!Strutil::iends_with(history,
ot.full_command_line)) { // don't add twice
if (ot.metadata_history && !Strutil::iends_with(history, cmdline)) {
if (history.length() && !Strutil::iends_with(history, "\n"))
history += std::string("\n");
history += ot.full_command_line;
history += cmdline;
spec.attribute("Exif:ImageHistory", history);
}

std::string software = Strutil::fmt::format("OpenImageIO {} : {}",
OIIO_VERSION_STRING,
ot.full_command_line);
cmdline);
spec.attribute("Software", software);
}

Expand Down Expand Up @@ -6849,8 +6864,12 @@ Oiiotool::getargs(int argc, char* argv[])
ap.arg("--clear-keywords")
.help("Clear all keywords")
.OTACTION(clear_keywords);
ap.arg("--history", &ot.metadata_history)
.help("Write full command line into Exif:ImageHistory, Software metadata attributes");
ap.arg("--no-history %!", &ot.metadata_history)
.help("Do not write full command line into Exif:ImageHistory, Software metadata attributes");
ap.arg("--nosoftwareattrib", &ot.metadata_nosoftwareattrib)
.help("Do not write command line into Exif:ImageHistory, Software metadata attributes");
.help("Do not write any Exif:ImageHistory or Software metadata attributes");
ap.arg("--sansattrib", &sansattrib)
.help("Write command line into Software & ImageHistory but remove --sattrib and --attrib options");
ap.arg("--orientation %d:ORIENT")
Expand Down
1 change: 1 addition & 0 deletions src/oiiotool/oiiotool.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ class Oiiotool {
bool output_dither;
bool output_force_tiles; // for debugging
bool metadata_nosoftwareattrib;
bool metadata_history;

// Options for --diff
float diff_warnthresh;
Expand Down
4 changes: 0 additions & 4 deletions testsuite/oiiotool-attribs/ref/out-jpeg9d.txt
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ Reading noattribs.jpg
noattribs.jpg : 2048 x 1536, 3 channel, uint8 jpeg
SHA-1: A74C7DF2B01825DCB6881407AE77C11DC56AB741
channel list: R, G, B
Exif:ExifVersion: "0230"
Exif:FlashPixVersion: "0100"
jpeg:subsampling: "4:2:0"
oiio:ColorSpace: "sRGB"
initial keywords=
Expand Down Expand Up @@ -123,8 +121,6 @@ tahoe-icc.jpg : 128 x 96, 3 channel, uint8 jpeg
ResolutionUnit: "in"
XResolution: 72
YResolution: 72
Exif:ExifVersion: "0230"
Exif:FlashPixVersion: "0100"
ICCProfile:attributes: "Reflective, Glossy, Positive, Color"
ICCProfile:cmm_type: 1094992453
ICCProfile:color_space: "RGB"
Expand Down
4 changes: 0 additions & 4 deletions testsuite/oiiotool-attribs/ref/out.txt
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ Reading noattribs.jpg
noattribs.jpg : 2048 x 1536, 3 channel, uint8 jpeg
SHA-1: 2623446988E34395C6B0A4AA4FC75107C708BF18
channel list: R, G, B
Exif:ExifVersion: "0230"
Exif:FlashPixVersion: "0100"
jpeg:subsampling: "4:2:0"
oiio:ColorSpace: "sRGB"
initial keywords=
Expand Down Expand Up @@ -123,8 +121,6 @@ tahoe-icc.jpg : 128 x 96, 3 channel, uint8 jpeg
ResolutionUnit: "in"
XResolution: 72
YResolution: 72
Exif:ExifVersion: "0230"
Exif:FlashPixVersion: "0100"
ICCProfile:attributes: "Reflective, Glossy, Positive, Color"
ICCProfile:cmm_type: 1094992453
ICCProfile:color_space: "RGB"
Expand Down

0 comments on commit 751ce36

Please sign in to comment.