wps_enrollee cleaned up code

[Jouni Malinen]

On Mon, Nov 12, 2007 at 01:47:56PM -0800, Ted Merrill wrote:

> Logically, the new wps_enrollee sits above and commands wpa_supplicant, which
> continues to run... see below.
> It also however makes direct l2_packet calls.
> It could be argued that wps_enrollee should make use of the EAP code used 
> elsewhere, but i think this would only serve to confuse.
> Although WPS >uses< the EAP mechanism, WPS is not actually an authentication 
> protocol at all... using EAP is basically just a hack.

I think I will be arguing that this could will need to use the same
EAPOL and EAP implementation as all other EAP methods. I do not see much
point in duplicating this functionality just for WPS. A design that
separates WPS Registration Protocol from EAP and shares the already
existing EAPOL/EAP implementation looks much cleaner. I've added EAP-WSC
implementation (just the EAP specific functionality) into my Git tree
for both the EAP server and peer.

Since the Registration Protocol can be used over multiple transport
mechanisms, it would benefit of clean interface that passed M1..M8, ACK,
NACK, Done to WPS processing from wherever they came from (EAP, UPnP,
etc.). In other words, the main WPS code should not need to worry about
EAP details like fragmentation and reassembly or EAP header contents.
Nor should it have to even know anything about l2_packet. Please also
note that EAPOL frames can be received through the driver interface to
better support some platforms. Direct calls to l2_packet would mean this
option is not available (i.e., reduced OS/network stack support).

> -- wps_crypto -- key generation, authentication generation and checking.
> The state data consists only of key data.  Relies on the same underlying
> crypto routines as wpa_supplicant does (including can use the compact
> internal routines).  Some of this will be useful for wps server.

Some of this should really be integrated into src/crypto and only the
WPS specific parts should remain in wps_crypto. For example, the current
version seems to implement HMAC-SHA256 which is already available in
src/crypto/sha256.c.

> -- wpse_sm - enrollee state machine. The state data for the state machine
> consists of: configuration parameters; copies of packets sent and received;
> key state.  wpse_sm does NOT send or receive any packets, nor does it
> generate timeouts,  instead the next higher level queries wpse_sm and does
> the jobs for it.  This will simplify using this code in e.g. new embedded
> applications.

Does this really need to keep all possible packets (struct
wps_sm_packet_buffers) in memory all the time? That structure is close
to 30 kB.. Where does the 1520 byte maximum come from? WPS spec does not
seem to define a maximum length and at least EAP transport could handle
up to 64 kB packets..

Like I mentioned above, I don't think Ethernet/EAPOL/EAP processing
belongs here. While I really do not like the idea of UPnP, it sounds
better to keep the interface designed in a way that would handle, if
needed, both EAP and UPnP transports, assuming this can be done without
much extra complexity.

> -- wpse_supplicant -- enrollee interface to the supplicant, it commands the
> supplicant (via wpa_ctrl mechanism, using socket files on linux) to associate
> with the desired ssid in open mode... note that wpa_supplicant runs the whole
> time. This portion could be easily replaced to e.g. perform supplicant
> actions directly.

If the enrollee code will run as a separate process, ctrl_iface is a
proper place was interfacing with wpa_supplicant. However, if this is in
the same process, something else should be used.

> Currently, wps_enrollee is a separate program, and no modifications to
> wpa_supplicant were necessary (except for the Makefile and defconfig, to make
> wps_enrollee)... this is certainly the safe road for early development.
> I've gotten the feedback from you that for various reasons it would be
> better if it were part of wpa_supplicant.  My thinking is that it can be 
> linked as part
> of the wpa_supplicant program yet still logically sit above (the rest of)
> wpa_supplicant... i hope you agree with that. My intention is to extend
> the wpa_ctrl mechanism to include control from within the same process or just 
> bypass wpa_ctrl entirely.

I would agree that it would be fine to have the WPS Registration
Protocol implemented as a separate component above wpa_supplicant.
However, I do not think I would go as far with the full WPS
implementation that would only use wpa_supplicant to associate with the
AP and then take care of all EAPOL/EAP processing.

The way I've been thinking of the WPS process to go would be something
like this:
- wpa_supplicant is running; maybe with empty configuration, but still,
  running and able to provide scan results
- OS specific network management, say NetworkManager (NM), monitors
  network configuration and scan results and finds that no configured
  networks are available, but there is a WPS-enabled AP that could
  potentially be used
- NM requests wpa_supplicant to start WPS with an AP
- wpa_supplicant configures the driver for WPS and requests association
  with the BSS
- wpa_supplicant takes care of EAPOL and EAP processing; EAP-WSC payload
  may be handled either in-process (integrated Enrollee/Registrar) or by
  sending it to an external process (Enrollee/Registrar) (the mechanism
  for this could be separate from ctrl_iface)
- wpa_supplicant notifies NM about progress and if needed, requests
  information from the user (say, PIN)
- wpa_supplicant notifies NM of completion of WPS and availability of
  network parameters
- wpa_supplicant could internally update its configuration and/or wait
  for NM to do that

> The wpa_ctrl mechanism is in my opinion rather weak, and could use some
> extensive rework (in contrast to most of your code which is quite good).
> It uses a text based communication protocol which isn't very easy to extend.

Could you please elaborate? I don't see why text based protocol would
make it harder to extend. ctrl_iface was certainly not designed for
in-process communication, but if there are areas that would benefit from
changes to support external processes better, I would be interested in
hearing more details.

-- 
Jouni Malinen                                            PGP id EFC895FA