From: Steffan Karger <stef...@karger.me> This removes the dependency of crypto.c on misc.c, which makes testing (stuff that needs) crypto.c functionality easier. In particular, this simplifies the --tls-crypt tests in one of the follow-up patches.
Apart from that, testing file access really belongs in options_postprocess_filechecks(), and moving it there enables us to perform the same check for other private files too. v2: change indenting, remove remaining warn_if_group_others_accessible() calls and move function to options.c. Signed-off-by: Steffan Karger <steffan.kar...@fox-it.com> --- src/openvpn/crypto.c | 5 +--- src/openvpn/misc.c | 27 --------------------- src/openvpn/misc.h | 3 --- src/openvpn/options.c | 61 ++++++++++++++++++++++++++++++++++++----------- src/openvpn/ssl_mbedtls.c | 2 -- src/openvpn/ssl_openssl.c | 2 -- 6 files changed, 48 insertions(+), 52 deletions(-) diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index ab43005..05622ce 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -36,7 +36,7 @@ #include "crypto.h" #include "error.h" #include "integer.h" -#include "misc.h" +#include "platform.h" #include "memdbg.h" @@ -1307,9 +1307,6 @@ read_key_file (struct key2 *key2, const char *file, const unsigned int flags) if (!(flags & RKF_INLINE)) buf_clear (&in); - if (key2->n) - warn_if_group_others_accessible (error_filename); - #if 0 /* DEBUGGING */ { diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index 3d40f0b..ea0d75d 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -194,31 +194,6 @@ save_inetd_socket_descriptor (void) } /* - * Warn if a given file is group/others accessible. - */ -void -warn_if_group_others_accessible (const char* filename) -{ -#ifndef WIN32 -#ifdef HAVE_STAT - if (strcmp (filename, INLINE_FILE_TAG)) - { - struct stat st; - if (stat (filename, &st)) - { - msg (M_WARN | M_ERRNO, "WARNING: cannot stat file '%s'", filename); - } - else - { - if (st.st_mode & (S_IRWXG|S_IRWXO)) - msg (M_WARN, "WARNING: file '%s' is group or others accessible", filename); - } - } -#endif -#endif -} - -/* * Print an error message based on the status code returned by system(). */ const char * @@ -1115,8 +1090,6 @@ get_user_pass_cr (struct user_pass *up, FILE *fp; char password_buf[USER_PASS_LEN] = { '\0' }; - warn_if_group_others_accessible (auth_file); - fp = platform_fopen (auth_file, "r"); if (!fp) msg (M_ERR, "Error opening '%s' auth file: %s", prefix, auth_file); diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index ceda323..cc0a474 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -71,9 +71,6 @@ void run_up_down (const char *command, void write_pid (const char *filename); -/* check file protections */ -void warn_if_group_others_accessible(const char* filename); - /* system flags */ #define S_SCRIPT (1<<0) #define S_FATAL (1<<1) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index e9dc17e..17732ce 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -2689,11 +2689,37 @@ options_postprocess_mutate (struct options *o) */ #ifndef ENABLE_SMALL /** Expect people using the stripped down version to know what they do */ +/* + * Warn if a given file is group/others accessible. + */ +static void +warn_if_group_others_accessible (const char* filename) +{ +#ifndef WIN32 +#ifdef HAVE_STAT + if (strcmp (filename, INLINE_FILE_TAG)) + { + struct stat st; + if (stat (filename, &st)) + { + msg (M_WARN | M_ERRNO, "WARNING: cannot stat file '%s'", filename); + } + else + { + if (st.st_mode & (S_IRWXG|S_IRWXO)) + msg (M_WARN, "WARNING: file '%s' is group or others accessible", filename); + } + } +#endif +#endif +} + #define CHKACC_FILE (1<<0) /** Check for a file/directory precense */ #define CHKACC_DIRPATH (1<<1) /** Check for directory precense where a file should reside */ #define CHKACC_FILEXSTWR (1<<2) /** If file exists, is it writable? */ #define CHKACC_INLINE (1<<3) /** File is present if it's an inline file */ #define CHKACC_ACPTSTDIN (1<<4) /** If filename is stdin, it's allowed and "exists" */ +#define CHKACC_PRIVATE (1<<5) /** Warn if this (private) file is group/others accessible */ static bool check_file_access(const int type, const char *file, const int mode, const char *opt) @@ -2734,6 +2760,11 @@ check_file_access(const int type, const char *file, const int mode, const char * if (platform_access (file, W_OK) != 0) errcode = errno; + if (type & CHKACC_PRIVATE) + { + warn_if_group_others_accessible (file); + } + /* Scream if an error is found */ if( errcode > 0 ) msg (M_NOPREFIX|M_OPTERR, "%s fails with '%s': %s", @@ -2850,10 +2881,12 @@ options_postprocess_filechecks (struct options *options) #ifdef MANAGMENT_EXTERNAL_KEY if(!(options->management_flags & MF_EXTERNAL_KEY)) #endif - errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->priv_key_file, R_OK, - "--key"); - errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->pkcs12_file, R_OK, - "--pkcs12"); + { + errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE|CHKACC_PRIVATE, + options->priv_key_file, R_OK, "--key"); + } + errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE|CHKACC_PRIVATE, + options->pkcs12_file, R_OK, "--pkcs12"); if (options->ssl_flags & SSLF_CRL_VERIFY_DIR) errs |= check_file_access_chroot (options->chroot_dir, CHKACC_FILE, options->crl_file, R_OK|X_OK, @@ -2862,26 +2895,26 @@ options_postprocess_filechecks (struct options *options) errs |= check_file_access_chroot (options->chroot_dir, CHKACC_FILE|CHKACC_INLINE, options->crl_file, R_OK, "--crl-verify"); - errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->tls_auth_file, R_OK, - "--tls-auth"); - errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->tls_crypt_file, R_OK, - "--tls-crypt"); - errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->shared_secret_file, R_OK, - "--secret"); + errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE|CHKACC_PRIVATE, + options->tls_auth_file, R_OK, "--tls-auth"); + errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE|CHKACC_PRIVATE, + options->tls_crypt_file, R_OK, "--tls-crypt"); + errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE|CHKACC_PRIVATE, + options->shared_secret_file, R_OK, "--secret"); errs |= check_file_access (CHKACC_DIRPATH|CHKACC_FILEXSTWR, - options->packet_id_file, R_OK|W_OK, "--replay-persist"); + options->packet_id_file, R_OK|W_OK, "--replay-persist"); /* ** Password files ** */ - errs |= check_file_access (CHKACC_FILE|CHKACC_ACPTSTDIN, + errs |= check_file_access (CHKACC_FILE|CHKACC_ACPTSTDIN|CHKACC_PRIVATE, options->key_pass_file, R_OK, "--askpass"); #endif /* ENABLE_CRYPTO */ #ifdef ENABLE_MANAGEMENT - errs |= check_file_access (CHKACC_FILE|CHKACC_ACPTSTDIN, + errs |= check_file_access (CHKACC_FILE|CHKACC_ACPTSTDIN|CHKACC_PRIVATE, options->management_user_pass, R_OK, "--management user/password file"); #endif /* ENABLE_MANAGEMENT */ #if P2MP - errs |= check_file_access (CHKACC_FILE|CHKACC_ACPTSTDIN, + errs |= check_file_access (CHKACC_FILE|CHKACC_ACPTSTDIN|CHKACC_PRIVATE, options->auth_user_pass_file, R_OK, "--auth-user-pass"); #endif /* P2MP */ diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 4414f01..ea6d4c4 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -361,8 +361,6 @@ tls_ctx_load_priv_file (struct tls_root_ctx *ctx, const char *priv_key_file, return 1; } - warn_if_group_others_accessible (priv_key_file); - if (!mbed_ok(mbedtls_pk_check_pair(&ctx->crt_chain->pk, ctx->priv_key))) { msg (M_WARN, "Private key does not match the certificate"); diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 7002907..3064c20 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -582,7 +582,6 @@ tls_ctx_load_pkcs12(struct tls_root_ctx *ctx, const char *pkcs12_file, /* Load Private Key */ if (!SSL_CTX_use_PrivateKey (ctx->ctx, pkey)) crypto_msg (M_FATAL, "Cannot use private key"); - warn_if_group_others_accessible (pkcs12_file); /* Check Private Key */ if (!SSL_CTX_check_private_key (ctx->ctx)) @@ -758,7 +757,6 @@ tls_ctx_load_priv_file (struct tls_root_ctx *ctx, const char *priv_key_file, crypto_msg (M_WARN, "Cannot load private key file %s", priv_key_file); goto end; } - warn_if_group_others_accessible (priv_key_file); /* Check Private Key */ if (!SSL_CTX_check_private_key (ssl_ctx)) -- 2.7.4 ------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi _______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel