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.