diff mbox series

tools: k3_gen_x509_cert: Allow selecting early JTAG debug value

Message ID 20200422170941.13129-1-afd@ti.com
State Superseded
Headers show
Series tools: k3_gen_x509_cert: Allow selecting early JTAG debug value | expand

Commit Message

Andrew Davis April 22, 2020, 5:09 p.m. UTC
When authenticating the initial boot binary the ROM will check a debug
type value in the certificate and based on that open JTAG access to that
core.

The default is currently full access, on HS this is useful for early
developers, but should not be the default as to prevent end system
integrators from unintentionally leaving this open.

On non-HS devices JTAG is left open anyway so this does not change
behavior on those devices. Change the default to off and allow it to be
switched back on by command line flag if needed.

Signed-off-by: Andrew F. Davis <afd at ti.com>
---
 tools/k3_gen_x509_cert.sh | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Lokesh Vutla April 23, 2020, 6:38 a.m. UTC | #1
On 22/04/20 10:39 PM, Andrew F. Davis wrote:
> When authenticating the initial boot binary the ROM will check a debug
> type value in the certificate and based on that open JTAG access to that
> core.
> 
> The default is currently full access, on HS this is useful for early
> developers, but should not be the default as to prevent end system
> integrators from unintentionally leaving this open.

Won't JTAG access is useful for early developers. UART as well not available at
this point. What we offer out of the box is not a production ready secure
system.  I would prefer to have it enabled by default.

Thanks and regards,
Lokesh

> 
> On non-HS devices JTAG is left open anyway so this does not change
> behavior on those devices. Change the default to off and allow it to be
> switched back on by command line flag if needed.
> 
> Signed-off-by: Andrew F. Davis <afd at ti.com>
> ---
>  tools/k3_gen_x509_cert.sh | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/k3_gen_x509_cert.sh b/tools/k3_gen_x509_cert.sh
> index b6d055f6f5..298cec1313 100755
> --- a/tools/k3_gen_x509_cert.sh
> +++ b/tools/k3_gen_x509_cert.sh
> @@ -12,6 +12,7 @@ RAND_KEY=eckey.pem
>  LOADADDR=0x41c00000
>  BOOTCORE_OPTS=0
>  BOOTCORE=16
> +DEBUG_TYPE=0
>  
>  gen_degen_template() {
>  cat << 'EOF' > degen-template.txt
> @@ -79,7 +80,7 @@ cat << 'EOF' > x509-template.txt
>  
>   [ debug ]
>   debugUID = FORMAT:HEX,OCT:0000000000000000000000000000000000000000000000000000000000000000
> - debugType = INTEGER:4
> + debugType = INTEGER:TEST_DEBUG_TYPE
>   coreDbgEn = INTEGER:0
>   coreDbgSecEn = INTEGER:0
>  EOF
> @@ -151,8 +152,9 @@ options_help[k]="key_file:file with key inside it. If not provided script genera
>  options_help[o]="output_file:Name of the final output file. default to $OUTPUT"
>  options_help[c]="core_id:target core id on which the image would be running. Default to $BOOTCORE"
>  options_help[l]="loadaddr: Target load address of the binary in hex. Default to $LOADADDR"
> +options_help[d]="debug_type: Debug type, set to 4 to enable early JTAG. Default to $DEBUG_TYPE"
>  
> -while getopts "b:k:o:c:l:h" opt
> +while getopts "b:k:o:c:l:d:h" opt
>  do
>  	case $opt in
>  	b)
> @@ -170,6 +172,9 @@ do
>  	c)
>  		BOOTCORE=$OPTARG
>  	;;
> +	d)
> +		DEBUG_TYPE=$OPTARG
> +	;;
>  	h)
>  		usage
>  		exit 0
> @@ -224,12 +229,15 @@ gen_cert() {
>  	#echo "	LOADADDR = 0x$ADDR"
>  	#echo "	IMAGE_SIZE = $BIN_SIZE"
>  	#echo "	CERT_TYPE = $CERTTYPE"
> +	#echo "	DEBUG_TYPE = $DEBUG_TYPE"
>  	sed -e "s/TEST_IMAGE_LENGTH/$BIN_SIZE/"	\
>  		-e "s/TEST_IMAGE_SHA_VAL/$SHA_VAL/" \
>  		-e "s/TEST_CERT_TYPE/$CERTTYPE/" \
>  		-e "s/TEST_BOOT_CORE_OPTS/$BOOTCORE_OPTS/" \
>  		-e "s/TEST_BOOT_CORE/$BOOTCORE/" \
> -		-e "s/TEST_BOOT_ADDR/$ADDR/" x509-template.txt > $TEMP_X509
> +		-e "s/TEST_BOOT_ADDR/$ADDR/" \
> +		-e "s/TEST_DEBUG_TYPE/$DEBUG_TYPE/" \
> +		x509-template.txt > $TEMP_X509
>  	openssl req -new -x509 -key $KEY -nodes -outform DER -out $CERT -config $TEMP_X509 -sha512
>  }
>  
>
Andrew Davis April 23, 2020, 12:42 p.m. UTC | #2
On 4/23/20 2:38 AM, Lokesh Vutla wrote:
> 
> 
> On 22/04/20 10:39 PM, Andrew F. Davis wrote:
>> When authenticating the initial boot binary the ROM will check a debug
>> type value in the certificate and based on that open JTAG access to that
>> core.
>>
>> The default is currently full access, on HS this is useful for early
>> developers, but should not be the default as to prevent end system
>> integrators from unintentionally leaving this open.
> 
> Won't JTAG access is useful for early developers. UART as well not available at
> this point. What we offer out of the box is not a production ready secure
> system.  I would prefer to have it enabled by default.
> 


Who are these early developers? Pre-SYSFW on HS is a harsh environment,
firewalls and other pitfalls limit one to only what is needed to get
SYSFW loaded. Only a handful of folks will ever touch the source this
early, and they will be using a GP device for that development, in which
case debug is enabled, even with this change.

I'd guess I'm the only developer touching code this early on HS, I say
this as so far I'm the only one who has noticed that there is a ROM
issue that makes early debug on HS almost unusable.

Accidentally leaving this open completely defeats the chain of trust, I
guarantee some production system will do this if we leave JTAG open by
default, it happens all the time.

So debug doesn't work here anyway, no one uses it and those who do can
flip this bit with the command line flag, and leaving it on will lead to
a huge security issue for one of our customers.

Andrew


> Thanks and regards,
> Lokesh
> 
>>
>> On non-HS devices JTAG is left open anyway so this does not change
>> behavior on those devices. Change the default to off and allow it to be
>> switched back on by command line flag if needed.
>>
>> Signed-off-by: Andrew F. Davis <afd at ti.com>
>> ---
>>  tools/k3_gen_x509_cert.sh | 14 +++++++++++---
>>  1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/k3_gen_x509_cert.sh b/tools/k3_gen_x509_cert.sh
>> index b6d055f6f5..298cec1313 100755
>> --- a/tools/k3_gen_x509_cert.sh
>> +++ b/tools/k3_gen_x509_cert.sh
>> @@ -12,6 +12,7 @@ RAND_KEY=eckey.pem
>>  LOADADDR=0x41c00000
>>  BOOTCORE_OPTS=0
>>  BOOTCORE=16
>> +DEBUG_TYPE=0
>>  
>>  gen_degen_template() {
>>  cat << 'EOF' > degen-template.txt
>> @@ -79,7 +80,7 @@ cat << 'EOF' > x509-template.txt
>>  
>>   [ debug ]
>>   debugUID = FORMAT:HEX,OCT:0000000000000000000000000000000000000000000000000000000000000000
>> - debugType = INTEGER:4
>> + debugType = INTEGER:TEST_DEBUG_TYPE
>>   coreDbgEn = INTEGER:0
>>   coreDbgSecEn = INTEGER:0
>>  EOF
>> @@ -151,8 +152,9 @@ options_help[k]="key_file:file with key inside it. If not provided script genera
>>  options_help[o]="output_file:Name of the final output file. default to $OUTPUT"
>>  options_help[c]="core_id:target core id on which the image would be running. Default to $BOOTCORE"
>>  options_help[l]="loadaddr: Target load address of the binary in hex. Default to $LOADADDR"
>> +options_help[d]="debug_type: Debug type, set to 4 to enable early JTAG. Default to $DEBUG_TYPE"
>>  
>> -while getopts "b:k:o:c:l:h" opt
>> +while getopts "b:k:o:c:l:d:h" opt
>>  do
>>  	case $opt in
>>  	b)
>> @@ -170,6 +172,9 @@ do
>>  	c)
>>  		BOOTCORE=$OPTARG
>>  	;;
>> +	d)
>> +		DEBUG_TYPE=$OPTARG
>> +	;;
>>  	h)
>>  		usage
>>  		exit 0
>> @@ -224,12 +229,15 @@ gen_cert() {
>>  	#echo "	LOADADDR = 0x$ADDR"
>>  	#echo "	IMAGE_SIZE = $BIN_SIZE"
>>  	#echo "	CERT_TYPE = $CERTTYPE"
>> +	#echo "	DEBUG_TYPE = $DEBUG_TYPE"
>>  	sed -e "s/TEST_IMAGE_LENGTH/$BIN_SIZE/"	\
>>  		-e "s/TEST_IMAGE_SHA_VAL/$SHA_VAL/" \
>>  		-e "s/TEST_CERT_TYPE/$CERTTYPE/" \
>>  		-e "s/TEST_BOOT_CORE_OPTS/$BOOTCORE_OPTS/" \
>>  		-e "s/TEST_BOOT_CORE/$BOOTCORE/" \
>> -		-e "s/TEST_BOOT_ADDR/$ADDR/" x509-template.txt > $TEMP_X509
>> +		-e "s/TEST_BOOT_ADDR/$ADDR/" \
>> +		-e "s/TEST_DEBUG_TYPE/$DEBUG_TYPE/" \
>> +		x509-template.txt > $TEMP_X509
>>  	openssl req -new -x509 -key $KEY -nodes -outform DER -out $CERT -config $TEMP_X509 -sha512
>>  }
>>  
>>
Lokesh Vutla April 24, 2020, 5:10 a.m. UTC | #3
On 23/04/20 6:12 PM, Andrew F. Davis wrote:
> On 4/23/20 2:38 AM, Lokesh Vutla wrote:
>>
>>
>> On 22/04/20 10:39 PM, Andrew F. Davis wrote:
>>> When authenticating the initial boot binary the ROM will check a debug
>>> type value in the certificate and based on that open JTAG access to that
>>> core.
>>>
>>> The default is currently full access, on HS this is useful for early
>>> developers, but should not be the default as to prevent end system
>>> integrators from unintentionally leaving this open.
>>
>> Won't JTAG access is useful for early developers. UART as well not available at
>> this point. What we offer out of the box is not a production ready secure
>> system.  I would prefer to have it enabled by default.
>>
> 
> 
> Who are these early developers? Pre-SYSFW on HS is a harsh environment,
> firewalls and other pitfalls limit one to only what is needed to get
> SYSFW loaded. Only a handful of folks will ever touch the source this
> early, and they will be using a GP device for that development, in which
> case debug is enabled, even with this change.
> 
> I'd guess I'm the only developer touching code this early on HS, I say

For how long you are guaranteeing that? This cannot be an assumption for making
default options.

> this as so far I'm the only one who has noticed that there is a ROM
> issue that makes early debug on HS almost unusable.
> 
> Accidentally leaving this open completely defeats the chain of trust, I
> guarantee some production system will do this if we leave JTAG open by
> default, it happens all the time.

Did you audit all other default options?

> 
> So debug doesn't work here anyway, no one uses it and those who do can
> flip this bit with the command line flag, and leaving it on will lead to
> a huge security issue for one of our customers.

Then please split this change into two patches.
1- adding a command line option.
2- changing the default options with reasons clearly mentioned in the commit
description.


Thanks and regards,
Lokesh
diff mbox series

Patch

diff --git a/tools/k3_gen_x509_cert.sh b/tools/k3_gen_x509_cert.sh
index b6d055f6f5..298cec1313 100755
--- a/tools/k3_gen_x509_cert.sh
+++ b/tools/k3_gen_x509_cert.sh
@@ -12,6 +12,7 @@  RAND_KEY=eckey.pem
 LOADADDR=0x41c00000
 BOOTCORE_OPTS=0
 BOOTCORE=16
+DEBUG_TYPE=0
 
 gen_degen_template() {
 cat << 'EOF' > degen-template.txt
@@ -79,7 +80,7 @@  cat << 'EOF' > x509-template.txt
 
  [ debug ]
  debugUID = FORMAT:HEX,OCT:0000000000000000000000000000000000000000000000000000000000000000
- debugType = INTEGER:4
+ debugType = INTEGER:TEST_DEBUG_TYPE
  coreDbgEn = INTEGER:0
  coreDbgSecEn = INTEGER:0
 EOF
@@ -151,8 +152,9 @@  options_help[k]="key_file:file with key inside it. If not provided script genera
 options_help[o]="output_file:Name of the final output file. default to $OUTPUT"
 options_help[c]="core_id:target core id on which the image would be running. Default to $BOOTCORE"
 options_help[l]="loadaddr: Target load address of the binary in hex. Default to $LOADADDR"
+options_help[d]="debug_type: Debug type, set to 4 to enable early JTAG. Default to $DEBUG_TYPE"
 
-while getopts "b:k:o:c:l:h" opt
+while getopts "b:k:o:c:l:d:h" opt
 do
 	case $opt in
 	b)
@@ -170,6 +172,9 @@  do
 	c)
 		BOOTCORE=$OPTARG
 	;;
+	d)
+		DEBUG_TYPE=$OPTARG
+	;;
 	h)
 		usage
 		exit 0
@@ -224,12 +229,15 @@  gen_cert() {
 	#echo "	LOADADDR = 0x$ADDR"
 	#echo "	IMAGE_SIZE = $BIN_SIZE"
 	#echo "	CERT_TYPE = $CERTTYPE"
+	#echo "	DEBUG_TYPE = $DEBUG_TYPE"
 	sed -e "s/TEST_IMAGE_LENGTH/$BIN_SIZE/"	\
 		-e "s/TEST_IMAGE_SHA_VAL/$SHA_VAL/" \
 		-e "s/TEST_CERT_TYPE/$CERTTYPE/" \
 		-e "s/TEST_BOOT_CORE_OPTS/$BOOTCORE_OPTS/" \
 		-e "s/TEST_BOOT_CORE/$BOOTCORE/" \
-		-e "s/TEST_BOOT_ADDR/$ADDR/" x509-template.txt > $TEMP_X509
+		-e "s/TEST_BOOT_ADDR/$ADDR/" \
+		-e "s/TEST_DEBUG_TYPE/$DEBUG_TYPE/" \
+		x509-template.txt > $TEMP_X509
 	openssl req -new -x509 -key $KEY -nodes -outform DER -out $CERT -config $TEMP_X509 -sha512
 }