Imminent bugfix release (1.97.1)

Imminent bugfix release (1.97.1)

[Robert Millan]

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."

Re: Imminent bugfix release (1.97.1)

[Robert Millan]

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."

Re: Imminent bugfix release (1.97.1)

[Jordan Uggla]

[-- 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

Re: Imminent bugfix release (1.97.1)

[Bean]

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

Re: Imminent bugfix release (1.97.1)

[Vladimir 'phcoder' Serbinenko]

[-- 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 --]

Re: Imminent bugfix release (1.97.1)

[Robert Millan]

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."

Re: Imminent bugfix release (1.97.1)

[Robert Millan]

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."

Re: Imminent bugfix release (1.97.1)

[Bean]

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

Re: Imminent bugfix release (1.97.1)

[Vladimir 'phcoder' Serbinenko]

[-- 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 --]

Re: Imminent bugfix release (1.97.1)

[Duboucher Thomas]

-----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-----

Re: Imminent bugfix release (1.97.1)

[Robert Millan]

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."

Re: Imminent bugfix release (1.97.1)

[Vladimir 'phcoder' Serbinenko]

[-- 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 --]

Re: Imminent bugfix release (1.97.1)

[Robert Millan]

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."

Re: Imminent bugfix release (1.97.1)

[Bean]

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

Re: Imminent bugfix release (1.97.1)

[Vladimir 'phcoder' Serbinenko]

[-- 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 --]

Re: Imminent bugfix release (1.97.1)

[Bean]

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

Re: Imminent bugfix release (1.97.1)

[Duboucher Thomas]

-----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-----

Re: Imminent bugfix release (1.97.1)

[Vladimir 'phcoder' Serbinenko]

[-- 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 --]

Re: Imminent bugfix release (1.97.1)

[Duboucher Thomas]

-----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-----

Re: Imminent bugfix release (1.97.1)

[Robert Millan]

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."

Re: Imminent bugfix release (1.97.1)

[Duboucher Thomas]

-----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-----

Re: Imminent bugfix release (1.97.1)

[Darron Black]

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

Re: Imminent bugfix release (1.97.1)

[richardvoigt]

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
>

Re: Imminent bugfix release (1.97.1)

[richardvoigt]

> 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
>

Re: Imminent bugfix release (1.97.1)

[Darron Black]

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
>   

Re: Imminent bugfix release (1.97.1)

[Bean]

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

Re: Imminent bugfix release (1.97.1)

[Bean]

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

Re: Imminent bugfix release (1.97.1)

[Bean]

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

Re: Imminent bugfix release (1.97.1)

[Bean]

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

Re: Imminent bugfix release (1.97.1)

[Bean]

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

Re: Imminent bugfix release (1.97.1)

[Bean]

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

Re: Imminent bugfix release (1.97.1)

[Duboucher Thomas]

-----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-----

Re: Imminent bugfix release (1.97.1)

[Bean]

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

Re: Imminent bugfix release (1.97.1)

[richardvoigt]

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
>

Re: Imminent bugfix release (1.97.1)

[Duboucher Thomas]

-----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-----

Re: Imminent bugfix release (1.97.1)

[Duboucher Thomas]

-----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-----

Re: Imminent bugfix release (1.97.1)

[Vladimir 'phcoder' Serbinenko]

[-- 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 --]

Re: Imminent bugfix release (1.97.1)

[Vladimir 'phcoder' Serbinenko]

[-- 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 --]

Re: Imminent bugfix release (1.97.1)

[Duboucher Thomas]

-----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-----