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

Enabling Secure Boot on ODROID M1 #593

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

m-iwanicki
Copy link
Contributor

No description provided.

@m-iwanicki m-iwanicki marked this pull request as ready for review April 17, 2024 13:37
@m-iwanicki m-iwanicki changed the title WIP: Enabling Secure Boot on ODROID M1 Enabling Secure Boot on ODROID M1 Apr 17, 2024
Copy link
Member

@pietrushnic pietrushnic left a comment

Choose a reason for hiding this comment

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

@m-iwanicki are we considering this blog post ready for review?

@TomaszAIR
Copy link
Contributor

@m-iwanicki please cherry pick c670c52 and 0ab8b78, it should help with SEO fails

@m-iwanicki
Copy link
Contributor Author

@m-iwanicki please cherry pick c670c52 and 0ab8b78, it should help with SEO fails

@TomaszAIR Done, but some links still fail.

Copy link
Contributor

@TomaszAIR TomaszAIR left a comment

Choose a reason for hiding this comment

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

@m-iwanicki I left a review, basically it is nice that everything worked but this blog post needs some edits and definitely a plan for itself. Right now it is hard to read, please let me know if I could help more.

blog/content/post/2024-04-12-odroid-m1-secure-boot.md Outdated Show resolved Hide resolved
blog/content/post/2024-04-12-odroid-m1-secure-boot.md Outdated Show resolved Hide resolved
blog/content/post/2024-04-12-odroid-m1-secure-boot.md Outdated Show resolved Hide resolved
blog/content/post/2024-04-12-odroid-m1-secure-boot.md Outdated Show resolved Hide resolved
Enabling Secure Mode is based on storing the hash of the public key in
the OTP memory and enabling verification of the pre-loader’s signature.
After correctly executing this step, the RK3568B SoC will check if the hash
of the public key that is contained in the booted image matches the hash
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, some diagram here explaining what is happening would be nice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in 9605d08, rewrote it a little and added a diagram.
Even on your slide (18) there is information that it only saves hash to OTP/EFUSE.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TomaszAIR And I'm not sure if I can use/store this diagram from Rockchip. I took it from Rockchip Secure Boot Application Note_v1.2_20160202.pdf that's inside this zip. There are also interesting diagrams in Rockchip Secure Boot Application Note.
Or should I make one myself?

Copy link
Member

Choose a reason for hiding this comment

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

@m-iwanicki Since the linked document is classified as "Publicity," it indicates that it is likely meant to be shared publicly, but this doesn't always automatically grant permission for unrestricted use. What we can do is:

  1. Create our own simple diagram
  2. Add the annotation with the source

Copy link
Contributor

Choose a reason for hiding this comment

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

@m-iwanicki, have you addressed this?

Copy link
Contributor Author

@m-iwanicki m-iwanicki Sep 23, 2024

Choose a reason for hiding this comment

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

@artur-rs Should I embed link inside diagram or is using ![https://link.to.source](path/to/image) enough? Though it's probably not good from accessibility point.

Copy link
Contributor

Choose a reason for hiding this comment

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

@m-iwanicki, you can as well add the link into MD comment below the image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess you are right. Just checked to make sure that comments are also added in html page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

6101ce1
Added as a quote as a comments <!-- --> weren't in html code at least in preview.

Source

blog/content/post/2024-04-12-odroid-m1-secure-boot.md Outdated Show resolved Hide resolved
### U-Boot configuration

To build U-Boot I used `odroid-m1-rk3568_defconfig` configuration. The only
change I needed to make to this configuration was changing
Copy link
Contributor

Choose a reason for hiding this comment

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

@m-iwanicki we now what is happening here? Can we explain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't debug this too deeply so I can't say much more than what's can be glanced from error itself which is it tries to allocate more memory than it's allowed. Maybe it needs to load whole FIT image before verifying/calculating hashes/signatures and with added signature it's too big.

@artur-rs
Copy link
Member

@m-iwanicki @TomaszAIR we can resolve SEO fails on the develop branch

@DaniilKl
Copy link
Contributor

@m-iwanicki, I am currently reviewing this PR, could you meanwhile rebase it on the latest master?

How to write key hash to OTP, changes needed to U-Boot to verify
signature, how to sign loader.

Signed-off-by: Michał Iwanicki <[email protected]>
Add requested changes

Signed-off-by: Michał Iwanicki <[email protected]>
@m-iwanicki
Copy link
Contributor Author

@DaniilKl rebased on current develop

Copy link
Contributor

@DaniilKl DaniilKl left a comment

Choose a reason for hiding this comment

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

@m-iwanicki, I really was trying not to argue about the form of the blog post or the way language is being used. Here are my recommendations for future:

  • Pay attention to perfect tenses, these (especially past or future) are not needed in technical articles, almost everything can be explained using simple tenses. The perfects only complicate the situation.
  • Pay attention to articles and use some tools to check them if you are not sure.
  • Pay attention to readability. Sometimes (when an article is too big) it is not enough to write an intro with a sum up, reader will forget it somewhere in the half of the article and will get lost. First thing you can do here: add an intro and sum up into every chapter, so the reader will be able to connect everything in line while reading.

Actually, have you checked this blog post with e.g. Grammarly?

contained in rkbin repository can't handle loaders generated with new idb
header.

I used newest commits available in those repositories.
Copy link
Contributor

Choose a reason for hiding this comment

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

"newest" is a relative version, could you use absolute versioning?

Copy link
Contributor Author

@m-iwanicki m-iwanicki Sep 23, 2024

Choose a reason for hiding this comment

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

I do, check links. newest (at the time of writing) is explanation why those commits in particular.

return -EINVAL;
```

To generate RSA keys and certificate I decided to use `openssl` command.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
To generate RSA keys and certificate I decided to use `openssl` command.
To generate RSA keys and certificate I decided to use `openssl` tool.

python3-setuptools python3-pyelftools swig libssl-dev device-tree-compiler python2 bc
```

To build Rockchip U-Boot I also needed cross-compiler. By default `make.sh`
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you specify more precisely the make.sh you are referring to?

openssl rsa -in keys/dev.key -pubout -out keys/dev.pubkey
openssl req -batch -new -x509 -key keys/dev.key -out keys/dev.crt
```

Copy link
Contributor

Choose a reason for hiding this comment

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

You can add here a sentence where the keys will be used, so the reader will note this and will be able to track progress more easily. This will improve navigation and readability of your blog post.


In `rkbin/RKBOOT/RK3568MINIALL.ini` I had to update FlashBoot so it points to
`u-boot-spl.bin` file. Later this `.ini` file will be used by `boot_merger` tool
to create `rk356x_spl_loader_v1.21.113.bin` file which will contain U-Boot SPL.
Copy link
Contributor

Choose a reason for hiding this comment

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

You have described the .ini file. But what is the rk356x_spl_loader_v1.21.113.bin file used for? Maybe you could mention that this is the loader, that loads the keys.

## What's next

While I managed enable Secure Boot on Odroid it would be good to more
thoroughly test its security and capability.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
thoroughly test its security and capability.
thoroughly test its security and capabilities.

While I managed enable Secure Boot on Odroid it would be good to more
thoroughly test its security and capability.
Some of the questions that I would like to find answers for are whether there
really isn't way to overwrite key hash and if it's possible to store more than
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
really isn't way to overwrite key hash and if it's possible to store more than
really isn't any way to overwrite the hash stoored in OTP and if it's possible to store more than

one. OTP has 8k bits of memory based on RK3568 datasheet while hashes are only
256 bits in size so theoretically we could store 32 different hashes.

Good next step would be to transfer capability of writing hash to OTP from
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Good next step would be to transfer capability of writing hash to OTP from
Good next step would be to mainstream capability of writing hash to OTP from

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe upstream?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep 😅


Good next step would be to transfer capability of writing hash to OTP from
Rockchip U-Boot to mainline U-Boot which would simplify whole implementation
quite a bit.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
quite a bit.
a lot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not quite a lot?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not perfect picture for a blog post, too complicated and needs additional explanation. I have not understood it fully yet.

@m-iwanicki
Copy link
Contributor Author

@DaniilKl

Actually, have you checked this blog post with e.g. Grammarly?

Tried to but it went poorly. Currently I'm using languagetool

@pietrushnic
Copy link
Member

pietrushnic commented Sep 23, 2024

Actually, have you checked this blog post with e.g. Grammarly?

That is the main point, and Grammarly should provide improvements if used correctly.

Tried to but it went poorly. Currently I'm using languagetool

Please provide more details about the issues.

@m-iwanicki
Copy link
Contributor Author

Please provide more details about the issues.

  • Hundreds of errors
  • have to copy/paste text to their website, fix and then copy back
  • markdown has no highlights so it's harder to read
  • doesn't ignore code blocks (a lot of errors)
  • It's online so I have to keep in mind what I use it with
  • I'm not sure but I think it's a little overeager with the (feels like it wants to add it every third word)

Maybe it can be configured (custom rules/words?) and used e.g. from cli?

@pietrushnic
Copy link
Member

Please provide more details about the issues.

* Hundreds of errors

Why do you assume those are false positives?

* have to copy/paste text to their website, fix and then copy back

Everyone who writes public-facing text should try to make it as high-quality as possible. If that means copying to a tool that improves that, then why not do that? Is copying to another window at the end of writing that much overhead?

* markdown has no highlights so it's harder to read

True. But that is not the goal of that tool, and AFAIK, it deals quite well with markdown syntax not showing markdown [link](URL) as an issue. Why would the text editorial tool need markdown highlights? How does it help in the task?

* doesn't ignore code blocks (a lot of errors)

Code blocks should not be copied for editorial corrections, so that is unnecessary.

* It's online so I have to keep in mind what I use it with

We use the premium version with certain guarantees. I don't know what the precise issue is. We ask to use Grammarly with text we plan to make public, e.g., blog posts.

* I'm not sure but I think it's a little overeager with `the` (feels like it wants to add it every third word)

You can always ignore some suggestions if you know better.

Maybe it can be configured (custom rules/words?) and used e.g. from cli?

It can be configured and trained but has no support for cli. Also, I don't like the lack of integration with Vim/vim. Still, it is much better than wasting the time of a reviewer correcting articles or improving sentence quality if you can delegate that to the machine. We pay for that service, so I would ask you to use it for public text. If you have some better alternatives, we should consider those.

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.

5 participants