Re: [Openvpn-devel] [PATCH 37/52] build: proper pkcs11-helper detection and usage

2012-03-08 Thread Samuli Seppänen
These changes follow the same style as earlier patches, e.g. the selinux
patch. Pkg-config is now being used to detect pkcs11-helper afaics.
Also, pkcs11-helper now disabled by default, which I think makes sense.

I don't see why this shouldn't be included, so it's an ACK.

-- 
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock


> Signed-off-by: Alon Bar-Lev 
> ---
>  configure.ac   |   49 ---
>  distro/rpm/openvpn.spec.in |5 ++-
>  src/openvpn/Makefile.am|4 +++
>  src/openvpn/ssl.c  |2 +-
>  src/openvpn/syshead.h  |7 --
>  5 files changed, 26 insertions(+), 41 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 2388f17..baa66b2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -111,9 +111,9 @@ AC_ARG_ENABLE(
>  
>  AC_ARG_ENABLE(
>   [pkcs11],
> - [AS_HELP_STRING([--disable-pkcs11], [disable pkcs11 support])],
> + [AS_HELP_STRING([--enable-pkcs11], [enable pkcs11 support])],
>   ,
> - [enable_pkcs11="yes"]
> + [enable_pkcs11="no"]
>  )
>  
>  AC_ARG_ENABLE(
> @@ -254,19 +254,6 @@ AC_ARG_WITH(
>  )
>  
>  AC_ARG_WITH(
> - [pkcs11-helper-headers],
> - [AS_HELP_STRING([--with-pkcs11-helper-headers=DIR], [pkcs11-helper 
> Include files location])],
> - [PKCS11_HELPER_HDR_DIR="$withval"]
> - [CPPFLAGS="$CPPFLAGS -I$withval"] 
> -)
> -
> -AC_ARG_WITH(
> - [pkcs11-helper-lib],
> - [AS_HELP_STRING([--with-pkcs11-helper-lib=DIR], [pkcs11-helper Library 
> location])],
> - [LDFLAGS="$LDFLAGS -L$withval"] 
> -)
> -
> -AC_ARG_WITH(
>   [mem-check],
>   [AS_HELP_STRING([--with-mem-check=TYPE], [build with debug memory 
> checking, TYPE=dmalloc|valgrind|ssl])],
>   [
> @@ -719,22 +706,12 @@ if test "${enable_lzo_stub}" = "yes"; then
>   AC_DEFINE([LZO_STUB], [1], [Enable LZO stub capability])
>  fi
>  
> -dnl
> -dnl enable pkcs11 capability
> -dnl
> -if test "${enable_pkcs11}" = "yes"; then
> -   AC_CHECKING([for pkcs11-helper Library and Header files])
> -   AC_CHECK_HEADER(pkcs11-helper-1.0/pkcs11h-core.h,
> - [AC_CHECK_LIB(pkcs11-helper, pkcs11h_initialize,
> - [
> -AC_DEFINE(USE_PKCS11, 1, [Enable PKCS11 capability])
> -LIBS="${LIBS} -lpkcs11-helper"
> - ],
> - [AC_MSG_RESULT([pkcs11-helper library not found.])]
> - )],
> - [AC_MSG_RESULT([pkcs11-helper headers not found.])]
> -   )
> -fi
> +PKG_CHECK_MODULES(
> + [PKCS11_HELPER],
> + [libpkcs11-helper-1 >= 1.02],
> + [have_pkcs11_helper="yes"],
> + []
> +)
>  
>  dnl
>  dnl check for SSL-crypto library
> @@ -890,6 +867,14 @@ if test "${enable_selinux}" = "yes"; then
>   AC_DEFINE([ENABLE_SELINUX], [1], [SELinux support])
>  fi
>  
> +if test "${enable_pkcs11}" = "yes"; then
> + test "${have_pkcs11_helper}" != "yes" && AC_MSG_ERROR([PKCS11 enabled 
> but libpkcs11-helper is missing])
> + test "${enable_ssl}" != "yes" && AC_MSG_ERROR([PKCS11 can be enabled 
> only if SSL is enabled])
> + OPTIONAL_PKCS11_HELPER_CFLAGS="${PKCS11_HELPER_CFLAGS}"
> + OPTIONAL_PKCS11_HELPER_LIBS="${PKCS11_HELPER_LIBS}"
> + AC_DEFINE([ENABLE_PKCS11], [1], [Enable PKCS11])
> +fi
> +
>  if test "${enable_pedantic}" = "yes"; then
>   enable_strict="yes"
>   CFLAGS="${CFLAGS} -ansi -pedantic"
> @@ -917,6 +902,8 @@ AC_SUBST([TAP_WIN_MIN_MINOR])
>  
>  AC_SUBST([OPTIONAL_DL_LIBS])
>  AC_SUBST([OPTIONAL_SELINUX_LIBS])
> +AC_SUBST([OPTIONAL_PKCS11_HELPER_CFLAGS])
> +AC_SUBST([OPTIONAL_PKCS11_HELPER_LIBS])
>  
>  AM_CONDITIONAL([WIN32], [test "${WIN32}" = "yes"])
>  
> diff --git a/distro/rpm/openvpn.spec.in b/distro/rpm/openvpn.spec.in
> index 455f739..8db5172 100644
> --- a/distro/rpm/openvpn.spec.in
> +++ b/distro/rpm/openvpn.spec.in
> @@ -52,8 +52,8 @@ Requires:  openssl   >= 0.9.6
>  %{!?without_pam:BuildRequires: pam-devel}
>  %{!?without_pam:Requires:  pam}
>  
> -%{!?with_pkcs11:BuildRequires: pkcs11-helper-devel}
> -%{!?with_pkcs11:Requires:  pkcs11-helper}
> +%{?with_pkcs11:BuildRequires: pkcs11-helper-devel}
> +%{?with_pkcs11:Requires:  pkcs11-helper}
>  
>  #
>  # Description
> @@ -111,6 +111,7 @@ Development support for OpenVPN.
>   --docdir="%{_docdir}/%{name}-%{version}" \
>   %{?with_password_save:--enable-password-save} \
>   %{?without_lzo:--disable-lzo} \
> + %{?with_pkcs11:--enable-pkcs11} \
>   %{?with_kerberos:--with-ssl-headers=/usr/kerberos/include}
>  %__make
>  
> diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
> index a3f8b3a..fd92225 100644
> --- a/src/openvpn/Makefile.am
> +++ b/src/openvpn/Makefile.am
> @@ -16,6 +16,9 @@ MAINTAINERCLEANFILES = \
>  
>  INCLUDES = -I$(top_srcdir)/include
>  
> +AM_CFLAGS = \
> + $(OPTIONAL_PKCS11_HELPER_CFLAGS)
> +
>  sbin_PROGRAMS = openvpn
>  
>  openvpn_SOURCES = \
> @@ -97,6 +100,7 @@ openvpn_SOURCES = \
>   cryptoapi.h 

[Openvpn-devel] [PATCH 37/52] build: proper pkcs11-helper detection and usage

2012-02-29 Thread Alon Bar-Lev

Signed-off-by: Alon Bar-Lev 
---
 configure.ac   |   49 ---
 distro/rpm/openvpn.spec.in |5 ++-
 src/openvpn/Makefile.am|4 +++
 src/openvpn/ssl.c  |2 +-
 src/openvpn/syshead.h  |7 --
 5 files changed, 26 insertions(+), 41 deletions(-)

diff --git a/configure.ac b/configure.ac
index 2388f17..baa66b2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -111,9 +111,9 @@ AC_ARG_ENABLE(

 AC_ARG_ENABLE(
[pkcs11],
-   [AS_HELP_STRING([--disable-pkcs11], [disable pkcs11 support])],
+   [AS_HELP_STRING([--enable-pkcs11], [enable pkcs11 support])],
,
-   [enable_pkcs11="yes"]
+   [enable_pkcs11="no"]
 )

 AC_ARG_ENABLE(
@@ -254,19 +254,6 @@ AC_ARG_WITH(
 )

 AC_ARG_WITH(
-   [pkcs11-helper-headers],
-   [AS_HELP_STRING([--with-pkcs11-helper-headers=DIR], [pkcs11-helper 
Include files location])],
-   [PKCS11_HELPER_HDR_DIR="$withval"]
-   [CPPFLAGS="$CPPFLAGS -I$withval"] 
-)
-
-AC_ARG_WITH(
-   [pkcs11-helper-lib],
-   [AS_HELP_STRING([--with-pkcs11-helper-lib=DIR], [pkcs11-helper Library 
location])],
-   [LDFLAGS="$LDFLAGS -L$withval"] 
-)
-
-AC_ARG_WITH(
[mem-check],
[AS_HELP_STRING([--with-mem-check=TYPE], [build with debug memory 
checking, TYPE=dmalloc|valgrind|ssl])],
[
@@ -719,22 +706,12 @@ if test "${enable_lzo_stub}" = "yes"; then
AC_DEFINE([LZO_STUB], [1], [Enable LZO stub capability])
 fi

-dnl
-dnl enable pkcs11 capability
-dnl
-if test "${enable_pkcs11}" = "yes"; then
-   AC_CHECKING([for pkcs11-helper Library and Header files])
-   AC_CHECK_HEADER(pkcs11-helper-1.0/pkcs11h-core.h,
-   [AC_CHECK_LIB(pkcs11-helper, pkcs11h_initialize,
-   [
-  AC_DEFINE(USE_PKCS11, 1, [Enable PKCS11 capability])
-  LIBS="${LIBS} -lpkcs11-helper"
-   ],
-   [AC_MSG_RESULT([pkcs11-helper library not found.])]
-   )],
-   [AC_MSG_RESULT([pkcs11-helper headers not found.])]
-   )
-fi
+PKG_CHECK_MODULES(
+   [PKCS11_HELPER],
+   [libpkcs11-helper-1 >= 1.02],
+   [have_pkcs11_helper="yes"],
+   []
+)

 dnl
 dnl check for SSL-crypto library
@@ -890,6 +867,14 @@ if test "${enable_selinux}" = "yes"; then
AC_DEFINE([ENABLE_SELINUX], [1], [SELinux support])
 fi

+if test "${enable_pkcs11}" = "yes"; then
+   test "${have_pkcs11_helper}" != "yes" && AC_MSG_ERROR([PKCS11 enabled 
but libpkcs11-helper is missing])
+   test "${enable_ssl}" != "yes" && AC_MSG_ERROR([PKCS11 can be enabled 
only if SSL is enabled])
+   OPTIONAL_PKCS11_HELPER_CFLAGS="${PKCS11_HELPER_CFLAGS}"
+   OPTIONAL_PKCS11_HELPER_LIBS="${PKCS11_HELPER_LIBS}"
+   AC_DEFINE([ENABLE_PKCS11], [1], [Enable PKCS11])
+fi
+
 if test "${enable_pedantic}" = "yes"; then
enable_strict="yes"
CFLAGS="${CFLAGS} -ansi -pedantic"
@@ -917,6 +902,8 @@ AC_SUBST([TAP_WIN_MIN_MINOR])

 AC_SUBST([OPTIONAL_DL_LIBS])
 AC_SUBST([OPTIONAL_SELINUX_LIBS])
+AC_SUBST([OPTIONAL_PKCS11_HELPER_CFLAGS])
+AC_SUBST([OPTIONAL_PKCS11_HELPER_LIBS])

 AM_CONDITIONAL([WIN32], [test "${WIN32}" = "yes"])

diff --git a/distro/rpm/openvpn.spec.in b/distro/rpm/openvpn.spec.in
index 455f739..8db5172 100644
--- a/distro/rpm/openvpn.spec.in
+++ b/distro/rpm/openvpn.spec.in
@@ -52,8 +52,8 @@ Requires:  openssl   >= 0.9.6
 %{!?without_pam:BuildRequires: pam-devel}
 %{!?without_pam:Requires:  pam}

-%{!?with_pkcs11:BuildRequires: pkcs11-helper-devel}
-%{!?with_pkcs11:Requires:  pkcs11-helper}
+%{?with_pkcs11:BuildRequires: pkcs11-helper-devel}
+%{?with_pkcs11:Requires:  pkcs11-helper}

 #
 # Description
@@ -111,6 +111,7 @@ Development support for OpenVPN.
--docdir="%{_docdir}/%{name}-%{version}" \
%{?with_password_save:--enable-password-save} \
%{?without_lzo:--disable-lzo} \
+   %{?with_pkcs11:--enable-pkcs11} \
%{?with_kerberos:--with-ssl-headers=/usr/kerberos/include}
 %__make

diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index a3f8b3a..fd92225 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -16,6 +16,9 @@ MAINTAINERCLEANFILES = \

 INCLUDES = -I$(top_srcdir)/include

+AM_CFLAGS = \
+   $(OPTIONAL_PKCS11_HELPER_CFLAGS)
+
 sbin_PROGRAMS = openvpn

 openvpn_SOURCES = \
@@ -97,6 +100,7 @@ openvpn_SOURCES = \
cryptoapi.h cryptoapi.c
 openvpn_LDADD = \
$(SOCKETS_LIBS) \
+   $(OPTIONAL_PKCS11_HELPER_LIBS) \
$(OPTIONAL_SELINUX_LIBS) \
$(OPTIONAL_DL_LIBS)
 if WIN32
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index c26756e..e260718 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -264,7 +264,7 @@ ssl_purge_auth (const bool auth_user_pass_only)
 {
   if (!auth_user_pass_only)
 {
-#ifdef USE_PKCS11
+#ifdef ENABLE_PKCS11
   pkcs11_logout ();
 #endif
   purge_user_pass (, true);
diff --git a/src/openvpn/syshead.h b/src/openvpn/syshead.h
index