mkrofs: encrypt files with AES and verify image with AES-CMAC #60
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @adamgreloch, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the security of Read-Only File System (ROFS) images generated by the mkrofs tool. It introduces capabilities to encrypt the contents of files within the image using AES and to verify the overall image integrity using AES-CMAC. This ensures both the confidentiality of the data and protection against unauthorized modifications.
Highlights
- New Dependency and Cryptographic Library Integration: The
mkrofstool now integrates thelibtinyaeslibrary, enabling the use of AES for encryption and CMAC for integrity verification. - Image Integrity Verification with AES-CMAC: The ROFS image header has been extended to include a field for storing the AES-CMAC, which is calculated over the image data to ensure its integrity.
- File-level Encryption using AES CTR Mode: Individual files within the ROFS image are now encrypted using AES in CTR (Counter) mode, enhancing the confidentiality of the stored data. An Initialization Vector (IV) is constructed based on file metadata.
- Command-line Key Specification: A new command-line option,
-k, has been added tomkrofs, allowing users to specify the AES key as a hexadecimal string for encryption operations.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces AES encryption for files and AES-CMAC for image verification in mkrofs. The changes are generally well-structured, adding a new dependency, command-line options for an encryption key, and the core cryptographic logic. However, I've found a few significant issues. There's a critical bug in how the AES-CMAC is formatted for logging, which would print incorrect data. There's also an issue with error handling during file reads that could lead to misleading error messages, and another issue where file encryption is performed unconditionally, even without a user-provided key, which is a security risk. I've provided detailed comments and suggestions to address these points.
adamgreloch
force-pushed
the
adamgreloch/NIL-693
branch
from
57f21d1 to
58e8f86
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces AES encryption for file contents and AES-CMAC for image verification in mkrofs. The changes are well-structured, adding a new dependency on libtinyaes and integrating encryption and MAC generation into the image creation process. I've found a bug in the logging of the AES-CMAC value and have also provided some suggestions to improve code consistency and readability, particularly around IV generation and command-line argument parsing.
adamgreloch
force-pushed
the
adamgreloch/NIL-693
branch
from
58e8f86 to
9d02a3f
Compare
It was moved to rofs_layout.h (p-r-filesystems) to unify the definition between mkrofs and rofs server implementation JIRA: NIL-693
adamgreloch
force-pushed
the
adamgreloch/NIL-693
branch
from
9d02a3f to
4a44dc1
Compare
JIRA: NIL-693
Description
Motivation and Context
Types of changes
How Has This Been Tested?
Checklist:
Special treatment
!rofs: add AES decryption and authentication via AES-CMAC phoenix-rtos-filesystems#155
host-tools: build libtinyaes and install rofs headers phoenix-rtos-build#242