Discussion:
[PATCH] Add Keychain support
Yoshimasa Niwa
2018-10-21 16:19:15 UTC
Permalink
This patch adds macOS Keychain support to fill specific password
option.
If Keychain doesn't have a password entry, it will prompt it then
save it in Keychain.

This patch is squashed commit from
https://github.com/niw/openconnect/tree/add_keychain_support

Signed-off-by: Yoshimasa Niwa <***@niw.at>
---
Makefile.am | 2 +-
configure.ac | 16 +++++++
main.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++---
3 files changed, 143 insertions(+), 7 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..ef87a2a7 100644
--- a/main.c
+++ b/main.c
@@ -62,6 +62,11 @@
static const char *legacy_charset;
#endif

+#if ENABLE_KEYCHAIN
+#include <CoreFoundation/CoreFoundation.h>
+#include <Security/Security.h>
+#endif
+
static int write_new_config(void *_vpninfo,
const char *buf, int buflen);
static void __attribute__ ((format(printf, 3, 4)))
@@ -85,6 +90,7 @@ static int do_passphrase_from_fsid;
static int non_inter;
static int cookieonly;
static int allow_stdin_read;
+static char *keychain_opt_name = NULL;

static char *token_filename;
static char *server_cert = NULL;
@@ -171,6 +177,7 @@ enum {
OPT_NO_XMLPOST,
OPT_PIDFILE,
OPT_PASSWORD_ON_STDIN,
+ OPT_USE_KEYCHAIN,
OPT_PRINTCOOKIE,
OPT_RECONNECT_TIMEOUT,
OPT_SERVERCERT,
@@ -246,6 +253,9 @@ 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),
+#endif
OPTION("no-passwd", 0, OPT_NO_PASSWD),
OPTION("reconnect-timeout", 1, OPT_RECONNECT_TIMEOUT),
OPTION("dtls-ciphers", 1, OPT_DTLS_CIPHERS),
@@ -813,6 +823,9 @@ static void usage(void)
#ifndef HAVE_LIBPCSCLITE
printf(" %s\n", _("(NOTE: Yubikey OATH disabled in this build)"));
#endif
+#if ENABLE_KEYCHAIN
+ printf(" --use-keychain=NAME %s\n", _("Name of password option to lookup Keychain"));
+#endif

printf("\n%s:\n", _("Server validation"));
printf(" --servercert=FINGERPRINT %s\n", _("Server's certificate SHA1 fingerprint"));
@@ -1284,6 +1297,12 @@ int main(int argc, char **argv)
read_stdin(&password, 0, 0);
allow_stdin_read = 1;
break;
+#if ENABLE_KEYCHAIN
+ case OPT_USE_KEYCHAIN:
+ free(keychain_opt_name);
+ keychain_opt_name = dup_config_arg();
+ break;
+#endif
case OPT_NO_PASSWD:
vpninfo->nopasswd = 1;
break;
@@ -1946,6 +1965,83 @@ retry:
return 0;
}

+#if ENABLE_KEYCHAIN
+static char *lookup_keychain_password(const char *user, const char *prompt, struct openconnect_info *vpninfo)
+{
+ OSStatus err = 0;
+
+ CFMutableDictionaryRef query = NULL;
+ CFStringRef account = NULL, server = NULL, path = NULL;
+ CFTypeRef data = NULL;
+ char *result = NULL;
+
+ if (verbose > PRG_ERR) {
+ fprintf(stderr, "Lookup keychain for user: %s url: https://%s%s\n", user, vpninfo->hostname, vpninfo->urlpath);
+ }
+
+ query = CFDictionaryCreateMutable(kCFAllocatorDefault, 0, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);
+ if (!query) goto end;
+
+ account = CFStringCreateWithCString(kCFAllocatorDefault, user, kCFStringEncodingUTF8);
+ if (!account) goto end;
+ server = CFStringCreateWithCString(kCFAllocatorDefault, vpninfo->hostname, kCFStringEncodingUTF8);
+ if (!server) goto end;
+ path = CFStringCreateWithCString(kCFAllocatorDefault, vpninfo->urlpath, kCFStringEncodingUTF8);
+ if (!path) goto end;
+
+ CFDictionaryAddValue(query, kSecClass, kSecClassInternetPassword);
+ CFDictionaryAddValue(query, kSecAttrAccount, account);
+ CFDictionaryAddValue(query, kSecAttrProtocol, kSecAttrProtocolHTTPS);
+ CFDictionaryAddValue(query, kSecAttrServer, server);
+ CFDictionaryAddValue(query, kSecAttrPath, path);
+ CFDictionaryAddValue(query, kSecMatchLimit, kSecMatchLimitOne);
+ CFDictionaryAddValue(query, kSecReturnData, kCFBooleanTrue);
+
+ err = SecItemCopyMatching(query, &data);
+ if (err == errSecItemNotFound) {
+ if (data) CFRelease(data);
+
+ fprintf(stderr, "Item not found in Keychain\n");
+
+ result = prompt_for_input(prompt, vpninfo, 1);
+ if (!result) goto end;
+ size_t len = strlen(result);
+ if (len == 0) goto end;
+
+ data = CFDataCreate(kCFAllocatorDefault, (UInt8 *)result, len + 1);
+ if (!data) goto end;
+
+ CFDictionaryAddValue(query, kSecValueData, data);
+ CFDictionaryRemoveValue(query, kSecReturnData);
+
+ err = SecItemAdd(query, NULL);
+ if (err != errSecSuccess) {
+ if (verbose > PRG_ERR) {
+ fprintf(stderr, "Fail to add item to Keychain\n");
+ }
+ }
+ goto end;
+ }
+ if (err != errSecSuccess) goto end;
+ if (!data || CFGetTypeID(data) != CFDataGetTypeID()) goto end;
+
+ CFIndex size = CFDataGetLength(data);
+ result = malloc((size_t)size);
+ if (!result) goto end;
+
+ CFDataGetBytes(data, CFRangeMake(0, size), (UInt8 *)result);
+
+end:
+ if (query) CFRelease(query);
+ if (account) CFRelease(account);
+ if (server) CFRelease(server);
+ if (path) CFRelease(path);
+ if (data) CFRelease(data);
+
+ return result;
+}
+#endif
+
/* Return value:
* < 0, on error
* = 0, when form was parsed and POST required
@@ -1955,8 +2051,9 @@ static int process_auth_form_cb(void *_vpninfo,
struct oc_auth_form *form)
{
struct openconnect_info *vpninfo = _vpninfo;
- struct oc_form_opt *opt;
+ struct oc_form_opt *opt, *prev_opt;
int empty = 1;
+ char *user;

if (form->banner && verbose > PRG_ERR)
fprintf(stderr, "%s\n", form->banner);
@@ -1981,6 +2078,18 @@ static int process_auth_form_cb(void *_vpninfo,
}
}

+ // Reorder `opts` to bring `user` first.
+ for (prev_opt = NULL, opt = form->opts; opt; prev_opt = opt, opt = opt->next) {
+ if ((opt->type == OC_FORM_OPT_TEXT) && !strncmp(opt->name, "user", 4)) {
+ if (prev_opt) {
+ prev_opt->next = opt->next;
+ opt->next = form->opts;
+ form->opts = opt;
+ }
+ break;
+ }
+ }
+
for (opt = form->opts; opt; opt = opt->next) {

if (opt->flags & OC_FORM_OPT_IGNORE)
@@ -1998,10 +2107,14 @@ static int process_auth_form_cb(void *_vpninfo,
empty = 0;

} else if (opt->type == OC_FORM_OPT_TEXT) {
- if (username &&
- !strncmp(opt->name, "user", 4)) {
- opt->_value = username;
- username = NULL;
+ if (!strncmp(opt->name, "user", 4)) {
+ if (username) {
+ opt->_value = username;
+ username = NULL;
+ } else {
+ opt->_value = prompt_for_input(opt->label, vpninfo, 0);
+ }
+ user = opt->_value;
} else {
opt->_value = prompt_for_input(opt->label, vpninfo, 0);
}
@@ -2014,7 +2127,14 @@ static int process_auth_form_cb(void *_vpninfo,
if (password) {
opt->_value = password;
password = NULL;
- } else {
+ }
+#if ENABLE_KEYCHAIN
+ else if (keychain_opt_name && user && !strcmp(opt->name, keychain_opt_name)) {
+ opt->_value = lookup_keychain_password(user, opt->label, vpninfo);
+ keychain_opt_name = NULL;
+ }
+#endif
+ else {
opt->_value = prompt_for_input(opt->label, vpninfo, 1);
}
--
Yoshimasa Niwa
David Woodhouse
2018-10-28 12:46:23 UTC
Permalink
Post by Yoshimasa Niwa
This patch adds macOS Keychain support to fill specific password
option.
If Keychain doesn't have a password entry, it will prompt it then
save it in Keychain.
This patch is squashed commit from
https://github.com/niw/openconnect/tree/add_keychain_support
This looks great; thanks! I'm not really looking at the computer this
week but will play with it later. I'll probably try doing something
more generic in main.c to allow other sources of information, perhaps
like '--formentry main:foo=bar' on the command line.

Man page too please.
Yoshimasa Niwa
2018-10-29 00:31:55 UTC
Permalink
Hi, David,

Thank you for reply and comment.
That generic solution would be better for sure, but may require many
changes, if we could that would be nice.
Let me update man page at least, sorry I completely forgot it.

Y
Post by David Woodhouse
Post by Yoshimasa Niwa
This patch adds macOS Keychain support to fill specific password
option.
If Keychain doesn't have a password entry, it will prompt it then
save it in Keychain.
This patch is squashed commit from
https://github.com/niw/openconnect/tree/add_keychain_support
This looks great; thanks! I'm not really looking at the computer this
week but will play with it later. I'll probably try doing something
more generic in main.c to allow other sources of information, perhaps
like '--formentry main:foo=bar' on the command line.
Man page too please.
--
Yoshimasa Niwa
Yoshimasa Niwa
2018-10-21 16:19:15 UTC
Permalink
Post by David Woodhouse
Man page too please.
Update man page, amend help text.

-- >8 --
Subject: [PATCH] Add Keychain support

This patch adds macOS Keychain support to fill specific password
option.
If Keychain doesn't have a password entry, it will prompt it then
save it in Keychain.

This patch is squashed commit from
https://github.com/niw/openconnect/tree/add_keychain_support

Signed-off-by: Yoshimasa Niwa <***@niw.at>
---
Makefile.am | 2 +-
configure.ac | 16 ++++++
main.c | 132 ++++++++++++++++++++++++++++++++++++++++++++---
openconnect.8.in | 6 +++
4 files changed, 149 insertions(+), 7 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..59c9f481 100644
--- a/main.c
+++ b/main.c
@@ -62,6 +62,11 @@
static const char *legacy_charset;
#endif

+#if ENABLE_KEYCHAIN
+#include <CoreFoundation/CoreFoundation.h>
+#include <Security/Security.h>
+#endif
+
static int write_new_config(void *_vpninfo,
const char *buf, int buflen);
static void __attribute__ ((format(printf, 3, 4)))
@@ -85,6 +90,7 @@ static int do_passphrase_from_fsid;
static int non_inter;
static int cookieonly;
static int allow_stdin_read;
+static char *keychain_opt_name = NULL;

static char *token_filename;
static char *server_cert = NULL;
@@ -171,6 +177,7 @@ enum {
OPT_NO_XMLPOST,
OPT_PIDFILE,
OPT_PASSWORD_ON_STDIN,
+ OPT_USE_KEYCHAIN,
OPT_PRINTCOOKIE,
OPT_RECONNECT_TIMEOUT,
OPT_SERVERCERT,
@@ -246,6 +253,9 @@ 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),
+#endif
OPTION("no-passwd", 0, OPT_NO_PASSWD),
OPTION("reconnect-timeout", 1, OPT_RECONNECT_TIMEOUT),
OPTION("dtls-ciphers", 1, OPT_DTLS_CIPHERS),
@@ -798,6 +808,9 @@ 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=NAME %s\n", _("Name of password form option that look up Keychain to fill"));
+#endif
printf(" --authgroup=GROUP %s\n", _("Choose authentication login selection"));
printf(" -c, --certificate=CERT %s\n", _("Use SSL client certificate CERT"));
printf(" -k, --sslkey=KEY %s\n", _("Use SSL private key file KEY"));
@@ -1284,6 +1297,12 @@ int main(int argc, char **argv)
read_stdin(&password, 0, 0);
allow_stdin_read = 1;
break;
+#if ENABLE_KEYCHAIN
+ case OPT_USE_KEYCHAIN:
+ free(keychain_opt_name);
+ keychain_opt_name = dup_config_arg();
+ break;
+#endif
case OPT_NO_PASSWD:
vpninfo->nopasswd = 1;
break;
@@ -1946,6 +1965,83 @@ retry:
return 0;
}

+#if ENABLE_KEYCHAIN
+static char *lookup_keychain_password(const char *user, const char *prompt, struct openconnect_info *vpninfo)
+{
+ OSStatus err = 0;
+
+ CFMutableDictionaryRef query = NULL;
+ CFStringRef account = NULL, server = NULL, path = NULL;
+ CFTypeRef data = NULL;
+ char *result = NULL;
+
+ if (verbose > PRG_ERR) {
+ fprintf(stderr, "Lookup keychain for user: %s url: https://%s%s\n", user, vpninfo->hostname, vpninfo->urlpath);
+ }
+
+ query = CFDictionaryCreateMutable(kCFAllocatorDefault, 0, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);
+ if (!query) goto end;
+
+ account = CFStringCreateWithCString(kCFAllocatorDefault, user, kCFStringEncodingUTF8);
+ if (!account) goto end;
+ server = CFStringCreateWithCString(kCFAllocatorDefault, vpninfo->hostname, kCFStringEncodingUTF8);
+ if (!server) goto end;
+ path = CFStringCreateWithCString(kCFAllocatorDefault, vpninfo->urlpath, kCFStringEncodingUTF8);
+ if (!path) goto end;
+
+ CFDictionaryAddValue(query, kSecClass, kSecClassInternetPassword);
+ CFDictionaryAddValue(query, kSecAttrAccount, account);
+ CFDictionaryAddValue(query, kSecAttrProtocol, kSecAttrProtocolHTTPS);
+ CFDictionaryAddValue(query, kSecAttrServer, server);
+ CFDictionaryAddValue(query, kSecAttrPath, path);
+ CFDictionaryAddValue(query, kSecMatchLimit, kSecMatchLimitOne);
+ CFDictionaryAddValue(query, kSecReturnData, kCFBooleanTrue);
+
+ err = SecItemCopyMatching(query, &data);
+ if (err == errSecItemNotFound) {
+ if (data) CFRelease(data);
+
+ fprintf(stderr, "Item not found in Keychain\n");
+
+ result = prompt_for_input(prompt, vpninfo, 1);
+ if (!result) goto end;
+ size_t len = strlen(result);
+ if (len == 0) goto end;
+
+ data = CFDataCreate(kCFAllocatorDefault, (UInt8 *)result, len + 1);
+ if (!data) goto end;
+
+ CFDictionaryAddValue(query, kSecValueData, data);
+ CFDictionaryRemoveValue(query, kSecReturnData);
+
+ err = SecItemAdd(query, NULL);
+ if (err != errSecSuccess) {
+ if (verbose > PRG_ERR) {
+ fprintf(stderr, "Fail to add item to Keychain\n");
+ }
+ }
+ goto end;
+ }
+ if (err != errSecSuccess) goto end;
+ if (!data || CFGetTypeID(data) != CFDataGetTypeID()) goto end;
+
+ CFIndex size = CFDataGetLength(data);
+ result = malloc((size_t)size);
+ if (!result) goto end;
+
+ CFDataGetBytes(data, CFRangeMake(0, size), (UInt8 *)result);
+
+end:
+ if (query) CFRelease(query);
+ if (account) CFRelease(account);
+ if (server) CFRelease(server);
+ if (path) CFRelease(path);
+ if (data) CFRelease(data);
+
+ return result;
+}
+#endif
+
/* Return value:
* < 0, on error
* = 0, when form was parsed and POST required
@@ -1955,8 +2051,9 @@ static int process_auth_form_cb(void *_vpninfo,
struct oc_auth_form *form)
{
struct openconnect_info *vpninfo = _vpninfo;
- struct oc_form_opt *opt;
+ struct oc_form_opt *opt, *prev_opt;
int empty = 1;
+ char *user;

if (form->banner && verbose > PRG_ERR)
fprintf(stderr, "%s\n", form->banner);
@@ -1981,6 +2078,18 @@ static int process_auth_form_cb(void *_vpninfo,
}
}

+ // Reorder `opts` to bring `user` first.
+ for (prev_opt = NULL, opt = form->opts; opt; prev_opt = opt, opt = opt->next) {
+ if ((opt->type == OC_FORM_OPT_TEXT) && !strncmp(opt->name, "user", 4)) {
+ if (prev_opt) {
+ prev_opt->next = opt->next;
+ opt->next = form->opts;
+ form->opts = opt;
+ }
+ break;
+ }
+ }
+
for (opt = form->opts; opt; opt = opt->next) {

if (opt->flags & OC_FORM_OPT_IGNORE)
@@ -1998,10 +2107,14 @@ static int process_auth_form_cb(void *_vpninfo,
empty = 0;

} else if (opt->type == OC_FORM_OPT_TEXT) {
- if (username &&
- !strncmp(opt->name, "user", 4)) {
- opt->_value = username;
- username = NULL;
+ if (!strncmp(opt->name, "user", 4)) {
+ if (username) {
+ opt->_value = username;
+ username = NULL;
+ } else {
+ opt->_value = prompt_for_input(opt->label, vpninfo, 0);
+ }
+ user = opt->_value;
} else {
opt->_value = prompt_for_input(opt->label, vpninfo, 0);
}
@@ -2014,7 +2127,14 @@ static int process_auth_form_cb(void *_vpninfo,
if (password) {
opt->_value = password;
password = NULL;
- } else {
+ }
+#if ENABLE_KEYCHAIN
+ else if (keychain_opt_name && user && !strcmp(opt->name, keychain_opt_name)) {
+ opt->_value = lookup_keychain_password(user, opt->label, vpninfo);
+ keychain_opt_name = NULL;
+ }
+#endif
+ else {
opt->_value = prompt_for_input(opt->label, vpninfo, 1);
}

diff --git a/openconnect.8.in b/openconnect.8.in
index 37a33d0c..2fe57c05 100644
--- a/openconnect.8.in
+++ b/openconnect.8.in
@@ -57,6 +57,7 @@ openconnect \- Multi-protocol VPN client, for Cisco AnyConnect VPNs and others
.OP \-\-no\-xmlpost
.OP \-\-non\-inter
.OP \-\-passwd\-on\-stdin
+.OP \-\-use-keychain string
.OP \-\-protocol proto
.OP \-\-token\-mode mode
.OP \-\-token\-secret {secret\fR[\fI,counter\fR]|@\fIfile\fR}
@@ -426,6 +427,11 @@ Do not expect user input; exit if it is required.
.B \-\-passwd\-on\-stdin
Read password from standard input
.TP
+.B \-\-use\-keychain=NAME
+Look up Keychain to fill one of password form options.
+.I NAME
+is the name of the password form option. It may be "password".
+.TP
.B \-\-protocol=PROTO
Select VPN protocol
.I PROTO
--
Yoshimasa Niwa
David Woodhouse
2018-10-29 08:31:15 UTC
Permalink
Post by Yoshimasa Niwa
Hi, David,
Thank you for reply and comment.
That generic solution would be better for sure, but may require many
changes, if we could that would be nice.
That should be relatively simple I think. Not API changes but just in
main.c. I was thinking of.jist looking it up in a list (of --formentry
args that were provided on the command line) in the same place your code
does the keychain lookup.

Why the sorting to put user first, btw? Is that mandatory?
--
dwmw2
Yoshimasa Niwa
2018-10-30 01:30:25 UTC
Permalink
Post by David Woodhouse
I was thinking of.jist looking it up in a list (of --formentry
args that were provided on the command line) in the same place your code
does the keychain lookup.
I see, like `--formentry password=keychain` to ask Keychain,
`--formentry password=stdin` to select from where it reads each value?
Post by David Woodhouse
Why the sorting to put user first, btw? Is that mandatory?
Sort of, because to lookup password in Keychain (or any similar
vaults,) the entries are usually paired with the user name,
(and a few other keys like URL,) it needs to know `user` first.

Y
--
Yoshimasa Niwa
David Woodhouse
2018-10-30 07:58:11 UTC
Permalink
Post by Yoshimasa Niwa
Post by David Woodhouse
I was thinking of.jist looking it up in a list (of --formentry
args that were provided on the command line) in the same place your code
does the keychain lookup.
I see, like `--formentry password=keychain` to ask Keychain,
`--formentry password=stdin` to select from where it reads each value?
No, the --formentry options would contain the field name and the actual
answer. For example
--formentry main:user=dwmw2 --formentry main:group=foo

Using keychain would be a separate option.
Post by Yoshimasa Niwa
Post by David Woodhouse
Why the sorting to put user first, btw? Is that mandatory?
Sort of, because to lookup password in Keychain (or any similar
vaults,) the entries are usually paired with the user name,
(and a few other keys like URL,) it needs to know `user` first.
Hm, take a look at the way NetworkManager-openconnect constructs the
'key' for looking up this data. That just uses $FORMNAME:$FIELDNAME in
the style I've been using in my examples above....
Yoshimasa Niwa
2018-10-31 08:13:04 UTC
Permalink
Post by David Woodhouse
No, the --formentry options would contain the field name and the actual
answer. For example
--formentry main:user=dwmw2 --formentry main:group=foo
Using keychain would be a separate option.
Ah, got it.
Post by David Woodhouse
Hm, take a look at the way NetworkManager-openconnect constructs the
'key' for looking up this data. That just uses $FORMNAME:$FIELDNAME in
the style I've been using in my examples above....
I see. This patch is using internet password type item in Keychain,
which has username and URL as a key and password as a value.
However, we can use a generic type item instead, to store secrets in
the flexible scheme.
In that case, probably I will change the patch to like this:

1) Change `--use-keychain=...` to take Keychain entry name, not
password form field name. For example, `--use-keychain=companyvpn`
2) When it sees each password form field, lookup Keychain with given
entry name and if exists, load the value. If not, ask it on tty, then
store it in Keychain, if the user wants.
For example, if we see "formname:password" form field, then ask it on tty, then
ask to want to save to Keychain or not, then if the answer is yes,
create a generic type item like:
key: service=openconnect, account=companyvpn, generic=formname:password
value: (value from tty)

Slightly complicated than the current patch, but I think this approach
is much more flexible and works better with suggested `--formentry`
option..?

Y



--
Yoshimasa Niwa
David Woodhouse
2018-11-01 08:03:07 UTC
Permalink
Post by Yoshimasa Niwa
I see. This patch is using internet password type item in Keychain,
which has username and URL as a key and password as a value.
However, we can use a generic type item instead, to store secrets in
the flexible scheme.
1) Change `--use-keychain=...` to take Keychain entry name, not
password form field name. For example, `--use-keychain=companyvpn`
2) When it sees each password form field, lookup Keychain with given
entry name and if exists, load the value. If not, ask it on tty, then
store it in Keychain, if the user wants.
For example, if we see "formname:password" form field, then ask it on tty, then
ask to want to save to Keychain or not, then if the answer is yes,
key: service=openconnect, account=companyvpn, generic=formname:password
value: (value from tty)
Slightly complicated than the current patch, but I think this approach
is much more flexible and works better with suggested `--formentry`
option..?
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.

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?

FWIW what I'd *really* like to see is SSL certificate support using the
keychain...
Yoshimasa Niwa
2018-11-04 05:37:32 UTC
Permalink
Post by David Woodhouse
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.
Post by David Woodhouse
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.
Post by David Woodhouse
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.

Y
--
Yoshimasa Niwa
Yoshimasa Niwa
2018-10-21 16:19:15 UTC
Permalink
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 <***@niw.at>
---
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 <CoreFoundation/CoreFoundation.h>
+#include <Security/Security.h>
+#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(" --authgroup=GROUP %s\n", _("Choose authentication login selection"));
printf(" -c, --certificate=CERT %s\n", _("Use SSL client certificate CERT"));
printf(" -k, --sslkey=KEY %s\n", _("Use SSL private key file KEY"));
@@ -1284,6 +1301,18 @@ int main(int argc, char **argv)
read_stdin(&password, 0, 0);
allow_stdin_read = 1;
break;
+#if ENABLE_KEYCHAIN
+ case OPT_USE_KEYCHAIN:
+ keychain_account = keep_config_arg();
+ break;
+ case OPT_SAVE_TO_KEYCHAIN: {
+ struct oc_text_list_item *field = malloc(sizeof(*field));
+ field->data = keep_config_arg();
+ field->next = keychain_saving_fields;
+ keychain_saving_fields = field;
+ break;
+ }
+#endif
case OPT_NO_PASSWD:
vpninfo->nopasswd = 1;
break;
@@ -1946,6 +1975,98 @@ retry:
return 0;
}

+#if ENABLE_KEYCHAIN
+static char *lookup_keychain_password(const char *acc,
+ struct oc_form_opt *opt,
+ struct openconnect_info *vpninfo)
+{
+ OSStatus err = 0;
+
+ CFMutableDictionaryRef query = NULL;
+ CFStringRef account = NULL, name = NULL, key = NULL, label = NULL;
+ CFTypeRef data = NULL;
+ char *result = NULL;
+
+ if (verbose > PRG_INFO)
+ fprintf(stderr, "Lookup keychain for account: %s name: %s\n", acc, opt->name);
+
+ query = CFDictionaryCreateMutable(kCFAllocatorDefault, 0, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);
+ if (!query) goto end;
+
+ account = CFStringCreateWithCString(kCFAllocatorDefault, acc, kCFStringEncodingUTF8);
+ if (!account) goto end;
+ name = CFStringCreateWithCString(kCFAllocatorDefault, opt->name, kCFStringEncodingUTF8);
+ if (!name) goto end;
+ key = CFStringCreateWithFormat(kCFAllocatorDefault, NULL, CFSTR("%@:%@"), account, name);
+ if (!key) goto end;
+
+ CFDictionaryAddValue(query, kSecClass, kSecClassGenericPassword);
+ CFDictionaryAddValue(query, kSecAttrService, CFSTR("openconnect"));
+ CFDictionaryAddValue(query, kSecAttrAccount, key);
+ CFDictionaryAddValue(query, kSecMatchLimit, kSecMatchLimitOne);
+ CFDictionaryAddValue(query, kSecReturnData, kCFBooleanTrue);
+
+ err = SecItemCopyMatching(query, &data);
+ if (err == errSecItemNotFound) {
+ if (data) CFRelease(data);
+
+ if (verbose > PRG_ERR)
+ fprintf(stderr, "Item not found in Keychain\n");
+
+ result = prompt_for_input(opt->label, vpninfo, 1);
+ if (!result) goto end;
+ size_t len = strlen(result);
+ if (len == 0) goto end;
+
+ for (struct oc_text_list_item *field = keychain_saving_fields; field; field = field->next) {
+ if (strcmp(opt->name, field->data))
+ continue;
+
+ label = CFStringCreateWithFormat(kCFAllocatorDefault, NULL, CFSTR("openconnect: %@ (%@)"), account, name);
+ if (!label) goto end;
+ data = CFDataCreate(kCFAllocatorDefault, (UInt8 *)result, len + 1);
+ if (!data) goto end;
+
+ CFDictionaryAddValue(query, kSecAttrLabel, label);
+ CFDictionaryAddValue(query, kSecValueData, data);
+ CFDictionaryRemoveValue(query, kSecReturnData);
+
+ err = SecItemAdd(query, NULL);
+ if (err != errSecSuccess) {
+ if (verbose > PRG_ERR)
+ fprintf(stderr, "Failed to add item to Keychain error: %d\n", err);
+ } else {
+ if (verbose > PRG_INFO)
+ fprintf(stderr, "Item saved in Keychain\n");
+ }
+ goto end;
+ }
+ goto end;
+ } else if (err != errSecSuccess) {
+ if (verbose > PRG_ERR)
+ fprintf(stderr, "Failed to find item in Keychain error: %d\n", err);
+ goto end;
+ }
+ if (!data || CFGetTypeID(data) != CFDataGetTypeID()) goto end;
+
+ CFIndex size = CFDataGetLength(data);
+ result = malloc((size_t)size);
+ if (!result) goto end;
+
+ CFDataGetBytes(data, CFRangeMake(0, size), (UInt8 *)result);
+
+end:
+ if (query) CFRelease(query);
+ if (account) CFRelease(account);
+ if (name) CFRelease(name);
+ if (key) CFRelease(key);
+ if (label) CFRelease(label);
+ if (data) CFRelease(data);
+
+ return result;
+}
+#endif
+
/* Return value:
* < 0, on error
* = 0, when form was parsed and POST required
@@ -2014,7 +2135,13 @@ static int process_auth_form_cb(void *_vpninfo,
if (password) {
opt->_value = password;
password = NULL;
- } else {
+ }
+#if ENABLE_KEYCHAIN
+ else if (keychain_account) {
+ opt->_value = lookup_keychain_password(keychain_account, opt, vpninfo);
+ }
+#endif
+ else {
opt->_value = prompt_for_input(opt->label, vpninfo, 1);
}

diff --git a/openconnect-internal.h b/openconnect-internal.h
index 8aa8fc89..c6b8686e 100644
--- a/openconnect-internal.h
+++ b/openconnect-internal.h
@@ -197,6 +197,11 @@ struct oc_text_buf {
int error;
};

+struct oc_text_list_item {
+ char *data;
+ struct oc_text_list_item *next;
+};
+
#define TLS_MASTER_KEY_SIZE 48

#define RECONNECT_INTERVAL_MIN 10
diff --git a/openconnect.8.in b/openconnect.8.in
index 37a33d0c..437d374f 100644
--- a/openconnect.8.in
+++ b/openconnect.8.in
@@ -57,6 +57,8 @@ openconnect \- Multi-protocol VPN client, for Cisco AnyConnect VPNs and others
.OP \-\-no\-xmlpost
.OP \-\-non\-inter
.OP \-\-passwd\-on\-stdin
+.OP \-\-use\-keychain string
+.OP \-\-save\-to\-keychain string
.OP \-\-protocol proto
.OP \-\-token\-mode mode
.OP \-\-token\-secret {secret\fR[\fI,counter\fR]|@\fIfile\fR}
@@ -426,6 +428,27 @@ Do not expect user input; exit if it is required.
.B \-\-passwd\-on\-stdin
Read password from standard input
.TP
+.B \-\-use\-keychain=ACCOUNT
+Look up Keychain to fill password form field.
+.I ACCOUNT
+is a base name of Keychain items. For example, if
+.I ACCOUNT
+is "companyvpn", it looks up Keychain item named "companyvpn:token" for
+"token" password form field.
+.TP
+.B \-\-save\-to\-keychain=NAME
+Name of password form field to be saved to Keychain.
+.I \-\-use\-keychain
+option is required.
+For example, if
+.I \-\-use\-keychain
+options's
+.I ACCOUNT
+is "companyvpn" and
+.I NAME
+is "token", it saves input value to Keychain item named "companyvpn:token" for
+"token" password form field.
+.TP
.B \-\-protocol=PROTO
Select VPN protocol
.I PROTO
--
Yoshimasa Niwa
David Woodhouse
2018-11-04 15:36:48 UTC
Permalink
Post by Yoshimasa Niwa
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
David Woodhouse
2018-11-04 17:50:50 UTC
Permalink
Post by Yoshimasa Niwa
Post by David Woodhouse
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.
Post by David Woodhouse
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?
Post by Yoshimasa Niwa
Post by David Woodhouse
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...
Nikos Mavrogiannopoulos
2018-11-14 06:07:34 UTC
Permalink
I would certainly welcome a patch on that for gnutls!
Post by Yoshimasa Niwa
Post by David Woodhouse
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.
Post by David Woodhouse
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.
Post by David Woodhouse
FWIW what I'd *really* like to see is SSL certificate support using
the
Post by David Woodhouse
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.
Y
--
Yoshimasa Niwa
_______________________________________________
openconnect-devel mailing list
http://lists.infradead.org/mailman/listinfo/openconnect-devel
--
Sent from my mobile. Please excuse my brevity.
Continue reading on narkive:
Loading...