[Openvpn-devel] Don't throw fatal errors from verify_cert_export_cert()

Message ID 1512729211-8407-1-git-send-email-steffan@karger.me
State Superseded
Headers show
Series [Openvpn-devel] Don't throw fatal errors from verify_cert_export_cert() | expand

Commit Message

Steffan Karger Dec. 7, 2017, 11:33 p.m. UTC
From: Steffan Karger <steffan.karger@fox-it.com>

As with create_temp_file(), this function is called on client connects and
should not cause fatal errors when I/O (possibly temporarily) fails.

The callers of this function are already fixed in the commit that does the
same for create_temp_file().

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
---
 src/openvpn/ssl_verify.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Selva Nair Dec. 8, 2017, 4:16 a.m. UTC | #1
Hi,

On Fri, Dec 8, 2017 at 5:33 AM, Steffan Karger <steffan@karger.me> wrote:

> From: Steffan Karger <steffan.karger@fox-it.com>
>
> As with create_temp_file(), this function is called on client connects and
> should not cause fatal errors when I/O (possibly temporarily) fails.
>
> The callers of this function are already fixed in the commit that does the
> same for create_temp_file().
>
> Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
> ---
>  src/openvpn/ssl_verify.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
> index ebb1da2..12cff9a 100644
> --- a/src/openvpn/ssl_verify.c
> +++ b/src/openvpn/ssl_verify.c
> @@ -557,13 +557,14 @@ verify_cert_export_cert(openvpn_x509_cert_t
> *peercert, const char *tmp_dir, stru
>      peercert_file = fopen(peercert_filename, "w+");
>      if (!peercert_file)
>      {
> -        msg(M_ERR, "Failed to open temporary file : %s",
> peercert_filename);
> +        msg(M_ERRNO, "Failed to open temporary file : %s",
> peercert_filename);
>

I think M_ERRNO alone is not a good flag  -- if you OR it with M_NONFATAL,
syslog
will get level = LOG_ERR, management will get the 'N' flag etc.

Selva
<div dir="ltr">Hi,<br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 8, 2017 at 5:33 AM, Steffan Karger <span dir="ltr">&lt;<a href="mailto:steffan@karger.me" target="_blank">steffan@karger.me</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From: Steffan Karger &lt;<a href="mailto:steffan.karger@fox-it.com">steffan.karger@fox-it.com</a>&gt;<br>
<br>
As with create_temp_file(), this function is called on client connects and<br>
should not cause fatal errors when I/O (possibly temporarily) fails.<br>
<br>
The callers of this function are already fixed in the commit that does the<br>
same for create_temp_file().<br>
<br>
Signed-off-by: Steffan Karger &lt;<a href="mailto:steffan.karger@fox-it.com">steffan.karger@fox-it.com</a>&gt;<br>
---<br>
 src/openvpn/ssl_verify.c | 5 +++--<br>
 1 file changed, 3 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c<br>
index ebb1da2..12cff9a 100644<br>
--- a/src/openvpn/ssl_verify.c<br>
+++ b/src/openvpn/ssl_verify.c<br>
@@ -557,13 +557,14 @@ verify_cert_export_cert(<wbr>openvpn_x509_cert_t *peercert, const char *tmp_dir, stru<br>
     peercert_file = fopen(peercert_filename, &quot;w+&quot;);<br>
     if (!peercert_file)<br>
     {<br>
-        msg(M_ERR, &quot;Failed to open temporary file : %s&quot;, peercert_filename);<br>
+        msg(M_ERRNO, &quot;Failed to open temporary file : %s&quot;, peercert_filename);<br></blockquote><div><br></div><div>I think M_ERRNO alone is not a good flag  -- if you OR it with M_NONFATAL, syslog</div><div>will get level = LOG_ERR, management will get the &#39;N&#39; flag etc.</div><div><br></div><div>Selva </div></div></div></div>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Patch

diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index ebb1da2..12cff9a 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -557,13 +557,14 @@  verify_cert_export_cert(openvpn_x509_cert_t *peercert, const char *tmp_dir, stru
     peercert_file = fopen(peercert_filename, "w+");
     if (!peercert_file)
     {
-        msg(M_ERR, "Failed to open temporary file : %s", peercert_filename);
+        msg(M_ERRNO, "Failed to open temporary file : %s", peercert_filename);
         return NULL;
     }
 
     if (SUCCESS != x509_write_pem(peercert_file, peercert))
     {
-        msg(M_ERR, "Error writing PEM file containing certificate");
+        msg(M_WARN, "Error writing PEM file containing certificate");
+        peercert_filename = NULL;
     }
 
     fclose(peercert_file);