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

Absolute paths are recorded in attestations #12

Closed
kpcyrd opened this issue Sep 22, 2021 · 9 comments
Closed

Absolute paths are recorded in attestations #12

kpcyrd opened this issue Sep 22, 2021 · 9 comments

Comments

@kpcyrd
Copy link
Contributor

kpcyrd commented Sep 22, 2021

One of the rebuilders is currently generating attestations looking like this:

{"signatures":[{"keyid":"585a2a5c5efec5fc22d84f7fa7a4a22cc1c62507cf97b6d8e7df7aaea8e5f659","sig":"57430b2e677e8f91f474054b9618ff78339b7d65d921903b0f743af41f1812dcbdf82cc06334a02270cb759a8db175b62569f192d9cc25409bc80294cd19910a"}],"signed":{"_type":"link","byproducts":{},"env":{},"materials":{"/tmp/rebuilderd7oil7Z/inputs/vultr-cli-2.8.3-1-x86_64.pkg.tar.zst":{"sha256":"768bc8b6ee2f0164036c5bdb6c2dadc644c6ee2a8549b3ba22eacd109c206649","sha512":"81bf157c90a603b0e70c816959ca5860fa7d060ddffa56b464e73f89b0b9aff5af351620beef1e3d6f01faf301e4ced277c3ba46d56fd9e356c5521a90a9da18"}},"name":"rebuild vultr-cli-2.8.3-1-x86_64.pkg.tar.zst","products":{"/tmp/rebuilderd7oil7Z/out/vultr-cli-2.8.3-1-x86_64.pkg.tar.zst":{"sha256":"768bc8b6ee2f0164036c5bdb6c2dadc644c6ee2a8549b3ba22eacd109c206649","sha512":"81bf157c90a603b0e70c816959ca5860fa7d060ddffa56b464e73f89b0b9aff5af351620beef1e3d6f01faf301e4ced277c3ba46d56fd9e356c5521a90a9da18"}}}}

This string:

/tmp/rebuilderd7oil7Z/inputs/vultr-cli-2.8.3-1-x86_64.pkg.tar.zst

should likely be this instead (both in materials and products):

vultr-cli-2.8.3-1-x86_64.pkg.tar.zst
@joyliu-q
Copy link
Contributor

Hi! Thank you for finding this: I will look into this issue.
Do you know what the rebuilder is trying to rebuild so I can try to reproduce the issue?

@kpcyrd
Copy link
Contributor Author

kpcyrd commented Sep 23, 2021

hi :)

this happens with every package, the paths that are passed to in_toto_run by rebuilderd are absolute paths, the in-toto lib then converts them into strings here:

Ok((VirtualTargetPath::new(String::from(path))?, hashes))

I'm wondering if it'd make sense to generate the (VirtualTargetPath, TargetDescription) in rebuilderd because it's already aware of the filename and the directory traversal code in record_artifacts isn't needed. :)

@joyliu-q
Copy link
Contributor

joyliu-q commented Oct 1, 2021

Hmm, this is a bit tricky. Because that line of code is nested somewhat deeply and called inside the record_artifact function (which lays inside record_artifacts which is in in_toto_run), I'm not sure if there's a simple way to have rebuilderd step in for the (VirtualTargetPath, TargetDescription) generation.

So far, I see 3 solutions to the absolute path solution:

  1. Quick fix (but not very pretty): like you've said, the directory traversal is unneeded in this step. Because of that, maybe we can assemble the link using the subfunctions called in in_toto_run without calling in_toto_run itself. We can replace the record_artifacts step with rebuilderd hashing the package and insert it into the link in a similar format.
  2. Add an optional input parameter to in_toto_run called strip_artifact_prefix for in_toto_run that takes in a string and strips the prefix from material and product paths. strip_artifact_prefix would then be passed to record_artifacts and record_artifact.
  3. Before passing the rebuilderd paths into in_toto_run, convert absolute paths to relative paths(?). It's generally not good to store absolute paths in a file either way, so this might be the fix we need. However, to be honest, I'm not super sure how converting absolute back to relative would work (maybe similar to strip_prefix?)

Would love some input from @SantiagoTorres @adityasaky as well!

@SantiagoTorres
Copy link
Member

Well, this sounds like something we do in the in-toto-run bits, with allowing callers to lstrip paths. I think we can definitely make this feature appear soon™ for -rs...

@adityasaky
Copy link
Member

Since rebuilderd is aware of the paths of both, can we just add lstrip functionality (i.e., option 2) to in_toto_run?

@adityasaky
Copy link
Member

@adityasaky
Copy link
Member

Opened #13 to track this. :)

@adityasaky
Copy link
Member

I believe we can now close this. We have left-strip capabilities courtesy of #19.

@adityasaky
Copy link
Member

I've opened a separate thread here: kpcyrd/rebuilderd#129

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants