g: fix signer to always use the manifest

Since the proper signer utility has been introduced in the chroot,
there is no need in generating reduced command option set when
building a self signed image.

Also, the same manifest can be used for all images, self signed or
signed using a fob. The manifest needs to be tweaked for the self
signed images to match the test Key ID.

Since the same base manifest is used for all signings, there is no
need to support the "poor man's json parser" any more.

Rearranged build.mk to accommodate new logic, and added some comments.

BRANCH=cr50, cr50-mp
BUG=b:78212718
TEST=verified that images with proper header version are created when
     both self signed and signed with a private key coming from the
     signing fob.

Change-Id: I5a1f8a223098b0a6c830ef24ffe380fc0badcafa
Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1017238
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

4 files changed, 22 insertions, 82 deletions

diff --git a/Makefile b/Makefile
index e1ba98270d..edae085750 100644
--- a/Makefile
+++ b/Makefile
@@ -197,7 +197,6 @@ endif
 include test/build.mk
 include util/build.mk
 include util/lock/build.mk
-include util/signer/build.mk
 
 includes+=$(includes-y)
 
diff --git a/chip/g/build.mk b/chip/g/build.mk
index 0c80af042d..e6363f9e10 100644
--- a/chip/g/build.mk
+++ b/chip/g/build.mk
@@ -124,19 +124,37 @@ endif
 
 MANIFEST := util/signer/ec_RW-manifest-dev.json
 CR50_RO_KEY ?= rom-testkey-A.pem
+
+# Make sure signing happens only when the signer is available.
 REAL_SIGNER = /usr/bin/cr50-codesigner
 ifneq ($(wildcard $(REAL_SIGNER)),)
 SIGNED_IMAGES = 1
 SIGNER := $(REAL_SIGNER)
 endif
 
+ifeq ($(CHIP_MK_INCLUDED_ONCE),)
+
+CHIP_MK_INCLUDED_ONCE := 1
+# We'll have to tweak the manifest no matter what, but different ways
+# depending on the way the image is built.
+SIGNER_MANIFEST := $(shell mktemp /tmp/h1.signer.XXXXXX)
+RW_SIGNER_EXTRAS += -j $(SIGNER_MANIFEST) -x util/signer/fuses.xml
+
+ifneq ($(CR50_SWAP_RMA_KEYS),)
+RMA_KEY_BASE := board/$(BOARD)/rma_key_blob
+RW_SIGNER_EXTRAS += --swap $(RMA_KEY_BASE).test,$(RMA_KEY_BASE).prod
+endif
+
+endif
+
 ifeq ($(H1_DEVIDS),)
+# Signing with non-secret test key.
 CR50_RW_KEY = loader-testkey-A.pem
-SIGNER_EXTRAS =
-SIGNER_MANIFEST := $(MANIFEST)
+# Make sure manifset Key ID field matches the actual key.
+DUM := $(shell sed 's/1187158727/764428053/' $(MANIFEST) > $(SIGNER_MANIFEST))
 else
+# The private key comes from the sighing fob.
 CR50_RW_KEY = cr50_rom0-dev-blsign.pem.pub
-RW_SIGNER_EXTRAS = -x util/signer/fuses.xml
 
 ifneq ($(CHIP_MK_INCLUDED_ONCE),)
 #
@@ -152,9 +170,6 @@ ifneq ($(CHIP_MK_INCLUDED_ONCE),)
 #
 # H1_DEVIDS='<num 1> <num 2>' make ...
 #
-ifeq ($(SIGNER_MANIFEST),)
-SIGNER_MANIFEST := $(shell mktemp /tmp/h1.signer.XXXXXX)
-endif
 ifneq ($(CR50_DEV),)
 
 #
@@ -175,7 +190,6 @@ REPLACEMENT := $(shell printf \
 NODE_JSON :=  $(shell sed -i \
 	"s/\"fuses\": {/\"fuses\": {$(REPLACEMENT)/" $(SIGNER_MANIFEST))
 
-RW_SIGNER_EXTRAS += -j $(SIGNER_MANIFEST)
 endif  # CHIP_MK_INCLUDED_ONCE defined
 endif  # H1_DEVIDS defined
 
@@ -184,15 +198,7 @@ endif  # H1_DEVIDS defined
 # # and then again after defining all the CONFIG_ and HAS_TASK variables. We use
 # # a guard so that recipe definitions and variable extensions only happen the
 # # second time.
-ifeq ($(CHIP_MK_INCLUDED_ONCE),)
-CHIP_MK_INCLUDED_ONCE=1
-else
-
-ifneq ($(CR50_SWAP_RMA_KEYS),)
-RMA_KEY_BASE := board/$(BOARD)/rma_key_blob
-RW_SIGNER_EXTRAS += --swap $(RMA_KEY_BASE).test,$(RMA_KEY_BASE).prod
-endif
-
+ifneq ($(CHIP_MK_INCLUDED_ONCE),)
 $(out)/RW/ec.RW_B.flat: $(out)/RW/ec.RW.flat
 $(out)/RW/ec.RW.flat $(out)/RW/ec.RW_B.flat: SIGNER_EXTRAS = $(RW_SIGNER_EXTRAS)
 
diff --git a/util/signer/build.mk b/util/signer/build.mk
deleted file mode 100644
index faab4f085f..0000000000
--- a/util/signer/build.mk
+++ /dev/null
@@ -1,12 +0,0 @@
-# -*- makefile -*-
-# Copyright 2015 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.
-
-# When building self signed Cr50 images we still want the epoch/major/minor
-# fields come from the dev manifest. Since a full blown JSON parser for C is
-# not readily available, this rule generates a small .h file with only the
-# fields of interest retrieved from the dev JSON file.
-$(out)/pmjp.h: util/signer/pmjp.py util/signer/ec_RW-manifest-dev.json
-	@echo "  PMJP $@"
-	$(Q)./util/signer/pmjp.py ./util/signer/ec_RW-manifest-dev.json > $@
diff --git a/util/signer/pmjp.py b/util/signer/pmjp.py
deleted file mode 100755
index 92e3db035c..0000000000
--- a/util/signer/pmjp.py
+++ /dev/null
@@ -1,53 +0,0 @@
-#!/usr/bin/python
-# Copyright 2017 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.
-
-"""Poor man's JSON parser.
-
-This module reads the input JSON file, retrieves from it some name/value pairs
-and generates a .h file to allow a C code use the definitions.
-
-The JSON file name is required to be passed in in the command line, the nodes
-this script pays attention to are included in required_keys tuple below.
-"""
-
-import json
-import sys
-
-required_keys = ('epoch', 'major', 'minor')
-
-
-def main(json_file_name):
-  # get rid of the comments
-  json_text = []
-  h_file_text = ['''
-/*
- * Copyright %d 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.
- */
-
-/* This file was autogenerated, do not edit. */
-''',]
-
-  json_file = open(json_file_name, 'r')
-  for line in json_file.read().splitlines():
-    json_text.append(line.split('//')[0])
-
-  j = json.loads('\n'.join(json_text))
-
-  for key in required_keys:
-    if key in j.keys():
-      value = j[key]
-    else:
-      value = '0'
-
-    h_file_text.append('#define MANIFEST_%s %s' % (key.upper(), value))
-
-  h_file_text.append('')
-  return '\n'.join(h_file_text)
-
-
-if __name__ == '__main__':
-  print main(sys.argv[1])