PX4 / PX4-Autopilot

PX4 Autopilot Software
https://px4.io
BSD 3-Clause "New" or "Revised" License
8.4k stars 13.46k forks source link

Secure communication in PX4 (Diploma thesis) #13538

Open rligocki opened 4 years ago

rligocki commented 4 years ago

Hi everyone,

currently, I am working on my diploma thesis, where the goal is to design and implement a secure communication scheme for all devices in mavlink network. Because the current idea needs to modify a few thinks in mavlink and firmware and QGC, I decided to open a discussion about that design.

I would like to share with you current version of my diploma thesis that is written in plaintext (No formating, no images, no corrections so far, only ideas ;)), where is chapter "PX4 security architecture", where are all details described. In that chapter, you can find how devices might be able to verify each other, how session keys might be shared between two devices and how message might be encrypted, signed and then received and verified.

During next few weeks I will be adding missing parts and images, that will make understandment of security implementation easier. If you have any question just ask and I will try to answer ASAP.

I just need to have some feedback from you guys, that knows PX4 platform and might be able to say that this good way and if it will be possible to impelement.

Looking forward to hear from you. ;)

PX4 security analyse.pdf

LorenzMeier commented 4 years ago

Thanks for sharing this!

hamishwillee commented 4 years ago

Thanks for sharing.

For other readers.

  1. Up to page 8 is background on PX4 and definitions.
  2. Page 9/10 is mavlink overview
  3. Page 11/12 is identifying mavlink security issues: signing uses symmetric single key - break once, break everywhere, keys pain to distribute, no encryption, no access control.
  4. Page 12-18 evaluation/comparison of 2 libraries that might be use for encryption, authentication, (pseudo) random number generation: LibHydrogen, Monocypher.
  5. Page 18 listing of supposed PX4 security vulnerabilities.
  6. Page 19 - 21 broadly propose design.

Broadly speaking, if you know PX4 and Mavlink, start reading page 12. If you really know MAVLink then read from page 18.

@rligocki Some minor comments.

This is somewhat confusing in that it refers to "maintaining mavlink 2 compatibility". That is not exactly what you'll have, in the sense that a system that does not understand the new packet format will not be able to handle it. What you'll probably need to do is extend the mavlink 2 protocol with a new incompatibility bit for the new features. Then add your nonce etc as additional data either after or where the signature is now.

I hope that is helpful.

rligocki commented 4 years ago

In a number of places you say "no authentication". MAVLink does support authentication. PX4 also supports authentication in software using symmetric keys. There is a disconnect in that it is not easy to set the keys from QGC.

You are right. That need to be corrected. But I think, that there should be written, that for now there is no easy way to set up symmetric key and enable authentication or is it?

The first section refers to authentication/encryption etc, but when you come to write your thesis you will need to be more clear about the fact that the setups here refer only to hardware approaches (which you only mention when you get to conclusions)

I don't understand what you mean.

You say "Reason to make secret key fixed in firmware is that autopilot boards don’t have Trusted platform modules to store keys safely." That is true for many but probably not all flight controllers. It certainly won't be true in the next year. Any solution will need to support systems that do and do not support such.

You are right. It should be mentioned, that for now there is no supported board with TPM, but in some time there will be some, so there should be an option to store the certificate on SD card, but also to make it simple to add TPM support.

I learned a new word "suppositious" :-)

I learned it too. Google translate told me this secret :D

Point to point messages includes the target address in the payload, which will be encrypted. So encryption breaks routing. It might be worth considering adding the target addresses as additional post-signature fields that are identified by an incompatibility bit (or could be "duplicated" and then marked as an incompatibility bit).

To make a new security system compatible with mavlink 2.0, there should be no need for the target address. For now, I found a solution, that if broadcasted and encrypted message is received, the signature should be validated using all recent session keys. After the right key is founded, then the message might be decrypted. If there is no key found in memory, then the packet is discarded. Sign validation should take less time, than decryption. This solution will be time-consuming. Should be ok for at least 5 devices on one network.

Encryption and authentication are an overhead, so you would have to be able to turn them off.

Encryption using MonoCypher library shouldn't add any overhead. Packet format should stay the same. As nonce, the message sequence number might be used. It is not a perfect solution, but to make it compatible, there is no other way or for now, I didn't find a better one.

If you use the new system you would not need the old signing, so you would want to be able to turn that off.

Old signing system will be replaced. The new singing system will use a session key instead of a preshared key. To make it possible to found out if the message is encrypted or which signing system is used, new incompatibility flags should be added (MAVLINK_IFLAG_SIGNED_V2 [0x02], MAVLIK_IFLAG_ENCRYPTED [0x04]).

@hamishwillee I am grateful for your review, and that you spend some time to read blueprint of my thesis. It looks like, there are some parts, that are confusing. For example, after reading that thesis it should be understandable, that for example, packet format doesn't need to be changed. Looks like there is huge amount of mistakes, that needs to be corrected.

hamishwillee commented 4 years ago

In a number of places you say "no authentication". MAVLink does support authentication. PX4 also supports authentication in software using symmetric keys. There is a disconnect in that it is not easy to set the keys from QGC.

You are right. That need to be corrected. But I think, that there should be written, that for now there is no easy way to set up symmetric key and enable authentication or is it?

Probably you need to differentiate what MAVLink supports vs what PX4 supports.

So you you'd clearly state up front that the MAVLink supports authentication in software using a symmetric keys, but that end to end support needs to be implemented in flight stacks and ground stations to make this practically useful. Also symmetric keys have the other issues you point out further down.

Further you'd explain that encryption isn't supported in MAVLink (at all) so systems that require encrypted transmission generally send encrypted packets over an encrypted channel.

That is implied, but not stated.

The first section refers to authentication/encryption etc, but when you come to write your thesis you will need to be more clear about the fact that the setups here refer only to hardware approaches (which you only mention when you get to conclusions)

I don't understand what you mean.

As above, it isn't clear from the description "up front) that the options where you're using Wifi to get "security" are running MAVLink over a transport that has security, not something inherent in MAVLink.

You say "Reason to make secret key fixed in firmware is that autopilot boards don’t have Trusted platform modules to store keys safely." That is true for many but probably not all flight controllers. It certainly won't be true in the next year. Any solution will need to support systems that do and do not support such.

You are right. It should be mentioned, that for now there is no supported board with TPM, but in some time there will be some, so there should be an option to store the certificate on SD card, but also to make it simple to add TPM support.

Yes. So in other words hooks that allow the implementer to provide the key storage location, and docs on the best way to do it.

Point to point messages includes the target address in the payload, which will be encrypted. So encryption breaks routing. It might be worth considering adding the target addresses as additional post-signature fields that are identified by an incompatibility bit (or could be "duplicated" and then marked as an incompatibility bit).

To make a new security system compatible with mavlink 2.0, there should be no need for the target address. For now, I found a solution, that if broadcasted and encrypted message is received, the signature should be validated using all recent session keys. After the right key is founded, then the message might be decrypted. If there is no key found in memory, then the packet is discarded. Sign validation should take less time, than decryption. This solution will be time-consuming. Should be ok for at least 5 devices on one network.

What you're saying then is that compatibility with MAVLink 2 packet format is more important than the system actually working (with respect to routing). That might be the case, but definitely "for discussion".

Firstly, you can't just discard the message if you can't decrypt it. Otherwise you'll break any routing systems that sit between the sender and the destination unless they have the key. Note, it is OK for them to not be able to read the message to be able to forward it to something that can.

Further, what this assumption means is that when messages are encrypted they must be treated as broadcast, because you can't get a handle on the target address. That won't really matter for broadcast messages which should be forwarded to all interfaces. However it does mean that there will be more traffic of point-to-point messages, because to get them to their destination you'll have to forward them to all destinations (while the routing rules state that you only route them if you know that the destination is on the alternative route). I suspect unintended consequences.

Further, what should a system on the network do if it gets an encrypted message and it hasn't been updated to support encryption? The package will potentially look valid but the payload will be screwed. So at this point you will need either an incompatibility or compatibility bit set on every packet so that systems know how to handle them. I think it will have to be an incompatibility bit because a system that doesn't understand the format will otherwise attempt to decode. Also because you'll probably want to dump the existing signature behaviour.

Encryption and authentication are an overhead, so you would have to be able to turn them off.

Encryption using MonoCypher library shouldn't add any overhead. Packet format should stay the same. As nonce, the message sequence number might be used. It is not a perfect solution, but to make it compatible, there is no other way or for now, I didn't find a better one.

Like I said, I don't think packet-format compatibility is the end all and be all. What you need is interoperability. Either way you need the system to work sensibly with libraries that don't support encryption.

If you use the new system you would not need the old signing, so you would want to be able to turn that off.

Old signing system will be replaced. The new singing system will use a session key instead of a preshared key. To make it possible to found out if the message is encrypted or which signing system is used, new incompatibility flags should be added (MAVLINK_IFLAG_SIGNED_V2 [0x02], MAVLIK_IFLAG_ENCRYPTED [0x04]).

Ah. OK at this point you can do whatever you like to the packet format because parsers know that an encrypted packet can't be handled by a parser that doesn't understand the message. You could for example include the target and destination in the packet format explicitly.

I am grateful for your review, and that you spend some time to read blueprint of my thesis. It looks like, there are some parts, that are confusing. For example, after reading that thesis it should be understandable, that for example, packet format doesn't need to be changed. Looks like there is huge amount of mistakes, that needs to be corrected.

You're welcome. There are some mistakes, but you're in early days. The bit that is interesting to me personally is the MAVLink integration.

rligocki commented 4 years ago

A few days ago I analyzed TRNG (True number generators) on Pixhawk 1 and in Qt framework. Results look good. Generated keys will have enough entropy. It means, that they will be secure. During next week I will publish the next version of my diploma thesis.

msadowski commented 4 years ago

@rligocki Many thanks for sharing your thesis! Are you planning some follow up in the coming days?

rligocki commented 4 years ago

Still there is a lot of things that I would like to improve. Also, there is a huge amount of mistakes in my English. The latest version is here:

Diploma_Thesis-9-2.pdf

For now, I have quite a lot of work at my company. At the end of January, it should be better so then I will start to work on this implementation.

rligocki commented 4 years ago

Here is example implementation, that was used for performance measurement. Using this implementation it is possible to understand how this whole implementation should work.

https://github.com/rligocki/px4_libhydrogen_example/blob/b42faf885f858b6d955467f1295ef24e8e98557c/x86_example/main.c

msadowski commented 4 years ago

@rligocki many thanks for sharing the thesis! It looks very solid and informative!

rligocki commented 4 years ago

During this month I would like to create a pull request for pymavlink, mavlink, PX4-firmware and QGroundControl repositories. This pull request should allow testing of end-to-end encryption and key exchange system using the public key infrastructure. Also, I am thinking about the idea of secure bootloader, that would allow installing only signed firmware. For now, it is impossible to prevent the customer from rewriting firmware in commercial UAV.

hamishwillee commented 4 years ago

I would start with updates to repo in ArduPilot/pymavlink. That's a gate for MAVLink changes that all the other systems are dependent on.

rligocki commented 4 years ago

Ok, but in pymavlink there are only a few changes. I added monocypher library, added and modified few functions in mavlink_helper. Also added new structure for certificate storage in mavlink_types. Do you think, that I should create pull request right now or should I wait until I will create prototype that works?

hamishwillee commented 4 years ago

There is no point submitting a PR until you have working code. I would submit an issue where you explain what you expect your code to do (in that repo) so that the engineers can assess any risks.

rligocki commented 4 years ago

First pull request for mavlink repository (https://github.com/mavlink/mavlink/pull/1333). I would like to add CERTIFICATE message into mavlink repository. These is my first pull request into public repository. Hurrey !!!

bys1123 commented 4 years ago

Pixhawk 2.1 now changed name to CubePilot Black

rligocki commented 4 years ago

Ok ... I will change it in my thesis.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.