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

First stab at Mavlink signing #7530

Closed
wants to merge 1 commit into from
Closed

First stab at Mavlink signing #7530

wants to merge 1 commit into from

Conversation

simonegu
Copy link
Contributor

@simonegu simonegu commented Jul 5, 2017

DO NOT MERGE! First attempt to enable MAVLink v2 signing.

A modification is need in the mavlink module. This one https://github.com/mavlink/pymavlink/pull/1 in order to work.
This adds a signing variable to each MAVLink instances.
Right now the key is hard coded this need to be replaced with loading from a persistent memory.

ToDos:

  • Load Key from persistent memory
  • Save and update time stamp
  • End to end testing

@simonegu simonegu changed the title Mavlink signing DO NOT MERGE: Mavlink signing Jul 5, 2017
@TSC21
Copy link
Member

TSC21 commented Jul 7, 2017

@simonegu when do you expect to have it ready to be reviewed, based on your bandwidth?

@simonegu
Copy link
Contributor Author

simonegu commented Jul 10, 2017

@TSC21 right now I'm validating it if effectively can parse correctly the signed messages.
In addition I'm waiting for the feedback on this https://github.com/mavlink/pymavlink/pull/1 which it is required right now to enable a thread safe reading of the signature.
A time estimate is difficult to make, but the goal is to have a functioning prototype for the end of this week.

@mrpollo
Copy link
Contributor

mrpollo commented Jul 18, 2017

@simonegu do you think you can attend the dev call this coming Wednesday (July 19th)? we think this should be discussed at the dev call with the rest of the team, can you please confirm?

@simonegu
Copy link
Contributor Author

@mrpollo sure.

simonegu pushed a commit to simonegu/qgroundcontrol that referenced this pull request Jul 18, 2017
simonegu pushed a commit to simonegu/qgroundcontrol that referenced this pull request Jul 18, 2017
@simonegu
Copy link
Contributor Author

simonegu commented Jul 18, 2017

Update: Thanks to @DonLakeFlyer for implementing the MAVLink signing on QGC (mavlink/qgroundcontrol#5463) with some tweaks, such as hard coding the signing key, I was able to get the parameter, do a take off and a small mission in SITL.
The question is what would be the best place to store the signing key/timestamp, such that is compatible with all the different hardware configuration? @bkueng @LorenzMeier

@mrpollo mrpollo removed the devcall label Jul 26, 2017
DonLakeFlyer pushed a commit to DonLakeFlyer/qgroundcontrol that referenced this pull request Aug 10, 2017
@TSC21
Copy link
Member

TSC21 commented Aug 15, 2017

@simonegu did you make progress on this? We are interested on adding this on MAVROS (mavlink/mavros#545), so we just wanted to be sync with the implementation on the Firmware and on the GCS.

@LorenzMeier
Copy link
Member

New QGC PR: mavlink/qgroundcontrol#5582

@LorenzMeier LorenzMeier force-pushed the mavlink_signing branch 3 times, most recently from b171630 to c4ce5ce Compare August 18, 2017 14:17
@LorenzMeier LorenzMeier changed the title DO NOT MERGE: Mavlink signing First stab at Mavlink signing Aug 18, 2017
@LorenzMeier
Copy link
Member

This is good to go as a first stab. It is not configurable yet but achieves the goal for anyone wanting to set a fixed key and can serve as proof-of-concept. We need to hash out how to restart both sides and make sure system time is right before deploying this widely.

This adds support for MAVLink 2.0 signing. When enabled only signed messages (or the ones in the unsigned accept list) will be parsed by the system. This allows to harden the link and to ensure that only authorized access is possible.
@TSC21
Copy link
Member

TSC21 commented Aug 29, 2017

@simonegu can you rebase please?

@dagar dagar added this to the Release v1.8.0 milestone Dec 13, 2017
@LorenzMeier
Copy link
Member

@simonegu Could we update / rebase this and cut down the diff to your working version?

@dagar dagar removed this from the Release v1.8.0 milestone May 4, 2018
@TSC21
Copy link
Member

TSC21 commented Nov 25, 2018

We let this die unfortunately. But there's always time to revive it? @simonegu is this still something you are willing to bring in?

@dagar
Copy link
Member

dagar commented Jan 31, 2019

Could you give this a rebase?

@julianoes julianoes removed their assignment Apr 25, 2019
@bkueng bkueng removed this from the Release v1.9.0 milestone Jul 10, 2019
@dk7xe
Copy link
Contributor

dk7xe commented Sep 27, 2019

Since we really need the possibility to sign messages. What is the hampering factor to move forward?

@hamishwillee
Copy link
Contributor

hamishwillee commented Oct 1, 2019

@dk7xe Rebase, then someone to do the remainder of the work, which has essentially not changed since the initial post:

  • Load Key from persistent memory
  • Save and update time stamp
  • End to end testing
  • UI in QGC to enter a plain-text key and set the content of that key into a set of parameters ("chopping up" the bytes in the key into 4-byte chunks).

How it should work is pretty well documented: https://mavlink.io/en/guide/message_signing.html

@simonegu Can you confirm that is a fair summation of status?

@stale
Copy link

stale bot commented Dec 30, 2019

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

@mcsauder
Copy link
Contributor

@simonegu , can you rebase this branch to continue the PR? Thanks!

@stale stale bot removed the stale label Jan 27, 2021
@LorenzMeier LorenzMeier deleted the mavlink_signing branch March 10, 2021 23:12
@dk7xe
Copy link
Contributor

dk7xe commented Mar 11, 2021

@jbeyerstedt fyi

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.