From 2da167fbbea6ce6a7da8f70e1219ce25af656cbe Mon Sep 17 00:00:00 2001 From: Casey Marshall Date: Sat, 15 Dec 2012 14:30:05 -0500 Subject: [PATCH] openpgp: improve parser resilience & flexibility, add PublicKey.BitLength() These are improvements I've made as necessary to develop Hockeypuck, an OpenPGP keyserver (https://launchpad.net/hockeypuck). The max armor line length was increased to 96 because some keyservers (SKS) will output armor with lines greater than 64 (the prior max). I've exposed packet.ReadEntity to support stream-parsing, useful for large SKS dump files. ReadKeyRing attempts to recover in the event of malformed data. Because many packet types are not yet supported, I added OpaquePacket to capture unsupported types for offline storage and later reparsing. R=agl CC=golang-dev https://golang.org/cl/6927044 --- openpgp/armor/armor.go | 2 +- openpgp/keys.go | 11 ++- openpgp/packet/opaque.go | 143 +++++++++++++++++++++++++++++++++++ openpgp/packet/packet.go | 18 ++++- openpgp/packet/public_key.go | 15 ++++ 5 files changed, 183 insertions(+), 6 deletions(-) create mode 100644 openpgp/packet/opaque.go diff --git a/openpgp/armor/armor.go b/openpgp/armor/armor.go index 91267d6..c7ab2ef 100644 --- a/openpgp/armor/armor.go +++ b/openpgp/armor/armor.go @@ -111,7 +111,7 @@ func (l *lineReader) Read(p []byte) (n int, err error) { return 0, io.EOF } - if len(line) > 64 { + if len(line) > 96 { return 0, ArmorCorrupt } diff --git a/openpgp/keys.go b/openpgp/keys.go index 6404aff..08e6c71 100644 --- a/openpgp/keys.go +++ b/openpgp/keys.go @@ -199,11 +199,16 @@ func ReadKeyRing(r io.Reader) (el EntityList, err error) { for { var e *Entity - e, err = readEntity(packets) + e, err = ReadEntity(packets) if err != nil { + // TODO: warn about skipped unsupported/unreadable keys if _, ok := err.(errors.UnsupportedError); ok { lastUnsupportedError = err err = readToNextPublicKey(packets) + } else if _, ok := err.(errors.StructuralError); ok { + // Skip unreadable, badly-formatted keys + lastUnsupportedError = err + err = readToNextPublicKey(packets) } if err == io.EOF { err = nil @@ -249,9 +254,9 @@ func readToNextPublicKey(packets *packet.Reader) (err error) { panic("unreachable") } -// readEntity reads an entity (public key, identities, subkeys etc) from the +// ReadEntity reads an entity (public key, identities, subkeys etc) from the // given Reader. -func readEntity(packets *packet.Reader) (*Entity, error) { +func ReadEntity(packets *packet.Reader) (*Entity, error) { e := new(Entity) e.Identities = make(map[string]*Identity) diff --git a/openpgp/packet/opaque.go b/openpgp/packet/opaque.go new file mode 100644 index 0000000..14b9759 --- /dev/null +++ b/openpgp/packet/opaque.go @@ -0,0 +1,143 @@ +// Copyright 2012 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package packet + +import ( + "bytes" + "code.google.com/p/go.crypto/openpgp/errors" + "io" + "io/ioutil" +) + +// OpaquePacket represents an OpenPGP packet as raw, unparsed data. This is +// useful for splitting and storing the original packet contents separately, +// handling unsupported packet types or accessing parts of the packet not yet +// implemented by this package. +type OpaquePacket struct { + // Packet type + Tag uint8 + // Reason why the packet was parsed opaquely + Reason error + // Binary contents of the packet data + Contents []byte +} + +func (op *OpaquePacket) parse(r io.Reader) (err error) { + op.Contents, err = ioutil.ReadAll(r) + return +} + +// Serialize marshals the packet to a writer in its original form, including +// the packet header. +func (op *OpaquePacket) Serialize(w io.Writer) (err error) { + err = serializeHeader(w, packetType(op.Tag), len(op.Contents)) + if err == nil { + _, err = w.Write(op.Contents) + } + return +} + +// Parse attempts to parse the opaque contents into a structure supported by +// this package. If the packet is not known then the result will be another +// OpaquePacket. +func (op *OpaquePacket) Parse() (p Packet, err error) { + hdr := bytes.NewBuffer(nil) + err = serializeHeader(hdr, packetType(op.Tag), len(op.Contents)) + if err != nil { + return op, err + } + return Read(io.MultiReader(hdr, bytes.NewBuffer(op.Contents))) +} + +// OpaqueReader reads OpaquePackets from an io.Reader. +type OpaqueReader struct { + r io.Reader +} + +func NewOpaqueReader(r io.Reader) *OpaqueReader { + return &OpaqueReader{r: r} +} + +// Read the next OpaquePacket. +func (or *OpaqueReader) Next() (op *OpaquePacket, err error) { + tag, _, contents, err := readHeader(or.r) + if err != nil { + return + } + op = &OpaquePacket{Tag: uint8(tag), Reason: err} + err = op.parse(contents) + if err != nil { + consumeAll(contents) + } + return +} + +// OpaqueSubpacket represents an unparsed OpenPGP subpacket, +// as found in signature and user attribute packets. +type OpaqueSubpacket struct { + Length uint32 + SubType uint8 + Contents []byte +} + +// OpaqueSubpackets extracts opaque, unparsed OpenPGP subpackets from +// their byte representation. +func OpaqueSubpackets(contents []byte) ([]*OpaqueSubpacket, error) { + var result []*OpaqueSubpacket + var err error + var ( + subHeaderLen uint32 + subPacket *OpaqueSubpacket + ) + for len(contents) > 0 { + subHeaderLen, subPacket, err = nextSubpacket(contents) + if err != nil { + break + } + result = append(result, subPacket) + contents = contents[subHeaderLen+subPacket.Length:] + } + return result, err +} + +func nextSubpacket(contents []byte) (subHeaderLen uint32, subPacket *OpaqueSubpacket, err error) { + // RFC 4880, section 5.2.3.1 + if len(contents) < 1 { + goto Truncated + } + subPacket = &OpaqueSubpacket{} + switch { + case contents[0] < 192: + subHeaderLen = 1 + subPacket.Length = uint32(contents[0]) + contents = contents[1:] + case contents[0] < 255: + subHeaderLen = 2 + if len(contents) < 2 { + goto Truncated + } + subPacket.Length = uint32(contents[0]-192)<<8 + uint32(contents[1]) + 192 + contents = contents[2:] + default: + subHeaderLen = 5 + if len(contents) < 5 { + goto Truncated + } + subPacket.Length = uint32(contents[1])<<24 | + uint32(contents[2])<<16 | + uint32(contents[3])<<8 | + uint32(contents[4]) + contents = contents[5:] + } + if subPacket.Length > uint32(len(contents)) { + goto Truncated + } + subPacket.SubType = contents[0] + subPacket.Contents = contents[1:subPacket.Length] + return +Truncated: + err = errors.StructuralError("subpacket truncated") + return +} diff --git a/openpgp/packet/packet.go b/openpgp/packet/packet.go index 9b5cd88..9c4a1b1 100644 --- a/openpgp/packet/packet.go +++ b/openpgp/packet/packet.go @@ -7,6 +7,7 @@ package packet import ( + "bytes" "code.google.com/p/go.crypto/cast5" "code.google.com/p/go.crypto/openpgp/errors" "crypto/aes" @@ -338,10 +339,23 @@ func Read(r io.Reader) (p Packet, err error) { se.MDC = true p = se default: - err = errors.UnknownPacketTypeError(tag) + p = new(OpaquePacket) } if p != nil { - err = p.parse(contents) + backup := bytes.NewBuffer(nil) + // FIXME: this causes large packets to be buffered in memory. + // Ideally we would like a better solution here so that huge, + // encrypted files could still be handled in a streaming + // fashion. Perhaps the encrypted data packet could call a + // method on backup that causes it to stop recording. + tee := io.TeeReader(contents, backup) + err = p.parse(tee) + switch err.(type) { + case errors.UnsupportedError, errors.UnknownPacketTypeError: + p = &OpaquePacket{Tag: uint8(tag), Reason: err} + err = nil + err = p.parse(io.MultiReader(backup, contents)) + } } if err != nil { consumeAll(contents) diff --git a/openpgp/packet/public_key.go b/openpgp/packet/public_key.go index a757090..fc9012d 100644 --- a/openpgp/packet/public_key.go +++ b/openpgp/packet/public_key.go @@ -412,3 +412,18 @@ func writeMPIs(w io.Writer, mpis ...parsedMPI) (err error) { } return } + +// BitLength returns the bit length for the given public key. +func (pk *PublicKey) BitLength() (bitLength uint16, err error) { + switch pk.PubKeyAlgo { + case PubKeyAlgoRSA, PubKeyAlgoRSAEncryptOnly, PubKeyAlgoRSASignOnly: + bitLength = pk.n.bitLength + case PubKeyAlgoDSA: + bitLength = pk.p.bitLength + case PubKeyAlgoElGamal: + bitLength = pk.p.bitLength + default: + err = errors.InvalidArgumentError("bad public-key algorithm") + } + return +}