On Fri, Jan 13, 2017 at 1:09 AM, Cory Benfield <c...@lukasa.co.uk> wrote:
> On 13 Jan 2017, at 09:07, Nathaniel Smith <n...@pobox.com> wrote:
> I was imagining something along the lines of
> ClientContext({...}).wrap_socket(...)
> where ClientContext.__init__ would do the validation. Or there are
> various other ways to slot the pieces together, but in general
> validation doesn't need to be done incrementally; there's going to be
> some place where the config dict hits the library and it can/should be
> validated at that point. And before that point we get all the dict
> niceties for free.
> What are the dict niceties, exactly? I’m still not seeing what the advantage
> is in throwing away a typed and restricted API for a dict is.

Getters & setters (right now the spec has setters for everything, but
a getter for validate_certificates only -- that's kinda weird?), an
obvious serialization story for the parts of the config that use basic
types (e.g. to/from JSON/TOML/YAML), an obvious/natural way to
represent "here are the config changes I want to make in response to
that ClientHello" for the sni callback as a declarative object, a nice
__repr__, familiarity, ... obviously these are all things one can put
into the spec piece by piece, but you're the one who said you wanted
to simplify things :-).

I get what you're saying about a typed API -- basically in the current
setup, the way _BaseContext is written as a bunch of Python methods
means that the interpreter will automatically catch if someone tries
to call set_cihpers, whereas in the dict version each implementation
would have to catch this itself. But in practice this difference seems
really minimal to me -- either way it's a runtime check, and it's
really difficult to write a loop like

  for key, value in config.items():

that doesn't implicitly validate the keys anyway. There's also the
option of using e.g. JSON-Schema to write down a formal description of
what goes in the dict -- this could actually be substantially stricter
than what Python gets you, because you can actually statically enforce
that validate_certificates is a bool. For whatever that's worth --
probably not much.

If we really want to simplify it down, then as Alex noted in the PR,
TLSWrappedSockets are superfluous, so we could make it just:

# Simple functions, not methods:
client_wrap_buffers(config, incoming, outgoing) -> TLSWrappedBuffers
server_wrap_buffers(config, incoming, outgoing) -> TLSWrappedBuffers

class TLSWrappedBuffers:
    ... read and all that stuff ...
    def get_config(self) -> dict:

    def update_config(self, new_settings):
        # Attempt to do the equivalent of self.config.update(new_settings);
        # raises TLSError if any of the settings can't be changed given
        # current backend or current connection state.

Anyway, I'm not actually sure if this is a good idea, but it seemed
worth putting the two approaches up where we could look at them and
compare :-).


Nathaniel J. Smith -- https://vorpus.org
Security-SIG mailing list

Reply via email to