golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.7k stars 17.62k forks source link

proposal: encoding/asn1: extend support to rfc2578, application-define types and opaque sub-types #63399

Closed upsampled closed 1 year ago

upsampled commented 1 year ago

I was working on gosnmp/gosnmp and noticed it needed to ASN1 encode many values internally. When I tried to offload this work to encoding/asn1 instead, I found that it did not provide support for rfc2578 application-defined types used often in SNMP:

-- the "base types" defined here are:
--   3 built-in ASN.1 types: INTEGER, OCTET STRING, OBJECT IDENTIFIER
--   8 application-defined types: Integer32, IpAddress, Counter32,
--              Gauge32, Unsigned32, TimeTicks, Opaque, and Counter64

In addition to the above types, SNMP libraries also normaly explicitly support Opaque subtypes: OpaqueFloats, OpaqueDouble, OpaqueI64 and OpaqueU64 as defined in this draft rfc. While I am not thrilled that it is a draft RFC, it is adopted explicitly by net-snmp, other SNMP libraries followed suit, and then it showed up in MIBs (example MIB usage getting battery charge level).

I also imagine these types would/could be used in x509 encoding, but i don't have any examples offhand.

seankhliao commented 1 year ago

What does support look like for the package? API changes, behaviour changes, etc.

upsampled commented 1 year ago

Given the exiting tag-based design, I would suggest:

The additional un/marshal tags to be added

integer32:  causes int type to be marshaled as ASN.1, Integer32 value
uinteger32:  causes int type to be marshaled as ASN.1, Uinteger32 value
counter32:  causes int type be marshaled as ASN.1, Counter32 value   
gauge32:  causes int type to be marshaled as ASN.1, Guage32 value  
counter64:  causes int type to be marshaled as ASN.1, Counter64 value  
opaqueU64: causes int type to be marshaled as ASN.1, OpaqueU64 value 
opaqueI64: causes int type to be marshaled as ASN.1, OpaqueI64 value  
opaqueI64: causes int type to be marshaled as ASN.1, OpaqueI64 value  
opaqueF32: causes float type to be marshaled as ASN.1, OpaqueFloat value 
opaqueF64: causes float type to be marshaled as ASN.1, OpaqueDouble value  

The default marshaling behavior to change from unknown Go type for the following ASN1/Go type pairs

Go Type ASN.1 Type
net.IP if .To4() != nil IpAddress
time.Duration TimeTicks
float32 OpaqueFloat
float64 OpaqueDouble

If this is accepted, then an additional ticket can be opened to handle more modern, network-specific ASN.1 encoding. Looks like most IPv6 MIBs are just octet strings and can be handled with existing capabilities

ianlancetaylor commented 1 year ago

CC @agl @FiloSottile @golang/security

upsampled commented 1 year ago

Ok I was able to get most of the integer, application types working by playing with the existing encoding/asn1 tags: application and tag. For example the below encodes an Uinterger32.

    x := int32(9)
    out, err := asn1.MarshalWithParams(x, "application,tag:7")
    if err != nil {
        panic(err)
    }
    fmt.Printf("%x\n", out)

It would definitely be more clear as a single uinteger32 tag, but workable.

I have not been able encode any of the Opaque sub-types yet though.

upsampled commented 1 year ago

Ok, after looking into this more I wrote some comparison tests against net-snmp. Right now these tests show:

Given this, I'd like to refine the proposal further with:

The default marshaling behavior to change from unknown Go type for the following ASN1/Go type pairs

Go Type ASN.1 Type
net.IP if .To4() != nil IpAddress
net.IP if .To4() == nil OctString
time.Duration TimeTicks
float32 OpaqueFloat
float64 OpaqueDouble
uint64 Counter64

and the only additional tag needed would be:

opaque:  causes type to be marshaled as ASN.1, Opaque type if possible 

So this tag would use opaque encoding for uint64,int64 (ASN_OPAQUE_U64,ASN_OPAQUE_I64). May need a special case or workaround for uint64 to ASN_OPAQUE_COUNTER64.

rolandshoemaker commented 1 year ago

It's not clear to me that we should add this functionality to encoding/asn1.

These specific types are not ASN.1 base types, they are SMI/SNMP application types, and as such they should be handled by the user (the application), rather than being handled by encoding/asn1.

From a quick skim of the specification almost all of these types can be handled without any fancy encoding using the application,tag:... field tag syntax, i.e. an IpAddress is just a OCTET STRING with an application tag (0), an Unsigned32 is just a INTEGER with a application tag (2), etc. I.e.

IpAddress  []byte `asn1:"application,tag:0"`
Unsigned32 int `asn1:"application,tag:2"`
TimeTicks  time.Duration `asn1:"application,tag:3"`
Counter64  int `asn1:"application,tag:6"`

The opaque types are a bit weird, given that they are arbitrary encodings, wrapped in an OCTET STRING, but this can be accomplished by just using a []byte field and populating it with the result of the defined encoding (most of these encodings seem unrelated to BER, the float one appears to use the textual encoding from RFC 6340) for the wrapped type, before marshaling the entire struct (or just the byte slice if you're not marshaling a struct). I.e.

// encodeFloat returns a []byte with the RFC 6340 textual encoding of a float
encFloat := encodeFloat(f)
b, err := asn1.MarshalWithParams(encFloat, "application,tag:4") // Opaque OCTET STRING wrapper
upsampled commented 1 year ago

@rolandshoemaker my bad. I think I have just been looking at too much SNMP code that doesn't delineate where ASN.1 stops and SMI begins. The asn1 package provides enough functionality for a smi package to be created and provide the mentioned functionality. Closing ticket.