Feat/0039 rfc5126: support for CADES-BES/EPES with long term support #88

erny · 2018-04-05T05:39:19Z

Basic tests are included, at least for signature parsing. Still no tests for building new structures.

erny · 2018-04-05T05:45:09Z

The Circleci job failed, but it seems unrelated to the commits.

erny · 2018-04-05T09:46:06Z

Do we need still python 2.6 compat? I can remove dict comprehension of the tests.

wbond · 2018-04-05T11:44:18Z

Yes, Python 2.6 compatibility is important, because this library is used in Package Control, which still supports Sublime Text 2, which runs on Python 2.6. https://travis-ci.org/wbond/asn1crypto/jobs/362466545

Also, all strings are unicode by default and should not use the u prefix since it doesn't work with Python 3.2: https://travis-ci.org/wbond/asn1crypto/jobs/362466547.

The L suffix on integers is not supported in Python 3.3: https://travis-ci.org/wbond/asn1crypto/jobs/362466548.

If you can rebase on top of master, that will help get CircleCI running properly against the latest PyPi SSL changes. However, while my setup worked in a branch, it seems to be funky right now, so maybe hold off on the rebase until master is green.

erny · 2018-04-05T11:51:05Z

Ok, thanks a lot. I'll revisit my changes to assure I'm not using Python 2.7 syntax, and get rid of any "unicode" string constants. Regards. Ernesto Revilla Área Técnica TangramBPM.es Tlf: 630 244 136 2018-04-05 13:44 GMT+02:00 Will Bond <notifications@github.com>:

…

Yes, Python 2.6 compatibility is important, because this library is used in Package Control, which still supports Sublime Text 2, which runs on Python 2.6. https://travis-ci.org/wbond/asn1crypto/jobs/362466545 Also, all strings are unicode by default and should not use the u prefix since it doesn't work with Python 3.2: https://travis-ci.org/wbond/ asn1crypto/jobs/362466547. The L suffix on integers is not supported in Python 3.3: https://travis-ci.org/wbond/asn1crypto/jobs/362466548. If you can rebase on top of master, that will help get CircleCI running properly against the latest PyPi SSL changes. However, while my setup worked in a branch, it seems to be funky right now, so maybe hold off on the rebase until master is green. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#88 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEtcB_zQhhPlAoamaAbXpkxbGxPVaQpks5tlgOSgaJpZM4TH4k6> .

codecov · 2018-04-05T12:21:29Z

Codecov Report

❗ No coverage uploaded for pull request base (master@291cd95). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master      #88   +/-   ##
=========================================
  Coverage          ?   85.48%           
=========================================
  Files             ?       26           
  Lines             ?     5541           
  Branches          ?        0           
=========================================
  Hits              ?     4737           
  Misses            ?      804           
  Partials          ?        0

Impacted Files	Coverage Δ
asn1crypto/cades.py	`100% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 291cd95...7f9cc31. Read the comment docs.

wbond · 2018-04-05T18:05:46Z

Gah, the tests are breaking because PyPi is doing brownouts of TLS 1.0 and 1.1 support, but the stable pip is broken on Mac with Python 2.6. I have a PR in to try and fix the underlying issue, but it may take some time before that is worked out.

erny · 2018-04-05T21:23:33Z

Ok, I corrected the tests. Now it should work with python 2.6 and 3.x. 2018-04-05 20:05 GMT+02:00 Will Bond <notifications@github.com>:

…

Gah, the tests are breaking because PyPi is doing brownouts of TLS 1.0 and 1.1 support, but the stable pip is broken on Mac with Python 2.6. I have a PR in to try and fix the underlying issue, but it may take some time before that is worked out. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#88 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEtcOm0GqIRJii8g4e2NnLUfa11_PJqks5tllz8gaJpZM4TH4k6> .

erny · 2018-04-05T21:57:13Z

Ok. travis finally passed (after some minor commits).

erny · 2018-05-11T17:43:33Z

Any news on this?

sherpya · 2019-01-24T23:11:42Z

Hi, since 2019 Italy needs cades xml files for invoices with public administrations entities, this would be very useful, I've tested it and I'm able to parse .xml.p7m files as needed, this pr can be rebased to master without problems, and test are ok

erny · 2019-01-25T10:36:14Z

Hi, since 2019 Italy needs cades xml files for invoices with public administrations entities, this would be very useful, I've tested it and I'm able to parse .xml.p7m files as needed, this pr can be rebased to master without problems, and test are ok

Hey, thanks! Cool that you could do that.

I wrote the CAdES long term support initially to be able to parse EPES, some new signature attributes, and all the long term attributes that I receive from spanish signature validation / upgrade infrastructure.

The CAdES and all the ETSI other signature formats (PAdES, XAdES) are standards mostly used the European Union (due to EU directives). But asn1cryto was designed initially for use in Sublime Text editor and it may be a bit overkill to include the CAdES support in the core. It would be nice to have a plugin support, so you could do something like "pip install asn1crypto[cades]", a upstream compatible fork (eg. asn1crypto-cades?) or something similar.

I hope that Will Bond could give us any ideas on how to proceed. Best regards.

joernheissler · 2019-01-25T10:45:49Z

In my opinion, there's no reason not to include CAdES etc. in asn1crypto. But I'm not familiar with those standards (although I live in the EU) and couldn't find the time to take a close look at this PR.

35 commits is a lot (even though only 10 changed filed). And to review this, one would also need to read+understand the RFC(s). There might be some important subtles.

I don't see the need for a plugin system. CAdES (and other public asn.1 types) should be in the core. And it's always possible to create another python package that requires asn1crypto and only defines the new asn.1 types. I do exactly that at my day job where I invented some proprietary asn.1 types.

sherpya · 2019-01-25T15:02:52Z

35 commits is a lot (even though only 10 changed filed). And to review this, one would also need to read+understand the RFC(s). There might be some important subtles.

I think it can be squashed in a better way

erny · 2019-01-25T15:39:49Z

...
I don't see the need for a plugin system. CAdES (and other public asn.1 types) should be in the core. And it's always possible to create another python package that requires asn1crypto and only defines the new asn.1 types. I do exactly that at my day job where I invented some proprietary asn.1 types.

Could you provide a concrete example? How could be proceed? I would be willing to move all this stuff into another package. Thx.

erny · 2019-01-25T15:57:46Z

...
35 commits is a lot (even though only 10 changed filed). And to review this, one would also need to read+understand the RFC(s). There might be some important subtles.
...

As could be seen in the "files" section, only one file is changed (tests/init.py) to allow running new tests, only one core file is added (cades.py). All other files are for testing or documentation. IMHO, on the other hand, any tests should be done on real world examples, as done here, because the spec is really complex. Perhaps, this should not be seen as 100% error free (sure it isn't), but if it provides value for those people who need CAdES support, why not release it and include a "beta quality cades long term signature support" comment?

Regards

wbond · 2019-01-25T16:11:50Z

Here are the reasons I haven't reviewed/merged yet:

asn1crypto deals with cryptography and has my name plastered all over it, hence I don't want to just throw stuff in. Granted, it has had plenty of bugs and will continue to for a while, but I am more apprehensive with this project than something like pybars3.
This is a sizable change. I'm not familiar with CADES, and this is big first changes for someone who hasn't made a lot of changes to asn1crypto.
I had my fifth child earlier this year, and barely have time for open source these days. I have so many projects that need attention that I mostly spot a hour or two here to there to prevent things from blowing up or disintegrating.

When I started this project I was self-employed and focused on something in the security space, but now I work on Sublime Text, so this is sort of a side-project. @joernheissler has shown a good handle on ASN.1 in general and making a bunch of contributions so I've added him as a collaborator and he's been the one keeping things from becoming frozen in time. I'd love to have more people involved, but I'm the sort of person who likes contributors to ease into a project one small bit at a time and then I give them access to contribute. Afterward I typically check in on changes to make sure things look reasonable. This is probably means my projects won't ever reach their full potential, but I'd rather not get into a situation like event-stream in npm.

In summary, I apologize that this hasn't made it in. I wish I had the time to go through and review it and get it merged in. I'm sure lots of people would find it beneficial. Perhaps someday I'll find a way to take useful projects like these and turn them into an reliable income source providing enhancements and support to companies using them, but for now they are just "best effort" as it fits into the rest of my life.

erny · 2019-01-25T20:57:59Z

Thanks Will for your response. Everything sounds very reasonable. Congrats for your new kid. Your suggestions to make the cades support external sounds also reasonable to me. I'm just thinking about the best way to do this. Jörn pointed me already into this direction.

Today I did a new merge from master, and now all test fail (still don't know why).
Best wishes for 2019.
Regards.

joernheissler · 2019-01-25T21:15:42Z

Congrats, @wbond! And btw, thanks for adding me as collaborator.
I wish I'd find more time, too. E.g. I still didn't complete #86, maybe I should split it up a bit and describe the trouble I'm facing.

@erny, I think that cades support SHOULD NOT be made external. It should be in the asn1crypto package. I merely said that it's technically possible.

I'll try to review your PR in a few days.

joernheissler · 2019-01-25T21:27:32Z

all test fail

wbond/badtls.io#6

erny · 2019-01-26T00:01:21Z

$ openssl s_client -connect dh1024.badtls.io:10005 -servername dh1024.badtls.io -showcerts </dev/null 2>/dev/null | openssl x509 -noout -enddate 

notAfter=Jan  1 00:00:00 2019 GMT

joernheissler · 2019-02-09T11:20:16Z

asn1crypto/cades.py

+# coding: utf-8
+
+"""
+ASN.1 type classes for cryptographic message syntax (CADES, RFC 5126 and RFC 3126). Structures are also


Links to those standards would be nice

joernheissler · 2019-02-09T11:22:04Z

asn1crypto/cades.py

+
+All camelCase key names have been converterd to under_score_notation
+
+-----------------


No need to have a copy of the ASN.1 definitions here.

joernheissler · 2019-02-09T11:24:46Z

asn1crypto/cades.py

+
+
+# fake usage
+ContentInfo


Purpose of this?

joernheissler · 2019-02-09T11:26:39Z

asn1crypto/cades.py

+ASN.1 type classes for cryptographic message syntax (CADES, RFC 5126 and RFC 3126). Structures are also
+compatible with PKCS#7. Exports the following items:
+
+ - AuthenticatedData()


copy+paste from cms.py?

The former commit included ASN.1 as of wbond#39 comment: wbond#39 (comment) but X.208 ASN.1 Syntax given in Annex A.1. is easier to translate and has a minor change (sigQualifier vs qualifier SigPolicyQualifierInfo)

See wbond#39.

See wbond#39

This is a implicit signature.

content-{reference,identifier,hints}

Replace {k: v for ...} syntax with dict((k, v) for ...) for python 2.6 compat.

* Replace assertIn with assertTrue(x in y) * Remove L long constant suffix.

wbond · 2019-08-02T15:07:01Z

I just rebased onto master to hopefully get all of the CI going through cleanly.

I like all of the tests that there are. I think we can probable de-dupe some of the structures with x509 and as long as there aren't tagging conflicts use things like x509.DisplayText instead of cades.DisplayText.

I'd also rather reference RFC URLs than copy ANS.1 definitions wholesale into the source code, for consistency with other parts of the library.

I'm hoping sometime in the next month or so to go through this with a fine tooth comb and clean it up a bit for consistency. It doesn't seem like there are all that many additions in cades.py, and there are a bunch of tests which is helpful.

Thanks for all of your work on this @erny.

erny · 2019-08-02T15:44:30Z

Thanks a lot for your great work! I hadn't time ultimately to work on this. It may be a bit out of scope as this RFC is quite complex and long-term signature verification and storage may be not needed everywhere. I wish I had more time to work on this. Thanks again. Best regards. El 2 ago. 2019 5:07 p. m., "Will Bond" <notifications@github.com> escribió: I just rebased onto master to hopefully get all of the CI going through cleanly. I like all of the tests that there are. I think we can probable de-dupe some of the structures with x509 and as long as there aren't tagging conflicts use things like x509.DisplayText instead of cades.DisplayText. I'd also rather reference RFC URLs than copy ANS.1 definitions wholesale into the source code, for consistency with other parts of the library. I'm hoping sometime in the next month or so to go through this with a fine tooth comb and clean it up a bit for consistency. It doesn't seem like there are all that many additions in cades.py, and there are a bunch of tests which is helpful. Thanks for all of your work on this @erny <https://github.com/erny>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#88?email_source=notifications&email_token=AAAS24CZFO2EK23P4LKOGWTQCRERLA5CNFSM4EY7RE5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3OAMHQ#issuecomment-517735966>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAS24D2QOXQGXVY2NA3RFLQCRERLANCNFSM4EY7RE5A> .

joernheissler reviewed Feb 9, 2019

View reviewed changes

asn1crypto/cades.py

# fake usage

ContentInfo

Copy link

Collaborator

joernheissler Feb 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Purpose of this?

joernheissler reviewed Feb 9, 2019

View reviewed changes

erny added 6 commits August 2, 2019 10:59

1st commit for CADES RFC 5126. See wbond#39.

6cb719d

See wbond#39. Replace ASN.1 with Annex A.1.

53cd8bd

The former commit included ASN.1 as of wbond#39 comment: wbond#39 (comment) but X.208 ASN.1 Syntax given in Annex A.1. is easier to translate and has a minor change (sigQualifier vs qualifier SigPolicyQualifierInfo)

Fix docs ASN.1 spec. See wbond#39.

9a6c52e

See wbond#39. Fix some CMS attribute definitions.

33d54ae

Fix SetOfSignaturePolicy. See wbond#39.

6756651

See wbond#39. Add commitment_type CMS Attribute.

15deb18

erny added 27 commits August 2, 2019 10:59

signer-attributes attribute. See wbond#39.

a49b423

complete-certificate-references attr. See wbond#39.

3a9b9dc

complete-revocation-references attr. See wbond#39.

1161544

Make attribute names more uniform. See wbond#39.

8e0c6f5

Refactor for reuse. See wbond#39.

96965e4

Add rest of attributes. See wbond#39.

083563a

Add missing explicit options. See wbond#39.

0403794

Convert all key names to under_score_notation.

297233e

See wbond#39.

Some test data. See wbond#39.

48a53b9

make TODO of attributes to check / implement.

59c9d6f

See wbond#39

Import ContentInfo for direct use. See wbond#39.

c88fe38

Remove objects not defined from comment. See wbond#39.

272af31

Consider old timestamp attribute. See wbond#39.

e9e0989

Basic parse test for CADES BES. See wbond#39.

866e907

Remove ipdb ;-)

86c158c

Check signed content. See wbond#39.

61f5e65

This is a implicit signature.

Implement additional ESS attributes. See wbond#39.

2ab1bf8

content-{reference,identifier,hints}

Some additional (cert, signing_time). See wbond#39.

cb95a1e

Add tests for EPES and fix some attribute bugs. See wbond#39.

39f9937

Add tests for CADES-EPES-A-Explicit. See wbond#39.

e7d1e33

Test files for CADES EPES signatures. See wbond#39.

8b51337

Replace test file. See wbond#39.

5b74151

Fix strings for python3 in tests. See wbond#39.

b5c4d11

Python 2.6 compat in tests. See wbond#39.

919b170

Replace {k: v for ...} syntax with dict((k, v) for ...) for python 2.6 compat.

Python 3.3 and 2.6 in tests. See wbond#39.

3bf7290

* Replace assertIn with assertTrue(x in y) * Remove L long constant suffix.

Reduce line length for ci tests. See wbond#39.

64d4662

Python 3.2 compat of binary string. See wbond#39.

2734539

wbond force-pushed the feat/0039_RFC5126 branch from 7f9cc31 to 2734539 Compare August 2, 2019 15:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/0039 rfc5126: support for CADES-BES/EPES with long term support #88

Feat/0039 rfc5126: support for CADES-BES/EPES with long term support #88

erny commented Apr 5, 2018

erny commented Apr 5, 2018

erny commented Apr 5, 2018

wbond commented Apr 5, 2018

erny commented Apr 5, 2018 via email

codecov bot commented Apr 5, 2018 •

edited

Loading

wbond commented Apr 5, 2018

erny commented Apr 5, 2018 via email •

edited

Loading

erny commented Apr 5, 2018

erny commented May 11, 2018

sherpya commented Jan 24, 2019

erny commented Jan 25, 2019

joernheissler commented Jan 25, 2019

sherpya commented Jan 25, 2019

erny commented Jan 25, 2019

erny commented Jan 25, 2019 •

edited

Loading

wbond commented Jan 25, 2019

erny commented Jan 25, 2019

joernheissler commented Jan 25, 2019

joernheissler commented Jan 25, 2019

erny commented Jan 26, 2019

joernheissler Feb 9, 2019

joernheissler Feb 9, 2019

joernheissler Feb 9, 2019

joernheissler Feb 9, 2019

wbond commented Aug 2, 2019

erny commented Aug 2, 2019 via email


		All camelCase key names have been converterd to under_score_notation

		-----------------

Feat/0039 rfc5126: support for CADES-BES/EPES with long term support #88

Are you sure you want to change the base?

Feat/0039 rfc5126: support for CADES-BES/EPES with long term support #88

Conversation

erny commented Apr 5, 2018

erny commented Apr 5, 2018

erny commented Apr 5, 2018

wbond commented Apr 5, 2018

erny commented Apr 5, 2018 via email

codecov bot commented Apr 5, 2018 • edited Loading

Codecov Report

wbond commented Apr 5, 2018

erny commented Apr 5, 2018 via email • edited Loading

erny commented Apr 5, 2018

erny commented May 11, 2018

sherpya commented Jan 24, 2019

erny commented Jan 25, 2019

joernheissler commented Jan 25, 2019

sherpya commented Jan 25, 2019

erny commented Jan 25, 2019

erny commented Jan 25, 2019 • edited Loading

wbond commented Jan 25, 2019

erny commented Jan 25, 2019

joernheissler commented Jan 25, 2019

joernheissler commented Jan 25, 2019

erny commented Jan 26, 2019

joernheissler Feb 9, 2019

Choose a reason for hiding this comment

joernheissler Feb 9, 2019

Choose a reason for hiding this comment

joernheissler Feb 9, 2019

Choose a reason for hiding this comment

joernheissler Feb 9, 2019

Choose a reason for hiding this comment

wbond commented Aug 2, 2019

erny commented Aug 2, 2019 via email

codecov bot commented Apr 5, 2018 •

edited

Loading

erny commented Apr 5, 2018 via email •

edited

Loading

erny commented Jan 25, 2019 •

edited

Loading