From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Ted Zlatanov Newsgroups: gmane.emacs.devel Subject: Re: libnettle/libhogweed WIP Date: Fri, 17 Mar 2017 18:46:29 -0400 Organization: =?utf-8?B?0KLQtdC+0LTQvtGAINCX0LvQsNGC0LDQvdC+0LI=?= @ Cienfuegos Message-ID: <874lyrbjy2.fsf@lifelogs.com> References: <83a89gq3us.fsf@gnu.org> <87bmtjiv0w.fsf_-_@lifelogs.com> <83o9xjn06c.fsf@gnu.org> <87shmeb5ln.fsf_-_@lifelogs.com> <83y3w5z1ez.fsf@gnu.org> Reply-To: emacs-devel@gnu.org NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: quoted-printable X-Trace: blaine.gmane.org 1489790847 31794 195.159.176.226 (17 Mar 2017 22:47:27 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Fri, 17 Mar 2017 22:47:27 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux) To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Mar 17 23:47:23 2017 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cp0eX-0007Zx-15 for ged-emacs-devel@m.gmane.org; Fri, 17 Mar 2017 23:47:21 +0100 Original-Received: from localhost ([::1]:51118 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cp0ec-0005PT-N7 for ged-emacs-devel@m.gmane.org; Fri, 17 Mar 2017 18:47:26 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:58326) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cp0dz-0005PI-VM for emacs-devel@gnu.org; Fri, 17 Mar 2017 18:46:49 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cp0dw-0000ec-LD for emacs-devel@gnu.org; Fri, 17 Mar 2017 18:46:47 -0400 Original-Received: from [195.159.176.226] (port=59211 helo=blaine.gmane.org) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1cp0dw-0000eK-9x for emacs-devel@gnu.org; Fri, 17 Mar 2017 18:46:44 -0400 Original-Received: from list by blaine.gmane.org with local (Exim 4.84_2) (envelope-from ) id 1cp0dn-0003CR-1z for emacs-devel@gnu.org; Fri, 17 Mar 2017 23:46:35 +0100 X-Injected-Via-Gmane: http://gmane.org/ Mail-Followup-To: emacs-devel@gnu.org Original-Lines: 289 Original-X-Complaints-To: usenet@blaine.gmane.org X-Face: bd.DQ~'29fIs`T_%O%C\g%6jW)yi[zuz6; d4V0`@y-~$#3P_Ng{@m+e4o<4P'#(_GJQ%TT= D}[Ep*b!\e,fBZ'j_+#"Ps?s2!4H2-Y"sx" Mail-Copies-To: never Cancel-Lock: sha1:tiMfCmmVbR/veH7oimwQqpM9DzY= X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 195.159.176.226 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:213101 Archived-At: On Thu, 16 Mar 2017 17:28:20 +0200 Eli Zaretskii wrote:=20 >> From: Ted Zlatanov >> Date: Wed, 15 Mar 2017 17:19:32 -0400 >> * data is not wiped after use EZ> I'm not an expert, but AFAIU this is a serious flaw in EZ> security-related software. Should this be optional (as it probably EZ> has non-negligible run-time costs)? I don't think the costs are huge, so this is definitely TODO before the code is ready. >> * it compiles but doesn't link: "error adding symbols: DSO missing from >> command line" which I hope is something trivial EZ> Did the -lhogweed -lnettle switches appear on the link command line? Here's the full monster (I just did `./autogen.sh all && configure --with-nettle && make' on a Ubuntu system): gcc -Demacs -I. -I. -I../lib -I../lib -pthread -isystem /usr/include/gtk= -3.0 -isystem /usr/include/at-spi2-atk/2.0 -isystem /usr/include/at-spi-2.0= -isystem /usr/include/dbus-1.0 -isystem /usr/lib/x86_64-linux-gnu/dbus-1.0= /include -isystem /usr/include/gtk-3.0 -isystem /usr/include/gio-unix-2.0/ = -isystem /usr/include/mirclient -isystem /usr/include/mircommon -isystem /u= sr/include/mircookie -isystem /usr/include/cairo -isystem /usr/include/pang= o-1.0 -isystem /usr/include/harfbuzz -isystem /usr/include/pango-1.0 -isyst= em /usr/include/atk-1.0 -isystem /usr/include/cairo -isystem /usr/include/p= ixman-1 -isystem /usr/include/freetype2 -isystem /usr/include/libpng16 -isy= stem /usr/include/gdk-pixbuf-2.0 -isystem /usr/include/libpng16 -isystem /u= sr/include/glib-2.0 -isystem /usr/lib/x86_64-linux-gnu/glib-2.0/include -is= ystem /usr/include/freetype2 -isystem /usr/include/alsa -pthread -isystem = /usr/include/librsvg-2.0 -isystem /usr/include/gdk-pixbuf-2.0 -isystem /usr= /include/libpng16 -isystem /usr/include/cairo -isystem /usr/include/glib-2.= 0 -isystem /usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem /usr/include= /pixman-1 -isystem /usr/include/freetype2 -isystem /usr/include/libpng16 -f= openmp -DMAGICKCORE_HDRI_ENABLE=3D0 -DMAGICKCORE_QUANTUM_DEPTH=3D16 -fopenm= p -DMAGICKCORE_HDRI_ENABLE=3D0 -DMAGICKCORE_QUANTUM_DEPTH=3D16 -isystem /us= r/include/x86_64-linux-gnu/ImageMagick-6 -isystem /usr/include/ImageMagick-= 6 -isystem /usr/include/x86_64-linux-gnu/ImageMagick-6 -isystem /usr/includ= e/ImageMagick-6 -isystem /usr/include/libpng16 -isystem /usr/include/libxml= 2 -isystem /usr/include/dbus-1.0 -isystem /usr/lib/x86_64-linux-gnu/dbus-1.= 0/include -pthread -isystem /usr/include/glib-2.0 -isystem /usr/lib/x8= 6_64-linux-gnu/glib-2.0/include -pthread -isystem /usr/include/gconf/2 -isy= stem /usr/include/dbus-1.0 -isystem /usr/lib/x86_64-linux-gnu/dbus-1.0/incl= ude -isystem /usr/include/glib-2.0 -isystem /usr/lib/x86_64-linux-gnu/glib-= 2.0/include -isystem /usr/include/glib-2.0 -isystem /usr/lib/x86_64-linux-g= nu/glib-2.0/include -isystem /usr/include/freetype2 -isystem /usr/include/f= reetype2 -isystem /usr/include/freetype2 -MMD -MF deps/.d -MP -lnettle -= isystem /usr/include/p11-kit-1 -fno-common -W -Wabi -Waddress -Waggressi= ve-loop-optimizations -Wall -Wattributes -Wbool-compare -Wbuiltin-macro-red= efined -Wcast-align -Wchar-subscripts -Wchkp -Wclobbered -Wcomment -Wcommen= ts -Wcoverage-mismatch -Wcpp -Wdate-time -Wdeprecated -Wdeprecated-declarat= ions -Wdesignated-init -Wdisabled-optimization -Wdiscarded-array-qualifiers= -Wdiscarded-qualifiers -Wdiv-by-zero -Wdouble-promotion -Wduplicated-cond = -Wempty-body -Wendif-labels -Wenum-compare -Wextra -Wformat-contains-nul -W= format-extra-args -Wformat-security -Wformat-signedness -Wformat-y2k -Wform= at-zero-length -Wframe-address -Wfree-nonheap-object -Whsa -Wignored-attrib= utes -Wignored-qualifiers -Wimplicit -Wimplicit-function-declaration -Wimpl= icit-int -Wincompatible-pointer-types -Winit-self -Wint-conversion -Wint-to= -pointer-cast -Winvalid-memory-model -Winvalid-pch -Wjump-misses-init -Wlog= ical-not-parentheses -Wlogical-op -Wmain -Wmaybe-uninitialized -Wmemset-tra= nsposed-args -Wmisleading-indentation -Wmissing-braces -Wmissing-declaratio= ns -Wmissing-include-dirs -Wmissing-parameter-type -Wmissing-prototypes -Wm= ultichar -Wnarrowing -Wnested-externs -Wnonnull -Wnonnull-compare -Wnull-de= reference -Wodr -Wold-style-declaration -Wold-style-definition -Wopenmp-sim= d -Woverflow -Woverride-init -Wpacked -Wpacked-bitfield-compat -Wparenthese= s -Wpointer-arith -Wpointer-sign -Wpointer-to-int-cast -Wpragmas -Wreturn-l= ocal-addr -Wreturn-type -Wscalar-storage-order -Wsequence-point -Wshift-cou= nt-negative -Wshift-count-overflow -Wshift-negative-value -Wsizeof-array-ar= gument -Wsizeof-pointer-memaccess -Wstrict-aliasing -Wstrict-prototypes -Ws= uggest-attribute=3Dformat -Wsuggest-attribute=3Dnoreturn -Wsuggest-final-me= thods -Wsuggest-final-types -Wswitch-bool -Wtautological-compare -Wtrampoli= nes -Wtrigraphs -Wuninitialized -Wunknown-pragmas -Wunused -Wunused-but-set= -parameter -Wunused-but-set-variable -Wunused-function -Wunused-label -Wunu= sed-local-typedefs -Wunused-macros -Wunused-result -Wunused-value -Wunused-= variable -Wvarargs -Wvariadic-macros -Wvector-operation-performance -Wvolat= ile-register-var -Wwrite-strings -Warray-bounds=3D2 -Wnormalized=3Dnfc -Wsh= ift-overflow=3D2 -Wredundant-decls -Wno-missing-field-initializers -Wno-sig= n-compare -Wno-type-limits -Wno-unused-parameter -Wno-format-nonliteral -g3= -O2 -Wl,-znocombreloc -no-pie \ -o temacs dispnew.o frame.o scroll.o xdisp.o menu.o xmenu.o window.o ch= arset.o coding.o category.o ccl.o character.o chartab.o bidi.o cm.o term.o = terminal.o xfaces.o xterm.o xfns.o xselect.o xrdb.o xsmfns.o xsettings.o gt= kutil.o emacsgtkfixed.o dbusbind.o emacs.o keyboard.o macros.o keymap.o sys= dep.o buffer.o filelock.o insdel.o marker.o minibuf.o fileio.o dired.o cmds= .o casetab.o casefiddle.o indent.o search.o regex.o undo.o alloc.o data.o d= oc.o editfns.o callint.o eval.o floatfns.o fns.o font.o print.o lread.o sy= ntax.o unexelf.o bytecode.o process.o gnutls.o nettle.o callproc.o region-c= ache.o sound.o atimer.o doprnt.o intervals.o textprop.o composite.o xml.o i= notify.o profiler.o decompress.o thread.o systhread.o sheap.o xfont.o = ftfont.o xftfont.o ftxfont.o fontset.o fringe.o image.o xgselect.o termin= fo.o lastfile.o gmalloc.o ../lib/libegnu.a -ltiff -ljpeg -lpng16 = -lgif -lXpm -lgtk-3 -lgdk-3 -lpangocairo-1.0 -lpango-1.0 -latk-1.0 -lcairo-= gobject -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0 -lSM -l= ICE -lX11 -lX11-xcb -lxcb -lXrender -lXft -lasound -lrsvg-2 -lm -lgio-2.0 -= lgdk_pixbuf-2.0 -lgobject-2.0 -lglib-2.0 -lcairo -lMagickWand-6.Q16 -lMagic= kCore-6.Q16 -lacl -lrt -ldbus-1 -lXrandr -lXinerama -lXfixes -lXext -l= xml2 -lgpm -ltinfo -lgio-2.0 -lgobject-2.0 -lglib-2.0 -lgconf-2 -lglib-2= .0 -lgobject-2.0 -lglib-2.0 -lselinux -lfreetype -lfontconfig -lfreetype -l= freetype -lotf -lfreetype -lhogweed -lgnutls -lpthread -lanl -lm -lz=20 And the output: CCLD temacs /usr/bin/ld: nettle.o: undefined reference to symbol 'nettle_hmac_set_key@@= NETTLE_6' /usr/lib/gcc/x86_64-linux-gnu/6/../../../x86_64-linux-gnu/libnettle.so: err= or adding symbols: DSO missing from command line collect2: error: ld returned 1 exit status Makefile:612: recipe for target 'temacs' failed I'm still not sure why it's failing. Is it the order of the libraries? What does the error mean? I'm sure I had it working before. EZ> . the patch is missing additions to the manual and NEWS Of course, those are TODOs. EZ> . support for dynamic loading on MS-Windows should be added Can we agree that's a followup task? I don't think it should be a blocker. I have neither the expertise nor a machine running this OS so it needs a developer. EZ> . did you consider exposing this functionality through corresponding EZ> GnuTLS functions? I'm not sure what you mean. EZ> Specific comments follow: >> + # Windows loads libnettle dynamically >> + if test "${opsys}" =3D "mingw32"; then >> + LIBNETTLE_LIBS=3D >> + else >> + CFLAGS=3D"$CFLAGS $LIBNETTLE_CFLAGS" >> + LIBS=3D"$LIBNETTLE_LIBS $LIBS" >> + fi EZ> CFLAGS should be set for the Windows build as well. Only LIBS should EZ> not be added. OK. >> diff --git a/lisp/nettle.el b/lisp/nettle.el >> new file mode 100644 >> index 0000000000..6f49c3f031 >> --- /dev/null >> +++ b/lisp/nettle.el >> @@ -0,0 +1,335 @@ >> +;;; nettle.el --- Interface to libnettle/libhogweed -*- lexical-bindin= g: t; -*- EZ> Actually, this does not seem to be an interface at all. What you have EZ> here are defcustom that seems not to be used anywhere, a bunch of EZ> diagnostic functions useful for debugging, and tests that should be EZ> moved elsewhere anyway. Do we really need this file? This will be the test code, I think. The defcustoms and functions will mostly be useful for testing and debugging. I think when I did the original patch the tests directory was not as evolved :) Once the code compiles, I'll move this to the tests. >> +;; Copyright (C) 2013 Teodor Zlatanov EZ> This should cite the FSF as the copyright holder, I think. Fixed. >> +#ifdef HAVE_NETTLE >> + syms_of_nettle (); >> +#endif EZ> syms_of_nettle should be called only if !initialized. Fixed. >> +DEFUN ("nettle-available-p", Fnettle_available_p, Snettle_available_p, = 0, 0, 0, EZ> This function should be outside of the "#ifdef HAVE_NETTLE" EZ> conditional, otherwise it will not be available in an Emacs built EZ> without the library, and Lisp programs will need to use fboundp EZ> instead, which all but defeats the purpose of the function. Yup, fixed. >> +DEFUN ("nettle-hash", Fnettle_hash, Snettle_hash, 2, 2, 0, >> + doc: /* Hash INPUT string with HASH-METHOD into a unibyte string. EZ> Here and elsewhere, the doc string should explicitly tell that INPUT EZ> must be a unibyte string. Yeah, it's a pain. EZ> A design question: would it make sense to support vectors as INPUT, EZ> here and in the rest of the functions? I think so--I was heading that way. Since I see this as a low-level stateless interface, it makes sense to require consumers to use bindat, and it would remove the ambiguity. Any objections? EZ> Another design question: should be support buffer regions, instead of EZ> strings, as input to these functions? The way your code is written, EZ> Lisp programs must always cons a string from buffer text, before they EZ> invoke these functions. This could be gratuitous cost in some use EZ> cases. Yes, but I think a low-level interface shouldn't know about buffers. This has security implications as well--it's a little bit harder to see the data that was passed to the function, but I feel that's not enough to offset the complexity of translating from buffer data to C. >> + (Lisp_Object input, Lisp_Object hash_method) >> +{ >> + Lisp_Object ret =3D Qnil; >> + >> + CHECK_STRING (input); >> + CHECK_STRING (hash_method); >> + >> + for (int i =3D 0; NULL !=3D nettle_hashes[i]; i++) >> + { >> + if (NETTLE_STRING_EQUAL_UNIBYTE (hash_method, nettle_hashes[i]->n= ame)) EZ> I don't think it's a good idea to use strings for methods and other EZ> such fixed parameters in your patch. We usually use symbols for that. EZ> The advantage of symbols is that you can test equality with EQ, EZ> instead of the costly NETTLE_STRING_EQUAL_UNIBYTE, or any equivalent EZ> code which compares strings. Right. But I can't pre-define the symbols because I don't know what hashes and ciphers will be dynamically available. So is it OK to check if the symbol name equals a C string? Is there a fast safe way to do that (considering that symbol names have a pretty rich charset)? EZ> Here and elsewhere, the size of the result is known in advance, so I EZ> would avoid allocating a scratch buffer and then copying its data EZ> (inside make_unibyte_string) into a newly-allocated string. Instead, EZ> use make_uninit_string, and then pass its string data pointer to the EZ> algorithm that produces the digest. Got it, will be a TODO once compilation works. >> + uint8_t *digest; EZ> ^^^^^^^ EZ> Why not 'unsigned char'? I was matching nettle-types.h which uses uint8_t. I've switched to unsigned char. EZ> This doc string doesn't document the ITERATIONS and SALT arguments. (x several times) Definitely TODO all these docs. >> + mode =3D CAR_SAFE (Fmember (cipher_mode, Fnettle_cipher_modes ())); EZ> Wouldn't it be less expensive to access the data on the C level, EZ> without consing a list? ... EZ> I think there are more efficient ways of doing this, which don't need EZ> an explicit loop. ... EZ> I don't understand why you copy the data here, instead of passing the EZ> algorithm a pointer to the original string's data. ... EZ> Likewise here. In addition, since you are using 'min', shouldn't we EZ> signal an error if KEY is too long? I'll put these on the TODO list. EZ> Once again, since the length is known in advance, producing output EZ> into an allocated buffer and then creating a Lisp string by copying EZ> that buffer is wasteful. It is better to produce output directly into EZ> a string's data. I don't know enough about the Lisp string internals to do this safely. I've pushed your latest suggestions (the "fixed" ones above) to the branch. I'll proceed with the TODOs (for now, simply dropped in nettle.c) as time allows but I need the compilation to work. Ted