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

Intel metadata command #399

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft

Conversation

orangecms
Copy link
Collaborator

@orangecms orangecms commented Jan 17, 2023

The command prints some metadata for Intel firmware images.

It includes information on cryptographic material, security configation, and whether a known leaked Boot Guard key has been used.
Note that the intent is not to replicate https://github.com/9elements/converged-security-suite but to offer a simple command to print the metadata. Intepretation would be up to other UIs, such as fiedka.app.

I have no idea what makes sense to add. Help wanted.

@orangecms orangecms force-pushed the intel-manifest branch 2 times, most recently from 84629e5 to 0b5bfca Compare January 17, 2023 15:50
@codecov-commenter
Copy link

Codecov Report

Base: 42.20% // Head: 42.20% // No change to project coverage 👍

Coverage data is based on head (0f1122c) compared to base (8a127a6).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #399   +/-   ##
=======================================
  Coverage   42.20%   42.20%           
=======================================
  Files         125      125           
  Lines        8978     8978           
=======================================
  Hits         3789     3789           
  Misses       4484     4484           
  Partials      705      705           
Impacted Files Coverage Δ
pkg/intel/metadata/bg/bgkey/manifest.go 0.00% <ø> (ø)
pkg/intel/metadata/cbnt/cbntkey/manifest.go 0.00% <ø> (ø)
...ntel/metadata/bg/bgkey/manifest_manifestcodegen.go 52.20% <100.00%> (ø)
.../metadata/cbnt/cbntkey/manifest_manifestcodegen.go 66.16% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -43,6 +43,7 @@ func getRawPkg(
buildCtx := build.Default
buildCtx.GOPATH = strings.Join(goPaths, string(filepath.ListSeparator))
buildCtx.BuildTags = append(buildCtx.BuildTags, `manifestcodegen`)
fmt.Printf("search for `%v` in `%v` (%v) \n\n", path, goPaths, buildCtx.GOPATH)
Copy link
Member

Choose a reason for hiding this comment

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

Looks like leftover garbage :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's from the debug commit - to be dropped once this whole thing is ready, being a draft as of now
I had other stuff to do... but now is an interesting time to get back to BoitGuard things, so here I am back on it again :)

@tlaurion
Copy link

tlaurion commented Sep 12, 2024

Leak article containing magnet link to private keys: https://sizeof.cat/post/leak-intel-private-keys-msi-firmware/
Next steps discussion (inviting your opinion/insights for turning this into something good, maybe...): https://matrix.to/#/!HAonmNlisbeoADmnrN:matrix.org/$x9dssPoXOOR8qgLTkAdWL3Y5bhbGB8npf8WHwnYS8Lg?via=matrix.org&via=kit.edu&via=invisiblethingslab.com

CC: @orangecms

@tlaurion
Copy link

@orangecms : why draft and not merged?

@orangecms
Copy link
Collaborator Author

@orangecms : why draft and not merged?

not sure what to reply - it's been a draft... I'd be happy to team up with someone to finish this up. Doing it all alone is quite exhausting.

@tlaurion
Copy link

tlaurion commented Sep 12, 2024

@orangecms : why draft and not merged?

not sure what to reply - it's been a draft... I'd be happy to team up with someone to finish this up.
What areas need help? Maybe adding label "help needed" and areas where work is needed (todo) in OP with a minimal description for this PR would help?

If you didn't pointed here from a twitter/X url from somwwhere else, I would never landed here.
Not sure I can help here other then trying to get you helped by those who could help :)

Doing it all alone is quite exhausting.

Keep on, I know how lonely doing a project most alone sometimes feels. Slow and steady.
Otherwise, The goal should be to make other want to help, know where to help, what/where they should help in the goal of receiving help more easily for those that can help, isn't it? :)

@tlaurion
Copy link

tlaurion commented Sep 12, 2024

#399 (comment)
_ No description provided. _

That doesn't help receiving needed help, that is for sure, nor the generic title "Intel metadata command" of this PR :)
Please don't burn out. Help will come, hopefully.

@tlaurion
Copy link

tlaurion commented Sep 12, 2024

Leak article containing magnet link to private keys: https://sizeof.cat/post/leak-intel-private-keys-msi-firmware/ Next steps discussion (inviting your opinion/insights for turning this into something good, maybe...): https://matrix.to/#/!HAonmNlisbeoADmnrN:matrix.org/$x9dssPoXOOR8qgLTkAdWL3Y5bhbGB8npf8WHwnYS8Lg?via=matrix.org&via=kit.edu&via=invisiblethingslab.com

CC: @orangecms

Let me know your thoughts here or there. Don't forget to tag @tlaurion(github) or insurgo(matrix). I might otherwise miss it.

@orangecms
Copy link
Collaborator Author

What should I say?

I'm doing lots of stuff. Doing more and more to attract others hasn't worked out. I want to make computers more comprehensible, and that's about it. I'm not burning out or anything, I just put the priorities on what seems interesting. There is a hundred things I could use lots of help with.

If you really want to help out, let's hop on a multi-hour Jitsi session, work through this, and get it into the main branch. :-)

@tlaurion
Copy link

tlaurion commented Sep 13, 2024

@orangecms I restate the PR should have a description and areas where help is needed should be stated to move this PR from draft-> ready for review. Unfortunately this is not my area of expertise and I do not think I have neither the interest/time/needed skills to make this go forward on my own, but doing those little things might get this move forward by others having the interest/time/skills.

@orangecms
Copy link
Collaborator Author

@orangecms I restate the PR should have a description and areas where help is needed should be stated to move this PR from draft-> ready for review. Unfortunately this is not my area of expertise and I do not think I have neither the interest/time/needed skills to make this go forward on my own, but doing those little things might get this move forward by others having the interest/time/skills.

Neither do I have time nor any clue. There is no public documentation, and I just put two and two together.
As I said, I put my priorities as I do, and if someone wants to pair up to get this into a good initial state, I'm open for that.
I've added a description reflecting that.

Copy link
Contributor

@rminnich rminnich left a comment

Choose a reason for hiding this comment

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

This all seems very reasonable, time to make it not a draft?

cmds/intelmeta/main.go Show resolved Hide resolved
@orangecms
Copy link
Collaborator Author

This all seems very reasonable, time to make it not a draft?

yea, let me get back to this over the next days - a similar thing is #351

essentially, Fiano has lots of APIs but not so many CLIs, and those here are simple enough to just parse a ton of stuff; that's how I started with Fiedka before fully integrating everything, since it's easy to just take fixtures and transform them

cmds/bginfo/main.go Outdated Show resolved Hide resolved
flagJSON = flag.Bool("j", false, "Output as JSON")
)

func getLeakedKeys() ([10][]byte, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason to specify 10 here? not just a slice?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's just my lack of Go knowledge. This whole thing is just hacked up.

cmds/intelmeta/main.go Outdated Show resolved Hide resolved
@@ -18,11 +18,11 @@ import (
)

// EntrySACM represents a FIT entry of type "Startup AC Module Entry" (0x02)
type EntrySACM struct{ EntryBase }
Copy link
Contributor

Choose a reason for hiding this comment

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

SACM as a name b/c it matches the many docs. Many you can have a comment to the effect that
SACM
means
Startup Anchor Cove Module

or some such.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will just revert this change, it isn't necessary. It helped me to change it when I worked on this because I cannot remember all those damn acronyms.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OTOH, we also have EntryDiagnosticACM - so ... I think going with EntryStartupACM is actually sensible and I'll adjust the docs, too. I didn't know that Go strongly couples doc semantics with identifiers.

cmds/bginfo/main.go Outdated Show resolved Hide resolved
cmds/intelmeta/main.go Outdated Show resolved Hide resolved
cmds/intelmeta/main.go Outdated Show resolved Hide resolved
cmds/km/main.go Outdated Show resolved Hide resolved
Signed-off-by: Daniel Maslowski <info@orangecms.org>
Signed-off-by: Daniel Maslowski <info@orangecms.org>
Signed-off-by: Daniel Maslowski <info@orangecms.org>
This makes it easier to grasp. Intel docs also spell it out. See:
https://www.intel.com/content/dam/develop/external/us/en/documents/firmware-interface-table-bios-specification-r1p2p1.pdf

Signed-off-by: Daniel Maslowski <info@orangecms.org>
Signed-off-by: Daniel Maslowski <info@orangecms.org>
Signed-off-by: Daniel Maslowski <info@orangecms.org>
Signed-off-by: Daniel Maslowski <info@orangecms.org>
The IndexIOAddress variant only applies for version 0.

Signed-off-by: Daniel Maslowski <info@orangecms.org>
Signed-off-by: Daniel Maslowski <info@orangecms.org>
Signed-off-by: Daniel Maslowski <info@orangecms.org>
Signed-off-by: Daniel Maslowski <info@orangecms.org>
Signed-off-by: Daniel Maslowski <info@orangecms.org>
@orangecms
Copy link
Collaborator Author

Thanks for all the suggestion so far!

I've pushed a handful of fixups. And this whole thing had been left in a state of "I have no idea what all of this is" - really because I had never found the time to look into and make sense of everything. There is a lot of stuff I cannot know due to lack of public docs. That's why I'm still looking for help.

@orangecms orangecms added the help wanted Extra attention is needed label Sep 22, 2024
Signed-off-by: Daniel Maslowski <info@orangecms.org>
@orangecms
Copy link
Collaborator Author

I think it makes sense to make this here part of the analysis command I drafted in #351.
Just take some flags to say "print metadata", "analyze ME config and key material" etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants