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

Enable parsing of incomplete minisign keys, to enable re-indexing. #567

Merged
merged 1 commit into from
Dec 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions pkg/pki/minisign/minisign.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,30 @@ func NewPublicKey(r io.Reader) (*PublicKey, error) {

inputString := inputBuffer.String()

// There are three ways a minisign key can be stored.
// 1. The entire text key
// 2. A base64 encoded string
// 3. A legacy format we stored of just the key material (no key ID or Algorithm) due to bug fixed in https://github.com/sigstore/rekor/pull/562
key, err := minisign.DecodePublicKey(inputString)
if err != nil {
// try as a standalone base64 string
key, err = minisign.NewPublicKey(inputString)
if err != nil {
return nil, fmt.Errorf("unable to read minisign public key: %w", err)
}
if err == nil {
k.key = &key
return &k, nil
}
key, err = minisign.NewPublicKey(inputString)
if err == nil {
k.key = &key
return &k, nil
}

k.key = &key
return &k, nil
if len(inputString) == 32 {
k.key = &minisign.PublicKey{
SignatureAlgorithm: [2]byte{'E', 'd'},
KeyId: [8]byte{},
}
copy(k.key.PublicKey[:], inputBuffer.Bytes())
return &k, nil
}
return nil, fmt.Errorf("unable to read minisign public key: %w", err)
}

// CanonicalValue implements the pki.PublicKey interface
Expand Down
64 changes: 51 additions & 13 deletions pkg/pki/minisign/minisign_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,26 +32,64 @@ func TestMain(m *testing.M) {

func TestReadPublicKey(t *testing.T) {
type test struct {
caseDesc string
inputFile string
errorFound bool
caseDesc string
inputFile string
}

tests := []test{
{caseDesc: "Not a valid public key file", inputFile: "testdata/hello_world.txt.minisig", errorFound: true},
{caseDesc: "Valid public key (minisign)", inputFile: "testdata/minisign.pub", errorFound: false},
{caseDesc: "Valid public key (signify)", inputFile: "testdata/signify.pub", errorFound: false},
{caseDesc: "Valid public key (minisign)", inputFile: "testdata/minisign.pub"},
{caseDesc: "Valid public key (signify)", inputFile: "testdata/signify.pub"},
}

for _, tc := range tests {
file, err := os.Open(tc.inputFile)
if err != nil {
t.Errorf("%v: cannot open %v", tc.caseDesc, tc.inputFile)
}
t.Run(tc.caseDesc, func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It might make sense to add more test cases in the tests slice above

Copy link
Contributor

Choose a reason for hiding this comment

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

Or do we cover all added new functionality?

Copy link
Member Author

Choose a reason for hiding this comment

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

Reorganized the tests a bit and added one more case!

file, err := os.Open(tc.inputFile)
if err != nil {
t.Fatalf("%v: cannot open %v", tc.caseDesc, tc.inputFile)
}

if got, err := NewPublicKey(file); ((got != nil) == tc.errorFound) || ((err != nil) != tc.errorFound) {
t.Errorf("%v: unexpected result testing %v: %v", tc.caseDesc, tc.inputFile, err)
}
got, err := NewPublicKey(file)
if err != nil {
t.Fatalf("unexpected error %v", err)
}

// Try to send just the raw public key bytes too
rawBytes := got.key.PublicKey[:]

rawGot, err := NewPublicKey(bytes.NewReader(rawBytes))
if err != nil {
t.Fatalf("unexpected error re-parsing public key: %v", err)
}
if !bytes.Equal(rawGot.key.PublicKey[:], rawBytes) {
t.Errorf("expected parsed keys to be equal, %v != %v", rawGot.key.PublicKey, rawBytes)
}
})
}
}

func TestReadPublicKeyErr(t *testing.T) {
type test struct {
caseDesc string
inputFile string
}

tests := []test{
{caseDesc: "Not a valid public key file", inputFile: "testdata/hello_world.txt.minisig"},
{caseDesc: "Wrong length", inputFile: "testdata/hello_world.txt"},
}

for _, tc := range tests {
t.Run(tc.caseDesc, func(t *testing.T) {
file, err := os.Open(tc.inputFile)
if err != nil {
t.Fatalf("%v: cannot open %v", tc.caseDesc, tc.inputFile)
}

got, err := NewPublicKey(file)
if err == nil {
t.Errorf("error expected, got nil error and %v", got)
}
})
}
}

Expand Down