Skip to content

Conversation

@folbricht
Copy link
Owner

Implements #2

@folbricht folbricht marked this pull request as draft April 10, 2021 15:11
@folbricht folbricht mentioned this pull request Apr 10, 2021
@charles-dyfis-net
Copy link
Collaborator

I'll build an aptly patch to make use of this, and report back on any issues.

@folbricht
Copy link
Owner Author

There's an issue with the signing process by the looks. It uses time.Now() basically which changes the KeyID. So I'll have to change the command to also read the OpenPGP public key (created during the generate subcommand) to use its timestamp.

@folbricht
Copy link
Owner Author

That should be fixed as well now. It seems functional, just missing docs at this point. But I'll let you try it out first in case larger changes still have to be made.

@charles-dyfis-net
Copy link
Collaborator

Hmm. This is easy to implement for detached signatures. Cleartext signatures (which, alas, are used for Debian index files) look like another place where the upstream library expects a literal private key passed in... though admittedly I haven't looked past the documentation and into the source for golang.org/x/crypto/openpgp/clearsign yet.

@folbricht
Copy link
Owner Author

I think it calls the same Sign() function which should work for a crypto.Signer: https://github.com/golang/crypto/blob/0c34fe9e7dc2486962ef9867e3edb3503537209f/openpgp/packet/signature.go#L521

Will try that, and perhaps add another clear-sign subcommand. Let's review the command syntax once it's in, perhaps I should add the classic sign functionality as well rather than only supporting detached and clearsign signatures.

@charles-dyfis-net
Copy link
Collaborator

charles-dyfis-net commented Apr 11, 2021

Ahh -- looking closer, I see what you mean; the result of calling packet.NewSignerPrivateKey should be workable here.

Updating the aptly patch to take advantage of that.

@charles-dyfis-net
Copy link
Collaborator

Yup, worked nicely; apologies for the alarmism above. Can I check in an amended version of this PR with a -c option on tpmk openpgp sign somewhere?

@folbricht
Copy link
Owner Author

I'm about to push a commit that adds -c to the sign command.

@folbricht
Copy link
Owner Author

A few possible TODO:

  • Improve the library to build an openpgp.Entity from a public key and a TPM key, rather than handle them both independently
  • Add documentation
  • Consider supporting regular (non-detached) signatures as well
  • Options to configure the hash function (needed?)

@charles-dyfis-net
Copy link
Collaborator

So, this is bizarre.

Code builds fine for me, including the aptly draft PR using it (aptly-dev/aptly#955). However, in aptly's CI...

../../../../pkg/mod/github.com/folbricht/tpmk@v0.1.2-0.20210411225238-7061339773c3/openpgp.go:58:69: config.DefaultHash.String undefined (type crypto.Hash has no field or method String)

...even though they're using the same v0.0.0-20190320223903-b7391e95e576 release of golang.org/x/crypto that tpmk is.

@folbricht
Copy link
Owner Author

The String() method was introduced in Go 1.15 https://golang.org/pkg/crypto/#Hash.String. I'll have to update the version in go.mod to match that.

@folbricht
Copy link
Owner Author

I changed the internal methods a little to make it possible to read a key from disk and attach the private key to an openpgp.Entity. This allows library users to operate on data with the standard lib functions. It didn't change the command line interface.

@folbricht
Copy link
Owner Author

Since someone actually fixed the openpgp library to support crypto.Decryptor keys, it was pretty straightforward to add decryption as well. Might be useful. So there are now 3 sub-commands (generate, sign, decrypt).

@folbricht folbricht changed the title OpenPGP public key and signing support OpenPGP public key signing/decryption support Apr 19, 2021
@folbricht folbricht marked this pull request as ready for review April 25, 2021 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants