Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

feat: add signature library #255

Merged
merged 4 commits into from
Oct 3, 2018
Merged

Conversation

technosophos
Copy link
Member

This adds pkg/signature, which is a library for signing, attesting, and verifying bundles.

I decided to do this as a standalone PR before adding signature support to all of the commands, as this is a fairly big addition.

@technosophos technosophos self-assigned this Oct 1, 2018
Also, add a passphrase fetcher to the keyring, and have that fetcher passed to every key.
Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

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

LGTM; though admittedly rusty in opengpg signing (but refreshing my knowledge now). It all appears clear and intuitive. Nearly all comments are on comments :), though one small unit test question.

"golang.org/x/crypto/openpgp/packet"
)

// PassphraseFetcher recieves a keyname, and is responsible for returning the associated passphrase
Copy link
Member

Choose a reason for hiding this comment

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

i before e, except after c :)

Copy link
Member Author

Choose a reason for hiding this comment

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

lol. Fixing


// Key returns the key with the given ID.
//
// ID is a hex ID or (conventionally) and email address.
Copy link
Member

Choose a reason for hiding this comment

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

s/and/an

return nil, errors.New("multiple matching keys found")
}

// Save writes a keycring to disk as a binary entity list.
Copy link
Member

Choose a reason for hiding this comment

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

s/keycring/keyring

}()

newfile := filepath.Join(dirname, "save.gpg")
is.NoError(kr.Save(newfile, true))
Copy link
Member

Choose a reason for hiding this comment

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

To thoroughly test clobber=true functionality, would we want to pre-create/pre-populate newfile before this Save?


// Signer can sign bundles
//
// Signatures are OpenPGP Section 7 clear signed blocks represented as ASCII-armored.
Copy link
Member

Choose a reason for hiding this comment

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

It seems the convention is clear signed be one word


// Attest generates an attestation (detached signature) for a signed bundle.
//
// This ONLY works on signed bundle files, and it rerequires the signed bundle
Copy link
Member

Choose a reason for hiding this comment

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

Should read requires?


// We clearsign instead of using the openpgp.ArmoredDetachedSignText because the
// later does not handle subkeys at all. It ONLY allows using the private key on
// the main entity. Yet all the helper methods for that are unexported. This it
Copy link
Member

Choose a reason for hiding this comment

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

s/This/Thus?

@technosophos
Copy link
Member Author

Addressed all of @vdice 's concerns

@technosophos technosophos merged commit 9c206be into cnabio:master Oct 3, 2018
@technosophos technosophos deleted the feat/signing branch October 3, 2018 17:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants