thr3ads.net - nsd users - [nsd-users] fix compat b64_{pton,ntop} handling in nsd [Sep 2015]

If this information is useful, please help other people find it:
Share via:

Stuart Henderson

2015-Sep-10 11:54 UTC

[nsd-users] fix compat b64_{pton,ntop} handling in nsd

Currently NSD's compat functions for b64_pton and b64_ntop are always
used. Worse, they're the old versions which abort(). This is because
the library symbols are prefixed by __ (not just in OpenBSD) and are
only accessible as b64_xx when resolv.h is included.

First attempt at fixing this involved borrowing tmux's autoconf checks
to find these functions. This works on OpenBSD, but fails on OS with
non-ancient resolv.h/nameser.h files which have #defines for T_xxx
resource records which conflict with those in NSD.

So the diff below uses the __ versions directly (OpenSSH does similar).
I've tested this on OpenBSD and Linux (SL6), including with the autoconf
check artificially broken to force using the compat versions.
The autoconf check is borrowed/adapted from the tmux one.

OK for OpenBSD?

Would this be acceptable for NSD upstream?


Index: configparser.y
==================================================================RCS file:
/cvs/src/usr.sbin/nsd/configparser.y,v
retrieving revision 1.11
diff -u -p -r1.11 configparser.y
--- configparser.y	17 Jul 2015 17:36:33 -0000	1.11
+++ configparser.y	10 Sep 2015 11:18:41 -0000
@@ -828,7 +828,7 @@ key_secret: VAR_SECRET STRING
 		assert(cfg_parser->current_key);
 #endif
 		cfg_parser->current_key->secret =
region_strdup(cfg_parser->opt->region, $2);
-		size = b64_pton($2, data, sizeof(data));
+		size = __b64_pton($2, data, sizeof(data));
 		if(size == -1) {
 			c_error_msg("Cannot base64 decode tsig secret %s",
 				cfg_parser->current_key->name?
Index: configure.ac
==================================================================RCS file:
/cvs/src/usr.sbin/nsd/configure.ac,v
retrieving revision 1.21
diff -u -p -r1.21 configure.ac
--- configure.ac	17 Jul 2015 17:36:33 -0000	1.21
+++ configure.ac	10 Sep 2015 11:18:41 -0000
@@ -654,12 +654,45 @@ AC_REPLACE_FUNCS(snprintf)
 AC_REPLACE_FUNCS(strlcat)
 AC_REPLACE_FUNCS(strlcpy)
 AC_REPLACE_FUNCS(strptime)
-AC_REPLACE_FUNCS(b64_pton)
-AC_REPLACE_FUNCS(b64_ntop)
 AC_REPLACE_FUNCS(pselect)
 AC_REPLACE_FUNCS(memmove)
 AC_REPLACE_FUNCS(reallocarray)
 
+#
+# Check for b64_ntop (and assume result applies to b64_pton as well).
+# The functions are actually prefixed with __ and resolv.h defines
+# macros for the unprefixed versions, however including this header
+# causes conflicts with our T_xx defines.
+#
+AC_MSG_CHECKING(for __b64_ntop)
+AC_TRY_LINK([#include <stddef.h>],
+	[__b64_ntop(NULL, 0, NULL, 0);],
+	found_b64_ntop=yes,
+	found_b64_ntop=no
+)
+if test "x$found_b64_ntop" = xno; then
+	AC_MSG_RESULT(no)
+
+	AC_MSG_CHECKING(for __b64_ntop with -lresolv)
+	LIBS="$LIBS -lresolv"
+	AC_TRY_LINK([#include <stddef.h>],
+		[__b64_ntop(NULL, 0, NULL, 0);],
+		found_b64_ntop=yes,
+		found_b64_ntop=no
+	)
+	if test "x$found_b64_ntop" = xno; then
+		AC_MSG_RESULT(no)
+	fi
+fi
+if test "x$found_b64_ntop" = xyes; then
+	AC_DEFINE(HAVE_B64_NTOP)
+	AC_DEFINE(HAVE_B64_PTON)
+	AC_MSG_RESULT(yes)
+else
+	AC_LIBOBJ([b64_ntop])
+	AC_LIBOBJ([b64_pton])
+fi
+
 AC_MSG_CHECKING(for pselect prototype in sys/select.h)
 AC_EGREP_HEADER([[^a-zA-Z_]*pselect[^a-zA-Z_]], sys/select.h,
AC_DEFINE(HAVE_PSELECT_PROTO, 1,
 	[if sys/select.h provides pselect prototype]) AC_MSG_RESULT(yes),
AC_MSG_RESULT(no))
@@ -941,13 +974,11 @@ AH_BOTTOM([
 ])
 
 AH_BOTTOM([
-#ifndef HAVE_B64_NTOP
-int b64_ntop(uint8_t const *src, size_t srclength,
+int __b64_ntop(uint8_t const *src, size_t srclength,
 	     char *target, size_t targsize);
-#endif /* !HAVE_B64_NTOP */
-#ifndef HAVE_B64_PTON
-int b64_pton(char const *src, uint8_t *target, size_t targsize);
-#endif /* !HAVE_B64_PTON */
+int __b64_pton(char const *src, uint8_t *target, size_t targsize);
+])
+AH_BOTTOM([
 #ifndef HAVE_FSEEKO
 #define fseeko fseek
 #define ftello ftell
Index: options.c
==================================================================RCS file:
/cvs/src/usr.sbin/nsd/options.c,v
retrieving revision 1.1.1.12
diff -u -p -r1.1.1.12 options.c
--- options.c	3 Feb 2015 10:24:34 -0000	1.1.1.12
+++ options.c	10 Sep 2015 11:18:41 -0000
@@ -1198,7 +1198,7 @@ key_options_setup(region_type* region, k
 		key->tsig_key->size = 0;
 		key->tsig_key->data = NULL;
 	}
-	size = b64_pton(key->secret, data, sizeof(data));
+	size = __b64_pton(key->secret, data, sizeof(data));
 	if(size == -1) {
 		log_msg(LOG_ERR, "Failed to parse tsig key data %s",
 			key->name);
Index: rdata.c
==================================================================RCS file:
/cvs/src/usr.sbin/nsd/rdata.c,v
retrieving revision 1.11
diff -u -p -r1.11 rdata.c
--- rdata.c	17 Jul 2015 17:36:33 -0000	1.11
+++ rdata.c	10 Sep 2015 11:18:41 -0000
@@ -397,7 +397,7 @@ rdata_base64_to_string(buffer_type *outp
 	if(size == 0)
 		return 1;
 	buffer_reserve(output, size * 2 + 1);
-	length = b64_ntop(rdata_atom_data(rdata), size,
+	length = __b64_ntop(rdata_atom_data(rdata), size,
 			  (char *) buffer_current(output), size * 2);
 	if (length > 0) {
 		buffer_skip(output, length);
Index: zonec.c
==================================================================RCS file:
/cvs/src/usr.sbin/nsd/zonec.c,v
retrieving revision 1.15
diff -u -p -r1.15 zonec.c
--- zonec.c	17 Jul 2015 17:36:33 -0000	1.15
+++ zonec.c	10 Sep 2015 11:18:41 -0000
@@ -639,7 +639,7 @@ zparser_conv_b64(region_type *region, co
 	uint16_t *r = NULL;
 	int i;
 
-	i = b64_pton(b64, buffer, B64BUFSIZE);
+	i = __b64_pton(b64, buffer, B64BUFSIZE);
 	if (i == -1) {
 		zc_error_prev_line("invalid base64 data");
 	} else {
Index: compat/b64_ntop.c
==================================================================RCS file:
/cvs/src/usr.sbin/nsd/compat/b64_ntop.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 b64_ntop.c
--- compat/b64_ntop.c	15 Jan 2010 19:25:08 -0000	1.1.1.1
+++ compat/b64_ntop.c	10 Sep 2015 11:24:13 -0000
@@ -123,7 +123,7 @@ static const char Pad64 = '=';
    */
 
 int
-b64_ntop(uint8_t const *src, size_t srclength, char *target, size_t targsize) {
+__b64_ntop(uint8_t const *src, size_t srclength, char *target, size_t targsize)
{
 	size_t datalength = 0;
 	uint8_t input[3];
 	uint8_t output[4];
Index: compat/b64_pton.c
==================================================================RCS file:
/cvs/src/usr.sbin/nsd/compat/b64_pton.c,v
retrieving revision 1.4
diff -u -p -r1.4 b64_pton.c
--- compat/b64_pton.c	17 Jul 2015 17:36:33 -0000	1.4
+++ compat/b64_pton.c	10 Sep 2015 11:24:13 -0000
@@ -378,7 +378,7 @@ b64_pton_len(unsigned char const *src)
 
 
 int
-b64_pton(char const *src, uint8_t *target, size_t targsize)
+__b64_pton(char const *src, uint8_t *target, size_t targsize)
 {
 	if (!b64rmap_initialized)
 		b64_initialize_rmap ();
Index: configure
==================================================================RCS file:
/cvs/src/usr.sbin/nsd/configure,v
retrieving revision 1.22
diff -u -p -r1.22 configure
--- configure	17 Jul 2015 17:36:33 -0000	1.22
+++ configure	10 Sep 2015 11:18:41 -0000
@@ -8040,34 +8040,6 @@ esac
 fi
 
 
-ac_fn_c_check_func "$LINENO" "b64_pton"
"ac_cv_func_b64_pton"
-if test "x$ac_cv_func_b64_pton" = xyes; then :
-  $as_echo "#define HAVE_B64_PTON 1" >>confdefs.h
-
-else
-  case " $LIBOBJS " in
-  *" b64_pton.$ac_objext "* ) ;;
-  *) LIBOBJS="$LIBOBJS b64_pton.$ac_objext"
- ;;
-esac
-
-fi
-
-
-ac_fn_c_check_func "$LINENO" "b64_ntop"
"ac_cv_func_b64_ntop"
-if test "x$ac_cv_func_b64_ntop" = xyes; then :
-  $as_echo "#define HAVE_B64_NTOP 1" >>confdefs.h
-
-else
-  case " $LIBOBJS " in
-  *" b64_ntop.$ac_objext "* ) ;;
-  *) LIBOBJS="$LIBOBJS b64_ntop.$ac_objext"
- ;;
-esac
-
-fi
-
-
 ac_fn_c_check_func "$LINENO" "pselect"
"ac_cv_func_pselect"
 if test "x$ac_cv_func_pselect" = xyes; then :
   $as_echo "#define HAVE_PSELECT 1" >>confdefs.h
@@ -8111,6 +8083,86 @@ fi
 
 
 
+#
+# Check for b64_ntop (and assume result applies to b64_pton as well).
+# The functions are actually prefixed with __ and resolv.h defines
+# macros for the unprefixed versions, however including this header
+# causes conflicts with our T_xx defines.
+#
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __b64_ntop"
>&5
+$as_echo_n "checking for __b64_ntop... " >&6; }
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <stddef.h>
+int
+main ()
+{
+__b64_ntop(NULL, 0, NULL, 0);
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  found_b64_ntop=yes
+else
+  found_b64_ntop=no
+
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+if test "x$found_b64_ntop" = xno; then
+	{ $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+
+	{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __b64_ntop with
-lresolv" >&5
+$as_echo_n "checking for __b64_ntop with -lresolv... " >&6; }
+	LIBS="$LIBS -lresolv"
+	cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <stddef.h>
+int
+main ()
+{
+__b64_ntop(NULL, 0, NULL, 0);
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  found_b64_ntop=yes
+else
+  found_b64_ntop=no
+
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+	if test "x$found_b64_ntop" = xno; then
+		{ $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+	fi
+fi
+if test "x$found_b64_ntop" = xyes; then
+	$as_echo "#define HAVE_B64_NTOP 1" >>confdefs.h
+
+	$as_echo "#define HAVE_B64_PTON 1" >>confdefs.h
+
+	{ $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+else
+	case " $LIBOBJS " in
+  *" b64_ntop.$ac_objext "* ) ;;
+  *) LIBOBJS="$LIBOBJS b64_ntop.$ac_objext"
+ ;;
+esac
+
+	case " $LIBOBJS " in
+  *" b64_pton.$ac_objext "* ) ;;
+  *) LIBOBJS="$LIBOBJS b64_pton.$ac_objext"
+ ;;
+esac
+
+fi
+
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for pselect prototype in
sys/select.h" >&5
 $as_echo_n "checking for pselect prototype in sys/select.h... "
>&6; }
 cat confdefs.h - <<_ACEOF >conftest.$ac_ext
@@ -8895,6 +8947,7 @@ case " $LIBOBJS " in
 esac
 
 fi
+

Ted Unangst

2015-Sep-11 02:05 UTC

head link

[nsd-users] fix compat b64_{pton,ntop} handling in nsd

Stuart Henderson wrote:> Currently NSD's compat functions for b64_pton and b64_ntop are always
> used. Worse, they're the old versions which abort(). This is because
> the library symbols are prefixed by __ (not just in OpenBSD) and are
> only accessible as b64_xx when resolv.h is included.
> 
> First attempt at fixing this involved borrowing tmux's autoconf checks
> to find these functions. This works on OpenBSD, but fails on OS with
> non-ancient resolv.h/nameser.h files which have #defines for T_xxx
> resource records which conflict with those in NSD.
> 
> So the diff below uses the __ versions directly (OpenSSH does similar).
> I've tested this on OpenBSD and Linux (SL6), including with the
autoconf
> check artificially broken to force using the compat versions.
> The autoconf check is borrowed/adapted from the tmux one.
> 
> OK for OpenBSD?
> 
> Would this be acceptable for NSD upstream?
This seems suspicious. Where are the prototypes for these function coming
from?

Stuart Henderson

2015-Sep-11 08:18 UTC

head link

[nsd-users] fix compat b64_{pton,ntop} handling in nsd

On 2015/09/10 22:05, Ted Unangst wrote:> Stuart Henderson wrote:
> > Currently NSD's compat functions for b64_pton and b64_ntop are
always
> > used. Worse, they're the old versions which abort(). This is
because
> > the library symbols are prefixed by __ (not just in OpenBSD) and are
> > only accessible as b64_xx when resolv.h is included.
> > 
> > First attempt at fixing this involved borrowing tmux's autoconf
checks
> > to find these functions. This works on OpenBSD, but fails on OS with
> > non-ancient resolv.h/nameser.h files which have #defines for T_xxx
> > resource records which conflict with those in NSD.
> > 
> > So the diff below uses the __ versions directly (OpenSSH does
similar).
> > I've tested this on OpenBSD and Linux (SL6), including with the
autoconf
> > check artificially broken to force using the compat versions.
> > The autoconf check is borrowed/adapted from the tmux one.
> > 
> > OK for OpenBSD?
> > 
> > Would this be acceptable for NSD upstream?
> 
> This seems suspicious. Where are the prototypes for these function coming
> from?
They're from the existing prototypes in nsd's configure.ac, the
parameters
are the same size as the prototypes in resolv.h at least here and on Linux,
(written differently though, uint8_t / unsigned char).

These libc functions are a complete mess anyway, do you have any ideas
on how to do it better?

nsd users - Sep 2015 - fix compat b64_{pton,ntop} handling in nsd

[nsd-users] fix compat b64_{pton,ntop} handling in nsd

[nsd-users] fix compat b64_{pton,ntop} handling in nsd

[nsd-users] fix compat b64_{pton,ntop} handling in nsd