Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 21 Jul 2016 01:09:54 +0300
From: "Dmitry V. Levin" <ldv@...linux.org>
To: owl-dev@...ts.openwall.com
Subject: Re: passwdqc code quality

On Thu, Jul 21, 2016 at 12:06:47AM +0300, Solar Designer wrote:
> On Wed, Jul 20, 2016 at 06:57:43PM +0300, Solar Designer wrote:
> > For now, I am going to switch to size_t only for local variables holding
> > strlen() return value, so that our ABI is preserved.
> 
> Speaking of ABI, I added _passwdqc_memzero() and exported it from
> libpasswdqc and made use of it in pwqcheck and pwqgen and pam_passwdqc,
> all without bumping soname.  Dmitry, do you think this is acceptable?
> Not bumping soname means package managers won't catch this, and if
> someone upgrades only some of the subpackages (in case a distro splits
> passwdqc into subpackages), but not the library, then those upgraded
> subpackages will fail with an unknown symbol error.  Not great, but
> hardly reason enough to bump soname, right?

If I could imagine libpasswdqc exporting a new symbol in addition to those
five it exports from the beginning, I would have introduced not a simple
libpasswdqc.map filter but a proper ELF symbol versioning.

Unfortunately, introduction of a symbol versioning to a library that
doesn't have one has essentially the same effect as a soname change:
all users of the library have to be relinked.

In this particular case, _passwdqc_memzero doesn't look like a good
addition to libpasswdqc ABI, so I'd rather not export it from libpasswdqc
at all, e.g.

diff --git a/passwdqc/Makefile b/passwdqc/Makefile
index e7b7a67..f5c7cb3 100644
--- a/passwdqc/Makefile
+++ b/passwdqc/Makefile
@@ -85,10 +85,10 @@ LDLIBS_pam_DARWIN = -lpam -lSystem
 CONFIGS = passwdqc.conf
 BINS = pwqgen pwqcheck
 PROJ = $(SHARED_LIB) $(DEVEL_LIB) $(SHARED_PAM) $(BINS)
-OBJS_LIB = concat.o passwdqc_check.o passwdqc_load.o passwdqc_parse.o passwdqc_random.o wordset_4k.o
-OBJS_PAM = pam_passwdqc.o
-OBJS_GEN = pwqgen.o
-OBJS_CHECK = pwqcheck.o
+OBJS_LIB = concat.o passwdqc_check.o passwdqc_load.o passwdqc_memzero.o passwdqc_parse.o passwdqc_random.o wordset_4k.o
+OBJS_PAM = pam_passwdqc.o passwdqc_memzero.o
+OBJS_GEN = pwqgen.o passwdqc_memzero.o
+OBJS_CHECK = pwqcheck.o passwdqc_memzero.o
 
 default: all
 
diff --git a/passwdqc/libpasswdqc.map b/passwdqc/libpasswdqc.map
index 2856dde..ddbb51b 100644
--- a/passwdqc/libpasswdqc.map
+++ b/passwdqc/libpasswdqc.map
@@ -7,7 +7,6 @@
     passwdqc_params_parse;
     passwdqc_params_reset;
     passwdqc_random;
-    _passwdqc_memzero;
   local:
     *;
 };
diff --git a/passwdqc/passwdqc_check.c b/passwdqc/passwdqc_check.c
index e26cc6e..c42ca2c 100644
--- a/passwdqc/passwdqc_check.c
+++ b/passwdqc/passwdqc_check.c
@@ -538,10 +538,3 @@ out:
 
 	return reason;
 }
-
-static void memzero(void *buf, size_t len)
-{
-	memset(buf, 0, len);
-}
-
-void (*_passwdqc_memzero)(void *, size_t) = memzero;
--- /dev/null
+++ b/passwdqc/passwdqc_memzero.c
@@ -0,0 +1,12 @@
+/*
+ * Copyright (c) 2016 by Solar Designer.  See LICENSE.
+ */
+
+#include <string.h>
+
+static void memzero(void *buf, size_t len)
+{
+	memset(buf, 0, len);
+}
+
+void (*_passwdqc_memzero)(void *, size_t) = memzero;

-- 
ldv

Content of type "application/pgp-signature" skipped

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.