CGI-SE-Trusted-Services / c2c-common

http://pvendil.github.io/c2c-common/
GNU Affero General Public License v3.0
46 stars 18 forks source link

EtsiTs102941Data COER encoding #13

Closed nickdurante closed 3 years ago

nickdurante commented 3 years ago

Hi @pvendil,

I'm running into some errors while encoding and decoding EtsiTs102941Data from c2c-common (v2.0.0-Beta4) to Python. While the other datastructures (such as EtsiTs103097Data, EtsiTs103097Data-Signed, InnerEcRequestSignedForPop, etc..) are interpreted correctly in both languages, the EtsiTs102941Data isn't. I was wondering if the encoding procedure in your library has changed for that datastructure.

For example, this is a Base64 encoded EtsiTs102941Data generated with c2c-common:

AQGAA4EAQAOAgYUAGlNvbWVFbnJvbENyZWRDYW5vbmljYWxOYW1lAYCAgxxz8bh10xe90S+q3vtivF3+kz8ZZOhHM9RVZH2CG2H8AICDSs6ULGmmugAdkHesdicFqC6SYT3GHXAyuzPxCwfdhaF8gQdlbnJvbGwxHBBbGIYABYMBAYAC8CMBAYACAm+AAgEyQAICbwAB7wcbj3KQgoCAVFnWmgodt6dVR3y/nrDtX4VOERsiTo8UXIJB1eejlGITn2Tmzcg36CuNXt8tWhjrap5jQumvRSrdNzbz6lb4Bg==

And it is read in Java without problems with:

byte[] data102941bytes = Base64.getDecoder().decode(<that string>);
EtsiTs102941Data testdata = new EtsiTs102941Data(data102941bytes);

At the same time in Python the library asn1tools with the asn files compiled from the ETSI Gitlab fails to decode it:

asn1compiler = asn1tools.compile_files(<Etsi ASN specification>, codec='oer')
etsi102941data2 = asn1compiler.decode('EtsiTs102941Data', base64.b64decode(<that string>))

On the other hand, the following python code:

data102941_content = {"protocolVersion": 3, "content": ("unsecuredData", inner_ec_signed_for_pop_bytes)}
data102941 = {"version": 1, "content": ("enrolmentRequest", data102941_content)}
data102941_bytes = asn1compiler.encode('EtsiTs102941Data', data102941)

produces this Base64 encoded EtsiTs102941Data which is read correctly in Python and not in c2c-common:

AYADgIHiA4EAQAOAgYsAIGVucm9sbWVudENyZWRlbnRpYWxDYW5vbmljYWxOYW1lAYCAg+vP21dSRvv5RuyV+dAcRldmm8HNmTgXoZKAN+mADosYAICDyDour0L2Eg07m5npv9dox7wuSs/3S29xDL7M1Qb85/d8gQdlbnJvbGwxHBBbGIYABYMBAYAC8CMBAYACAm+AAgEyQAICbwAAMXRKblwsgoCAvrb1ky7z7923yEyPIJTwDmXLt16f6N49dBOxKy+WNsAiwUmyMBhkhj2WQ4lWYWrNKoEb/5/b7Np7dSN976R69A==

c2c-common fails with this error when reading this python generated string:

java.io.IOException: Error decoding COER enumeration, no matching enumeration value exists for ordinal: 108
    at org.certificateservices.custom.c2x.asn1.coer.COEREncodeHelper.readEnumeratonValueAsEnumeration(COEREncodeHelper.java:190)
    at org.certificateservices.custom.c2x.asn1.coer.COEREnumeration.decode(COEREnumeration.java:70)
    at org.certificateservices.custom.c2x.asn1.coer.COERSequence.decode(COERSequence.java:296)
    at org.certificateservices.custom.c2x.asn1.coer.COERChoice.decode(COERChoice.java:102)
    at org.certificateservices.custom.c2x.asn1.coer.COERSequence.decode(COERSequence.java:296)
    at org.certificateservices.custom.c2x.etsits102941.v131.datastructs.messagesca.EtsiTs102941Data.<init>(EtsiTs102941Data.java:62)
    at main.LoadERString.main(LoadERString.java:16)

Keeping in mind that the other datastructures are correctly interpreted between the two languages, you shed some light on this matter?

nickdurante commented 3 years ago

Found the solution. The ASN file from ETSI specified:

EtsiTs102941Data::= SEQUENCE {
  version Version (v1),
  content EtsiTs102941DataContent
  }

Which didn't work well with the asn1tools python compiler, changing it to:

EtsiTs102941Data::= SEQUENCE {
  version Version,
  content EtsiTs102941DataContent
  }

Solved the issue.

klepretr commented 3 years ago

Hi @nickdurante and @pvendil,

Your issue is very interesting. I'm facing to a similar issue with the version field in this EtsiTs102941Data structure. I'm not sure modifying the ASN file is the solution.

The problem seems to be in the Version.class. This class extends COERInteger. And the encoding of a COERInteger object depends on:

But the declaration of the constant is using the default constructor, without the minValue or the maxValue.

public static final Version V1 = new Version(1);

from PKI_TS102941/EtsiTs102941MessagesCA.asn file

EtsiTs102941Data::= SEQUENCE {
  version Version (v1),
  content EtsiTs102941DataContent
  }

and from PKI_TS102941/EtsiTs102941BaseTypes.asn file

Version ::= INTEGER {v1(1)}

My interpretation of the asn1 files is as follows, the version field is always set to v1 value, so the minimum value and the maximum value is also 1. May this is a false interpretation, feel free to correct me if I'm wrong.

So I suggest to add a new constructor with explicit max and min values:

public Version(long value, long minValue, long maxValue) {
    super(value, minValue, maxValue);
}

and to replace the declare of V1 by:

public static final Version V1 = new Version(1, 1, 1);

I'm currently working to a patch on the fork I done. I can create a PR, if @pvendil think it's relevant.

Thank you.

pvendil commented 3 years ago

Hi

Sorry for my late reply.

Regarding Version are you sure you have the correct COER encoding? I have tried using the asn1playground to encode the Version that I usually use as reference and it gives me an encoding of "0101" (as it currently is) and not "01" you get with Version(1,1,1).

[X]

Med vänlig hälsning / Best Regards Philip Vendil, Certificate Service Development Team Lead CGI Sverige AB | 164 98 Stockholm | Sweden Visiting address: Torshamnsgatan 24, KISTA Telephone: +46 709 88 58 14 @.**@.>| www.cgi.sehttp://www.cgi.se/


Från: Kévin Leprêtre @.***> Skickat: den 7 april 2021 10:54:21 Till: pvendil/c2c-common Kopia: Vendil, Philip (SE); Mention Ämne: Re: [pvendil/c2c-common] EtsiTs102941Data COER encoding (#13)

EXTERNAL SENDER: Do not click any links or open any attachments unless you trust the sender and know the content is safe. EXPÉDITEUR EXTERNE: Ne cliquez sur aucun lien et n’ouvrez aucune pièce jointe à moins qu’ils ne proviennent d’un expéditeur fiable, ou que vous ayez l'assurance que le contenu provient d'une source sûre.

Hi @nickdurantehttps://urldefense.com/v3/__https://github.com/nickdurante__;!!AaIhyw!64UO6ZjsNnkwOp-Q7bYtDjqQjNGAwpFFIv1CRi6P89HZcAXZ7tu6E54Sjd4F$ and @pvendilhttps://urldefense.com/v3/__https://github.com/pvendil__;!!AaIhyw!64UO6ZjsNnkwOp-Q7bYtDjqQjNGAwpFFIv1CRi6P89HZcAXZ7tu6E_nUUJ_w$,

Your issue is very interesting. I'm facing to a similar issue with the version field in this EtsiTs102941Data structure. I'm not sure modifying the ASN file is the solution.

The problem seems to be in the Version.class. This class extends COERInteger. And the encoding of a COERInteger object depends on:

But the declaration of the constant is using the default constructor, without the minValue or the maxValue.

public static final Version V1 = new Version(1);

from PKI_TS102941/EtsiTs102941MessagesCA.asn file

EtsiTs102941Data::= SEQUENCE { version Version (v1), content EtsiTs102941DataContent }

and from PKI_TS102941/EtsiTs102941BaseTypes.asn file

Version ::= INTEGER {v1(1)}

My interpretation of the asn1 files is as follows, the version field is always set to v1 value, so the minimum value and the maximum value is also 1. May this is a false interpretation, feel free to correct me if I'm wrong.

So I suggest to add a new constructor with explicit max and min values:

public Version(long value, long minValue, long maxValue) { super(value, minValue, maxValue); }

and to replace the declare of V1 by:

public static final Version V1 = new Version(1, 1, 1);

I'm currently working to a patch on the fork I done. I can create an MR, if @pvendilhttps://urldefense.com/v3/__https://github.com/pvendil__;!!AaIhyw!64UO6ZjsNnkwOp-Q7bYtDjqQjNGAwpFFIv1CRi6P89HZcAXZ7tu6E_nUUJ_w$ think it's relevant.

Thank you.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/pvendil/c2c-common/issues/13*issuecomment-814733715__;Iw!!AaIhyw!64UO6ZjsNnkwOp-Q7bYtDjqQjNGAwpFFIv1CRi6P89HZcAXZ7tu6Ex-1B3T1$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ACOB4P5HR3PE6H32B5RZWN3THQMT3ANCNFSM42E5OVBA__;!!AaIhyw!64UO6ZjsNnkwOp-Q7bYtDjqQjNGAwpFFIv1CRi6P89HZcAXZ7tu6E8VbwAEu$.

klepretr commented 3 years ago

Hi @pvendil,

Thank you to help us. I'm absolutely not an expert of asn.1. But I detected this encoding issue after a systematic signature verification error on EtsiTs102941 exchanges.

Did you test with the asn1playground tool provided by OSS Nokalva ?

This morning, I tried to compile the schema above :

Version ::= INTEGER {v1(1)}

in decoding mode

Hexadecimal Result
0101 Pass
01 Fail

Screenshot_2021-04-08 ASN 1 Playground ASN 1 compiler, encoder, decoder(1)

After that, I tried this other one schema composed of a random structure:

Version ::= INTEGER {v1(1)}
RandomStructure ::= SEQUENCE {
   version Version (v1)
}

in decoding mode

Hexadecimal Result
0101 Fail
01 Pass

Screenshot_2021-04-08 ASN 1 Playground ASN 1 compiler, encoder, decoder

I guess the second one schema declare a "constant" value for Version in the RandomStructure. May encoding rules are different in this case.

What's your opinion ?

pvendil commented 3 years ago

Hi again, yes your example seems very strange. I have to look into the COER spec and test some myself to figure these things out.

I will look at this coming days.

Med vänlig hälsning / Best Regards Philip Vendil, Certificate Service Development Team Lead CGI Sverige AB | 164 98 Stockholm | Sweden Visiting address: Torshamnsgatan 24, KISTA Telephone: +46 709 88 58 14 @.**@.>| www.cgi.sehttp://www.cgi.se/


Från: Kévin Leprêtre @.***> Skickat: den 8 april 2021 09:44:46 Till: pvendil/c2c-common Kopia: Vendil, Philip (SE); Mention Ämne: Re: [pvendil/c2c-common] EtsiTs102941Data COER encoding (#13)

EXTERNAL SENDER: Do not click any links or open any attachments unless you trust the sender and know the content is safe. EXPÉDITEUR EXTERNE: Ne cliquez sur aucun lien et n’ouvrez aucune pièce jointe à moins qu’ils ne proviennent d’un expéditeur fiable, ou que vous ayez l'assurance que le contenu provient d'une source sûre.

Hi @pvendilhttps://urldefense.com/v3/__https://github.com/pvendil__;!!AaIhyw!4tpV1gsYNTofs2HTsJaBKjFU48Jh1IWs6Ek-tJ_NU35yVm19PTa5y4WRrBrt$,

Thank you to help us. I'm absolutely not an expert of asn.1. But I detected this encoding issue after a systematic signature verification error on EtsiTs102941 exchanges.

Did you test with the asn1playground tool provided by OSS Nokalva ?

This morning, I tried to compile the schema above :

Version ::= INTEGER {v1(1)}

in decoding mode Hexadecimal Result 0101 Pass 01 Fail

[Screenshot_2021-04-08 ASN 1 Playground ASN 1 compiler, encoder, decoder(1)]https://urldefense.com/v3/__https://user-images.githubusercontent.com/26330160/113987421-aea74180-984e-11eb-824f-c7a16cf9c0cb.png__;!!AaIhyw!4tpV1gsYNTofs2HTsJaBKjFU48Jh1IWs6Ek-tJ_NU35yVm19PTa5yxUho30Z$

After that, I tried this other one schema composed of a random structure:

Version ::= INTEGER {v1(1)} RandomStructure ::= SEQUENCE { version Version (v1) }

in decoding mode Hexadecimal Result 0101 Fail 01 Pass

[Screenshot_2021-04-08 ASN 1 Playground ASN 1 compiler, encoder, decoder]https://urldefense.com/v3/__https://user-images.githubusercontent.com/26330160/113987429-b0710500-984e-11eb-9674-542e3a66c796.png__;!!AaIhyw!4tpV1gsYNTofs2HTsJaBKjFU48Jh1IWs6Ek-tJ_NU35yVm19PTa5y7EwlFbY$

I guess the second one schema declare a "constant" value for Version in the RandomStructure. May encoding rules are different in this case.

What's your opinion ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/pvendil/c2c-common/issues/13*issuecomment-815533489__;Iw!!AaIhyw!4tpV1gsYNTofs2HTsJaBKjFU48Jh1IWs6Ek-tJ_NU35yVm19PTa5yw0j98Yu$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ACOB4P72YSPMQHJCXQMTPBTTHVNG5ANCNFSM42E5OVBA__;!!AaIhyw!4tpV1gsYNTofs2HTsJaBKjFU48Jh1IWs6Ek-tJ_NU35yVm19PTa5y0Ju7L6E$.