Skip to content

Conversation

@ansasaki
Copy link
Contributor

@ansasaki ansasaki commented May 24, 2022

Rewind the file before reading its content.

Fixes: #383

Signed-off-by: Anderson Toshiyuki Sasaki ansasaki@redhat.com

@THS-on
Copy link
Member

THS-on commented May 24, 2022

The measured boot log should never change after boot, so we just could keep it in memory instead of reading it again.

@maugustosilva
Copy link

I can confirm this single line change indeed fixes the problem. LGTM

@ansasaki
Copy link
Contributor Author

The measured boot log should never change after boot, so we just could keep it in memory instead of reading it again.

This is probably the best solution. I'll make the changes towards this.

@lukehinds
Copy link
Member

lukehinds commented May 25, 2022

Just a nit, can the unwrap be error handled instead (I figure rewind is an Option or Result). Perhaps even .except with a meaningful error message.

EDIT: just noticed its a draft, so the above might already be your intent.

@ansasaki ansasaki force-pushed the rewind_mb branch 2 times, most recently from b381fdd to aa02df4 Compare May 25, 2022 13:24
@ansasaki ansasaki changed the title Draft: quotes_handler: Rewind measured boot log file Draft: main, quotes_handler: Read measured boot file at startup May 25, 2022
@ansasaki
Copy link
Contributor Author

@maugustosilva Could you please check if this fixes the issue?

Rewind the file before reading its content.

Fixes: keylime#383

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
@ansasaki ansasaki changed the title Draft: main, quotes_handler: Read measured boot file at startup quotes_handler: Rewind measured boot log file May 25, 2022
@ansasaki
Copy link
Contributor Author

After some discussion, it was decided to revert the change to simply add a rewind. There is no clear benefit on holding several KB of data on memory all the time to avoid rewinding and re-reading the file which is on a memory-based file system.

@maugustosilva
Copy link

Tested both solutions, and both pass all my tests.

@ansasaki
Copy link
Contributor Author

ansasaki commented Jun 1, 2022

@lkatalin @lukehinds @ashcrow Could you review/approve this?

@lkatalin lkatalin merged commit aed51c7 into keylime:master Jun 3, 2022
@ansasaki ansasaki deleted the rewind_mb branch September 27, 2023 08:32
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.

Measured boot policy fails with agent starting with commit 569d1e1

6 participants