* Imminent bugfix release (1.97.1) @ 2009-11-09 1:04 Robert Millan 2009-11-09 1:27 ` Robert Millan ` (2 more replies) 0 siblings, 3 replies; 39+ messages in thread From: Robert Millan @ 2009-11-09 1:04 UTC (permalink / raw) To: grub-devel A security problem [1] was found in our password-checking routines, which affects GRUB 1.97. I'll be releasing 1.97.1 tomorrow. Additionally, I cherry-picked fixes for a few problems that should have made it to the release, like GNU/Hurd support (see NEWS file for details). The release branch is available in: sftp://bzr.savannah.gnu.org/srv/bzr/grub/branches/release_1_97/ If you have time, please test this tree, specially password support, to help find possible problems. Thanks! [1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=555195 -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 1:04 Imminent bugfix release (1.97.1) Robert Millan @ 2009-11-09 1:27 ` Robert Millan 2009-11-09 2:08 ` Jordan Uggla 2009-11-09 13:33 ` Bean 2 siblings, 0 replies; 39+ messages in thread From: Robert Millan @ 2009-11-09 1:27 UTC (permalink / raw) To: grub-devel On Mon, Nov 09, 2009 at 02:04:22AM +0100, Robert Millan wrote: > The release branch is available in: > > sftp://bzr.savannah.gnu.org/srv/bzr/grub/branches/release_1_97/ Or via http if you don't have a Savannah account: http://bzr.savannah.gnu.org/r/grub/branches/release_1_97/ -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 1:04 Imminent bugfix release (1.97.1) Robert Millan 2009-11-09 1:27 ` Robert Millan @ 2009-11-09 2:08 ` Jordan Uggla 2009-11-09 14:15 ` Robert Millan 2009-11-09 13:33 ` Bean 2 siblings, 1 reply; 39+ messages in thread From: Jordan Uggla @ 2009-11-09 2:08 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 1376 bytes --] None of the .sh scripts ( autogen.sh and the scripts it uses ) are executable; I needed to "chmod 744 *.sh" before I could run ./autogen.sh successfully. After doing that make failed with an error in auth.c. This was with revision 1780. I've attached the output from ./configure and make. On Sun, Nov 8, 2009 at 5:04 PM, Robert Millan <rmh@aybabtu.com> wrote: > > A security problem [1] was found in our password-checking routines, > which affects GRUB 1.97. I'll be releasing 1.97.1 tomorrow. > > Additionally, I cherry-picked fixes for a few problems that should > have made it to the release, like GNU/Hurd support (see NEWS file > for details). The release branch is available in: > > sftp://bzr.savannah.gnu.org/srv/bzr/grub/branches/release_1_97/ > > If you have time, please test this tree, specially password support, > to help find possible problems. > > Thanks! > > [1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=555195 > > -- > Robert Millan > > The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and > how) you may access your data; but nobody's threatening your freedom: we > still allow you to remove your data and not access it at all." > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > [-- Attachment #2: configure.log --] [-- Type: text/x-log, Size: 4593 bytes --] checking build system type... i686-pc-linux-gnu checking host system type... i686-pc-linux-gnu checking target system type... i686-pc-linux-gnu checking for cmp... cmp checking for bison... bison checking for a BSD-compatible install... /usr/bin/install -c checking for gawk... no checking for mawk... mawk checking whether make sets $(MAKE)... yes checking for a thread-safe mkdir -p... /bin/mkdir -p checking for ruby... /usr/bin/ruby checking for makeinfo... /usr/bin/makeinfo checking for gcc... gcc checking for C compiler default output file name... a.out checking whether the C compiler works... yes checking whether we are cross compiling... no checking for suffix of executables... checking for suffix of object files... o checking whether we are using the GNU C compiler... yes checking whether gcc accepts -g... yes checking for gcc option to accept ISO C89... none needed checking how to run the C preprocessor... gcc -E checking for grep that handles long lines and -e... /bin/grep checking for egrep... /bin/grep -E checking for ANSI C header files... yes checking for sys/types.h... yes checking for sys/stat.h... yes checking for stdlib.h... yes checking for string.h... yes checking for memory.h... yes checking for strings.h... yes checking for inttypes.h... yes checking for stdint.h... yes checking for unistd.h... yes checking minix/config.h usability... no checking minix/config.h presence... no checking for minix/config.h... no checking whether it is safe to define __EXTENSIONS__... yes checking for special C compiler options needed for large files... no checking for _FILE_OFFSET_BITS value needed for large files... 64 checking whether byte ordering is bigendian... no checking size of void *... 4 checking size of long... 4 checking whether our compiler is apple cc... no checking for help2man... /usr/bin/help2man checking for posix_memalign... yes checking for memalign... yes checking for asprintf... yes checking for objcopy... objcopy checking for strip... strip checking for nm... nm checking whether optimization for size works... yes checking whether -falign-loops works... yes checking whether -fno-dwarf2-cfi-asm works... yes checking whether our target compiler is apple cc... no checking for command to convert module to ELF format... checking whether `gcc' generates calls to `__enable_execute_stack()'... no checking whether `gcc' has `-fPIE' as default... no checking whether `gcc' accepts `-fstack-protector'... yes checking whether `gcc' accepts `-mstack-arg-probe'... yes checking for __bswapsi2... yes checking for __bswapdi2... yes checking for __ashldi3... yes checking for __ashrdi3... yes checking for __lshrdi3... yes checking for __trampoline_setup... no checking for __ucmpdi2... yes checking whether target compiler is working... yes checking whether objcopy works for absolute addresses... yes checking whether linker accepts --build-id=none... yes checking if C symbols get an underscore after compilation... no checking if __bss_start is defined by the compiler... yes checking if edata is defined by the compiler... yes checking if _edata is defined by the compiler... yes checking if end is defined by the compiler... yes checking if _end is defined by the compiler... yes checking whether addr32 must be in the same line as the instruction... yes checking for .code16 addr32 assembler support... yes checking whether an absolute indirect call/jump must not be prefixed with an asterisk... no checking whether options required for efiemu work... yes checking for wgetch in -lncurses... yes checking ncurses/curses.h usability... no checking ncurses/curses.h presence... no checking for ncurses/curses.h... no checking ncurses.h usability... yes checking ncurses.h presence... yes checking for ncurses.h... yes checking for usb_claim_interface in -lusb... no checking for freetype-config... freetype-config checking whether ln can handle directories properly... yes configure: creating ./config.status config.status: creating Makefile config.status: creating gensymlist.sh config.status: creating genkernsyms.sh config.status: creating stamp-h config.status: creating config.h config.status: linking include/grub/i386 to include/grub/cpu config.status: linking include/grub/i386/pc to include/grub/machine ******************************************************* GRUB2 will be compiled with following components: Platform: i386-pc grub-emu: Yes USB support for grub-emu: No (need libusb library) With memory debugging: No efiemu runtime: Yes grub-fstest: Yes grub-mkfont: Yes ******************************************************* [-- Attachment #3: make.log --] [-- Type: text/x-log, Size: 14466 bytes --] gcc -o grub-mkimage grub_mkimage-util_i386_pc_grub_mkimage.o grub_mkimage-util_misc.o grub_mkimage-util_resolve.o grub_mkimage-lib_LzmaEnc.o grub_mkimage-lib_LzFind.o gcc -o grub-mkelfimage grub_mkelfimage-util_elf_grub_mkimage.o grub_mkelfimage-util_misc.o grub_mkelfimage-util_resolve.o rm -f grub_fstest_init.lst; grep GRUB_MOD_INIT util/grub-fstest.c util/hostfs.c util/misc.c kern/file.c kern/device.c kern/disk.c kern/err.c kern/misc.c disk/host.c disk/loopback.c kern/list.c kern/command.c lib/arg.c commands/extcmd.c normal/datetime.c normal/misc.c lib/hexdump.c lib/crc.c commands/blocklist.c commands/ls.c fs/affs.c fs/cpio.c fs/fat.c fs/ext2.c fs/hfs.c fs/hfsplus.c fs/iso9660.c fs/udf.c fs/jfs.c fs/minix.c fs/ntfs.c fs/ntfscomp.c fs/reiserfs.c fs/sfs.c fs/ufs.c fs/ufs2.c fs/xfs.c fs/afs.c fs/afs_be.c fs/befs.c fs/befs_be.c fs/tar.c kern/partition.c partmap/msdos.c partmap/apple.c partmap/sun.c partmap/gpt.c kern/fs.c kern/env.c fs/fshelp.c disk/raid.c disk/raid5_recover.c disk/raid6_recover.c disk/mdraid_linux.c disk/dmraid_nvidia.c disk/lvm.c /dev/null > grub_fstest_init.lst rm -f grub_fstest_init.h; sh ./geninitheader.sh grub_fstest_init.lst > grub_fstest_init.h gcc -Iutil -I./util -I. -I./include -I./include -Wall -W -DGRUB_LIBDIR=\"/usr/local/lib/`echo grub/i386-pc | sed 's,x,x,'`\" -g -O2 -DGRUB_UTIL=1 -MD -c -o grub_fstest-util_grub_fstest.o util/grub-fstest.c rm -f grub_fstest_init.c; sh ./geninit.sh grub_fstest_init.lst util/grub-fstest.c util/hostfs.c util/misc.c kern/file.c kern/device.c kern/disk.c kern/err.c kern/misc.c disk/host.c disk/loopback.c kern/list.c kern/command.c lib/arg.c commands/extcmd.c normal/datetime.c normal/misc.c lib/hexdump.c lib/crc.c commands/blocklist.c commands/ls.c fs/affs.c fs/cpio.c fs/fat.c fs/ext2.c fs/hfs.c fs/hfsplus.c fs/iso9660.c fs/udf.c fs/jfs.c fs/minix.c fs/ntfs.c fs/ntfscomp.c fs/reiserfs.c fs/sfs.c fs/ufs.c fs/ufs2.c fs/xfs.c fs/afs.c fs/afs_be.c fs/befs.c fs/befs_be.c fs/tar.c kern/partition.c partmap/msdos.c partmap/apple.c partmap/sun.c partmap/gpt.c kern/fs.c kern/env.c fs/fshelp.c disk/raid.c disk/raid5_recover.c disk/raid6_recover.c disk/mdraid_linux.c disk/dmraid_nvidia.c disk/lvm.c > grub_fstest_init.c gcc -I. -I./. -I. -I./include -I./include -Wall -W -DGRUB_LIBDIR=\"/usr/local/lib/`echo grub/i386-pc | sed 's,x,x,'`\" -g -O2 -DGRUB_UTIL=1 -MD -c -o grub_fstest-grub_fstest_init.o grub_fstest_init.c gcc -o grub-fstest grub_fstest-util_grub_fstest.o grub_fstest-util_hostfs.o grub_fstest-util_misc.o grub_fstest-kern_file.o grub_fstest-kern_device.o grub_fstest-kern_disk.o grub_fstest-kern_err.o grub_fstest-kern_misc.o grub_fstest-disk_host.o grub_fstest-disk_loopback.o grub_fstest-kern_list.o grub_fstest-kern_command.o grub_fstest-lib_arg.o grub_fstest-commands_extcmd.o grub_fstest-normal_datetime.o grub_fstest-normal_misc.o grub_fstest-lib_hexdump.o grub_fstest-lib_crc.o grub_fstest-commands_blocklist.o grub_fstest-commands_ls.o grub_fstest-fs_affs.o grub_fstest-fs_cpio.o grub_fstest-fs_fat.o grub_fstest-fs_ext2.o grub_fstest-fs_hfs.o grub_fstest-fs_hfsplus.o grub_fstest-fs_iso9660.o grub_fstest-fs_udf.o grub_fstest-fs_jfs.o grub_fstest-fs_minix.o grub_fstest-fs_ntfs.o grub_fstest-fs_ntfscomp.o grub_fstest-fs_reiserfs.o grub_fstest-fs_sfs.o grub_fstest-fs_ufs.o grub_fstest-fs_ufs2.o grub_fstest-fs_xfs.o grub_fstest-fs_afs.o grub_fstest-fs_afs_be.o grub_fstest-fs_befs.o grub_fstest-fs_befs_be.o grub_fstest-fs_tar.o grub_fstest-kern_partition.o grub_fstest-partmap_msdos.o grub_fstest-partmap_apple.o grub_fstest-partmap_sun.o grub_fstest-partmap_gpt.o grub_fstest-kern_fs.o grub_fstest-kern_env.o grub_fstest-fs_fshelp.o grub_fstest-disk_raid.o grub_fstest-disk_raid5_recover.o grub_fstest-disk_raid6_recover.o grub_fstest-disk_mdraid_linux.o grub_fstest-disk_dmraid_nvidia.o grub_fstest-disk_lvm.o grub_fstest-grub_fstest_init.o gcc -o grub-mkfont grub_mkfont-util_grub_mkfont.o grub_mkfont-util_misc.o -lfreetype -lz gcc -o grub-editenv grub_editenv-util_grub_editenv.o grub_editenv-lib_envblk.o grub_editenv-util_misc.o grub_editenv-kern_misc.o grub_editenv-kern_err.o rm -f grub_setup_init.lst; grep GRUB_MOD_INIT util/i386/pc/grub-setup.c util/hostdisk.c util/misc.c util/getroot.c kern/device.c kern/disk.c kern/err.c kern/misc.c kern/parser.c kern/partition.c kern/file.c kern/fs.c kern/env.c fs/fshelp.c fs/affs.c fs/cpio.c fs/ext2.c fs/fat.c fs/hfs.c fs/hfsplus.c fs/iso9660.c fs/udf.c fs/jfs.c fs/minix.c fs/ntfs.c fs/ntfscomp.c fs/reiserfs.c fs/sfs.c fs/ufs.c fs/ufs2.c fs/xfs.c fs/afs.c fs/afs_be.c fs/befs.c fs/befs_be.c fs/tar.c partmap/msdos.c partmap/gpt.c disk/raid.c disk/mdraid_linux.c disk/lvm.c util/raid.c util/lvm.c /dev/null > grub_setup_init.lst rm -f grub_setup_init.h; sh ./geninitheader.sh grub_setup_init.lst > grub_setup_init.h gcc -Iutil/i386/pc -I./util/i386/pc -I. -I./include -I./include -Wall -W -DGRUB_LIBDIR=\"/usr/local/lib/`echo grub/i386-pc | sed 's,x,x,'`\" -g -O2 -DGRUB_UTIL=1 -MD -c -o grub_setup-util_i386_pc_grub_setup.o util/i386/pc/grub-setup.c rm -f grub_setup_init.c; sh ./geninit.sh grub_setup_init.lst util/i386/pc/grub-setup.c util/hostdisk.c util/misc.c util/getroot.c kern/device.c kern/disk.c kern/err.c kern/misc.c kern/parser.c kern/partition.c kern/file.c kern/fs.c kern/env.c fs/fshelp.c fs/affs.c fs/cpio.c fs/ext2.c fs/fat.c fs/hfs.c fs/hfsplus.c fs/iso9660.c fs/udf.c fs/jfs.c fs/minix.c fs/ntfs.c fs/ntfscomp.c fs/reiserfs.c fs/sfs.c fs/ufs.c fs/ufs2.c fs/xfs.c fs/afs.c fs/afs_be.c fs/befs.c fs/befs_be.c fs/tar.c partmap/msdos.c partmap/gpt.c disk/raid.c disk/mdraid_linux.c disk/lvm.c util/raid.c util/lvm.c > grub_setup_init.c gcc -I. -I./. -I. -I./include -I./include -Wall -W -DGRUB_LIBDIR=\"/usr/local/lib/`echo grub/i386-pc | sed 's,x,x,'`\" -g -O2 -DGRUB_UTIL=1 -MD -c -o grub_setup-grub_setup_init.o grub_setup_init.c gcc -o grub-setup grub_setup-util_i386_pc_grub_setup.o grub_setup-util_hostdisk.o grub_setup-util_misc.o grub_setup-util_getroot.o grub_setup-kern_device.o grub_setup-kern_disk.o grub_setup-kern_err.o grub_setup-kern_misc.o grub_setup-kern_parser.o grub_setup-kern_partition.o grub_setup-kern_file.o grub_setup-kern_fs.o grub_setup-kern_env.o grub_setup-fs_fshelp.o grub_setup-fs_affs.o grub_setup-fs_cpio.o grub_setup-fs_ext2.o grub_setup-fs_fat.o grub_setup-fs_hfs.o grub_setup-fs_hfsplus.o grub_setup-fs_iso9660.o grub_setup-fs_udf.o grub_setup-fs_jfs.o grub_setup-fs_minix.o grub_setup-fs_ntfs.o grub_setup-fs_ntfscomp.o grub_setup-fs_reiserfs.o grub_setup-fs_sfs.o grub_setup-fs_ufs.o grub_setup-fs_ufs2.o grub_setup-fs_xfs.o grub_setup-fs_afs.o grub_setup-fs_afs_be.o grub_setup-fs_befs.o grub_setup-fs_befs_be.o grub_setup-fs_tar.o grub_setup-partmap_msdos.o grub_setup-partmap_gpt.o grub_setup-disk_raid.o grub_setup-disk_mdraid_linux.o grub_setup-disk_lvm.o grub_setup-util_raid.o grub_setup-util_lvm.o grub_setup-grub_setup_init.o gcc -o grub-mkdevicemap grub_mkdevicemap-util_grub_mkdevicemap.o grub_mkdevicemap-util_deviceiter.o grub_mkdevicemap-util_devicemap.o grub_mkdevicemap-util_misc.o bison -d -p grub_script_yy -b grub_script ./script/sh/parser.y gcc -Iscript/sh -I./script/sh -I. -I./include -I./include -Wall -W -DGRUB_LIBDIR=\"/usr/local/lib/`echo grub/i386-pc | sed 's,x,x,'`\" -g -O2 -DGRUB_UTIL=1 -MD -c -o grub_emu-script_sh_lexer.o script/sh/lexer.c gcc -I. -I./. -I. -I./include -I./include -Wall -W -DGRUB_LIBDIR=\"/usr/local/lib/`echo grub/i386-pc | sed 's,x,x,'`\" -g -O2 -DGRUB_UTIL=1 -MD -c -o grub_emu-grub_script_tab.o grub_script.tab.c rm -f grub_emu_init.lst; grep GRUB_MOD_INIT commands/minicmd.c commands/cat.c commands/cmp.c commands/configfile.c commands/echo.c commands/help.c commands/handler.c commands/ls.c commands/test.c commands/search.c commands/blocklist.c commands/hexdump.c lib/hexdump.c commands/i386/pc/halt.c commands/reboot.c lib/envblk.c commands/loadenv.c commands/gptsync.c commands/probe.c commands/xnu_uuid.c commands/i386/cpuid.c commands/password.c commands/keystatus.c disk/host.c disk/loopback.c disk/scsi.c fs/fshelp.c io/gzio.c kern/device.c kern/disk.c kern/dl.c kern/elf.c kern/env.c kern/err.c kern/list.c kern/handler.c kern/command.c kern/corecmd.c commands/extcmd.c kern/file.c kern/fs.c commands/boot.c kern/main.c kern/misc.c kern/parser.c kern/partition.c kern/reader.c kern/term.c kern/rescue_reader.c kern/rescue_parser.c lib/arg.c normal/cmdline.c normal/datetime.c normal/misc.c normal/handler.c normal/auth.c normal/autofs.c normal/completion.c normal/main.c normal/color.c normal/menu.c normal/menu_entry.c normal/menu_viewer.c normal/menu_text.c script/sh/main.c script/sh/execute.c script/sh/function.c script/sh/lexer.c script/sh/script.c grub_script.tab.c partmap/amiga.c partmap/apple.c partmap/msdos.c partmap/sun.c partmap/acorn.c partmap/gpt.c fs/affs.c fs/cpio.c fs/fat.c fs/ext2.c fs/hfs.c fs/hfsplus.c fs/iso9660.c fs/udf.c fs/jfs.c fs/minix.c fs/ntfs.c fs/ntfscomp.c fs/reiserfs.c fs/sfs.c fs/ufs.c fs/ufs2.c fs/xfs.c fs/afs.c fs/afs_be.c fs/befs.c fs/befs_be.c fs/tar.c util/console.c util/hostfs.c util/grub-emu.c util/misc.c util/hostdisk.c util/getroot.c disk/raid.c disk/raid5_recover.c disk/raid6_recover.c disk/mdraid_linux.c disk/dmraid_nvidia.c disk/lvm.c commands/parttool.c parttool/msdospart.c /dev/null > grub_emu_init.lst rm -f grub_emu_init.h; sh ./geninitheader.sh grub_emu_init.lst > grub_emu_init.h gcc -Iutil -I./util -I. -I./include -I./include -Wall -W -DGRUB_LIBDIR=\"/usr/local/lib/`echo grub/i386-pc | sed 's,x,x,'`\" -g -O2 -DGRUB_UTIL=1 -MD -c -o grub_emu-util_grub_emu.o util/grub-emu.c rm -f grub_emu_init.c; sh ./geninit.sh grub_emu_init.lst commands/minicmd.c commands/cat.c commands/cmp.c commands/configfile.c commands/echo.c commands/help.c commands/handler.c commands/ls.c commands/test.c commands/search.c commands/blocklist.c commands/hexdump.c lib/hexdump.c commands/i386/pc/halt.c commands/reboot.c lib/envblk.c commands/loadenv.c commands/gptsync.c commands/probe.c commands/xnu_uuid.c commands/i386/cpuid.c commands/password.c commands/keystatus.c disk/host.c disk/loopback.c disk/scsi.c fs/fshelp.c io/gzio.c kern/device.c kern/disk.c kern/dl.c kern/elf.c kern/env.c kern/err.c kern/list.c kern/handler.c kern/command.c kern/corecmd.c commands/extcmd.c kern/file.c kern/fs.c commands/boot.c kern/main.c kern/misc.c kern/parser.c kern/partition.c kern/reader.c kern/term.c kern/rescue_reader.c kern/rescue_parser.c lib/arg.c normal/cmdline.c normal/datetime.c normal/misc.c normal/handler.c normal/auth.c normal/autofs.c normal/completion.c normal/main.c normal/color.c normal/menu.c normal/menu_entry.c normal/menu_viewer.c normal/menu_text.c script/sh/main.c script/sh/execute.c script/sh/function.c script/sh/lexer.c script/sh/script.c grub_script.tab.c partmap/amiga.c partmap/apple.c partmap/msdos.c partmap/sun.c partmap/acorn.c partmap/gpt.c fs/affs.c fs/cpio.c fs/fat.c fs/ext2.c fs/hfs.c fs/hfsplus.c fs/iso9660.c fs/udf.c fs/jfs.c fs/minix.c fs/ntfs.c fs/ntfscomp.c fs/reiserfs.c fs/sfs.c fs/ufs.c fs/ufs2.c fs/xfs.c fs/afs.c fs/afs_be.c fs/befs.c fs/befs_be.c fs/tar.c util/console.c util/hostfs.c util/grub-emu.c util/misc.c util/hostdisk.c util/getroot.c disk/raid.c disk/raid5_recover.c disk/raid6_recover.c disk/mdraid_linux.c disk/dmraid_nvidia.c disk/lvm.c commands/parttool.c parttool/msdospart.c > grub_emu_init.c gcc -I. -I./. -I. -I./include -I./include -Wall -W -DGRUB_LIBDIR=\"/usr/local/lib/`echo grub/i386-pc | sed 's,x,x,'`\" -g -O2 -DGRUB_UTIL=1 -MD -c -o grub_emu-grub_emu_init.o grub_emu_init.c gcc -o grub-emu grub_emu-commands_minicmd.o grub_emu-commands_cat.o grub_emu-commands_cmp.o grub_emu-commands_configfile.o grub_emu-commands_echo.o grub_emu-commands_help.o grub_emu-commands_handler.o grub_emu-commands_ls.o grub_emu-commands_test.o grub_emu-commands_search.o grub_emu-commands_blocklist.o grub_emu-commands_hexdump.o grub_emu-lib_hexdump.o grub_emu-commands_i386_pc_halt.o grub_emu-commands_reboot.o grub_emu-lib_envblk.o grub_emu-commands_loadenv.o grub_emu-commands_gptsync.o grub_emu-commands_probe.o grub_emu-commands_xnu_uuid.o grub_emu-commands_i386_cpuid.o grub_emu-commands_password.o grub_emu-commands_keystatus.o grub_emu-disk_host.o grub_emu-disk_loopback.o grub_emu-disk_scsi.o grub_emu-fs_fshelp.o grub_emu-io_gzio.o grub_emu-kern_device.o grub_emu-kern_disk.o grub_emu-kern_dl.o grub_emu-kern_elf.o grub_emu-kern_env.o grub_emu-kern_err.o grub_emu-kern_list.o grub_emu-kern_handler.o grub_emu-kern_command.o grub_emu-kern_corecmd.o grub_emu-commands_extcmd.o grub_emu-kern_file.o grub_emu-kern_fs.o grub_emu-commands_boot.o grub_emu-kern_main.o grub_emu-kern_misc.o grub_emu-kern_parser.o grub_emu-kern_partition.o grub_emu-kern_reader.o grub_emu-kern_term.o grub_emu-kern_rescue_reader.o grub_emu-kern_rescue_parser.o grub_emu-lib_arg.o grub_emu-normal_cmdline.o grub_emu-normal_datetime.o grub_emu-normal_misc.o grub_emu-normal_handler.o grub_emu-normal_auth.o grub_emu-normal_autofs.o grub_emu-normal_completion.o grub_emu-normal_main.o grub_emu-normal_color.o grub_emu-normal_menu.o grub_emu-normal_menu_entry.o grub_emu-normal_menu_viewer.o grub_emu-normal_menu_text.o grub_emu-script_sh_main.o grub_emu-script_sh_execute.o grub_emu-script_sh_function.o grub_emu-script_sh_lexer.o grub_emu-script_sh_script.o grub_emu-grub_script_tab.o grub_emu-partmap_amiga.o grub_emu-partmap_apple.o grub_emu-partmap_msdos.o grub_emu-partmap_sun.o grub_emu-partmap_acorn.o grub_emu-partmap_gpt.o grub_emu-fs_affs.o grub_emu-fs_cpio.o grub_emu-fs_fat.o grub_emu-fs_ext2.o grub_emu-fs_hfs.o grub_emu-fs_hfsplus.o grub_emu-fs_iso9660.o grub_emu-fs_udf.o grub_emu-fs_jfs.o grub_emu-fs_minix.o grub_emu-fs_ntfs.o grub_emu-fs_ntfscomp.o grub_emu-fs_reiserfs.o grub_emu-fs_sfs.o grub_emu-fs_ufs.o grub_emu-fs_ufs2.o grub_emu-fs_xfs.o grub_emu-fs_afs.o grub_emu-fs_afs_be.o grub_emu-fs_befs.o grub_emu-fs_befs_be.o grub_emu-fs_tar.o grub_emu-util_console.o grub_emu-util_hostfs.o grub_emu-util_grub_emu.o grub_emu-util_misc.o grub_emu-util_hostdisk.o grub_emu-util_getroot.o grub_emu-disk_raid.o grub_emu-disk_raid5_recover.o grub_emu-disk_raid6_recover.o grub_emu-disk_mdraid_linux.o grub_emu-disk_dmraid_nvidia.o grub_emu-disk_lvm.o grub_emu-commands_parttool.o grub_emu-parttool_msdospart.o grub_emu-grub_emu_init.o -lncurses grub_emu-normal_auth.o: In function `grub_auth_check_authentication': /home/jordan/grub-1.97.1/release_1_97/normal/auth.c:266: undefined reference to `grub_sleep' collect2: ld returned 1 exit status make: *** [grub-emu] Error 1 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 2:08 ` Jordan Uggla @ 2009-11-09 14:15 ` Robert Millan 0 siblings, 0 replies; 39+ messages in thread From: Robert Millan @ 2009-11-09 14:15 UTC (permalink / raw) To: The development of GNU GRUB On Sun, Nov 08, 2009 at 06:08:39PM -0800, Jordan Uggla wrote: > None of the .sh scripts ( autogen.sh and the scripts it uses ) are > executable; I needed to "chmod 744 *.sh" before I could run > ./autogen.sh successfully. After doing that make failed with an error > in auth.c. This was with revision 1780. I've attached the output from > ./configure and make. This was fixed in trunk, but release_1_97 didn't have it. I'll cherry-pick that one... -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 1:04 Imminent bugfix release (1.97.1) Robert Millan 2009-11-09 1:27 ` Robert Millan 2009-11-09 2:08 ` Jordan Uggla @ 2009-11-09 13:33 ` Bean 2009-11-09 13:50 ` Vladimir 'phcoder' Serbinenko 2 siblings, 1 reply; 39+ messages in thread From: Bean @ 2009-11-09 13:33 UTC (permalink / raw) To: The development of GNU GRUB On Mon, Nov 9, 2009 at 9:04 AM, Robert Millan <rmh@aybabtu.com> wrote: > > A security problem [1] was found in our password-checking routines, > which affects GRUB 1.97. I'll be releasing 1.97.1 tomorrow. > > Additionally, I cherry-picked fixes for a few problems that should > have made it to the release, like GNU/Hurd support (see NEWS file > for details). The release branch is available in: > > sftp://bzr.savannah.gnu.org/srv/bzr/grub/branches/release_1_97/ > > If you have time, please test this tree, specially password support, > to help find possible problems. Hi, Actually, the function of grub_auth_strcmp puzzles me, why would it need to wait 100 ms to return the result ? grub_auth_strcmp is used in many place, so the authorized could take some time to complete. And there is a hidden issue in it, grub_auth_strcmp can accept NULL pointer as input, but grub_strcmp doesn't check for NULL pointer. -- Bean My repository: https://launchpad.net/burg Document: https://help.ubuntu.com/community/Burg ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 13:33 ` Bean @ 2009-11-09 13:50 ` Vladimir 'phcoder' Serbinenko 2009-11-09 14:18 ` Robert Millan 2009-11-09 14:21 ` Bean 0 siblings, 2 replies; 39+ messages in thread From: Vladimir 'phcoder' Serbinenko @ 2009-11-09 13:50 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 1294 bytes --] Bean wrote: > On Mon, Nov 9, 2009 at 9:04 AM, Robert Millan <rmh@aybabtu.com> wrote: > >> A security problem [1] was found in our password-checking routines, >> which affects GRUB 1.97. I'll be releasing 1.97.1 tomorrow. >> >> Additionally, I cherry-picked fixes for a few problems that should >> have made it to the release, like GNU/Hurd support (see NEWS file >> for details). The release branch is available in: >> >> sftp://bzr.savannah.gnu.org/srv/bzr/grub/branches/release_1_97/ >> >> If you have time, please test this tree, specially password support, >> to help find possible problems. >> > > Hi, > > Actually, the function of grub_auth_strcmp puzzles me, why would it > need to wait 100 ms to return the result ? 10 ms actually. The goal is to take same amount of time indpendently of input values. But probably the delay should be around whole thing and it's how I'll do but for this urgent release this will do it > grub_auth_strcmp is used in > many place, so the authorized could take some time to complete. And > there is a hidden issue in it, grub_auth_strcmp can accept NULL > pointer as input, but grub_strcmp doesn't check for NULL pointer. > > Current codebase didn't use it -- Regards Vladimir 'phcoder' Serbinenko [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 293 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 13:50 ` Vladimir 'phcoder' Serbinenko @ 2009-11-09 14:18 ` Robert Millan 2009-11-09 14:21 ` Bean 1 sibling, 0 replies; 39+ messages in thread From: Robert Millan @ 2009-11-09 14:18 UTC (permalink / raw) To: The development of GNU GRUB On Mon, Nov 09, 2009 at 02:50:36PM +0100, Vladimir 'phcoder' Serbinenko wrote: > > > > Actually, the function of grub_auth_strcmp puzzles me, why would it > > need to wait 100 ms to return the result ? > 10 ms actually. The goal is to take same amount of time indpendently of > input values. But probably the delay should be around whole thing and > it's how I'll do but for this urgent release this will do it Yes. I realized that too this morning, but it's not so important. The previous approach was tested and I'll release 1.97.1 with it, afterwards (or previously in trunk, thankfully we have branches now) we can fix it to do the right thing. -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 13:50 ` Vladimir 'phcoder' Serbinenko 2009-11-09 14:18 ` Robert Millan @ 2009-11-09 14:21 ` Bean 2009-11-09 14:34 ` Vladimir 'phcoder' Serbinenko 1 sibling, 1 reply; 39+ messages in thread From: Bean @ 2009-11-09 14:21 UTC (permalink / raw) To: The development of GNU GRUB On Mon, Nov 9, 2009 at 9:50 PM, Vladimir 'phcoder' Serbinenko <phcoder@gmail.com> wrote: > Bean wrote: >> On Mon, Nov 9, 2009 at 9:04 AM, Robert Millan <rmh@aybabtu.com> wrote: >> >>> A security problem [1] was found in our password-checking routines, >>> which affects GRUB 1.97. I'll be releasing 1.97.1 tomorrow. >>> >>> Additionally, I cherry-picked fixes for a few problems that should >>> have made it to the release, like GNU/Hurd support (see NEWS file >>> for details). The release branch is available in: >>> >>> sftp://bzr.savannah.gnu.org/srv/bzr/grub/branches/release_1_97/ >>> >>> If you have time, please test this tree, specially password support, >>> to help find possible problems. >>> >> >> Hi, >> >> Actually, the function of grub_auth_strcmp puzzles me, why would it >> need to wait 100 ms to return the result ? > 10 ms actually. The goal is to take same amount of time indpendently of > input values. But probably the delay should be around whole thing and > it's how I'll do but for this urgent release this will do it Hi, int grub_auth_strcmp (const char *s1, const char *s2) { int ret; grub_uint64_t end; end = grub_get_time_ms () + 100; ret = grub_strcmp (s1, s2); /* This prevents an attacker from deriving information about the password from the time it took to execute this function. */ while (grub_get_time_ms () < end); return ret; } Isn't this 100 ms ? Anyway, the longest supported string is 1024 long, I doubt there is any perceivable difference between them. -- Bean My repository: https://launchpad.net/burg Document: https://help.ubuntu.com/community/Burg ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 14:21 ` Bean @ 2009-11-09 14:34 ` Vladimir 'phcoder' Serbinenko 2009-11-09 17:46 ` Duboucher Thomas 0 siblings, 1 reply; 39+ messages in thread From: Vladimir 'phcoder' Serbinenko @ 2009-11-09 14:34 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 1850 bytes --] Bean wrote: > On Mon, Nov 9, 2009 at 9:50 PM, Vladimir 'phcoder' Serbinenko > <phcoder@gmail.com> wrote: > >> Bean wrote: >> >>> On Mon, Nov 9, 2009 at 9:04 AM, Robert Millan <rmh@aybabtu.com> wrote: >>> >>> >>>> A security problem [1] was found in our password-checking routines, >>>> which affects GRUB 1.97. I'll be releasing 1.97.1 tomorrow. >>>> >>>> Additionally, I cherry-picked fixes for a few problems that should >>>> have made it to the release, like GNU/Hurd support (see NEWS file >>>> for details). The release branch is available in: >>>> >>>> sftp://bzr.savannah.gnu.org/srv/bzr/grub/branches/release_1_97/ >>>> >>>> If you have time, please test this tree, specially password support, >>>> to help find possible problems. >>>> >>>> >>> Hi, >>> >>> Actually, the function of grub_auth_strcmp puzzles me, why would it >>> need to wait 100 ms to return the result ? >>> >> 10 ms actually. The goal is to take same amount of time indpendently of >> input values. But probably the delay should be around whole thing and >> it's how I'll do but for this urgent release this will do it >> > > Hi, > > int > grub_auth_strcmp (const char *s1, const char *s2) > { > int ret; > grub_uint64_t end; > > end = grub_get_time_ms () + 100; > ret = grub_strcmp (s1, s2); > > /* This prevents an attacker from deriving information about the > password from the time it took to execute this function. */ > while (grub_get_time_ms () < end); > > return ret; > } > > Isn't this 100 ms ? Anyway, the longest supported string is 1024 long, > I doubt there is any perceivable difference between them. > > If attacker is on fast serial connection he could possibly measure the difference -- Regards Vladimir 'phcoder' Serbinenko [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 293 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 14:34 ` Vladimir 'phcoder' Serbinenko @ 2009-11-09 17:46 ` Duboucher Thomas 2009-11-09 18:10 ` Robert Millan 0 siblings, 1 reply; 39+ messages in thread From: Duboucher Thomas @ 2009-11-09 17:46 UTC (permalink / raw) To: The development of GNU GRUB -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Vladimir 'phcoder' Serbinenko a écrit : > Bean wrote: >> On Mon, Nov 9, 2009 at 9:50 PM, Vladimir 'phcoder' Serbinenko >> <phcoder@gmail.com> wrote: >> >> Hi, >> >> int >> grub_auth_strcmp (const char *s1, const char *s2) >> { >> int ret; >> grub_uint64_t end; >> >> end = grub_get_time_ms () + 100; >> ret = grub_strcmp (s1, s2); >> >> /* This prevents an attacker from deriving information about the >> password from the time it took to execute this function. */ >> while (grub_get_time_ms () < end); >> >> return ret; >> } >> >> Isn't this 100 ms ? Anyway, the longest supported string is 1024 long, >> I doubt there is any perceivable difference between them. >> >> > If attacker is on fast serial connection he could possibly measure the > difference > Hi, I'm not very confident in this piece of code. I would rather go with something like int grub_auth_strcmp (const char *s1, const char *s2) { int n; volatile int ret = 0; for (n = grub_strlen (s1); n >= 0; n--) { if (*s1 != *s2) ret |= 1; else ret |= 0; s1++; s2++; } return ret; } Ok, I typed this in a few minutes and I'm not confident either with what I wrote; I would check that it works first. ;) But the point here is that whatever the user gives as an input, it is executed exactly n-th times, n being the length of the user input; and that whatever the result of the 'if' statement is, the CPU realizes the same amount of operations. By doing so, the attacker will only find out how long it takes to make the comparison with a n caracters long input. Thomas. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr4VWgACgkQBV7eXqefhqhcIQCgpviYSvNqGqH3fi2Td4QChsam JWQAni9R7zOWFMGBu5X9rXWOjenIXx31 =vnph -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 17:46 ` Duboucher Thomas @ 2009-11-09 18:10 ` Robert Millan 2009-11-09 18:15 ` Vladimir 'phcoder' Serbinenko 0 siblings, 1 reply; 39+ messages in thread From: Robert Millan @ 2009-11-09 18:10 UTC (permalink / raw) To: The development of GNU GRUB On Mon, Nov 09, 2009 at 06:46:16PM +0100, Duboucher Thomas wrote: > > Ok, I typed this in a few minutes and I'm not confident either with > what I wrote; I would check that it works first. ;) > But the point here is that whatever the user gives as an input, it is > executed exactly n-th times, n being the length of the user input; and > that whatever the result of the 'if' statement is, the CPU realizes the > same amount of operations. By doing so, the attacker will only find out > how long it takes to make the comparison with a n caracters long input. Actually, modern CPUs are very complex and the number of operations (or time taken by them) isn't easy to predict. -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 18:10 ` Robert Millan @ 2009-11-09 18:15 ` Vladimir 'phcoder' Serbinenko 2009-11-09 18:25 ` Robert Millan 0 siblings, 1 reply; 39+ messages in thread From: Vladimir 'phcoder' Serbinenko @ 2009-11-09 18:15 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 1259 bytes --] Robert Millan wrote: > On Mon, Nov 09, 2009 at 06:46:16PM +0100, Duboucher Thomas wrote: > >> Ok, I typed this in a few minutes and I'm not confident either with >> what I wrote; I would check that it works first. ;) >> But the point here is that whatever the user gives as an input, it is >> executed exactly n-th times, n being the length of the user input; and >> that whatever the result of the 'if' statement is, the CPU realizes the >> same amount of operations. By doing so, the attacker will only find out >> how long it takes to make the comparison with a n caracters long input. >> > > Actually, modern CPUs are very complex and the number of operations (or > time taken by them) isn't easy to predict. > > It's generally a good practice to do exactly same operations independently of result just store the result in a separate variable it's how RSA is correctly implemented for (n = grub_strlen (s1); n >= 0; n--) { if (*s1 != *s2) ret |= 1; else ret |= 0; s1++; s2++; } It's pproximately how my first attempt worked and it had this bug. If you can propose a good and tested code of this kind I would be ok with it -- Regards Vladimir 'phcoder' Serbinenko [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 293 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 18:15 ` Vladimir 'phcoder' Serbinenko @ 2009-11-09 18:25 ` Robert Millan 2009-11-09 18:36 ` Bean 0 siblings, 1 reply; 39+ messages in thread From: Robert Millan @ 2009-11-09 18:25 UTC (permalink / raw) To: The development of GNU GRUB On Mon, Nov 09, 2009 at 07:15:48PM +0100, Vladimir 'phcoder' Serbinenko wrote: > Robert Millan wrote: > > > > Actually, modern CPUs are very complex and the number of operations (or > > time taken by them) isn't easy to predict. > > > > > It's generally a good practice to do exactly same operations > independently of result just store the result in a separate variable > it's how RSA is correctly implemented > > for (n = grub_strlen (s1); n >= 0; n--) > { > if (*s1 != *s2) > ret |= 1; > else > ret |= 0; Uhm I didn't check, but I'd suspect -Os would optimize this out. Anyhow, if we move the fixed time wait to the outer loop, it should no longer be a problem. We could also check the approach taken by e.g. su from coreutils. -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 18:25 ` Robert Millan @ 2009-11-09 18:36 ` Bean 2009-11-09 18:46 ` Vladimir 'phcoder' Serbinenko 0 siblings, 1 reply; 39+ messages in thread From: Bean @ 2009-11-09 18:36 UTC (permalink / raw) To: The development of GNU GRUB On Tue, Nov 10, 2009 at 2:25 AM, Robert Millan <rmh@aybabtu.com> wrote: > On Mon, Nov 09, 2009 at 07:15:48PM +0100, Vladimir 'phcoder' Serbinenko wrote: >> Robert Millan wrote: >> > >> > Actually, modern CPUs are very complex and the number of operations (or >> > time taken by them) isn't easy to predict. >> > >> > >> It's generally a good practice to do exactly same operations >> independently of result just store the result in a separate variable >> it's how RSA is correctly implemented >> >> for (n = grub_strlen (s1); n >= 0; n--) >> { >> if (*s1 != *s2) >> ret |= 1; >> else >> ret |= 0; > > Uhm I didn't check, but I'd suspect -Os would optimize this out. > > Anyhow, if we move the fixed time wait to the outer loop, it should no > longer be a problem. > > We could also check the approach taken by e.g. su from coreutils. Hi, How about this one: int grub_auth_strcmp (const char *s1, const char *s2) { int result = 0; for (; *s1 != 0; s1++, s2++) result += (*s1 != *s2); return (result != 0); } -- Bean My repository: https://launchpad.net/burg Document: https://help.ubuntu.com/community/Burg ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 18:36 ` Bean @ 2009-11-09 18:46 ` Vladimir 'phcoder' Serbinenko 2009-11-09 18:49 ` Bean 0 siblings, 1 reply; 39+ messages in thread From: Vladimir 'phcoder' Serbinenko @ 2009-11-09 18:46 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 1598 bytes --] Bean wrote: > On Tue, Nov 10, 2009 at 2:25 AM, Robert Millan <rmh@aybabtu.com> wrote: > >> On Mon, Nov 09, 2009 at 07:15:48PM +0100, Vladimir 'phcoder' Serbinenko wrote: >> >>> Robert Millan wrote: >>> >>>> Actually, modern CPUs are very complex and the number of operations (or >>>> time taken by them) isn't easy to predict. >>>> >>>> >>>> >>> It's generally a good practice to do exactly same operations >>> independently of result just store the result in a separate variable >>> it's how RSA is correctly implemented >>> >>> for (n = grub_strlen (s1); n >= 0; n--) >>> { >>> if (*s1 != *s2) >>> ret |= 1; >>> else >>> ret |= 0; >>> >> Uhm I didn't check, but I'd suspect -Os would optimize this out. >> >> Anyhow, if we move the fixed time wait to the outer loop, it should no >> longer be a problem. >> >> We could also check the approach taken by e.g. su from coreutils. >> > > Hi, > > How about this one: > > int > grub_auth_strcmp (const char *s1, const char *s2) > { > int result = 0; > > for (; *s1 != 0; s1++, s2++) > result += (*s1 != *s2); > > return (result != 0); > } > > > Welcome to club: try it with "abc", "abcdef" They will match :(. Exactly the same problem as with my code but I like the approach. Perhaps: int grub_auth_strcmp (const char *s1, const char *s2) { int result = 0; for (; *s1 != 0; s1++, s2++) result += (*s1 != *s2); return !(result == 0 && *s2 == 0); } -- Regards Vladimir 'phcoder' Serbinenko [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 293 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 18:46 ` Vladimir 'phcoder' Serbinenko @ 2009-11-09 18:49 ` Bean 2009-11-09 21:13 ` Duboucher Thomas 0 siblings, 1 reply; 39+ messages in thread From: Bean @ 2009-11-09 18:49 UTC (permalink / raw) To: The development of GNU GRUB On Tue, Nov 10, 2009 at 2:46 AM, Vladimir 'phcoder' Serbinenko <phcoder@gmail.com> wrote: > Bean wrote: >> On Tue, Nov 10, 2009 at 2:25 AM, Robert Millan <rmh@aybabtu.com> wrote: >> >>> On Mon, Nov 09, 2009 at 07:15:48PM +0100, Vladimir 'phcoder' Serbinenko wrote: >>> >>>> Robert Millan wrote: >>>> >>>>> Actually, modern CPUs are very complex and the number of operations (or >>>>> time taken by them) isn't easy to predict. >>>>> >>>>> >>>>> >>>> It's generally a good practice to do exactly same operations >>>> independently of result just store the result in a separate variable >>>> it's how RSA is correctly implemented >>>> >>>> for (n = grub_strlen (s1); n >= 0; n--) >>>> { >>>> if (*s1 != *s2) >>>> ret |= 1; >>>> else >>>> ret |= 0; >>>> >>> Uhm I didn't check, but I'd suspect -Os would optimize this out. >>> >>> Anyhow, if we move the fixed time wait to the outer loop, it should no >>> longer be a problem. >>> >>> We could also check the approach taken by e.g. su from coreutils. >>> >> >> Hi, >> >> How about this one: >> >> int >> grub_auth_strcmp (const char *s1, const char *s2) >> { >> int result = 0; >> >> for (; *s1 != 0; s1++, s2++) >> result += (*s1 != *s2); >> >> return (result != 0); >> } >> >> >> > Welcome to club: try it with > "abc", "abcdef" > They will match :(. Exactly the same problem as with my code but I like > the approach. Perhaps: > > int > grub_auth_strcmp (const char *s1, const char *s2) > { > int result = 0; > > for (; *s1 != 0; s1++, s2++) > result += (*s1 != *s2); > > return !(result == 0 && *s2 == 0); > } Hi, This one work: int auth_strcmp (const char *s1, const char *s2) { int result = 0; while (1) { result += (*s1 != *s2); if (*s1 == 0) break; s1++; s2++; } return (result != 0); } The trick is to compare the ending '\0' as well, so that partial match is not satisfied. -- Bean My repository: https://launchpad.net/burg Document: https://help.ubuntu.com/community/Burg ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 18:49 ` Bean @ 2009-11-09 21:13 ` Duboucher Thomas 2009-11-09 21:34 ` Vladimir 'phcoder' Serbinenko 0 siblings, 1 reply; 39+ messages in thread From: Duboucher Thomas @ 2009-11-09 21:13 UTC (permalink / raw) To: The development of GNU GRUB -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Bean a écrit : > > Hi, > > This one work: > > int > auth_strcmp (const char *s1, const char *s2) > { > int result = 0; > > while (1) > { > result += (*s1 != *s2); > if (*s1 == 0) > break; > > s1++; > s2++; > } > > return (result != 0); > } > > The trick is to compare the ending '\0' as well, so that partial match > is not satisfied. > Yep, I like this one, but I would prefer using an OR instead of an ADD (with a highly hypothetical integer overflow :p) and because it's nicer in terms of pure logic. "The comparison beetwen s1 and s2 is false if *s1 is different from *s2, or recursively if the comparison beetwen s1+1 and s2+1 is false" int auth_strcmp (const char *s1, const char *s2) { int ret = 0; for (;;) { ret |= (*s1 != *s2); if (*s1 == '\0') break; s1++; s2++; } return ret; } Also, because s1 and s2 have two differents roles, I think it would be best to give them names that better suits them. ;) Thomas. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr4he4ACgkQBV7eXqefhqj0kACgkgE60xJe5X/zpmXoPEd9SsT9 6H8An113fF03h0cndz2LpJvqnPyJ3EPx =5MEi -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 21:13 ` Duboucher Thomas @ 2009-11-09 21:34 ` Vladimir 'phcoder' Serbinenko 2009-11-09 21:43 ` Duboucher Thomas 2009-11-10 5:39 ` Bean 0 siblings, 2 replies; 39+ messages in thread From: Vladimir 'phcoder' Serbinenko @ 2009-11-09 21:34 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 1557 bytes --] Duboucher Thomas wrote: > Bean a écrit : > > Hi, > > > This one work: > > > int > > auth_strcmp (const char *s1, const char *s2) > > { > > int result = 0; > > > while (1) > > { > > result += (*s1 != *s2); > > if (*s1 == 0) > > break; > > > s1++; > > s2++; > > } > > > return (result != 0); > > } > > > The trick is to compare the ending '\0' as well, so that partial match > > is not satisfied. > > > Yep, I like this one, but I would prefer using an OR instead of an ADD > (with a highly hypothetical integer overflow :p) and because it's nicer > in terms of pure logic. > "The comparison beetwen s1 and s2 is false if *s1 is different from > *s2, or recursively if the comparison beetwen s1+1 and s2+1 is false" > > int > auth_strcmp (const char *s1, const char *s2) > { > int ret = 0; > > for (;;) > { > ret |= (*s1 != *s2); > > if (*s1 == '\0') > break; > > s1++; > s2++; > } > > return ret; > } > But now it has a technical problem: it may read post array definitions. If any of post-array memory is MMIO or absent reading from it may have peculiar consequences > Also, because s1 and s2 have two differents roles, I think it would be > best to give them names that better suits them. ;) > > Thomas. _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel -- Regards Vladimir 'phcoder' Serbinenko [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 293 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 21:34 ` Vladimir 'phcoder' Serbinenko @ 2009-11-09 21:43 ` Duboucher Thomas 2009-11-09 22:06 ` Robert Millan 2009-11-10 5:39 ` Bean 1 sibling, 1 reply; 39+ messages in thread From: Duboucher Thomas @ 2009-11-09 21:43 UTC (permalink / raw) To: The development of GNU GRUB -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Vladimir 'phcoder' Serbinenko a écrit : > Duboucher Thomas wrote: >> Bean a écrit : >>> Hi, >>> This one work: >>> int >>> auth_strcmp (const char *s1, const char *s2) >>> { >>> int result = 0; >>> while (1) >>> { >>> result += (*s1 != *s2); >>> if (*s1 == 0) >>> break; >>> s1++; >>> s2++; >>> } >>> return (result != 0); >>> } >>> The trick is to compare the ending '\0' as well, so that partial match >>> is not satisfied. >> >> Yep, I like this one, but I would prefer using an OR instead of an ADD >> (with a highly hypothetical integer overflow :p) and because it's nicer >> in terms of pure logic. >> "The comparison beetwen s1 and s2 is false if *s1 is different from >> *s2, or recursively if the comparison beetwen s1+1 and s2+1 is false" >> >> int >> auth_strcmp (const char *s1, const char *s2) >> { >> int ret = 0; >> >> for (;;) >> { >> ret |= (*s1 != *s2); >> >> if (*s1 == '\0') >> break; >> >> s1++; >> s2++; >> } >> >> return ret; >> } >> > But now it has a technical problem: it may read post array definitions. > If any of post-array memory is MMIO or absent reading from it may have > peculiar consequences Well, the only way to solve that problem would be IMHO to add a limit to the size of s2, and use this maximum size as an end condition for the 'for' statement. Any better idea? :) int auth_strcmp (const char *s1, const char *s2) { int ret, n; for (ret = n = 0; ret < PASSPHRASE_MAXSIZE; ret++) { ret |= (*s1 != *s2); if (*s1 == '\0') break; s1++; s2++; } return ret; } >> Also, because s1 and s2 have two differents roles, I think it would be >> best to give them names that better suits them. ;) >> >> Thomas. > -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr4jRQACgkQBV7eXqefhqhJ3gCeNLHYAeVSb0qQ4GLgxbVvlDV7 P3oAoIvTa2Y+6i6BY1vTaOXXMklLVN8p =7x71 -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 21:43 ` Duboucher Thomas @ 2009-11-09 22:06 ` Robert Millan 2009-11-09 22:46 ` Duboucher Thomas 0 siblings, 1 reply; 39+ messages in thread From: Robert Millan @ 2009-11-09 22:06 UTC (permalink / raw) To: The development of GNU GRUB On Mon, Nov 09, 2009 at 10:43:48PM +0100, Duboucher Thomas wrote: > Well, the only way to solve that problem would be IMHO to add a limit > to the size of s2, and use this maximum size as an end condition for the > 'for' statement. Any better idea? :) We have a maximum line read size anyway. If we do this, we might as well make that limit global so that the macro can be shared with this routine. -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 22:06 ` Robert Millan @ 2009-11-09 22:46 ` Duboucher Thomas 2009-11-09 23:09 ` Darron Black 2009-11-09 23:46 ` richardvoigt 0 siblings, 2 replies; 39+ messages in thread From: Duboucher Thomas @ 2009-11-09 22:46 UTC (permalink / raw) To: The development of GNU GRUB -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Robert Millan a écrit : > On Mon, Nov 09, 2009 at 10:43:48PM +0100, Duboucher Thomas wrote: >> Well, the only way to solve that problem would be IMHO to add a limit >> to the size of s2, and use this maximum size as an end condition for the >> 'for' statement. Any better idea? :) > > We have a maximum line read size anyway. If we do this, we might as > well make that limit global so that the macro can be shared with > this routine. > Sounds good to me. :) Any ideas for renaming s1 and s2? Thomas. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr4m8EACgkQBV7eXqefhqj9SgCgjHomnoIkzzu5WuTCZQVcB/8t cwcAn1EkevCL3PXGlIuhLzFPlER9fXD3 =okR/ -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 22:46 ` Duboucher Thomas @ 2009-11-09 23:09 ` Darron Black 2009-11-09 23:50 ` richardvoigt 2009-11-09 23:46 ` richardvoigt 1 sibling, 1 reply; 39+ messages in thread From: Darron Black @ 2009-11-09 23:09 UTC (permalink / raw) To: The development of GNU GRUB Duboucher Thomas wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Robert Millan a écrit : > >> On Mon, Nov 09, 2009 at 10:43:48PM +0100, Duboucher Thomas wrote: >> >>> Well, the only way to solve that problem would be IMHO to add a limit >>> to the size of s2, and use this maximum size as an end condition for the >>> 'for' statement. Any better idea? :) >>> >> We have a maximum line read size anyway. If we do this, we might as >> well make that limit global so that the macro can be shared with >> this routine. >> >> > > Sounds good to me. :) > Any ideas for renaming s1 and s2? > > Thomas. > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.9 (MingW32) > Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ > > iEYEARECAAYFAkr4m8EACgkQBV7eXqefhqj9SgCgjHomnoIkzzu5WuTCZQVcB/8t > cwcAn1EkevCL3PXGlIuhLzFPlER9fXD3 > =okR/ > -----END PGP SIGNATURE----- > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > Hello, I'd be concerned about (s1 != s2). Depending on how efficiently this compiles, could not branch prediction make this faster for match vs. not match, etc?. I'd be worried about all the ways (and future ways) compilers might help us and introduce time differences. I'd feel most comfortable with the time delay, but why not stick to complete artithmetic? int i; int acc = 0; for(i=0;i<MAX_LEN;i++,s1++,s2++) { acc |= (*s1 ^ *s2); if (*s1 == 0) break; } return (acc == 0); Also, these strcmp functions don't properly return < or >. Just = / !=. However, my context being so new is quite limited. Darron ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 23:09 ` Darron Black @ 2009-11-09 23:50 ` richardvoigt 2009-11-09 23:56 ` Darron Black 0 siblings, 1 reply; 39+ messages in thread From: richardvoigt @ 2009-11-09 23:50 UTC (permalink / raw) To: The development of GNU GRUB > Hello, > > I'd be concerned about (s1 != s2). Depending on how efficiently this > compiles, could not branch prediction make this faster for match vs. not > match, etc?. I'd be worried about all the ways (and future ways) compilers > might help us and introduce time differences. I was avoiding suggesting new conditionals for that reason, but didn't see the one already there. Good find. > > I'd feel most comfortable with the time delay, but why not stick to complete > artithmetic? I agree. But I think you've inverted the return value (strcmp returns 0 on perfect match). > > > int i; > int acc = 0; > > for(i=0;i<MAX_LEN;i++,s1++,s2++) > { > acc |= (*s1 ^ *s2); > > if (*s1 == 0) > break; > } > > return (acc == 0); > > > Also, these strcmp functions don't properly return < or >. Just = / !=. > However, my context being so new is quite limited. > > > Darron > > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 23:50 ` richardvoigt @ 2009-11-09 23:56 ` Darron Black 0 siblings, 0 replies; 39+ messages in thread From: Darron Black @ 2009-11-09 23:56 UTC (permalink / raw) To: The development of GNU GRUB richardvoigt@gmail.com wrote: >> Hello, >> >> I'd be concerned about (s1 != s2). Depending on how efficiently this >> compiles, could not branch prediction make this faster for match vs. not >> match, etc?. I'd be worried about all the ways (and future ways) compilers >> might help us and introduce time differences. >> > > I was avoiding suggesting new conditionals for that reason, but didn't > see the one already there. Good find. > > >> I'd feel most comfortable with the time delay, but why not stick to complete >> artithmetic? >> > > I agree. But I think you've inverted the return value (strcmp returns > 0 on perfect match). > > Yeah, sorry. That'd be a slightly larger security hole, eh? I meant to just show the "acc |= (*s1 ^ *s2);" line, but I decided to throw the rest in for context and didn't really check it. I noticed that just AFTER sending. >> int i; >> int acc = 0; >> >> for(i=0;i<MAX_LEN;i++,s1++,s2++) >> { >> acc |= (*s1 ^ *s2); >> >> if (*s1 == 0) >> break; >> } >> >> return (acc == 0); >> >> >> Also, these strcmp functions don't properly return < or >. Just = / !=. >> However, my context being so new is quite limited. >> >> >> Darron >> >> >> >> _______________________________________________ >> Grub-devel mailing list >> Grub-devel@gnu.org >> http://lists.gnu.org/mailman/listinfo/grub-devel >> >> > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 22:46 ` Duboucher Thomas 2009-11-09 23:09 ` Darron Black @ 2009-11-09 23:46 ` richardvoigt 1 sibling, 0 replies; 39+ messages in thread From: richardvoigt @ 2009-11-09 23:46 UTC (permalink / raw) To: The development of GNU GRUB On Mon, Nov 9, 2009 at 4:46 PM, Duboucher Thomas <thomas@duboucher.eu> wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Robert Millan a écrit : >> On Mon, Nov 09, 2009 at 10:43:48PM +0100, Duboucher Thomas wrote: >>> Well, the only way to solve that problem would be IMHO to add a limit >>> to the size of s2, and use this maximum size as an end condition for the >>> 'for' statement. Any better idea? :) >> >> We have a maximum line read size anyway. If we do this, we might as >> well make that limit global so that the macro can be shared with >> this routine. >> > > Sounds good to me. :) > Any ideas for renaming s1 and s2? correct_key and attempted_key But there seems to still be potential for overflow on one of the strings. Are both strings in fixed-equal-length buffers? That should be clearly documented. > > Thomas. > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.9 (MingW32) > Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ > > iEYEARECAAYFAkr4m8EACgkQBV7eXqefhqj9SgCgjHomnoIkzzu5WuTCZQVcB/8t > cwcAn1EkevCL3PXGlIuhLzFPlER9fXD3 > =okR/ > -----END PGP SIGNATURE----- > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-09 21:34 ` Vladimir 'phcoder' Serbinenko 2009-11-09 21:43 ` Duboucher Thomas @ 2009-11-10 5:39 ` Bean 2009-11-10 8:28 ` Bean 1 sibling, 1 reply; 39+ messages in thread From: Bean @ 2009-11-10 5:39 UTC (permalink / raw) To: The development of GNU GRUB On Tue, Nov 10, 2009 at 5:34 AM, Vladimir 'phcoder' Serbinenko <phcoder@gmail.com> wrote: > But now it has a technical problem: it may read post array definitions. > If any of post-array memory is MMIO or absent reading from it may have > peculiar consequences >> Also, because s1 and s2 have two differents roles, I think it would be >> best to give them names that better suits them. ;) Hi, Right, I think it'd be better to use fixed size array, perhaps we can define a type grub_password_t for it. BTW, with fixed size array, the following algorithm should run exactly the same amount of instruction each time: typedef char grub_password_t[1024]; int grub_auth_strcmp (const grub_password_t s1, const grub_password_t s2) { int r1 = 0; int r2 = 0; int i, *p; p = &r1; for (i = 0; i < sizeof (grub_password_t); i++, s1++, s2++) { *p |= (*s1 ^ *s2); if (*s1 == '\0') p = &r2; } return (r1 != 0); } -- Bean My repository: https://launchpad.net/burg Document: https://help.ubuntu.com/community/Burg ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-10 5:39 ` Bean @ 2009-11-10 8:28 ` Bean 2009-11-10 8:46 ` Bean 0 siblings, 1 reply; 39+ messages in thread From: Bean @ 2009-11-10 8:28 UTC (permalink / raw) To: The development of GNU GRUB On Tue, Nov 10, 2009 at 1:39 PM, Bean <bean123ch@gmail.com> wrote: > On Tue, Nov 10, 2009 at 5:34 AM, Vladimir 'phcoder' Serbinenko > <phcoder@gmail.com> wrote: >> But now it has a technical problem: it may read post array definitions. >> If any of post-array memory is MMIO or absent reading from it may have >> peculiar consequences >>> Also, because s1 and s2 have two differents roles, I think it would be >>> best to give them names that better suits them. ;) > > Hi, > > Right, I think it'd be better to use fixed size array, perhaps we can > define a type grub_password_t for it. > > BTW, with fixed size array, the following algorithm should run exactly > the same amount of instruction each time: > > typedef char grub_password_t[1024]; > > int > grub_auth_strcmp (const grub_password_t s1, const grub_password_t s2) > { > int r1 = 0; > int r2 = 0; > int i, *p; > > p = &r1; > for (i = 0; i < sizeof (grub_password_t); i++, s1++, s2++) > { > *p |= (*s1 ^ *s2); > if (*s1 == '\0') > p = &r2; > } > > return (r1 != 0); > } Hi, Oh sorry, this one still have some issue, this should work: typedef char grub_password_t[1024]; int grub_auth_strcmp (const grub_password_t s1, const grub_password_t s2) { int r1 = 0; int r2 = 0; int i, *p1, *p2; p1 = p2 = &r1; for (i = 0; i < sizeof (grub_password_t); i++, s1++, s2++) { *p1 |= (*s1 ^ *s2); if (*s1 == '\0') p1 = &r2; else p2 = &r2; } return (r1 != 0); } -- Bean My repository: https://launchpad.net/burg Document: https://help.ubuntu.com/community/Burg ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-10 8:28 ` Bean @ 2009-11-10 8:46 ` Bean 2009-11-10 8:52 ` Bean 0 siblings, 1 reply; 39+ messages in thread From: Bean @ 2009-11-10 8:46 UTC (permalink / raw) To: The development of GNU GRUB On Tue, Nov 10, 2009 at 4:28 PM, Bean <bean123ch@gmail.com> wrote: > On Tue, Nov 10, 2009 at 1:39 PM, Bean <bean123ch@gmail.com> wrote: >> On Tue, Nov 10, 2009 at 5:34 AM, Vladimir 'phcoder' Serbinenko >> <phcoder@gmail.com> wrote: >>> But now it has a technical problem: it may read post array definitions. >>> If any of post-array memory is MMIO or absent reading from it may have >>> peculiar consequences >>>> Also, because s1 and s2 have two differents roles, I think it would be >>>> best to give them names that better suits them. ;) >> >> Hi, >> >> Right, I think it'd be better to use fixed size array, perhaps we can >> define a type grub_password_t for it. >> >> BTW, with fixed size array, the following algorithm should run exactly >> the same amount of instruction each time: >> >> typedef char grub_password_t[1024]; >> >> int >> grub_auth_strcmp (const grub_password_t s1, const grub_password_t s2) >> { >> int r1 = 0; >> int r2 = 0; >> int i, *p; >> >> p = &r1; >> for (i = 0; i < sizeof (grub_password_t); i++, s1++, s2++) >> { >> *p |= (*s1 ^ *s2); >> if (*s1 == '\0') >> p = &r2; >> } >> >> return (r1 != 0); >> } > > Hi, > > Oh sorry, this one still have some issue, this should work: > > typedef char grub_password_t[1024]; > > int > grub_auth_strcmp (const grub_password_t s1, const grub_password_t s2) > { > int r1 = 0; > int r2 = 0; > int i, *p1, *p2; > > p1 = p2 = &r1; > for (i = 0; i < sizeof (grub_password_t); i++, s1++, s2++) > { > *p1 |= (*s1 ^ *s2); > if (*s1 == '\0') > p1 = &r2; > else > p2 = &r2; > } > > return (r1 != 0); > } Hi, Just in case p2 is optimized out by gcc: typedef char grub_password_t[1024]; int grub_auth_strcmp (const grub_password_t s1, const grub_password_t s2) { char r1 = 0; char r2 = 0; char r3 = 0; char *p1, *p2; int i; p1 = &r1; p2 = &r3; for (i = 0; i < sizeof (grub_password_t); i++, s1++, s2++) { *p1 |= (*s1 ^ *s2); if (*s1 == '\0') p1 = &r2; else p2 = &r2; (*p2)++; } return (r1 != 0); } -- Bean My repository: https://launchpad.net/burg Document: https://help.ubuntu.com/community/Burg ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-10 8:46 ` Bean @ 2009-11-10 8:52 ` Bean 2009-11-10 9:05 ` Bean 0 siblings, 1 reply; 39+ messages in thread From: Bean @ 2009-11-10 8:52 UTC (permalink / raw) To: The development of GNU GRUB On Tue, Nov 10, 2009 at 4:46 PM, Bean <bean123ch@gmail.com> wrote: > Hi, > > Just in case p2 is optimized out by gcc: > > typedef char grub_password_t[1024]; > > int > grub_auth_strcmp (const grub_password_t s1, const grub_password_t s2) > { > char r1 = 0; > char r2 = 0; > char r3 = 0; > char *p1, *p2; > int i; > > p1 = &r1; > p2 = &r3; > for (i = 0; i < sizeof (grub_password_t); i++, s1++, s2++) > { > *p1 |= (*s1 ^ *s2); > if (*s1 == '\0') > p1 = &r2; > else > p2 = &r2; > (*p2)++; > } > > return (r1 != 0); > } Hi, Perhaps this one, it's more symmetrical: typedef char grub_password_t[1024]; int grub_auth_strcmp (const grub_password_t s1, const grub_password_t s2) { char r1 = 0; char r2 = 0; char r3 = 0; char *p1, *p2; int i; p1 = &r1; p2 = &r3; for (i = 0; i < sizeof (grub_password_t); i++, s1++, s2++) { char c; c = (*s1 ^ *s2); *p1 |= c; *p2 |= c; if (*s1 == '\0') p1 = &r2; else p2 = &r2; } return (r1 != 0); } -- Bean My repository: https://launchpad.net/burg Document: https://help.ubuntu.com/community/Burg ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-10 8:52 ` Bean @ 2009-11-10 9:05 ` Bean 2009-11-10 12:37 ` Bean 0 siblings, 1 reply; 39+ messages in thread From: Bean @ 2009-11-10 9:05 UTC (permalink / raw) To: The development of GNU GRUB On Tue, Nov 10, 2009 at 4:52 PM, Bean <bean123ch@gmail.com> wrote: > Hi, > > Perhaps this one, it's more symmetrical: > > typedef char grub_password_t[1024]; > > int > grub_auth_strcmp (const grub_password_t s1, const grub_password_t s2) > { > char r1 = 0; > char r2 = 0; > char r3 = 0; > char *p1, *p2; > int i; > > p1 = &r1; > p2 = &r3; > for (i = 0; i < sizeof (grub_password_t); i++, s1++, s2++) > { > char c; > > c = (*s1 ^ *s2); > *p1 |= c; > *p2 |= c; > if (*s1 == '\0') > p1 = &r2; > else > p2 = &r2; > } > > return (r1 != 0); > } Hi, BTW, just in case there is delay in hardware when the same memory address r2 is accessed repeatedly: typedef char grub_password_t[1024]; int grub_auth_strcmp (const grub_password_t s1, const grub_password_t s2) { char r1 = 0; char r2 = 0; char r3 = 0; char r4 = 0; char *p1, *p2; int i; p1 = &r1; p2 = &r3; for (i = 0; i < sizeof (grub_password_t); i++, s1++, s2++) { char c; c = (*s1 ^ *s2); *p1 |= c; *p2 |= c; if (*s1 == '\0') p1 = &r2; else p2 = &r4; } return (r1 != 0); } -- Bean My repository: https://launchpad.net/burg Document: https://help.ubuntu.com/community/Burg ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-10 9:05 ` Bean @ 2009-11-10 12:37 ` Bean 2009-11-10 14:25 ` Duboucher Thomas 0 siblings, 1 reply; 39+ messages in thread From: Bean @ 2009-11-10 12:37 UTC (permalink / raw) To: The development of GNU GRUB Hi, Oh, I just come up with a better way to do this: typedef char grub_password_t[1024]; int grub_auth_strcmp (const grub_password_t s1, const grub_password_t s2) { char r1 = 0; char r2 = 0; char *p; int i, c; p = &r1; c = 0; for (i = 0; i < sizeof (grub_password_t); i++, s1++, s2++) { *p | = (*s1 ^ *s2); if ((int) *s1 == c) { p = &r2; c = 0x100; } } return (r1 != 0); } The condition (int) *s1 == c would be true exactly once. -- Bean My repository: https://launchpad.net/burg Document: https://help.ubuntu.com/community/Burg ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-10 12:37 ` Bean @ 2009-11-10 14:25 ` Duboucher Thomas 2009-11-10 14:47 ` Bean 2009-11-10 15:27 ` richardvoigt 0 siblings, 2 replies; 39+ messages in thread From: Duboucher Thomas @ 2009-11-10 14:25 UTC (permalink / raw) To: The development of GNU GRUB -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Bean a écrit : > Hi, > > Oh, I just come up with a better way to do this: > > typedef char grub_password_t[1024]; > > int > grub_auth_strcmp (const grub_password_t s1, const grub_password_t s2) > { > char r1 = 0; > char r2 = 0; > char *p; > int i, c; > > p = &r1; > c = 0; > for (i = 0; i < sizeof (grub_password_t); i++, s1++, s2++) > { > *p | = (*s1 ^ *s2); > if ((int) *s1 == c) > { > p = &r2; > c = 0x100; > } > } > > return (r1 != 0); > } > > The condition (int) *s1 == c would be true exactly once. > Well, it seems I lost something somewhere. I don't understand the need of doing it exactly sizeof (grub_password_t) times, except from having a perfectly symetric function. IMHO, stopping the comparison when the input buffer is done reading, or when the maximum size of a passphrase is reached does not leak any information to the attacker. So I would stick to typedef char grub_password_t[1024]; int auth_strcmp (const grub_password_t input, grub_password_t key) { int retval, it; for (it = retval = 0; it < PASSPHRASE_MAXSIZE; it++, input++, key++) { retval |= (*input != *key); if (*input == '\0') break; } return !retval; } Also, take care that it requires to check how the function is optimized; sometimes you have surprises ... ;) Thomas. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr5d90ACgkQBV7eXqefhqio+QCfba54+l45DiQNyI3IzfnwgvVe tbUAnRTPI+yYSZoVZLfM9fze7c7cvRQN =EjYS -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-10 14:25 ` Duboucher Thomas @ 2009-11-10 14:47 ` Bean 2009-11-10 17:43 ` Duboucher Thomas 2009-11-10 19:04 ` Vladimir 'phcoder' Serbinenko 2009-11-10 15:27 ` richardvoigt 1 sibling, 2 replies; 39+ messages in thread From: Bean @ 2009-11-10 14:47 UTC (permalink / raw) To: The development of GNU GRUB On Tue, Nov 10, 2009 at 10:25 PM, Duboucher Thomas <thomas@duboucher.eu> wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Bean a écrit : >> Hi, >> >> Oh, I just come up with a better way to do this: >> >> typedef char grub_password_t[1024]; >> >> int >> grub_auth_strcmp (const grub_password_t s1, const grub_password_t s2) >> { >> char r1 = 0; >> char r2 = 0; >> char *p; >> int i, c; >> >> p = &r1; >> c = 0; >> for (i = 0; i < sizeof (grub_password_t); i++, s1++, s2++) >> { >> *p | = (*s1 ^ *s2); >> if ((int) *s1 == c) >> { >> p = &r2; >> c = 0x100; >> } >> } >> >> return (r1 != 0); >> } >> >> The condition (int) *s1 == c would be true exactly once. >> > > Well, it seems I lost something somewhere. I don't understand the need > of doing it exactly sizeof (grub_password_t) times, except from having a > perfectly symetric function. IMHO, stopping the comparison when the > input buffer is done reading, or when the maximum size of a passphrase > is reached does not leak any information to the attacker. So I would > stick to > > typedef char grub_password_t[1024]; > > int > auth_strcmp (const grub_password_t input, grub_password_t key) > { > int retval, it; > > for (it = retval = 0; it < PASSPHRASE_MAXSIZE; it++, input++, key++) > { > retval |= (*input != *key); > > if (*input == '\0') > break; > } > > return !retval; > } > > Also, take care that it requires to check how the function is > optimized; sometimes you have surprises ... ;) Hi, My previous function ensures that execution time is the same regardless of the input. Although it's not necessary, I guess it's a nice feature to have. BTW, the simpler function does leak one information, the size of buffer as the execution time would increase until the buffer size is reached. -- Bean My repository: https://launchpad.net/burg Document: https://help.ubuntu.com/community/Burg ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-10 14:47 ` Bean @ 2009-11-10 17:43 ` Duboucher Thomas 2009-11-10 19:01 ` Vladimir 'phcoder' Serbinenko 2009-11-10 19:04 ` Vladimir 'phcoder' Serbinenko 1 sibling, 1 reply; 39+ messages in thread From: Duboucher Thomas @ 2009-11-10 17:43 UTC (permalink / raw) To: The development of GNU GRUB -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Bean a écrit : > > Hi, > > My previous function ensures that execution time is the same > regardless of the input. Although it's not necessary, I guess it's a > nice feature to have. BTW, the simpler function does leak one > information, the size of buffer as the execution time would increase > until the buffer size is reached. > Hi, Yes, constant time of execution _is_ a constraint of this function. However, I don't think that giving access to the size of the buffer is a leak per se, the source code of Grub being available for everyone; We only need not to leak more informations than already available. Thomas. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr5pjgACgkQBV7eXqefhqhm8ACfVefuwV/IVdB2NzWrPeEzksfs jeIAn1xmJzNWwTa1gKdjvA/o4MQSLsLI =n0nc -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-10 17:43 ` Duboucher Thomas @ 2009-11-10 19:01 ` Vladimir 'phcoder' Serbinenko 0 siblings, 0 replies; 39+ messages in thread From: Vladimir 'phcoder' Serbinenko @ 2009-11-10 19:01 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 982 bytes --] Duboucher Thomas wrote: > Bean a écrit : > > Hi, > > > My previous function ensures that execution time is the same > > regardless of the input. Although it's not necessary, I guess it's a > > nice feature to have. BTW, the simpler function does leak one > > information, the size of buffer as the execution time would increase > > until the buffer size is reached. > > > Hi, > > Yes, constant time of execution _is_ a constraint of this function. > However, I don't think that giving access to the size of the buffer is a > leak per se, the source code of Grub being available for everyone; We > only need not to leak more informations than already available. > Yes. No security analysis can assume attacker doesn't have the source code > Thomas. _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel -- Regards Vladimir 'phcoder' Serbinenko [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 293 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-10 14:47 ` Bean 2009-11-10 17:43 ` Duboucher Thomas @ 2009-11-10 19:04 ` Vladimir 'phcoder' Serbinenko 2009-11-10 21:29 ` Duboucher Thomas 1 sibling, 1 reply; 39+ messages in thread From: Vladimir 'phcoder' Serbinenko @ 2009-11-10 19:04 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 2290 bytes --] Bean wrote: > On Tue, Nov 10, 2009 at 10:25 PM, Duboucher Thomas <thomas@duboucher.eu> wrote: > >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA1 >> >> Bean a écrit : >> >>> Hi, >>> >>> Oh, I just come up with a better way to do this: >>> >>> typedef char grub_password_t[1024]; >>> >>> int >>> grub_auth_strcmp (const grub_password_t s1, const grub_password_t s2) >>> { >>> char r1 = 0; >>> char r2 = 0; >>> char *p; >>> int i, c; >>> >>> p = &r1; >>> c = 0; >>> for (i = 0; i < sizeof (grub_password_t); i++, s1++, s2++) >>> { >>> *p | = (*s1 ^ *s2); >>> if ((int) *s1 == c) >>> { >>> p = &r2; >>> c = 0x100; >>> } >>> } >>> >>> return (r1 != 0); >>> } >>> >>> The condition (int) *s1 == c would be true exactly once. >>> >>> >> Well, it seems I lost something somewhere. I don't understand the need >> of doing it exactly sizeof (grub_password_t) times, except from having a >> perfectly symetric function. IMHO, stopping the comparison when the >> input buffer is done reading, or when the maximum size of a passphrase >> is reached does not leak any information to the attacker. So I would >> stick to >> >> typedef char grub_password_t[1024]; >> >> With this change grub_auth_strcmp becomes a misnomer. I would prefer to call it grub_auth_memcmp then. I'll also look into which other free secure strcmp are available >> int >> auth_strcmp (const grub_password_t input, grub_password_t key) >> { >> int retval, it; >> >> for (it = retval = 0; it < PASSPHRASE_MAXSIZE; it++, input++, key++) >> { >> retval |= (*input != *key); >> >> if (*input == '\0') >> break; >> } >> >> return !retval; >> } >> >> Also, take care that it requires to check how the function is >> optimized; sometimes you have surprises ... ;) >> > > Hi, > > My previous function ensures that execution time is the same > regardless of the input. Although it's not necessary, I guess it's a > nice feature to have. BTW, the simpler function does leak one > information, the size of buffer as the execution time would increase > until the buffer size is reached. > > > -- Regards Vladimir 'phcoder' Serbinenko [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 293 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-10 19:04 ` Vladimir 'phcoder' Serbinenko @ 2009-11-10 21:29 ` Duboucher Thomas 0 siblings, 0 replies; 39+ messages in thread From: Duboucher Thomas @ 2009-11-10 21:29 UTC (permalink / raw) To: The development of GNU GRUB -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Vladimir 'phcoder' Serbinenko a écrit : > With this change grub_auth_strcmp becomes a misnomer. I would prefer to > call it grub_auth_memcmp then. I'll also look into which other free > secure strcmp are available Asking developpers of projects such as GPG for advice could be a good idea. ;) -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr52zcACgkQBV7eXqefhqhz0ACgkQV+VVBiriyzqRUyKhZTqLmN HaAAnR8HJqH2kpfqfQvGZsy7iKDhZlf5 =v1Qb -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-10 14:25 ` Duboucher Thomas 2009-11-10 14:47 ` Bean @ 2009-11-10 15:27 ` richardvoigt 2009-11-10 17:38 ` Duboucher Thomas 1 sibling, 1 reply; 39+ messages in thread From: richardvoigt @ 2009-11-10 15:27 UTC (permalink / raw) To: The development of GNU GRUB On Tue, Nov 10, 2009 at 8:25 AM, Duboucher Thomas <thomas@duboucher.eu> wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Bean a écrit : >> Hi, >> >> Oh, I just come up with a better way to do this: >> >> typedef char grub_password_t[1024]; >> >> int >> grub_auth_strcmp (const grub_password_t s1, const grub_password_t s2) >> { >> char r1 = 0; >> char r2 = 0; >> char *p; >> int i, c; >> >> p = &r1; >> c = 0; >> for (i = 0; i < sizeof (grub_password_t); i++, s1++, s2++) >> { >> *p | = (*s1 ^ *s2); >> if ((int) *s1 == c) >> { >> p = &r2; >> c = 0x100; >> } >> } >> >> return (r1 != 0); >> } >> >> The condition (int) *s1 == c would be true exactly once. >> > > Well, it seems I lost something somewhere. I don't understand the need > of doing it exactly sizeof (grub_password_t) times, except from having a > perfectly symetric function. IMHO, stopping the comparison when the > input buffer is done reading, or when the maximum size of a passphrase > is reached does not leak any information to the attacker. So I would > stick to > > typedef char grub_password_t[1024]; > > int > auth_strcmp (const grub_password_t input, grub_password_t key) > { > int retval, it; > > for (it = retval = 0; it < PASSPHRASE_MAXSIZE; it++, input++, key++) After changing the parameter type, those postincrements won't do what you expect. > { > retval |= (*input != *key); > > if (*input == '\0') > break; > } > > return !retval; > } > > Also, take care that it requires to check how the function is > optimized; sometimes you have surprises ... ;) > > Thomas. > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.9 (MingW32) > Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ > > iEYEARECAAYFAkr5d90ACgkQBV7eXqefhqio+QCfba54+l45DiQNyI3IzfnwgvVe > tbUAnRTPI+yYSZoVZLfM9fze7c7cvRQN > =EjYS > -----END PGP SIGNATURE----- > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Imminent bugfix release (1.97.1) 2009-11-10 15:27 ` richardvoigt @ 2009-11-10 17:38 ` Duboucher Thomas 0 siblings, 0 replies; 39+ messages in thread From: Duboucher Thomas @ 2009-11-10 17:38 UTC (permalink / raw) To: The development of GNU GRUB -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 richardvoigt@gmail.com a écrit : > > for (it = retval = 0; it < PASSPHRASE_MAXSIZE; it++, input++, key++) > >> After changing the parameter type, those postincrements won't do what >> you expect. > Damn examinations; I really need to sleep! =) -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkr5pRoACgkQBV7eXqefhqgzwwCgh9BMUl9V7kQ6M/YjscwDIVRP OVIAoJj+WMW+tZ58wLg03oCuvEuxj77n =6gXS -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2009-11-10 21:30 UTC | newest] Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-11-09 1:04 Imminent bugfix release (1.97.1) Robert Millan 2009-11-09 1:27 ` Robert Millan 2009-11-09 2:08 ` Jordan Uggla 2009-11-09 14:15 ` Robert Millan 2009-11-09 13:33 ` Bean 2009-11-09 13:50 ` Vladimir 'phcoder' Serbinenko 2009-11-09 14:18 ` Robert Millan 2009-11-09 14:21 ` Bean 2009-11-09 14:34 ` Vladimir 'phcoder' Serbinenko 2009-11-09 17:46 ` Duboucher Thomas 2009-11-09 18:10 ` Robert Millan 2009-11-09 18:15 ` Vladimir 'phcoder' Serbinenko 2009-11-09 18:25 ` Robert Millan 2009-11-09 18:36 ` Bean 2009-11-09 18:46 ` Vladimir 'phcoder' Serbinenko 2009-11-09 18:49 ` Bean 2009-11-09 21:13 ` Duboucher Thomas 2009-11-09 21:34 ` Vladimir 'phcoder' Serbinenko 2009-11-09 21:43 ` Duboucher Thomas 2009-11-09 22:06 ` Robert Millan 2009-11-09 22:46 ` Duboucher Thomas 2009-11-09 23:09 ` Darron Black 2009-11-09 23:50 ` richardvoigt 2009-11-09 23:56 ` Darron Black 2009-11-09 23:46 ` richardvoigt 2009-11-10 5:39 ` Bean 2009-11-10 8:28 ` Bean 2009-11-10 8:46 ` Bean 2009-11-10 8:52 ` Bean 2009-11-10 9:05 ` Bean 2009-11-10 12:37 ` Bean 2009-11-10 14:25 ` Duboucher Thomas 2009-11-10 14:47 ` Bean 2009-11-10 17:43 ` Duboucher Thomas 2009-11-10 19:01 ` Vladimir 'phcoder' Serbinenko 2009-11-10 19:04 ` Vladimir 'phcoder' Serbinenko 2009-11-10 21:29 ` Duboucher Thomas 2009-11-10 15:27 ` richardvoigt 2009-11-10 17:38 ` Duboucher Thomas
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.