Configure script of rlm_mschaps checks for wbclient.h, but not core/ntstatus.h #1489

Closed
herwinw opened this Issue Jan 7, 2016 · 6 comments

Comments

Projects
None yet
4 participants
Member

herwinw commented Jan 7, 2016

A supplement for #1488.

Using Debian Sid, the configure script of rlm_mschap checks for the existence of wbclient.h to build auth_wbclient.c. This file also has a dependency on core/ntstatus.h, which is in a different package on Debian (needed for NT_STATUS_PASSWORD_EXPIRED and NT_STATUS_PASSWORD_MUST_CHANGE). Missing core/ntstatus.h results in build errors

CC src/modules/rlm_mschap/auth_wbclient.c
src/modules/rlm_mschap/auth_wbclient.c:30:27: fatal error: core/ntstatus.h: No such file or directory
compilation terminated.
scripts/boiler.mk:631: recipe for target 'build/objs/src/modules/rlm_mschap/auth_wbclient.lo' failed
make: *** [build/objs/src/modules/rlm_mschap/auth_wbclient.lo] Error 1

I think the configure script should check for both header files before trying to build auth_wbclient.c.

Member

alanbuxey commented Jan 8, 2016

what package is that include from?

Member

herwinw commented Jan 10, 2016

@alanbuxey: I don't really understand the question, you mean in which debian package?

Member

alanbuxey commented Jan 15, 2016

yes... i think its samba-dev or such required?

On 10 January 2016 at 20:09, Herwin notifications@github.com wrote:

@alanbuxey https://github.com/alanbuxey: I don't really understand the
question, you mean in which debian package?


Reply to this email directly or view it on GitHub
#1489 (comment)
.

Member

herwinw commented Jan 15, 2016

The Debian issue has already been fixed (or worked around) in #1488.

Contributor

matsimon commented Mar 11, 2016

@herwinw is right, on Debian it's only being worked around by enforcing the presence of samba-dev which provides said core/ntstatus.h. However on $anyothersystem the present configure script in src/modules/rlm_mschap does not yet verify its presence.

I sense src/modules/rlm_mschap/configure.ac needs to be modified to check for the prsence of this header, then the resulting configure would need to be regenerated.

Contributor

matsimon commented Mar 15, 2016

I'm just trying out something and I'm certain that's not yet sufficient:

--- a/src/modules/rlm_mschap/configure.ac
+++ b/src/modules/rlm_mschap/configure.ac
@@ -80,6 +80,13 @@ if test x$with_[]modname != xno; then
         AC_MSG_WARN([silently building without support for direct authentication via winbind. requires: libwbclient])
     fi

+    FR_SMART_CHECK_INCLUDE(core/ntstatus.h, [#include <stdint.h>
+                                       #include <stdbool.h>])
+    if test "x$ac_cv_header_core_ntstatus_h" != "xyes"; then
+        AC_MSG_WARN([core/ntstatus.h not found. Use --with-winbind-include-dir=<path>.])
+        AC_MSG_WARN([silently building without support for direct authentication via winbind. requires: libwbclient])
+    fi
+
     dnl ############################################################
     dnl # Check for libraries
     dnl ############################################################
@@ -90,8 +97,10 @@ if test x$with_[]modname != xno; then
       AC_MSG_WARN([winbind libraries not found. Use --with-winbind-lib-dir=<path>.])
       AC_MSG_WARN([Samba must be version 4.2.1 or higher to use this feature.])
     elif test "x$ac_cv_header_wbclient_h" = "xyes"; then
-      mschap_sources="$mschap_sources auth_wbclient.c"
-      AC_DEFINE([WITH_AUTH_WINBIND],[1],[Build with direct winbind auth support])
+      if test "x$ac_cv_header_core_ntstatus_h" = "xyes"; then
+        mschap_sources="$mschap_sources auth_wbclient.c"
+        AC_DEFINE([WITH_AUTH_WINBIND],[1],[Build with direct winbind auth support])
+      fi
     fi

     targetname=modname

With that autoconf -I ../../../ was executed in src/modules/rlm_mschap to update the configure script.

Results so far:

  • If wbclient.h is absent, build passes (OK, no change)
  • If both wbclient.h and core/ntstatus.h are found, the build passes and rlm_mschap.so is linked againt libwbclient (OK, no change)
  • Interestingly if libwbclient.h is found, but core/ntstatus.h the build passes (expected) but rlm_mschap.so is linked against libwbclient (Definitely NOT expected)

I have no means testing rlm_mschap with libwbclient mode on a system that's not in production. I feel something needs to be done additionnaly to avoid linking against libwbclient if core/ntstatus.h isn't on the system.

TBH: I thought I might still share that, but I don't think that should be integrated as is, maybe someone has an idea where to proceed further.

mcnewton self-assigned this Apr 12, 2016

mcnewton closed this in 852b913 Apr 12, 2016

@mcnewton mcnewton added a commit that referenced this issue Apr 12, 2016

@mcnewton mcnewton Only build mschap auth_wbclient when core/ntstatus.h is present
In addition, don't check for the library if the headers aren't there,
otherwise we'll link against libwbclient unnecessarily.

Closes #1489
2d09bfb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment