Skip to contentSkip to conversation

polarssl: improve logging

PATCH In review Steffan Karger 3 participants 5 messages Jan 6, 2016 – Jan 7, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 polarssl: improve logging

Add the functions polar_log_err(), polar_log_func_line() and a macro polar_ok(), to easily log human-readable PolarSSL errors from polarssl-specific code.

This does not provide the full logging interface as msg(), because I would have to add a lot more of macro-magic to achieve that on the various supported compilers and platforms, and this suffices too (for now at least).

Use the new polar_log_err() and polar_ok() functions to provide more log/debug output for polarssl errors.

This is commit is a combined cherry-pick of commits 6ef5df14 and d17d362d from the master branch, adjusted to the release/2.3 branch.

---
 src/openvpn/crypto_polarssl.c     | 60 ++++++++++++++++++++++++---------
 src/openvpn/crypto_polarssl.h     | 40 ++++++++++++++++++++++
 src/openvpn/ssl_polarssl.c        | 70 ++++++++++++++++++---------------------
 src/openvpn/ssl_verify_polarssl.c | 26 ++++-----------
 4 files changed, 125 insertions(+), 71 deletions(-)
src/openvpn/crypto_polarssl.c +45 -15
index 24712ed..92fdb78 100644
@@ -46,6 +46,7 @@
 #include "misc.h"
 
 #include <polarssl/des.h>
+#include <polarssl/error.h>
 #include <polarssl/md5.h>
 #include <polarssl/cipher.h>
 #include <polarssl/havege.h>
@@ -86,6 +87,32 @@ crypto_clear_error (void)
 {
 }
 
+bool polar_log_err(unsigned int flags, int errval, const char *prefix)
+{
+  if (0 != errval)
+    {
+      char errstr[256];
+      polarssl_strerror(errval, errstr, sizeof(errstr));
+
+      if (NULL == prefix) prefix = "PolarSSL error";
+      msg (flags, "%s: %s", prefix, errstr);
+    }
+
+  return 0 == errval;
+}
+
+bool polar_log_func_line(unsigned int flags, int errval, const char *func,
+    int line)
+{
+  char prefix[256];
+
+  if (!openvpn_snprintf(prefix, sizeof(prefix), "%s:%d", func, line))
+    return polar_log_err(flags, errval, func);
+
+  return polar_log_err(flags, errval, prefix);
+}
+
+
 #ifdef DMALLOC
 void
 crypto_init_dmalloc (void)
@@ -234,7 +261,8 @@ ctr_drbg_context * rand_ctx_get()
       /* Initialise PolarSSL RNG, and built-in entropy sources */
       entropy_init(&ec);
 
-      if (0 != ctr_drbg_init(&cd_ctx, entropy_func, &ec, BPTR(&pers_string), BLEN(&pers_string)))
+      if (!polar_ok(ctr_drbg_init(&cd_ctx, entropy_func, &ec,
+		    BPTR(&pers_string), BLEN(&pers_string))))
         msg (M_FATAL, "Failed to initialize random generator");
 
       gc_free(&gc);
@@ -445,10 +473,10 @@ cipher_ctx_init (cipher_context_t *ctx, uint8_t *key, int key_len,
 
   CLEAR (*ctx);
 
-  if (0 != cipher_init_ctx(ctx, kt))
+  if (!polar_ok(cipher_init_ctx(ctx, kt)))
     msg (M_FATAL, "PolarSSL cipher context init #1");
 
-  if (0 != cipher_setkey(ctx, key, key_len*8, enc))
+  if (!polar_ok(cipher_setkey(ctx, key, key_len*8, enc)))
     msg (M_FATAL, "PolarSSL cipher set key");
 
   /* make sure we used a big enough key */
@@ -487,36 +515,38 @@ cipher_ctx_get_cipher_kt (const cipher_ctx_t *ctx)
 
 int cipher_ctx_reset (cipher_context_t *ctx, uint8_t *iv_buf)
 {
-  int retval = cipher_reset(ctx);
+  if (!polar_ok(cipher_reset(ctx)))
+    return 0;
 
-  if (0 == retval)
-    retval = cipher_set_iv(ctx, iv_buf, ctx->cipher_info->iv_size);
+  if (!polar_ok(cipher_set_iv(ctx, iv_buf, ctx->cipher_info->iv_size)))
+    return 0;
 
-  return 0 == retval;
+  return 1;
 }
 
 int cipher_ctx_update (cipher_context_t *ctx, uint8_t *dst, int *dst_len,
     uint8_t *src, int src_len)
 {
-  int retval = 0;
   size_t s_dst_len = *dst_len;
 
-  retval = cipher_update(ctx, src, (size_t)src_len, dst, &s_dst_len);
+  if (!polar_ok(cipher_update(ctx, src, (size_t)src_len, dst, &s_dst_len)))
+    return 0;
 
   *dst_len = s_dst_len;
 
-  return 0 == retval;
+  return 1;
 }
 
 int cipher_ctx_final (cipher_context_t *ctx, uint8_t *dst, int *dst_len)
 {
-  int retval = 0;
   size_t s_dst_len = *dst_len;
 
-  retval = cipher_finish(ctx, dst, &s_dst_len);
+  if (!polar_ok(cipher_finish(ctx, dst, &s_dst_len)))
+    return 0;
+
   *dst_len = s_dst_len;
 
-  return 0 == retval;
+  return 1;
 }
 
 void
@@ -526,8 +556,8 @@ cipher_des_encrypt_ecb (const unsigned char key[DES_KEY_LENGTH],
 {
     des_context ctx;
 
-    des_setkey_enc(&ctx, key);
-    des_crypt_ecb(&ctx, src, dst);
+    ASSERT (polar_ok(des_setkey_enc(&ctx, key)));
+    ASSERT (polar_ok(des_crypt_ecb(&ctx, src, dst)));
 }
 
 
src/openvpn/crypto_polarssl.h +40 -0
index b6da436..3db3392 100644
@@ -91,4 +91,44 @@ ctr_drbg_context * rand_ctx_get();
 void rand_ctx_enable_prediction_resistance();
 #endif
 
+/**
+ * Log the supplied PolarSSL error, prefixed by supplied prefix.
+ *
+ * @param flags		Flags to indicate error type and priority.
+ * @param errval	PolarSSL error code to convert to error message.
+ * @param prefix	Prefix to PolarSSL error message.
+ *
+ * @returns true if no errors are detected, false otherwise.
+ */
+bool polar_log_err(unsigned int flags, int errval, const char *prefix);
+
+/**
+ * Log the supplied PolarSSL error, prefixed by function name and line number.
+ *
+ * @param flags		Flags to indicate error type and priority.
+ * @param errval	PolarSSL error code to convert to error message.
+ * @param func		Function name where error was reported.
+ * @param line		Line number where error was reported.
+ *
+ * @returns true if no errors are detected, false otherwise.
+ */
+bool polar_log_func_line(unsigned int flags, int errval, const char *func,
+    int line);
+
+/**
+ * Check errval and log on error.
+ *
+ * Convenience wrapper to put around polarssl library calls, e.g.
+ *   if (!polar_ok(polarssl_func())) return 0;
+ * or
+ *   ASSERT (polar_ok(polarssl_func()));
+ *
+ * @param errval	PolarSSL error code to convert to error message.
+ *
+ * @returns true if no errors are detected, false otherwise.
+ */
+#define polar_ok(errval) ((0 == errval) || \
+  polar_log_func_line(D_CRYPT_ERRORS, errval, __func__, __LINE__))
+
+
 #endif /* CRYPTO_POLARSSL_H_ */
src/openvpn/ssl_polarssl.c +33 -37

196 lines not on this page · raw message

src/openvpn/ssl_verify_polarssl.c +7 -19
index ac252a3..7ed87d6 100644
@@ -131,17 +131,12 @@ backend_x509_get_serial (openvpn_x509_cert_t *cert, struct gc_arena *gc)
   char *buf = NULL;
   size_t buflen = 0;
   mpi serial_mpi = { 0 };
-  int retval = 0;
 
   /* Transform asn1 integer serial into PolarSSL MPI */
   mpi_init(&serial_mpi);
-  retval = mpi_read_binary(&serial_mpi, cert->serial.p, cert->serial.len);
-  if (retval < 0)
+  if (!polar_ok(mpi_read_binary(&serial_mpi, cert->serial.p, cert->serial.len)))
     {
-      char errbuf[128];
-      error_strerror(retval, errbuf, sizeof(errbuf));
-
-      msg(M_WARN, "Failed to retrieve serial from certificate: %s.", errbuf);
+      msg(M_WARN, "Failed to retrieve serial from certificate.");
       return NULL;
     }
 
@@ -150,13 +145,9 @@ backend_x509_get_serial (openvpn_x509_cert_t *cert, struct gc_arena *gc)
   buf = gc_malloc(buflen, true, gc);
 
   /* Write MPI serial as decimal string into buffer */
-  retval = mpi_write_string(&serial_mpi, 10, buf, &buflen);
-  if (retval < 0)
+  if (!polar_ok(mpi_write_string(&serial_mpi, 10, buf, &buflen)))
     {
-      char errbuf[128];
-      error_strerror(retval, errbuf, sizeof(errbuf));
-
-      msg(M_WARN, "Failed to write serial to string: %s.", errbuf);
+      msg(M_WARN, "Failed to write serial to string.");
       return NULL;
     }
 
@@ -372,12 +363,9 @@ x509_verify_crl(const char *crl_file, x509_crt *cert, const char *subject)
   struct gc_arena gc = gc_new();
   char *serial;
 
-  int polar_retval = x509_crl_parse_file(&crl, crl_file);
-  if (polar_retval != 0)
+  if (!polar_ok(x509_crl_parse_file(&crl, crl_file)))
     {
-      char errstr[128];
-      polarssl_strerror(polar_retval, errstr, sizeof(errstr));
-      msg (M_WARN, "CRL: cannot read CRL from file %s (%s)", crl_file, errstr);
+      msg (M_WARN, "CRL: cannot read CRL from file %s", crl_file);
       goto end;
     }
 
@@ -390,7 +378,7 @@ x509_verify_crl(const char *crl_file, x509_crt *cert, const char *subject)
       goto end;
     }
 
-  if (0 != x509_crt_revoked(cert, &crl))
+  if (!polar_ok(x509_crt_revoked(cert, &crl)))
     {
       serial = backend_x509_get_serial_hex(cert, &gc);
       msg (D_HANDSHAKE, "CRL CHECK FAILED: %s (serial %s) is REVOKED", subject, (serial ? serial : "NOT AVAILABLE"));
Signature · 1 line
2.5.0
#1 lore raw

PATCH 2/2 openssl: improve logging

This improves OpenSSL logging and removes OpenSSL-specific error printing code from error.c. The crypto_msg() functions provide convenience wrappers, specific to OpenSSL. Instead of passing the magical 'M_SSLERR' flag to msg(), a developer now just calls crypto_msg() to get OpenSSL errors dumped to log.

This is commit is a combined cherry-pick of commits e795d6ba and 98ea2ec5 from the master branch, adjusted to the release/2.3 branch.

---
 src/openvpn/crypto_openssl.c |  51 +++++++++++++-------
 src/openvpn/crypto_openssl.h |  25 ++++++++++
 src/openvpn/error.c          |  29 -----------
 src/openvpn/error.h          |  11 +++--
 src/openvpn/ssl_openssl.c    | 111 ++++++++++++++++++++++++-------------------
 5 files changed, 126 insertions(+), 101 deletions(-)
src/openvpn/crypto_openssl.c +34 -17
index 4e195ce..c147245 100644
@@ -135,13 +135,15 @@ setup_engine (const char *engine)
       if ((e = ENGINE_by_id (engine)) == NULL
 	 && (e = try_load_engine (engine)) == NULL)
 	{
-	  msg (M_FATAL, "OpenSSL error: cannot load engine '%s'", engine);
+	  crypto_msg (M_FATAL, "OpenSSL error: cannot load engine '%s'",
+	      engine);
 	}
 
       if (!ENGINE_set_default (e, ENGINE_METHOD_ALL))
 	{
-	  msg (M_FATAL, "OpenSSL error: ENGINE_set_default failed on engine '%s'",
-	       engine);
+	  crypto_msg (M_FATAL,
+	      "OpenSSL error: ENGINE_set_default failed on engine '%s'",
+	      engine);
 	}
 
       msg (M_INFO, "Initializing OpenSSL support for engine '%s'",
@@ -230,6 +232,14 @@ crypto_clear_error (void)
   ERR_clear_error ();
 }
 
+void
+crypto_print_openssl_errors(const unsigned int flags) {
+  size_t err = 0;
+
+  while ((err = ERR_get_error ()))
+    msg (flags, "OpenSSL: %s", ERR_error_string (err, NULL));
+}
+
 /*
  *
  * OpenSSL memory debugging.  If dmalloc debugging is enabled, tell
@@ -380,7 +390,7 @@ int rand_bytes(uint8_t *output, int len)
 {
   if (unlikely(1 != RAND_bytes (output, len)))
     {
-      msg(D_CRYPT_ERRORS, "RAND_bytes() failed");
+      crypto_msg(D_CRYPT_ERRORS, "RAND_bytes() failed");
       return 0;
     }
   return 1;
@@ -426,17 +436,20 @@ key_des_check (uint8_t *key, int key_len, int ndc)
       DES_cblock *dc = (DES_cblock*) buf_read_alloc (&b, sizeof (DES_cblock));
       if (!dc)
 	{
-	  msg (D_CRYPT_ERRORS, "CRYPTO INFO: check_key_DES: insufficient key material");
+	  crypto_msg (D_CRYPT_ERRORS,
+	      "CRYPTO INFO: check_key_DES: insufficient key material");
 	  goto err;
 	}
       if (DES_is_weak_key(dc))
 	{
-	  msg (D_CRYPT_ERRORS, "CRYPTO INFO: check_key_DES: weak key detected");
+	  crypto_msg (D_CRYPT_ERRORS,
+	      "CRYPTO INFO: check_key_DES: weak key detected");
 	  goto err;
 	}
       if (!DES_check_key_parity (dc))
 	{
-	  msg (D_CRYPT_ERRORS, "CRYPTO INFO: check_key_DES: bad parity detected");
+	  crypto_msg (D_CRYPT_ERRORS,
+	      "CRYPTO INFO: check_key_DES: bad parity detected");
 	  goto err;
 	}
     }
@@ -485,7 +498,7 @@ cipher_kt_get (const char *ciphername)
   cipher = EVP_get_cipherbyname (ciphername);
 
   if (NULL == cipher)
-    msg (M_SSLERR, "Cipher algorithm '%s' not found", ciphername);
+    crypto_msg (M_FATAL, "Cipher algorithm '%s' not found", ciphername);
 
   if (EVP_CIPHER_key_length (cipher) > MAX_CIPHER_KEY_LENGTH)
     msg (M_FATAL, "Cipher algorithm '%s' uses a default key size (%d bytes) which is larger than " PACKAGE_NAME "'s current maximum key size (%d bytes)",
@@ -569,13 +582,13 @@ cipher_ctx_init (EVP_CIPHER_CTX *ctx, uint8_t *key, int key_len,
 
   EVP_CIPHER_CTX_init (ctx);
   if (!EVP_CipherInit (ctx, kt, NULL, NULL, enc))
-    msg (M_SSLERR, "EVP cipher init #1");
+    crypto_msg (M_FATAL, "EVP cipher init #1");
 #ifdef HAVE_EVP_CIPHER_CTX_SET_KEY_LENGTH
   if (!EVP_CIPHER_CTX_set_key_length (ctx, key_len))
-    msg (M_SSLERR, "EVP set key size");
+    crypto_msg (M_FATAL, "EVP set key size");
 #endif
   if (!EVP_CipherInit (ctx, NULL, key, NULL, enc))
-    msg (M_SSLERR, "EVP cipher init #2");
+    crypto_msg (M_FATAL, "EVP cipher init #2");
 
   /* make sure we used a big enough key */
   ASSERT (EVP_CIPHER_CTX_key_length (ctx) <= key_len);
@@ -622,7 +635,9 @@ int
 cipher_ctx_update (EVP_CIPHER_CTX *ctx, uint8_t *dst, int *dst_len,
     uint8_t *src, int src_len)
 {
-  return EVP_CipherUpdate (ctx, dst, dst_len, src, src_len);
+  if (!EVP_CipherUpdate (ctx, dst, dst_len, src, src_len))
+    crypto_msg(M_FATAL, "%s: EVP_CipherUpdate() failed", __func__);
+  return 1;
 }
 
 int
@@ -657,12 +672,14 @@ md_kt_get (const char *digest)
   ASSERT (digest);
   md = EVP_get_digestbyname (digest);
   if (!md)
-    msg (M_SSLERR, "Message hash algorithm '%s' not found", digest);
+    crypto_msg (M_FATAL, "Message hash algorithm '%s' not found", digest);
   if (EVP_MD_size (md) > MAX_HMAC_KEY_LENGTH)
-    msg (M_FATAL, "Message hash algorithm '%s' uses a default hash size (%d bytes) which is larger than " PACKAGE_NAME "'s current maximum hash size (%d bytes)",
-	 digest,
-	 EVP_MD_size (md),
-	 MAX_HMAC_KEY_LENGTH);
+    {
+      crypto_msg (M_FATAL, "Message hash algorithm '%s' uses a default hash "
+	  "size (%d bytes) which is larger than " PACKAGE_NAME "'s current "
+	  "maximum hash size (%d bytes)",
+	  digest, EVP_MD_size (md), MAX_HMAC_KEY_LENGTH);
+    }
   return md;
 }
 
src/openvpn/crypto_openssl.h +25 -0
index f883c2a..42c7e9a 100644
@@ -70,4 +70,29 @@ typedef HMAC_CTX hmac_ctx_t;
 #define DES_KEY_LENGTH 8
 #define MD4_DIGEST_LENGTH 	16
 
+/**
+ * Retrieve any occurred OpenSSL errors and print those errors.
+ *
+ * Note that this function uses the not thread-safe OpenSSL error API.
+ *
+ * @param flags		Flags to indicate error type and priority.
+ */
+void crypto_print_openssl_errors(const unsigned int flags);
+
+/**
+ * Retrieve any OpenSSL errors, then print the supplied error message.
+ *
+ * This is just a convenience wrapper for often occurring situations.
+ *
+ * @param flags		Flags to indicate error type and priority.
+ * @param format	Format string to print.
+ * @param format args	(optional) arguments for the format string.
+ */
+# define crypto_msg(flags, ...) \
+do { \
+  crypto_print_openssl_errors(nonfatal(flags)); \
+  msg((flags), __VA_ARGS__); \
+} while (false)
+
+
 #endif /* CRYPTO_OPENSSL_H_ */
src/openvpn/error.c +0 -29
index f503cf4..b47a9e5 100644
@@ -43,13 +43,6 @@
 #include "ps.h"
 #include "mstats.h"
 
-#ifdef ENABLE_CRYPTO
-#ifdef ENABLE_CRYPTO_OPENSSL
-#include <openssl/err.h>
-#endif
-#endif
-
-#include "memdbg.h"
 
 #if SYSLOG_CAPABILITY
 #ifndef LOG_OPENVPN
@@ -254,28 +247,6 @@ void x_msg_va (const unsigned int flags, const char *format, va_list arglist)
       SWAP;
     }
 
-#ifdef ENABLE_CRYPTO
-#ifdef ENABLE_CRYPTO_OPENSSL
-  if (flags & M_SSL)
-    {
-      int nerrs = 0;
-      int err;
-      while ((err = ERR_get_error ()))
-	{
-	  openvpn_snprintf (m2, ERR_BUF_SIZE, "%s: %s",
-			    m1, ERR_error_string (err, NULL));
-	  SWAP;
-	  ++nerrs;
-	}
-      if (!nerrs)
-	{
-	  openvpn_snprintf (m2, ERR_BUF_SIZE, "%s (OpenSSL)", m1);
-	  SWAP;
-	}
-    }
-#endif
-#endif
-
   if (flags & M_OPTERR)
     {
       openvpn_snprintf (m2, ERR_BUF_SIZE, "Options error: %s", m1);
src/openvpn/error.h +6 -5
index 42308e8..52ef0bd 100644
@@ -93,10 +93,6 @@ extern int x_msg_line_num;
 
 #define M_ERRNO           (1<<8)	 /* show errno description */
 
-#ifdef ENABLE_CRYPTO_OPENSSL
-#  define M_SSL             (1<<10)	 /* show SSL error */
-#endif
-
 #define M_NOMUTE          (1<<11)        /* don't do mute processing */
 #define M_NOPREFIX        (1<<12)        /* don't show date/time prefix */
 #define M_USAGE_SMALL     (1<<13)        /* fatal options error, call usage_small */
@@ -107,7 +103,6 @@ extern int x_msg_line_num;
 
 /* flag combinations which are frequently used */
 #define M_ERR     (M_FATAL | M_ERRNO)
-#define M_SSLERR  (M_FATAL | M_SSL)
 #define M_USAGE   (M_USAGE_SMALL | M_NOPREFIX | M_OPTERR)
 #define M_CLIENT  (M_MSG_VIRT_OUT | M_NOMUTE | M_NOIPREFIX)
 
@@ -360,6 +355,12 @@ ignore_sys_error (const int err)
   return false;
 }
 
+/** Convert fatal errors to nonfatal, don't touch other errors */
+static inline unsigned int
+nonfatal(const unsigned int err) {
+  return err & M_FATAL ? (err ^ M_FATAL) | M_NONFATAL : err;
+}
+
 #include "errlevel.h"
 
 #endif
src/openvpn/ssl_openssl.c +61 -50

325 lines not on this page · raw message

Signature · 1 line
2.5.0
#2 lore raw

[Openvpn-devel] [PATCH applied] Re: openssl: improve logging

ACK. It made sense for master, got quite a bit of testing by now, and trac #646 clearly demonstrates we want it in 2.3 as well.

Your patch has been applied to the release/2.3 branch.

commit 8e95e2091397283d3fa64507a0e444fdc4359d57

Author: Steffan Karger
Date:   Wed Jan 6 21:51:04 2016 +0100
     openssl: improve logging

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/10944
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

#3 lore raw

PATCH v2 1/1 polarssl: improve logging

In reply to Steffan Karger

From: Steffan Karger <[email protected]...>

Add the functions polar_log_err(), polar_log_func_line() and a macro polar_ok(), to easily log human-readable PolarSSL errors from polarssl-specific code.

This does not provide the full logging interface as msg(), because I would have to add a lot more of macro-magic to achieve that on the various supported compilers and platforms, and this suffices too (for now at least).

Use the new polar_log_err() and polar_ok() functions to provide more log/debug output for polarssl errors.

This is commit is a combined cherry-pick of commits 6ef5df14, d17d362d, aa416be9, and 3a39bf7d from the master branch, adjusted to the release/2.3 branch.

v2 - use static inline instead of macro for optimization, and include
'enable polarssl debug logging'.

---
 src/openvpn/crypto_polarssl.c     | 60 ++++++++++++++++++++++++--------
 src/openvpn/crypto_polarssl.h     | 49 ++++++++++++++++++++++++++
 src/openvpn/ssl_polarssl.c        | 72 +++++++++++++++++++--------------------
 src/openvpn/ssl_verify_polarssl.c | 26 ++++----------
 4 files changed, 136 insertions(+), 71 deletions(-)
src/openvpn/crypto_polarssl.c +45 -15
index 24712ed..92fdb78 100644
@@ -46,6 +46,7 @@
 #include "misc.h"
 
 #include <polarssl/des.h>
+#include <polarssl/error.h>
 #include <polarssl/md5.h>
 #include <polarssl/cipher.h>
 #include <polarssl/havege.h>
@@ -86,6 +87,32 @@ crypto_clear_error (void)
 {
 }
 
+bool polar_log_err(unsigned int flags, int errval, const char *prefix)
+{
+  if (0 != errval)
+    {
+      char errstr[256];
+      polarssl_strerror(errval, errstr, sizeof(errstr));
+
+      if (NULL == prefix) prefix = "PolarSSL error";
+      msg (flags, "%s: %s", prefix, errstr);
+    }
+
+  return 0 == errval;
+}
+
+bool polar_log_func_line(unsigned int flags, int errval, const char *func,
+    int line)
+{
+  char prefix[256];
+
+  if (!openvpn_snprintf(prefix, sizeof(prefix), "%s:%d", func, line))
+    return polar_log_err(flags, errval, func);
+
+  return polar_log_err(flags, errval, prefix);
+}
+
+
 #ifdef DMALLOC
 void
 crypto_init_dmalloc (void)
@@ -234,7 +261,8 @@ ctr_drbg_context * rand_ctx_get()
       /* Initialise PolarSSL RNG, and built-in entropy sources */
       entropy_init(&ec);
 
-      if (0 != ctr_drbg_init(&cd_ctx, entropy_func, &ec, BPTR(&pers_string), BLEN(&pers_string)))
+      if (!polar_ok(ctr_drbg_init(&cd_ctx, entropy_func, &ec,
+		    BPTR(&pers_string), BLEN(&pers_string))))
         msg (M_FATAL, "Failed to initialize random generator");
 
       gc_free(&gc);
@@ -445,10 +473,10 @@ cipher_ctx_init (cipher_context_t *ctx, uint8_t *key, int key_len,
 
   CLEAR (*ctx);
 
-  if (0 != cipher_init_ctx(ctx, kt))
+  if (!polar_ok(cipher_init_ctx(ctx, kt)))
     msg (M_FATAL, "PolarSSL cipher context init #1");
 
-  if (0 != cipher_setkey(ctx, key, key_len*8, enc))
+  if (!polar_ok(cipher_setkey(ctx, key, key_len*8, enc)))
     msg (M_FATAL, "PolarSSL cipher set key");
 
   /* make sure we used a big enough key */
@@ -487,36 +515,38 @@ cipher_ctx_get_cipher_kt (const cipher_ctx_t *ctx)
 
 int cipher_ctx_reset (cipher_context_t *ctx, uint8_t *iv_buf)
 {
-  int retval = cipher_reset(ctx);
+  if (!polar_ok(cipher_reset(ctx)))
+    return 0;
 
-  if (0 == retval)
-    retval = cipher_set_iv(ctx, iv_buf, ctx->cipher_info->iv_size);
+  if (!polar_ok(cipher_set_iv(ctx, iv_buf, ctx->cipher_info->iv_size)))
+    return 0;
 
-  return 0 == retval;
+  return 1;
 }
 
 int cipher_ctx_update (cipher_context_t *ctx, uint8_t *dst, int *dst_len,
     uint8_t *src, int src_len)
 {
-  int retval = 0;
   size_t s_dst_len = *dst_len;
 
-  retval = cipher_update(ctx, src, (size_t)src_len, dst, &s_dst_len);
+  if (!polar_ok(cipher_update(ctx, src, (size_t)src_len, dst, &s_dst_len)))
+    return 0;
 
   *dst_len = s_dst_len;
 
-  return 0 == retval;
+  return 1;
 }
 
 int cipher_ctx_final (cipher_context_t *ctx, uint8_t *dst, int *dst_len)
 {
-  int retval = 0;
   size_t s_dst_len = *dst_len;
 
-  retval = cipher_finish(ctx, dst, &s_dst_len);
+  if (!polar_ok(cipher_finish(ctx, dst, &s_dst_len)))
+    return 0;
+
   *dst_len = s_dst_len;
 
-  return 0 == retval;
+  return 1;
 }
 
 void
@@ -526,8 +556,8 @@ cipher_des_encrypt_ecb (const unsigned char key[DES_KEY_LENGTH],
 {
     des_context ctx;
 
-    des_setkey_enc(&ctx, key);
-    des_crypt_ecb(&ctx, src, dst);
+    ASSERT (polar_ok(des_setkey_enc(&ctx, key)));
+    ASSERT (polar_ok(des_crypt_ecb(&ctx, src, dst)));
 }
 
 
src/openvpn/crypto_polarssl.h +49 -0

53 lines not on this page · raw message

src/openvpn/ssl_polarssl.c +35 -37

206 lines not on this page · raw message

src/openvpn/ssl_verify_polarssl.c +7 -19
index ac252a3..7ed87d6 100644
@@ -131,17 +131,12 @@ backend_x509_get_serial (openvpn_x509_cert_t *cert, struct gc_arena *gc)
   char *buf = NULL;
   size_t buflen = 0;
   mpi serial_mpi = { 0 };
-  int retval = 0;
 
   /* Transform asn1 integer serial into PolarSSL MPI */
   mpi_init(&serial_mpi);
-  retval = mpi_read_binary(&serial_mpi, cert->serial.p, cert->serial.len);
-  if (retval < 0)
+  if (!polar_ok(mpi_read_binary(&serial_mpi, cert->serial.p, cert->serial.len)))
     {
-      char errbuf[128];
-      error_strerror(retval, errbuf, sizeof(errbuf));
-
-      msg(M_WARN, "Failed to retrieve serial from certificate: %s.", errbuf);
+      msg(M_WARN, "Failed to retrieve serial from certificate.");
       return NULL;
     }
 
@@ -150,13 +145,9 @@ backend_x509_get_serial (openvpn_x509_cert_t *cert, struct gc_arena *gc)
   buf = gc_malloc(buflen, true, gc);
 
   /* Write MPI serial as decimal string into buffer */
-  retval = mpi_write_string(&serial_mpi, 10, buf, &buflen);
-  if (retval < 0)
+  if (!polar_ok(mpi_write_string(&serial_mpi, 10, buf, &buflen)))
     {
-      char errbuf[128];
-      error_strerror(retval, errbuf, sizeof(errbuf));
-
-      msg(M_WARN, "Failed to write serial to string: %s.", errbuf);
+      msg(M_WARN, "Failed to write serial to string.");
       return NULL;
     }
 
@@ -372,12 +363,9 @@ x509_verify_crl(const char *crl_file, x509_crt *cert, const char *subject)
   struct gc_arena gc = gc_new();
   char *serial;
 
-  int polar_retval = x509_crl_parse_file(&crl, crl_file);
-  if (polar_retval != 0)
+  if (!polar_ok(x509_crl_parse_file(&crl, crl_file)))
     {
-      char errstr[128];
-      polarssl_strerror(polar_retval, errstr, sizeof(errstr));
-      msg (M_WARN, "CRL: cannot read CRL from file %s (%s)", crl_file, errstr);
+      msg (M_WARN, "CRL: cannot read CRL from file %s", crl_file);
       goto end;
     }
 
@@ -390,7 +378,7 @@ x509_verify_crl(const char *crl_file, x509_crt *cert, const char *subject)
       goto end;
     }
 
-  if (0 != x509_crt_revoked(cert, &crl))
+  if (!polar_ok(x509_crt_revoked(cert, &crl)))
     {
       serial = backend_x509_get_serial_hex(cert, &gc);
       msg (D_HANDSHAKE, "CRL CHECK FAILED: %s (serial %s) is REVOKED", subject, (serial ? serial : "NOT AVAILABLE"));
Signature · 1 line
2.5.0
#4 lore raw

ACK, same arguments as for the openssl patch.

Thanks for working all these patches into one big one for 2.3 - less work, and the history makes still clear where the stuff is coming from and who originally accepted it.

Your patch has been applied to the release/2.3 branch.

commit a9226fbdd90ac37891937b396c9c3212cd324262

Author: Steffan Karger
Date:   Thu Jan 7 14:01:30 2016 +0100
     polarssl: improve logging

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/10952
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering