On Wed, 2020-12-16 at 11:08 -0800, Eric Biggers wrote: > On Wed, Dec 16, 2020 at 05:27:19PM +0000, luca.boccassi@xxxxxxxxx wrote: > > From: Luca Boccassi <luca.boccassi@xxxxxxxxxxxxx> > > > > Add some minimal compat type defs, and stub out the enable/measure > > functions. Also add a way to handle the fact that mingw adds a > > .exe extension automatically in the Makefile install rules. > > > > Signed-off-by: Luca Boccassi <luca.boccassi@xxxxxxxxxxxxx> > > --- > > So this is proably going to look strange, and believe me the feeling is shared. > > It's actually the first time _ever_ I had to compile and run something on > > Windows, which is ironic in itself - the O_BINARY stuff is probably WIN32-101 and > > it took me an hour to find out. > > Anyway, I've got some groups building their payloads on Windows, so we need to > > provide native tooling. Among these are fsverity tools to get the digest and > > sign files. > > This patch stubs out and returns EOPNOTSUPP from the measure/enable functions, > > since they are linux-host only, and adds some (hopefully) minimal and unintrusive > > compat ifdefs for the rest. There's also a change in the makefile, since the > > build toolchain (yocto + mingw) for some reason automatically names executables > > as .exe. Biggest chunk is the types definitions I guess. The ugliest is the > > print stuff. > > > > Note that with this I do not ask you in any way, shape or form to be responsible > > for the correct functioning or even compiling on WIN32 of these utilities - if > > anything ever breaks, we'll find out and take care of it. I could keep all of this > > out of tree, but I figured I'd try to see if you are amenable to merge at least > > some part of it. > > > > I've tested that both Linux and WIN32 builds of digest and sign commands return > > the exact same output for the same input. > > On Linux, can you make it easily cross-compile for Windows using > 'make CC=x86_64-w64-mingw32-gcc'? That would be ideal, so that that command can > be added to scripts/run-tests.sh, so that I can make sure it stays building. > > I probably won't actually test *running* it on Windows, as that would be more > work. But building should be easy. Yes, that's how I've been compiling it - with the addition to find the openssl library and headers, as it seems on Debian mingw32-gcc doesn't define any system paths. Ie: make CC=x86_64-w64-mingw32-gcc-8.3-win32 CPPFLAGS="-I /tmp/win" LDFLAGS="-L/tmp/win" If libcrypto.dll and include/openssl are visible to mingw32-gcc out of the box, then CC is the only thing you should need. > > diff --git a/Makefile b/Makefile > > index bfe83c4..fe89e18 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -63,6 +63,7 @@ INCDIR ?= $(PREFIX)/include > > LIBDIR ?= $(PREFIX)/lib > > DESTDIR ?= > > PKGCONF ?= pkg-config > > +EXEEXT ?= > > It looks like you're requiring the caller to manually specify EXEEXT. You could > use something like the following to automatically detect when CC is MinGW and > set EXEEXT and AR appropriately: > > # Compiling for Windows with MinGW? > ifneq ($(findstring -mingw,$(shell $(CC) -dumpmachine 2>/dev/null)),) > EXEEXT := .exe > # Derive $(AR) from $(CC). > AR := $(shell echo $(CC) | \ > sed -E 's/g?cc(-?[0-9]+(\.[0-9]+)*)?(\.exe)?$$/ar\3/') > endif Done in v2, without overriding AR - it seems to work as-is. > > # Rebuild if a user-specified setting that affects the build changed. > > .build-config: FORCE > > @@ -87,9 +88,9 @@ CFLAGS += $(shell "$(PKGCONF)" libcrypto --cflags 2>/dev/null || echo) > > # If we are dynamically linking, when running tests we need to override > > # LD_LIBRARY_PATH as no RPATH is set > > ifdef USE_SHARED_LIB > > -RUN_FSVERITY = LD_LIBRARY_PATH=./ ./fsverity > > +RUN_FSVERITY = LD_LIBRARY_PATH=./ ./fsverity$(EXEEXT) > > else > > -RUN_FSVERITY = ./fsverity > > +RUN_FSVERITY = ./fsverity$(EXEEXT) > > endif > > > > ############################################################################## > > @@ -186,7 +187,7 @@ test_programs:$(TEST_PROGRAMS) > > # want to run the full tests. > > check:fsverity test_programs > > for prog in $(TEST_PROGRAMS); do \ > > - $(TEST_WRAPPER_PROG) ./$$prog || exit 1; \ > > + $(TEST_WRAPPER_PROG) ./$$prog$(EXEEXT) || exit 1; \ > > done > > $(RUN_FSVERITY) --help > /dev/null > > $(RUN_FSVERITY) --version > /dev/null > > @@ -202,7 +203,7 @@ check:fsverity test_programs > > > > install:all > > install -d $(DESTDIR)$(LIBDIR)/pkgconfig $(DESTDIR)$(INCDIR) $(DESTDIR)$(BINDIR) > > - install -m755 fsverity $(DESTDIR)$(BINDIR) > > + install -m755 fsverity$(EXEEXT) $(DESTDIR)$(BINDIR) > > install -m644 libfsverity.a $(DESTDIR)$(LIBDIR) > > install -m755 libfsverity.so.$(SOVERSION) $(DESTDIR)$(LIBDIR) > > ln -sf libfsverity.so.$(SOVERSION) $(DESTDIR)$(LIBDIR)/libfsverity.so > > @@ -215,7 +216,7 @@ install:all > > chmod 644 $(DESTDIR)$(LIBDIR)/pkgconfig/libfsverity.pc > > > > uninstall: > > - rm -f $(DESTDIR)$(BINDIR)/fsverity > > + rm -f $(DESTDIR)$(BINDIR)/fsverity$(EXEEXT) > > rm -f $(DESTDIR)$(LIBDIR)/libfsverity.a > > rm -f $(DESTDIR)$(LIBDIR)/libfsverity.so.$(SOVERSION) > > rm -f $(DESTDIR)$(LIBDIR)/libfsverity.so > > @@ -232,4 +233,4 @@ help: > > > > clean: > > rm -f $(DEFAULT_TARGETS) $(TEST_PROGRAMS) \ > > - lib/*.o programs/*.o .build-config fsverity.sig > > + fsverity$(EXEEXT) lib/*.o programs/*.o .build-config fsverity.sig > > Do you need libfsverity to be built properly for Windows (producing .dll, .lib, > and .def files), or are you just looking to build the fsverity binary? At the > moment you're just doing the latter. There are a bunch of differences between > libraries on Windows and Linux, so hopefully you don't need the library built > properly for Windows, but it would be possible. I do not need the library at the moment no, I just need the binary. > > diff --git a/common/common_defs.h b/common/common_defs.h > > index 279385a..a869532 100644 > > --- a/common/common_defs.h > > +++ b/common/common_defs.h > > @@ -15,6 +15,30 @@ > > #include <stddef.h> > > #include <stdint.h> > > > > +#ifdef _WIN32 > > +/* Some minimal definitions to allow the digest/sign commands to run under Windows */ > > +# ifndef ENOPKG > > +# define ENOPKG 65 > > +# endif > > +# ifndef __cold > > +# define __cold > > +# endif > > +typedef __signed__ char __s8; > > +typedef unsigned char __u8; > > +typedef __signed__ short __s16; > > +typedef unsigned short __u16; > > +typedef __signed__ int __s32; > > +typedef unsigned int __u32; > > +typedef __signed__ long long __s64; > > +typedef unsigned long long __u64; > > +typedef __u16 __le16; > > +typedef __u16 __be16; > > +typedef __u32 __le32; > > +typedef __u32 __be32; > > +typedef __u64 __le64; > > +typedef __u64 __be64; > > +#endif /* _WIN32 */ > > + > > typedef uint8_t u8; > > typedef uint16_t u16; > > typedef uint32_t u32; > > Could you put most of the Windows compatibility definitions in a single new > header so that they don't clutter things up too much? > > Including for things you put in other places, like O_BINARY. Added common/win32_defs.h in v2 > > diff --git a/lib/enable.c b/lib/enable.c > > index 2478077..b49ba26 100644 > > --- a/lib/enable.c > > +++ b/lib/enable.c > > @@ -11,14 +11,10 @@ > > > > #include "lib_private.h" > > > > +#ifndef _WIN32 > > + > > Could you just have the Makefile exclude files from the build instead? > > lib/enable.c > programs/cmd_measure.c > programs/cmd_enable.c > > Then in programs/fsverity.c, ifdef out the 'measure' and 'enable' commands in > fsverity_commands[]. > > I think that would be easier. Plus users won't get weird errors if they try to > use unsupported commands on Windows; the commands just won't be available. Done in v2 > > +#ifndef _WIN32 > > if (asprintf(&msg2, "%s: %s", msg, > > strerror_r(err, errbuf, sizeof(errbuf))) < 0) > > +#else > > + strerror_s(errbuf, sizeof(errbuf), err); > > + if (asprintf(&msg2, "%s: %s", msg, errbuf) < 0) > > +#endif > > goto out2; > > Instead of doing this, could you provide a strerror_r() implementation in > programs/utils.c for _WIN32? Added in v2. Thanks for the review! -- Kind regards, Luca Boccassi
Attachment:
signature.asc
Description: This is a digitally signed message part