Feat/signed canon units #6659

Scoppio · 2025-03-06T01:59:36Z

What it does?

Functional and full feature proof of concept for digitally signing canon units to allow players to have always a list of canon units without worry that a custom unit may overwrite their units, this also does not depend on the MUL ID being present and differently from the OUL it don't suffer with name collisions.

It also has functionalities like generating signing keys, signing the files using github actions, etc.

How it works?

The current way to say a unit is canon is looking up it's name in the Oficial Units List, it works fine but has a few edge cases that it can't deal, like a canonical unit being customized, which should remove its canonicity but it does not, chassis and model collision-If the custom unit has the same model and chassis it may be read as canon. The OUL can also be generated at any moment and it will add to it any custom units you have as canonical.

The signing of the files can make sure that only the files that were signed with the private key used during this dist build are recognized as canon. If the unit is customized in any way its signature becomes invalid since it will change the checksum of the unit file. This feature deals with the problems where a non-canonical unit would by mistake be considered canonical, it does not solve the problem of people with malicious intent as it.

This serves specifically to tell players "Hey, this list of units are those that the copyright holders of the game have released as their own original designs, if you wan't to play using only this kind of unit, feel free, the option to show only canonical units will only show those they released, ok?". We are just stamping those as we curate the list of units, nothing else.

Ok, but HOW it works?!

The unit file is hashed, the hash is signed by a private key, the resulting binary is converted to base64 and saved as the <signature>...</signature> at the bottom of the file.

When loading a file, the signature is striped of it and hold in a temporary variable, then the file is hashed and this hash is loaded into the public key, the public key then compares it to the signature and says if it matches or not. If there is any change to the file then the check will fail. If the file has no signature or an invalid signature it will also fail.

There is a gradel task added in this PR that generates the private public key pair, you have to run it in your own computer, it will generate one binary public key, you add it into the resources folder as public.key, and it will generate two private keys, one private.key and one private.key.b64, you will copy the contents of the private.key.b64 and add it into the repository settings > security > secrets and variables > actions and inside of it you click the button new repository secret, you name it FILE_SIGNING_PRIVATE_KEY and then paste the contents of the private.key.b64 in there.

If you want to run it locally, sure, just add the private key in a place you wont publish it by mistake in the repo.

Help! I published the private key by mistake!!!

Ouch, ok... generate a new one, substitute it in the github repo secrets, add the public key to the repo substituting the old one, generate a new release to sign all files again, publish the new release.

Future improvements?

Allows players in a match during the handshake/connection to exchange/verify the public keys in each others client, and flag any player that has a different public key as using a non-compliant client. This is to my point of view imporant since people do play tournaments and disputes over prize money and imaginary internet points, so a modicum of security is welcome.

We could even do a kind of "only play with same versions" where we can use the public key of each new release to be different, this way two different version programs can't play in the same lobby because their keys are different.

Performance

With signed units feature

Cold opening the game - no cache
12 seconds

Cold opening the game - with cache
1 second

Master branch

Cold opening the game - no cache
8 seconds

Cold opening the game - with cache
1 second

This load process is async and dont block anything in the UI while it happens, so this extra 4 second isn't necessarily something the user ever experiences except for a slightly slower load the first time they open the game after installing

Scoppio · 2025-03-06T02:02:51Z

megamek/src/megamek/common/Entity.java

+    /**
+     * Get the source file for this unit.
+     * @return The source file for this unit, null if it does not exists.
+     */
+    public @Nullable File getSourceFile() {
+        return MekSummary.getSourceFile(getChassis() + " " + getModel());
+    }


turns out this piece of code is unnecessary

megamek/src/megamek/common/MekFileParser.java

@@ -151,7 +153,18 @@
        return m_entity;
    }

-    public void parse(InputStream is, String fileName) throws Exception {
+    public void parse(InputStream ns, String fileName) throws Exception {
+        File tempFile = File.createTempFile("temp", ".tmp");


megamek/src/megamek/common/MekFileParser.java

+                writer.newLine();
+            }
+        }
+        InputStream is = new FileInputStream(tempFile);


Scoppio · 2025-03-06T02:04:01Z

megamek/src/megamek/common/MekFileParser.java

+        File tempFile = File.createTempFile("temp", ".tmp");
+        tempFile.deleteOnExit();
+        try (BufferedReader reader = new BufferedReader(new InputStreamReader(ns));
+             BufferedWriter writer = new BufferedWriter(new FileWriter(tempFile))) {
+            String line;
+            while ((line = reader.readLine()) != null) {
+                writer.write(line);
+                writer.newLine();
+            }
+        }
+        InputStream is = new FileInputStream(tempFile);


I hate it as much as everybody else... best way would be to move this to its own small method and nobody talks about it ever again

Scoppio · 2025-03-06T02:04:34Z

megamek/src/megamek/common/MekFileParser.java

+        // canonicity of the unit was set during the basic data setup with the new method
        // Check if it's canon; if it is, mark it as such.
-        ent.setCanon(false);// Guilty until proven innocent
-        if (canonUnitNames == null) {
-            initCanonUnitNames();
-        }
-
-        int index = Collections.binarySearch(canonUnitNames, ent.getShortNameRaw());
-        if (index >= 0) {
-            ent.setCanon(true);
-        }
+//        ent.setCanon(false);// Guilty until proven innocent
+//        if (canonUnitNames == null) {
+//            initCanonUnitNames();
+//        }
+
+//        int index = Collections.binarySearch(canonUnitNames, ent.getShortNameRaw());
+//        if (index >= 0) {
+//            ent.setCanon(true);
+//        }


Just a proof of concept, this would be removed when integrating it in the project.

Scoppio · 2025-03-06T02:04:52Z

megamek/src/megamek/common/MekSummary.java

@@ -1382,6 +1382,18 @@ public String formatSUA(BattleForceSUA sua, String delimiter, ASSpecialAbilityCo
        return null;
    }

+    public static @Nullable File getSourceFile(String fullName) {


Missing javadoc

Scoppio · 2025-03-06T02:05:43Z

megamek/src/megamek/utilities/FileSignerTool.java

+
+        File privateKeyFile = new File(args[0]);
+        if (!privateKeyFile.exists()) {
+            System.out.println("Private key file not found: " + privateKeyFile.getAbsolutePath());


Have to go with system out here, otherwise it wont print in the github actions

Scoppio · 2025-03-06T02:06:42Z

megamek/src/megamek/utilities/KeyGeneratorTool.java

+    /**
+     * Generate a new key pair and save to files.
+     */
+    public static void generateKeyPair(File privateKeyFile, File publicKeyFile) throws Exception {


I tried generating the key pairs using only linux commands, but I could not, after 5+ hours of multiple tries, make it generate a pair that would work.

Probably better to not introduce a platform dependency anyhow.
This seems fine.

megamek/data/mekfiles/meks/Shrapnel/Vol 18/Thunder Hawk TDK-7ZEM.mtf

…EM.mtf

Sleet01 · 2025-03-10T19:22:38Z

.github/workflows/nightly-ci.yml

+      - name: Setup private key for file signing
+        run: |
+          mkdir -p megamek/keystore
+          echo "${{ secrets.FILE_SIGNING_PRIVATE_KEY }}" > megamek/keystore/private.key
+          chmod 600 megamek/keystore/private.key
+
+      - name: Setup private key for file signing
+        run: |
+          mkdir -p megamek/megamek/keystore
+          echo "${{ secrets.FILE_SIGNING_PRIVATE_KEY }}" | base64 --decode > megamek/megamek/keystore/private.key


I assume these should not have the same name.

Sleet01 · 2025-03-10T19:24:37Z

.gitignore

@@ -14,6 +14,11 @@ Thumbs.db
 /megamek/classes/
 /megamek/bin/

+# Sigining keys


s/Sigining/Signing/

Sleet01

A couple spots need updating, and the POC section that's commented out would need to go.

Scoppio added 5 commits March 5, 2025 12:28

feat: sign and build the files

c112cbc

feat: maybe it works now

81fd5ee

feat: signature working

6807547

feat: working signer

c64de58

chore: cleaning imports

8f5d9e7

Scoppio added Refactoring For New Dev Cycle This PR should be merged at the beginning of a dev cycle Utility Improvement to Existing Feature Used with the RFE tag to indicate an improvement to an existing feature labels Mar 6, 2025

Scoppio self-assigned this Mar 6, 2025

Scoppio requested review from rjhancock and Sleet01 March 6, 2025 01:59

Merge branch 'master' into feat/signed-canon-units

d141f16

Scoppio commented Mar 6, 2025

View reviewed changes

github-advanced-security bot found potential problems Mar 6, 2025

View reviewed changes

Scoppio commented Mar 6, 2025

View reviewed changes

megamek/data/mekfiles/meks/Shrapnel/Vol 18/Thunder Hawk TDK-7ZEM.mtf Outdated Show resolved Hide resolved

Update megamek/data/mekfiles/meks/Shrapnel/Vol 18/Thunder Hawk TDK-7Z…

f0f748e

…EM.mtf

Sleet01 reviewed Mar 10, 2025

View reviewed changes

.gitignore

@@ -14,6 +14,11 @@ Thumbs.db

/megamek/classes/

/megamek/bin/

# Sigining keys

Copy link

Collaborator

Sleet01 Mar 10, 2025

Choose a reason for hiding this comment

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

s/Sigining/Signing/

Sleet01 requested changes Mar 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/signed canon units #6659

Feat/signed canon units #6659

Scoppio commented Mar 6, 2025 •

edited

Loading

Scoppio Mar 6, 2025

Scoppio Mar 6, 2025

Scoppio Mar 6, 2025

Scoppio Mar 6, 2025

Scoppio Mar 6, 2025

Scoppio Mar 6, 2025

Sleet01 Mar 10, 2025

Sleet01 Mar 10, 2025

Sleet01 Mar 10, 2025

Sleet01 left a comment

Feat/signed canon units #6659

Are you sure you want to change the base?

Feat/signed canon units #6659

Conversation

Scoppio commented Mar 6, 2025 • edited Loading

What it does?

How it works?

Ok, but HOW it works?!

Help! I published the private key by mistake!!!

Future improvements?

Performance

With signed units feature

Master branch

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sleet01 left a comment

Choose a reason for hiding this comment

Scoppio commented Mar 6, 2025 •

edited

Loading