-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Awesomeness? #1
Comments
Hey!, we know each other? |
¯\(ツ)/¯ how could I know such a magnificent person that is able to create a Django package that is able to sign Apple related stuff with PKCS7 ?! |
Since this is (sort-of) open conversation issue I'd like to give my complements to creators/maintainers, but also I've dug through source code of this library and I have couple of questions:
|
Hi! All complements to @NachE !
|
Hi @captain-fox. As @alexandernst point out, the signing logic of The problem of using certs from DB comes from hyper library that needs a file to handle tls: https://github.com/python-hyper/hyper/blob/master/hyper/tls.py#L71 and https://github.com/Develatio/django-walletpass/blob/master/django_walletpass/services.py#L38 |
@NachE it's great that we're on the same page here! I'm using very simple solution for
(Obviously this is ovesimplified implementation) So this method would be more convenient accept certs, key, data and password as agruments rather than reference them from within itself. |
Implementing a KeyStore sounds good to me. A convenient approach could be using a "configurable backend strategy" for getting certs. Also, keeping a default backend |
@NachE I also think KeyStore would be a great improvement.
This way user of the library would be able to have his/her own models that may contain other fields unrelated to |
Making all models abstract will break the purpose of this package (a working solution). If you need to implement your own fields to models, you can do it extending models: example:
Of course if you want to use REST feature yo also need to extends class from Anyway, pull requests are allowed if yo want to send it and your proposed code don't break backward compatibility. |
For some weird reason I'm getting |
Same certs for sign are used to access APNs, so no extra work is needed. This error appears to be related to security checks performed by openssl lib (https://github.com/openssl/openssl/blob/master/ssl/t1_lib.c#L2635). Maybe your certs are enough strong for openssl 1.0.x but too weak for openssl 1.1.x. Review your openssl version and/or regenerate your certs using openssl 1.1.x. |
This is my
And I'm running those two commands to generate
With this done I'm still getting |
Can you try adding |
I've re-generated certificate from ground up and ran:
Result is the same: |
Can you check your openssl*.cnf file , under the |
@alexandernst It just says |
Can you change it to |
Changed it, re-generated keys – problem remains |
What distro is this? Can you try on another machine with Debian 9 or 10? |
I'm running this in Docker using |
did you manage to fix this? |
We found a way to repro this, stay tuned for a possible fix. |
Several people faced the same issue in a bunch of apns-related projects such as django-push-notifications issue 532 But all narrows down to solution that has to be either explaned or fixed in scope of pyapns2 implementation. I'd suggest we continue investigation under an issue in APNS repo: |
The recently uploaded 1.2 version of django-walletpass implements JWT auth for push notifications. You could use this new feature to bypass this problem. |
As I understand |
You are right. The |
Isn't To create I do not know how If I knew how to address this issue myself, I'd gladly do it and share the solution with everyone, but SSL is not something you can learn and understand in a matter of minutes, so I can only help test and try out proposed solutions. |
The problem with the certs probably comes from the base cert that has been used to generate key and cert. If you regenerate your base cert (with more strong settings) and regenerate key/cert with it, this problem should be fixed. You can confirm this reducing security check level in from:
to:
The introduction of JWT for push is a configurable option and you can still use legacy (default) mode. For push notifications (JWT) you need to currently generate a .p8 until we get more time to merge the code and use just one for both. |
@NachE That is perfectly understandable. The question that arises is what is the meaning of "stronger settings"? I'm following the same steps as everyone else:
There's only one step in entire process that allows some configuration related to stregth of certificate and it is available during generation of request to certifying authority: It this the step which can potentially make the main issue go away? |
You can get more information about cert sec limitation introduced in openssl 1.1.1 with debian installation here: https://wiki.debian.org/ContinuousIntegration/TriagingTips/openssl-1.1.1 wich bring some clarifications:
This can be disabled with Looking at the code that sends push notifications (https://github.com/Develatio/django-walletpass/blob/master/django_walletpass/services.py), the certs used to do this work are your two cert and key files. If your two files are strong enough, and this issue persist, we need to dig more deeply into this issue. Besides, apple has published an update note: https://developer.apple.com/news/?id=11042019a
As you can see, binary protocol will stop working on November 2020, so implementation of JWT seems to be the right path to follow in order to keep a good health of |
@NachE many thanks for this update, the explanations in documentation for |
Hi, guys! I'd be glad to hear your feedback. P.S. I've copied implementation of Cheers! |
Merge pull request #12 from patroqueeet/master
Create event loop, if code is executing in non-main thread.
How can this library be so fucking awesome?! Who made it?!
The text was updated successfully, but these errors were encountered: