Skip to contentSkip to conversation

Fix --show-pkcs11-ids

PATCH In review Adriaan de Jong 5 participants 10 messages Nov 14, 2012 – Jan 18, 2013

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix --show-pkcs11-ids

[PATCH] Fix --show-pkcs11-ids (Bug #239)

Broken by 75b49e406430299b187964744f82e50a9035a0d3.

---
 src/openvpn/pkcs11.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
src/openvpn/pkcs11.c +1 -1
@@ -887,7 +887,7 @@ show_pkcs11_ids (
 		      (dn = pkcs11_certificate_dn (
 				certificate,
 				&gc
-		      ))
+		      )) == NULL
 		) {
 			goto cleanup1;
 		}

--
1.7.9.5

Acked-by Adriaan de Jong on v1 1/1
#1 lore raw
Quoted 8 lines · Adriaan de Jong

[PATCH] Fix --show-pkcs11-ids (Bug #239)

Broken by 75b49e406430299b187964744f82e50a9035a0d3.

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

Hi Joachim,

Thanks a lot for you fix! Just a little question. You mention this commit as being the offending commit:

commit 75b49e406430299b187964744f82e50a9035a0d3

Author: Alon Bar-Lev <[email protected]...>
Date:   Sun Apr 1 16:46:28 2012 +0300

cleanup: gc usage

Cleanup of "Use the garbage collector when retrieving x509 fields" patch series.

Discussed at [1].

There should be an effort to produce common function prologue and epilogue, so that cleanups will be done at single point.

[1] http://comments.gmane.org/gmane.network.openvpn.devel/5401

I'm just wondering how this could be related to you fix. As I see it, the syntax of the if() statement you change didn't change, neither the behaviour of pkcs11_certificate_dn().

I see that commit 00b973f8af85c3ea8fa3cef80eec55e8dc139b27 changes the behaviour of pkcs11_certificate_dn() in both the OpenSSL and PolarSSL implementation. So I'm suspecting you referenced the wrong commit. Right?

Signature · 3 lines
kind regards,

David Sommerseth
Acked-by Adriaan de Jong
#2 lore raw

Yes, that seems to be correct. Sorry!

Joachim

-----Original Message-----
From: David Sommerseth [mailto:[email protected]...]
Sent: dinsdag 20 november 2012 10:19
To: Joachim Schipper
Cc: [email protected]; Adriaan de Jong
Subject: Re: [Openvpn-devel] [PATCH] Fix --show-pkcs11-ids

Quoted 8 lines · Adriaan de Jong

[PATCH] Fix --show-pkcs11-ids (Bug #239)

Broken by 75b49e406430299b187964744f82e50a9035a0d3.

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

Hi Joachim,

Thanks a lot for you fix! Just a little question. You mention this commit as being the offending commit:

commit 75b49e406430299b187964744f82e50a9035a0d3

Author: Alon Bar-Lev <[email protected]...>
Date:   Sun Apr 1 16:46:28 2012 +0300

cleanup: gc usage

Cleanup of "Use the garbage collector when retrieving x509 fields" patch series.

Discussed at [1].

There should be an effort to produce common function prologue and epilogue, so that cleanups will be done at single point.

[1] http://comments.gmane.org/gmane.network.openvpn.devel/5401

I'm just wondering how this could be related to you fix. As I see it, the syntax of the if() statement you change didn't change, neither the behaviour of pkcs11_certificate_dn().

I see that commit 00b973f8af85c3ea8fa3cef80eec55e8dc139b27 changes the behaviour of pkcs11_certificate_dn() in both the OpenSSL and PolarSSL implementation. So I'm suspecting you referenced the wrong commit. Right?

--
kind regards,

David Sommerseth

#3 lore raw
Quoted 1 line · Joachim Schipper

Yes, that seems to be correct. Sorry!

Perfect! I'll update your commit message to point at the correct commit then and apply it :)

Signature · 3 lines
kind regards,

David Sommerseth
Quoted 63 lines

-----Original Message-----
From: David Sommerseth [mailto:[email protected]...]
Sent: dinsdag 20 november 2012 10:19
To: Joachim Schipper
Cc: [email protected]; Adriaan de Jong
Subject: Re: [Openvpn-devel] [PATCH] Fix --show-pkcs11-ids

Quoted 8 lines · Adriaan de Jong

[PATCH] Fix --show-pkcs11-ids (Bug #239)

Broken by 75b49e406430299b187964744f82e50a9035a0d3.

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

Hi Joachim,

Thanks a lot for you fix! Just a little question. You mention this commit as being the offending commit:

commit 75b49e406430299b187964744f82e50a9035a0d3

Author: Alon Bar-Lev <[email protected]...>
Date:   Sun Apr 1 16:46:28 2012 +0300

cleanup: gc usage

Cleanup of "Use the garbage collector when retrieving x509 fields" patch series.

Discussed at [1].

There should be an effort to produce common function prologue and epilogue, so that cleanups will be done at single point.

[1] http://comments.gmane.org/gmane.network.openvpn.devel/5401

I'm just wondering how this could be related to you fix. As I see it, the syntax of the if() statement you change didn't change, neither the behaviour of pkcs11_certificate_dn().

I see that commit 00b973f8af85c3ea8fa3cef80eec55e8dc139b27 changes the behaviour of pkcs11_certificate_dn() in both the OpenSSL and PolarSSL implementation. So I'm suspecting you referenced the wrong commit. Right?

--
kind regards,

David Sommerseth

------------------------------------------------------------------------------
Monitor your physical, virtual and cloud infrastructure from a single web console. Get in-depth insight into apps, servers, databases, vmware, SAP, cloud infrastructure, etc. Download 30-day Free Trial. Pricing starts from $795 for 25 servers or applications!
http://p.sf.net/sfu/zoho_dev2dev_nov
_______________________________________________
Openvpn-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

#4 lore raw
In reply to Adriaan de Jong
Quoted 8 lines · Adriaan de Jong

[PATCH] Fix --show-pkcs11-ids (Bug #239)

Broken by 75b49e406430299b187964744f82e50a9035a0d3.

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

ACK. Applied to master and beta/2.3 branches.

commit 5fd3e56430678bee3e6e3a3cf8dfd6db7e105676 (master) commit fd02ae905df21e1119fb63521e7ff773d6f812dc (beta/2.3)

Author: Adriaan de Jong <[email protected]...>
Date:   Wed Nov 14 10:03:12 2012 +0100
    Fix --show-pkcs11-ids (Bug #239)
        
    Signed-off-by: Joachim Schipper <[email protected]...>
    Acked-by: David Sommerseth <[email protected]...>
    Trac-ticket: 239
    Message-Id: [email protected]...
    URL: http://article.gmane.org/gmane.network.openvpn.devel/7129
    Signed-off-by: David Sommerseth <[email protected]...>
Signature · 3 lines
kind regards,

David Sommerseth
#5 lore raw
In reply to Adriaan de Jong

Hi,
I have been following the steps to build OpenVPN from the sources and
was scratching my head as to why the prebuilt binary would work, but my
build from the sources would bork on --show-pkcs11-ids ?!?! Searching
through the mailling list archives shows the patch below, which fixes my problem. It seems that this patch is not in the generic sources?

I was following the directions at
https://community.openvpn.net/openvpn/wiki/BuildingUsingGenericBuildsystem#Checkingoutopenvpn-buildrepository

My question is, why would I not get the correct sources with the fix below using the generic build ? Is there a different / better way to do it that whats outlined in the above url ?

Thanks!

	---Mike
Quoted 37 lines · Adriaan de Jong

[PATCH] Fix --show-pkcs11-ids (Bug #239)

Broken by 75b49e406430299b187964744f82e50a9035a0d3.

---
 src/openvpn/pkcs11.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
src/openvpn/pkcs11.c +1 -2
@@ -887,7 +887,7 @@ show_pkcs11_ids (
 		      (dn = pkcs11_certificate_dn (
 				certificate,
 				&gc
-		      ))
+		      )) == NULL
 		) {
 			goto cleanup1;
 		}
--

1.7.9.5

------------------------------------------------------------------------------
Monitor your physical, virtual and cloud infrastructure from a single web console. Get in-depth insight into apps, servers, databases, vmware, SAP, cloud infrastructure, etc. Download 30-day Free Trial. Pricing starts from $795 for 25 servers or applications!
http://p.sf.net/sfu/zoho_dev2dev_nov
_______________________________________________
Openvpn-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Signature · 5 lines
-------------------
Mike Tancsa, tel +1 519 651 3400
Sentex Communications, [email protected]...
Providing Internet services since 1994 www.sentex.net
Cambridge, Ontario Canada   http://www.tancsa.com/
#6 lore raw
Quoted 7 lines · Mike Tancsa

Hi,
I have been following the steps to build OpenVPN from the sources and
was scratching my head as to why the prebuilt binary would work, but my
build from the sources would bork on --show-pkcs11-ids ?!?! Searching
through the mailling list archives shows the patch below, which fixes my problem. It seems that this patch is not in the generic sources?

Quoted 8 lines

I was following the directions at
https://community.openvpn.net/openvpn/wiki/BuildingUsingGenericBuildsystem#Checkingoutopenvpn-buildrepository

My question is, why would I not get the correct sources with the fix below using the generic build ? Is there a different / better way to do it that whats outlined in the above url ?

Thanks!

The patch you point at should be in the git repo (applied Nov 22). It's commit 5fd3e56430678bee3e6e3a3cf8dfd6db7e105676 in master and commit fd02ae905df21e1119fb63521e7ff773d6f812dc in beta/2.3.

<http://thread.gmane.org/gmane.network.openvpn.devel/7129/focus=7136>

   $ git tag --contains fd02ae905df21e1119fb63521e7ff773d6f812dc
   v2.3.0
   v2.3_rc2

However, it seems that the generic build tool needs some more tweaking to grab the packages correctly from github - they changed the download URLs. Might be more issues there as well.

Samuli, can you check this?

How I compile OpenVPN is basically directly from the git tree, with these steps:

  $ autoreconf -vi
  ....
  $ ./configure
  ...
  $ make

And the binary is available in src/openvpn/. You need pkcs11-helper, lzo2 and OpenSSL or PolarSSL libraries and development headers pre-installed though.

But this is probably mostly handy unless you need to do Windows cross-builds.

--
kind regards,

David Sommerseth

Quoted 28 lines
	---Mike
Quoted 25 lines · Adriaan de Jong

[PATCH] Fix --show-pkcs11-ids (Bug #239)

Broken by 75b49e406430299b187964744f82e50a9035a0d3.

---
 src/openvpn/pkcs11.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
src/openvpn/pkcs11.c +1 -2
@@ -887,7 +887,7 @@ show_pkcs11_ids (
 		      (dn = pkcs11_certificate_dn (
 				certificate,
 				&gc
-		      ))
+		      )) == NULL
 		) {
 			goto cleanup1;
 		}
--

1.7.9.5

#7 lore raw
Quoted 10 lines · David Sommerseth

On 16/01/13 23:11, Mike Tancsa wrote:

$ git tag --contains fd02ae905df21e1119fb63521e7ff773d6f812dc 
v2.3.0 v2.3_rc2

However, it seems that the generic build tool needs some more tweaking to grab the packages correctly from github - they changed the download URLs. Might be more issues there as well.

Quoted 6 lines

Samuli, can you check this?

How I compile OpenVPN is basically directly from the git tree, with these steps:

$ autoreconf -vi

Hi,
I think for me, the issue was in the initial cloning of the repo.

Looking at
https://github.com/OpenVPN

I see
https://github.com/OpenVPN/openvpn
updated 10 days ago

But the instructions point to
https://github.com/OpenVPN/openvpn-build

which was updated 6 months ago

	---Mike
Signature · 5 lines
-------------------
Mike Tancsa, tel +1 519 651 3400
Sentex Communications, [email protected]...
Providing Internet services since 1994 www.sentex.net
Cambridge, Ontario Canada   http://www.tancsa.com/
#8 lore raw
In reply to Mike Tancsa

Hi,

Quoted 5 lines · Mike Tancsa

I have been following the steps to build OpenVPN from the sources and
was scratching my head as to why the prebuilt binary would work, but my
build from the sources would bork on --show-pkcs11-ids ?!?! Searching
through the mailling list archives shows the patch below, which fixes my problem. It seems that this patch is not in the generic sources?

Reading through the git log for 2.3.0, it most certainly is (5fd3e56430678bee).

Quoted 2 lines

I was following the directions at
https://community.openvpn.net/openvpn/wiki/BuildingUsingGenericBuildsystem#Checkingoutopenvpn-buildrepository

On what platform did you build, and for which target? Which git repository was checked out, and which branch?

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]...
#9 lore raw
Quoted 7 lines · Gert Doering
Quoted 5 lines

I was following the directions at
https://community.openvpn.net/openvpn/wiki/BuildingUsingGenericBuildsystem#Checkingoutopenvpn-buildrepository

On what platform did you build, and for which target? Which git repository was checked out, and which branch?

I think the issue is that the source codes that is downloaded in the above URL through the git clone, does not grab 2.3, but a snapshot from 6 months ago. I am cross building on Fedora, AMD64.

%git clone https://github.com/OpenVPN/openvpn-build.git
% cd openvpn-build/generic

% IMAGEROOT=`pwd`/image-win32 CHOST=i686-w64-mingw32 \
    CBUILD=x86_64-pc-linux-gnu ./build

Will build just fine, but the older version.

There also needs some manual fetching of files as URLs are out of date for pkcs
FATAL: Cannot download
https://github.com/downloads/alonbl/pkcs11-helper/pkcs11-helper-1.10.tar.bz2
FATAL: Cannot download
https://github.com/downloads/OpenVPN/tap-windows/tap-windows-9.9.0_master.zip

If I make the following changes, I get the newer version built. manually fetch
http://swupdate.openvpn.org/community/releases/openvpn-2.3.0.tar.gz
into the openvpn-build/generic/sources
Get rid of the 2.3_master.tar.gz file
change the build.vars

build.vars 2013-01-17 16:56:21.970578237 -0500 renamed from build.vars.prev 2013-01-17 16:56:06.356577031 -0500 +1 -1
@@ -5,7 +5,7 @@
 PKCS11_HELPER_VERSION="${PKCS11_HELPER_VERSION:-1.10}"
 LZO_VERSION="${LZO_VERSION:-2.05}"
 TAP_WINDOWS_VERSION="${TAP_WINDOWS_VERSION:-9.9.0_master}"
-OPENVPN_VERSION="${OPENVPN_VERSION:-2.3_master}"
+OPENVPN_VERSION="${OPENVPN_VERSION:-2.3.0}"
 OPENVPN_GUI_VERSION="${OPENVPN_GUI_VERSION:-1.0.3}"
 
 OPENSSL_URL="${OPENSSL_URL:-http://www.openssl.org/source/openssl-${OPENSSL_VERSION}.tar.gz}"

and that builds the latest release.

	---Mike
Signature · 5 lines
-------------------
Mike Tancsa, tel +1 519 651 3400
Sentex Communications, [email protected]...
Providing Internet services since 1994 www.sentex.net
Cambridge, Ontario Canada   http://www.tancsa.com/