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

Undesired external configuration editor behaviour #330

Open
vit9696 opened this issue May 14, 2019 · 11 comments

Comments

6 participants
@vit9696
Copy link
Contributor

commented May 14, 2019

There seem to appear tools intending to modify OpenCore configuration files based on the configuration specification[1][2]. I believe it is somewhat unavoidable, but we will have serious issues in the future if these tools do not adhere to the workflow we provided. In particular I mean the following:

  • Both tools entirely ignore specification versioning requirements mentioned in the last paragraph of OpenCore documentation [3];
  • To confirm that the former is the case I had to actually reverse engineer the closed source tool [2], which makes me concerned about the possibility to audit its code. I believe it is a must to demand configuration tools to be open source;
  • Neither of the tools has adequate vaulting support, full documentation link, ACPI table preview, kext sanity checking, which implies that the users of the tools will be more confused and be in a less secure position from the beginning. What is worse I believe their authors do not even understand how this works.
  • Some tools (I was unable to launch the opensource one[1] as it crashes) seem to make me wonder whether they even understand the spec:
    • I saw Apple vendor being used in PlatformInfo, even though it is prohibited by the spec[4]
    • PlatformInfo Generic is used together with the rest of the sections, and all disablable, when the only allowed configuration is Generic vs 3 others[5].
    • The list of dxe drivers contains conflicting and/or incompatible modules.

All in all this is quite displeasing and I believe we need to establish a dialogue with the authors of the tools, and request them follow our rules. With the opensource tool author I believe we can contact through hackintosh-forum.de, and is of less importance, as the software is in unreleased state. With the other tool it is more difficult, especially since the binaries have already been published.

I will leave this issue open for a couple of weeks to let more people try to establish the contact. If this fails I am afraid we will have to seek for other ways to prevent people from inducing damage to their systems. For the time being I added links to Xcode and ProperTree to the documentation to at least try to avoid problems [6].

  1. https://github.com/notiflux/OpenCore-Configurator
  2. https://mackie100projects.altervista.org/opencore-configurator/
  3. https://github.com/acidanthera/OpenCorePkg/blob/0.0.1/Docs/Configuration.tex#L498-L504
  4. https://github.com/acidanthera/OpenCorePkg/blob/0.0.1/Docs/Configuration.tex#L2188-L2193
  5. https://github.com/acidanthera/OpenCorePkg/blob/0.0.1/Docs/Configuration.tex#L1813-L1815
  6. acidanthera/OpenCorePkg@99c70ee

@vit9696 vit9696 added the project:oc label May 14, 2019

@MacFriedIntel

This comment has been minimized.

Copy link

commented May 14, 2019

Hi, I agree with some of the comments and understand the frustration surrounding external configurators, but i do believe Notiflux's version of the editor is on the right track and i've been testing both versions. Mackies CC is buggy and saves data to the config that isn't needed ie Apple vendor . I do however feel we will need an editor as the project matures and more people want to replace clover and amateur users want ease of transition. and not everyone will may struggle to understand how to use xcode to edit their configs. I Personally will continue to test Notiflux's Editor that i prefer. Your advise is noted, Notiflux's editor is supported by the community and I hope the Developer will be better supported to help grow this tool, Thanks

@roddy20

This comment has been minimized.

Copy link

commented May 14, 2019

not everyone will be able to use xcode to compile their configs.

what is the reason "not to be able" to use xcode? it can be installed to any Mac/Hack and it is obvious how to use it for editing
PlistEditPro is not recommended but it is more reliable for editing than any custom made tool

@usr-sse2

This comment has been minimized.

Copy link
Contributor

commented May 14, 2019

@MacFriedIntel

This comment has been minimized.

Copy link

commented May 14, 2019

I edited my comment so you understand what i actually meant. I know xcode is free but its massive for just an editor.

@roddy20

This comment has been minimized.

Copy link

commented May 14, 2019

what about Property List Editor (Not Plist Edit pro)? it is old program from Apple included with additional tools to older Xcode

@notiflux

This comment has been minimized.

Copy link

commented May 14, 2019

@vit9696 first of all thank you for your feedback. I just committed most of the changes you requested here.

I actually planned to have helpful info texts for every option with references to the docs, but as you already said, this is prerelease software, so that may be a little further into the future while I try to get the core features down. For now I just added a little help icon that links to the configuration PDF, but since my App is currently ahead of the latest official OC release, it just points to the configuration at master. I will change that to point to the OC version currently installed on the machine with the option to change that in settings when the latest release and OCC are on the same level, commit wise.

The OC version checks are also pretty primitive at the moment because each of the version arrays only hold one version for now, but it's easily extensible for future versions.

I was checking if an Info.plist is present in a kext before, but I added a bunch more checks now like if the kext has multiple executables, if it has a "Contents" directory, if it is a directory and if it has the ".kext" suffix. Let me know if I should check for anything else.

My PlatformInfo auto fill tools also only fill in the Generic dictionary as per doc, but I made it so all other tables are disabled depending on the Automatic value now.

As for ACPI diffing, I'm working on that, I hope to push an update on that tomorrow.

For vaulting support, I'm afraid I have never had the need to use FV2, so I never looked into it, which means I have no idea how it works. Since it can be risky if you don't know what you're doing, I won't let others test this and possibly ruin their data. However, Pavo has offered to help with this, so I'll be happy to accept a PR from him in the following days/weeks.

Lastly, I would appreciate it if you could either open an issue on my repo about the OCC crashes you are experiencing or upload a crash report to the IM thread so I can try to work it out.

Regards,
notiflux

@vit9696

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

@roddy20 Property List Editor is 32-bit only, so we cannot rely on it at present. ProperTree looks very promising to my eyes as a small alternative.

@notiflux, thanks for the improvements! I will provide more details as time permits, but here are the key points:

  • The crash was caused by some optional value missing (at least from what I remember), and currently it is not reproducible, which I believe is caused by the latest commit. I tested from 10.12 onwards, so far so good.
  • Regarding configuration, it is probably a good idea to sync with dev builds as long as we stabilise, and the current warning makes good sense to me. One thing though, the build is not necessary REL, but it can also be DBG, both are fine. (There also are NPT builds, but we do not distribute those, so it is fine to ignore).
  • With kexts, acpi, and drivers, indeed it is a good idea to run basic checks like file presence, file magic match, or basic Info.plist sanity checking. In addition to that you could check OSBundleLibraries in kext's Info.plist to ensure that the kexts are properly sorted when put to the configuration. Currently OpenCore requires that the dependencies are put prior to their consumers.
  • Vaulting, not to confuse with full disk encrypted called FileVault 2, is a concept invented by ourselves to protect OpenCore installation from accidental modifications. Vaulting hashes and cryptographically signs all the kexts, acpi tables, drivers, config itself. Afterwards OpenCore.efi is supposed to enforce this signature during the boot process. Once we finish secure boot compatibility, OpenCore itself can be signed, and thus the configuration will be unmodifiable as long as firmware implementation is not broken. For this very reason it is important to provide a way to ensure that the user can see what he signs (bundled scripts show a list of hashes, but I guess GUI tools can add previewing in addition to hashes).

Take your time =)

2All I politely ask everyone to refrain from posting opinions and wishes here, we have definite technical issues to focus on.

@Pavo-IM

This comment has been minimized.

Copy link

commented May 16, 2019

@vit9696 quick question on some of the default values in the docs, does OC automatically add a key/value for those if none is found in the config or does it parse the missing key/values from the config as nil?

@vit9696

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2019

@Pavo-IM, actual implementation should not matter. OC configuration spec document explicitly states that unless the value is explicitly specified as optional, not providing it in the config yields to undefined behaviour. Currently the only optional values are some sections in PlatformInfo. All other values are so far required.

We do think of a possibility of making more values optional and the future, and the underlying implementation in OC does support this. However, currently it is not allowed. I.e. currently you can think about "Default values" in the spec as some potential future guideline.

@vit9696

This comment has been minimized.

Copy link
Contributor Author

commented May 18, 2019

Sent an e-mail to mackie100_projects hosted on YaHoo, as found on the donation page with the link to this page.


log

@notiflux

This comment has been minimized.

Copy link

commented May 23, 2019

@vit9696 some more updates. I'm almost done adding advanced file checking, you can see the progress on that here.

The ACPI patch previewer is also almost done, however, I'm struggling to implement Mask and ReplaceMask patching because instead of iterating over the bytes of the table like you are in OC, I'm making use of the replaceSubrange function, which means I can't perform a bitwise OR operation on the input.

I've also started working on a vault manager which should be done in the next few days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.