Re: [PATCH] Add Keychain support
Updated a patch.
* Change `--use-keychain` behavior as I suggested in previous email.
* Update help texts and man page
* Add `--save-to-keychain` option to specify which fields are saved
to Keychain.
-- >8 --
Subject: [PATCH] Add Keychain support
This patch adds macOS Keychain support to fill specific password
fields.
If Keychain doesn't have a password entry, it will prompt it then
save it to Keychain if needed.
This patch is squashed commit from
https://github.com/niw/openconnect/tree/add_keychain_support
Signed-off-by: Yoshimasa Niwa
---
Makefile.am| 2 +-
configure.ac | 16 +
main.c | 129 -
openconnect-internal.h | 5 ++
openconnect.8.in | 23
5 files changed, 173 insertions(+), 2 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 522725eb..2e006a90 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -22,7 +22,7 @@ AM_CPPFLAGS = -DLOCALEDIR="\"$(localedir)\""
openconnect_SOURCES = xml.c main.c
openconnect_CFLAGS = $(AM_CFLAGS) $(SSL_CFLAGS) $(DTLS_SSL_CFLAGS)
$(LIBXML2_CFLAGS) $(LIBPROXY_CFLAGS) $(ZLIB_CFLAGS) $(LIBSTOKEN_CFLAGS)
$(LIBPSKC_CFLAGS) $(GSSAPI_CFLAGS) $(INTL_CFLAGS) $(ICONV_CFLAGS)
$(LIBPCSCLITE_CFLAGS)
-openconnect_LDADD = libopenconnect.la $(SSL_LIBS) $(LIBXML2_LIBS)
$(LIBPROXY_LIBS) $(INTL_LIBS) $(ICONV_LIBS)
+openconnect_LDADD = libopenconnect.la $(SSL_LIBS) $(LIBXML2_LIBS)
$(LIBPROXY_LIBS) $(INTL_LIBS) $(ICONV_LIBS) $(KEYCHAIN_LIBS)
if OPENCONNECT_WIN32
openconnect_SOURCES += openconnect.rc
diff --git a/configure.ac b/configure.ac
index 5065a298..3c4cb83a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -204,6 +204,21 @@ AC_CHECK_FUNC(__android_log_vprint, [], AC_CHECK_LIB(log,
__android_log_vprint,
AC_ENABLE_SHARED
AC_DISABLE_STATIC
+keychain_support=no
+AC_ARG_ENABLE([keychain],
+ AS_HELP_STRING([--enable-keychain], [Enable Keychain support]),
+ [ENABLE_KEYCHAIN=$enableval],
+ [ENABLE_KEYCHAIN=no])
+if test "$ENABLE_KEYCHAIN" = "yes"; then
+ AC_CHECK_HEADER([CoreFoundation/CoreFoundation.h],
+ [], [AC_MSG_ERROR(Cannot find CoreFoundaation header.)])
+ AC_CHECK_HEADER([Security/Security.h],
+ [], [AC_MSG_ERROR(Cannot find Security header.)])
+ AC_DEFINE([ENABLE_KEYCHAIN], 1, [Enable Keychain support])
+ keychain_support=yes
+ AC_SUBST(KEYCHAIN_LIBS, ["-framework Foundation -framework Security"])
+fi
+
AC_CHECK_FUNC(nl_langinfo, [AC_DEFINE(HAVE_NL_LANGINFO, 1, [Have nl_langinfo()
function])], [])
if test "$ac_cv_func_nl_langinfo" = "yes"; then
@@ -1042,6 +1057,7 @@ SUMMARY([Java bindings], [$with_java])
SUMMARY([Build docs], [$build_www])
SUMMARY([Unit tests], [$have_cwrap])
SUMMARY([Net namespace tests], [$have_netns])
+SUMMARY([Keychain support], [$keychain_support])
if test "$ssl_library" = "OpenSSL"; then
AC_MSG_WARN([[
diff --git a/main.c b/main.c
index 2e9e3059..195cae71 100644
--- a/main.c
+++ b/main.c
@@ -62,6 +62,11 @@
static const char *legacy_charset;
#endif
+#if ENABLE_KEYCHAIN
+#include
+#include
+#endif
+
static int write_new_config(void *_vpninfo,
const char *buf, int buflen);
static void __attribute__ ((format(printf, 3, 4)))
@@ -85,6 +90,8 @@ static int do_passphrase_from_fsid;
static int non_inter;
static int cookieonly;
static int allow_stdin_read;
+static char *keychain_account = NULL;
+static struct oc_text_list_item *keychain_saving_fields = NULL;
static char *token_filename;
static char *server_cert = NULL;
@@ -171,6 +178,8 @@ enum {
OPT_NO_XMLPOST,
OPT_PIDFILE,
OPT_PASSWORD_ON_STDIN,
+ OPT_USE_KEYCHAIN,
+ OPT_SAVE_TO_KEYCHAIN,
OPT_PRINTCOOKIE,
OPT_RECONNECT_TIMEOUT,
OPT_SERVERCERT,
@@ -246,6 +255,10 @@ static const struct option long_options[] = {
OPTION("xmlconfig", 1, 'x'),
OPTION("cookie-on-stdin", 0, OPT_COOKIE_ON_STDIN),
OPTION("passwd-on-stdin", 0, OPT_PASSWORD_ON_STDIN),
+#if ENABLE_KEYCHAIN
+ OPTION("use-keychain", 1, OPT_USE_KEYCHAIN),
+ OPTION("save-to-keychain", 1, OPT_SAVE_TO_KEYCHAIN),
+#endif
OPTION("no-passwd", 0, OPT_NO_PASSWD),
OPTION("reconnect-timeout", 1, OPT_RECONNECT_TIMEOUT),
OPTION("dtls-ciphers", 1, OPT_DTLS_CIPHERS),
@@ -798,6 +811,10 @@ static void usage(void)
printf(" --no-passwd %s\n", _("Disable
password/SecurID authentication"));
printf(" --non-inter %s\n", _("Do not expect user
input; exit if it is required"));
printf(" --passwd-on-stdin %s\n", _("Read password from
standard input"));
+#if ENABLE_KEYCHAIN
+ printf(" --use-keychain=ACCOUNT %s\n", _("Look up Keychain to
fill password form fields"));
+ printf(" --save-to-keychain=NAME %s\n", _("Name of password
form field to be saved to Keychain"));
+#endif
printf(" --aut
Re: [PATCH] Add Keychain support
On Sun, 2018-10-21 at 09:19 -0700, Yoshimasa Niwa wrote: > Updated a patch. > > * Change `--use-keychain` behavior as I suggested in previous email. > * Update help texts and man page > * Add `--save-to-keychain` option to specify which fields are saved >to Keychain. > > -- >8 -- > Subject: [PATCH] Add Keychain support > > This patch adds macOS Keychain support to fill specific password > fields. > If Keychain doesn't have a password entry, it will prompt it then > save it to Keychain if needed. > > This patch is squashed commit from > https://github.com/niw/openconnect/tree/add_keychain_support CC openconnect-main.o ../main.c:94:34: warning: ‘keychain_saving_fields’ defined but not used [-Wunused-variable] static struct oc_text_list_item *keychain_saving_fields = NULL; ^~ ../main.c:93:14: warning: ‘keychain_account’ defined but not used [-Wunused-variable] static char *keychain_account = NULL; ^~~~ CCLD openconnect smime.p7s Description: S/MIME cryptographic signature ___ openconnect-devel mailing list openconnect-devel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/openconnect-devel
Re: [PATCH] Add Keychain support
On Sat, 2018-11-03 at 22:37 -0700, Yoshimasa Niwa wrote: > > Hm... or maybe only the 'password' type fields should be stored in > > keychain and every other form field can be provided on the command > > line? Those ones aren't secret, after all. > > Yeah, I agree. Keychain should only fill the password type field. > > > We do still need to allow for the fact that there might be multiple > > passwords though (and one day, maybe some saveable and some not, for > > example a password and a separate OTP). But specifying on the command > > line which password(s) to save would be OK, I think? > > Yes, that's why I mentioned in previous email asking user to save it > or not in Keychain, > but giving it as an argument would be better option. > I knew it because for my personal usage, the form requires two passwords and > one is for OTP as exactly what you described. > Let me change a little bit more on my patch. Some of this is already handled by the UI authentication tools. The NetworkManager auth-dialog, for example, already stores the form entries and uses libsecret for password fields. Although it doesn't support selectively storing *some* password fields and not others. (At least, not except by using the trick of turning the 'save passwords' option off manually instead of through the UI so that the saved passwords aren't cleared, then manually clearing the password(s) that you don't want to be stored...) I think a --use-keychain argument which either stands alone *or* takes a field name in the same form as the '--form-field' I just added in the 'fields' branch, might make sense? Do we need to allow OpenConnect to *write* those secrets to the keychain/libsecret too? Or is reading them sufficient? > > FWIW what I'd *really* like to see is SSL certificate support using the > > keychain... > > Looks like GnuTLS has common API that is for supporting system key > store, however, > according to their documents, it’s at this moment only supporting Windows one. > I think it may be not much difficult to use Keychain to lookup > certificates and keys > like what current `ANDROID_KEYSTORE` does. > Let me try after implementing above changes for passwords. Yeah, whatever we do here, we'd be looking to Nikos to include it in GnuTLS. The TPMv1 code in OpenConnect might make a reasonable example here — we just need to provide a gnutls_privkey_t with a suitable sign_func that calls into the keychain functions to actually do the signature. Unlike TPM we presumably want to support *certificates* from the keychain too rather than just private keys. But that should be even easier; certificates are public so we just need to obtain the data and populate a gnutls_x509_crt_t with it. Note that the ANDROID_KEYSTORE support predates the Android keystore actually doing anything sane — it assumes you can actually read the private key data from the store, instead of being limited to performing *operations* using it. We should probably fix that up one day... smime.p7s Description: S/MIME cryptographic signature ___ openconnect-devel mailing list openconnect-devel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/openconnect-devel
Re: Complicated web login flows with Pulse Secure VPN
On Tue, 2018-10-30 at 16:54 -0500, Andy Wang wrote: > I was, up until very recently, using openconnect and > NetworkManager-openconnect to connect to my work VPN. I had a private > hack to make the stoken stuff work (it was submitted in an email on > this list) as well as another hack to deal with our token form not > having the same expected form type. Remind me of those please. As I prepare for the 8.0 release it would be good to pull those in unless they're completely horrible hacks specific to your setup. > A couple of weeks ago we moved to a whole new login flow, where we now > are redirected to a saml login page for authentication and then > prompted to choose one of two types of MFA access - token code or > mobile application notification based. > > With the more complicated flow I've had to switch back to the pulse > secure client which embeds a webkitgtk UI to handle those flows. > > Just curious but is there anyone working on some similar flow support > with NetworkManager-openconnect? I'm guessing that this type of > authentication is way outside of the scope of openconnect's built in > html client. (Pulse Secure's cli client can't handle this login flow > either). It's been talked about, repeatedly :) The first step is to add a 'webview' callback method which the GUI authentications can implement, which bypasses the current hackish HTML screen-scraping. That much is relatively easy, in fact, but then we'd need to do the WebKitGtk stuff inside the NetworkManager auth-dialog for GNOME and KDE, etc. If there's a volunteer for the latter, I could certainly put together the former. I'm just not that keen on throwing together the API change for the webview callback without properly testing it. smime.p7s Description: S/MIME cryptographic signature ___ openconnect-devel mailing list openconnect-devel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/openconnect-devel
Re: Complicated web login flows with Pulse Secure VPN
On Sun, Nov 4, 2018 at 11:55 AM David Woodhouse wrote: > > Remind me of those please. As I prepare for the 8.0 release it would be > good to pull those in unless they're completely horrible hacks specific > to your setup. > The second patch I mentioned was a pretty bad hack (especially after discussing it with Daniei Lenski). Worked for me but definitely not the right solution. The issue is my work vpn used the same loginForm form ID for both password and stoken input. So there was no easy way to distinguish the two and I made an ugly hack that worked but results failed login attempts as it tries the token id as the password field. I'll re-send the stoken patch request and add you to the thread so you can see that one. > > It's been talked about, repeatedly :) > > The first step is to add a 'webview' callback method which the GUI > authentications can implement, which bypasses the current hackish HTML > screen-scraping. That much is relatively easy, in fact, but then we'd > need to do the WebKitGtk stuff inside the NetworkManager auth-dialog > for GNOME and KDE, etc. > > If there's a volunteer for the latter, I could certainly put together > the former. I'm just not that keen on throwing together the API change > for the webview callback without properly testing it. I'd definitely be interested, but I haven't done any C/C++ programming in nearly 2 decades now :) If I get some time on this I might try to dust off the rust and see how I can do with it, but I'm not counting on making any real headway at least not soon. If I get some serious time I'll let you know. Thanks for the info, Andy ___ openconnect-devel mailing list openconnect-devel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/openconnect-devel
Re: [PATCH] Fix stoken support for Juniper VPN
David,
This is the stoken patch that you asked about on my other thread.
Thanks,
Andy
On Fri, Sep 7, 2018 at 10:49 AM Andy Wang wrote:
>
> Ensure stoken seed is properly prepared using block copied from Cisco
> VPN support in auth.c
>
> Signed-off-by: Andy Wang
> ---
> auth-juniper.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/auth-juniper.c b/auth-juniper.c
> index 30ceb3ae..bc560823 100644
> --- a/auth-juniper.c
> +++ b/auth-juniper.c
> @@ -576,6 +576,14 @@ int oncp_obtain_cookie(struct openconnect_info *vpninfo)
> char *form_id = NULL;
> int try_tncc = !!vpninfo->csd_wrapper;
>
> +#ifdef HAVE_LIBSTOKEN
> +if (vpninfo->token_mode == OC_TOKEN_MODE_STOKEN) {
> +ret = prepare_stoken(vpninfo);
> +if (ret)
> +goto out;
> +}
> +#endif
> +
> resp_buf = buf_alloc();
> if (buf_error(resp_buf))
> return -ENOMEM;
> --
> 2.17.1
>
___
openconnect-devel mailing list
openconnect-devel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/openconnect-devel
