P1sec / pycrate

A Python library to ease the development of encoders and decoders for various protocols and file formats; contains ASN.1 and CSN.1 compilers.
GNU Lesser General Public License v2.1
381 stars 132 forks source link

printable_str contains \n\r, which causes invalid ASN comments #146

Closed impratikjaiswal closed 2 years ago

impratikjaiswal commented 3 years ago

Copied from \pycrate_asn1rt\utils.py:

printable_str = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'\
                '!"#$%&\'()*+,-./:;<=>?@[\\]^_`{|}~ \t\n\r\x0b\x0c'

printable_str is called while converting to ASN format (to_asn1()). Here \n\r are causing invalid ASN comments. printable_str should not contain any whitespace character like \t \n, \r, \x0b, \x0c.

Scenario 1: Current:

ef-arr { fileDescriptor : { fileDescriptor '4221006E'H, efFileSize '0A50'H -- P -- }

Expected:

ef-arr { fileDescriptor : { fileDescriptor '4221006E'H, efFileSize '0A50'H }

Scenario 2: Current:

pukCodes { { keyReference 1 -- pukAppl1 --, pukValue '303030303030300A'H -- 0000000 --, maxNumOfAttemps-retryNumLeft 153 } }

Expected:

pukCodes { { keyReference 1 -- pukAppl1 --, pukValue '303030303030300A'H, maxNumOfAttemps-retryNumLeft 153 } }

p1-bmu commented 3 years ago

Thanks for your feedback. https://github.com/P1sec/pycrate/commit/3e0b832d4a6331cd797d80659fa149f592cef22e should fix generating invalid comments. You can also simply disable those ASN.1 comments by switching off the _ASN_WASC class attribute for OCTET STRING and / or BIT STRING: https://github.com/P1sec/pycrate/blob/3e0b832d4a6331cd797d80659fa149f592cef22e/pycrate_asn1rt/asnobj_str.py#L102 https://github.com/P1sec/pycrate/blob/3e0b832d4a6331cd797d80659fa149f592cef22e/pycrate_asn1rt/asnobj_str.py#L1233

impratikjaiswal commented 3 years ago

Thanks. This works for me.

impratikjaiswal commented 3 years ago

Now problem is occuring when calling from_asn1() on the generated ASN.

getting additional pair of comments (due to \n) from scan_for_comments.

https://github.com/P1sec/pycrate/blob/3e0b832d4a6331cd797d80659fa149f592cef22e/pycrate_asn1c/utils.py#L479

Sample Data:

pukCodes : {
  puk-Header {
    mandated NULL,
    identification 2
  },
  pukCodes {
    {
      keyReference 1 -- pukAppl1 --,
      pukValue '303030303030300A'H -- '0000000\n' --,
      maxNumOfAttemps-retryNumLeft 153
    }
  }
}
impratikjaiswal commented 3 years ago

However is we pass raw string, then problem is not occuring.

r"""pukCodes : {
  puk-Header {
    mandated NULL,
    identification 2
  },
  pukCodes {
    {
      keyReference 1 -- pukAppl1 --,
      pukValue '303030303030300A'H -- '0000000\n' --,
      maxNumOfAttemps-retryNumLeft 153
    }
  }
}"""
p1-bmu commented 3 years ago

OK, good to know. If I remember correctly, it may be due to the ' character. I'll check the best way how to handle it when I have time. Thanks for reporting.

p1-bmu commented 3 years ago

I hope this one will solve your issue: https://github.com/P1sec/pycrate/commit/4efc57b6750c65789d2d0dadfc0f30f31febf36d Quick question about the ASN.1 schema your are using : is it a public one ? If yes, where can we get it ? Thanks

p1-bmu commented 2 years ago

@impratikjaiswal : any feedback ?? Or I'll close this as is.

impratikjaiswal commented 2 years ago

Hello Benoit,

My issue is solved. Many Thanks.

Quick question about the ASN.1 schema your are using : is it a public one ? If yes, where can we get it ? I am using ASN from TCA. You can get used schema at below path: https://trustedconnectivityalliance.org/wp-content/uploads/2020/01/SIMalliance-eUICC-Profile-Package-Interoperable-Format-Technical-Specification-v2.2-and-ASN-Module.zip

However this might not work, due to multi line comments, so you can test with attached copy (Multi line comments converted to single line sommets & test profile removed.) Please let me know if you need any more information.

Best Regards, Pratik Jaiswal M: +91 (0)8010413967

Please consider the environment before printing this email!

On Mon, Mar 28, 2022 at 1:42 PM Benoit Michau @.***> wrote:

@impratikjaiswal https://github.com/impratikjaiswal : any feedback ?? Or I'll close this as is.

— Reply to this email directly, view it on GitHub https://github.com/P1sec/pycrate/issues/146#issuecomment-1080332838, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADC5ANMAPDV3IWP2K6K2OHDVCFSXFANCNFSM45ZWURGQ . You are receiving this because you were mentioned.Message ID: @.***>

impratikjaiswal commented 2 years ago

Looks like attachment is lost. adding here. PEDefinitions V2.2_wo_MultiLineComments_wo_TestProfile.asn.txt

p1-bmu commented 2 years ago

Thanks for your feedback.

I recently added those specs: https://github.com/P1sec/pycrate/tree/master/pycrate_asn1dir/TCA_eUICCPP_IFTv2 https://github.com/P1sec/pycrate/tree/master/pycrate_asn1dir/TCA_eUICCPP_IFTv3 I think they are the same as yours. They compile fine with recent versions of pycrate.

impratikjaiswal commented 2 years ago

Hello,

Thanks for your update.

I tried with the latest version (0.5.4) of pycrate; I am able to compile original ASNs (with multi line comments) successfully.

Yes. These are similar versions but: 1) https://github.com/P1sec/pycrate /tree/master/pycrate_asn1dir/TCA_eUICCPP_IFTv2 Here probably you added the Unknown Tag knowingly. Line Num: 204 to 209. Should be removed. Also you should delete Test Profile from the ASN: Starting from Line Number 919 upto Line Number 2025.

2) https://github.com/P1sec/pycrate /tree/master/pycrate_asn1dir/TCA_eUICCPP_IFTv3 Here probably you added the Unknown Tag knowingly. Line Num: 217 to 221. Should be removed. Also you should delete Test Profile from the ASN: Starting from Line Number 974 upto Line Number 2150.

However we should remove "Test Profile" as this will generate some unwanted data. In actual test profile is a sample provided by TCA which is developed against provided asn.

Below is the link of latest version: https://trustedconnectivityalliance.org/wp-content/uploads/2021/05/eUICC-Profile-Package-v3.1.zip

Other (old) versions can be found at " https://trustedconnectivityalliance.org/technology-library-sim-specifications/ " under spec "eUICC Profile Package: Interoperable Format Technical Specification"

Feel free to contact me if you need any more clarification regarding these ASNs.

Best Regards, Pratik Jaiswal M: +91 (0)8010413967

Please consider the environment before printing this email!

On Thu, Mar 31, 2022 at 9:27 PM Benoit Michau @.***> wrote:

Thanks for your feedback.

I recently added those specs:

https://github.com/P1sec/pycrate/tree/master/pycrate_asn1dir/TCA_eUICCPP_IFTv2

https://github.com/P1sec/pycrate/tree/master/pycrate_asn1dir/TCA_eUICCPP_IFTv3 I think they are the same as yours. They compile fine with recent versions of pycrate.

— Reply to this email directly, view it on GitHub https://github.com/P1sec/pycrate/issues/146#issuecomment-1084781711, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADC5ANIP343IQYG6QWSYWRTVCXDOZANCNFSM45ZWURGQ . You are receiving this because you modified the open/close state.Message ID: @.***>

impratikjaiswal commented 2 years ago

Thanks for your feedback.

I recently added those specs: https://github.com/P1sec/pycrate/tree/master/pycrate_asn1dir/TCA_eUICCPP_IFTv2 https://github.com/P1sec/pycrate/tree/master/pycrate_asn1dir/TCA_eUICCPP_IFTv3 I think they are the same as yours. They compile fine with recent versions of pycrate.

I think now I knew from where you took these ASNs & why unknown tags are present. actually the ASNs which you added are part of Test Spec named as "eUICC Profile Package: Interoperable Format Test Specification Version" & designed for specific test cases.

My statement can be validated from below: Path is: https://trustedconnectivityalliance.org/wp-content/uploads/2021/09/eUICC-Profile-Package-Interoperable-Format-Test-Specification-Version-3.1.1.zip & further refer to read me at: \eUICC-Profile-Package-Interoperable-Format-Test-Specification-Version-3.1.1.zip\eUICC Interop Profile Test Specification v3.1.1\Specific_ASN1_definitions_v3_1_1.zip\Specific_ASN1_definitions_v3_1_1\readme.txt

Conclusion: Request you to follow my previous comment and add ASN from technical spec only (current asn should be deleted).