cleanup: Handle signed RW images a bit cleaner

For signed EC RW images (CONFIG_RWSIG), there's no point in
embedding the public key or signature into the image itself since
it will just be replaced by the signer (either as the next step
in the build process, or after the fact for MP releases). This
takes that out and just points to where the pubkey and signature
will be placed.

BUG=none
BRANCH=none
TEST=make buildall

I also checked the signatures with

  futility show -t build/*/ec.bin

They still look good, and the one signed image I booted (Cr50)
works as before.

Change-Id: Ib39b7c508914851f81a1bebb2450e08ef0def76c
Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/302630
Reviewed-by: Randall Spangler <rspangler@chromium.org>

9 files changed, 61 insertions, 308 deletions

diff --git a/Makefile.rules b/Makefile.rules
index 65500f317b..e2951d095a 100644
--- a/Makefile.rules
+++ b/Makefile.rules
@@ -71,7 +71,6 @@ cmd_sharedlib_elf = $(CC) $(libsharedobjs_deps) \
 	-Wl,-Map,$(out)/$(SHOBJLIB)/$(SHOBJLIB).map
 
 # commands for RSA signature
-cmd_pubkey = ./util/pem_extract_pubkey.py $(PEM) > $@
 cmd_rsasign = futility sign --type usbpd1 --pem $(PEM) $(out)/$*.bin.tmp
 
 # commands to build optional xref files
@@ -274,12 +273,6 @@ $(sharedlib-objs): | $(out)/ec_version.h
 $(out)/ec_version.h:
 	$(call quiet,version,VERSION)
 
-$(out)/gen_pub_key.h: $(PEM)
-	$(call quiet,pubkey,PUBKEY )
-
-$(out)/RO/common/rwsig.o: $(out)/gen_pub_key.h
-$(out)/RW/common/rwsig.o: $(out)/gen_pub_key.h
-
 $(build-utils): $(out)/%:$(build-srcs)
 	$(call quiet,c_to_build,BUILDCC)
 
diff --git a/board/zinger/board.c b/board/zinger/board.c
index f7552523a5..ac3b95db20 100644
--- a/board/zinger/board.c
+++ b/board/zinger/board.c
@@ -16,13 +16,6 @@
 #include "util.h"
 #include "version.h"
 
-/* Insert the RSA public key definition */
-const struct rsa_public_key pkey __attribute__((section(".rsa_pubkey"))) =
-#include "gen_pub_key.h"
-/* The RSA signature is stored at the end of the RW firmware */
-static const void *rw_sig = (void *)CONFIG_PROGRAM_MEMORY_BASE +
-				    CONFIG_RW_MEM_OFF + CONFIG_RW_SIZE -
-				    RSANUMBYTES;
 /* Large 768-Byte buffer for RSA computation : could be re-use afterwards... */
 static uint32_t rsa_workbuf[3 * RSANUMWORDS];
 
@@ -64,7 +57,9 @@ static int check_rw_valid(void *rw_hash)
 	if (*rw_rst == 0xffffffff)
 		return 0;
 
-	good = rsa_verify(&pkey, (void *)rw_sig, rw_hash, rsa_workbuf);
+	good = rsa_verify((const struct rsa_public_key *)CONFIG_RO_PUBKEY_ADDR,
+			  (const uint8_t *)CONFIG_RW_SIG_ADDR,
+			  rw_hash, rsa_workbuf);
 	if (!good) {
 		debug_printf("RSA FAILED\n");
 		pd_log_event(PD_EVENT_ACC_RW_FAIL, 0, 0, NULL);
diff --git a/board/zinger/build.mk b/board/zinger/build.mk
index fd04fe0521..4e9e2f1f6d 100644
--- a/board/zinger/build.mk
+++ b/board/zinger/build.mk
@@ -12,7 +12,3 @@ CHIP_VARIANT:=stm32f03x
 
 board-y=board.o hardware.o runtime.o usb_pd_policy.o
 board-$(CONFIG_DEBUG_PRINTF)+=debug.o
-
-# Add dependency to generate the public key coefficients header
-$(out)/RO/board/$(BOARD)/board.o: $(out)/gen_pub_key.h
-$(out)/RW/board/$(BOARD)/board.o: $(out)/gen_pub_key.h
diff --git a/common/rwsig.c b/common/rwsig.c
index c58e7ab84d..ee1bd6c9fc 100644
--- a/common/rwsig.c
+++ b/common/rwsig.c
@@ -20,15 +20,6 @@
 #define CPRINTF(format, args...) cprintf(CC_SYSTEM, format, ## args)
 #define CPRINTS(format, args...) cprints(CC_SYSTEM, format, ## args)
 
-/* Insert the RSA public key definition */
-const struct rsa_public_key pkey __attribute__((section(".rsa_pubkey"))) =
-#include "gen_pub_key.h"
-
-/* The RSA signature is stored at the end of the RW firmware */
-static const void *rw_sig = (void *)CONFIG_PROGRAM_MEMORY_BASE
-				 + CONFIG_RW_MEM_OFF
-				 + CONFIG_RW_SIZE - RSANUMBYTES;
-
 /* RW firmware reset vector */
 static uint32_t * const rw_rst =
 	(uint32_t *)(CONFIG_PROGRAM_MEMORY_BASE + CONFIG_RW_MEM_OFF + 4);
@@ -62,10 +53,12 @@ void check_rw_signature(void)
 	SHA256_init(&ctx);
 	SHA256_update(&ctx, (void *)CONFIG_PROGRAM_MEMORY_BASE
 		      + CONFIG_RW_MEM_OFF,
-		      CONFIG_RW_SIZE - RSANUMBYTES);
+		      CONFIG_RW_SIZE - CONFIG_RW_SIG_SIZE);
 	hash = SHA256_final(&ctx);
 
-	good = rsa_verify(&pkey, (void *)rw_sig, (void *)hash, rsa_workbuf);
+	good = rsa_verify((const struct rsa_public_key *)CONFIG_RO_PUBKEY_ADDR,
+			  (const uint8_t *)CONFIG_RW_SIG_ADDR,
+			  hash, rsa_workbuf);
 	if (good) {
 		CPRINTS("RW image verified");
 		/* Jump to the RW firmware */
diff --git a/core/cortex-m/ec.lds.S b/core/cortex-m/ec.lds.S
index ca99d43b1b..3e026331fe 100644
--- a/core/cortex-m/ec.lds.S
+++ b/core/cortex-m/ec.lds.S
@@ -40,11 +40,6 @@ MEMORY
         ORIGIN = CONFIG_PROGRAM_MEMORY_BASE + FW_MEM_OFF(SECTION), \
         LENGTH = FW_SIZE(SECTION)
 #endif /* CONFIG_EXTERNAL_STORAGE */
-#ifdef RSA_PUBLIC_KEY_SIZE
-    PSTATE(r)  : \
-        ORIGIN = FW_OFF(SECTION) + FW_SIZE(SECTION), \
-        LENGTH = CONFIG_FW_PSTATE_SIZE
-#endif
 #ifdef CONFIG_USB_RAM_SIZE
     USB_RAM (rw) : \
         ORIGIN = CONFIG_USB_RAM_BASE, \
@@ -266,24 +261,6 @@ SECTIONS
 	/* NOTHING MAY GO AFTER THIS! */
     } > IRAM
 
-#ifdef RSA_PUBLIC_KEY_SIZE
-#ifdef SECTION_IS_RO
-    .flash_suffix : {
-        FILL(0xff);
-        /*
-	 * Put the public key coefficients at the end of the partition
-	 * after the pstate bits.
-	 */
-        . = ORIGIN(PSTATE) + LENGTH(PSTATE) - RSA_PUBLIC_KEY_SIZE;
-        *(.rsa_pubkey)
-    } > PSTATE
-#else /* RW section: we don't need the RSA public key, put it anywhere */
-    .flash_suffix  : AT(LOADADDR(.data) + SIZEOF(.data)) {
-        *(.rsa_pubkey)
-    } > FLASH
-#endif
-#endif
-
     /* The linker won't notice if the .data section is too big to fit,
      * apparently because we're sending it into IRAM, not FLASH. The following
      * symbol isn't used by the code, but running "objdump -t *.elf | grep hey"
@@ -291,8 +268,14 @@ SECTIONS
      * explicit ASSERT afterwards will cause the linker to abort if we use too
      * much. */
     __hey_flash_used = LOADADDR(.data) + SIZEOF(.data) - FW_OFF(SECTION);
-    ASSERT(FW_SIZE(SECTION) >=
-                 (LOADADDR(.data) + SIZEOF(.data) - FW_OFF(SECTION)),
+    ASSERT((FW_SIZE(SECTION)
+#if defined(CONFIG_RWSIG) && defined(SECTION_IS_RO)
+        - CONFIG_RO_PUBKEY_SIZE
+#endif
+#if defined(CONFIG_RWSIG) && defined(SECTION_IS_RW)
+        - CONFIG_RW_SIG_SIZE
+#endif
+        ) >= (LOADADDR(.data) + SIZEOF(.data) - FW_OFF(SECTION)),
                  "No room left in the flash")
 
 #ifdef CONFIG_USB_RAM_SIZE
diff --git a/core/cortex-m0/ec.lds.S b/core/cortex-m0/ec.lds.S
index f76c5c7e03..4bf9865136 100644
--- a/core/cortex-m0/ec.lds.S
+++ b/core/cortex-m0/ec.lds.S
@@ -22,10 +22,6 @@ MEMORY
 #endif
     FLASH (rx) : ORIGIN = FW_OFF(SECTION), LENGTH = FW_SIZE(SECTION)
     IRAM (rw)  : ORIGIN = CONFIG_RAM_BASE, LENGTH = CONFIG_RAM_SIZE
-#ifdef RSA_PUBLIC_KEY_SIZE
-    PSTATE(r)  : ORIGIN = FW_OFF(SECTION) + FW_SIZE(SECTION), \
-                 LENGTH = CONFIG_FW_PSTATE_SIZE
-#endif
 #ifdef CONFIG_USB_RAM_SIZE
     USB_RAM (rw) : \
         ORIGIN = CONFIG_USB_RAM_BASE, \
@@ -213,24 +209,6 @@ SECTIONS
 	/* NOTHING MAY GO AFTER THIS! */
     } > IRAM
 
-#ifdef RSA_PUBLIC_KEY_SIZE
-#ifdef SECTION_IS_RO
-    .flash_suffix : {
-        FILL(0xff);
-        /*
-	 * Put the public key coefficients at the end of the partition
-	 * after the pstate bits.
-	 */
-        . = ORIGIN(PSTATE) + LENGTH(PSTATE) - RSA_PUBLIC_KEY_SIZE;
-        *(.rsa_pubkey)
-    } > PSTATE
-#else /* RW section: we don't need the RSA public key, put it anywhere */
-    .flash_suffix  : AT(LOADADDR(.data) + SIZEOF(.data)) {
-        *(.rsa_pubkey)
-    } > FLASH
-#endif
-#endif
-
     /* The linker won't notice if the .data section is too big to fit,
      * apparently because we're sending it into IRAM, not FLASH. The following
      * symbol isn't used by the code, but running "objdump -t *.elf | grep hey"
@@ -238,8 +216,14 @@ SECTIONS
      * explicit ASSERT afterwards will cause the linker to abort if we use too
      * much. */
     __hey_flash_used = LOADADDR(.data) + SIZEOF(.data) - FW_OFF(SECTION);
-    ASSERT(FW_SIZE(SECTION) >=
-                 (LOADADDR(.data) + SIZEOF(.data) - FW_OFF(SECTION)),
+    ASSERT((FW_SIZE(SECTION)
+#if defined(CONFIG_RWSIG) && defined(SECTION_IS_RO)
+        - CONFIG_RO_PUBKEY_SIZE
+#endif
+#if defined(CONFIG_RWSIG) && defined(SECTION_IS_RW)
+        - CONFIG_RW_SIG_SIZE
+#endif
+        ) >= (LOADADDR(.data) + SIZEOF(.data) - FW_OFF(SECTION)),
                  "No room left in the flash")
 
 #ifdef CONFIG_USB_RAM_SIZE
diff --git a/include/config.h b/include/config.h
index 1094c89f11..bdffee4cfa 100644
--- a/include/config.h
+++ b/include/config.h
@@ -1412,6 +1412,15 @@
  * (for accessories without software sync)
  */
 #undef CONFIG_RWSIG
+/*
+ * By default the pubkey and sig are put at the end of the first and second
+ * half of the total flash, and take up the minimum space possible. You can
+ * override those defaults with these.
+ */
+#undef CONFIG_RO_PUBKEY_ADDR
+#undef CONFIG_RO_PUBKEY_SIZE
+#undef CONFIG_RW_SIG_ADDR
+#undef CONFIG_RW_SIG_SIZE
 
 /****************************************************************************/
 /* Shared objects library. */
diff --git a/include/rsa.h b/include/rsa.h
index ab45695910..2fb896c652 100644
--- a/include/rsa.h
+++ b/include/rsa.h
@@ -12,7 +12,7 @@
 #define CONFIG_RSA_KEY_SIZE 2048 /* default to 2048-bit key length */
 #endif
 
-#define RSANUMBYTES ((CONFIG_RSA_KEY_SIZE)/8)
+#define RSANUMBYTES ((CONFIG_RSA_KEY_SIZE) / 8)
 #define RSANUMWORDS (RSANUMBYTES / sizeof(uint32_t))
 
 #ifdef CONFIG_RSA /* reserve space for public key only if used */
@@ -53,4 +53,32 @@ void check_rw_signature(void);
 
 #endif /* !__ASSEMBLER__ */
 
+/*
+ * The signer puts the public key and signature into the RO and RW images
+ * (respectively) at known locations after the complete image is assembled. But
+ * since we compile the RO & RW images separately, the other image's addresses
+ * can't be computed by the linker. So we just hardcode the addresses here.
+ * These can be overridden in board.h files if desired.
+ */
+
+/* The pubkey goes at the end of the first half of flash */
+#ifndef CONFIG_RO_PUBKEY_SIZE
+#define CONFIG_RO_PUBKEY_SIZE RSA_PUBLIC_KEY_SIZE
+#endif
+#ifndef CONFIG_RO_PUBKEY_ADDR
+#define CONFIG_RO_PUBKEY_ADDR (CONFIG_PROGRAM_MEMORY_BASE	\
+			       + (CONFIG_FLASH_SIZE / 2)	\
+			       - CONFIG_RO_PUBKEY_SIZE)
+#endif
+
+/* The signature goes at the end of the second half of flash */
+#ifndef CONFIG_RW_SIG_SIZE
+#define CONFIG_RW_SIG_SIZE RSANUMBYTES
+#endif
+#ifndef CONFIG_RW_SIG_ADDR
+#define CONFIG_RW_SIG_ADDR (CONFIG_PROGRAM_MEMORY_BASE	\
+			    + CONFIG_FLASH_SIZE		\
+			    - CONFIG_RW_SIG_SIZE)
+#endif
+
 #endif /* __CROS_EC_RSA_H */
diff --git a/util/pem_extract_pubkey.py b/util/pem_extract_pubkey.py
deleted file mode 100755
index d368da8ee7..0000000000
--- a/util/pem_extract_pubkey.py
+++ /dev/null
@@ -1,228 +0,0 @@
-#!/usr/bin/env python
-# Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style license that can be
-# found in the LICENSE file.
-"""Extract the public key from a .pem RSA private key file (PKCS#1),
-  and compute the public key coefficients used by the signature verification
-  code.
-
-  Example:
-    ./util/pem_extract_pubkey board/zinger/zinger_dev_key.pem
-
-  Note: to generate a suitable private key :
-  RSA 2048-bit with public exponent F4 (65537)
-  you can use the following OpenSSL command :
-  openssl genrsa -F4 -out private.pem 2048
-"""
-
-import array
-import base64
-import sys
-
-VERSION = '0.0.1'
-
-"""
-RSA Private Key file (PKCS#1) encoding :
-
-It starts and ends with the tags:
------BEGIN RSA PRIVATE KEY-----
-BASE64 ENCODED DATA
------END RSA PRIVATE KEY-----
-
-The base64 encoded data is using an ASN.1 / DER structure :
-
-RSAPrivateKey ::= SEQUENCE {
-  version           Version,
-  modulus           INTEGER,  -- n
-  publicExponent    INTEGER,  -- e
-  privateExponent   INTEGER,  -- d
-  prime1            INTEGER,  -- p
-  prime2            INTEGER,  -- q
-  exponent1         INTEGER,  -- d mod (p-1)
-  exponent2         INTEGER,  -- d mod (q-1)
-  coefficient       INTEGER,  -- (inverse of q) mod p
-  otherPrimeInfos   OtherPrimeInfos OPTIONAL
-}
-"""
-
-PEM_HEADER='-----BEGIN RSA PRIVATE KEY-----'
-PEM_FOOTER='-----END RSA PRIVATE KEY-----'
-
-# supported RSA key sizes
-RSA_KEY_SIZES=[2048, 4096, 8192]
-
-class PEMError(Exception):
-  """Exception class for pem_extract_pubkey utility."""
-
-# "Constructed" bit in DER tag
-DER_C=0x20
-# DER Sequence tag (always constructed)
-DER_SEQUENCE=DER_C|0x10
-# DER Integer tag
-DER_INTEGER=0x02
-
-class DER:
-  """DER encoded binary data storage and parser."""
-  def __init__(self, data):
-    # DER encoded binary data
-    self._data = data
-    # Initialize index in the data stream
-    self._idx = 0
-
-  def get_byte(self):
-    octet = ord(self._data[self._idx])
-    self._idx += 1
-    return octet
-
-  def get_len(self):
-    octet = self.get_byte()
-    if octet == 0x80:
-      raise PEMError('length indefinite form not supported')
-    if octet & 0x80: # Length long form
-      bytecnt = octet & ~0x80
-      total = 0
-      for i in range(bytecnt):
-        total = (total << 8) | self.get_byte()
-      return total
-    else: # Length short form
-      return octet
-
-  def get_tag(self):
-    tag = self.get_byte()
-    length = self.get_len()
-    data = self._data[self._idx:self._idx + length]
-    self._idx += length
-    return {"tag" : tag, "length" : length, "data" : data}
-
-def pem_get_mod(filename):
-  """Extract the modulus from a PEM private key file.
-
-  the PEM file is DER encoded according the structure quoted above.
-
-  Args:
-    filename : Full path to the .pem private key file.
-
-  Raises:
-    PEMError: If unable to parse .pem file or invalid file format.
-  """
-  # Read all the content of the .pem file
-  content = file(filename).readlines()
-  # Check the PEM RSA Private key tags
-  if content[0].strip() != PEM_HEADER:
-    raise PEMError('invalid PEM private key header')
-  if content[-1].strip() != PEM_FOOTER:
-    raise PEMError('invalid PEM private key footer')
-  # Decode the DER binary stream from the base64 data
-  b64 = "".join([l.strip() for l in content[1:-1]])
-  der = DER(base64.b64decode(b64))
-
-  # Parse the DER and fail at the first error
-  # The private key should be a (constructed) sequence
-  seq = der.get_tag()
-  if seq["tag"] != DER_SEQUENCE:
-    raise PEMError('expecting an ASN.1 sequence')
-  seq = DER(seq["data"])
-
-  # 1st field is Version
-  ver = seq.get_tag()
-  if ver["tag"] != DER_INTEGER:
-    raise PEMError('version field should be an integer')
-
-  # 2nd field is Modulus
-  mod = seq.get_tag()
-  if mod["tag"] != DER_INTEGER:
-    raise PEMError('modulus field should be an integer')
-  # 2048 bits + mandatory ASN.1 sign (0) => 257 Bytes
-  modSize = (mod["length"] - 1) * 8
-  if modSize not in RSA_KEY_SIZES or mod["data"][0] != '\x00':
-    raise PEMError('Invalid key length : %d bits' % (modSize))
-
-  # 3rd field is Public Exponent
-  exp = seq.get_tag()
-  if exp["tag"] != DER_INTEGER:
-    raise PEMError('exponent field should be an integer')
-  if exp["length"] != 3 or exp["data"] != "\x01\x00\x01":
-    raise PEMError('the public exponent must be F4 (65537)')
-
-  return mod["data"]
-
-def modinv(a, m):
-  """ The multiplicitive inverse of a in the integers modulo m.
-
-  Return b when a * b == 1 mod m
-  """
-  # Extended GCD
-  lastrem, rem = abs(a), abs(m)
-  x, lastx, y, lasty = 0, 1, 1, 0
-  while rem:
-    lastrem, (quotient, rem) = rem, divmod(lastrem, rem)
-    x, lastx = lastx - quotient*x, x
-    y, lasty = lasty - quotient*y, y
-  #
-  if lastrem != 1:
-    raise ValueError
-  x = lastx * (-1 if a < 0 else 1)
-  return x % m
-
-def to_words(n, count):
-  h = '%x' % n
-  s = ('0'*(len(h) % 2) + h).zfill(count*8).decode('hex')
-  return array.array("I", s[::-1])
-
-def compute_mod_parameters(modulus):
-  ''' Prepare/pre-compute coefficients for the RSA public key signature
-    verification code.
-  '''
-  # create an array of uint32_t to store the modulus but skip the sign byte
-  w = array.array("I",modulus[1:])
-  wordCount = (len(modulus) - 1) / 4
-  # all integers in DER encoded .pem file are big endian.
-  w.reverse()
-  w.byteswap()
-  # convert the big-endian modulus to a big integer for the computations
-  N = 0
-  for i in range(len(modulus)):
-    N = (N << 8) | ord(modulus[i])
-  # -1 / N[0] mod 2^32
-  B = 0x100000000L
-  n0inv = B - modinv(w[0], B)
-  # R = 2^(modulo size); RR = (R * R) % N
-  RR = pow(2, 4096, N)
-  rr_words = to_words(RR, wordCount)
-
-  return {'mod':w, 'rr':rr_words, 'n0inv':n0inv}
-
-def print_header(params):
-  print "{\n\t.n = {%s}," % (",".join(["0x%08x" % (i) for i in params['mod']]))
-  print "\t.rr = {%s}," % (",".join(["0x%08x" % (i) for i in params['rr']]))
-  print "\t.n0inv = 0x%08x\n};" % (params['n0inv'])
-
-def dump_blob(params):
-  mod_bin = params['mod'].tostring()
-  rr_bin = params['rr'].tostring()
-  n0inv_bin = array.array("I",[params['n0inv']]).tostring()
-  return mod_bin + rr_bin + n0inv_bin
-
-def extract_pubkey(pemfile, headerMode=True):
-  # Read the modulus in the .pem file
-  mod = pem_get_mod(sys.argv[1])
-  # Pre-compute the parameters used by the verification code
-  p = compute_mod_parameters(mod)
-
-  if headerMode:
-    # Generate a C header file with the parameters
-    print_header(p)
-  else:
-    # Generate the packed structure as a binary blob
-    return dump_blob(p)
-
-if __name__ == '__main__':
-  try:
-    if len(sys.argv) < 2:
-      raise PEMError('Invalid arguments. Usage: ./pem_extract_pubkey priv.pem')
-    extract_pubkey(sys.argv[1])
-  except KeyboardInterrupt:
-    sys.exit(0)
-  except PEMError as e:
-    sys.stderr.write("Error: %s\n" % (e.message))
-    sys.exit(1)