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

[WIP] Securing EVE Logs #4435

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

shjala
Copy link
Member

@shjala shjala commented Nov 8, 2024


  • secure-logs : add implementation of FssAgg 7151bd4
    Add implementation "Forward-secure sequential aggregate authentication.", this is testable using shjala/adam@478cd64

  • evetpm : update seal/unseal functions signature 4f21372
    Update the Seal/Unseal disk key functions to be generic and accept TPM NV
    indexes as parameters. These functions are not bound to just store and
    retrieve disk keys, and this modification makes it obvious.


TODO :

  • performance cost
  • success test with collect logs and verify
  • success test with collect logs and verify (multiple gzip files)
  • fail test with tampering (content)
  • fail test with tampering (delete from the middle)
  • fail test with tampering (delete from the tail)
  • save num key evolve in the batch metadata
  • each log entry in a batch should have seq number so we can verify it.
  • initial key management (PCR bound, read lock, backed up in the controler)
  • will adding delete detection in the next version breaks logging?
  • EVE API changes
  • secure log switch in config
  • enable logging for ssh
  • enable logging for edgeview
  • documentation

@shjala shjala requested a review from deitch as a code owner November 8, 2024 12:42
@shjala shjala self-assigned this Nov 8, 2024
@shjala shjala marked this pull request as draft November 8, 2024 12:42
@shjala shjala changed the title ]secure-logs : add implementation of FssAgg [WIP] Securing EVE Logs Nov 8, 2024
@shjala shjala force-pushed the fssagg.secure.log branch 2 times, most recently from d022c29 to dceef89 Compare November 8, 2024 12:50
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.

The forward chaining to generate a key for block N+1 from content in block N is secure and simple. But it has an issue if a block is missing (for instance due to running out of space for logs while being disconnected). In such a case it would be nice if an admin could validate that it is ok that 1 (or k) blocks where dropped and still have some forward chaining.

Would we get sufficient forward chaining if we include the block number (N) in how the key is generated? If so, then the verifier can detect that a block N+1 is missing by looking at the sequence numbers, but they calculate the key used to verify block N+2 and following.

@shjala
Copy link
Member Author

shjala commented Nov 11, 2024

The forward chaining to generate a key for block N+1 from content in block N is secure and simple. But it has an issue if a block is missing (for instance due to running out of space for logs while being disconnected). In such a case it would be nice if an admin could validate that it is ok that 1 (or k) blocks where dropped and still have some forward chaining.

I'm not aware of any cryptographic schema that allows that, detecting tampering with the order or breaking the chain in general relies on lost information. If a block is missing, there is nothing to tell us it happened because someone wanted to cover their tracks or disk was full. If we want have the ability to skip logs, then we have to define triggers for certain events that might cause log related failure and then reset the chain from that point on (start a new independently verifiable chain) and then somehow securely tell the verifier this is a new chain, this can quickly lead to a lot of complications.

I'm aware of some cryptographic schemas called seekable key generators which in theory should let us verify each log entry individually, and offers forward secure, but not protection against order or deleting logs from the middle of the chain.

Here the aggregated MAC of log N is based HMAC of content of N-1 past logs. So if just one block is missing, we can't verify the final aggregated MAC. To reduce the issues of missing logs, I'm aiming to add the MAC and chaining related data in the last stage (in doMoveCompressFile) only to the gzipped logs, so we don't end up with missing logs and each gzip file is individually verifiable and any missing or modified logs is an indication of tampering. sure this is not ideal (not yet gzipped logs are not protected) but we can play with the risk factor by gzipping logs more often.
https://github.com/lf-edge/eve/blob/e2a67f53647a986861a5173f0dc6ec948e7a7a7e/pkg/newlog/cmd/newlogd.go#L1102C1-L1134

Would we get sufficient forward chaining if we include the block number (N) in how the key is generated? If so, then the verifier can detect that a block N+1 is missing by looking at the sequence numbers, but they calculate the key used to verify block N+2 and following.

As far as I'm aware, no. We can achieve forward security without relying on chaining logs (which we do in this design), but not the log order or intact log chain, as I mentioned for that latter we need to rely on the information that if lost, is cryptographically detectable. Also attacker can modify and lie about everything, including the block number in the logs.

I will add documents about the whole design in the next commits to make it more clear, and we can discuss this in the EVE Community meeting next week.

@shjala shjala force-pushed the fssagg.secure.log branch 6 times, most recently from b4eb86e to ca30855 Compare November 13, 2024 14:04
@shjala shjala added the enhancement New feature or request label Nov 13, 2024
Copy link

codecov bot commented Nov 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 20.93%. Comparing base (05f2182) to head (3ceee84).
Report is 17 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4435   +/-   ##
=======================================
  Coverage   20.93%   20.93%           
=======================================
  Files          13       13           
  Lines        2895     2895           
=======================================
  Hits          606      606           
  Misses       2163     2163           
  Partials      126      126           

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

@shjala shjala force-pushed the fssagg.secure.log branch 2 times, most recently from b6c0d2b to 05b0985 Compare November 26, 2024 14:23
@github-actions github-actions bot requested a review from rucoder November 27, 2024 09:12
Add implementation "Forward-secure sequential aggregate authentication."

Signed-off-by: Shahriyar Jalayeri <[email protected]>
Update the Seal/Unseal disk key functions to be generic and accept TPM NV
indexes as parameters. These functions are not bound to just store and
retrive disk keys, and this modification makes it obvious.

Signed-off-by: Shahriyar Jalayeri <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants