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

[feature] generator_container_slsa3 should support passing image as a secret #2917

Open
Danil-Grigorev opened this issue Oct 26, 2023 · 6 comments · Fixed by #2918
Open

[feature] generator_container_slsa3 should support passing image as a secret #2917

Danil-Grigorev opened this issue Oct 26, 2023 · 6 comments · Fixed by #2918
Labels
area:container Issue with the generic container generator type:feature New feature or request

Comments

@Danil-Grigorev
Copy link
Contributor

Danil-Grigorev commented Oct 26, 2023

Is your feature request related to a problem? Please describe.

The image input of the generator_container_slsa3 workflow is declared as input which prohibits using secrets as values. The following example fails:

  provenance:
    uses: slsa-framework/slsa-github-generator/.github/workflows/generator_container_slsa3.yml@v1.4.0
    with:
      image: ${{ secrets.REGISTRY_IMAGE }}
      # Passing secrets in any form is impossible in a reusable workflow.
      # Encrypted value could not be decrypteded as the workflow can't be used as a step,
      # and secret outputs are always redacted when are passed between jobs as an output.
    secrets:
      registry-username: ${{ secrets.REGISTRY_USERNAME }}
      registry-password: ${{ secrets.REGISTRY_PASSWORD }}

Describe the solution you'd like

The image input should be available to provide a secret. Example workflow could look liks this:

  provenance:
    uses: slsa-framework/slsa-github-generator/.github/workflows/generator_container_slsa3.yml@v1.4.0
    with:
      digest: ${{ needs.build.outputs.digest }}
    secrets:
      secret: ${{ secrets.REGISTRY_IMAGE }}
      registry-username: ${{ secrets.REGISTRY_USERNAME }}
      registry-password: ${{ secrets.REGISTRY_PASSWORD }}

Describe alternatives you've considered

Unfortunately due to destination registry restrictions, we have no alternatives, but to provide every value as a secret, and we are unable to change the overlapping values between the registry endpoint and the published image name.

Additional context

@Danil-Grigorev Danil-Grigorev added status:triage Issue that has not been triaged type:feature New feature or request labels Oct 26, 2023
@Danil-Grigorev Danil-Grigorev changed the title [feature] [feature] generator_container_slsa3 should support passing image as a secret Oct 26, 2023
@ianlewis ianlewis added area:container Issue with the generic container generator and removed status:triage Issue that has not been triaged labels Oct 27, 2023
ianlewis added a commit that referenced this issue Oct 27, 2023
Add option to provide image as a secret for private registries.

Fixes #2917

---------

Signed-off-by: Danil Grigorev <danil.grigorev@suse.com>
Signed-off-by: Ian Lewis <ianlewis@google.com>
Co-authored-by: Ian Lewis <ianlewis@google.com>
@laurentsimon laurentsimon reopened this Oct 27, 2023
@laurentsimon
Copy link
Collaborator

Temporarily re-opening for a question. Low-entropy secrets tend to create intermittent problems due to this bug https://github.com/orgs/community/discussions/37942. We have not heard of folks running into this problem with the container generator though, even though username isa low-entropy secret too. But I think it could trigger the problem. @behnazh-w is my understanding correct?

@ianlewis
Copy link
Member

Ah yeah. I totally forgot about that issue. Apologies for merging too quickly.

I agree, it might be an issue so I'll take a look. It could trigger if we have any job outputs whose value match any part of the image name.

@ianlewis
Copy link
Member

BTW, we included the registry-username secret to support AWS IDs but document that you shouldn't use it for regular low-entropy username values.

In the case of image it will pretty much always be low-entropy so probably shouldn't be a secret. I think the workaround for these types of values was to use variables to move the secret value to a non-secret.
https://docs.github.com/en/actions/learn-github-actions/variables

@laurentsimon
Copy link
Collaborator

ahh, I forgot about these variables! Yeah now I remember that's the solution to the problem. So let's document this in the generic container workflow then. @Danil-Grigorev Can you confirm this would solve your problem?

@Danil-Grigorev
Copy link
Contributor Author

Unfortunately with variables the issue is the same - they are visible in the logs. While encrypting output values may help, github contexts does not allow to pass a masked variable directly between the jobs too. And due to specifics of the provenance job - requirement to run on a separate VM, if I understand this correctly, the value can’t be decrypted in an additional step.

I can confirm that the change is working when I tried that, and is not affected by the bug you mentioned with the given configuration. In fact this is the first success with everything in place 😄

  provenance:
    needs: [sign, build]
    permissions:
      actions: read
      id-token: write
      packages: write
    uses: slsa-framework/slsa-github-generator/.github/workflows/generator_container_slsa3.yml@main
    with:
      digest: ${{ needs.build.outputs.digest }}
      compile-generator: true
    secrets:
      image: ${{ inputs.secret_registry && secrets[inputs.image] || inputs.image }}-${{ inputs.arch }}
      registry-username: ${{ inputs.secret_registry && secrets[inputs.username] || inputs.username }}
      registry-password: ${{ secrets[inputs.password] }}

@laurentsimon
Copy link
Collaborator

Unfortunately with variables the issue is the same - they are visible in the logs.

Note that the container name will be available in the provenance, which is currently recorded in the transparency log too. So its values will leak no matter what.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:container Issue with the generic container generator type:feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants