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.
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.
See the patch below for a rough idea; there are still other changes to be
made, like syncing the TOKEN_MODE_* symbol changes.
john
diff --git a/libopenconnect.map.in b/libopenconnect.map.in
index f38c380..16d3380 100644
--- a/libopenconnect.map.in
+++ b/libopenconnect.map.in
@@ -33,12 +33,16 @@ OPENCONNECT_2.0 {
OPENCONNECT_2.1 {
global:
openconnect_has_stoken_support;
- openconnect_has_oath_support;
openconnect_set_stoken_mode;
- openconnect_set_oath_mode;
openconnect_set_reported_os;
} OPENCONNECT_2.0;
+OPENCONNECT_2.2 {
+ global:
+ openconnect_has_oath_support;
+ openconnect_set_token_mode;
+} OPENCONNECT_2.1;
+
OPENCONNECT_PRIVATE {
global: @SYMVER_TIME@ @SYMVER_ASPRINTF@ @SYMVER_GETLINE@ @SYMVER_PRINT_ERR@
openconnect_SSL_gets;
diff --git a/library.c b/library.c
index 6f46565..1669cca 100644
--- a/library.c
+++ b/library.c
@@ -340,11 +340,49 @@ int openconnect_has_oath_support(void)
}
/*
+ * Enable software token generation.
+ *
+ * If token_mode is OC_TOKEN_MODE_STOKEN and token_str is NULL,
+ * read the token data from ~/.stokenrc.
+ *
+ * Return value:
+ * = -EOPNOTSUPP, if the underlying library (libstoken, liboath) is not
+ * available or an invalid token_mode was provided
+ * = -EINVAL, if the token string is invalid (token_str was provided)
+ * = -ENOENT, if token_mode is OC_TOKEN_MODE_STOKEN and ~/.stokenrc is
+ * missing (token_str was NULL)
+ * = -EIO, for other failures in the underlying library (libstoken, liboath)
+ * = 0, on success
+ */
+int openconnect_set_stoken_mode(struct openconnect_info *vpninfo,
+ oc_token_mode_t token_mode,
+ const char *token_str)
+{
+ vpninfo->token_mode = OC_TOKEN_MODE_NONE;
+
+ switch (token_mode) {
+ case OC_TOKEN_MODE_NONE:
+ return 0;
+
+ case OC_TOKEN_MODE_STOKEN:
+ return set_libstoken_mode(vpninfo, token_str);
+
+ case OC_TOKEN_MODE_TOTP:
+ return set_oath_mode(vpninfo, token_str);
+
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+/*
* Enable libstoken token generation if use_stoken == 1.
*
* If token_str is not NULL, try to parse the string. Otherwise, try to read
* the token data from ~/.stokenrc
*
+ * DEPRECATED: use openconnect_set_stoken_mode() instead.
+ *
* Return value:
* = -EOPNOTSUPP, if libstoken is not available
* = -EINVAL, if the token string is invalid (token_str was provided)
@@ -355,13 +393,20 @@ int openconnect_has_oath_support(void)
int openconnect_set_stoken_mode(struct openconnect_info *vpninfo,
int use_stoken, const char *token_str)
{
-#ifdef LIBSTOKEN_HDR
- int ret;
-
vpninfo->token_mode = TOKEN_MODE_NONE;
if (!use_stoken)
return 0;
+ return openconnect_set_token_mode(vpninfo, OC_TOKEN_MODE_STOKEN,
+ token_str);
+}
+
+static int set_libstoken_mode(struct openconnect_info *vpninfo,
+ const char *token_str)
+{
+#ifdef LIBSTOKEN_HDR
+ int ret;
+
if (!vpninfo->stoken_ctx) {
vpninfo->stoken_ctx = stoken_new();
if (!vpninfo->stoken_ctx)
@@ -381,17 +426,8 @@ int openconnect_set_stoken_mode(struct openconnect_info *vpninfo,
#endif
}
-/*
- * Enable OATH software token generation.
- *
- * Return value:
- * = -EOPNOTSUPP, if libstoken is not available
- * = -EINVAL, if the token string is invalid (token_str was provided)
- * = -EIO, for other libstoken failures
- * = 0, on success
- */
-int openconnect_set_oath_mode(struct openconnect_info *vpninfo,
- const char *token_str)
+static int set_oath_mode(struct openconnect_info *vpninfo,
+ const char *token_str)
{
#ifdef LIBOATH_HDR
int ret;
@@ -414,6 +450,7 @@ int openconnect_set_oath_mode(struct openconnect_info *vpninfo,
vpninfo->oath_secret_len = strlen(token_str);
}
+ vpninfo->token_mode = TOKEN_MODE_TOTP;
return 0;
#else
return -EOPNOTSUPP;
diff --git a/openconnect-internal.h b/openconnect-internal.h
index 1d1daa4..0352af1 100644
--- a/openconnect-internal.h
+++ b/openconnect-internal.h
@@ -66,12 +66,6 @@
#include LIBSTOKEN_HDR
#endif
-enum {
- TOKEN_MODE_NONE,
- TOKEN_MODE_STOKEN,
- TOKEN_MODE_TOTP,
-};
-
#ifdef ENABLE_NLS
#include <locale.h>
#include <libintl.h>
diff --git a/openconnect.h b/openconnect.h
index 2b40c3b..bfac8b0 100644
--- a/openconnect.h
+++ b/openconnect.h
@@ -138,6 +138,12 @@ struct openconnect_info;
#define OPENCONNECT_X509 void
+typedef enum {
+ OC_TOKEN_MODE_NONE,
+ OC_TOKEN_MODE_STOKEN,
+ OC_TOKEN_MODE_TOTP,
+} oc_token_mode_t;
+
/* Unless otherwise specified, all functions which set strings will take
ownership of those strings and the library will free them later in
openconnect_vpninfo_free() */
--
John Morrissey _o /\ ---- __o
jwm at horde.net _-< \_ / \ ---- < \,
www.horde.net/ __(_)/_(_)________/ \_______(_) /_(_)__
