Serialization and security

[agronholm]

In APScheduler 3.x, pickle was used as the sole method of serialization. In the APScheduler 4.0 alphas, I've added the options of JSON and CBOR, somewhat mimicking Celery. But I've recently come to question whether this approach is appropriate in APScheduler 4. The reasons for offering different serialization options in Celery boil down to the following (IMHO):

1. Cross-language compatibility (for workers using a language that is not Python)
2. Security (pickle allows execution of arbitrary code by default)

Point 1) does not really apply to APScheduler as it was never meant for cross-language execution. Point 2) still applies, but that can be handled in one of two ways:

1. Restrict what pickle can deserialize
2. Cryptographically sign and check all pickles

Do the alternative serializers make any sense here if we can configure/augment pickle? If so, maybe I should disable the ability of the JSON and CBOR serializers to serialize/deserialize arbitrary objects containing the __getstate__() and __setstate__() methods? Discuss.

[doluk]

I like the option of the JSON serializer for developing. sure, sometimes you can dump the args kwargs somehow in the schedule id to allow checking on what's going on. But especially with more complex input arguments this gets annoying. Especially with a function in different states it is quite useful to have always the same name to prevent any double scheduling of the function, which doesnt work with having an input parameter determining this state. Then it is hand to check on the arguments in a readable form. But beside this usecase, I dont see a way to use it really which wouldn't be possible with pickle.

[agronholm]

What would you consider to be "sensible defaults" for pickle?

[econeale]

That is the eternal question. For pickling, I would explicitly specify a protocol version; version 5 would be fine with me, but if support for Python < 3.8 is necessary version 4 would be fine too as long as it's specific and not just the pickle.DEFAULT_PROTOCOL. I would also set fix_imports=False since we're not using Python 2. I don't think the other options are used if it isn't importing from Python 2 but I could be wrong.

For unpickling, I'm assuming somewhere there's a find_class implementation to avoid super arbitrary code execution as described here. But I'd only bother checking for APScheduler specific names. I wouldn't bother with crypto verification and whatnot. Then I would set fix_imports=False again.

Other than that, I don't think there's anything else to be done other than adding a warning to the documentation to not unpickle untrusted files.

[agronholm]

The minimum supported Python version for APScheduler 4 is 3.8, so protocol level 5 is what we want. The fix_imports option is irrelevant for the current protocols. As for unpickling, are you suggesting restricting the allowed imports to APScheduler code? How would users be able to serialize their own argument values and custom triggers then?

[econeale]

I think it'd have to be pretty liberal check. Maybe something like

import pickle
from apscheduler.abc import Trigger

def find_class(self, module, name):
    if not issubclass(name, Trigger):
        raise pickle.UnpicklingError("Could not unpickle %s" % name)

That'll get pretty cumbersome pretty quick though and I'm not sure how much actual peace of mind it would provide. I'd be okay with not having it, honestly, and just a warning in the documentation to not unpickle untrusted files.

[agronholm]

I think that this is why Celery opted for signed pickles instead, for those with concerns over pickle safety. The other reason why they offer alternative serializers is probably to allow for worker nodes using other programming languages.