Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

encoding/asn1: error when unmarshalling SEQUENCE OF SET #27426

Open
kaxap opened this issue Aug 31, 2018 · 8 comments
Open

encoding/asn1: error when unmarshalling SEQUENCE OF SET #27426

kaxap opened this issue Aug 31, 2018 · 8 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@kaxap
Copy link
Contributor

kaxap commented Aug 31, 2018

What version of Go are you using (go version)?

go1.11 windows/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\xxx\AppData\Local\go-build
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\dev;C:\gopath
set GOPROXY=
set GORACE=
set GOROOT=C:\Go
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\xxx\AppData\Local\Temp\go-build551125774=/tmp/go-build -gno-rec
ord-gcc-switches

What did you do?

package main

import (
	"encoding/asn1"
	"fmt"
)

//` schema:
//
//	World-Schema DEFINITIONS AUTOMATIC TAGS ::=
//	BEGIN
//	SomeStruct ::= SEQUENCE
//	{
//		id      INTEGER,
//		field1  SEQUENCE OF SomeSet
//	}
//	SomeSet ::= SET
//	{
//		field2 INTEGER
//	}
//	END
//`

// data
//	{
//		"id":1,
//		"field1": [{"field2": 1}, {"field2": 2}]
//	}

// encode with http://asn1-playground.oss.com/
const encodedDer = "\x30\x0F\x80\x01\x01\xA1\x0A\x31\x03\x80\x01\x01\x31\x03\x80\x01\x02"

type SomeStruct struct {
	ID int `asn1:"tag:0"`
	SomeSetSlice []SomeSet `asn1:"tag:1,set"`
}

type SomeSet struct {
	Field2 int `asn1:"tag:0"`
}

func main() {
	var b SomeStruct
	_, err := asn1.Unmarshal([]byte(encodedDer), &b)
	if err != nil {
		panic(err)
	}
	fmt.Println(b)
}

play link:
https://play.golang.org/p/V-za5Cu1wkr

What did you expect to see?

SEQUENCE OF SET should be properly unmarshalled into a slice of structs. Documentation says that SET can be unmarshalled into a struct. So I believe a SEQUENCE OF SET is expected to be properly unmarshalled into a slice of structs.

What did you see instead?

asn1: structure error: sequence tag mismatch.

The problem might be in getUniversalTag function when it is called from parseSequenceOf. E.g.

	case reflect.Struct:
		return false, TagSequence, true, true

When struct is an element of slice there is no possibility to tag a struct as a SET (17) , it's always tagged as a SEQUENCE (16).
@kaxap kaxap changed the title encoding/asn1 error when unmarshalling SEQUENCE OF SET encoding/asn1: error when unmarshalling SEQUENCE OF SET Aug 31, 2018
kaxap added a commit to kaxap/go that referenced this issue Aug 31, 2018
@kaxap
encoding/asn1: Fix unmarshalling SEQUENCE OF SET. Fixes golang#27426
9140a0f
@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 4, 2018
@andybons andybons added this to the Unplanned milestone Sep 4, 2018
@andybons
Copy link
Member

andybons commented Sep 4, 2018

@FiloSottile

@ken5scal
Copy link

ken5scal commented Jan 3, 2019

I'm encountering a similar issue.

@gopherbot
Copy link

Change https://golang.org/cl/160819 mentions this issue: encoding/asn1: fix unmarshalling SEQUENCE OF SET. Fixes #27426

kaxap added a commit to kaxap/go that referenced this issue Feb 1, 2019
@kaxap
encoding/asn1: Fix unmarshalling SEQUENCE OF SET
d562cb9 
Fixes golang#27426
@kaxap kaxap mentioned this issue Aug 29, 2019
Closed
@bradfitz bradfitz reopened this Nov 8, 2019
@bradfitz
Copy link
Contributor

bradfitz commented Nov 8, 2019

Reverted as it caused crypto/tls failures.

You can try again when the Go 1.15 dev tree opens in Feb.

@bragi92
Copy link

bragi92 commented Mar 4, 2020

@bradfitz How do I use the Go 1.15 dev build to see if this issue is fixed or not?

I'm facing a similar problem when I'm trying to read a certificate using tls.loadx509keypair.

@shefalikamal
Copy link

shefalikamal commented Mar 14, 2020 •
edited

Hi,
@bradfitz I am facing similar issue when trying to unmarshal sequence of sets I am using golang 1.11.4. Is this issue resolved ??
@bragi92 Did you find any workaround on this I too have a composite extensions for which I am getting this error??

@bragi92
Copy link

bragi92 commented Mar 17, 2020

@shefalikamal For my scenario, I was the one who was generating the self signed certificate using a dotnet core executable. On removing the following lines of code from the certificate I was generating

//certificateGenerator.AddExtension(X509Extensions.ExtendedKeyUsage.Id, false,
// new AuthorityKeyIdentifier(
// new GeneralNames(new GeneralName(certName)), serialNumber));
the error got fixed and I was able to read the certificate using tls.loadx509keypair.

So I did work around the issue but I'm still unsure as to what the exact problem was! :(

@rolandshoemaker
Copy link
Member

This is an interesting one, encoding/asn1 seems to support SET of SEQUENCE (by using the SET suffix on typed slices, i.e. type RelativeDistinguishedNameSET []AttributeTypeAndValue from crypto/x509/pkix), but not SEQUENCE of SET (where the SET is a struct, rather than a nested SET OF...), which is what the documentation suggests:

If the type name of a slice element ends with "SET" then it's treated as if the "set" tag was set on it.

The confusing part here is what it refers to, is it the type of the element, or the slice. It seems to imply the former, but actually does the latter (the suffix is checked on the slice type, rather than the elements, and results in a SET of ... rather than a SEQUENCE of SET).

This behavior is currently relied on, e.g. for the previously mentioned pkix.RelativeDistinguishedNameSET, to generate SET of SEQUENCE, so it seems like that documentation should be fixed.

In order to support SEQUENCE of SET it seems like the approach suggested in CL 160819 is appropriate. The failures that change caused seem mainly due to existing usage of the SET suffix in crypto/x509 for x509.AttributeTypeAndValueSET which ended up being marshaled to a SET rather than a SEQUENCE. RFC 2986 defines this structure as a SEQUENCE, so the naming convention appears to be an error (or may have referred to some context in the past that has since been lost). In order to work in a CL 160819 world this type would need to be re-aliased internally so that asn1.Marshal doesn't try to convert it to a SET, which works fine but is a bit of a hack, and brings up that other non-stdlib code may be using this naming scheme and expecting to get a SEQUENCE rather than a SET.

The only other solution to this problem that I can think of is to add a new asn1 field parameter for slices which would get passed through to makeBody which would indicate that the elements of the slice should be SET, rather than the slice itself, say elements-set or something... (which would match what the documentation currently describes).

@gopherbot gopherbot mentioned this issue Apr 20, 2020
Closed
@rolandshoemaker rolandshoemaker mentioned this issue May 12, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

Successfully merging a pull request may close this issue.

encoding/asn1: fix unmarshalling SEQUENCE OF SET kaxap/go
8 participants
@bradfitz @andybons @kaxap @rolandshoemaker @gopherbot @ken5scal @bragi92 @shefalikamal