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

security: change command line embedding for oiiotool & maketx output #4237

Merged
merged 2 commits into from
Apr 20, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should it be this?

Suggested change
OpenImageIO 2.5.0.)
OpenImageIO 2.6.0.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was assuming people would want this backported to the next monthly patch of the release branch.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So then should it be 2.5.11?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, I answered that too fast. It shouldn't read 2.5.0 either. It will end up being 2.5.11.

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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
default changed only say the software name and version (unless the
default changed only save the software name and version (unless the

Copy link
Collaborator

Choose a reason for hiding this comment

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

or since "saved" is used later in the sentence in a different way and so could be confusing:

Suggested change
default changed only say the software name and version (unless the
default changed only except for the software name and version (unless the

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that's awkward, I will reword a bit

`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
Loading