Skip to content

Commit

Permalink
cryptobyte: fix ReadOptionalASN1Boolean
Browse files Browse the repository at this point in the history
ReadOptionalASN1Boolean was completely broken, it would only work when
there were two BOOLEAN fields in a row, with the first being OPTIONAL
(which is itself invalid ASN.1 due to the ambiguity). This fixes it
to properly expect a BOOLEAN wrapped in a context-specific tag, as is
the case for all of the other ReadOptionalASN1* methods, and updates
its doc string.

This is a breaking change as it requires adding the tag field to
properly support context-specific tags. Given the method would
previously not work this seems like a reasonable breakage.

Fixes golang/go#43019

Change-Id: I42398256216c59988e249c90bc7aa668f64df945
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/274242
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Roland Shoemaker <roland@golang.org>
  • Loading branch information
rolandshoemaker authored and gopherbot committed Nov 9, 2023
1 parent ff15cd5 commit a2edfb5
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 6 deletions.
13 changes: 7 additions & 6 deletions cryptobyte/asn1.go
Original file line number Diff line number Diff line change
Expand Up @@ -733,13 +733,14 @@ func (s *String) ReadOptionalASN1OctetString(out *[]byte, outPresent *bool, tag
return true
}

// 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.
// It reports whether the operation was successful.
func (s *String) ReadOptionalASN1Boolean(out *bool, defaultValue bool) bool {
// ReadOptionalASN1Boolean attempts to read an optional ASN.1 BOOLEAN
// explicitly tagged with tag into out and advances. If no element with a
// matching tag is present, it sets "out" to defaultValue instead. It reports
// whether the read was successful.
func (s *String) ReadOptionalASN1Boolean(out *bool, tag asn1.Tag, defaultValue bool) bool {
var present bool
var child String
if !s.ReadOptionalASN1(&child, &present, asn1.BOOLEAN) {
if !s.ReadOptionalASN1(&child, &present, tag) {
return false
}

Expand All @@ -748,7 +749,7 @@ func (s *String) ReadOptionalASN1Boolean(out *bool, defaultValue bool) bool {
return true
}

return s.ReadASN1Boolean(out)
return child.ReadASN1Boolean(out)
}

func (s *String) readASN1(out *String, outTag *asn1.Tag, skipHeader bool) bool {
Expand Down
22 changes: 22 additions & 0 deletions cryptobyte/asn1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,28 @@ func TestReadASN1OptionalInteger(t *testing.T) {
}
}

const defaultBool = false

var optionalBoolTestData = []readASN1Test{
{"empty", []byte{}, 0xa0, true, false},
{"invalid", []byte{0xa1, 0x3, 0x1, 0x2, 0x7f}, 0xa1, false, defaultBool},
{"missing", []byte{0xa1, 0x3, 0x1, 0x1, 0x7f}, 0xa0, true, defaultBool},
{"present", []byte{0xa1, 0x3, 0x1, 0x1, 0xff}, 0xa1, true, true},
}

func TestReadASN1OptionalBoolean(t *testing.T) {
for _, test := range optionalBoolTestData {
t.Run(test.name, func(t *testing.T) {
in := String(test.in)
var out bool
ok := in.ReadOptionalASN1Boolean(&out, test.tag, defaultBool)
if ok != test.ok || ok && out != test.out.(bool) {
t.Errorf("in.ReadOptionalASN1Boolean() = %v, want %v; out = %v, want %v", ok, test.ok, out, test.out)
}
})
}
}

func TestReadASN1IntegerSigned(t *testing.T) {
testData64 := []struct {
in []byte
Expand Down

0 comments on commit a2edfb5

Please sign in to comment.