cryptobyte: various API and documentation updates.

Change-Id: I83236ecea0774d4ec49e978a391eb3ff5dabdeb6
Reviewed-on: https://go-review.googlesource.com/57810
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Adam Langley <agl@golang.org>

cryptobyte/asn1.go

@@ -5,40 +5,30 @@
 package cryptobyte
 
 import (
-	"encoding/asn1"
+	encoding_asn1 "encoding/asn1"
 	"fmt"
 	"math/big"
 	"reflect"
 	"time"
+
+	"golang.org/x/crypto/cryptobyte/asn1"
 )
 
 // This file contains ASN.1-related methods for String and Builder.
 
-// Tag represents an ASN.1 tag number and class (together also referred to as
-// identifier octets). Methods in this package only support the low-tag-number
-// form, i.e. a single identifier octet with bits 7-8 encoding the class and
-// bits 1-6 encoding the tag number.
-type Tag uint8
-
-// Contructed returns t with the context-specific class bit set.
-func (t Tag) ContextSpecific() Tag { return t | 0x80 }
-
-// Contructed returns t with the constructed class bit set.
-func (t Tag) Constructed() Tag { return t | 0x20 }
-
 // Builder
 
 // AddASN1Int64 appends a DER-encoded ASN.1 INTEGER.
 func (b *Builder) AddASN1Int64(v int64) {
-	b.addASN1Signed(asn1.TagInteger, v)
+	b.addASN1Signed(asn1.INTEGER, v)
 }
 
 // AddASN1Enum appends a DER-encoded ASN.1 ENUMERATION.
 func (b *Builder) AddASN1Enum(v int64) {
-	b.addASN1Signed(asn1.TagEnum, v)
+	b.addASN1Signed(asn1.ENUM, v)
 }
 
-func (b *Builder) addASN1Signed(tag Tag, v int64) {
+func (b *Builder) addASN1Signed(tag asn1.Tag, v int64) {
 	b.AddASN1(tag, func(c *Builder) {
 		length := 1
 		for i := v; i >= 0x80 || i < -0x80; i >>= 8 {
@@ -54,7 +44,7 @@ func (b *Builder) addASN1Signed(tag Tag, v int64) {
 
 // AddASN1Uint64 appends a DER-encoded ASN.1 INTEGER.
 func (b *Builder) AddASN1Uint64(v uint64) {
-	b.AddASN1(asn1.TagInteger, func(c *Builder) {
+	b.AddASN1(asn1.INTEGER, func(c *Builder) {
 		length := 1
 		for i := v; i >= 0x80; i >>= 8 {
 			length++
@@ -73,7 +63,7 @@ func (b *Builder) AddASN1BigInt(n *big.Int) {
 		return
 	}
 
-	b.AddASN1(asn1.TagInteger, func(c *Builder) {
+	b.AddASN1(asn1.INTEGER, func(c *Builder) {
 		if n.Sign() < 0 {
 			// A negative number has to be converted to two's-complement form. So we
 			// invert and subtract 1. If the most-significant-bit isn't set then
@@ -103,7 +93,7 @@ func (b *Builder) AddASN1BigInt(n *big.Int) {
 
 // AddASN1OctetString appends a DER-encoded ASN.1 OCTET STRING.
 func (b *Builder) AddASN1OctetString(bytes []byte) {
-	b.AddASN1(asn1.TagOctetString, func(c *Builder) {
+	b.AddASN1(asn1.OCTET_STRING, func(c *Builder) {
 		c.AddBytes(bytes)
 	})
 }
@@ -116,27 +106,97 @@ func (b *Builder) AddASN1GeneralizedTime(t time.Time) {
 		b.err = fmt.Errorf("cryptobyte: cannot represent %v as a GeneralizedTime", t)
 		return
 	}
-	b.AddASN1(asn1.TagGeneralizedTime, func(c *Builder) {
+	b.AddASN1(asn1.GeneralizedTime, func(c *Builder) {
 		c.AddBytes([]byte(t.Format(generalizedTimeFormatStr)))
 	})
 }
 
-// AddASN1BitString appends a DER-encoded ASN.1 BIT STRING.
-func (b *Builder) AddASN1BitString(s asn1.BitString) {
-	// TODO(martinkr): Implement.
-	b.MarshalASN1(s)
+// AddASN1BitString appends a DER-encoded ASN.1 BIT STRING. This does not
+// support BIT STRINGs that are not a whole number of bytes.
+func (b *Builder) AddASN1BitString(data []byte) {
+	b.AddASN1(asn1.BIT_STRING, func(b *Builder) {
+		b.AddUint8(0)
+		b.AddBytes(data)
+	})
+}
+
+func (b *Builder) addBase128Int(n int64) {
+	var length int
+	if n == 0 {
+		length = 1
+	} else {
+		for i := n; i > 0; i >>= 7 {
+			length++
+		}
+	}
+
+	for i := length - 1; i >= 0; i-- {
+		o := byte(n >> uint(i*7))
+		o &= 0x7f
+		if i != 0 {
+			o |= 0x80
+		}
+
+		b.add(o)
+	}
+}
+
+func isValidOID(oid encoding_asn1.ObjectIdentifier) bool {
+	if len(oid) < 2 {
+		return false
+	}
+
+	if oid[0] > 2 || (oid[0] <= 1 && oid[1] >= 40) {
+		return false
+	}
+
+	for _, v := range oid {
+		if v < 0 {
+			return false
+		}
+	}
+
+	return true
+}
+
+func (b *Builder) AddASN1ObjectIdentifier(oid encoding_asn1.ObjectIdentifier) {
+	b.AddASN1(asn1.OBJECT_IDENTIFIER, func(b *Builder) {
+		if !isValidOID(oid) {
+			b.err = fmt.Errorf("cryptobyte: invalid OID: %v", oid)
+			return
+		}
+
+		b.addBase128Int(int64(oid[0])*40 + int64(oid[1]))
+		for _, v := range oid[2:] {
+			b.addBase128Int(int64(v))
+		}
+	})
+}
+
+func (b *Builder) AddASN1Boolean(v bool) {
+	b.AddASN1(asn1.BOOLEAN, func(b *Builder) {
+		if v {
+			b.AddUint8(0xff)
+		} else {
+			b.AddUint8(0)
+		}
+	})
+}
+
+func (b *Builder) AddASN1NULL() {
+	b.add(uint8(asn1.NULL), 0)
 }
 
-// MarshalASN1 calls asn1.Marshal on its input and appends the result if
+// MarshalASN1 calls encoding_asn1.Marshal on its input and appends the result if
 // successful or records an error if one occurred.
 func (b *Builder) MarshalASN1(v interface{}) {
 	// NOTE(martinkr): This is somewhat of a hack to allow propagation of
-	// asn1.Marshal errors into Builder.err. N.B. if you call MarshalASN1 with a
+	// encoding_asn1.Marshal errors into Builder.err. N.B. if you call MarshalASN1 with a
 	// value embedded into a struct, its tag information is lost.
 	if b.err != nil {
 		return
 	}
-	bytes, err := asn1.Marshal(v)
+	bytes, err := encoding_asn1.Marshal(v)
 	if err != nil {
 		b.err = err
 		return
@@ -148,7 +208,7 @@ func (b *Builder) MarshalASN1(v interface{}) {
 // Tags greater than 30 are not supported and result in an error (i.e.
 // low-tag-number form only). The child builder passed to the
 // BuilderContinuation can be used to build the content of the ASN.1 object.
-func (b *Builder) AddASN1(tag Tag, f BuilderContinuation) {
+func (b *Builder) AddASN1(tag asn1.Tag, f BuilderContinuation) {
 	if b.err != nil {
 		return
 	}
@@ -164,6 +224,24 @@ func (b *Builder) AddASN1(tag Tag, f BuilderContinuation) {
 
 // String
 
+func (s *String) ReadASN1Boolean(out *bool) bool {
+	var bytes String
+	if !s.ReadASN1(&bytes, asn1.INTEGER) || len(bytes) != 1 {
+		return false
+	}
+
+	switch bytes[0] {
+	case 0:
+		*out = false
+	case 0xff:
+		*out = true
+	default:
+		return false
+	}
+
+	return true
+}
+
 var bigIntType = reflect.TypeOf((*big.Int)(nil)).Elem()
 
 // ReadASN1Integer decodes an ASN.1 INTEGER into out and advances. If out does
@@ -215,7 +293,7 @@ var bigOne = big.NewInt(1)
 
 func (s *String) readASN1BigInt(out *big.Int) bool {
 	var bytes String
-	if !s.ReadASN1(&bytes, asn1.TagInteger) || !checkASN1Integer(bytes) {
+	if !s.ReadASN1(&bytes, asn1.INTEGER) || !checkASN1Integer(bytes) {
 		return false
 	}
 	if bytes[0]&0x80 == 0x80 {
@@ -235,7 +313,7 @@ func (s *String) readASN1BigInt(out *big.Int) bool {
 
 func (s *String) readASN1Int64(out *int64) bool {
 	var bytes String
-	if !s.ReadASN1(&bytes, asn1.TagInteger) || !checkASN1Integer(bytes) || !asn1Signed(out, bytes) {
+	if !s.ReadASN1(&bytes, asn1.INTEGER) || !checkASN1Integer(bytes) || !asn1Signed(out, bytes) {
 		return false
 	}
 	return true
@@ -258,7 +336,7 @@ func asn1Signed(out *int64, n []byte) bool {
 
 func (s *String) readASN1Uint64(out *uint64) bool {
 	var bytes String
-	if !s.ReadASN1(&bytes, asn1.TagInteger) || !checkASN1Integer(bytes) || !asn1Unsigned(out, bytes) {
+	if !s.ReadASN1(&bytes, asn1.INTEGER) || !checkASN1Integer(bytes) || !asn1Unsigned(out, bytes) {
 		return false
 	}
 	return true
@@ -286,7 +364,7 @@ func asn1Unsigned(out *uint64, n []byte) bool {
 func (s *String) ReadASN1Enum(out *int) bool {
 	var bytes String
 	var i int64
-	if !s.ReadASN1(&bytes, asn1.TagEnum) || !checkASN1Integer(bytes) || !asn1Signed(&i, bytes) {
+	if !s.ReadASN1(&bytes, asn1.ENUM) || !checkASN1Integer(bytes) || !asn1Signed(&i, bytes) {
 		return false
 	}
 	if int64(int(i)) != i {
@@ -315,9 +393,9 @@ func (s *String) readBase128Int(out *int) bool {
 
 // ReadASN1ObjectIdentifier decodes an ASN.1 OBJECT IDENTIFIER into out and
 // advances. It returns true on success and false on error.
-func (s *String) ReadASN1ObjectIdentifier(out *asn1.ObjectIdentifier) bool {
+func (s *String) ReadASN1ObjectIdentifier(out *encoding_asn1.ObjectIdentifier) bool {
 	var bytes String
-	if !s.ReadASN1(&bytes, asn1.TagOID) || len(bytes) == 0 {
+	if !s.ReadASN1(&bytes, asn1.OBJECT_IDENTIFIER) || len(bytes) == 0 {
 		return false
 	}
 
@@ -356,7 +434,7 @@ func (s *String) ReadASN1ObjectIdentifier(out *asn1.ObjectIdentifier) bool {
 // advances. It returns true on success and false on error.
 func (s *String) ReadASN1GeneralizedTime(out *time.Time) bool {
 	var bytes String
-	if !s.ReadASN1(&bytes, asn1.TagGeneralizedTime) {
+	if !s.ReadASN1(&bytes, asn1.GeneralizedTime) {
 		return false
 	}
 	t := string(bytes)
@@ -373,9 +451,9 @@ func (s *String) ReadASN1GeneralizedTime(out *time.Time) bool {
 
 // ReadASN1BitString decodes an ASN.1 BIT STRING into out and advances. It
 // returns true on success and false on error.
-func (s *String) ReadASN1BitString(out *asn1.BitString) bool {
+func (s *String) ReadASN1BitString(out *encoding_asn1.BitString) bool {
 	var bytes String
-	if !s.ReadASN1(&bytes, asn1.TagBitString) || len(bytes) == 0 {
+	if !s.ReadASN1(&bytes, asn1.BIT_STRING) || len(bytes) == 0 {
 		return false
 	}
 
@@ -392,10 +470,27 @@ func (s *String) ReadASN1BitString(out *asn1.BitString) bool {
 	return true
 }
 
+// ReadASN1BitString decodes an ASN.1 BIT STRING into out and advances. It is
+// an error if the BIT STRING is not a whole number of bytes. This function
+// returns true on success and false on error.
+func (s *String) ReadASN1BitStringAsBytes(out *[]byte) bool {
+	var bytes String
+	if !s.ReadASN1(&bytes, asn1.BIT_STRING) || len(bytes) == 0 {
+		return false
+	}
+
+	paddingBits := uint8(bytes[0])
+	if paddingBits != 0 {
+		return false
+	}
+	*out = bytes[1:]
+	return true
+}
+
 // ReadASN1Bytes reads the contents of a DER-encoded ASN.1 element (not including
 // tag and length bytes) into out, and advances. The element must match the
 // given tag. It returns true on success and false on error.
-func (s *String) ReadASN1Bytes(out *[]byte, tag Tag) bool {
+func (s *String) ReadASN1Bytes(out *[]byte, tag asn1.Tag) bool {
 	return s.ReadASN1((*String)(out), tag)
 }
 
@@ -404,8 +499,8 @@ func (s *String) ReadASN1Bytes(out *[]byte, tag Tag) bool {
 // given tag. It returns true on success and false on error.
 //
 // Tags greater than 30 are not supported (i.e. low-tag-number format only).
-func (s *String) ReadASN1(out *String, tag Tag) bool {
-	var t Tag
+func (s *String) ReadASN1(out *String, tag asn1.Tag) bool {
+	var t asn1.Tag
 	if !s.ReadAnyASN1(out, &t) || t != tag {
 		return false
 	}
@@ -417,8 +512,8 @@ func (s *String) ReadASN1(out *String, tag Tag) bool {
 // given tag. It returns true on success and false on error.
 //
 // Tags greater than 30 are not supported (i.e. low-tag-number format only).
-func (s *String) ReadASN1Element(out *String, tag Tag) bool {
-	var t Tag
+func (s *String) ReadASN1Element(out *String, tag asn1.Tag) bool {
+	var t asn1.Tag
 	if !s.ReadAnyASN1Element(out, &t) || t != tag {
 		return false
 	}
@@ -430,7 +525,7 @@ func (s *String) ReadASN1Element(out *String, tag Tag) bool {
 // returns true on success and false on error.
 //
 // Tags greater than 30 are not supported (i.e. low-tag-number format only).
-func (s *String) ReadAnyASN1(out *String, outTag *Tag) bool {
+func (s *String) ReadAnyASN1(out *String, outTag *asn1.Tag) bool {
 	return s.readASN1(out, outTag, true /* skip header */)
 }
 
@@ -439,40 +534,59 @@ func (s *String) ReadAnyASN1(out *String, outTag *Tag) bool {
 // advances. It returns true on success and false on error.
 //
 // Tags greater than 30 are not supported (i.e. low-tag-number format only).
-func (s *String) ReadAnyASN1Element(out *String, outTag *Tag) bool {
+func (s *String) ReadAnyASN1Element(out *String, outTag *asn1.Tag) bool {
 	return s.readASN1(out, outTag, false /* include header */)
 }
 
 // PeekASN1Tag returns true if the next ASN.1 value on the string starts with
 // the given tag.
-func (s String) PeekASN1Tag(tag Tag) bool {
+func (s String) PeekASN1Tag(tag asn1.Tag) bool {
 	if len(s) == 0 {
 		return false
 	}
-	return Tag(s[0]) == tag
+	return asn1.Tag(s[0]) == tag
 }
 
-// ReadOptionalASN1 attempts to read the contents of a DER-encoded ASN.Element
-// (not including tag and length bytes) tagged with the given tag into out. It
-// stores whether an element with the tag was found in outPresent, unless
-// outPresent is nil. It returns true on success and false on error.
-func (s *String) ReadOptionalASN1(out *String, outPresent *bool, tag Tag) bool {
+// SkipASN1 reads and discards an ASN.1 element with the given tag.
+func (s *String) SkipASN1(tag asn1.Tag) bool {
+	var unused String
+	return s.ReadASN1(&unused, tag)
+}
+
+// ReadOptionalASN1 attempts to read the contents of a DER-encoded ASN.1
+// element (not including tag and length bytes) tagged with the given tag into
+// out. It stores whether an element with the tag was found in outPresent,
+// unless outPresent is nil. It returns true on success and false on error.
+func (s *String) ReadOptionalASN1(out *String, outPresent *bool, tag asn1.Tag) bool {
 	present := s.PeekASN1Tag(tag)
 	if outPresent != nil {
 		*outPresent = present
 	}
+	if out != nil {
+		*out = String(nil)
+	}
 	if present && !s.ReadASN1(out, tag) {
 		return false
 	}
 	return true
 }
 
+// SkipOptionalASN1 advances s over an ASN.1 element with the given tag, or
+// else leaves s unchanged.
+func (s *String) SkipOptionalASN1(tag asn1.Tag) bool {
+	if !s.PeekASN1Tag(tag) {
+		return true
+	}
+	var unused String
+	return s.ReadASN1(&unused, tag)
+}
+
 // ReadOptionalASN1Integer attempts to read an optional ASN.1 INTEGER
 // explicitly tagged with tag into out and advances. If no element with a
 // matching tag is present, it writes defaultValue into out instead. If out
 // does not point to an integer or to a big.Int, it panics. It returns true on
 // success and false on error.
-func (s *String) ReadOptionalASN1Integer(out interface{}, tag Tag, defaultValue interface{}) bool {
+func (s *String) ReadOptionalASN1Integer(out interface{}, tag asn1.Tag, defaultValue interface{}) bool {
 	if reflect.TypeOf(out).Kind() != reflect.Ptr {
 		panic("out is not a pointer")
 	}
@@ -510,7 +624,7 @@ func (s *String) ReadOptionalASN1Integer(out interface{}, tag Tag, defaultValue
 // explicitly tagged with tag into out and advances. If no element with a
 // matching tag is present, it writes defaultValue into out instead. It returns
 // true on success and false on error.
-func (s *String) ReadOptionalASN1OctetString(out *[]byte, outPresent *bool, tag Tag) bool {
+func (s *String) ReadOptionalASN1OctetString(out *[]byte, outPresent *bool, tag asn1.Tag) bool {
 	var present bool
 	var child String
 	if !s.ReadOptionalASN1(&child, &present, tag) {
@@ -521,7 +635,7 @@ func (s *String) ReadOptionalASN1OctetString(out *[]byte, outPresent *bool, tag
 	}
 	if present {
 		var oct String
-		if !child.ReadASN1(&oct, asn1.TagOctetString) || !child.Empty() {
+		if !child.ReadASN1(&oct, asn1.OCTET_STRING) || !child.Empty() {
 			return false
 		}
 		*out = oct
@@ -531,7 +645,24 @@ func (s *String) ReadOptionalASN1OctetString(out *[]byte, outPresent *bool, tag
 	return true
 }
 
-func (s *String) readASN1(out *String, outTag *Tag, skipHeader bool) bool {
+// ReadOptionalASN1Boolean sets *out to the value of the next ASN.1 BOOLEAN or,
+// if the next bytes are not an ASN.1 BOOLEAN, to the value of defaultValue.
+func (s *String) ReadOptionalASN1Boolean(out *bool, defaultValue bool) bool {
+	var present bool
+	var child String
+	if !s.ReadOptionalASN1(&child, &present, asn1.BOOLEAN) {
+		return false
+	}
+
+	if !present {
+		*out = defaultValue
+		return true
+	}
+
+	return s.ReadASN1Boolean(out)
+}
+
+func (s *String) readASN1(out *String, outTag *asn1.Tag, skipHeader bool) bool {
 	if len(*s) < 2 {
 		return false
 	}
@@ -547,7 +678,7 @@ func (s *String) readASN1(out *String, outTag *Tag, skipHeader bool) bool {
 	}
 
 	if outTag != nil {
-		*outTag = Tag(tag)
+		*outTag = asn1.Tag(tag)
 	}
 
 	// ITU-T X.690 section 8.1.3
@@ -596,7 +727,7 @@ func (s *String) readASN1(out *String, outTag *Tag, skipHeader bool) bool {
 	if uint32(int(length)) != length || !s.ReadBytes((*[]byte)(out), int(length)) {
 		return false
 	}
-	if skipHeader && !out.Skip(int(headerLen)) {
+	if skipHeader && out != nil && !out.Skip(int(headerLen)) {
 		panic("cryptobyte: internal error")
 	}
 

cryptobyte/asn1/asn1.go

@@ -0,0 +1,46 @@
+// Copyright 2017 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 asn1 contains supporting types for parsing and building ASN.1
+// messages with the cryptobyte package.
+package asn1 // import "golang.org/x/crypto/cryptobyte/asn1"
+
+// Tag represents an ASN.1 identifier octet, consisting of a tag number
+// (indicating a type) and class (such as context-specific or constructed).
+//
+// Methods in the cryptobyte package only support the low-tag-number form, i.e.
+// a single identifier octet with bits 7-8 encoding the class and bits 1-6
+// encoding the tag number.
+type Tag uint8
+
+const (
+	classConstructed     = 0x20
+	classContextSpecific = 0x80
+)
+
+// Constructed returns t with the constructed class bit set.
+func (t Tag) Constructed() Tag { return t | classConstructed }
+
+// ContextSpecific returns t with the context-specific class bit set.
+func (t Tag) ContextSpecific() Tag { return t | classContextSpecific }
+
+// The following is a list of standard tag and class combinations.
+const (
+	BOOLEAN           = Tag(1)
+	INTEGER           = Tag(2)
+	BIT_STRING        = Tag(3)
+	OCTET_STRING      = Tag(4)
+	NULL              = Tag(5)
+	OBJECT_IDENTIFIER = Tag(6)
+	ENUM              = Tag(10)
+	UTF8String        = Tag(12)
+	SEQUENCE          = Tag(16 | classConstructed)
+	SET               = Tag(17 | classConstructed)
+	PrintableString   = Tag(19)
+	T61String         = Tag(20)
+	IA5String         = Tag(22)
+	UTCTime           = Tag(23)
+	GeneralizedTime   = Tag(24)
+	GeneralString     = Tag(27)
+)

cryptobyte/asn1_test.go

@@ -6,17 +6,19 @@ package cryptobyte
 
 import (
 	"bytes"
-	"encoding/asn1"
+	encoding_asn1 "encoding/asn1"
 	"math/big"
 	"reflect"
 	"testing"
 	"time"
+
+	"golang.org/x/crypto/cryptobyte/asn1"
 )
 
 type readASN1Test struct {
 	name string
 	in   []byte
-	tag  Tag
+	tag  asn1.Tag
 	ok   bool
 	out  interface{}
 }
@@ -194,7 +196,7 @@ func TestReadASN1IntegerInvalid(t *testing.T) {
 	}
 }
 
-func TestReadASN1ObjectIdentifier(t *testing.T) {
+func TestASN1ObjectIdentifier(t *testing.T) {
 	testData := []struct {
 		in  []byte
 		ok  bool
@@ -212,10 +214,23 @@ func TestReadASN1ObjectIdentifier(t *testing.T) {
 
 	for i, test := range testData {
 		in := String(test.in)
-		var out asn1.ObjectIdentifier
+		var out encoding_asn1.ObjectIdentifier
 		ok := in.ReadASN1ObjectIdentifier(&out)
 		if ok != test.ok || ok && !out.Equal(test.out) {
 			t.Errorf("#%d: in.ReadASN1ObjectIdentifier() = %v, want %v; out = %v, want %v", i, ok, test.ok, out, test.out)
+			continue
+		}
+
+		var b Builder
+		b.AddASN1ObjectIdentifier(out)
+		result, err := b.Bytes()
+		if builderOk := err == nil; test.ok != builderOk {
+			t.Errorf("#%d: error from Builder.Bytes: %s", i, err)
+			continue
+		}
+		if test.ok && !bytes.Equal(result, test.in) {
+			t.Errorf("#%d: reserialisation didn't match, got %x, want %x", i, result, test.in)
+			continue
 		}
 	}
 }
@@ -250,7 +265,7 @@ func TestReadASN1GeneralizedTime(t *testing.T) {
 		{"201001020304-10Z", false, time.Time{}},
 	}
 	for i, test := range testData {
-		in := String(append([]byte{asn1.TagGeneralizedTime, byte(len(test.in))}, test.in...))
+		in := String(append([]byte{byte(asn1.GeneralizedTime), byte(len(test.in))}, test.in...))
 		var out time.Time
 		ok := in.ReadASN1GeneralizedTime(&out)
 		if ok != test.ok || ok && !reflect.DeepEqual(out, test.out) {
@@ -263,20 +278,20 @@ func TestReadASN1BitString(t *testing.T) {
 	testData := []struct {
 		in  []byte
 		ok  bool
-		out asn1.BitString
+		out encoding_asn1.BitString
 	}{
-		{[]byte{}, false, asn1.BitString{}},
-		{[]byte{0x00}, true, asn1.BitString{}},
-		{[]byte{0x07, 0x00}, true, asn1.BitString{Bytes: []byte{0}, BitLength: 1}},
-		{[]byte{0x07, 0x01}, false, asn1.BitString{}},
-		{[]byte{0x07, 0x40}, false, asn1.BitString{}},
-		{[]byte{0x08, 0x00}, false, asn1.BitString{}},
-		{[]byte{0xff}, false, asn1.BitString{}},
-		{[]byte{0xfe, 0x00}, false, asn1.BitString{}},
+		{[]byte{}, false, encoding_asn1.BitString{}},
+		{[]byte{0x00}, true, encoding_asn1.BitString{}},
+		{[]byte{0x07, 0x00}, true, encoding_asn1.BitString{Bytes: []byte{0}, BitLength: 1}},
+		{[]byte{0x07, 0x01}, false, encoding_asn1.BitString{}},
+		{[]byte{0x07, 0x40}, false, encoding_asn1.BitString{}},
+		{[]byte{0x08, 0x00}, false, encoding_asn1.BitString{}},
+		{[]byte{0xff}, false, encoding_asn1.BitString{}},
+		{[]byte{0xfe, 0x00}, false, encoding_asn1.BitString{}},
 	}
 	for i, test := range testData {
 		in := String(append([]byte{3, byte(len(test.in))}, test.in...))
-		var out asn1.BitString
+		var out encoding_asn1.BitString
 		ok := in.ReadASN1BitString(&out)
 		if ok != test.ok || ok && (!bytes.Equal(out.Bytes, test.out.Bytes) || out.BitLength != test.out.BitLength) {
 			t.Errorf("#%d: in.ReadASN1BitString() = %v, want %v; out = %v, want %v", i, ok, test.ok, out, test.out)

cryptobyte/builder.go

@@ -10,15 +10,25 @@ import (
 )
 
 // A Builder builds byte strings from fixed-length and length-prefixed values.
+// Builders either allocate space as needed, or are ‘fixed’, which means that
+// they write into a given buffer and produce an error if it's exhausted.
+//
 // The zero value is a usable Builder that allocates space as needed.
+//
+// Simple values are marshaled and appended to a Builder using methods on the
+// Builder. Length-prefixed values are marshaled by providing a
+// BuilderContinuation, which is a function that writes the inner contents of
+// the value to a given Builder. See the documentation for BuilderContinuation
+// for details.
 type Builder struct {
-	err           error
-	result        []byte
-	fixedSize     bool
-	child         *Builder
-	offset        int
-	pendingLenLen int
-	pendingIsASN1 bool
+	err            error
+	result         []byte
+	fixedSize      bool
+	child          *Builder
+	offset         int
+	pendingLenLen  int
+	pendingIsASN1  bool
+	inContinuation *bool
 }
 
 // NewBuilder creates a Builder that appends its output to the given buffer.
@@ -86,9 +96,9 @@ func (b *Builder) AddBytes(v []byte) {
 
 // BuilderContinuation is continuation-passing interface for building
 // length-prefixed byte sequences. Builder methods for length-prefixed
-// sequences (AddUint8LengthPrefixed etc.) will invoke the BuilderContinuation
+// sequences (AddUint8LengthPrefixed etc) will invoke the BuilderContinuation
 // supplied to them. The child builder passed to the continuation can be used
-// to build the content of the length-prefixed sequence. Example:
+// to build the content of the length-prefixed sequence. For example:
 //
 //   parent := cryptobyte.NewBuilder()
 //   parent.AddUint8LengthPrefixed(func (child *Builder) {
@@ -102,8 +112,19 @@ func (b *Builder) AddBytes(v []byte) {
 // length prefix. After the continuation returns, the child must be considered
 // invalid, i.e. users must not store any copies or references of the child
 // that outlive the continuation.
+//
+// If the continuation panics with a value of type BuildError then the inner
+// error will be returned as the error from Bytes. If the child panics
+// otherwise then Bytes will repanic with the same value.
 type BuilderContinuation func(child *Builder)
 
+// BuildError wraps an error. If a BuilderContinuation panics with this value,
+// the panic will be recovered and the inner error will be returned from
+// Builder.Bytes.
+type BuildError struct {
+	Err error
+}
+
 // AddUint8LengthPrefixed adds a 8-bit length-prefixed byte sequence.
 func (b *Builder) AddUint8LengthPrefixed(f BuilderContinuation) {
 	b.addLengthPrefixed(1, false, f)
@@ -119,6 +140,34 @@ func (b *Builder) AddUint24LengthPrefixed(f BuilderContinuation) {
 	b.addLengthPrefixed(3, false, f)
 }
 
+// AddUint32LengthPrefixed adds a big-endian, 32-bit length-prefixed byte sequence.
+func (b *Builder) AddUint32LengthPrefixed(f BuilderContinuation) {
+	b.addLengthPrefixed(4, false, f)
+}
+
+func (b *Builder) callContinuation(f BuilderContinuation, arg *Builder) {
+	if !*b.inContinuation {
+		*b.inContinuation = true
+
+		defer func() {
+			*b.inContinuation = false
+
+			r := recover()
+			if r == nil {
+				return
+			}
+
+			if buildError, ok := r.(BuildError); ok {
+				b.err = buildError.Err
+			} else {
+				panic(r)
+			}
+		}()
+	}
+
+	f(arg)
+}
+
 func (b *Builder) addLengthPrefixed(lenLen int, isASN1 bool, f BuilderContinuation) {
 	// Subsequent writes can be ignored if the builder has encountered an error.
 	if b.err != nil {
@@ -128,15 +177,20 @@ func (b *Builder) addLengthPrefixed(lenLen int, isASN1 bool, f BuilderContinuati
 	offset := len(b.result)
 	b.add(make([]byte, lenLen)...)
 
+	if b.inContinuation == nil {
+		b.inContinuation = new(bool)
+	}
+
 	b.child = &Builder{
-		result:        b.result,
-		fixedSize:     b.fixedSize,
-		offset:        offset,
-		pendingLenLen: lenLen,
-		pendingIsASN1: isASN1,
+		result:         b.result,
+		fixedSize:      b.fixedSize,
+		offset:         offset,
+		pendingLenLen:  lenLen,
+		pendingIsASN1:  isASN1,
+		inContinuation: b.inContinuation,
 	}
 
-	f(b.child)
+	b.callContinuation(f, b.child)
 	b.flushChild()
 	if b.child != nil {
 		panic("cryptobyte: internal error")

cryptobyte/cryptobyte_test.go

@@ -6,6 +6,7 @@ package cryptobyte
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"testing"
 )
@@ -18,6 +19,54 @@ func builderBytesEq(b *Builder, want ...byte) error {
 	return nil
 }
 
+func TestContinuationError(t *testing.T) {
+	const errorStr = "TestContinuationError"
+	var b Builder
+	b.AddUint8LengthPrefixed(func(b *Builder) {
+		b.AddUint8(1)
+		panic(BuildError{Err: errors.New(errorStr)})
+	})
+
+	ret, err := b.Bytes()
+	if ret != nil {
+		t.Error("expected nil result")
+	}
+	if err == nil {
+		t.Fatal("unexpected nil error")
+	}
+	if s := err.Error(); s != errorStr {
+		t.Errorf("expected error %q, got %v", errorStr, s)
+	}
+}
+
+func TestContinuationNonError(t *testing.T) {
+	defer func() {
+		recover()
+	}()
+
+	var b Builder
+	b.AddUint8LengthPrefixed(func(b *Builder) {
+		b.AddUint8(1)
+		panic(1)
+	})
+
+	t.Error("Builder did not panic")
+}
+
+func TestGeneratedPanic(t *testing.T) {
+	defer func() {
+		recover()
+	}()
+
+	var b Builder
+	b.AddUint8LengthPrefixed(func(b *Builder) {
+		var p *byte
+		*p = 0
+	})
+
+	t.Error("Builder did not panic")
+}
+
 func TestBytes(t *testing.T) {
 	var b Builder
 	v := []byte("foobarbaz")

cryptobyte/example_test.go

@@ -5,9 +5,11 @@
 package cryptobyte_test
 
 import (
-	"encoding/asn1"
+	"errors"
 	"fmt"
+
 	"golang.org/x/crypto/cryptobyte"
+	"golang.org/x/crypto/cryptobyte/asn1"
 )
 
 func ExampleString_lengthPrefixed() {
@@ -37,7 +39,7 @@ func ExampleString_lengthPrefixed() {
 	fmt.Printf("%#v\n", result)
 }
 
-func ExampleString_asn1() {
+func ExampleString_aSN1() {
 	// This is an example of parsing ASN.1 data that looks like:
 	//    Foo ::= SEQUENCE {
 	//      version [6] INTEGER DEFAULT 0
@@ -51,12 +53,12 @@ func ExampleString_asn1() {
 		data, inner, versionBytes cryptobyte.String
 		haveVersion               bool
 	)
-	if !input.ReadASN1(&inner, cryptobyte.Tag(asn1.TagSequence).Constructed()) ||
+	if !input.ReadASN1(&inner, asn1.SEQUENCE) ||
 		!input.Empty() ||
-		!inner.ReadOptionalASN1(&versionBytes, &haveVersion, cryptobyte.Tag(6).Constructed().ContextSpecific()) ||
+		!inner.ReadOptionalASN1(&versionBytes, &haveVersion, asn1.Tag(6).Constructed().ContextSpecific()) ||
 		(haveVersion && !versionBytes.ReadASN1Integer(&version)) ||
 		(haveVersion && !versionBytes.Empty()) ||
-		!inner.ReadASN1(&data, asn1.TagOctetString) ||
+		!inner.ReadASN1(&data, asn1.OCTET_STRING) ||
 		!inner.Empty() {
 		panic("bad format")
 	}
@@ -65,7 +67,7 @@ func ExampleString_asn1() {
 	fmt.Printf("haveVersion: %t, version: %d, data: %s\n", haveVersion, version, string(data))
 }
 
-func ExampleBuilder_asn1() {
+func ExampleBuilder_aSN1() {
 	// This is an example of building ASN.1 data that looks like:
 	//    Foo ::= SEQUENCE {
 	//      version [6] INTEGER DEFAULT 0
@@ -77,9 +79,9 @@ func ExampleBuilder_asn1() {
 	const defaultVersion = 0
 
 	var b cryptobyte.Builder
-	b.AddASN1(cryptobyte.Tag(asn1.TagSequence).Constructed(), func(b *cryptobyte.Builder) {
+	b.AddASN1(asn1.SEQUENCE, func(b *cryptobyte.Builder) {
 		if version != defaultVersion {
-			b.AddASN1(cryptobyte.Tag(6).Constructed().ContextSpecific(), func(b *cryptobyte.Builder) {
+			b.AddASN1(asn1.Tag(6).Constructed().ContextSpecific(), func(b *cryptobyte.Builder) {
 				b.AddASN1Int64(version)
 			})
 		}
@@ -118,3 +120,35 @@ func ExampleBuilder_lengthPrefixed() {
 	// Output: 000c0568656c6c6f05776f726c64
 	fmt.Printf("%x\n", result)
 }
+
+func ExampleBuilder_lengthPrefixOverflow() {
+	// Writing more data that can be expressed by the length prefix results
+	// in an error from Bytes().
+
+	tooLarge := make([]byte, 256)
+
+	var b cryptobyte.Builder
+	b.AddUint8LengthPrefixed(func(b *cryptobyte.Builder) {
+		b.AddBytes(tooLarge)
+	})
+
+	result, err := b.Bytes()
+	fmt.Printf("len=%d err=%s\n", len(result), err)
+
+	// Output: len=0 err=cryptobyte: pending child length 256 exceeds 1-byte length prefix
+}
+
+func ExampleBuilderContinuation_errorHandling() {
+	var b cryptobyte.Builder
+	// Continuations that panic with a BuildError will cause Bytes to
+	// return the inner error.
+	b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
+		b.AddUint32(0)
+		panic(cryptobyte.BuildError{Err: errors.New("example error")})
+	})
+
+	result, err := b.Bytes()
+	fmt.Printf("len=%d err=%s\n", len(result), err)
+
+	// Output: len=0 err=example error
+}

cryptobyte/string.go

@@ -2,9 +2,19 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Package cryptobyte implements building and parsing of byte strings for
-// DER-encoded ASN.1 and TLS messages. See the examples for the Builder and
-// String types to get started.
+// Package cryptobyte contains types that help with parsing and constructing
+// length-prefixed, binary messages, including ASN.1 DER. (The asn1 subpackage
+// contains useful ASN.1 constants.)
+//
+// The String type is for parsing. It wraps a []byte slice and provides helper
+// functions for consuming structures, value by value.
+//
+// The Builder type is for constructing messages. It providers helper functions
+// for appending values and also for appending length-prefixed submessages –
+// without having to worry about calculating the length prefix ahead of time.
+//
+// See the documentation and examples for the Builder and String types to get
+// started.
 package cryptobyte // import "golang.org/x/crypto/cryptobyte"
 
 // String represents a string of bytes. It provides methods for parsing