Fix encoding error and error suppression

[Androbin]

The try-except swallowed the following error: ord() expected string of length 1, but int found
Poor error handling resulted in a syntax error and subsequent compilation failure in Updater.cpp

[earlephilhower]

LGTM. I think this error will only pop("r" vs "rb") up under Windows but doesn't look like it'll affect UNIX/MacOS.

[Androbin]

I just ran into this on Ubuntu.

[oddstr13]

This is a Python 2 vs. 3 thing.
Iterating over a binary string in python3 yields integers, but in python2 it yields one char length (byte)strings.

The proposed fix is acceptable if, and only if the file in question is a (utf8 encoded) text file (containing only ascii).

Python 3.7.2

>>> x = b"foobar"
>>> for i in x: print(i)
...
102
111
111
98
97
114

Python 2.7.13

>>> x = b'foobar'
>>> for i in x: print(i)
...
f
o
o
b
a
r

Potential ways to deal with this

Use bytearray (New in Python 2.6);

for i in bytearray(pub):
    val += "0x%02x, \n" % i

Check for Python version;

if sys.version_info[0] < 3:
    for i in pub:
        val += "0x%02x, \n" % ord(i)
else:
    for i in pub:
        val += "0x%02x, \n" % i

I do not recommend opening the file in text mode. We are constructing a char[] after all, and ord() on a unicode character can return numbers larger than 255.

[oddstr13]

I do not recommend opening the file in text mode.
We are constructing a char[] after all, and ord() on a unicode character can return numbers larger than 255.

[Androbin]

As far as I can tell, this is all ASCII:

-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAxzYuc22QSst/dS7geYYK
5l5kLxU0tayNdixkEQ17ix+CUcUbKIsnyftZxaCYT46rQtXgCaYRdJcbB3hmyrOa
vkhTpX79xJZnQmfuamMbZBqitvscxW9zRR9tBUL6vdi/0rpoUwPMEh8+Bw7CgYR0
FK0DhWYBNDfe9HKcyZEv3max8Cdq18htxjEsdYO0iwzhtKRXomBWTdhD5ykd/fAC
VTr4+KEY+IeLvubHVmLUhbE5NgWXxrRpGasDqzKhCTmsa2Ysf712rl57SlH0Wz/M
r3F7aM9YpErzeYLrl0GhQr9BVJxOvXcVd4kmY+XkiCcrkyS1cnghnllh+LCwQu1s
YwIDAQAB
-----END PUBLIC KEY-----

[earlephilhower]

Keys can be in the PEM ASCII format w/header (basically base64 encoded) or binary (DER IIRC).

We need to be able to simply convert a stream of bytes (Unicode is not an option w/any public key format) into a stream of 0x##s, without any conversion. I think the "rb" will handle it, but let's make sure we understand it. BearSSL guts can take either as long as the *.key file is faithfully translated byte-by-byte to flash somehow.

Do you have a failing case for master you can attach in a post, @Androbin ? I use Ubuntu, too (but developed the code at 16.04 and only until recently moved to 18.04 so a Python2->Python 3 issue might not have been hit by me yet).

[oddstr13]

If the file can maybe possibly contain binary data, we should not be using text mode file handles, as in addition to encoding it may also do newline translation. \r\n -> \n

[earlephilhower]

Agreed, @oddstr13, it should be opened rb not r mode. On Unix they're identical since there's no hidden \r\n->\r like under Win32/Win64 when a file is opened in text mode, so I don't see how it's changing anything (but it may be Python 2 vs. Python 3 related and not \r\n=>\n).

You seem to be arguing against the change, though, which I don't understand. Can you clarify why you think the solution @Androbin proposed is wrong? I don't see the harm in what's proposed...

[oddstr13]

@earlephilhower the proposed change, as it currently stands in the pr, changes key file open mode from rb (binary read) to r (text read).
This will lead to issues if the input file contains characters outside printable ascii.

The change to the exception to be more specific should be just fine.

[earlephilhower]

@oddstr13 is absolutely correct. I looked at the patch and missed up before and after. The file must be opened in binary mode. His suggestions as to the Python workarounds sound right, too.

[oddstr13]

This changes nothing in regards to my objection of this patch.
If anything, this makes it worse, as this replaces any sequence of r'[\r\t ]*\n' with '\n'.

As I've understood this, the key may be in either binary or ascii armor format.

Suggested change

	pub = b"\n".join(map(lambda x: x.rstrip(), f.readlines()))
	pub = f.read()

[oddstr13]

Won't work with Python 2.5 or older, but that is an ancient version I hardly ever see.
Won't be a problem on any system that is even somewhat remotely up to date.

I haven't tested the patch myself, but this looks good.

[earlephilhower]

LGTM now. Did a test generating a header from a random binary file and a text file using python 2.7 and python 3.6 under Ubuntu and got an exact match on the output header.