Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cryptographically signed update support #5213

Merged
merged 27 commits into from
Dec 3, 2018

Conversation

earlephilhower
Copy link
Collaborator

@earlephilhower earlephilhower commented Oct 6, 2018

--edit 11/05-- Remove WIP

Using a pluggable architecture, allow updates delivered via the Update
class to be verified as signed by a certificate. By using plugins, avoid
pulling either axTLS or BearSSL into normal builds.

A signature is appended to a binary image, followed by the size of the
signature as a 32-bit int. The updater takes a verification function
and checks this signature using whatever method it chooses, and if it
fails the update is not applied.

A SHA256 hash class is presently implemented for the signing hash (since
MD5 is a busted algorithm).

A BearSSLPublicKey based verifier is implemented for RSA keys. The
application only needs the Public Key, while to sign you can use
OpenSSL and your private key (which should never leave your control
or be deployed on any endpoints).

This code has been tested by manually signing a Blink.ino.bin file using the BearSSL server example pub/private keypair, but it needs to have a better detailed example and be generalized for different RSA and EC keys.

Inspired by @madpilot's PR #3105 which, unfortunately seems to be abandoned and for some reason had to pull in the entire axTLS source tree and new binary. Maybe the functions weren't exported in axTLS. They are exported in BearSSL, so the changes are minimal here.

Using a pluggable architecture, allow updates delivered via the Update
class to be verified as signed by a certificate.  By using plugins, avoid
pulling either axTLS or BearSSL into normal builds.

A signature is appended to a binary image, followed by the size of the
signature as a 32-bit int.  The updater takes a verification function
and checks this signature using whatever method it chooses, and if it
fails the update is not applied.

A SHA256 hash class is presently implemented for the signing hash (since
MD5 is a busted algorithm).

A BearSSLPublicKey based verifier is implemented for RSA keys.  The
application only needs the Public Key, while to sign you can use
OpenSSL and your private key (which should never leave your control
or be deployed on any endpoints).
@earlephilhower
Copy link
Collaborator Author

Signing example using /tmp/sk as the signing private key. Mostly for my benefit

sha256sum /tmp/file.bin  | cut -f1  -d" " | xxd -r -p > /tmp/sha256.bin
openssl rsautl  -sign -inkey /tmp/sk -in /tmp/sha256.bin  > /tmp/sha256.sign
echo 00010000 | xxd -r -p > /tmp/256.bin
cat //tmp/file.bin /tmp/sha256.sign /tmp/256.bin  > /tmp/esp8266.bin

The corresponding public key to the private one used should also, of course, be in the source code.

@earlephilhower earlephilhower force-pushed the signedupdates branch 2 times, most recently from 5f62397 to ddfa01b Compare October 6, 2018 22:58
Allow EC or RSA keys to be used to verify, as well as any RSA key length
supported by BearSSL (currently 4096 bits).
@devyte
Copy link
Collaborator

devyte commented Oct 7, 2018

Please don't merge this yet, I have several comments.

@earlephilhower
Copy link
Collaborator Author

Sure, @devyte . Apologies in advance for the virtual functions and base classes. Given we've got stuck with a single, globally allocated Updater variable templates aren't an option.

Potential things I think still need checking for are 4-byte length alignment, but I think that's actually guaranteed by the binary format. OTW it's only a readme away from general usability, methinks. Tested three different signed uploads, all worked (and one unsigned and one signed but doctored binary failed with SIGN_ERROR as expected).

@madpilot
Copy link

madpilot commented Oct 8, 2018

So glad you have picked this up! You are correct about importing all of AxTLS, but sounds like BearSSL could be a winner. I'll be following along with interest (I wish I had more bandwidth to help more)

Signed updates provide a better guarantee than unsigned MD5 checking,
and the signature may change the MD5 value anyway.  Because of this,
don't even bother doing MD5 work when a signed update is expected.
@earlephilhower
Copy link
Collaborator Author

There's been some discussion on how to expose this to users. Right now what's implemented is "manually sign," but each has positives and negatives.

I'm inclined to go KISS on the implementation more than the user interface on this because only technically knowledgeable users should really be looking at signed exes. Lose your private key, you can't update without physical presence. Your key gets out in the wild (accidentally committing to a public repo, posting it, etc.), it's all open. And this isn't encryption, so it's not like it is protecting your code, either.

Thoughts?

Manually sign:
* Document how to sign given a pub and priv key
* User changes code to include pubkey explicitly
* User builds a binary, runs a script to sign a .bin into a .bin.signed
+ Easiest to code, test
+ No build process changes
+ Potential for customized signing (i.e. future may have attach cert chain and use a hardcoded TA to verify and get pkey from signed exe, not just hardcode it)
+/- Only "knowledgeable" users can do it
- Manual process
- Code change (1-line + include pubkey)
- Probably Linux/Mac only

JAVA plugin:
* User builds normally
* User changes code to include pubkey explicitly
* Runs java plugin to select private key
* Java signs executable automatically
+ Works Windows or Linux
+ Potential for customized signing
- Requires writing a JAVA plugin, users need to explicitly install it
- Code change (1-line + include pubkey)

Fully Automated:
* User gens pub/priv key, puts in sketch directory
* Build process checks for presence of keys
* If keys exist, build adds a #define to require signed updates to gcc
* Updater.cpp automatically includes pubkey, set always to require signed uploads
* After gcc, build step signs .bin, ready for web upload.
+ Simple to use, best Arduino experience
+/- "Anyone" can do it...even if they really shouldn't
- Private key in sketch directory (accidentally commit to public tree)
- Changes build process
- Removes flexibility
- Requires Windows-compatible signing chain

@d-a-v
Copy link
Collaborator

d-a-v commented Oct 12, 2018

IMHO, KISSest would be 3/ and python.
un-knowledgeable users don't have python.
Keys defined/located with a name/directory, generated files into arduino {build} (/tmp) directory.

@devyte
Copy link
Collaborator

devyte commented Oct 13, 2018

I'm also inclined towards 3. In order for it work, the user still has to have some knowledge.
If found necessary, 2 can come later to add flexibility and mitigate the dangers. In the meantime, we can put together a guide describing what to do and what not to do.
I don't like 1, mostly because I'm pretty sure I'd have trouble using it :p

@earlephilhower
Copy link
Collaborator Author

I'm pretty sure every Linux system will have Python installed, @d-a-v. No idea if it's on the Mac by default.

But, if it might not be installed, I can't use Python, because it will crash all builds if it's not installed. So it's a bash script, then, I think. That's ugly, but I guess doable.

Next problem is how to handle normal case when no key is present or the bash script fails (or is never run like under Windows).

I basically need to overwrite a #include in the core source tree because I can't see how I can change parameters of a later build stage (i.e. I can't add -DSIGUPDT to build.1 stage).
I can't just make a new #include file, because it it may not be generated and there's no way to say "gcc, try and include this file but silently ignore w/o even a warning should it not be present."

Overwriting a core include file on each build is a very ugly hack that I would really not want to do. I'm not even sure if I could do it safely, because you need to reset it to the default case at the start of each compile process.

@liebman
Copy link
Contributor

liebman commented Oct 14, 2018

Mac's have python by default. 10.13 has Python 2.7.

@earlephilhower
Copy link
Collaborator Author

Great, that makes it a little easier. I also forgot the post-bingen step, actually signing the executable. Doing that in Python would be easier and saner than the bash I'm using now.

Unfortunately I'm still stuck on how to conditionally change the compiled code. Any ideas that don't include overwriting include files? If I can run bash scripts in the build, I suppose it would be possilbe (but ugly) to have 2 copies of the compile command line prefixed with something like if -e ./signed.private.key gcc ... -D SIGNED and one that is if -not -e ./signed.private.key ... (or whatever bash ! is)

@d-a-v
Copy link
Collaborator

d-a-v commented Oct 14, 2018

Does test -r somefile && INCLUDEIT='-include somefile'; gcc $INCLUDEIT ... would help ?
For windows10/linux/mac we "officially" have bash (sh in fact). That would leave windows-pre10 (7/8) aside though...
About linux, I think python2.7 is preinstalled on most major distributions by default.

edit
can't get cmd.exe to work:
if exist x (set inc=-include z) & echo a %inc%
if x does not exist, then the echo command is not executed... (& is bash's ;, not &&)

@earlephilhower
Copy link
Collaborator Author

(3) is implemented.

The latest push can now sign the executable automatically in the build process assuming that openssl is installed and there is a "public.key" and "private.key" file in the sketch directory. The manual method of installing a key and signing from the command line is also available.

I'm not too happy about having to include BearSSL calls inside cores/esp8266, but that's the only way I can see of making it work. The example is also wonky: because the IDE does NOT copy any files other than the .ino when you save the example, it won't duplicate the key files. But, if you build directly w/o saving to a custom directory, it does include the key.

@earlephilhower
Copy link
Collaborator Author

Looks like Platform.IO doesn't use platform.txt, so have to hack that as well to build the signing header. Using the existing Python script instead of bash looks like it'll turn out useful as I won't have to duplicate any weird bash in the pio build setup.

@earlephilhower earlephilhower force-pushed the signedupdates branch 2 times, most recently from defa89f to 37ac8a7 Compare October 16, 2018 17:52
@corbolais
Copy link

@earlephilhower, thank you for you efforts. Greatly appreciated.

earlephilhower and others added 9 commits November 28, 2018 19:25
Users normally won't be signing their apps.  In this case a message like
"Not signing binaries" would make them worry over nothing.  Silence it.
Users who do care about signing will still get the Signing binaries and
other output.
Saves ~600 bytes when in debug mode.
Windows can't run the signing script, nor does it normally have OpenSSL
installed.  When trying to build an automatically signed binary, warn
and don't run the python.
@Misiu
Copy link

Misiu commented Jan 28, 2019

@earlephilhower I'd like to ask for more details about PlatformIO and Windows 10.
PlatformIO has advanced scripting maybe this feature could be used on Windows to allow automatic signing?
What tools are needed on Windows (15ca564#diff-9f3d735bebc6c9e39a84eecbbb897a6dR81).
If this isn't supported by default maybe an easy walkthrough could be added (I was thinking about PlatformIO)?
Some of use use WIndows 🙂

@earlephilhower
Copy link
Collaborator Author

@Misiu , your best bet to get this followed up on is to open a new issue or else it's going to be lost on a closed PR.

We need python and native openssl executables. #5635 adds a native, self-contained Python tool which is one part. Win32 OpenSSL executables are still needed and very rare among Arduino users (OpenSSL exes are part of most Linux (and I think Mac) installations by default).

@earlephilhower earlephilhower deleted the signedupdates branch January 29, 2019 21:20
@Misiu
Copy link

Misiu commented Jan 29, 2019

@earlephilhower OpenSSL isn't a problem, it is easily installable on Windows.
I don't mind manual installation on OpenSSL, especially is it done only once.
If installing OpenSSL manually and adding it to the path (so that OpenSSL executable is available from the command line) is the only step then this should be definitively added to docs.
I'll open a new issue to track this.

@earlephilhower
Copy link
Collaborator Author

It's more than just getting the OpenSSL binaries. Python needs to be there and the platform.txt needs to be updated to run under Windows. But please do open up an issue so it can be tracked after the other changes land.

@Misiu
Copy link

Misiu commented Jan 29, 2019

@earlephilhower I've already did - #5694.
If I'm not mistaking Python is installed with PlatformIO, but as with OpenSSL installation of Python is done only once, so no problem with that 🙂

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants