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

Add LinkAttestor for in-toto Attestation Framework Link Predicate #288

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Forrin
Copy link
Contributor

@Forrin Forrin commented Nov 29, 2023

This change adds a "LinkAttestor" which is able to create in-toto Attestation compliant metadata for a link predicate.

I experimented with a variety of ways to implement this, but chose to add an Attestor type over updating the InTotoRun function to include this capability (like how --use-dsse is implemented).

This is added with the thought that additional attestation predicate types may be supported in the future. In addition, I assume that we may not want to use DSSE for all signing operations (thus support creating an unsigned statement).

This won't work with layouts as there isn't a layout structure for the Attestation framework yet, I believe.

The Attestor code can easily be moved into its own package if desired. I'm not certain what other Attestors, if any, should be supported, but this can probably be made more dynamic if needed.

A smidge of code is duplicated, which can be cleaned up, but I didn't want to touch the v1 code too much for this submission.
 
Example usage;

mkdir testingdir
cd testingdir
go run .. run -k ../test/data/carol -m . -p helloworld -n test -a link -- touch helloworld

Output:

{
  "payloadType": "application/vnd.in-toto+json",
  "payload": "eyJwcmVkaWNhdGUiOnsiYnlwcm9kdWN0cyI6eyJyZXR1cm4tdmFsdWUiOjAsInN0ZGVyciI6IiIsInN0ZG91dCI6IiJ9LCJjb21tYW5kIjpbInRvdWNoIiwiaGVsbG93b3JsZCJdLCJuYW1lIjoidGVzdCJ9LCJwcmVkaWNhdGVfdHlwZSI6Imh0dHBzOi8vaW4tdG90by5pby9MaW5rL3YxIiwic3ViamVjdCI6W3siZGlnZXN0Ijp7InNoYTI1NiI6ImUzYjBjNDQyOThmYzFjMTQ5YWZiZjRjODk5NmZiOTI0MjdhZTQxZTQ2NDliOTM0Y2E0OTU5OTFiNzg1MmI4NTUifSwibmFtZSI6ImhlbGxvd29ybGQifV0sInR5cGUiOiJodHRwczovL2luLXRvdG8uaW8vU3RhdGVtZW50L3YxIn0=",
  "signatures": [
    {
      "keyid": "be6371bc627318218191ce0780fd3183cce6c36da02938a477d2e4dfae1804a6",
      "sig": "rMeB/jUl6/zZh9Kms3BK0ZnrCJChd3Y82e5ROuWnpq+vOxWwDff3zyieOrnItlhi13u2DxAkAG/oKpiXlGuZDQ=="
    }
  ]
}

Decoded Payload;

{
    "predicate": {
        "byproducts": {
            "return-value": 0,
            "stderr": "",
            "stdout": ""
        },
        "command": [
            "touch",
            "helloworld"
        ],
        "name": "test"
    },
    "predicate_type": "https://in-toto.io/attestation/link/v0.3",
    "subject": [
        {
            "digest": {
                "sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
            },
            "name": "helloworld"
        }
    ],
    "type": "https://in-toto.io/Statement/v1"
}

edit: fixed predicate string to use latest version over deprecated.

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

@Forrin Forrin changed the title Adding link attestor Add LinkAttestor for in-toto Attestation Framework Link Predicate Nov 29, 2023
@adityasaky adityasaky requested review from adityasaky, pxp928 and shibumi and removed request for adityasaky November 29, 2023 06:53
Copy link
Member

@adityasaky adityasaky left a comment

Choose a reason for hiding this comment

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

This mostly looks good to me. I think I agree we can't avoid the repetition with InTotoRun here but I wonder if we can refactor that to use this? cc @pxp928 @shibumi

in_toto/attestations.go Outdated Show resolved Hide resolved
in_toto/attestations.go Outdated Show resolved Hide resolved
in_toto/attestations.go Outdated Show resolved Hide resolved
in_toto/attestations.go Outdated Show resolved Hide resolved
in_toto/attestations.go Outdated Show resolved Hide resolved
in_toto/attestations.go Show resolved Hide resolved
in_toto/attestations_test.go Outdated Show resolved Hide resolved
@Forrin
Copy link
Contributor Author

Forrin commented Dec 7, 2023

@adityasaky One thing I should mention with this change. I wasn't certain what naming convention to use here for the file dump, so kept the link style that's defined in the in-toto v1 specification. However, I can see the desire to use a different naming convention, especially if multiple predicates are generated for a single step.

I assume that multiple envelopes that would be combined into a bundle if there are multiple predicates for a step. Though, I think that witness handles this with multiple predicates within a predicate.

@marcelamelara
Copy link
Contributor

marcelamelara commented Dec 19, 2023

Nice! I need to look into how this will interact with the work I'm doing over in #268 . Hoping not to duplicate work in either case. Regarding supporting other non-Link predicates, I'm imagining the predicateType will either need to be passed as an argument to InTotoRun, or parsed from the passed in structure. The latter might be a bit smoother.

EDIT: I wrote this not having fully internalized how the Attestor works. Apologies. First off, I think the notion of an Attestor makes sense. But also, taking a closer look at this proposal together with #268, I think we can avoid duplicating the implementation of each Attestor at the Statement level since that will be the same for every type of predicate. That is, with the general-purpose v1 APIs in place, I think the interface should expose a GeneratePredicate() API, rather than GenerateStatement().

I'm also not entirely sure we need to duplicate InTotoRun functionality if the Attestors are integrated into the existing InTotoRun function (as is ultimately the goal with the APIs from #268), rather than the other way around. But this may also be something we can address retroactively, too.

@Forrin
Copy link
Contributor Author

Forrin commented Dec 20, 2023

Nice! I need to look into how this will interact with the work I'm doing over in #268 . Hoping not to duplicate work in either case. Regarding supporting other non-Link predicates, I'm imagining the predicateType will either need to be passed as an argument to InTotoRun, or parsed from the passed in structure. The latter might be a bit smoother.

EDIT: I wrote this not having fully internalized how the Attestor works. Apologies. First off, I think the notion of an Attestor makes sense. But also, taking a closer look at this proposal together with #268, I think we can avoid duplicating the implementation of each Attestor at the Statement level since that will be the same for every type of predicate. That is, with the general-purpose v1 APIs in place, I think the interface should expose a GeneratePredicate() API, rather than GenerateStatement().

I'm also not entirely sure we need to duplicate InTotoRun functionality if the Attestors are integrated into the existing InTotoRun function (as is ultimately the goal with the APIs from #268), rather than the other way around. But this may also be something we can address retroactively, too.

I'll take a look at that PR. Originally I did have the functionality within the in-toto run command, but didn't like the idea of mixing it in there. I wanted to support not using link predicate as well, though perhaps not needed.

Signed-off-by: Adam Nauth <Forrin@users.noreply.github.com>

remove slice package

Signed-off-by: Adam Nauth <Forrin@users.noreply.github.com>

Add attestation flag

Signed-off-by: Adam Nauth <Forrin@users.noreply.github.com>

Fix link predicate version. Cleanup statement pointer

Signed-off-by: Adam Nauth <Forrin@users.noreply.github.com>

fix tests, use latest predicate type string

Signed-off-by: Adam Nauth <Forrin@users.noreply.github.com>

switch to pointer receiver

Signed-off-by: Adam Nauth <Forrin@users.noreply.github.com>

switch sign statement to private

Signed-off-by: Adam Nauth <Forrin@users.noreply.github.com>

switch to using make for products and materials

Signed-off-by: Adam Nauth <Forrin@users.noreply.github.com>

fix link attestor test

Signed-off-by: Adam Nauth <Forrin@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

None yet

3 participants