Set tag to COSIGN_PRIVATE_KEY_PEM_LABEL for ec.rs

[danbev]

Signed-off-by: Daniel Bevenius [email protected]

Summary

This commit suggests changing the tag used in EcdsaKeys::private_key_to_encrypted_pem from SIGSTORE_PRIVATE_KEY_PEM_LABEL to COSIGN_PRIVATE_KEY_PEM_LABEL.

The motivation for this is that using such a key with Go cosign will currently result in an unsupported pem type error. It seems like Go cosign is using CosignPrivateKeyPemType when checking the tag of the pem.

I'm not sure about the differences between
SIGSTORE_PRIVATE_KEY_PEM_LABEL and COSIGN_PRIVATE_KEY_PEM_LABEL and I might be missing something here but if the change in this PR is a valid then this should probably be done for rsa and ed25519 as well.

Release Note

NONE

Documentation

NONE

[Xynnn007]

Hi @danbev Thanks for pointing out the bug here! It LGTM. But still some problems to ensure.

The pem label SIGSTORE_PRIVATE_KEY_PEM_LABEL was brought in sigstore-rs at the first time here https://github.com/sigstore/sigstore-rs/pull/87/files#diff-1264d08d322b15d92a0ff5f49178e377fd247b5d9e1f02f5b5cd0b928c4df6e6R66 by me.
I referred the countepart in sigstore-go here https://github.com/sigstore/sigstore/blob/main/pkg/cryptoutils/privatekey.go#L41 added by @dekkagaijin .

It seems that this label only shows in the definition here with name EncryptedSigstorePrivateKeyPEMType .

I also wonder whether this definition is useful. And I'm happy and welcome that change that rsa, ed25519 together with ec keys all change the label to COSIGN_PRIVATE_KEY_PEM_LABEL and abondon the label SIGSTORE_PRIVATE_KEY_PEM_LABEL .

Hi @lukehinds @flavio , could you give some suggestions and is there anything I ignored?

[lukehinds]

I believe going forward we should implement the SIGSTORE_ prefix especially with the desire to standardise client implementations.

What are the current consequences of using SIGSTORE_PRIVATE_KEY_PEM_LABEL, am I right in that sigstore-rs generated keys won't work in cosign and sigstore-rs can work with cosign generated keys?

cc @znewman01 & @asraa

[danbev]

What are the current consequences of using SIGSTORE_PRIVATE_KEY_PEM_LABEL, am I right in that sigstore-rs generated keys won't work in cosign and sigstore-rs can work with cosign generated keys?

That is my understanding as well, that a keypair generated with cosign will work with sigstore-rs, but not the other way around.
I've tried to verify this using a the sigstore-rs/examples/cosign/verify:

check.sh

cosign version

echo "Generate keypair"
env COSIGN_PASSWORD=" " cosign generate-key-pair

echo something > artifact
uuid=`uuidgen | head -c 8`

ttl_path=ttl.sh/danbev/${uuid}:2h

echo -e "\nUpload $uuid"
cosign upload blob -f artifact $ttl_path

echo -e "\nSign $ttl_path"
env COSIGN_PASSWORD=" " cosign sign --key cosign.key $ttl_path

echo -e "\nRun examples/cosign/verify"
cargo run --example verify -- \
    -k cosign.pub \
    --rekor-pub-key ~/.sigstore/root/targets/rekor.pub \
    --fulcio-cert ~/.sigstore/root/targets/fulcio.crt.pem \
    $ttl_path

output

$ ./check.sh 
  ______   ______        _______. __    _______ .__   __.
 /      | /  __  \      /       ||  |  /  _____||  \ |  |
|  ,----'|  |  |  |    |   (----`|  | |  |  __  |   \|  |
|  |     |  |  |  |     \   \    |  | |  | |_ | |  . `  |
|  `----.|  `--'  | .----)   |   |  | |  |__| | |  |\   |
 \______| \______/  |_______/    |__|  \______| |__| \__|
cosign: A tool for Container Signing, Verification and Storage in an OCI registry.

GitVersion:    devel
GitCommit:     8dc365ae43931c8695f4ffab164e62395e108926
GitTreeState:  clean
BuildDate:     2022-10-14T05:19:34
GoVersion:     go1.19.1
Compiler:      gc
Platform:      linux/amd64

Generate keypair
Private key written to cosign.key
Public key written to cosign.pub

Upload 575bcc1c
Uploading file from [artifact] to [ttl.sh/danbev/575bcc1c:2h] with media type [text/plain]
File [artifact] is available directly at [ttl.sh/v2/danbev/575bcc1c/blobs/sha256:4bc453b53cb3d914b45f4b250294236adba2c0e09ff6f03793949e7e39fd4cc1]
Uploaded image to:
ttl.sh/danbev/575bcc1c@sha256:8539c5652ad0decf33bccf7adae9c43f174c0314ff265658e5d83519190d10e2

Sign ttl.sh/danbev/575bcc1c:2h
WARNING: Image reference ttl.sh/danbev/575bcc1c:2h uses a tag, not a digest, to identify the image to sign.

This can lead you to sign a different image than the intended one. Please use a
digest (example.com/ubuntu@sha256:abc123...) rather than tag
(example.com/ubuntu:latest) for the input to cosign. The ability to refer to
images by tag will be removed in a future release.
Pushing signature to: ttl.sh/danbev/575bcc1c

Run examples/cosign/verify
    Finished dev [unoptimized + debuginfo] target(s) in 0.19s
     Running `/home/danielbevenius/work/security/sigstore/sigstore-rs/target/debug/examples/verify -k cosign.pub --rekor-pub-key /home/danielbevenius/.sigstore/root/targets/rekor.pub --fulcio-cert /home/danielbevenius/.sigstore/root/targets/fulcio.crt.pem 'ttl.sh/danbev/575bcc1c:2h'`
Image successfully verified

[znewman01]

I believe going forward we should implement the SIGSTORE_ prefix especially with the desire to standardise client implementations.

Strong +1 to this. Filed sigstore/cosign#2471

[flavio]

I believe going forward we should implement the SIGSTORE_ prefix especially with the desire to standardise client implementations.

Strong +1 to this. Filed sigstore/cosign#2471

That means we don't have to do any change to our code. Do I get it right? Can this PR be closed then?

[viccuad]

Agree on closing, given that there is consensus on targetting only SIGSTORE_PRIVATE_KEY_PEM_LABEL, and signing is a new unreleased feature; we aren't even introducing a regression on sigstore-rs.

[danbev]

Thanks everyone for commenting, I'll close this now.