Add TOTP (RFC6238) one-time password support

[John Morrissey]

On Sun, Mar 10, 2013 at 03:25:19PM +0000, David Woodhouse wrote:
> On Sun, 2013-03-10 at 11:18 -0400, John Morrissey wrote:
> > On Sat, Mar 09, 2013 at 10:40:30PM +0000, David Woodhouse wrote:
> > > On Fri, 2013-03-08 at 21:48 -0500, John Morrissey wrote:
> > > > On Thu, Mar 07, 2013 at 11:55:18PM +0000, David Woodhouse wrote:
> > > > > On Thu, 2013-03-07 at 18:39 -0500, John Morrissey wrote:
> > > > > > - openconnect_set_stoken_mode no longer accepts the use_stoken
> > > > > >   argument and instead always tries to initialize libstoken when
> > > > > >   called. This makes sense in openconnect(8), but I'm not sure
> > > > > >   how much of a concern this API change is for upstream
> > > > > >   consumers of libopenconnect. I also wasn't sure how to account
> > > > > >   for this in libopenconnect.map.in.
> > > > > 
> > > > > You can't account for it. It's an ABI break and it would take us to
> > > > > libopenconnect.so.3. I'd like to avoid this change, if possible.
> > > > 
> > > > Sure, it's easy enough. See this iteration of the patch.
> > > 
> > > Hm, but now your openconnect_set_oath_mode() API is inconsistent with
> > > openconnect_set_stoken_mode().
> > > 
> > > I'd probably be inclined to make them match.
> > > 
> > > Or even to use openconnect_set_stoken_mode() for *both*. Just pass zero
> > > to disable, 1 for stoken and 2 for OATH.
> > 
> > Actually, I'm wondering whether revving the API is unavoidable at this
> > point. 4.99 was released last month, which revved the API to include
> > libstoken support, and if we add TOTP support to API v2.1, it makes it
> > harder for consumers to determine whether the OATH-related functions are
> > available (they'd have to do their own autoconf checks instead of simply
> > checking the API version). It seems that adding symbols to the API has
> > resulted in a version bump in the past, too.
> 
> There's a different in bumping from 2.1 to 2.2, and bumping from 2.1 to
> 3.0.
[snip]

Yes, I know that. You mentioned wanting to 'avoid this ABI change', which I
incorrectly took to mean *any* ABI version change. We missed having to do
this minor-version API change in previous iterations of this patch, so
that's why I'm bring it up now, since I just realized it.

> > How about revving the API, making the TOKEN_MODE_* enum a typedef in
> > openconnect.h, and adding openconnect_set_token_mode() that takes an
> > OC_TOKEN_MODE_*?
> > 
> > That way, the symbol naming is clear in what it's doing (i.e., that it's not
> > RSA/libstoken-specific), there are no magic numbers to pass thanks to the
> > enum typedef, and there's a single symbol to call. We can leave
> > openconnect_set_stoken_mode() as a shim in front of
> > openconnect_set_token_mode() for transparent backwards compatibility.
> 
> Sounds reasonable.

I'll plan to wrap up the TOTP patch in the next few days, then.

john
-- 
John Morrissey          _o            /\         ----  __o
jwm at horde.net        _-< \_          /  \       ----  <  \,
www.horde.net/    __(_)/_(_)________/    \_______(_) /_(_)__