Importing RSA and EC keypairs

[venafi-iw]

Summary

This PR provides support for importing RSA and EC keypairs that were generated using 3rd party utilities such as OpenSSL. Imported keypairs are converted to Cosign keypairs.

Ticket Link

This PR is in relation to #549

Release Notes

• Added support for importing EC and RSA keypairs generated using 3rd party utilities such as OpenSSL
• New command line option import-key-pair
• added instructions on how to import keypairs

[dlorenc]

Looks like DCO is missing! You'll need to use the git commit -s flag

[SantiagoTorres]

Overall, it looks in a good direction, but I wonder how this fits the broader codebase (I think there's a lot of possibly repeated code).

I also feel there are a couple of minor fix/tweaks that may be making the code slightly harder to review. I wonder if we could split those into a smaller PR...

[SantiagoTorres]

I'm not entirely fond of pw2 when this should be a confirmation. I'm not sure what the golang guidelines are for variable naming, but maybe something more explicit would make the code more readable.

[venafi-iw]

Ok, I can update the variable to something more explcit such as confirmpw

[asraa]

small ping on this comment

[asraa]

Thanks! looking really good so far :D appreciate all the changes

[asraa]

nit: spelling MarshalKeyPair

[asraa]

could you share this code with GenerateKeyPair? and given that it's only called within this function, maybe it should be made private.

[venafi-iw]

made changes.

[asraa]

Sorry for the delay, taking a review! Could you check on the test failures as well?

?   	github.com/sigstore/cosign/pkg/blob	[no test files]
--- FAIL: TestImportPrivateKey (0.12s)
    keys_test.go:158: unexpected error importing key: parsing error
FAIL

Also, running and committing the change from go run -tags pivkey,pkcs11key,cgo ./cmd/help/ will fix the doc gen CI check

[asraa]

Thanks! Could you just please address the lint comments and update the branch and this would LGTM!

[venafi-iw]

#1050 (comment)

Your code suggestion doesn't compile.

[asraa]

Your code suggestion doesn't compile.

Was hoping you would take away the gist of what I was saying. I pulled your branch and completed it for you:

diff --git a/pkg/cosign/keys.go b/pkg/cosign/keys.go
index 98f92f9..d592b0e 100644
--- a/pkg/cosign/keys.go
+++ b/pkg/cosign/keys.go
@@ -71,20 +71,20 @@ func ImportKeyPair(keyPath string, pf PassFunc) (*KeysBytes, error) {
                return nil, errors.New("invalid pem block")
        }
 
+       var pk crypto.Signer
        switch p.Type {
        case RSAPrivateKeyPemType:
-               pk, err := x509.ParsePKCS1PrivateKey(p.Bytes)
+               pk, err = x509.ParsePKCS1PrivateKey(p.Bytes)
                if err != nil {
                        return nil, fmt.Errorf("parsing error")
                }
-               return marshalKeyPair(Keys{pk, pk.Public()}, pf)
        default:
-               pk, err := x509.ParseECPrivateKey(p.Bytes)
+               pk, err = x509.ParseECPrivateKey(p.Bytes)
                if err != nil {
                        return nil, fmt.Errorf("parsing error")
                }
-               return marshalKeyPair(Keys{pk, pk.Public()}, pf)
        }
+       return marshalKeyPair(Keys{pk, pk.Public()}, pf)
 }
 
 func marshalKeyPair(keypair Keys, pf PassFunc) (*KeysBytes, error) {

[venafi-iw]

ImportKeyPair function updated.

[asraa]

cc @lukehinds @dlorenc do you want to give this a merge? LGTM just want a final pass in case I'm missing something cosign-style-y

[lukehinds]

cc @lukehinds @dlorenc do you want to give this a merge? LGTM just want a final pass in case I'm missing something cosign-style-y

looks goof from a cursory view, but there are a quite a few CI failures.

[dlorenc]

Yeah looks like a merge conflict somewhere, otherwise should be good to go.

[venafi-iw]

@dlorenc Made some code updates and looks good for next steps.

[dlorenc]

This looks good to me, there's one more conflict though :(

Once that gets fixed i'll merge right away so you don't keep getting hit!

[venafi-iw]

Conflict resolved

[dlorenc]

Thanks for dealing with all the delays and merge conflicts!

[cpanato]

this import is needed?

or should be just cosign sign --key import-cosign.key dlorenc/demo ?

[venafi-iw]

documentation needs to be updated as it should be:

cosign sign --key import-cosign.key dlorenc/demo

#1290