On Sun Jun 02, 2019 at 05:41:37PM +0200, Jeremie Courreges-Anglas wrote:
> On Sun, Jun 02 2019, Rafael Sadowski <raf...@sizeofvoid.org> wrote:
> > Update keepassxc to 2.4.2
> >
> > CHANGELOG:
> > https://github.com/keepassxreboot/keepassxc/releases/tag/2.4.2
> >
> > I send the Alloc class patch upsteream as usual. Tests and feedback
> > welcome.
>
> I didn't check anything else, but the Alloc class patch shouldn't be
> sent upstream as-is, IMO. Please see below.
>
> > RS
> >
> > ===================================================================
> > RCS file: patches/patch-src_core_Alloc_cpp
> > diff -N patches/patch-src_core_Alloc_cpp
> > --- /dev/null 1 Jan 1970 00:00:00 -0000
> > +++ patches/patch-src_core_Alloc_cpp 2 Jun 2019 09:44:06 -0000
> > @@ -0,0 +1,23 @@
> > +$OpenBSD$
> > +
> > +Index: src/core/Alloc.cpp
> > +--- src/core/Alloc.cpp.orig
> > ++++ src/core/Alloc.cpp
> > +@@ -20,6 +20,8 @@
> > + #include <sodium.h>
> > + #ifdef Q_OS_MACOS
> > + #include <malloc/malloc.h>
> > ++#elif defined(Q_OS_OPENBSD)
> > ++#include <stdlib.h>
> > + #else
> > + #include <malloc.h>
> > + #endif
> > +@@ -61,7 +63,7 @@ void operator delete(void* ptr) noexcept
> > + ::operator delete(ptr, _msize(ptr));
> > + #elif defined(Q_OS_MACOS)
> > + ::operator delete(ptr, malloc_size(ptr));
> > +-#elif defined(Q_OS_UNIX)
> > ++#elif defined(Q_OS_LINUX)
> > + ::operator delete(ptr, malloc_usable_size(ptr));
> > + #else
> > + // whatever OS this is, give up and simply free stuff
> >
>
> I think the port's cmake setup should check for malloc_usable_size and
> malloc.h, use them if available, and fall back on stdlib.h / std::free()
> if not. OpenBSD is not the only OS where malloc.h isn't available, and
> Linux (glibc really) isn't the only OS where malloc_usable_size() is
> available. This kind of #ifdef practice is actively harmful for
> portability.
>
> Also if the goal is to securely erase memory before freeing it,
> maybe freezero(3) is a possible alternative?
>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
>
Thanks Jeremie for saving me from a mistake.
There is no way to fix this on OpenBSD so let's disable for now. New
diff which disabled the custom delete operator.
General question, what does it make sense to delete each heap allocated
(just allocated memory with "new") memory so?
In my opinion, we're not losing anything here.
Index: Makefile
===================================================================
RCS file: /cvs/ports/security/keepassxc/Makefile,v
retrieving revision 1.20
diff -u -p -u -p -r1.20 Makefile
--- Makefile 21 Apr 2019 11:29:44 -0000 1.20
+++ Makefile 3 Jun 2019 19:17:16 -0000
@@ -2,7 +2,7 @@
COMMENT = management tool for password and sensitive data
-V = 2.4.1
+V = 2.4.2
DISTNAME = keepassxc-${V}
CATEGORIES = security
@@ -16,7 +16,7 @@ PERMIT_PACKAGE_CDROM = Yes
WANTLIB += ${COMPILER_LIBCXX} Qt5Concurrent Qt5Core Qt5DBus Qt5Gui
WANTLIB += Qt5Network Qt5Svg Qt5Widgets Qt5X11Extras X11 Xi Xtst
-WANTLIB += argon2 c gcrypt gpg-error m qrencode z
+WANTLIB += argon2 c gcrypt gpg-error m qrencode sodium z
MASTER_SITES =
https://github.com/keepassxreboot/keepassxc/releases/download/${V}/
EXTRACT_SUFX = -src.tar.xz
@@ -25,6 +25,7 @@ MODULES = x11/qt5 \
devel/cmake
LIB_DEPENDS = security/libgcrypt \
+ security/libsodium \
security/argon2 \
graphics/libqrencode \
x11/qt5/qtsvg \
@@ -36,8 +37,8 @@ RUN_DEPENDS = devel/desktop-file-utils \
CONFIGURE_ARGS= -DCMAKE_INSTALL_MANDIR="man" \
-DWITH_GUI_TESTS=ON \
- -DWITH_XC_SSHAGENT=ON \
- -DWITH_XC_AUTOTYPE=ON
+ -DWITH_XC_AUTOTYPE=ON \
+ -DWITH_XC_SSHAGENT=ON
TEST_IS_INTERACTIVE = X11
@@ -52,10 +53,8 @@ WANTLIB += yubikey ykpers-1
.endif
.if ${FLAVOR:Mbrowser}
-LIB_DEPENDS += security/libsodium
CONFIGURE_ARGS += -DWITH_XC_BROWSER=ON \
-DWITH_XC_NETWORKING=ON
-WANTLIB += sodium
.endif
post-patch:
Index: distinfo
===================================================================
RCS file: /cvs/ports/security/keepassxc/distinfo,v
retrieving revision 1.11
diff -u -p -u -p -r1.11 distinfo
--- distinfo 21 Apr 2019 11:29:44 -0000 1.11
+++ distinfo 3 Jun 2019 19:17:16 -0000
@@ -1,2 +1,2 @@
-SHA256 (keepassxc-2.4.1-src.tar.xz) =
Dal70SedG5sGpjufcjtDq4wHi38SA9bBNQT91HNUias=
-SIZE (keepassxc-2.4.1-src.tar.xz) = 3277856
+SHA256 (keepassxc-2.4.2-src.tar.xz) =
FfptCikgVYZLGXnGcfE+W65QVuGeKAwwzBzwuW6lYTg=
+SIZE (keepassxc-2.4.2-src.tar.xz) = 3290468
Index: patches/patch-src_CMakeLists_txt
===================================================================
RCS file: patches/patch-src_CMakeLists_txt
diff -N patches/patch-src_CMakeLists_txt
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_CMakeLists_txt 3 Jun 2019 19:17:16 -0000
@@ -0,0 +1,19 @@
+$OpenBSD$
+
+https://github.com/keepassxreboot/keepassxc/pull/3020
+
+Remove the custom delete operator which zeroes out allocated memory before
+freeing it. OpenBSD does not and will implement glibc's malloc_usable_size(3)
+interface. "The main use of this function is for debugging and introspection."
+
+Index: src/CMakeLists.txt
+--- src/CMakeLists.txt.orig
++++ src/CMakeLists.txt
+@@ -24,7 +24,6 @@ if(NOT ZXCVBN_LIBRARIES)
+ endif(NOT ZXCVBN_LIBRARIES)
+
+ set(keepassx_SOURCES
+- core/Alloc.cpp
+ core/AutoTypeAssociations.cpp
+ core/AutoTypeMatch.cpp
+ core/Compare.cpp
Index: patches/patch-src_proxy_CMakeLists_txt
===================================================================
RCS file: patches/patch-src_proxy_CMakeLists_txt
diff -N patches/patch-src_proxy_CMakeLists_txt
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_proxy_CMakeLists_txt 3 Jun 2019 19:17:16 -0000
@@ -0,0 +1,19 @@
+$OpenBSD$
+
+https://github.com/keepassxreboot/keepassxc/pull/3020
+
+Remove the custom delete operator which zeroes out allocated memory before
+freeing it. OpenBSD does not and will implement glibc's malloc_usable_size(3)
+interface. "The main use of this function is for debugging and introspection."
+
+Index: src/proxy/CMakeLists.txt
+--- src/proxy/CMakeLists.txt.orig
++++ src/proxy/CMakeLists.txt
+@@ -18,7 +18,6 @@ if(WITH_XC_BROWSER)
+ include_directories(${BROWSER_SOURCE_DIR})
+
+ set(proxy_SOURCES
+- ../core/Alloc.cpp
+ keepassxc-proxy.cpp
+ ${BROWSER_SOURCE_DIR}/NativeMessagingBase.cpp
+ NativeMessagingHost.cpp)
