Properly require --key even if defined(MANAGMENT_EXTERNAL_KEY)

PATCH In review Joachim Schipper 4 participants 5 messages Nov 26, 2012 – Nov 29, 2012

Share & tools

Joachim Schipper

author

2012-11-26 14:12 UTC #0 lore raw

PATCH 1/1 Properly require --key even if defined(MANAGMENT_EXTERNAL_KEY)

When specifying --tls-client --cert ... without a --key (or --pkcs11) option, OpenVPN should complain: "You must define private key file (--key)...". Ensure that this works even when compiling with MANAGMENT_EXTERNAL_KEY.

Signed-off-by Joachim Schipper <[email protected]...>

---
 src/openvpn/options.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

src/openvpn/options.c +1 -1

index 9baa4ff..caa9d17 100644
@@ -2274,7 +2274,7 @@ options_postprocess_verify_ce (const struct options *options, const struct conne
 	    {
 	      notnull (options->cert_file, "certificate file (--cert) or PKCS#12 file (--pkcs12)");
 #ifdef MANAGMENT_EXTERNAL_KEY
-          if (!options->management_flags & MF_EXTERNAL_KEY)
+          if (!(options->management_flags & MF_EXTERNAL_KEY))
 #endif
 	      notnull (options->priv_key_file, "private key file (--key) or PKCS#12 file (--pkcs12)");
 	    }

Signature · 1 line

1.7.9.5

Arne Schwabe

2012-11-26 14:21 UTC #1 lore raw

Am 26.11.12 15:12, schrieb Joachim Schipper:

Quoted 14 lines

When specifying --tls-client --cert ... without a --key (or --pkcs11) option, OpenVPN should complain: "You must define private key file (--key)...". Ensure that this works even when compiling with MANAGMENT_EXTERNAL_KEY.
Signed-off-by Joachim Schipper <[email protected]...>
---
 src/openvpn/options.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
src/openvpn/options.c +0 -0
index 9baa4ff..caa9d17 100644

src/openvpn/options.c options_postprocess_verify_ce (const struct options *options, const struct conne · line 2280

Quoted 8 lines

src/openvpn/options.c +1 -1

@@ -2274,7 +2274,7 @@ options_postprocess_verify_ce (const struct options *options, const struct conne
 	    {
 	      notnull (options->cert_file, "certificate file (--cert) or PKCS#12 file (--pkcs12)");
 #ifdef MANAGMENT_EXTERNAL_KEY
-          if (!options->management_flags & MF_EXTERNAL_KEY)
+          if (!(options->management_flags & MF_EXTERNAL_KEY))
 #endif
 	      notnull (options->priv_key_file, "private key file (--key) or PKCS#12 file (--pkcs12)");

The fix is correct. There is also another instance where I made the same mistake. Do you want to submit a patch with both instances or should I make a patch?

Arne

Arne Schwabe

2012-11-26 14:33 UTC #2 lore raw

PATCH 1/1 Properly require --key even if defined(MANAGMENT_EXTERNAL_KEY)

In reply to Joachim Schipper

Also fix the other instance where this logic is broken

Orginal-Patch-by Found by Joachim Schipper <[email protected]...>

Signed-off-by Arne Schwabe <[email protected]...>

---
 src/openvpn/options.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

src/openvpn/options.c +2 -2

index 5f19c27..a8843ed 100644
@@ -2274,7 +2274,7 @@ options_postprocess_verify_ce (const struct options *options, const struct conne
 	    {
 	      notnull (options->cert_file, "certificate file (--cert) or PKCS#12 file (--pkcs12)");
 #ifdef MANAGMENT_EXTERNAL_KEY
-          if (!options->management_flags & MF_EXTERNAL_KEY)
+          if (!(options->management_flags & MF_EXTERNAL_KEY))
 #endif
 	      notnull (options->priv_key_file, "private key file (--key) or PKCS#12 file (--pkcs12)");
 	    }

@@ -2666,7 +2666,7 @@ options_postprocess_filechecks (struct options *options)
   errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->extra_certs_file, R_OK,
                              "--extra-certs");
 #ifdef MANAGMENT_EXTERNAL_KEY
-  if(!options->management_flags & MF_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");

Signature · 1 line

1.7.9.5

Gert Doering

2012-11-26 19:00 UTC #3 lore raw

Hi,

Quoted 5 lines · Arne Schwabe

When specifying --tls-client --cert ... without a --key (or --pkcs11) option, OpenVPN should complain: "You must define private key file (--key)...". Ensure that this works even when compiling with MANAGMENT_EXTERNAL_KEY.
Also fix the other instance where this logic is broken

ACK. Operator precedence in C, gah.

gert

Signature · 4 lines

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

David Sommerseth

Applied

2012-11-29 21:16 UTC #4 lore raw

In reply to Arne Schwabe

Quoted 11 lines · Arne Schwabe

When specifying --tls-client --cert ... without a --key (or --pkcs11) option, OpenVPN should complain: "You must define private key file (--key)...". Ensure that this works even when compiling with MANAGMENT_EXTERNAL_KEY.
Also fix the other instance where this logic is broken
Orginal-Patch-by Found by Joachim Schipper <[email protected]...>
Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/options.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Applied to master and beta/2.3

commit feca0900dd00577779ab02f2e52b41eb3dab935f (master) commit 9447858c38c5cf51265c7eab3445d7d5fb4f7bba (beta/2.3)

Author: Arne Schwabe <[email protected]...>
Date:   Mon Nov 26 15:33:09 2012 +0100

    Properly require --key even if defined(MANAGMENT_EXTERNAL_KEY)
    
    Orginal-Patch-by: Found by Joachim Schipper <[email protected]....>
    Signed-off-by: Arne Schwabe <[email protected]...>
    Acked-by: Gert Doering <[email protected]...>
    Message-Id: [email protected]...
    URL: http://article.gmane.org/gmane.network.openvpn.devel/7143
    Signed-off-by: David Sommerseth <[email protected]...>

Signature · 3 lines

kind regards,

David Sommerseth