Hey there, I already finished it yesterday and tested everything.
Basically, --git triggers git stash, otherwise exit. That seems like a good compromise. Documented it too.
I’ll take a look again but it’s pretty much done. You can write your version and we’ll compare later.
I discussed it with Patrick some, and I think we actually would like to not include git update code in derivative-maker-docker at all. The reason for this is:
derivative-maker-docker-run if they haven’t verified the repo since the last time they or a script pulled the repo, but that’s a lot to remember, and any mistake could be fatal.That being said, I do like the focus on low user interaction, since using git directly for updates and tag checkouts is a pain if you aren’t already very familiar with Git. Therefore, I’m thinking it may be better to break out all of the git update functionality into its own script (maybe call it derivative-update? not sure). Then the user will be able to verify the repo once, then use derivative-update to update it securely from there on out. It will require very careful coding so that it can do things like revert if code fails to verify properly, but that would reduce user error and improve security rather than worsen it.
So this is what I made after review. Most of the TODOs are solved, and the git update code is mostly preserved but disabled for now: GitHub - ArrayBolt3/derivative-maker at arraybolt3/docker
Note, due to a grml-deboostrap limitation (caused by a Docker quirk which triggered an lsblk malfunction), VM images created using derivative-maker-docker will only work when booted in EFI mode: amd64 images build under a Debian 12 Docker image end up with no BIOS bootloader due to `lsblk` malfunctioning · Issue #348 · grml/grml-debootstrap · GitHub I have this in my task list to fix. ISO images should be fine, I’m able to boot a Kicksecure-Xfce ISO in both BIOS and UEFI mode.
Ok cool, I took a quick glance.
So git-gpg-verify key import and fingerprint is basically what I just wrote, with seemingly arbitrary changes. run_git() with "${UPDATE_REPO}" is fine, but the code is the same. I’m sure it can be improved, but my version fixes state alteration, in terms of git fetch, checkout etc. Otherwise, as you stated, yours is not functional.
I discussed it with Patrick some, and I think we actually would like to not include git update code in derivative-maker-docker at all
## TODO: Disabled for now, code should be repurposed as possible for a
## dedicated updater script.
Maybe you should’ve included that, so there’s a rough outline.
You also made some changes in derivative-maker-docker-run that don’t seem very elegant. REBUILD_CONTAINER="1" can be done cleaner. Individual mount options for cacher and binary is fine too, but summing that up in one option is probably cleaner (wrapper usage). And no error handling.
Yes, Patrick has indicated that and I’ve already written something for initial end-user verification. I didn’t notice anything related here, but it’s basically just a short instruction set in the readme that guides the user through exporting the key to the key_mount volume and manually verifying. Haven’t included it yet, because of recent key related stuff, but I can add that now.
You can’t force the user to manually verify.
Thus, there must be a way to remain functional without it, making automatic verification unavoidable. I don’t see the dependence on the user’s awareness in regards to the effectiveness of that process.
Offering instructions to manually verify should be recommended. I don’t think you can do much, beyond that.
How can you guarantee the integrity of that script in the event of malicious activity? Same problem.
Manual verification is the only way.
Where this code is executed, really just comes down to preference. I actually like this idea, though. (already proposed it)
Patrick suggested usage of git-gpg-verify (among others), but maybe he’ll prefer this now.
I’ll take a closer look later, but if you’ve tested everything and Patrick approves then sure.
If you want to collaborate on this, I suggest that we work together on the same fork, so that sudden, unforeseen changes can be avoided.
This is getting more serious now, so I’ll do it locally, as well.
Maybe you should’ve included that, so there’s a rough outline.
I’m not quite sure I catch your meaning. I included a mention of it in both source code and in the forums here. If you meant I should have started working on an updater script, I was done with work on derivative-maker for the day and that was as far as I got.
I should also clarify, I seem to have misunderstood things around the --git option in discussion with Patrick. Having the update code part of the build code is fine, as long as we’re very sure it works as expected. I’m a bit nervous about the idea of updating the code and then immediately building, because if a bug in the updater resulted in unverified code sneaking by, that code would be immediately executed during a build before the user has a chance to react. Splitting the updater from the builder means that the worst the updater can do is pull in malicious code and then stop, which gives the user a chance to notice that something’s gone wrong and take corrective actions. It’s a form of “security by correctness” vs. “security by isolation”, and I personally prefer the isolation-based approach.
REBUILD_CONTAINER="1"can be done cleaner.
Great. I’ll take a look at your implementation (or if you haven’t implemented it and have suggestions of how to do it more elegantly, that’s great too).
Individual mount options for cacher and binary is fine too, but summing that up in one option is probably cleaner (wrapper usage).
I don’t really agree that summing it up in one option is cleaner. I don’t expect a user will usually want to change the binary, cache, and key directories all at the same time. Even if they do, having to explicitly name each one should help reduce mistakes; it would be annoying to accidentally put the cache directory on /var/tiny_disk_for_key_storage while putting the key directory on /var/huge_disk_for_cache, for instance.
That sounds like a good idea. I’m sorry for not having included your changes earlier, I had done a lot of work on derivative-maker before you posted Docker Container that builds Whonix Images - #142 by e-coin, and wanted to finish that before pulling in any new code so I didn’t get confused. By the time I was done with the job, I was also done working on derivative-maker for the day and didn’t manage to look at your code yet, so I posted what I had and then figured I’d come back and take another look in a day or two when I was working on derivative-maker again. I’m not trying to override anything you’re doing, I’m very sorry if it came across that way.
No that’s fine. I think you’re just supposed to confirm some of the new git related stuff.
If you have the time, you can test git-gpg-verify individually, but it’s basically these two.
1.)
Fetching only tags by specifying ref.
Found this here: https://stackoverflow.com/questions/1204190/does-git-fetch-tags-include-git-fetch
Works as expected.
git fetch https://github.com/derivative-maker/derivative-maker --depth=1 -- 'refs/tags/*:refs/tags/*'
2.)
Using git switch instead of checkout.
Basically, achieves exactly the same result.
switch is just newer, simpler and has no restore capability.
Apparently, it’s good practice to use switch after stash.
Tested and works fine.
git switch --detach -- "${TAG}"
Even assuming that the average user would be willing to repeatedly execute an isolated updater script, which itself may very well be compromised given your example, how would such a person be able to identify malicious code without careful review? Not reasonable.
Does the key volume really need a separate option? It’s tiny and doesn’t really have to be flexible in terms of mount location. That’s why I kept it hidden.
Realistically, it’s just the binary and cacher volumes that are likely to be mounted elsewhere. Let’s discuss these types of minor things on github.
No problem, thank you for your understanding. I’d also like to apologize for getting a bit snappy.
Please add your docker related code to my fork from now on, so that I can have a look and discuss it with you. 
Even assuming that the average user would be willing to repeatedly execute an isolated updater script, which itself may very well be compromised given your example, how would such a person be able to identify malicious code without careful review? Not reasonable.
We need to take a step back and look at the intended use case scenario I think.
There are some users who are going to really not understand how any of this verification business is supposed to work, won’t know good code from bad code, won’t know trusted code from untrusted code, etc. That’s definitely a group of users we want to keep in mind. The opposite extreme exists as well though - there are some users who are going to fully understand how verification works, when something is trusted or untrusted, and will be able to recognize malicious code. Users like you and me. If we lump all users into the group of people who don’t hardly know what they’re doing, and call some things “good enough” because they’re at least better than running random code from the Internet blindly, we’re going to be doing a disservice to the experienced users. A significant portion of the experienced users are likely going to be experienced because they’re high-profile targets, so their ability to use their computer safely may depend on us.
You are correct that the added safety of a split out updater isn’t reasonable for the “average user”. But we aren’t catering to only just the average user. We’re also trying to provide tools for the experienced, tech-savvy activist who has a nation-state coming after them. For that person, an isolated updater script would be worth it, and I don’t think the isolated updater script would cause much of an issue for the average user. We could even take both approaches by having an isolated updater that derivative-maker-docker can call before building things. I’d be happy with that.
Does the key volume really need a separate option? It’s tiny and doesn’t really have to be flexible in terms of mount location. That’s why I kept it hidden.
I don’t think it can hurt for it to have a custom location, and size isn’t the only factor at play here. Maybe the user has both encrypted and unencrypted partitions (like I do) and prefers to keep keys on an encrypted partition for some reason.
Let’s discuss these types of minor things on github.
Once I’m able to do a proper code review, sure. I’m here though because I’m working on other things and am not ready to do a proper code review yet. The conversation is active here, so this is the easiest place to have it.
Speaking of code review, I haven’t let this fall off the table, I just have a decent stack of other tasks in my task list that are currently marked as higher priorities. I will get to this soon, there’s just other fires I need to put out (like the lock screen button being broken in sysmaint mode in some setups).
Did I fail to accept the invitation to the repository, or did you end up removing me as a collaborator? I’m unable to push a new branch containing changes I wanted to suggest. Most of it is integrating the work I did previously with the work you did.
┌─╴aaron@kf-m2g5:/srv/data/vmshare/kicksecure/3rdparty/tabletseeker/derivative-maker/docker
└─╴$ git push --set-upstream origin arraybolt3/work
ERROR: Permission to tabletseeker/derivative-maker.git denied to ArrayBolt3.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
Just saw you sent an invitation again. Thank you 
Hmm, I think you are suggesting a level of vulnerability that would render the existence of any script unuseful without prior verification.
Users would have no reason to suspect malicious activity after successful execution of said script, bug induced or otherwise, and thus may not be inclined to look for any such code to begin with.
So I believe what you are actually prescribing is an initial or routine manual verification, which seems more practical, in light of a manual code review.
But let’s compromise and create two branches. 
One with the current state and a readme guide that recommends manual verification and one with your updater script in derivative-maker-docker/docker or wherever you prefer.
Then we’ll just push to both and decide later.
I originally wanted to make derivative-maker more accessible to average users with this project by maximizing user friendliness through automation, but I see your point.
Been considering some activism work myself lately, or at least supporting groups who live in countries with oppressive governments. Not that I’m in a position to judge with our current president.
High-profile targets pursued by state actors will probably avoid docker, no?
Okay, that’s actually a good point. Done.
Merged fixes by tabletseeker · Pull Request #26 · derivative-maker/derivative-maker · GitHub just now.
There’s going to be a quite some refactoring all over the code base related to docker, git and verification.
I advice to hold for a few days until the dust has settled to avoid merge conflicts.
It seems you want to avoid word splitting, so I was gonna improve that fingerprint array loop today. I would like to perfect the git stuff (and anything else) )with @arraybolt3 so you don’t have to do it yourself.
Do you want help-steps/git-gpg-verify or put the entire git stage in derivative-maker/derivative-maker-docker-update?
His point was that this script being manually executed, before derivative-maker-docker-run, may be a security benefit.
I guess you can always add manual verification instructions, regardless.
I kinda like this too, but when everything has been manually verified anyway, couldn’t it at least be executed automatically?
For example, you could exit if the key hasn’t already been imported to ~/gnupg once the container starts. So basically, the user would be forced to download the key and export it to the KEY_VOLUME.
More replies from my side later.
derivaitve-update looks cool. I’ll try it out later.
The refactoring should be done or mostly done. We had talked about keeping a sync feature in derivative-maker-docker, but I ended up removing it entirely after all because it messes with a derivative-update feature that (sorta) removes the need for initial manual verification. Specifically, derivative-update will import GPG keys into the user’s main keyring, then use them to verify the repo’s current state in addition to the commit, branch, or tag the user is trying to update to. This provides trust-on-first-use (TOFU), which isn’t bulletproof but it a lot better than just never verifying at all. As long as the user is “lucky” enough to get an uncompromised repo the first time, future compromises should be guarded against. (This works in practice with SSH pretty well in a lot of scenarios, and while a high-risk user won’t want to stake their safety on it, it will at least provide lower-risk users an additional layer of safety.)
derivative-maker-docker uses a different keyring than the user’s normal keyring, which messes with this mechanism a bit. Depending on how the user does things, they may end up trusting unverified code more than once, which adds additional danger. (This would happen if a user cloned the repo, immediately ran derivative-maker-docker-run to do a build, then later used derivative-update manually to update. It would also happen again any time the user wiped the Docker key volume or moved it to a new location.)
There is an issue with derivative-maker-docker I’ve not been able to fix, which is that when building a VirtualBox VM image, the resulting image is only bootable when using UEFI firmware, not BIOS firmware. The problem is described in detail in amd64 images build under a Debian 12 Docker image end up with no BIOS bootloader due to `lsblk` malfunctioning · Issue #348 · grml/grml-debootstrap · GitHub and `lsblk` fails to report partition type UUIDs within a privileged Debian 12 container · Issue #50304 · moby/moby · GitHub. In summary, lsblk needs some bits of udev to be able to display partition type UUIDs. systemd-udevd.service needs to be running in the container, and we need to run udevadm trigger on loop devices at specific points in the build process (possibly within grml-debootstrap itself). I wasn’t able to solve this on my own yet, because I ran into weird issues with kpartx (it was able to recognize the partitions in a loop device, but it couldn’t make the device-mapper devices corresponding to each partition, probably due to user error on my part). If we want derivative-maker-docker to work fully in the near future, we’ll probably need to solve this. If we’re willing to wait a while, newer versions of lsblk should allow us to avoid the need for udev, so once Kicksecure ports to Trixie, this might be a lot easier to resolve.
I’ll split derivative-update into a different topic, different from docker, since mostly unrelated except maybe on integration of one with the other.
confirm_remote_update() {
query_user "\
NOTICE:
You have requested to update the repository with code from the default remote
repository. Are you sure you want to do this? [Y/N]
"
}
Is this essential? What’s the threat model? Fetching from remote repository is the main feature if derivative-update? I think this will confuse users and they’ll ask about risks, alternatives and advice. But I don’t think there’s any except manual steps?
Please consider it alpha.
Through testing appreciated. I don’t think I’ll personally use the git stash feature.
Unless contributed and not super complex code, will wait for Trixie port. This task is up for grabs.
It’s mainly to avoid mistakes in situations where the user might consider updating more dangerous than usual (like if connected to a known-malicious network). Users might easily run derivative-update on accident if they’re going too fast when using tab completion, or attempting to run derivative-maker from history using Ctrl+R. Updating isn’t really that dangerous unless TLS is compromised, so we could live without it.
Yes, the only alternative is manual steps, so I can see how this could result in confusion.
Alright. Removed just now.
Also the tools used by derivative-maker use networking. I guess one could construct a case “a known malicious network. use derivative-update offline only to change the tag for the purpose off offline manual inspection” but I guess for such advanced use cases it’s best to use command line and/or disable networking.
The automatic git stash and git stash pop feature… I am not a fan of it… Using git on the command line is difficult. Automated use in scripts is even more difficult. There are many commands that can fail. Recovering the git stash just bloats the code and might go wrong. I don’t think it needs to be part of derivative-update. Just 1 thing and do it well.
If a developer wants git stash, perhaps just use a local script to automate git stash + derivative-update + git stash pop. Then if derivative-update goes wrong, just manually recover the git stash. I’ll remove this feature. Difficult to implement inside derivative-update but probably much easier to implement in a wrapper script outside.