You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2012/03/30 19:12:37 UTC
svn commit: r1307538 - in /subversion/trunk: build.conf
subversion/libsvn_subr/crypto.c subversion/libsvn_subr/crypto.h
subversion/svn/main.c subversion/tests/libsvn_subr/
subversion/tests/libsvn_subr/crypto-test.c
Author: cmpilato
Date: Fri Mar 30 17:12:36 2012
New Revision: 1307538
URL: http://svn.apache.org/viewvc?rev=1307538&view=rev
Log:
For issue #4145 ("Master passphrase and encrypted credentials cache"),
begin adding support for some cryptographic routines in Subversion.
NOTE: The code thus far is no where near complete, but I want to get
this into version control sooner rather than later.
* subversion/libsvn_subr/crypto.h,
* subversion/libsvn_subr/crypto.c
New files, with incomplete and as-yet-unused functions in them.
* subversion/svn/main.c
(crypto_init): New function.
(main): Call crypto_init().
* build.conf
(crypto-test): New section (for a new test binary, also added to
other relevant bits of this configuration file.
* subversion/tests/libsvn_subr
Add 'crypto-test' to svn:ignores.
* subversion/tests/libsvn_subr/crypto-test.c
New skeletal test file.
Added:
subversion/trunk/subversion/libsvn_subr/crypto.c (with props)
subversion/trunk/subversion/libsvn_subr/crypto.h (with props)
subversion/trunk/subversion/tests/libsvn_subr/crypto-test.c (with props)
Modified:
subversion/trunk/build.conf
subversion/trunk/subversion/svn/main.c
subversion/trunk/subversion/tests/libsvn_subr/ (props changed)
Modified: subversion/trunk/build.conf
URL: http://svn.apache.org/viewvc/subversion/trunk/build.conf?rev=1307538&r1=1307537&r2=1307538&view=diff
==============================================================================
--- subversion/trunk/build.conf (original)
+++ subversion/trunk/build.conf Fri Mar 30 17:12:36 2012
@@ -744,6 +744,14 @@ sources = config-test.c
install = test
libs = libsvn_test libsvn_subr apriconv apr
+[crypto-test]
+description = Test svn_crypto utilities
+type = exe
+path = subversion/tests/libsvn_subr
+sources = crypto-test.c
+install = test
+libs = libsvn_test libsvn_subr apr
+
[dirent_uri-test]
description = Test dirent_uri library
type = exe
@@ -1133,7 +1141,7 @@ libs = __ALL__
strings-reps-test changes-test locks-test repos-test
checksum-test compat-test config-test hashdump-test mergeinfo-test
opt-test path-test stream-test string-test time-test utf-test
- target-test error-test cache-test spillbuf-test
+ target-test error-test cache-test spillbuf-test crypto-test
revision-test
subst_translate-test
translate-test
Added: subversion/trunk/subversion/libsvn_subr/crypto.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/crypto.c?rev=1307538&view=auto
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/crypto.c (added)
+++ subversion/trunk/subversion/libsvn_subr/crypto.c Fri Mar 30 17:12:36 2012
@@ -0,0 +1,193 @@
+/*
+ * crypto.c : cryptographic routines
+ *
+ * ====================================================================
+ * 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.
+ * ====================================================================
+ */
+
+#ifdef APU_HAVE_CRYPTO
+
+#include "crypto.h"
+
+#include <apr_random.h>
+
+
+/* If APU_ERR is non-NULL, create and return a Subversion error using
+ APR_ERR and APU_ERR. */
+static svn_error_t *
+err_from_apu_err(apr_status_t apr_err,
+ const apu_err_t *apu_err)
+{
+ if (apu_err)
+ return svn_error_createf(apr_err, NULL,
+ _("code (%d), reason (\"%s\"), msg (\"%s\")"),
+ apu_err->rc,
+ apu_err->reason ? apu_err->reason : "",
+ apu_err->msg ? apu_err->msg : "",
+ return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+get_random_bytes(void **rand_bytes,
+ apr_size_t rand_len,
+ apr_pool_t *pool)
+{
+ apr_random_t *apr_rand;
+ apr_status_t apr_err;
+
+ *rand_bytes = apr_palloc(pool, rand_len);
+ apr_rand = apr_random_standard_new(pool);
+ apr_err = apr_random_secure_bytes(apr_rand, *rand_bytes, rand_len);
+ if (apr_err != APR_SUCCESS)
+ return svn_error_create(apr_err, NULL, _("Error obtaining random data"));
+
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_crypto__context_create(apr_crypto_t **crypto_ctx,
+ apr_pool_t *pool)
+{
+ apr_status_t apr_err;
+ const apu_err_t *apu_err = NULL;
+ const apr_crypto_driver_t *driver;
+
+ /* ### TODO: So much for abstraction. APR's wrappings around NSS
+ and OpenSSL aren't quite as opaque as I'd hoped, requiring us
+ to specify a driver type and then params to the driver. We
+ *could* use APU_CRYPTO_RECOMMENDED_DRIVER for the driver bit,
+ but we'd still have to then dynamically ask APR which driver
+ it used and then figure out the parameters to send to that
+ driver at apr_crypto_make() time. Or maybe I'm just
+ overlooking something... -- cmpilato */
+
+ apr_err = apr_crypto_get_driver(&driver, "openssl", NULL, &apu_err, pool);
+ if ((apr_err != APR_SUCCESS) || (! driver))
+ return svn_error_createf(apr_err,
+ err_from_apu_err(apr_err, apu_err),
+ _("OpenSSL crypto driver error"));
+
+ apr_err = apr_crypto_make(crypto_ctx, driver, "engine=openssl", pool);
+ if ((apr_err != APR_SUCCESS) || (! *crypto_ctx))
+ return svn_error_create(apr_err, NULL,
+ _("Error creating OpenSSL crypto context"));
+
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_crypto__encrypt_cstring(unsigned char **ciphertext,
+ apr_size_t *ciphertext_len,
+ const unsigned char **iv,
+ apr_size_t *iv_len,
+ const unsigned char **salt,
+ apr_size_t *salt_len,
+ apr_crypto_t *crypto_ctx,
+ const char *plaintext,
+ const char *secret,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+ svn_error_t *err = SVN_NO_ERROR;
+ apr_crypto_key_t *key = NULL;
+ const apu_err_t *apu_err = NULL;
+ apr_status_t apr_err;
+ const unsigned char *prefix;
+ apr_crypto_block_t *block_ctx = NULL;
+ apr_size_t block_size = 0, encrypted_len = 0;
+
+ SVN_ERR_ASSERT(crypto_ctx);
+
+ /* Generate the salt. */
+ *salt_len = 8;
+ SVN_ERR(get_random_bytes((void **)salt, *salt_len, result_pool));
+
+ /* Initialize the passphrase. */
+ apr_err = apr_crypto_passphrase(&key, NULL, secret, strlen(secret),
+ *salt, 8 /* salt_len */,
+ APR_KEY_AES_256, APR_MODE_CBC,
+ 1 /* doPad */, 4096, crypto_ctx,
+ scratch_pool);
+ if (apr_err != APR_SUCCESS)
+ {
+ apr_crypto_error(&apu_err, crypto_ctx);
+ return svn_error_create(apr_err, err_from_apu_err(apr_err, apu_err),
+ _("Error creating derived key"));
+ }
+
+ /* Generate a 4-byte prefix. */
+ SVN_ERR(get_random_bytes((void **)&prefix, 4, scratch_pool));
+
+ /* Initialize block encryption. */
+ apr_err = apr_crypto_block_encrypt_init(&block_ctx, iv, key, &block_size,
+ result_pool);
+ if ((apr_err != APR_SUCCESS) || (! block_ctx))
+ {
+ apr_crypto_error(&apu_err, crypto_ctx);
+ return svn_error_create(apr_err, err_from_apu_err(apr_err, apu_err),
+ _("Error initializing block encryption"));
+ }
+
+ /* ### FIXME: We need to actually use the prefix! */
+
+ /* Encrypt the block. */
+ apr_err = apr_crypto_block_encrypt(ciphertext, ciphertext_len,
+ (unsigned char *)plaintext,
+ strlen(plaintext) + 1, block_ctx);
+ if (apr_err != APR_SUCCESS)
+ {
+ apr_crypto_error(&apu_err, crypto_ctx);
+ err = svn_error_create(apr_err, err_from_apu_err(apr_err, apu_err),
+ _("Error encrypting block"));
+ goto cleanup;
+ }
+
+ /* Finalize the block encryption. */
+ apr_err = apr_crypto_block_encrypt_finish(*ciphertext + *ciphertext_len,
+ &encrypted_len, block_ctx);
+ if (apr_err != APR_SUCCESS)
+ {
+ apr_crypto_error(&apu_err, crypto_ctx);
+ err = svn_error_create(apr_err, err_from_apu_err(apr_err, apu_err),
+ _("Error finalizing block encryption"));
+ goto cleanup;
+ }
+
+ cleanup:
+ apr_crypto_block_cleanup(block_ctx);
+ return err;
+}
+
+svn_error_t *
+svn_crypto__decrypt_cstring(const svn_string_t **plaintext,
+ apr_crypto_t *crypto_ctx,
+ const unsigned char *ciphertext,
+ apr_size_t ciphertext_len,
+ const unsigned char *iv,
+ apr_size_t iv_len,
+ const unsigned char *salt,
+ apr_size_t salt_len,
+ const svn_string_t *secret,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+ return svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, NULL, NULL);
+}
+
+#endif /* APU_HAVE_CRYPTO */
Propchange: subversion/trunk/subversion/libsvn_subr/crypto.c
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: subversion/trunk/subversion/libsvn_subr/crypto.c
------------------------------------------------------------------------------
svn:mime-type = text/x-csrc
Added: subversion/trunk/subversion/libsvn_subr/crypto.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/crypto.h?rev=1307538&view=auto
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/crypto.h (added)
+++ subversion/trunk/subversion/libsvn_subr/crypto.h Fri Mar 30 17:12:36 2012
@@ -0,0 +1,69 @@
+/*
+ * crypto.h : cryptographic routines
+ *
+ * ====================================================================
+ * 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.
+ * ====================================================================
+ */
+
+#ifndef SVN_LIBSVN_SUBR_CRYPTO_H
+#define SVN_LIBSVN_SUBR_CRYPTO_H
+
+#include <apr.h>
+#include <apr_crypto.h>
+
+#ifdef APU_HAVE_CRYPTO
+
+/* Set *CRYPTO_CTX to an APR-managed OpenSSL cryptography context
+ object allocated from POOL. */
+/* ### TODO: Should this be something done once at apr_crypto_init()
+ ### time, with the apr_crypto_t object stored in, perhaps,
+ ### Subversion's svn_client_ctx_t? */
+svn_error_t *
+svn_crypto__context_create(apr_crypto_t **crypto_ctx,
+ apr_pool_t *pool);
+
+
+svn_error_t *
+svn_crypto__encrypt_cstring(unsigned char **ciphertext,
+ apr_size_t *ciphertext_len,
+ const unsigned char **iv,
+ apr_size_t *iv_len,
+ const unsigned char **salt,
+ apr_size_t *salt_len,
+ apr_crypto_t *crypto_ctx,
+ const char *plaintext,
+ const char *secret,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+svn_error_t *
+svn_crypto__decrypt_cstring(const svn_string_t **plaintext,
+ apr_crypto_t *crypto_ctx,
+ const unsigned char *ciphertext,
+ apr_size_t ciphertext_len,
+ const unsigned char *iv,
+ apr_size_t iv_len,
+ const unsigned char *salt,
+ apr_size_t salt_len,
+ const svn_string_t *secret,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+#endif /* APU_HAVE_CRYPTO */
+#endif /* SVN_CRYPTO_H */
Propchange: subversion/trunk/subversion/libsvn_subr/crypto.h
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: subversion/trunk/subversion/libsvn_subr/crypto.h
------------------------------------------------------------------------------
svn:mime-type = text/x-chdr
Modified: subversion/trunk/subversion/svn/main.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/main.c?rev=1307538&r1=1307537&r2=1307538&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/main.c (original)
+++ subversion/trunk/subversion/svn/main.c Fri Mar 30 17:12:36 2012
@@ -34,6 +34,7 @@
#include <apr_tables.h>
#include <apr_general.h>
#include <apr_signal.h>
+#include <apr_crypto.h>
#include "svn_cmdline.h"
#include "svn_pools.h"
@@ -1535,6 +1536,21 @@ svn_cl__check_cancel(void *baton)
return SVN_NO_ERROR;
}
+/* Initialize the APR cryptography subsystem (if available), using
+ POOL for the registration of cleanups, shutdowns, etc. */
+static svn_error_t *
+crypto_init(apr_pool_t *pool)
+{
+#ifdef APU_HAVE_CRYPTO
+ apr_status_t apr_err = apr_crypto_init(pool);
+ if (apr_err)
+ return svn_error_wrap_apr(apr_err,
+ _("Failed to initialize cryptography subsystem"));
+#endif /* APU_HAVE_CRYPTO */
+ return SVN_NO_ERROR;
+}
+
+
/*** Main. ***/
@@ -1592,6 +1608,11 @@ main(int argc, const char *argv[])
}
#endif
+ /* Initialize the cryptography subsystem. */
+ err = crypto_init(pool);
+ if (err)
+ return svn_cmdline_handle_exit_error(err, pool, "svn: ");
+
/* Initialize the RA library. */
err = svn_ra_initialize(pool);
if (err)
Propchange: subversion/trunk/subversion/tests/libsvn_subr/
------------------------------------------------------------------------------
--- svn:ignore (original)
+++ svn:ignore Fri Mar 30 17:12:36 2012
@@ -11,6 +11,7 @@ Release
checksum-test
compat-test
config-test
+crypto-test
error-test
hashdump-test
hashdump.out
Added: subversion/trunk/subversion/tests/libsvn_subr/crypto-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/crypto-test.c?rev=1307538&view=auto
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/crypto-test.c (added)
+++ subversion/trunk/subversion/tests/libsvn_subr/crypto-test.c Fri Mar 30 17:12:36 2012
@@ -0,0 +1,55 @@
+/*
+ * crypto-test.c -- test cryptographic routines
+ *
+ * ====================================================================
+ * 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.
+ * ====================================================================
+ */
+
+#include <stdio.h>
+#include <string.h>
+
+#include "../svn_test.h"
+#include "../../libsvn_subr/crypto.h"
+
+#ifdef APU_HAVE_CRYPTO
+
+static svn_error_t *
+test_encrypt_decrypt(apr_pool_t *pool)
+{
+ /* ### TODO: Anything! */
+
+ return SVN_NO_ERROR;
+}
+
+
+
+#endif /* APU_HAVE_CRYPTO */
+
+
+/* The test table. */
+
+struct svn_test_descriptor_t test_funcs[] =
+ {
+ SVN_TEST_NULL,
+#ifdef APU_HAVE_CRYPTO
+ SVN_TEST_PASS2(test_encrypt_decrypt,
+ "basic encryption/decryption test"),
+#endif /* APU_HAVE_CRYPTO */
+ SVN_TEST_NULL
+ };
Propchange: subversion/trunk/subversion/tests/libsvn_subr/crypto-test.c
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: subversion/trunk/subversion/tests/libsvn_subr/crypto-test.c
------------------------------------------------------------------------------
svn:mime-type = text/x-csrc
Re: svn commit: r1307538 - in /subversion/trunk: build.conf
subversion/libsvn_subr/crypto.c subversion/libsvn_subr/crypto.h
subversion/svn/main.c subversion/tests/libsvn_subr/
subversion/tests/libsvn_subr/crypto-test.c
Posted by Daniel Shahaf <da...@elego.de>.
cmpilato@apache.org wrote on Fri, Mar 30, 2012 at 17:12:37 -0000:
> Author: cmpilato
> Date: Fri Mar 30 17:12:36 2012
> New Revision: 1307538
>
> URL: http://svn.apache.org/viewvc?rev=1307538&view=rev
> Log:
> For issue #4145 ("Master passphrase and encrypted credentials cache"),
> begin adding support for some cryptographic routines in Subversion.
>
> NOTE: The code thus far is no where near complete, but I want to get
> this into version control sooner rather than later.
>
> * subversion/libsvn_subr/crypto.h,
> * subversion/libsvn_subr/crypto.c
> New files, with incomplete and as-yet-unused functions in them.
>
> * subversion/svn/main.c
> (crypto_init): New function.
> (main): Call crypto_init().
>
> * build.conf
> (crypto-test): New section (for a new test binary, also added to
> other relevant bits of this configuration file.
>
> * subversion/tests/libsvn_subr
> Add 'crypto-test' to svn:ignores.
>
> * subversion/tests/libsvn_subr/crypto-test.c
> New skeletal test file.
>
> Added: subversion/trunk/subversion/libsvn_subr/crypto.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/crypto.c?rev=1307538&view=auto
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/crypto.c (added)
> +++ subversion/trunk/subversion/libsvn_subr/crypto.c Fri Mar 30 17:12:36 2012
> @@ -0,0 +1,193 @@
> +/*
> + * crypto.c : cryptographic routines
> + *
> + * ====================================================================
> + * 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.
> + * ====================================================================
> + */
> +
> +#ifdef APU_HAVE_CRYPTO
> +
> +#include "crypto.h"
> +
> +#include <apr_random.h>
> +
> +
> +static svn_error_t *
> +get_random_bytes(void **rand_bytes,
> + apr_size_t rand_len,
> + apr_pool_t *pool)
> +{
> + apr_random_t *apr_rand;
Shouldn't you reuse this context as much as possible? (instead of
creating a new context every time)
> + apr_status_t apr_err;
> +
> + *rand_bytes = apr_palloc(pool, rand_len);
> + apr_rand = apr_random_standard_new(pool);
Can this call block? I assume it can't, but APR docs aren't explicit.
Do you need to call apr_random_add_entropy() somewhere? From code
inspection the code will just error out if that function hasn't been
called (that function is the only codepath that sets 'g->secure_started'
to '1').
> + apr_err = apr_random_secure_bytes(apr_rand, *rand_bytes, rand_len);
> + if (apr_err != APR_SUCCESS)
> + return svn_error_create(apr_err, NULL, _("Error obtaining random data"));
> +
> + return SVN_NO_ERROR;
> +}
> +
> +svn_error_t *
> +svn_crypto__context_create(apr_crypto_t **crypto_ctx,
> + apr_pool_t *pool)
> +{
> + apr_status_t apr_err;
> + const apu_err_t *apu_err = NULL;
> + const apr_crypto_driver_t *driver;
> +
> + /* ### TODO: So much for abstraction. APR's wrappings around NSS
> + and OpenSSL aren't quite as opaque as I'd hoped, requiring us
> + to specify a driver type and then params to the driver. We
> + *could* use APU_CRYPTO_RECOMMENDED_DRIVER for the driver bit,
> + but we'd still have to then dynamically ask APR which driver
> + it used and then figure out the parameters to send to that
> + driver at apr_crypto_make() time. Or maybe I'm just
> + overlooking something... -- cmpilato */
> +
> + apr_err = apr_crypto_get_driver(&driver, "openssl", NULL, &apu_err, pool);
> + if ((apr_err != APR_SUCCESS) || (! driver))
> + return svn_error_createf(apr_err,
> + err_from_apu_err(apr_err, apu_err),
If the local variable APR_ERR has value APR_SUCCESS, you'll be creating
here an svn_error_t object whose APR_ERR member is APR_SUCCESS. Perhaps
return a more useful error code when (driver == NULL && ! apr_err)?
> + _("OpenSSL crypto driver error"));
> +
> + apr_err = apr_crypto_make(crypto_ctx, driver, "engine=openssl", pool);
> + if ((apr_err != APR_SUCCESS) || (! *crypto_ctx))
> + return svn_error_create(apr_err, NULL,
> + _("Error creating OpenSSL crypto context"));
> +
> + return SVN_NO_ERROR;
> +}
> +
> +svn_error_t *
> +svn_crypto__encrypt_cstring(unsigned char **ciphertext,
> + apr_size_t *ciphertext_len,
svn_string_t **ciphertext ?
> + const unsigned char **iv,
> + apr_size_t *iv_len,
> + const unsigned char **salt,
> + apr_size_t *salt_len,
> + apr_crypto_t *crypto_ctx,
> + const char *plaintext,
> + const char *secret,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool)
> +{
> + svn_error_t *err = SVN_NO_ERROR;
> + apr_crypto_key_t *key = NULL;
> + const apu_err_t *apu_err = NULL;
Could move this variable to block scope.
> + apr_status_t apr_err;
> + const unsigned char *prefix;
> + apr_crypto_block_t *block_ctx = NULL;
> + apr_size_t block_size = 0, encrypted_len = 0;
> +
> + SVN_ERR_ASSERT(crypto_ctx);
> +
> + /* Generate the salt. */
> + *salt_len = 8;
> + SVN_ERR(get_random_bytes((void **)salt, *salt_len, result_pool));
> +
> + /* Initialize the passphrase. */
> + apr_err = apr_crypto_passphrase(&key, NULL, secret, strlen(secret),
> + *salt, 8 /* salt_len */,
> + APR_KEY_AES_256, APR_MODE_CBC,
> + 1 /* doPad */, 4096, crypto_ctx,
> + scratch_pool);
> + if (apr_err != APR_SUCCESS)
> + {
> + apr_crypto_error(&apu_err, crypto_ctx);
Need to check the return value of apr_crypto_error() before
dereferencing APU_ERR in err_from_apu_err())
> + return svn_error_create(apr_err, err_from_apu_err(apr_err, apu_err),
> + _("Error creating derived key"));
> + }
> +
> + /* Generate a 4-byte prefix. */
> + SVN_ERR(get_random_bytes((void **)&prefix, 4, scratch_pool));
> +
> + /* Initialize block encryption. */
> + apr_err = apr_crypto_block_encrypt_init(&block_ctx, iv, key, &block_size,
> + result_pool);
> + if ((apr_err != APR_SUCCESS) || (! block_ctx))
> + {
> + apr_crypto_error(&apu_err, crypto_ctx);
> + return svn_error_create(apr_err, err_from_apu_err(apr_err, apu_err),
> + _("Error initializing block encryption"));
> + }
> +
> + /* ### FIXME: We need to actually use the prefix! */
Also, need to avoid adding 1 to strlen() below if it's a multiple of 16.
> +
> + /* Encrypt the block. */
> + apr_err = apr_crypto_block_encrypt(ciphertext, ciphertext_len,
> + (unsigned char *)plaintext,
> + strlen(plaintext) + 1, block_ctx);
> + if (apr_err != APR_SUCCESS)
> + {
> + apr_crypto_error(&apu_err, crypto_ctx);
> + err = svn_error_create(apr_err, err_from_apu_err(apr_err, apu_err),
> + _("Error encrypting block"));
> + goto cleanup;
> + }
> +
> + /* Finalize the block encryption. */
> + apr_err = apr_crypto_block_encrypt_finish(*ciphertext + *ciphertext_len,
> + &encrypted_len, block_ctx);
> + if (apr_err != APR_SUCCESS)
> + {
> + apr_crypto_error(&apu_err, crypto_ctx);
> + err = svn_error_create(apr_err, err_from_apu_err(apr_err, apu_err),
> + _("Error finalizing block encryption"));
> + goto cleanup;
> + }
> +
> + cleanup:
> + apr_crypto_block_cleanup(block_ctx);
> + return err;
> +}
Re: svn commit: r1307538 - in /subversion/trunk: build.conf
subversion/libsvn_subr/crypto.c subversion/libsvn_subr/crypto.h
subversion/svn/main.c subversion/tests/libsvn_subr/
subversion/tests/libsvn_subr/crypto-test.c
Posted by Daniel Shahaf <da...@elego.de>.
cmpilato@apache.org wrote on Fri, Mar 30, 2012 at 17:12:37 -0000:
> Author: cmpilato
> Date: Fri Mar 30 17:12:36 2012
> New Revision: 1307538
>
> URL: http://svn.apache.org/viewvc?rev=1307538&view=rev
> Log:
> For issue #4145 ("Master passphrase and encrypted credentials cache"),
> begin adding support for some cryptographic routines in Subversion.
>
> NOTE: The code thus far is no where near complete, but I want to get
> this into version control sooner rather than later.
>
> * subversion/libsvn_subr/crypto.h,
> * subversion/libsvn_subr/crypto.c
> New files, with incomplete and as-yet-unused functions in them.
>
> * subversion/svn/main.c
> (crypto_init): New function.
> (main): Call crypto_init().
>
> * build.conf
> (crypto-test): New section (for a new test binary, also added to
> other relevant bits of this configuration file.
>
> * subversion/tests/libsvn_subr
> Add 'crypto-test' to svn:ignores.
>
> * subversion/tests/libsvn_subr/crypto-test.c
> New skeletal test file.
>
> Added: subversion/trunk/subversion/libsvn_subr/crypto.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/crypto.c?rev=1307538&view=auto
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/crypto.c (added)
> +++ subversion/trunk/subversion/libsvn_subr/crypto.c Fri Mar 30 17:12:36 2012
> @@ -0,0 +1,193 @@
> +/*
> + * crypto.c : cryptographic routines
> + *
> + * ====================================================================
> + * 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.
> + * ====================================================================
> + */
> +
> +#ifdef APU_HAVE_CRYPTO
> +
> +#include "crypto.h"
> +
> +#include <apr_random.h>
> +
> +
> +static svn_error_t *
> +get_random_bytes(void **rand_bytes,
> + apr_size_t rand_len,
> + apr_pool_t *pool)
> +{
> + apr_random_t *apr_rand;
Shouldn't you reuse this context as much as possible? (instead of
creating a new context every time)
> + apr_status_t apr_err;
> +
> + *rand_bytes = apr_palloc(pool, rand_len);
> + apr_rand = apr_random_standard_new(pool);
Can this call block? I assume it can't, but APR docs aren't explicit.
Do you need to call apr_random_add_entropy() somewhere? From code
inspection the code will just error out if that function hasn't been
called (that function is the only codepath that sets 'g->secure_started'
to '1').
> + apr_err = apr_random_secure_bytes(apr_rand, *rand_bytes, rand_len);
> + if (apr_err != APR_SUCCESS)
> + return svn_error_create(apr_err, NULL, _("Error obtaining random data"));
> +
> + return SVN_NO_ERROR;
> +}
> +
> +svn_error_t *
> +svn_crypto__context_create(apr_crypto_t **crypto_ctx,
> + apr_pool_t *pool)
> +{
> + apr_status_t apr_err;
> + const apu_err_t *apu_err = NULL;
> + const apr_crypto_driver_t *driver;
> +
> + /* ### TODO: So much for abstraction. APR's wrappings around NSS
> + and OpenSSL aren't quite as opaque as I'd hoped, requiring us
> + to specify a driver type and then params to the driver. We
> + *could* use APU_CRYPTO_RECOMMENDED_DRIVER for the driver bit,
> + but we'd still have to then dynamically ask APR which driver
> + it used and then figure out the parameters to send to that
> + driver at apr_crypto_make() time. Or maybe I'm just
> + overlooking something... -- cmpilato */
> +
> + apr_err = apr_crypto_get_driver(&driver, "openssl", NULL, &apu_err, pool);
> + if ((apr_err != APR_SUCCESS) || (! driver))
> + return svn_error_createf(apr_err,
> + err_from_apu_err(apr_err, apu_err),
If the local variable APR_ERR has value APR_SUCCESS, you'll be creating
here an svn_error_t object whose APR_ERR member is APR_SUCCESS. Perhaps
return a more useful error code when (driver == NULL && ! apr_err)?
> + _("OpenSSL crypto driver error"));
> +
> + apr_err = apr_crypto_make(crypto_ctx, driver, "engine=openssl", pool);
> + if ((apr_err != APR_SUCCESS) || (! *crypto_ctx))
> + return svn_error_create(apr_err, NULL,
> + _("Error creating OpenSSL crypto context"));
> +
> + return SVN_NO_ERROR;
> +}
> +
> +svn_error_t *
> +svn_crypto__encrypt_cstring(unsigned char **ciphertext,
> + apr_size_t *ciphertext_len,
svn_string_t **ciphertext ?
> + const unsigned char **iv,
> + apr_size_t *iv_len,
> + const unsigned char **salt,
> + apr_size_t *salt_len,
> + apr_crypto_t *crypto_ctx,
> + const char *plaintext,
> + const char *secret,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool)
> +{
> + svn_error_t *err = SVN_NO_ERROR;
> + apr_crypto_key_t *key = NULL;
> + const apu_err_t *apu_err = NULL;
Could move this variable to block scope.
> + apr_status_t apr_err;
> + const unsigned char *prefix;
> + apr_crypto_block_t *block_ctx = NULL;
> + apr_size_t block_size = 0, encrypted_len = 0;
> +
> + SVN_ERR_ASSERT(crypto_ctx);
> +
> + /* Generate the salt. */
> + *salt_len = 8;
> + SVN_ERR(get_random_bytes((void **)salt, *salt_len, result_pool));
> +
> + /* Initialize the passphrase. */
> + apr_err = apr_crypto_passphrase(&key, NULL, secret, strlen(secret),
> + *salt, 8 /* salt_len */,
> + APR_KEY_AES_256, APR_MODE_CBC,
> + 1 /* doPad */, 4096, crypto_ctx,
> + scratch_pool);
> + if (apr_err != APR_SUCCESS)
> + {
> + apr_crypto_error(&apu_err, crypto_ctx);
Need to check the return value of apr_crypto_error() before
dereferencing APU_ERR in err_from_apu_err())
> + return svn_error_create(apr_err, err_from_apu_err(apr_err, apu_err),
> + _("Error creating derived key"));
> + }
> +
> + /* Generate a 4-byte prefix. */
> + SVN_ERR(get_random_bytes((void **)&prefix, 4, scratch_pool));
> +
> + /* Initialize block encryption. */
> + apr_err = apr_crypto_block_encrypt_init(&block_ctx, iv, key, &block_size,
> + result_pool);
> + if ((apr_err != APR_SUCCESS) || (! block_ctx))
> + {
> + apr_crypto_error(&apu_err, crypto_ctx);
> + return svn_error_create(apr_err, err_from_apu_err(apr_err, apu_err),
> + _("Error initializing block encryption"));
> + }
> +
> + /* ### FIXME: We need to actually use the prefix! */
Also, need to avoid adding 1 to strlen() below if it's a multiple of 16.
> +
> + /* Encrypt the block. */
> + apr_err = apr_crypto_block_encrypt(ciphertext, ciphertext_len,
> + (unsigned char *)plaintext,
> + strlen(plaintext) + 1, block_ctx);
> + if (apr_err != APR_SUCCESS)
> + {
> + apr_crypto_error(&apu_err, crypto_ctx);
> + err = svn_error_create(apr_err, err_from_apu_err(apr_err, apu_err),
> + _("Error encrypting block"));
> + goto cleanup;
> + }
> +
> + /* Finalize the block encryption. */
> + apr_err = apr_crypto_block_encrypt_finish(*ciphertext + *ciphertext_len,
> + &encrypted_len, block_ctx);
> + if (apr_err != APR_SUCCESS)
> + {
> + apr_crypto_error(&apu_err, crypto_ctx);
> + err = svn_error_create(apr_err, err_from_apu_err(apr_err, apu_err),
> + _("Error finalizing block encryption"));
> + goto cleanup;
> + }
> +
> + cleanup:
> + apr_crypto_block_cleanup(block_ctx);
> + return err;
> +}