crypto/openpgp: add error and armor

crypto/openpgp: add error and armor

error is needed by all the OpenPGP packages as they return a shared
family of error types.

armor implements OpenPGP armoring. It's very like PEM except:
  a) it includes a CRC24 checksum
  b) PEM values are small (a few KB) and so encoding/pem assumes that
     they fit in memory. Armored data can be very large and so this
     package presents a streaming interface.

[nsz_port70.net, 2010-12-24 00:22:00]

* agl@golang.org <agl@golang.org> [2010-12-21 15:21:36 +0000]:
> crypto/openpgp: add error and armor
> 
nice, i didn't know there was plan for an openpgp package

i implemented clear text dsa signing and verification for
a personal project, but my armor code is uglier

> armor implements OpenPGP armoring. It's very like PEM except:
>   a) it includes a CRC24 checksum
i wonder why you used uint64 for crc24
i used uint32 like in the crc32 package
(i also masked the return value with 0xffffff but that's
probably not important)

>   b) PEM values are small (a few KB) and so encoding/pem assumes that
>      they fit in memory. Armored data can be very large and so this
>      package presents a streaming interface.
i followed the pem approach because i was lazy

the only problem i see with io.Reader interface is the case
when there is extra data before or after the armored block
then you cannot really access that data, but i guess that's
not a real issue

i had more problems representing openpgp packets in a sensible
way, i'm curious about how that will turn out

[rsc, 2011-01-04 16:35:55]

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
File src/pkg/crypto/openpgp/armor/armor.go (right):

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
src/pkg/crypto/openpgp/armor/armor.go:34: Contents io.Reader         // A Reader
from which the contents can be read
s/Headers/Header/
s/Contents/Body/
?
matches http.Request

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
src/pkg/crypto/openpgp/armor/armor.go:193: return
p = nil
return

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
src/pkg/crypto/openpgp/armor/armor.go:199: if len(bytes.TrimSpace(line)) == 0 {
do you want to save the trimmed line here
so that the header key and value are trimmed?

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
File src/pkg/crypto/openpgp/armor/encode.go (right):

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
src/pkg/crypto/openpgp/armor/encode.go:44: out:        out}
please move } to next line

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
src/pkg/crypto/openpgp/armor/encode.go:63: l.used += len(b)
can say l.used += copy(...)

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
src/pkg/crypto/openpgp/armor/encode.go:131: // Encode builds a WriteCloser which
will encoding the data written to it in
s/builds/returns/
s/encoding/encode/

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/error/...
File src/pkg/crypto/openpgp/error/Makefile (right):

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/error/...
src/pkg/crypto/openpgp/error/Makefile:7: TARG=crypto/openpgp/error
What's the long term plan for crypto/openpgp?
What kinds of things will it have?

It would be nice if the errors, which will be part of the API,
were, say, openpgp.InvalidArgumentError instead of 
error.InvalidArgumentError, but that would mean collapsing
everything into one package.  What's here so far is small
enough for one package but I don't know what else you're
planning to add.

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/error/...
File src/pkg/crypto/openpgp/error/error.go (right):

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/error/...
src/pkg/crypto/openpgp/error/error.go:18: type Unsupported string
Please end error types in Error, so for example

UnsupportedFeatureError
ArgumentError
SignatureError

[agl1, 2011-01-08 15:37:16]

On Thu, Dec 23, 2010 at 7:21 PM, Szabolcs Nagy <nsz@port70.net> wrote:
> i wonder why you used uint64 for crc24
> i used uint32 like in the crc32 package

Good point. I don't know why I used a 64-bit value either. Changed it
to a uint32, thanks.

> the only problem i see with io.Reader interface is the case
> when there is extra data before or after the armored block
> then you cannot really access that data, but i guess that's
> not a real issue

With the addition of clearsigning support (which isn't in this CL) I
needed to add an interface for constructing an armor reader from a
line.Reader. That could be exported if people have a need for it.

I guess time will tell but I'm hoping that, if the armored block has
interesting surrounding text then it'll probably be pretty small. In
which case users can cut it out themselves and feed a bytes.Buffer
into openpgp/armor. As ever, if people have a use-case which which is
ugly, please let me know.


Cheers

AGL

[agl1, 2011-01-08 15:51:01]

PTAL

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
File src/pkg/crypto/openpgp/armor/armor.go (right):

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
src/pkg/crypto/openpgp/armor/armor.go:34: Contents io.Reader         // A Reader
from which the contents can be read
On 2011/01/04 16:35:55, rsc wrote:
> s/Headers/Header/
> s/Contents/Body/
> ?
> matches http.Request

Done.

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
src/pkg/crypto/openpgp/armor/armor.go:193: return
On 2011/01/04 16:35:55, rsc wrote:
> p = nil
> return

Done, although I don't typically nil out any partially constructed return value
when returning an error.

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
src/pkg/crypto/openpgp/armor/armor.go:199: if len(bytes.TrimSpace(line)) == 0 {
On 2011/01/04 16:35:55, rsc wrote:
> do you want to save the trimmed line here
> so that the header key and value are trimmed?

Done.

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
File src/pkg/crypto/openpgp/armor/encode.go (right):

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
src/pkg/crypto/openpgp/armor/encode.go:44: out:        out}
On 2011/01/04 16:35:55, rsc wrote:
> please move } to next line

Done.

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
src/pkg/crypto/openpgp/armor/encode.go:63: l.used += len(b)
On 2011/01/04 16:35:55, rsc wrote:
> can say l.used += copy(...)

Done.

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
src/pkg/crypto/openpgp/armor/encode.go:131: // Encode builds a WriteCloser which
will encoding the data written to it in
On 2011/01/04 16:35:55, rsc wrote:
> s/builds/returns/
> s/encoding/encode/

Done.

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/error/...
File src/pkg/crypto/openpgp/error/Makefile (right):

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/error/...
src/pkg/crypto/openpgp/error/Makefile:7: TARG=crypto/openpgp/error
On 2011/01/04 16:35:55, rsc wrote:
> What's the long term plan for crypto/openpgp?
> What kinds of things will it have?

In my working directory I have

crypto/
  openpgp/
    armor/
    error/
    packet/

The top level crypto/openpgp package will have functions like
CheckDetatchedSignature. 'packet' contains lower level functions for parsing and
serialising OpenPGP data.

I split it out this way so that the documentation is clearly separated.

> It would be nice if the errors, which will be part of the API,
> were, say, openpgp.InvalidArgumentError instead of 
> error.InvalidArgumentError, but that would mean collapsing
> everything into one package.  What's here so far is small
> enough for one package but I don't know what else you're
> planning to add.

I'm assuming that users will rename the import to 'openpgperror' or such.

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/error/...
File src/pkg/crypto/openpgp/error/error.go (right):

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/error/...
src/pkg/crypto/openpgp/error/error.go:18: type Unsupported string
On 2011/01/04 16:35:55, rsc wrote:
> Please end error types in Error, so for example
> 
> UnsupportedFeatureError
> ArgumentError
> SignatureError

Done.

[rsc, 2011-01-11 15:50:09]

LGTM

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
File src/pkg/crypto/openpgp/armor/armor.go (right):

http://codereview.appspot.com/3786043/diff/2001/src/pkg/crypto/openpgp/armor/...
src/pkg/crypto/openpgp/armor/armor.go:193: return
On 2011/01/08 15:51:01, agl1 wrote:
> On 2011/01/04 16:35:55, rsc wrote:
> > p = nil
> > return
> 
> Done, although I don't typically nil out any partially constructed return
value
> when returning an error. 

Typically you should unless you're intending
the caller to be able to use a partial value
(like when a partial Write returns n>0 and an error).

[agl1, 2011-01-12 02:09:41]

On Tue, Jan 11, 2011 at 10:50 AM,  <rsc@golang.org> wrote:
> LGTM

Submittted.