feat: Adds a new raw file metadata storage for clients

[kommendorkapten]

This came up during a discussion related to new features for the sigstore TUF client, as there is a desire to make implementations in different languages sharing the target and metadata cache.

Release Notes: Possibility to store metadata files as raw JSON files to allow for interoperability with other TUF implementations.

Types of changes:

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

Description of the changes being introduced by the pull request: This pull request introduces a new implementation of the client's LocalStore interface. This storage stores the metadata files are regular files in a provided directory. This would improve compatibility to share metadata cache between different language implementations.

Please verify and check that the pull request fulfills the following requirements:

• [x] Tests have been added for the bug fix or new feature
• [ ] Docs have been added for the bug fix or new feature

[kommendorkapten]

Yeah, let me add version that's safe for concurrent use!

[kommendorkapten]

I added a new wrapper, ConcurrentLocalStore which can be used to protect any LocalStore (e.g. the in memory one and the raw JSON one) for concurrent access. This would let the client decide what use depending on their access pattern.

[kommendorkapten]

Windows tests are failing, I will look into this tomorrow.

[kommendorkapten]

The failing tests are due to filesystem permission, which on Windows is very different from UNIX-like operating system. I found this post on the topic: https://medium.com/@MichalPristas/go-and-file-perms-on-windows-3c944d55dd44 The summary is that in windows you can only create a file with either:

• 0666 (Read/writeable by all users)
• 0444 (Readable by all users)

And this seems to be correct according the source code: https://github.com/golang/go/blob/master/src/syscall/syscall_windows.go#L307 The provided permission is only used to determine if a file is intended to be create as read-only. The same is valid for Mkdir, the permission mode bits are ignored on Windows: https://github.com/golang/go/blob/master/src/syscall/syscall_windows.go#L518 And for reference the Chmod call: https://github.com/golang/go/blob/master/src/syscall/syscall_windows.go#L646 (only the read-only flag is manipulated).

It seems that there are some ACL concept in Windows that we can rely on. But I'm not qualified to speak if the normal permission modes set (all-read or all-write) actually are system-wide or if the effective permission applies to the user only. I found this ACL implementation in go: https://github.com/hectane/go-acl It haven't had any updates in three years, and I would not be comfortable relying on this, as I lack understanding on Windows filesystem access semantics.

As this feature of verifying the filesystem permission bits, is new for this implementation (LevelDB implementation does not care), nor is there any guidance for implementors of LocalStore to consider this, my suggestion is to skip file permissions for Windows.

Please advise on how to proceed.

[kommendorkapten]

Any updates on how to proceed with this (file permission on Windows host)? My comments and proposal are in the comment above: https://github.com/theupdateframework/go-tuf/pull/347#issuecomment-1211615181 cc @trishankatdatadog @ethan-lowman-dd

[trishankatdatadog]

Any updates on how to proceed with this (file permission on Windows host)? My comments and proposal are in the comment above: #347 (comment) cc @trishankatdatadog @ethan-lowman-dd

I'm going to make a point of looking at this tonight.

[trishankatdatadog]

Please advise on how to proceed.

Sorry for the late reply.

Let's take a look at the python-tuf reference implementation: there, the permissions of the metadata/targets cache directories is not checked, and metadata/targets files appear to be RW-able by the user but not others (using both tempfile.NamedTemporaryFile() and open()).

I like your idea for checking the permissions on both the directories and files, but technically, this is not a requirement in the TUF specification. At least the directory permissions can probably be left to the application calling go-tuf, but the files inside them are trickier. If the golang stdlib doesn't give control beyond either RW or RO by all, then there is not much we can do for now.

Is not supporting Windows for now an option? That seems the safest to me.

[trishankatdatadog]

@kommendorkapten do you need another review? If so, I recommend pinging those who have reviewed before. Thanks!

[kommendorkapten]

@ethan-lowman-dd would you mind taking another look? Addressed your concerns and added filesystem permission check for the metadata files/directories on UNIX-like systems (see discussion above on why Windows is exempted).

[trishankatdatadog]

@kommendorkapten if there isn't already a GitHub issue to track the Windows file permissions issue, would you mind making one for it? We should track it. Thanks!

[kommendorkapten]

@trishankatdatadog Of course! Created this: https://github.com/theupdateframework/go-tuf/issues/360

[kommendorkapten]

Good feedback @ethan-lowman-dd, all your comments should be addressed now 👍

[ethan-lowman-dd]

@asraa or @joshuagl Could you please provide a followup review when you get the chance?

[trishankatdatadog]

Good feedback @ethan-lowman-dd, all your comments should be addressed now 👍

Nice work, thanks Ethan

[trishankatdatadog]

@kommendorkapten would you please rebase and resolve conflicts? thx!

[kommendorkapten]

Was off on PTO, so didn't see you message @trishankatdatadog until now. PR is updated now.

[trishankatdatadog]

Was off on PTO, so didn't see you message @trishankatdatadog until now. PR is updated now.

Thanks! Would you pls fix the linting errors?

[kommendorkapten]

Thanks! Would you pls fix the linting errors?

Sorry, was not aware of them, fixed now.