asn1crypto
asn1crypto copied to clipboard
Feat/0039 rfc5126: support for CADES-BES/EPES with long term support
Basic tests are included, at least for signature parsing. Still no tests for building new structures.
The Circleci job failed, but it seems unrelated to the commits.
Do we need still python 2.6 compat? I can remove dict comprehension of the tests.
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.
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 [email protected]:
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 https://github.com/wbond/asn1crypto/pull/88#issuecomment-378908447, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEtcB_zQhhPlAoamaAbXpkxbGxPVaQpks5tlgOSgaJpZM4TH4k6 .
Codecov Report
:exclamation: No coverage uploaded for pull request base (
master@291cd95). Click here to learn what that means. The diff coverage is100%.
@@ 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 dataPowered by Codecov. Last update 291cd95...7f9cc31. Read the comment docs.
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.
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 [email protected]:
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 https://github.com/wbond/asn1crypto/pull/88#issuecomment-379025856, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEtcOm0GqIRJii8g4e2NnLUfa11_PJqks5tllz8gaJpZM4TH4k6 .
Ok. travis finally passed (after some minor commits).
Any news on this?
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
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.
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.
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
... 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.
... 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
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.
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.
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.
all test fail
https://github.com/wbond/badtls.io/issues/6
$ 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
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.
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" [email protected] 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 https://github.com/wbond/asn1crypto/pull/88?email_source=notifications&email_token=AAAS24CZFO2EK23P4LKOGWTQCRERLA5CNFSM4EY7RE5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3OAMHQ#issuecomment-517735966, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAS24D2QOXQGXVY2NA3RFLQCRERLANCNFSM4EY7RE5A .