search

wbond / asn1crypto

Python ASN.1 library with a focus on performance and a pythonic API
MIT License
335 stars 140 forks source link

Support for A-XDR: Adapted External Data Representation #196

Closed Krolken closed 3 years ago

Krolken commented 3 years ago

Hi

I am wondering if it would be possible to add support (or how I would subclass / exchange parsers) to support A-XDR (IEC 61334-6), Adapted External Data Representation or "XDR for DLMS". DLMS is used in most electricity meters.

I am writing a protocol library for DLMS and I think I started out over 2 years ago, then I used another ASN1 library but go so annoyed of the API so I shelved the project for a while. Now I am at it again to add full DLMS support to the library.

Since I was annoyed at the ASN1 API last time I built my own parser. But I am hitting annoying edge-cases so I was looking into asn1crypto since the API seemed nice. Work quite well for the parse I added and now I am looking to use it everywhere where BER encoding is used.

But the problem is that BER is only used for ACSE portions of the DLMS protocol. In everything else they use X-ADR.

Which i pretty much BER, just tag away the length option since everyone knows the length of the data. The aim of this is to reduce the APDU size.

Example


# Sequence of integer and boolean in BER
seq = b"\x30\x02\x02\x02\x12\x34\x01\x01\x00"

# Sequence of interger and boolean in X-ADR
# Sequence lenght is still there since it is variable, but integer and boolan lenght bytes have been stripped.
seq = b"\x30\x02\x02\x12\x34\x01\x00"

For optional value they are encoded with \x00 of not present and \x01+ value if present.

For default values the defaiult is encoded with \x00 and \x01\ + value if a non default values is used.

Another "annoying" thing is that they mix encoding in APDUs. Some field carry X-ADR encoded data and that is then put in a BER encoded OctetString.

I think all my ASCE code base would be alot more sane if I would start using asn1crypto, but also if I could use simmilar API as asn1crypto for the X-ADR parts the codebase would be much more manageable and I could spend time on the application instead of ensuring bytes gets parsed correctly.

So question is if you are open to an X-ADR contribution and how you would prefer it. Or if you could give me some pointers to change the BER/DER loading/dumping to facilitate X-ADR till using you nice API for the ASN.1 specs.

wbond commented 3 years ago

Unfortunately I don't have much time for my open source projects these days, however @joernheissler is also involved with this project and may have some thoughts.

The high-level parsing and dumping of DER encoded byte strings happens in parser.py. My hunch is that to make it possible for asn1crypto to support different encoding schemes, each class would need to have a property containing it's encoding and then parser.py would need need to be adapted to have a "pluggable" encoding registry.

However, that change wouldn't adapt each concrete ASN.1 type's encoder. My guess is that there would either need to be a new abstraction layer for encoding/parsing each encoding for each type, or each type would need to have implementations of each encoding and it would be called based on the object's encoding property.

I have a feeling it would end up being a somewhat invasive change. If such work was done, it should probably be done with the intent of making it possible to support other encodings also, such as CER and PER. My concern with adding more abstraction layers is that each layer adds a performance hit, just in terms of function call overhead.

Overall my gut reaction is that this would be a bunch of specialized code for very few users, and a maintenance burden. Partially because it doesn't seem to be widely used, but also because it seems the spec for A-XDR is something you have to purchase.

Krolken commented 3 years ago

Yeah. After reading the code a bit more it seems like alot of work to get make it fit well into the current code base. But that seems to be mostly because the classes aren't that easily changed.

I think I could make something work for the dumping as I could override the dump method on new classes and call them AxdrInterger or something instead. If a class overridable _dump_header where available it could be done more easily. But it seems harder to override the parsing as I need to provide the length of it from the class.

Its ok to define my own classes to do the stupid X-ADR, I just want the same kind of nice ASN1 handling everywhere in the code so it becomes manageable. If I can't get it to work properly I guess I'll just have to reimplement the same API for it.

And yes, you need to buy the standard. The joy of IEC standardization :)

I'll fiddle around with it when I have time.

But how would you stand on adding a class method _dump_header or provide it via a mixin, that would just call the current _dump_header. If it is organized like that it would be possible to exchange the class method _dump_header with my own that handles the A-XDR nonsense. And hopefully it would't impact any current code out there.

Krolken commented 3 years ago

I'll go on an close this. I did some more coding and it is not gonna work anyway. Once you go into variable length it gets to be too much code trying to go around the asn1crypto implementation. I'll just keep on writing on my decoder and keep the BER and X-ADR encoding/decoding separate.