Add debug mode to patchless#1
Conversation
for the regular mode.
mpass99
left a comment
There was a problem hiding this comment.
Awesome additions to the vphone and especially the patchless variant so people can consciously choose which patches they would like without directly having to disable major parts of iOS security!
I added a few minor comments, just to leave my thoughts 👾
| | grep -o 'Allow Research Guests status:.*' || true) | ||
| if [[ -n "$_result" ]]; then | ||
| echo "(auto-selected: Macintosh HD) $_result" | ||
| echo "(auto-selected: ${_boot_vol}) $_result" |
There was a problem hiding this comment.
Nice that you are fixing multi boot! Maybe it would be nice to extract this into a separate PR directly at Lakr233/vphone-cli
| JB_MODE=0 | ||
| DEV_MODE=0 | ||
| LESS_MODE=0 | ||
| DEBUG_MODE=0 |
There was a problem hiding this comment.
With "Debug" I'm mainly associating more verbose output of the tool.
Personally, I would understand it faster if you would rename it to the individual added features (like "DEBUGSERVER_PATCH", "SSH_PATCH", etc)
| run_make_sudo "Firmware patch" "$fw_patch_target" "DEBUG=1" | ||
| else | ||
| run_make_sudo "Firmware patch" "$fw_patch_target" | ||
| fi |
There was a problem hiding this comment.
patch_make_runner=run_make
[[ "$LESS_MODE" -ne 0 ]] && patch_make_runner=run_make_sudo
patch_make_args=("Firmware patch" "$fw_patch_target")
if [[ "$DEBUG_MODE" -eq 1 ]]; then
patch_make_args+=("DEBUG=1")
fi
"$patch_make_runner" "${patch_make_args[@]}"| --jb Use jailbreak firmware patching + jailbreak CFW install. | ||
| --dev Use dev firmware patching + dev CFW install. | ||
| --less Use patchless firmware patching + CFW install. | ||
| --debug Add debug tooling to less variant (SRD, devmode, debugserver, SSH). |
There was a problem hiding this comment.
What do you think about making this more flexible - not bound to the "less" variant, but rather as general options to ensure specific patches? Etc. the regular mode might also want to have the SRD patch etc.. We could also split this into multiple flags.. This PR should not contain everything, but if it's not much more than changing the description that might be more maintainable..
| echo "[1/5] Checking brew packages..." | ||
|
|
||
| BREW_PACKAGES=(aria2 gnu-tar openssl@3 ldid-procursus sshpass) | ||
| BREW_PACKAGES=(aria2 gnu-tar openssl@3 ldid-procursus sshpass dropbear) |
There was a problem hiding this comment.
Why do you need the dropbrear brew package (when you install dropbear via the cfw_input)?
| try patchDebugserver(targetMount: targetMount, cfwInput: cfwInputPath) | ||
|
|
||
| print("- Add dropbear (SSH)") | ||
| try addDropbear(targetMount: targetMount, cfwInput: cfwInputPath) |
There was a problem hiding this comment.
Super nice features. Maybe it would be nice to add them with two different flags (best, in two different commits)
| try FileManager.default.createDirectory(at: launchDaemonsPath, withIntermediateDirectories: false) | ||
| try FileManager.default.moveItem(at: launchdOgPath, to: launchdPath) | ||
| try FileManager.default.copyItem(at: vphonedLaunchdPlist, to: launchDaemonsPath.appending(path: vphonedLaunchdPlist.lastPathComponent)) | ||
| // Include SSH daemon plists so inject-daemons registers them in launchd |
There was a problem hiding this comment.
I would not expect this in the "addVphone" function. Maybe you can extract the "inject-daemons" patch into a new function next to the "patchLaunchdCacheLoader" call.
| "-convert", "xml1", "-o", researchEntPath.path, stockEntPath.path | ||
| ]) | ||
|
|
||
| // Merge SRD entitlements on top of stock (matching Apple's SRD tooling approach). |
There was a problem hiding this comment.
Ah, I see, the debugserver patch depends on the SRD patch for the less variant.. Maybe it still makes sense to have the SRD patch controlled with a different flag and just automatically apply the SRD patch for the debugserver patch (+ documentation) 🤷
| for name in ["dropbear.plist", "bash.plist"] { | ||
| let src = daemonPlistsDir.appending(path: name) | ||
| let dst = launchDaemonsDir.appending(path: name) | ||
| if fm.fileExists(atPath: src.path) && !fm.fileExists(atPath: dst.path) { | ||
| try fm.copyItem(at: src, to: dst) | ||
| } | ||
| } |
| exit 1; \ | ||
| fi; \ | ||
| "$(CURDIR)/$(PATCHER_BINARY)" patch-firmware --vm-directory "$(CURDIR)/$(VM_DIR)" --variant less' | ||
| "$(CURDIR)/$(PATCHER_BINARY)" patch-firmware --vm-directory "$(CURDIR)/$(VM_DIR)" --variant less $(if $(filter 1 true yes YES TRUE,$(DEBUG)),--debug,)' |
There was a problem hiding this comment.
When we split the functionality into multiple flags, some of the flags might also be valid for the other variants
3658f13 to
1d67b35
Compare
No description provided.