Skip to content
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

Open
alexandernst opened this issue Jun 8, 2019 · 32 comments
Open

Awesomeness? #1

alexandernst opened this issue Jun 8, 2019 · 32 comments

Comments

@alexandernst
Copy link
Member

How can this library be so fucking awesome?! Who made it?!

@NachE
Copy link
Member

NachE commented Jun 8, 2019

Hey!, we know each other?

@alexandernst
Copy link
Member Author

alexandernst commented Jun 8, 2019

¯\(ツ)/¯ 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 ?!

@captain-fox
Copy link

captain-fox commented Feb 5, 2020

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:

  1. Is it possible and have you tried to implement pass signing using .pem without providing password using same libraries (or about)? This is somewhat painfull subject for me because for quite a long time I've been trying to achieve this without a need for M2Crypto (which is VERY nasty library and it is a pain in the ass to work with).
  2. This may be the case only for me, but I find storing keys and certs locally on the server is somewhat unnecessary. I'm signing several different pass types with several different certificates, so storing this data in database is more handy. This way you can pull related certs and keys from DB rather then having only one sert of credentials.

@alexandernst
Copy link
Member Author

Hi! All complements to @NachE !

  1. Yes, this should be possible. Check

  2. I don't think I'm understading your question. You're free to store your certs wherever you like. When using the lib, just pull them from where you stored them and pass them as IO objects. You don't need to store them on the FS if you don't want to.

@NachE
Copy link
Member

NachE commented Feb 5, 2020

Hi @captain-fox.

As @alexandernst point out, the signing logic of django-walletpass has been encapsulated in https://github.com/Develatio/django-walletpass/blob/master/django_walletpass/crypto.py and can be used to sign wathever you want (and password=None is allowed). The limitation here is that this code expects a separeted key and cert, so, if you want to sign with .pem files, you have to parse it and separate both parts.

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 django-walletpass depends on apn2 wich depends on hyper to send push notifications:

https://github.com/Develatio/django-walletpass/blob/master/django_walletpass/services.py#L38

@captain-fox
Copy link

captain-fox commented Feb 5, 2020

@NachE it's great that we're on the same page here!

I'm using very simple solution for hyper by creating tempfile directory server-side and instantiating FileSystemStorage for certs inside it:

from django.core.files.storage import FileSystemStorage
...
temp_path = tempfile.mkdtemp(dir='media')
storage = FileSystemStorage(location=temp_path)
...
storage.save(name='cert.pem', content=cert_stored_as_model_field.open())
...

(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.

@NachE
Copy link
Member

NachE commented Feb 5, 2020

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 SingleFileKeyStore reusing current configuration schema in order to avoid breaking backward compatibility, will be an appropriate solution to be merged in a minor version.

@captain-fox
Copy link

captain-fox commented Feb 6, 2020

@NachE I also think KeyStore would be a great improvement.
Have you considered making all models abstract?

...
class Meta:
        abstract = True

This way user of the library would be able to have his/her own models that may contain other fields unrelated to wallet directly and still use all the great features that are already implemented in this package.

@NachE
Copy link
Member

NachE commented Feb 6, 2020

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:

from django_walletpass import models as walletpass_models

class AbstractCustomPass(walletpass_models.Pass):
   class Meta(walletpass_models.Pass.Meta):
       abstract = True

class AbstractCustomRegistration(walletpass_models.Registration):
    class Meta():
        abstract = True

class CustomPass(AbstractCustomPass):
    # my own fields here

class CustomRegistration(AbstractCustomRegistration):
    # my own fields here
    pazz = models.ForeignKey(
        CustomPass,
        on_delete=models.CASCADE,
        related_name='registrations',
    )

Of course if you want to use REST feature yo also need to extends class from classviews.py and point to new models.

Anyway, pull requests are allowed if yo want to send it and your proposed code don't break backward compatibility.

@captain-fox
Copy link

For some weird reason I'm getting [SSL: CA_MD_TOO_WEAK] ca md too weak (_ssl.c:3880) when trying to send push notification to pass. I understand that same cert and key that are used to sign passes are later used to access APNS? Is there something extra that needs to be done with certs to be able to use APNS?

@NachE
Copy link
Member

NachE commented Feb 24, 2020

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.

@captain-fox
Copy link

This is my openssl config:

~ > openssl version -a
OpenSSL 1.1.1d  10 Sep 2019
built on: Sat Sep 28 13:18:07 2019 UTC
platform: darwin64-x86_64-cc
options:  bn(64,64) rc4(16x,int) des(int) idea(int) blowfish(ptr) 
compiler: clang -fPIC -arch x86_64 -O3 -Wall -DL_ENDIAN -DOPENSSL_PIC -DOPENSSL_CPUID_OBJ -DOPENSSL_IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_MONT5 -DOPENSSL_BN_ASM_GF2m -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DKECCAK1600_ASM -DRC4_ASM -DMD5_ASM -DVPAES_ASM -DGHASH_ASM -DECP_NISTZ256_ASM -DX25519_ASM -DPOLY1305_ASM -D_REENTRANT -DNDEBUG
OPENSSLDIR: "/usr/local/etc/openssl@1.1"
ENGINESDIR: "/usr/local/Cellar/openssl@1.1/1.1.1d/lib/engines-1.1"
Seeding source: os-specific

And I'm running those two commands to generate .pem files:

  1. openssl pkcs12 -in "key.p12" -clcerts -nokeys -out certificate.pem
  2. openssl pkcs12 -in "key.p12" -nocerts -out key.pem

With this done I'm still getting [SSL: CA_MD_TOO_WEAK] error.

@alexandernst
Copy link
Member Author

Can you try adding -macalg SHA1 to both commands?

@captain-fox
Copy link

I've re-generated certificate from ground up and ran:

  1. openssl pkcs12 -in "Certificates.p12" -clcerts -macalg SHA1 -nokeys -out certificate.pem
  2. openssl pkcs12 -in "Certificates.p12" -macalg SHA1 -nocerts -out key.pem

Result is the same: [SSL: CA_MD_TOO_WEAK] ca md too weak (_ssl.c:3880)

@alexandernst
Copy link
Member Author

Can you check your openssl*.cnf file , under the [CA_default] section there should be a default_md option. Can you check if it's value is sha256?

@captain-fox
Copy link

captain-fox commented Feb 25, 2020

@alexandernst It just says default

@alexandernst
Copy link
Member Author

Can you change it to sha256?

@captain-fox
Copy link

Changed it, re-generated keys – problem remains

@alexandernst
Copy link
Member Author

What distro is this? Can you try on another machine with Debian 9 or 10?

@captain-fox
Copy link

I'm running this in Docker using FROM python:3.7.6 image

@JamesKey-iq
Copy link

For some weird reason I'm getting [SSL: CA_MD_TOO_WEAK] ca md too weak (_ssl.c:3880) when trying to send push notification to pass. I understand that same cert and key that are used to sign passes are later used to access APNS? Is there something extra that needs to be done with certs to be able to use APNS?

did you manage to fix this?

@alexandernst
Copy link
Member Author

We found a way to repro this, stay tuned for a possible fix.

@captain-fox
Copy link

For some weird reason I'm getting [SSL: CA_MD_TOO_WEAK] ca md too weak (_ssl.c:3880) when trying to send push notification to pass. I understand that same cert and key that are used to sign passes are later used to access APNS? Is there something extra that needs to be done with certs to be able to use APNS?

did you manage to fix this?

@alexandernst

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:
Pr0Ger/PyAPNs2#103 (comment)

@NachE
Copy link
Member

NachE commented Mar 25, 2020

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.

@captain-fox
Copy link

Te 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 xyz.p8 certificate is yet another cert that needs to be generated, this time specifically for APNS, to allow JWT auth?

@NachE
Copy link
Member

NachE commented Mar 25, 2020

As I understand xyz.p8 certificate is yet another cert that needs to be generated, this time specifically for APNS, to allow JWT auth?

You are right. The CA_MD_TOO_WEAK comes from the new openssl version wich indroduces a security cert check (#1 (comment)). Implementing JWT auth for push notifications is our best aproach to keep this app working.

@captain-fox
Copy link

captain-fox commented Mar 26, 2020

@NachE

Isn't JWT auth meant to serve as APNS communication for an actual app rather than means of sending pass updates? Apple explicitly says that PassTypeID certificate serves to sign and update passes:

Screenshot 2020-03-26 at 10 24 05

To create APNS cert, one also needs to create App ID, and this is comparable to hammering a nail with a wrench. Unless I was confused about type of file one needs to generate and it's actually not a certificate, but a key:

Screenshot 2020-03-26 at 10 57 14

I do not know how OpenSSL works, apart from the fact that there are always problems with it, but is there a way to comply with this new security checks to do things the right way, rather than using JWT as a workaround?

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.

@NachE
Copy link
Member

NachE commented Mar 26, 2020

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 /etc/ssl/openssl.cnf

from:

CipherString = DEFAULT@SECLEVEL=2

to:

CipherString = DEFAULT@SECLEVEL=1

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.

@captain-fox
Copy link

captain-fox commented Mar 26, 2020

@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:

  1. Create a certificate beginning with a request to certifying authority
  2. Uploade request file to apple developer
  3. Import obtained certificate into KeyChain
  4. Export certificate as .p12 file
  5. Convert p12 file into key and cert files with .pem extension

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:

Screenshot 2020-03-26 at 12 59 39

Screenshot 2020-03-26 at 12 52 51

It this the step which can potentially make the main issue go away?

@NachE
Copy link
Member

NachE commented Apr 3, 2020

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:

  • RSA and DHE keys need to be at least 2048 bit long
  • SHA-1 is no longer supported for signatures in certificates and you need at least SHA-256
  • CAs have stopped issuing certificates that didn't meet those requirements in January 2015
  • Since January 2017 all valid CA certificates should meet those requirements
  • There are certificates generated by private CAs or that are in a test suite that do not meet those requirements

This can be disabled with CipherString = DEFAULT@SECLEVEL=1 into /etc/ssl/openssl.cnf as described abobe.

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

If you send push notifications with the legacy binary protocol, we recommend updating to the HTTP/2-based APNs provider API as soon as possible. You’ll be able to take advantage of great modern features, such as authentication with a JSON Web Token, improved error messaging, and per-notification feedback.
The Apple Push Notification service (APNs) will no longer support the legacy binary protocol as of November 2020.

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 django-walletpass

@captain-fox
Copy link

@NachE many thanks for this update, the explanations in documentation for openssl 1.1.1 makes everything clear and resolced; I wasn't aware that apple has made statement about JWT – then JWT it is from now on.

@captain-fox
Copy link

captain-fox commented May 11, 2020

Hi, guys!
@NachE @alexandernst
I've created this over the weekend:
https://github.com/captain-fox/airpress
Maybe something you'd be willing to use for this project as a library for pkpass compression?
The description isn't very long (yet), so please checkout the source from repo to see all the features I've added.

I'd be glad to hear your feedback.

P.S. I've copied implementation of crypto from your project as there's really not many ways to implement pkcs7 signature and you've had it nice and clean.

Cheers!

alexandernst pushed a commit that referenced this issue May 16, 2023
Merge pull request #12 from patroqueeet/master
alexandernst pushed a commit that referenced this issue Jan 23, 2024
Create event loop, if code is executing in non-main thread.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants