search

otr4j / otr4j-issues

1 stars 0 forks source link

Strict state separation #11

Closed cobratbq closed 4 years ago

cobratbq commented 9 years ago

Currently, states are distinguished based solely on the value of certain variables, such as sessionStatus in Session.java. Although this works, it is still possible to make a mistake in an implementation in which you do not correctly check all the necessary variables to perfectly distinguish one state from another. This leaves a chance that implementations read or write variables at inappropriate times and as a consequence may leak data or corrupt an otr session unintendedly.

There is no reason to believe that this is the case currently, however it would be better to leverage the Java type system to more strictly separate the states. If we separate states in such as way that we can throw exceptions whenever a method is called at an inappropriate time, then we move the risk from the user to the developer, as it would trigger an exception instead of continuing in an inconsistent state. A protocol such as OTR, whose spec clearly describes the expectations in each state, is especially suitable to be structured like this.

As described in the OTR spec, there are 2 state machines (at least):

  1. The OTR session (messaging): plaintext, encrypted, finished.
  2. The Authentication state: none, waiting-dhkey, awaiting-revealsig, awaiting-sig (with a special state for v1)

Of course there significantly changes how the implementation is structured, and increases the chance of Exceptions for imperfect implementations. As such, we should look into this after the initial burst of changes.

languitar commented 9 years ago

This really calls for the state design pattern: https://en.wikipedia.org/wiki/State_pattern

eighthave commented 9 years ago

This sounds very valuable. otr4j has proven itself valuable, so I think this is exactly the kind of stuff we need to be thinking about so we can make otr4j provide rock solid private messaging (hence all my work on tests, also).

cobratbq commented 9 years ago

@languitar yes it does. That was exactly what I was thinking of.

@eighthave My thoughts exactly. Also, I'm glad I took the effort to write tests for the Fragmenter, initially.

cobratbq commented 5 years ago

Done.