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

GCS: Obfuscate chip serial # in AT results #469

Merged
merged 2 commits into from
Jan 18, 2016

Conversation

mluessi
Copy link
Member

@mluessi mluessi commented Jan 18, 2016

Use a cryptographic hash to obfuscate the chip serial number in the shared autotune results.

The rationale for doing this is that the serial number may be used for other things, like warranty claims or registering the controller with a manufacturer, so the serial number should be kept private. While this is not 100% secure (it may be possible to guess a valid serial # due to limited entropy), this is IMO better than not obfuscating the number at all.

Compile but not tested. This should go into the Renatus release, otherwise we will not be able to associate boards with each other in the future.

Review on Reviewable

@tracernz
Copy link
Member

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from the review on Reviewable.io

@tracernz
Copy link
Member

Review status: all files reviewed at latest revision, 1 unresolved discussion.


ground/gcs/src/plugins/config/configautotunewidget.cpp, line 510 [r1] (raw file):
If the UUID is empty we should probably not hash it as hashing it makes it appear non-empty.


Comments from the review on Reviewable.io

@tracernz
Copy link
Member

@mluessi
Copy link
Member Author

mluessi commented Jan 18, 2016

@tracernz added to usage tracker.

see also d-ronin/autotown#2

@mlyle
Copy link
Member

mlyle commented Jan 18, 2016

We should have done this on the autotown side, but since we're coming up to release it seems like this should merge.

@mlyle
Copy link
Member

mlyle commented Jan 18, 2016

:lgtm:


Reviewed 1 of 1 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from the review on Reviewable.io

mlyle added a commit that referenced this pull request Jan 18, 2016
GCS: Obfuscate chip serial # in AT results
@mlyle mlyle merged commit 5d59759 into d-ronin:next Jan 18, 2016
@mluessi
Copy link
Member Author

mluessi commented Jan 18, 2016

@mlyle I think we should do both, send a hash and don't publicly show the hash in autotown. If the hash is public, it may also be abused to submit bogus results that then get associated with a submission by another user (-> bad user experience).

@tracernz
Copy link
Member

We should have done this on the autotown side, but since we're coming up to release it seems like this should merge.

Discussed that with @dustin and he thought it would be too difficult to do it there.

@dustin
Copy link
Member

dustin commented Jan 19, 2016

Not showing data that got sent is possible, but not sending it is better if
we can do that.

On Mon, Jan 18, 2016 at 2:27 PM Michael Corcoran notifications@github.com
wrote:

We should have done this on the autotown side, but since we're coming up
to release it seems like this should merge.

Discussed that with @dustin https://github.com/dustin and he thought it
would be too difficult to do it there.


Reply to this email directly or view it on GitHub
#469 (comment).

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

Successfully merging this pull request may close these issues.

4 participants