Skip to content

(Draft) Change Edgeview Token Hash size to full 256-bit SHA-256#5541

Draft
naiming-zededa wants to merge 1 commit intolf-edge:masterfrom
naiming-zededa:naiming-edgeview-token-hashsize
Draft

(Draft) Change Edgeview Token Hash size to full 256-bit SHA-256#5541
naiming-zededa wants to merge 1 commit intolf-edge:masterfrom
naiming-zededa:naiming-edgeview-token-hashsize

Conversation

@naiming-zededa
Copy link
Contributor

Description

  • a simple change, but would break the backwords compatibility due to EVE devices may continue to run with the older version images
  • this patch will for now, try the full size first, if we don't get a match, then tried the original hash size of the token. We can remove the short version later on if we are sure all the EVE devices are upgraded to at least this version
  • the dispatch currently supports 'probing' message, this patch utilize this feature to probing to see which size we are going to use.

PR dependencies

How to test and validate this PR

  1. load the EVE image w/ this patch on device, then use the client to verify it is working
  2. use the latest Edgeview container w/ this patch, and connect to the previous version of EVE image device and make sure it also works.

Changelog notes

Change Edgeview Token Hash size to full 256-bit SHA-256

PR Backports

Checklist

  • I've provided a proper description
  • I've added the proper documentation
  • I've tested my PR on amd64 device
  • I've tested my PR on arm64 device
  • I've written the test verification instructions
  • I've set the proper labels to this PR

For backport PRs (remove it if it's not a backport):

  • I've added a reference link to the original PR
  • PR's title follows the template

And the last but not least:

  • I've checked the boxes above, or I've provided a good reason why I didn't
    check them.

@naiming-zededa naiming-zededa requested a review from shjala January 9, 2026 01:30
@naiming-zededa naiming-zededa force-pushed the naiming-edgeview-token-hashsize branch from 7bd4fca to f9ecabc Compare January 9, 2026 01:34
@codecov
Copy link

codecov bot commented Jan 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 28.32%. Comparing base (2281599) to head (35387b1).
⚠️ Report is 226 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5541      +/-   ##
==========================================
+ Coverage   19.52%   28.32%   +8.79%     
==========================================
  Files          19       18       -1     
  Lines        3021     2256     -765     
==========================================
+ Hits          590      639      +49     
+ Misses       2310     1475     -835     
- Partials      121      142      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM.

But I don't understand the text in the description about needing to keep the fallback around until all EVE nodes being updated. Isn't it all the dispatchers which need to be updated? [If so, can we release updated dispatcher containers and push for folks to run the latest to reduce the amount of time we need to keep the fallback in place.]

@naiming-zededa
Copy link
Contributor Author

Code LGTM.

But I don't understand the text in the description about needing to keep the fallback around until all EVE nodes being updated. Isn't it all the dispatchers which need to be updated? [If so, can we release updated dispatcher containers and push for folks to run the latest to reduce the amount of time we need to keep the fallback in place.]

No, the encryption here is on the payload of edgeview message, which is transparent to dispatcher. The idea is if the dispatcher is compromised, the hacker still can not decode the edgeview message.

@eriknordmark
Copy link
Contributor

Code LGTM.
But I don't understand the text in the description about needing to keep the fallback around until all EVE nodes being updated. Isn't it all the dispatchers which need to be updated? [If so, can we release updated dispatcher containers and push for folks to run the latest to reduce the amount of time we need to keep the fallback in place.]

No, the encryption here is on the payload of edgeview message, which is transparent to dispatcher. The idea is if the dispatcher is compromised, the hacker still can not decode the edgeview message.

Does that mean that the all edgeview clients need to be updated before we can remove the short key support from EVE?

@naiming-zededa
Copy link
Contributor Author

Code LGTM.
But I don't understand the text in the description about needing to keep the fallback around until all EVE nodes being updated. Isn't it all the dispatchers which need to be updated? [If so, can we release updated dispatcher containers and push for folks to run the latest to reduce the amount of time we need to keep the fallback in place.]

No, the encryption here is on the payload of edgeview message, which is transparent to dispatcher. The idea is if the dispatcher is compromised, the hacker still can not decode the edgeview message.

Does that mean that the all edgeview clients need to be updated before we can remove the short key support from EVE?

yes. so I'm really wondering if we should do this PR or not.
I had a wrong reply above that i thought it was encryption case. but this one is just a hash to be used as 'index'. but both sides, the client and the eve device side need to send the same hash for 'index'.
so, 32bytes is better, but this backwards compatible thing is difficult to support.

@naiming-zededa naiming-zededa force-pushed the naiming-edgeview-token-hashsize branch from f9ecabc to d1812a2 Compare January 13, 2026 22:25
@github-actions github-actions bot requested a review from eriknordmark January 13, 2026 22:25
- a simple change, but would break the backwords compatibility due to
  EVE devices may continue to run with the older version images
- this patch will for now, try the full size first, if we don't get a
  match, then tried the original hash size of the token. We can remove
  the short version later on if we are sure all the EVE devices are
  upgraded to at least this version
- the dispatch currently supports 'probing' message, this patch utilize
  this feature to probing to see which size we are going to use.

Signed-off-by: naiming-zededa <naiming@zededa.com>
@naiming-zededa naiming-zededa force-pushed the naiming-edgeview-token-hashsize branch from d1812a2 to 35387b1 Compare January 23, 2026 02:14
@eriknordmark
Copy link
Contributor

How about marking this PR as draft until we have controller support for the full sha-256?

@naiming-zededa naiming-zededa changed the title Change Edgeview Token Hash size to full 256-bit SHA-256 (Draft) Change Edgeview Token Hash size to full 256-bit SHA-256 Jan 26, 2026
@eriknordmark eriknordmark marked this pull request as draft January 26, 2026 18:26
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.

3 participants