Security issue due to boost::filesystem removal.

[marsupial]

f0f7879 moved away from boost::filesystem to https://github.com/gulrak/filesystem.
This is causing issue building in firewalled environments, as it requires pulling from an external repo.

I'm curious if you could go into detail about the following:

1. Why move away from boost which is already a dependency everyone will have already fulfilled as USD requires boost.
2. Is the maintainer employed by Autodesk, or is there any auditing that is occurring within Autodesk to make sure no malicious code is getting in? (I'm generally not this paranoid, but this is a file-system level API...).
3. If there is auditing occurring, can the repository be a sub-module tagged to an SHA1 or CMake FetchContent_Declare call actually pull a specific SHA1 rather than a tag which can arbitrarily change underneath us ?
4. If there is not any auditing, and we have to do that ourselves, can the usage of the repo be made a proper dependency rather than always pulling from the internet.

-Thanks

[dgovil]

I would +1 the request to switching it from a Git tag to a specific hash commit as a baseline for security (since tags can be redirected).

I think it would also make sense to allow finding gulrak locally, so that the build could be fully local, but also prevent repeat hits to GitHub, should a studio desire that. I think a good default would be to not download code by default, and have it as an opt in flag? Generally build scripts are often run with some amount of privileges, so downloading any code could be a vulnerability.

[HamedSabri-adsk]

@marsupial

This is causing issue building in firewalled environments, as it requires pulling from an external repo.

I would like to understand what security issues you are experiencing? This is not the first time maya-usd pulls from an external repo. We also pull from external repo for Googletest library.

Why move away from boost which is already a dependency everyone will have already fulfilled as USD requires boost.

In recent years the standard C++ has already adopted a lot of the features ( e.g std::filesystem , string_view, std::variant, etc... ) from Boost library and it is mature enough that relying on Boost library will no longer is required. With the exception to Boost::python, maya-usd shouldn't need any C++ features that can't be already found in the standard.

Dependency on Boost is expensive. It requires a lot of maintenance from version to version mostly on the build side. It has its own fair share of problems with CMake ( at least it used to in the older versions ).

Is the maintainer employed by Autodesk, or is there any auditing that is occurring within Autodesk to make sure no malicious code is getting in? (I'm generally not this paranoid, but this is a file-system level API...).

Hmm yes, the author is a full time employee of Autodesk. Any file-system level API is vulnerable to malicious attacks. Do you have an example of malicious code that you could share with us?

If there is auditing occurring, can the repository be a sub-module tagged to an SHA1 or CMake FetchContent_Declare call actually pull a specific SHA1 rather than a tag which can arbitrarily change underneath us ?

Relying on Github tags shouldn't be problematic. At least I have never heard or experienced any issues with pointing to Git tags before. Do you have an example when this has happened?

If there is not any auditing, and we have to do that ourselves, can the usage of the repo be made a proper dependency rather than always pulling from the internet.

Not quite sure what you mean by "made a proper dependency". adding sub-module for these tiny projects requires us to make changes in our build pipeline.

Not to mention that the switch to gulark library is temporary as it has been mentioned in "f0f7879" description. Once we move to C++17 ( pending USD ), the entire dependency to gulark project will be removed.

We also have other dependency for such as "GoogleTest" that also requires internet connectivity.

[HamedSabri-adsk]

@dgovil

so that the build could be fully local, but also prevent repeat hits to GitHub, should a studio desire that.

Is this true? We have always had dependency on GoogleTest which is currently build as an external project and requires the internet for the full clean build.

[dgovil]

@HamedSabri-adsk For GoogleTest, you can provide GTEST_ROOT which will allow you to skip downloading of GoogleTest and be fully offline. Which is probably why we've never downloaded it during the build ourselves.

gulark is the only one we download currently. It's not so much a problem for us, but I assume it's a stronger requirement for any MPAA firewalled studio like @marsupial is beholden to.

If it were possible to add a GULARK_ROOT or something, it might make it friendlier to firewalled folks.

Relying on Github tags shouldn't be problematic. At least I have never heard or experienced any issues with pointing to Git tags before.

With regards to the tag vulnerability, it's because tags can be overwritten to point to a different commit. So theoretically, assuming the gulark repo merged in a bad commit or the maintainer went rogue (both are extremely unlikely of course since the maintainer is at Autodesk), the tag of 1.50 could point to a different commit and bring in a vulnerability. This is theoretical of course, and I'm not so worried about it, but I could argue both ways for it...

[marsupial]

As far as GoogleTest, yes it's external, but it's also setup so that a local GoogleTest install will work. At a minimum the gulark dependency should do the same.

I understand Dependency on Boost is expensive, but we've already payed that expense (as will everyone who builds this as USD requires boost). Adding a new dependency doesn't alleviate the expense, you've actually made it cost more!

Relying on Github tags shouldn't be problematic

A tag is an alias, so the SHA1 it points to can change. An attacker can insert malicious code into a branch and redirect that tag to that branch...its a security hole whether its been actively exploited or not.

Adding sub-module for these tiny projects requires us to make changes in our build pipeline.

I understand, but the way it is now (where a lot of studios may have strict firewalls) requires others to change their build pipeline, or introduce annoying workarounds. Currently I have to go onto a very specific machine setup to communicate with the outside world, just to build the repo...previously I did not have to do this...and I really have no way of knowing what it downloaded is what you intended to be downloaded.

[HamedSabri-adsk]

but it's also setup so that a local GoogleTest install will work.

Yes that's the variable that Google Test CMake Module offers. We don't have the firewall strictness and are always connected to internet via VPN. Downloading Google Test is very fast ( a few seconds ) on the first clean build.

Let me have a look to find a solution to allow you bypass the internet to build gulark libary.

Adding a new dependency doesn't alleviate the expense, you've actually made it cost more!

With all due respect I strongly disagree on this. Going deeper into boost dependency rabbit hole is much more worst and costly.
This change will shine once we switch to C++17. All a developer needs to do is to replace ghc:: namespace with std:: and move on.

[HamedSabri-adsk]

For GoogleTest, you can provide GTEST_ROOT which will allow you to skip downloading of GoogleTest and be fully offline. Which is probably why we've never downloaded it during the build ourselves.

You are right. As I mentioned in above comment, GTEST_ROOT is a variable that Google Test CMake Module offers. We don't have such firewall strictness and are always connected to internet via VPN.

Downloading Google Test is very fast ( a few seconds ) on the first clean build and we don't ever bother with using the offline build of Gtest.

I will find a solution to allow bypassing the internet to build gulark libary.

[dgovil]

Thanks. I think that would help alleviate the main part of the concerns if a user could point to Gulark locally.

[HamedSabri-adsk]

@marsupial @dgovil

I added support for building Gulark locally to avoid the need to pull from an external repo via internet. See #1291

I can also offer a second solution which works outside of the box without manually setting of "GULARK_SOURCE_DIR" plus it should be secure.

1- Package up Gulark source code (gulark_1.5.0.zip) and have it checked-in under maya-usd/thirdParty
2- Provide a MD5 checksum and have CMake to validate the file identity before consuming it.

Again this is going to extreme here but thought to offer you this option if it helps alleviate the security concerns.

[dgovil]

That's great @HamedSabri-adsk .

Personally, I think your PR solves the problem in general since it allows studios to locally manage the repo, without needing the extra steps you suggested (since your team manages gulark too).

[marsupial]

I think #1291 is enough for our concern...I'll give it a try tomorrow.

Thanks for the quick response!

[HamedSabri-adsk]

Is the maintainer employed by Autodesk, or is there any auditing that is occurring within Autodesk to make sure no malicious code is getting in? (I'm generally not this paranoid, but this is a file-system level API...).

I would like to clarify that Autodesk is not the maintainer of open source project. https://github.com/gulrak/filesystem

My original answer meant to the changes I made specifically in f0f7879

Hope this clears any ambiguity.