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

blob verification fails when used with unsanitized base64 signature strings #288

Open
jleightcap opened this issue Jul 28, 2023 · 4 comments · May be fixed by #403
Open

blob verification fails when used with unsanitized base64 signature strings #288

jleightcap opened this issue Jul 28, 2023 · 4 comments · May be fixed by #403
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@jleightcap
Copy link
Contributor

Description

When verifying a signature passed via a file, trailing newlines are not sanitized.

Using the verify_blob API with a signature file generated by sigstore-python, verify_signature fails with

Error: Invalid byte 10, offset 96.

the root issue seems to be differing signature files generated between sigstore-python and cosign (e.g. https://github.com/sigstore/sigstore-rs/tree/main/examples/cosign/verify-blob#sign-the-artifacttxt-file-using-cosign).

(byte 10 == '\n')

This is pretty easily fixed with a .trim() at the fs::read_to_string callsite, but compatibility doesn't seem guaranteed between sigstore clients.

Version

e23046c

@jleightcap jleightcap added the bug Something isn't working label Jul 28, 2023
@jleightcap
Copy link
Contributor Author

(CC @woodruffw)

@flavio flavio added the good first issue Good for newcomers label Sep 17, 2024
@jku
Copy link
Member

jku commented Oct 11, 2024

the errors are not quite the same now:
Verification failed Base64DecodeError(InvalidByte(0, 45))

I'll have a look at the code but the differences I can see in the data are:

  • python adds a linefeed to signature, cosign does not
  • python uses PEM with ascii armoring for certificate. cosign uses PEM with ascii armoring encoded in base64 (???).

I think compatibility would be useful... but from sigstore-python perspective these files are not the "API": the signature bundle is.

cosigns use of double-base64 seems useless in practice.

@jku
Copy link
Member

jku commented Oct 11, 2024

Verified in code: there are two differences that would need to be handled to make verify-blob work with data from sigstore-python:

  • trim the signature -- this seems reasonable
  • Avoid base64 decoding the certificate -- this is not as clearly useful as you'd now need two code paths

@jku
Copy link
Member

jku commented Oct 11, 2024

I will make an attempt at fixing this, I think it looks like something the CLI layer can take care of.

@jku jku linked a pull request Oct 11, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants