You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2010/10/06 16:41:35 UTC
svn commit: r1005065 - in /subversion/branches/gpg-agent-password-store: ./
build/generator/ subversion/include/ subversion/include/private/
subversion/libsvn_auth_gpg_agent/ subversion/libsvn_subr/
Author: stylesen
Date: Wed Oct 6 14:41:35 2010
New Revision: 1005065
URL: http://svn.apache.org/viewvc?rev=1005065&view=rev
Log:
On the 'gpg-agent-password-store' branch:
Add GPG-AGENT support for Subversion password caching.
* Makefile.in
Include support for building libsvn_auth_gpg_agent.
* build.conf
Include support for building libsvn_auth_gpg_agent.
* configure.ac
Include support for configuring a build to include gpg-agent
support.
* build/generator/extractor.py
Add gpg-agent auth related functions.
* subversion/libsvn_subr/auth.c
Add gpg-agent to the list of password store types supported.
* subversion/libsvn_subr/simple_providers.c
(svn_auth__simple_save_creds_helper): Check for gpg-agent encrypted
password store.
* subversion/include/private/svn_auth_private.h
Add password type #define for gpg-agent type.
* subversion/include/svn_auth.h
(svn_auth_gpg_agent_version,
svn_auth_get_gpg_agent_simple_provider): Add new function
prototypes.
* subversion/libsvn_auth_gpg_agent/version.c
Provide version information for libsvn_auth_gpg_agent.
* subversion/libsvn_auth_gpg_agent/gpg_agent.c
Password handling functions for gpg-agent password store.
Added:
subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/
subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/gpg_agent.c
subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c
Modified:
subversion/branches/gpg-agent-password-store/Makefile.in
subversion/branches/gpg-agent-password-store/build.conf
subversion/branches/gpg-agent-password-store/build/generator/extractor.py
subversion/branches/gpg-agent-password-store/configure.ac
subversion/branches/gpg-agent-password-store/subversion/include/private/svn_auth_private.h
subversion/branches/gpg-agent-password-store/subversion/include/svn_auth.h
subversion/branches/gpg-agent-password-store/subversion/libsvn_subr/auth.c
subversion/branches/gpg-agent-password-store/subversion/libsvn_subr/simple_providers.c
Modified: subversion/branches/gpg-agent-password-store/Makefile.in
URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/Makefile.in?rev=1005065&r1=1005064&r2=1005065&view=diff
==============================================================================
--- subversion/branches/gpg-agent-password-store/Makefile.in (original)
+++ subversion/branches/gpg-agent-password-store/Makefile.in Wed Oct 6 14:41:35 2010
@@ -47,6 +47,7 @@ SVN_APR_LIBS = @SVN_APR_LIBS@
SVN_APRUTIL_LIBS = @SVN_APRUTIL_LIBS@
SVN_APR_MEMCACHE_LIBS = @SVN_APR_MEMCACHE_LIBS@
SVN_DB_LIBS = @SVN_DB_LIBS@
+SVN_GPG_AGENT_LIBS = @SVN_GPG_AGENT_LIBS@
SVN_GNOME_KEYRING_LIBS = @SVN_GNOME_KEYRING_LIBS@
SVN_GSSAPI_LIBS = @SVN_GSSAPI_LIBS@
SVN_KWALLET_LIBS = @SVN_KWALLET_LIBS@
@@ -65,6 +66,7 @@ fsmod_libdir = @libdir@
ramod_libdir = @libdir@
bdb_libdir = @libdir@
gnome_keyring_libdir = @libdir@
+gpg_agent_libdir = @libdir@
gssapi_libdir = @libdir@
kwallet_libdir = @libdir@
neon_libdir = @libdir@
@@ -242,6 +244,7 @@ INSTALL_FSMOD_LIB = $(INSTALL_LIB)
INSTALL_RAMOD_LIB = $(INSTALL_LIB)
INSTALL_APR_MEMCACHE_LIB = $(INSTALL_LIB)
INSTALL_BDB_LIB = $(INSTALL_LIB)
+INSTALL_GPG_AGENT_LIB = $(INSTALL_LIB)
INSTALL_GNOME_KEYRING_LIB = $(INSTALL_LIB)
INSTALL_GSSAPI_LIB = $(INSTALL_LIB)
INSTALL_KWALLET_LIB = $(INSTALL_LIB)
Modified: subversion/branches/gpg-agent-password-store/build.conf
URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/build.conf?rev=1005065&r1=1005064&r2=1005065&view=diff
==============================================================================
--- subversion/branches/gpg-agent-password-store/build.conf (original)
+++ subversion/branches/gpg-agent-password-store/build.conf Wed Oct 6 14:41:35 2010
@@ -175,6 +175,14 @@ libs = libsvn_client libsvn_ra libsvn_re
install = bin
manpages = subversion/svnrdump/svnrdump.1
+# Support for GPG Agent
+[libsvn_auth_gpg_agent]]
+description = Subversion GPG Agent library
+type = lib
+install = gpg-agent-lib
+path = subversion/libsvn_auth_gpg_agent
+libs = apr libsvn_subr
+
# Support for GNOME Keyring
[libsvn_auth_gnome_keyring]
description = Subversion GNOME Keyring Library
Modified: subversion/branches/gpg-agent-password-store/build/generator/extractor.py
URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/build/generator/extractor.py?rev=1005065&r1=1005064&r2=1005065&view=diff
==============================================================================
--- subversion/branches/gpg-agent-password-store/build/generator/extractor.py (original)
+++ subversion/branches/gpg-agent-password-store/build/generator/extractor.py Wed Oct 6 14:41:35 2010
@@ -53,8 +53,10 @@ _filter_names = [
'svn_auth_get_gnome_keyring_ssl_client_cert_pw_provider',
'svn_auth_get_kwallet_simple_provider',
'svn_auth_get_kwallet_ssl_client_cert_pw_provider',
+ 'svn_auth_get_gpg_agent_simple_provider',
'svn_auth_gnome_keyring_version',
'svn_auth_kwallet_version',
+ 'svn_auth_gpg_agent_version',
]
if __name__ == '__main__':
Modified: subversion/branches/gpg-agent-password-store/configure.ac
URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/configure.ac?rev=1005065&r1=1005064&r2=1005065&view=diff
==============================================================================
--- subversion/branches/gpg-agent-password-store/configure.ac (original)
+++ subversion/branches/gpg-agent-password-store/configure.ac Wed Oct 6 14:41:35 2010
@@ -548,6 +548,37 @@ int main()
fi
fi
+dnl GPG Agent -------------------
+
+AC_ARG_WITH(gpg_agent,
+ AS_HELP_STRING([--with-gpg-agent],
+ [Enable use of GPG AGENT for auth credentials]),
+ [with_gpg_agent="$withval"],
+ [with_gpg_agent=no])
+
+AC_MSG_CHECKING([whether to look for GPG AGENT])
+if test "$with_gpg_agent" != "no"; then
+ AC_MSG_RESULT([yes])
+ if test "$svn_enable_shared" = "yes"; then
+ if test "$APR_HAS_DSO" = "yes"; then
+ if test "$with_gpg_agent" = "yes"; then
+ AC_MSG_RESULT([yes])
+ AC_DEFINE([SVN_HAVE_GPG_AGENT], [1],
+ [Is GPG Agent support enabled?])
+ SVN_GPG_AGENT_LIBS="-lgcrypt11"
+ fi
+ else
+ AC_MSG_ERROR([APR does not have support for DSOs])
+ fi
+ else
+ AC_MSG_ERROR([--with-gpg-agent conflicts with --disable-shared])
+ fi
+else
+ AC_MSG_RESULT([no])
+fi
+
+AC_SUBST(SVN_GPG_AGENT_LIBS)
+
dnl GNOME Keyring -------------------
AC_ARG_WITH(gnome_keyring,
@@ -736,6 +767,12 @@ if test "$with_gnome_keyring" = "yes"; t
INSTALL_STATIC_RULES="$INSTALL_STATIC_RULES install-gnome-keyring-lib"
fi
+if test "$with_gpg_agent" = "yes"; then
+ BUILD_RULES="$BUILD_RULES gpg-agent-lib"
+ INSTALL_RULES="`echo $INSTALL_RULES | sed 's/install-lib/install-lib install-gpg-agent-lib/'`"
+ INSTALL_STATIC_RULES="$INSTALL_STATIC_RULES install-gpg-agent-lib"
+fi
+
if test "$USE_NLS" = "yes"; then
BUILD_RULES="$BUILD_RULES locale"
INSTALL_RULES="$INSTALL_RULES install-locale"
Modified: subversion/branches/gpg-agent-password-store/subversion/include/private/svn_auth_private.h
URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/subversion/include/private/svn_auth_private.h?rev=1005065&r1=1005064&r2=1005065&view=diff
==============================================================================
--- subversion/branches/gpg-agent-password-store/subversion/include/private/svn_auth_private.h (original)
+++ subversion/branches/gpg-agent-password-store/subversion/include/private/svn_auth_private.h Wed Oct 6 14:41:35 2010
@@ -46,6 +46,7 @@ extern "C" {
#define SVN_AUTH__KEYCHAIN_PASSWORD_TYPE "keychain"
#define SVN_AUTH__KWALLET_PASSWORD_TYPE "kwallet"
#define SVN_AUTH__GNOME_KEYRING_PASSWORD_TYPE "gnome-keyring"
+#define SVN_AUTH__GPG_AGENT_PASSWORD_TYPE "gpg-agent"
/* A function that stores in *PASSWORD (potentially after decrypting it)
the user's password. It might be obtained directly from CREDS, or
Modified: subversion/branches/gpg-agent-password-store/subversion/include/svn_auth.h
URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/subversion/include/svn_auth.h?rev=1005065&r1=1005064&r2=1005065&view=diff
==============================================================================
--- subversion/branches/gpg-agent-password-store/subversion/include/svn_auth.h (original)
+++ subversion/branches/gpg-agent-password-store/subversion/include/svn_auth.h Wed Oct 6 14:41:35 2010
@@ -1070,6 +1070,35 @@ void
svn_auth_get_kwallet_ssl_client_cert_pw_provider(
svn_auth_provider_object_t **provider,
apr_pool_t *pool);
+
+
+/**
+ * Get libsvn_auth_gpg_agent version information.
+ *
+ */
+const svn_version_t *
+svn_auth_gpg_agent_version(void);
+
+
+/**
+ * Set @a *provider to an authentication provider of type @c
+ * svn_auth_cred_simple_t that gets/sets information from the user's
+ * ~/.subversion configuration directory.
+ *
+ * This is like svn_client_get_simple_provider(), except that the
+ * password is obtained from gpg_agent, which will keep it in
+ * a memory cache.
+ *
+ * Allocate @a *provider in @a pool.
+ *
+ * @since New in 1.7
+ * @note This function actually works only on systems with
+ * libsvn_auth_gpg_agent and GNU Privacy Guard installed.
+ */
+void
+svn_auth_get_gpg_agent_simple_provider
+ (svn_auth_provider_object_t **provider,
+ apr_pool_t *pool);
#endif /* (!DARWIN && !WIN32) || DOXYGEN */
Added: subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/gpg_agent.c
URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/gpg_agent.c?rev=1005065&view=auto
==============================================================================
--- subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/gpg_agent.c (added)
+++ subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/gpg_agent.c Wed Oct 6 14:41:35 2010
@@ -0,0 +1,248 @@
+/*
+ * gpg_agent.c: GPG Agent provider for SVN_AUTH_CRED_*
+ *
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ */
+
+/* ==================================================================== */
+
+
+
+/*** Includes. ***/
+
+#include <unistd.h>
+
+#include <sys/socket.h>
+#include <sys/un.h>
+
+#include <apr_pools.h>
+#include "svn_auth.h"
+#include "svn_config.h"
+#include "svn_error.h"
+#include "svn_pools.h"
+#include "svn_cmdline.h"
+#include "svn_checksum.h"
+#include "svn_string.h"
+
+#include "private/svn_auth_private.h"
+
+#include "svn_private_config.h"
+
+static const int buffer_size = 1024;
+
+/* Implementation of password_get_t that retrieves the password
+ from gpg-agent */
+static svn_boolean_t
+password_get_gpg_agent(const char **password,
+ apr_hash_t *creds,
+ const char *realmstring,
+ const char *username,
+ apr_hash_t *parameters,
+ svn_boolean_t non_interactive,
+ apr_pool_t *pool)
+{
+ int sd;
+ char *gpg_agent_info = NULL;
+ char *value;
+ char *p = NULL;
+ char *ep = NULL;
+ char *buffer;
+
+ apr_array_header_t *socket_details;
+ char *request = NULL;
+ const char *cache_id = NULL;
+ struct sockaddr_un addr;
+ int recvd;
+ char *tty_name;
+ char *tty_type;
+ const char *socket_name = NULL;
+ svn_checksum_t *digest = NULL;
+
+ value = getenv( "GPG_AGENT_INFO");
+
+ if (value != NULL)
+ {
+ gpg_agent_info = apr_pstrmemdup(pool, value, strlen(value));
+ socket_details = svn_cstring_split(gpg_agent_info, ":", TRUE, pool);
+ socket_name = APR_ARRAY_IDX(socket_details, 0, const char *);
+ }
+ else
+ return FALSE;
+
+ value = getenv("GPG_TTY");
+ if (value != NULL)
+ tty_name = apr_pstrmemdup(pool, value, strlen(value));
+ else
+ return FALSE;
+
+ value = getenv("TERM");
+ if (value != NULL)
+ tty_type = apr_pstrmemdup(pool, value, strlen(value));
+ else
+ return FALSE;
+
+ if (socket_name != NULL)
+ {
+ addr.sun_family = AF_UNIX;
+ strncpy(addr.sun_path, socket_name, 108);
+ sd = socket(AF_UNIX, SOCK_STREAM, 0);
+ if (sd == -1)
+ return FALSE;
+
+ if (connect(sd, (struct sockaddr *)&addr, sizeof(addr)) == -1)
+ {
+ close(sd);
+ return FALSE;
+ }
+ }
+ else
+ return FALSE;
+
+ /* Receive the connection status from the gpg-agent daemon. */
+ buffer = apr_palloc(pool, buffer_size);
+ recvd = recv(sd, buffer, buffer_size-1, 0);
+ buffer[recvd] = '\0';
+
+ if (strncmp(buffer, "OK", 2) != 0)
+ return FALSE;
+
+ /* Send TTY_NAME to the gpg-agent daemon. */
+ request = apr_psprintf(pool, "OPTION ttyname=%s\n", tty_name);
+ send(sd, request, strlen(request), 0);
+ recvd = recv(sd, buffer, buffer_size - 1, 0);
+ buffer[recvd] = '\0';
+
+ if (strncmp(buffer, "OK", 2) != 0)
+ return FALSE;
+
+ /* Send TTY_TYPE to the gpg-agent daemon. */
+ request = apr_psprintf(pool, "OPTION ttytype=%s\n", tty_type);
+ send(sd, request, strlen(request), 0);
+ recvd = recv(sd, buffer, buffer_size - 1, 0);
+ buffer[recvd] = '\0';
+
+ if (strncmp(buffer, "OK", 2) != 0)
+ return FALSE;
+
+ /* Create the CACHE_ID which will be generated based on REALMSTRING similar
+ to other password caching mechanisms. */
+ digest = svn_checksum_create(svn_checksum_md5, pool);
+ svn_checksum(&digest, svn_checksum_md5, realmstring, strlen(realmstring),
+ pool);
+ cache_id = svn_checksum_to_cstring(digest, pool);
+
+ if (non_interactive)
+ request = apr_psprintf(pool,
+ "GET_PASSPHRASE --data --no-ask %s X Password: \n",
+ cache_id);
+ else
+ request = apr_psprintf(pool,
+ "GET_PASSPHRASE --data %s X Password: \n",
+ cache_id);
+
+ send(sd, request, strlen(request)+1, 0);
+ recvd = recv(sd, buffer, buffer_size - 1, 0);
+ buffer[recvd] = '\0';
+
+ if (strncmp(buffer, "ERR", 3) == 0)
+ return FALSE;
+
+ if (strncmp(buffer, "D", 1) == 0)
+ p = &buffer[2];
+
+ ep = strchr(p, '\n');
+ if (ep != NULL)
+ *ep = '\0';
+
+ *password = apr_pstrmemdup(pool, p, recvd);
+
+ close(sd);
+ return TRUE;
+}
+
+/* Implementation of password_set_t that stores the password in
+ GPG Agent. */
+static svn_boolean_t
+password_set_gpg_agent(apr_hash_t *creds,
+ const char *realmstring,
+ const char *username,
+ const char *password,
+ apr_hash_t *parameters,
+ svn_boolean_t non_interactive,
+ apr_pool_t *pool)
+{
+ return TRUE;
+}
+
+
+static svn_error_t *
+simple_gpg_agent_first_creds(void **credentials,
+ void **iter_baton,
+ void *provider_baton,
+ apr_hash_t *parameters,
+ const char *realmstring,
+ apr_pool_t *pool)
+{
+ return svn_auth__simple_first_creds_helper
+ (credentials,
+ iter_baton, provider_baton,
+ parameters, realmstring,
+ password_get_gpg_agent,
+ SVN_AUTH__GPG_AGENT_PASSWORD_TYPE,
+ pool);
+}
+
+
+/* Save encrypted credentials to the simple provider's cache. */
+static svn_error_t *
+simple_gpg_agent_save_creds(svn_boolean_t *saved,
+ void *credentials,
+ void *provider_baton,
+ apr_hash_t *parameters,
+ const char *realmstring,
+ apr_pool_t *pool)
+{
+ return svn_auth__simple_save_creds_helper
+ (saved, credentials,
+ provider_baton, parameters,
+ realmstring,
+ password_set_gpg_agent,
+ SVN_AUTH__GPG_AGENT_PASSWORD_TYPE,
+ pool);
+}
+
+static const svn_auth_provider_t gpg_agent_simple_provider = {
+ SVN_AUTH_CRED_SIMPLE,
+ simple_gpg_agent_first_creds,
+ NULL,
+ simple_gpg_agent_save_creds
+};
+
+/* Public API */
+void
+svn_auth_get_gpg_agent_simple_provider
+ (svn_auth_provider_object_t **provider,
+ apr_pool_t *pool)
+{
+ svn_auth_provider_object_t *po = apr_pcalloc(pool, sizeof(*po));
+
+ po->vtable = &gpg_agent_simple_provider;
+ *provider = po;
+}
Added: subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c
URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c?rev=1005065&view=auto
==============================================================================
--- subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c (added)
+++ subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c Wed Oct 6 14:41:35 2010
@@ -0,0 +1,30 @@
+/*
+ * version.c: libsvn_auth_gpg_agent version number
+ *
+ * ====================================================================
+ * Copyright (c) 2008 CollabNet. All rights reserved.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at http://subversion.tigris.org/license-1.html.
+ * If newer versions of this license are posted there, you may use a
+ * newer version instead, at your option.
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals. For exact contribution history, see the revision
+ * history and logs, available at http://subversion.tigris.org/.
+ * ====================================================================
+ */
+
+/* ==================================================================== */
+
+
+
+#include "svn_auth.h"
+#include "svn_version.h"
+
+const svn_version_t *
+svn_auth_gpg_agent_version(void)
+{
+ SVN_VERSION_BODY;
+}
Modified: subversion/branches/gpg-agent-password-store/subversion/libsvn_subr/auth.c
URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/subversion/libsvn_subr/auth.c?rev=1005065&r1=1005064&r2=1005065&view=diff
==============================================================================
--- subversion/branches/gpg-agent-password-store/subversion/libsvn_subr/auth.c (original)
+++ subversion/branches/gpg-agent-password-store/subversion/libsvn_subr/auth.c Wed Oct 6 14:41:35 2010
@@ -390,9 +390,11 @@ svn_auth_get_platform_specific_provider
*provider = NULL;
if (apr_strnatcmp(provider_name, "gnome_keyring") == 0 ||
- apr_strnatcmp(provider_name, "kwallet") == 0)
+ apr_strnatcmp(provider_name, "kwallet") == 0 ||
+ apr_strnatcmp(provider_name, "gpg_agent") == 0)
{
-#if defined(SVN_HAVE_GNOME_KEYRING) || defined(SVN_HAVE_KWALLET)
+#if defined(SVN_HAVE_GNOME_KEYRING) || defined(SVN_HAVE_KWALLET) || \
+defined(SVN_HAVE_GPG_AGENT)
apr_dso_handle_t *dso;
apr_dso_handle_sym_t provider_function_symbol, version_function_symbol;
const char *library_label, *library_name;
@@ -494,15 +496,17 @@ svn_auth_get_platform_specific_client_pr
if (config)
{
- svn_config_get(config,
- &password_stores_config_option,
- SVN_CONFIG_SECTION_AUTH,
- SVN_CONFIG_OPTION_PASSWORD_STORES,
- "gnome-keyring,kwallet,keychain,windows-cryptoapi");
+ svn_config_get
+ (config,
+ &password_stores_config_option,
+ SVN_CONFIG_SECTION_AUTH,
+ SVN_CONFIG_OPTION_PASSWORD_STORES,
+ "gpg-agent,gnome-keyring,kwallet,keychain,windows-cryptoapi");
}
else
{
- password_stores_config_option = "gnome-keyring,kwallet,keychain,windows-cryptoapi";
+ password_stores_config_option =
+ "gpg-agent,gnome-keyring,kwallet,keychain,windows-cryptoapi";
}
*providers = apr_array_make(pool, 12, sizeof(svn_auth_provider_object_t *));
@@ -538,6 +542,20 @@ svn_auth_get_platform_specific_client_pr
continue;
}
+ /* GPG-AGENT Keyring */
+ if (apr_strnatcmp(password_store, "gpg-agent") == 0)
+ {
+ SVN_ERR(svn_auth_get_platform_specific_provider(&provider,
+ "gpg_agent",
+ "simple",
+ pool));
+
+ if (provider)
+ APR_ARRAY_PUSH(*providers, svn_auth_provider_object_t *) = provider;
+
+ continue;
+ }
+
/* KWallet */
if (apr_strnatcmp(password_store, "kwallet") == 0)
{
Modified: subversion/branches/gpg-agent-password-store/subversion/libsvn_subr/simple_providers.c
URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/subversion/libsvn_subr/simple_providers.c?rev=1005065&r1=1005064&r2=1005065&view=diff
==============================================================================
--- subversion/branches/gpg-agent-password-store/subversion/libsvn_subr/simple_providers.c (original)
+++ subversion/branches/gpg-agent-password-store/subversion/libsvn_subr/simple_providers.c Wed Oct 6 14:41:35 2010
@@ -357,7 +357,8 @@ svn_auth__simple_save_creds_helper(svn_b
(strcmp(passtype, SVN_AUTH__WINCRYPT_PASSWORD_TYPE) == 0
|| strcmp(passtype, SVN_AUTH__KEYCHAIN_PASSWORD_TYPE) == 0
|| strcmp(passtype, SVN_AUTH__KWALLET_PASSWORD_TYPE) == 0
- || strcmp(passtype, SVN_AUTH__GNOME_KEYRING_PASSWORD_TYPE) == 0) )
+ || strcmp(passtype, SVN_AUTH__GNOME_KEYRING_PASSWORD_TYPE) == 0
+ || strcmp(passtype, SVN_AUTH__GPG_AGENT_PASSWORD_TYPE) == 0))
{
may_save_password = TRUE;
}
Re: svn commit: r1005065 - in /subversion/branches/gpg-agent-password-store:
./ build/generator/ subversion/include/ subversion/include/private/ subversion/libsvn_auth_gpg_agent/
subversion/libsvn_subr/
Posted by Senthil Kumaran S <se...@collab.net>.
Hi Stefan,
Stefan Sperling wrote:
> On Wed, Oct 06, 2010 at 02:41:35PM -0000, stylesen@apache.org wrote:
>> Author: stylesen
>> Date: Wed Oct 6 14:41:35 2010
>> New Revision: 1005065
>>
>> URL: http://svn.apache.org/viewvc?rev=1005065&view=rev
>> Log:
>> On the 'gpg-agent-password-store' branch:
>>
>> Add GPG-AGENT support for Subversion password caching.
>
>
> Hi Senthil,
>
> some review inline below.
Thanks for the review. Made the necessary changes in r1005764.
>> + /* Create the CACHE_ID which will be generated based on REALMSTRING similar
>> + to other password caching mechanisms. */
>> + digest = svn_checksum_create(svn_checksum_md5, pool);
>> + svn_checksum(&digest, svn_checksum_md5, realmstring, strlen(realmstring),
>> + pool);
>
> Maybe use SHA1 instead?
I didn't make this change because the filename inside 'svn.simple' matches with
the 'CACHE_ID' now. I want the users to have some easy way to map it with their
realmstring, since we already use MD5 for this purpose I do not want to
introduce a variant using SHA1 which may confuse the users.
Thank You.
--
Senthil Kumaran S
http://www.stylesen.org/
Re: svn commit: r1005065 - in
/subversion/branches/gpg-agent-password-store: ./ build/generator/
subversion/include/ subversion/include/private/
subversion/libsvn_auth_gpg_agent/ subversion/libsvn_subr/
Posted by Peter Samuelson <pe...@p12n.org>.
[Stefan Sperling]
> > + /* Create the CACHE_ID which will be generated based on REALMSTRING similar
> > + to other password caching mechanisms. */
> > + digest = svn_checksum_create(svn_checksum_md5, pool);
> > + svn_checksum(&digest, svn_checksum_md5, realmstring, strlen(realmstring),
> > + pool);
>
> Maybe use SHA1 instead?
Why? All you're really after here is a unique key in a trusted datastore.
Peter
Re: svn commit: r1005065 - in
/subversion/branches/gpg-agent-password-store: ./ build/generator/
subversion/include/ subversion/include/private/
subversion/libsvn_auth_gpg_agent/ subversion/libsvn_subr/
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Oct 06, 2010 at 02:41:35PM -0000, stylesen@apache.org wrote:
> Author: stylesen
> Date: Wed Oct 6 14:41:35 2010
> New Revision: 1005065
>
> URL: http://svn.apache.org/viewvc?rev=1005065&view=rev
> Log:
> On the 'gpg-agent-password-store' branch:
>
> Add GPG-AGENT support for Subversion password caching.
Hi Senthil,
some review inline below.
> Added: subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/gpg_agent.c
> URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/gpg_agent.c?rev=1005065&view=auto
> ==============================================================================
> --- subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/gpg_agent.c (added)
> +++ subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/gpg_agent.c Wed Oct 6 14:41:35 2010
> @@ -0,0 +1,248 @@
> +/*
> + * gpg_agent.c: GPG Agent provider for SVN_AUTH_CRED_*
> + *
> + * ====================================================================
> + * Licensed to the Apache Software Foundation (ASF) under one
> + * or more contributor license agreements. See the NOTICE file
> + * distributed with this work for additional information
> + * regarding copyright ownership. The ASF licenses this file
> + * to you under the Apache License, Version 2.0 (the
> + * "License"); you may not use this file except in compliance
> + * with the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> + * software distributed under the License is distributed on an
> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + * KIND, either express or implied. See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + * ====================================================================
> + */
> +
> +/* ==================================================================== */
> +
> +
> +
> +/*** Includes. ***/
> +
> +#include <unistd.h>
> +
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +
> +#include <apr_pools.h>
> +#include "svn_auth.h"
> +#include "svn_config.h"
> +#include "svn_error.h"
> +#include "svn_pools.h"
> +#include "svn_cmdline.h"
> +#include "svn_checksum.h"
> +#include "svn_string.h"
> +
> +#include "private/svn_auth_private.h"
> +
> +#include "svn_private_config.h"
> +
> +static const int buffer_size = 1024;
Please write the constant in upper case: BUFFER_SIZE
And maybe use a #define instead?
> +
> +/* Implementation of password_get_t that retrieves the password
> + from gpg-agent */
> +static svn_boolean_t
> +password_get_gpg_agent(const char **password,
> + apr_hash_t *creds,
> + const char *realmstring,
> + const char *username,
> + apr_hash_t *parameters,
> + svn_boolean_t non_interactive,
> + apr_pool_t *pool)
> +{
> + int sd;
> + char *gpg_agent_info = NULL;
> + char *value;
> + char *p = NULL;
> + char *ep = NULL;
> + char *buffer;
> +
> + apr_array_header_t *socket_details;
> + char *request = NULL;
> + const char *cache_id = NULL;
> + struct sockaddr_un addr;
> + int recvd;
> + char *tty_name;
> + char *tty_type;
> + const char *socket_name = NULL;
> + svn_checksum_t *digest = NULL;
> +
> + value = getenv( "GPG_AGENT_INFO");
> +
> + if (value != NULL)
> + {
> + gpg_agent_info = apr_pstrmemdup(pool, value, strlen(value));
> + socket_details = svn_cstring_split(gpg_agent_info, ":", TRUE, pool);
> + socket_name = APR_ARRAY_IDX(socket_details, 0, const char *);
> + }
> + else
> + return FALSE;
> +
> + value = getenv("GPG_TTY");
> + if (value != NULL)
> + tty_name = apr_pstrmemdup(pool, value, strlen(value));
> + else
> + return FALSE;
> +
> + value = getenv("TERM");
> + if (value != NULL)
> + tty_type = apr_pstrmemdup(pool, value, strlen(value));
> + else
> + return FALSE;
> +
> + if (socket_name != NULL)
> + {
> + addr.sun_family = AF_UNIX;
> + strncpy(addr.sun_path, socket_name, 108);
Using 108 is dangerous, because the buffer isn't guaranteed to be 109 bytes
long. E.g. on OpenBSD it's 104. In this case, we could overrun the buffer.
On Linux it's 108, and because strncpy() does not nul-terminate the
string if the nul doesn't fit, the above risks a non-terminated string.
So please use (sizeof(addr.sun_path) - 1) instead (- 1 to account for '\0').
This seems to be common practice, and aparrently sun_path is always
defined as an array rather than a pointer.
See also http://mail-index.netbsd.org/tech-kern/1997/02/25/0014.html
which explains historical reasons for the different buffer sizes between
systems.
Also, since strncpy() does not guarantee to nul-terminate the buffer,
please add:
addr.sun_path[sizeof(addr.sun_path) - 1] = '\0';
> + sd = socket(AF_UNIX, SOCK_STREAM, 0);
> + if (sd == -1)
> + return FALSE;
> +
> + if (connect(sd, (struct sockaddr *)&addr, sizeof(addr)) == -1)
> + {
> + close(sd);
> + return FALSE;
> + }
> + }
> + else
> + return FALSE;
> +
> + /* Receive the connection status from the gpg-agent daemon. */
> + buffer = apr_palloc(pool, buffer_size);
> + recvd = recv(sd, buffer, buffer_size-1, 0);
Spaces around binary operators:
recvd = recv(sd, buffer, buffer_size - 1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "OK", 2) != 0)
> + return FALSE;
> +
> + /* Send TTY_NAME to the gpg-agent daemon. */
> + request = apr_psprintf(pool, "OPTION ttyname=%s\n", tty_name);
> + send(sd, request, strlen(request), 0);
> + recvd = recv(sd, buffer, buffer_size - 1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "OK", 2) != 0)
> + return FALSE;
> +
> + /* Send TTY_TYPE to the gpg-agent daemon. */
> + request = apr_psprintf(pool, "OPTION ttytype=%s\n", tty_type);
> + send(sd, request, strlen(request), 0);
> + recvd = recv(sd, buffer, buffer_size - 1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "OK", 2) != 0)
> + return FALSE;
> +
> + /* Create the CACHE_ID which will be generated based on REALMSTRING similar
> + to other password caching mechanisms. */
> + digest = svn_checksum_create(svn_checksum_md5, pool);
> + svn_checksum(&digest, svn_checksum_md5, realmstring, strlen(realmstring),
> + pool);
Maybe use SHA1 instead?
The rest looks fine.
Thanks,
Stefan
Re: svn commit: r1005065 - in /subversion/branches/gpg-agent-password-store:
./ build/generator/ subversion/include/ subversion/include/private/ subversion/libsvn_auth_gpg_agent/
subversion/libsvn_subr/
Posted by Senthil Kumaran S <se...@collab.net>.
Hi Philip,
Thanks for the review.
Philip Martin wrote:
>> +
>> + if (socket_name != NULL)
>> + {
>> + addr.sun_family = AF_UNIX;
>> + strncpy(addr.sun_path, socket_name, 108);
>> + sd = socket(AF_UNIX, SOCK_STREAM, 0);
>> + if (sd == -1)
>> + return FALSE;
>> +
>> + if (connect(sd, (struct sockaddr *)&addr, sizeof(addr)) == -1)
>> + {
>> + close(sd);
>> + return FALSE;
>> + }
>> + }
>> + else
>> + return FALSE;
>
> Use APR's socket interface?
Will explore this. I was thinking to use the assuan library (libassuan) which I
ve reserved for future.
>> Added: subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c
>> URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c?rev=1005065&view=auto
>> ==============================================================================
>> --- subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c (added)
>> +++ subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c Wed Oct 6 14:41:35 2010
>> @@ -0,0 +1,30 @@
>> +/*
>> + * version.c: libsvn_auth_gpg_agent version number
>> + *
>> + * ====================================================================
>> + * Copyright (c) 2008 CollabNet. All rights reserved.
>> + *
>> + * This software is licensed as described in the file COPYING, which
>> + * you should have received as part of this distribution. The terms
>> + * are also available at http://subversion.tigris.org/license-1.html.
>> + * If newer versions of this license are posted there, you may use a
>> + * newer version instead, at your option.
>> + *
>> + * This software consists of voluntary contributions made by many
>> + * individuals. For exact contribution history, see the revision
>> + * history and logs, available at http://subversion.tigris.org/.
>> + * ====================================================================
>> + */
>
> That's the wrong file header.
I ve made this change in r1005770. Will complete the other things which you
have pointed out in a separate commit.
Thank You.
--
Senthil Kumaran S
http://www.stylesen.org/
Re: svn commit: r1005065 - in /subversion/branches/gpg-agent-password-store:
./ build/generator/ subversion/include/ subversion/include/private/ subversion/libsvn_auth_gpg_agent/
subversion/libsvn_subr/
Posted by Senthil Kumaran S <se...@collab.net>.
Hi Philip,
Philip Martin wrote:
> stylesen@apache.org writes:
>
>> Author: stylesen
>> Date: Wed Oct 6 14:41:35 2010
>> New Revision: 1005065
>
>> +static svn_boolean_t
>> +password_get_gpg_agent(const char **password,
>> + apr_hash_t *creds,
>> + const char *realmstring,
>> + const char *username,
>> + apr_hash_t *parameters,
>> + svn_boolean_t non_interactive,
>> + apr_pool_t *pool)
>> +{
>> + int sd;
>> + char *gpg_agent_info = NULL;
>> + char *value;
>> + char *p = NULL;
>> + char *ep = NULL;
>> + char *buffer;
>> +
>> + apr_array_header_t *socket_details;
>> + char *request = NULL;
>> + const char *cache_id = NULL;
>> + struct sockaddr_un addr;
>> + int recvd;
>> + char *tty_name;
>> + char *tty_type;
>> + const char *socket_name = NULL;
>> + svn_checksum_t *digest = NULL;
>> +
>> + value = getenv( "GPG_AGENT_INFO");
>> +
>> + if (value != NULL)
>> + {
>> + gpg_agent_info = apr_pstrmemdup(pool, value, strlen(value));
>> + socket_details = svn_cstring_split(gpg_agent_info, ":", TRUE, pool);
>
> No need to apr_pstrmemdup if passing to svn_cstring_split.
I ve incorporated many of the changes you mentioned, in r1005780, except for
the apr sockets and config file stuff thingy which I shall work on later.
Once again, thanks for the review :)
--
Senthil Kumaran S
http://www.stylesen.org/
Re: svn commit: r1005065 - in /subversion/branches/gpg-agent-password-store: ./ build/generator/ subversion/include/ subversion/include/private/ subversion/libsvn_auth_gpg_agent/ subversion/libsvn_subr/
Posted by Philip Martin <ph...@wandisco.com>.
stylesen@apache.org writes:
> Author: stylesen
> Date: Wed Oct 6 14:41:35 2010
> New Revision: 1005065
> +static svn_boolean_t
> +password_get_gpg_agent(const char **password,
> + apr_hash_t *creds,
> + const char *realmstring,
> + const char *username,
> + apr_hash_t *parameters,
> + svn_boolean_t non_interactive,
> + apr_pool_t *pool)
> +{
> + int sd;
> + char *gpg_agent_info = NULL;
> + char *value;
> + char *p = NULL;
> + char *ep = NULL;
> + char *buffer;
> +
> + apr_array_header_t *socket_details;
> + char *request = NULL;
> + const char *cache_id = NULL;
> + struct sockaddr_un addr;
> + int recvd;
> + char *tty_name;
> + char *tty_type;
> + const char *socket_name = NULL;
> + svn_checksum_t *digest = NULL;
> +
> + value = getenv( "GPG_AGENT_INFO");
> +
> + if (value != NULL)
> + {
> + gpg_agent_info = apr_pstrmemdup(pool, value, strlen(value));
> + socket_details = svn_cstring_split(gpg_agent_info, ":", TRUE, pool);
No need to apr_pstrmemdup if passing to svn_cstring_split.
> + socket_name = APR_ARRAY_IDX(socket_details, 0, const char *);
> + }
> + else
> + return FALSE;
> +
> + value = getenv("GPG_TTY");
> + if (value != NULL)
> + tty_name = apr_pstrmemdup(pool, value, strlen(value));
> + else
> + return FALSE;
> +
> + value = getenv("TERM");
> + if (value != NULL)
> + tty_type = apr_pstrmemdup(pool, value, strlen(value));
> + else
> + return FALSE;
Do you need to apr_pstrmemdup?
> +
> + if (socket_name != NULL)
> + {
> + addr.sun_family = AF_UNIX;
> + strncpy(addr.sun_path, socket_name, 108);
> + sd = socket(AF_UNIX, SOCK_STREAM, 0);
> + if (sd == -1)
> + return FALSE;
> +
> + if (connect(sd, (struct sockaddr *)&addr, sizeof(addr)) == -1)
> + {
> + close(sd);
> + return FALSE;
> + }
> + }
> + else
> + return FALSE;
Use APR's socket interface?
> +
> + /* Receive the connection status from the gpg-agent daemon. */
> + buffer = apr_palloc(pool, buffer_size);
> + recvd = recv(sd, buffer, buffer_size-1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "OK", 2) != 0)
> + return FALSE;
> +
> + /* Send TTY_NAME to the gpg-agent daemon. */
> + request = apr_psprintf(pool, "OPTION ttyname=%s\n", tty_name);
> + send(sd, request, strlen(request), 0);
> + recvd = recv(sd, buffer, buffer_size - 1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "OK", 2) != 0)
> + return FALSE;
> +
> + /* Send TTY_TYPE to the gpg-agent daemon. */
> + request = apr_psprintf(pool, "OPTION ttytype=%s\n", tty_type);
> + send(sd, request, strlen(request), 0);
> + recvd = recv(sd, buffer, buffer_size - 1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "OK", 2) != 0)
> + return FALSE;
> +
> + /* Create the CACHE_ID which will be generated based on REALMSTRING similar
> + to other password caching mechanisms. */
> + digest = svn_checksum_create(svn_checksum_md5, pool);
> + svn_checksum(&digest, svn_checksum_md5, realmstring, strlen(realmstring),
> + pool);
> + cache_id = svn_checksum_to_cstring(digest, pool);
> +
> + if (non_interactive)
> + request = apr_psprintf(pool,
> + "GET_PASSPHRASE --data --no-ask %s X Password: \n",
> + cache_id);
> + else
> + request = apr_psprintf(pool,
> + "GET_PASSPHRASE --data %s X Password: \n",
> + cache_id);
> +
> + send(sd, request, strlen(request)+1, 0);
> + recvd = recv(sd, buffer, buffer_size - 1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "ERR", 3) == 0)
> + return FALSE;
> +
> + if (strncmp(buffer, "D", 1) == 0)
> + p = &buffer[2];
> +
> + ep = strchr(p, '\n');
> + if (ep != NULL)
> + *ep = '\0';
> +
> + *password = apr_pstrmemdup(pool, p, recvd);
buffer, and so p, is already allocated from pool.
> +
> + close(sd);
> + return TRUE;
> Added: subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c
> URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c?rev=1005065&view=auto
> ==============================================================================
> --- subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c (added)
> +++ subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c Wed Oct 6 14:41:35 2010
> @@ -0,0 +1,30 @@
> +/*
> + * version.c: libsvn_auth_gpg_agent version number
> + *
> + * ====================================================================
> + * Copyright (c) 2008 CollabNet. All rights reserved.
> + *
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution. The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals. For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + */
That's the wrong file header.
> +
> +/* ==================================================================== */
> +
> +
> +
> +#include "svn_auth.h"
> +#include "svn_version.h"
> +
> +const svn_version_t *
> +svn_auth_gpg_agent_version(void)
> +{
> + SVN_VERSION_BODY;
> +}
You need to update the password-stores bit of the config documentation
strings in libsvn_subr/config_file.c.
--
Philip
Re: svn commit: r1005065 - in /subversion/branches/gpg-agent-password-store: ./ build/generator/ subversion/include/ subversion/include/private/ subversion/libsvn_auth_gpg_agent/ subversion/libsvn_subr/
Posted by Philip Martin <ph...@wandisco.com>.
stylesen@apache.org writes:
> Author: stylesen
> Date: Wed Oct 6 14:41:35 2010
> New Revision: 1005065
> +static svn_boolean_t
> +password_get_gpg_agent(const char **password,
> + apr_hash_t *creds,
> + const char *realmstring,
> + const char *username,
> + apr_hash_t *parameters,
> + svn_boolean_t non_interactive,
> + apr_pool_t *pool)
> +{
> + int sd;
> + char *gpg_agent_info = NULL;
> + char *value;
> + char *p = NULL;
> + char *ep = NULL;
> + char *buffer;
> +
> + apr_array_header_t *socket_details;
> + char *request = NULL;
> + const char *cache_id = NULL;
> + struct sockaddr_un addr;
> + int recvd;
> + char *tty_name;
> + char *tty_type;
> + const char *socket_name = NULL;
> + svn_checksum_t *digest = NULL;
> +
> + value = getenv( "GPG_AGENT_INFO");
> +
> + if (value != NULL)
> + {
> + gpg_agent_info = apr_pstrmemdup(pool, value, strlen(value));
> + socket_details = svn_cstring_split(gpg_agent_info, ":", TRUE, pool);
No need to apr_pstrmemdup if passing to svn_cstring_split.
> + socket_name = APR_ARRAY_IDX(socket_details, 0, const char *);
> + }
> + else
> + return FALSE;
> +
> + value = getenv("GPG_TTY");
> + if (value != NULL)
> + tty_name = apr_pstrmemdup(pool, value, strlen(value));
> + else
> + return FALSE;
> +
> + value = getenv("TERM");
> + if (value != NULL)
> + tty_type = apr_pstrmemdup(pool, value, strlen(value));
> + else
> + return FALSE;
Do you need to apr_pstrmemdup?
> +
> + if (socket_name != NULL)
> + {
> + addr.sun_family = AF_UNIX;
> + strncpy(addr.sun_path, socket_name, 108);
> + sd = socket(AF_UNIX, SOCK_STREAM, 0);
> + if (sd == -1)
> + return FALSE;
> +
> + if (connect(sd, (struct sockaddr *)&addr, sizeof(addr)) == -1)
> + {
> + close(sd);
> + return FALSE;
> + }
> + }
> + else
> + return FALSE;
Use APR's socket interface?
> +
> + /* Receive the connection status from the gpg-agent daemon. */
> + buffer = apr_palloc(pool, buffer_size);
> + recvd = recv(sd, buffer, buffer_size-1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "OK", 2) != 0)
> + return FALSE;
> +
> + /* Send TTY_NAME to the gpg-agent daemon. */
> + request = apr_psprintf(pool, "OPTION ttyname=%s\n", tty_name);
> + send(sd, request, strlen(request), 0);
> + recvd = recv(sd, buffer, buffer_size - 1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "OK", 2) != 0)
> + return FALSE;
> +
> + /* Send TTY_TYPE to the gpg-agent daemon. */
> + request = apr_psprintf(pool, "OPTION ttytype=%s\n", tty_type);
> + send(sd, request, strlen(request), 0);
> + recvd = recv(sd, buffer, buffer_size - 1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "OK", 2) != 0)
> + return FALSE;
> +
> + /* Create the CACHE_ID which will be generated based on REALMSTRING similar
> + to other password caching mechanisms. */
> + digest = svn_checksum_create(svn_checksum_md5, pool);
> + svn_checksum(&digest, svn_checksum_md5, realmstring, strlen(realmstring),
> + pool);
> + cache_id = svn_checksum_to_cstring(digest, pool);
> +
> + if (non_interactive)
> + request = apr_psprintf(pool,
> + "GET_PASSPHRASE --data --no-ask %s X Password: \n",
> + cache_id);
> + else
> + request = apr_psprintf(pool,
> + "GET_PASSPHRASE --data %s X Password: \n",
> + cache_id);
> +
> + send(sd, request, strlen(request)+1, 0);
> + recvd = recv(sd, buffer, buffer_size - 1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "ERR", 3) == 0)
> + return FALSE;
> +
> + if (strncmp(buffer, "D", 1) == 0)
> + p = &buffer[2];
> +
> + ep = strchr(p, '\n');
> + if (ep != NULL)
> + *ep = '\0';
> +
> + *password = apr_pstrmemdup(pool, p, recvd);
buffer, and so p, is already allocated from pool.
> +
> + close(sd);
> + return TRUE;
> Added: subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c
> URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c?rev=1005065&view=auto
> ==============================================================================
> --- subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c (added)
> +++ subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c Wed Oct 6 14:41:35 2010
> @@ -0,0 +1,30 @@
> +/*
> + * version.c: libsvn_auth_gpg_agent version number
> + *
> + * ====================================================================
> + * Copyright (c) 2008 CollabNet. All rights reserved.
> + *
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution. The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals. For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + */
That's the wrong file header.
> +
> +/* ==================================================================== */
> +
> +
> +
> +#include "svn_auth.h"
> +#include "svn_version.h"
> +
> +const svn_version_t *
> +svn_auth_gpg_agent_version(void)
> +{
> + SVN_VERSION_BODY;
> +}
You need to update the password-stores bit of the config documentation
strings in libsvn_subr/config_file.c.
--
Philip
Re: svn commit: r1005065 - in /subversion/branches/gpg-agent-password-store: ./ build/generator/ subversion/include/ subversion/include/private/ subversion/libsvn_auth_gpg_agent/ subversion/libsvn_subr/
Posted by Philip Martin <ph...@wandisco.com>.
stylesen@apache.org writes:
> Author: stylesen
> Date: Wed Oct 6 14:41:35 2010
> New Revision: 1005065
> +static svn_boolean_t
> +password_get_gpg_agent(const char **password,
> + apr_hash_t *creds,
> + const char *realmstring,
> + const char *username,
> + apr_hash_t *parameters,
> + svn_boolean_t non_interactive,
> + apr_pool_t *pool)
> +{
> + int sd;
> + char *gpg_agent_info = NULL;
> + char *value;
> + char *p = NULL;
> + char *ep = NULL;
> + char *buffer;
> +
> + apr_array_header_t *socket_details;
> + char *request = NULL;
> + const char *cache_id = NULL;
> + struct sockaddr_un addr;
> + int recvd;
> + char *tty_name;
> + char *tty_type;
> + const char *socket_name = NULL;
> + svn_checksum_t *digest = NULL;
> +
> + value = getenv( "GPG_AGENT_INFO");
> +
> + if (value != NULL)
> + {
> + gpg_agent_info = apr_pstrmemdup(pool, value, strlen(value));
> + socket_details = svn_cstring_split(gpg_agent_info, ":", TRUE, pool);
No need to apr_pstrmemdup if passing to svn_cstring_split.
> + socket_name = APR_ARRAY_IDX(socket_details, 0, const char *);
> + }
> + else
> + return FALSE;
> +
> + value = getenv("GPG_TTY");
> + if (value != NULL)
> + tty_name = apr_pstrmemdup(pool, value, strlen(value));
> + else
> + return FALSE;
> +
> + value = getenv("TERM");
> + if (value != NULL)
> + tty_type = apr_pstrmemdup(pool, value, strlen(value));
> + else
> + return FALSE;
Do you need to apr_pstrmemdup?
> +
> + if (socket_name != NULL)
> + {
> + addr.sun_family = AF_UNIX;
> + strncpy(addr.sun_path, socket_name, 108);
> + sd = socket(AF_UNIX, SOCK_STREAM, 0);
> + if (sd == -1)
> + return FALSE;
> +
> + if (connect(sd, (struct sockaddr *)&addr, sizeof(addr)) == -1)
> + {
> + close(sd);
> + return FALSE;
> + }
> + }
> + else
> + return FALSE;
Use APR's socket interface?
> +
> + /* Receive the connection status from the gpg-agent daemon. */
> + buffer = apr_palloc(pool, buffer_size);
> + recvd = recv(sd, buffer, buffer_size-1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "OK", 2) != 0)
> + return FALSE;
> +
> + /* Send TTY_NAME to the gpg-agent daemon. */
> + request = apr_psprintf(pool, "OPTION ttyname=%s\n", tty_name);
> + send(sd, request, strlen(request), 0);
> + recvd = recv(sd, buffer, buffer_size - 1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "OK", 2) != 0)
> + return FALSE;
> +
> + /* Send TTY_TYPE to the gpg-agent daemon. */
> + request = apr_psprintf(pool, "OPTION ttytype=%s\n", tty_type);
> + send(sd, request, strlen(request), 0);
> + recvd = recv(sd, buffer, buffer_size - 1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "OK", 2) != 0)
> + return FALSE;
> +
> + /* Create the CACHE_ID which will be generated based on REALMSTRING similar
> + to other password caching mechanisms. */
> + digest = svn_checksum_create(svn_checksum_md5, pool);
> + svn_checksum(&digest, svn_checksum_md5, realmstring, strlen(realmstring),
> + pool);
> + cache_id = svn_checksum_to_cstring(digest, pool);
> +
> + if (non_interactive)
> + request = apr_psprintf(pool,
> + "GET_PASSPHRASE --data --no-ask %s X Password: \n",
> + cache_id);
> + else
> + request = apr_psprintf(pool,
> + "GET_PASSPHRASE --data %s X Password: \n",
> + cache_id);
> +
> + send(sd, request, strlen(request)+1, 0);
> + recvd = recv(sd, buffer, buffer_size - 1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "ERR", 3) == 0)
> + return FALSE;
> +
> + if (strncmp(buffer, "D", 1) == 0)
> + p = &buffer[2];
> +
> + ep = strchr(p, '\n');
> + if (ep != NULL)
> + *ep = '\0';
> +
> + *password = apr_pstrmemdup(pool, p, recvd);
buffer, and so p, is already allocated from pool.
> +
> + close(sd);
> + return TRUE;
> Added: subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c
> URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c?rev=1005065&view=auto
> ==============================================================================
> --- subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c (added)
> +++ subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c Wed Oct 6 14:41:35 2010
> @@ -0,0 +1,30 @@
> +/*
> + * version.c: libsvn_auth_gpg_agent version number
> + *
> + * ====================================================================
> + * Copyright (c) 2008 CollabNet. All rights reserved.
> + *
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution. The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals. For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + */
That's the wrong file header.
> +
> +/* ==================================================================== */
> +
> +
> +
> +#include "svn_auth.h"
> +#include "svn_version.h"
> +
> +const svn_version_t *
> +svn_auth_gpg_agent_version(void)
> +{
> + SVN_VERSION_BODY;
> +}
You need to update the password-stores bit of the config documentation
strings in libsvn_subr/config_file.c.
--
Philip
Re: svn commit: r1005065 - in
/subversion/branches/gpg-agent-password-store: ./ build/generator/
subversion/include/ subversion/include/private/
subversion/libsvn_auth_gpg_agent/ subversion/libsvn_subr/
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Oct 06, 2010 at 02:41:35PM -0000, stylesen@apache.org wrote:
> Author: stylesen
> Date: Wed Oct 6 14:41:35 2010
> New Revision: 1005065
>
> URL: http://svn.apache.org/viewvc?rev=1005065&view=rev
> Log:
> On the 'gpg-agent-password-store' branch:
>
> Add GPG-AGENT support for Subversion password caching.
Hi Senthil,
some review inline below.
> Added: subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/gpg_agent.c
> URL: http://svn.apache.org/viewvc/subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/gpg_agent.c?rev=1005065&view=auto
> ==============================================================================
> --- subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/gpg_agent.c (added)
> +++ subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/gpg_agent.c Wed Oct 6 14:41:35 2010
> @@ -0,0 +1,248 @@
> +/*
> + * gpg_agent.c: GPG Agent provider for SVN_AUTH_CRED_*
> + *
> + * ====================================================================
> + * Licensed to the Apache Software Foundation (ASF) under one
> + * or more contributor license agreements. See the NOTICE file
> + * distributed with this work for additional information
> + * regarding copyright ownership. The ASF licenses this file
> + * to you under the Apache License, Version 2.0 (the
> + * "License"); you may not use this file except in compliance
> + * with the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> + * software distributed under the License is distributed on an
> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + * KIND, either express or implied. See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + * ====================================================================
> + */
> +
> +/* ==================================================================== */
> +
> +
> +
> +/*** Includes. ***/
> +
> +#include <unistd.h>
> +
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +
> +#include <apr_pools.h>
> +#include "svn_auth.h"
> +#include "svn_config.h"
> +#include "svn_error.h"
> +#include "svn_pools.h"
> +#include "svn_cmdline.h"
> +#include "svn_checksum.h"
> +#include "svn_string.h"
> +
> +#include "private/svn_auth_private.h"
> +
> +#include "svn_private_config.h"
> +
> +static const int buffer_size = 1024;
Please write the constant in upper case: BUFFER_SIZE
And maybe use a #define instead?
> +
> +/* Implementation of password_get_t that retrieves the password
> + from gpg-agent */
> +static svn_boolean_t
> +password_get_gpg_agent(const char **password,
> + apr_hash_t *creds,
> + const char *realmstring,
> + const char *username,
> + apr_hash_t *parameters,
> + svn_boolean_t non_interactive,
> + apr_pool_t *pool)
> +{
> + int sd;
> + char *gpg_agent_info = NULL;
> + char *value;
> + char *p = NULL;
> + char *ep = NULL;
> + char *buffer;
> +
> + apr_array_header_t *socket_details;
> + char *request = NULL;
> + const char *cache_id = NULL;
> + struct sockaddr_un addr;
> + int recvd;
> + char *tty_name;
> + char *tty_type;
> + const char *socket_name = NULL;
> + svn_checksum_t *digest = NULL;
> +
> + value = getenv( "GPG_AGENT_INFO");
> +
> + if (value != NULL)
> + {
> + gpg_agent_info = apr_pstrmemdup(pool, value, strlen(value));
> + socket_details = svn_cstring_split(gpg_agent_info, ":", TRUE, pool);
> + socket_name = APR_ARRAY_IDX(socket_details, 0, const char *);
> + }
> + else
> + return FALSE;
> +
> + value = getenv("GPG_TTY");
> + if (value != NULL)
> + tty_name = apr_pstrmemdup(pool, value, strlen(value));
> + else
> + return FALSE;
> +
> + value = getenv("TERM");
> + if (value != NULL)
> + tty_type = apr_pstrmemdup(pool, value, strlen(value));
> + else
> + return FALSE;
> +
> + if (socket_name != NULL)
> + {
> + addr.sun_family = AF_UNIX;
> + strncpy(addr.sun_path, socket_name, 108);
Using 108 is dangerous, because the buffer isn't guaranteed to be 109 bytes
long. E.g. on OpenBSD it's 104. In this case, we could overrun the buffer.
On Linux it's 108, and because strncpy() does not nul-terminate the
string if the nul doesn't fit, the above risks a non-terminated string.
So please use (sizeof(addr.sun_path) - 1) instead (- 1 to account for '\0').
This seems to be common practice, and aparrently sun_path is always
defined as an array rather than a pointer.
See also http://mail-index.netbsd.org/tech-kern/1997/02/25/0014.html
which explains historical reasons for the different buffer sizes between
systems.
Also, since strncpy() does not guarantee to nul-terminate the buffer,
please add:
addr.sun_path[sizeof(addr.sun_path) - 1] = '\0';
> + sd = socket(AF_UNIX, SOCK_STREAM, 0);
> + if (sd == -1)
> + return FALSE;
> +
> + if (connect(sd, (struct sockaddr *)&addr, sizeof(addr)) == -1)
> + {
> + close(sd);
> + return FALSE;
> + }
> + }
> + else
> + return FALSE;
> +
> + /* Receive the connection status from the gpg-agent daemon. */
> + buffer = apr_palloc(pool, buffer_size);
> + recvd = recv(sd, buffer, buffer_size-1, 0);
Spaces around binary operators:
recvd = recv(sd, buffer, buffer_size - 1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "OK", 2) != 0)
> + return FALSE;
> +
> + /* Send TTY_NAME to the gpg-agent daemon. */
> + request = apr_psprintf(pool, "OPTION ttyname=%s\n", tty_name);
> + send(sd, request, strlen(request), 0);
> + recvd = recv(sd, buffer, buffer_size - 1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "OK", 2) != 0)
> + return FALSE;
> +
> + /* Send TTY_TYPE to the gpg-agent daemon. */
> + request = apr_psprintf(pool, "OPTION ttytype=%s\n", tty_type);
> + send(sd, request, strlen(request), 0);
> + recvd = recv(sd, buffer, buffer_size - 1, 0);
> + buffer[recvd] = '\0';
> +
> + if (strncmp(buffer, "OK", 2) != 0)
> + return FALSE;
> +
> + /* Create the CACHE_ID which will be generated based on REALMSTRING similar
> + to other password caching mechanisms. */
> + digest = svn_checksum_create(svn_checksum_md5, pool);
> + svn_checksum(&digest, svn_checksum_md5, realmstring, strlen(realmstring),
> + pool);
Maybe use SHA1 instead?
The rest looks fine.
Thanks,
Stefan
Re: svn commit: r1005065 - in
/subversion/branches/gpg-agent-password-store: ./ build/generator/
subversion/include/ subversion/include/private/
subversion/libsvn_auth_gpg_agent/ subversion/libsvn_subr/
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Senthil Kumaran S wrote on Mon, Oct 18, 2010 at 15:35:39 +0530:
> Daniel Shahaf wrote:
>> e.g., under gvim, ':echo $TERM' shows nothing. So, is it the case that,
>> as written, gpg support will only be available to cmdline programs, but
>> not to generic library users?
>
> This is an optional argument sent to the Assuan protocol based server. If
> this does not work out it is not a problem, the socket is most important
> here.
You trimmed context, but if the variable does not exist in the
environment then you return FALSE from the password_get() function ---
that is, you say "I couldn't find a password" and the providers system
moves on to try the next provider.
So, either I'm in dire need of coffee, or yes there is a problem if the
variable does not exist.
Re: svn commit: r1005065 - in /subversion/branches/gpg-agent-password-store:
./ build/generator/ subversion/include/ subversion/include/private/ subversion/libsvn_auth_gpg_agent/
subversion/libsvn_subr/
Posted by Senthil Kumaran S <se...@collab.net>.
Daniel Shahaf wrote:
> e.g., under gvim, ':echo $TERM' shows nothing. So, is it the case that,
> as written, gpg support will only be available to cmdline programs, but
> not to generic library users?
This is an optional argument sent to the Assuan protocol based server. If this
does not work out it is not a problem, the socket is most important here.
> Another point: error checking. The calls to send(), close(),
> svn_checksum() (and maybe others) don't check the return values.
Will do it in my future works.
Thank You.
--
Senthil Kumaran S
http://www.stylesen.org/
Re: svn commit: r1005065 - in
/subversion/branches/gpg-agent-password-store: ./ build/generator/
subversion/include/ subversion/include/private/
subversion/libsvn_auth_gpg_agent/ subversion/libsvn_subr/
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stylesen@apache.org wrote on Wed, Oct 06, 2010 at 14:41:35 -0000:
> + value = getenv( "GPG_AGENT_INFO");
> +
> + if (value != NULL)
> + {
> + gpg_agent_info = apr_pstrmemdup(pool, value, strlen(value));
> + socket_details = svn_cstring_split(gpg_agent_info, ":", TRUE, pool);
> + socket_name = APR_ARRAY_IDX(socket_details, 0, const char *);
> + }
> + else
> + return FALSE;
> +
> + value = getenv("GPG_TTY");
> + if (value != NULL)
> + tty_name = apr_pstrmemdup(pool, value, strlen(value));
> + else
> + return FALSE;
> +
> + value = getenv("TERM");
> + if (value != NULL)
> + tty_type = apr_pstrmemdup(pool, value, strlen(value));
> + else
> + return FALSE;
getenv("TERM") will fail in GUI programs, won't it?
e.g., under gvim, ':echo $TERM' shows nothing. So, is it the case that,
as written, gpg support will only be available to cmdline programs, but
not to generic library users?
Another point: error checking. The calls to send(), close(),
svn_checksum() (and maybe others) don't check the return values.
Re: svn commit: r1005065 - in /subversion/branches/gpg-agent-password-store:
./ build/generator/ subversion/include/ subversion/include/private/ subversion/libsvn_auth_gpg_agent/
subversion/libsvn_subr/
Posted by Senthil Kumaran S <se...@collab.net>.
Hi Daniel,
Daniel Shahaf wrote:
> Needs "Patch by:"?
Edited the log. I missed it in my log file which I edited in a hurry :(
Thank You.
--
Senthil Kumaran S
http://www.stylesen.org/
Re: svn commit: r1005065 - in
/subversion/branches/gpg-agent-password-store: ./ build/generator/
subversion/include/ subversion/include/private/
subversion/libsvn_auth_gpg_agent/ subversion/libsvn_subr/
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Needs "Patch by:"?
stylesen@apache.org wrote on Wed, Oct 06, 2010 at 14:41:35 -0000:
> Author: stylesen
> Date: Wed Oct 6 14:41:35 2010
> New Revision: 1005065
>
> URL: http://svn.apache.org/viewvc?rev=1005065&view=rev
> Log:
> On the 'gpg-agent-password-store' branch:
>
> Add GPG-AGENT support for Subversion password caching.
>
> * Makefile.in
> Include support for building libsvn_auth_gpg_agent.
>
> * build.conf
> Include support for building libsvn_auth_gpg_agent.
>
> * configure.ac
> Include support for configuring a build to include gpg-agent
> support.
>
> * build/generator/extractor.py
> Add gpg-agent auth related functions.
>
> * subversion/libsvn_subr/auth.c
> Add gpg-agent to the list of password store types supported.
>
> * subversion/libsvn_subr/simple_providers.c
> (svn_auth__simple_save_creds_helper): Check for gpg-agent encrypted
> password store.
>
> * subversion/include/private/svn_auth_private.h
> Add password type #define for gpg-agent type.
>
> * subversion/include/svn_auth.h
> (svn_auth_gpg_agent_version,
> svn_auth_get_gpg_agent_simple_provider): Add new function
> prototypes.
>
> * subversion/libsvn_auth_gpg_agent/version.c
> Provide version information for libsvn_auth_gpg_agent.
>
> * subversion/libsvn_auth_gpg_agent/gpg_agent.c
> Password handling functions for gpg-agent password store.
>
> Added:
> subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/
> subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/gpg_agent.c
> subversion/branches/gpg-agent-password-store/subversion/libsvn_auth_gpg_agent/version.c
> Modified:
> subversion/branches/gpg-agent-password-store/Makefile.in
> subversion/branches/gpg-agent-password-store/build.conf
> subversion/branches/gpg-agent-password-store/build/generator/extractor.py
> subversion/branches/gpg-agent-password-store/configure.ac
> subversion/branches/gpg-agent-password-store/subversion/include/private/svn_auth_private.h
> subversion/branches/gpg-agent-password-store/subversion/include/svn_auth.h
> subversion/branches/gpg-agent-password-store/subversion/libsvn_subr/auth.c
> subversion/branches/gpg-agent-password-store/subversion/libsvn_subr/simple_providers.c