Re: [PATCH v7 06/13] x86: Add early SHA support for Secure Launch early measurements
On Sat, 2023-11-11 at 18:19 +, Andrew Cooper wrote: > On 11/11/2023 5:44 pm, Eric Biggers wrote: > > On Fri, Nov 10, 2023 at 05:27:44PM -0500, Ross Philipson wrote: > > > arch/x86/boot/compressed/early_sha1.c | 12 > > > lib/crypto/sha1.c | 81 > > > + > > It's surprising to still see this new use of SHA-1 after so many > > people objected to it in the v6 patchset. It's also frustrating > > that the SHA-1 support is still being obfuscated by being combined > > in one patch with SHA-2 support, perhaps in an attempt to conflate > > the two algorithms and avoid having to give a rationale for the > > inclusion of SHA-1. Finally, new functions should not be added to > > lib/crypto/sha1.c unless those functions have multiple users. > > The rational was given. Let me reiterate it. > > There are real TPMs in the world that can't use SHA-2. The use of > SHA-1 is necessary to support DRTM on such systems, and there are > real users of such configurations. Given that TPM 2.0 has been shipping in bulk since Windows 10 (2015) and is required for Windows 11 (2021), are there really such huge numbers of TPM 1.2 systems involved in security functions? > DRTM with SHA-1-only is a damnsight better than no DTRM, even if SHA- > 1 is getting a little long in the tooth. That's not the problem. The problem is that sha1 is seen as a compromised algorithm by NIST which began deprecating it in 2011 and is now requiring it to be removed from all systems supplied to the US government by 2030 https://www.nist.gov/news-events/news/2022/12/nist-retires-sha-1-cryptographic-algorithm That means we have to control all uses of sha1 in the kernel and have an option to build without it. FIPS has an even tighter timetable: it requires sha1 to be out by 2025. > So unless you have a credible plan to upgrade every non-SHA-2 TPM in > the world, you are deliberately breaking part of the usecase paying > for the effort of trying to upstream DRTM support into Linux. Given that most CSOs follow NIST and FIPS it seems a little strange that there would be a huge demand for such an intricate security protocol as Dynamic Launch on a system that can't be FIPS 140-3 certified. James
Re: [PATCH v7 06/13] x86: Add early SHA support for Secure Launch early measurements
On 11/11/2023 5:44 pm, Eric Biggers wrote: > On Fri, Nov 10, 2023 at 05:27:44PM -0500, Ross Philipson wrote: >> arch/x86/boot/compressed/early_sha1.c | 12 >> lib/crypto/sha1.c | 81 + > It's surprising to still see this new use of SHA-1 after so many people > objected > to it in the v6 patchset. It's also frustrating that the SHA-1 support is > still > being obfuscated by being combined in one patch with SHA-2 support, perhaps in > an attempt to conflate the two algorithms and avoid having to give a rationale > for the inclusion of SHA-1. Finally, new functions should not be added to > lib/crypto/sha1.c unless those functions have multiple users. The rational was given. Let me reiterate it. There are real TPMs in the world that can't use SHA-2. The use of SHA-1 is necessary to support DRTM on such systems, and there are real users of such configurations. DRTM with SHA-1-only is a damnsight better than no DTRM, even if SHA-1 is getting a little long in the tooth. So unless you have a credible plan to upgrade every non-SHA-2 TPM in the world, you are deliberately breaking part of the usecase paying for the effort of trying to upstream DRTM support into Linux. ~Andrew
Re: [PATCH v7 06/13] x86: Add early SHA support for Secure Launch early measurements
On Fri, Nov 10, 2023 at 05:27:44PM -0500, Ross Philipson wrote: > arch/x86/boot/compressed/early_sha1.c | 12 > lib/crypto/sha1.c | 81 + It's surprising to still see this new use of SHA-1 after so many people objected to it in the v6 patchset. It's also frustrating that the SHA-1 support is still being obfuscated by being combined in one patch with SHA-2 support, perhaps in an attempt to conflate the two algorithms and avoid having to give a rationale for the inclusion of SHA-1. Finally, new functions should not be added to lib/crypto/sha1.c unless those functions have multiple users. - Eric
Re: [PATCH v7 09/13] x86: Secure Launch SMP bringup support
Hi Ross,
kernel test robot noticed the following build warnings:
[auto build test WARNING on tip/x86/core]
[also build test WARNING on herbert-cryptodev-2.6/master
herbert-crypto-2.6/master linus/master v6.6 next-20231110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Ross-Philipson/x86-boot-Place-kernel_info-at-a-fixed-offset/2023-063453
base: tip/x86/core
patch link:
https://lore.kernel.org/r/20231110222751.219836-10-ross.philipson%40oracle.com
patch subject: [PATCH v7 09/13] x86: Secure Launch SMP bringup support
config: x86_64-rhel-8.3-rust
(https://download.01.org/0day-ci/archive/2023/20231806.sbmcwun1-...@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git
ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/2023/20231806.sbmcwun1-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/20231806.sbmcwun1-...@intel.com/
All warnings (new ones prefixed by >>):
>> arch/x86/kernel/smpboot.c:1097:6: warning: variable 'ret' is used
>> uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (slaunch_is_txt_launch())
^~~
arch/x86/kernel/smpboot.c:1107:6: note: uninitialized use occurs here
if (ret)
^~~
arch/x86/kernel/smpboot.c:1097:2: note: remove the 'if' if its condition is
always false
if (slaunch_is_txt_launch())
^~~~
arch/x86/kernel/smpboot.c:1046:9: note: initialize the variable 'ret' to
silence this warning
int ret;
^
= 0
1 warning generated.
vim +1097 arch/x86/kernel/smpboot.c
1036
1037 /*
1038 * NOTE - on most systems this is a PHYSICAL apic ID, but on multiquad
1039 * (ie clustered apic addressing mode), this is a LOGICAL apic ID.
1040 * Returns zero if startup was successfully sent, else error code from
1041 * ->wakeup_secondary_cpu.
1042 */
1043 static int do_boot_cpu(u32 apicid, int cpu, struct task_struct *idle)
1044 {
1045 unsigned long start_ip = real_mode_header->trampoline_start;
1046 int ret;
1047
1048 #ifdef CONFIG_X86_64
1049 /* If 64-bit wakeup method exists, use the 64-bit mode
trampoline IP */
1050 if (apic->wakeup_secondary_cpu_64)
1051 start_ip = real_mode_header->trampoline_start64;
1052 #endif
1053 idle->thread.sp = (unsigned long)task_pt_regs(idle);
1054 initial_code = (unsigned long)start_secondary;
1055
1056 if (IS_ENABLED(CONFIG_X86_32)) {
1057 early_gdt_descr.address = (unsigned
long)get_cpu_gdt_rw(cpu);
1058 initial_stack = idle->thread.sp;
1059 } else if (!(smpboot_control & STARTUP_PARALLEL_MASK)) {
1060 smpboot_control = cpu;
1061 }
1062
1063 /* Enable the espfix hack for this CPU */
1064 init_espfix_ap(cpu);
1065
1066 /* So we see what's up */
1067 announce_cpu(cpu, apicid);
1068
1069 /*
1070 * This grunge runs the startup process for
1071 * the targeted processor.
1072 */
1073 if (x86_platform.legacy.warm_reset) {
1074
1075 pr_debug("Setting warm reset code and vector.\n");
1076
1077 smpboot_setup_warm_reset_vector(start_ip);
1078 /*
1079 * Be paranoid about clearing APIC errors.
1080 */
1081 if (APIC_INTEGRATED(boot_cpu_apic_version)) {
1082 apic_write(APIC_ESR, 0);
1083 apic_read(APIC_ESR);
1084 }
1085 }
1086
1087 smp_mb();
1088
1089 /*
1090 * Wake up a CPU in difference cases:
1091 * - Intel TXT DRTM launch uses its own method to wake the APs
1092 * - Use a method from the APIC driver if one defined, with
wakeup
1093 * straight to 64-bit mode preferred over wakeup to RM.
1094 * Otherwise,
1095 * - Use an INIT boot APIC message
1096 */
> 1097 if (slaunch_is_txt_launch())
1098 slaunch_wakeup_cpu_from_txt(cpu, apicid);
1099 else if (apic->
