Add signature verification to gradle dependencies

[Pururun]

Add support for verifying the signature of dependencies for dependencies that are signed.

I have kept the checksum for now, the default gradle behaviour is to remove checksum verification for those dependencies with signatures, but we can discuss this.

This is mostly based on how fdroid does it.
https://gitlab.com/fdroid/fdroidclient/-/blob/master/gradle/update-verification-metadata.sh

Resolves: DROID-719, DROID-782

---

This change is 

[Pururun]

Setting this as reviewable.

See description for more information.

[linear]

DROID-719 Verify Jetpack Dependencies signature

DROID-782 Update gradle lockfile script to support signed dependencies

[Rawa]

Reviewed 1 of 4 files at r1, all commit messages.
Reviewable status: 1 of 4 files reviewed, 3 unresolved discussions (waiting on @Pururun)

---

.github/workflows/android-audit.yml line 96 at r1 (raw file):

      - name: Re-generate lockfile keys
        run: android/scripts/update-lockfile-keys.sh   

How well does these scripts play together? Should we have one script to update both at the same time?

---

android/scripts/update-lockfile-keys.sh line 32 at r1 (raw file):

function cleanup {
    echo "Cleaning up temp dirs..."
    rm -rf -- "$TEMP_GRADLE_PROJECT_CACHE_DIR" ../gradle/verification-metadata.dryrun.xml ../gradle/verification-keyring.dryrun.keys ../gradle/verification-keyring.dryrun.gpg

Since we use mktemp wouldn't it create a new unique temp folder? Do we need to remove it?

Always a bit scary to do rm -rf especially when passing in an argument.

---

android/scripts/update-lockfile-keys.sh line 50 at r1 (raw file):

# Move keys from dry run file to existing file (This part is taken from: https://gitlab.com/fdroid/fdroidclient/-/blob/master/gradle/update-verification-metadata.sh)
# extract the middle of the new file, https://github.com/gradle/gradle/issues/18569
grep -B 10000 "<trusted-keys>" ../gradle/verification-metadata.dryrun.xml > "$TEMP_GRADLE_PROJECT_CACHE_DIR/new.head"

Feels like this should be doable in a much cleaner way with a third party dependency, but it has some benefits of doing it in the same was as fdroid I suppose. How do you guys feel? It is something we should consider reworking?

[Pururun]

Reviewable status: 1 of 4 files reviewed, 3 unresolved discussions (waiting on @Rawa)

---

.github/workflows/android-audit.yml line 96 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

How well does these scripts play together? Should we have one script to update both at the same time?

I was thinking of running it in the same script, but I ran into caching issues, but I could test it again now that I am not longer generating the keys from scratch and it is not as unstable.

---

android/scripts/update-lockfile-keys.sh line 32 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

Since we use mktemp wouldn't it create a new unique temp folder? Do we need to remove it?

Always a bit scary to do rm -rf especially when passing in an argument.

Not sure if I follow, the temp directory code is the same as the other script.

---

android/scripts/update-lockfile-keys.sh line 50 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

Feels like this should be doable in a much cleaner way with a third party dependency, but it has some benefits of doing it in the same was as fdroid I suppose. How do you guys feel? It is something we should consider reworking?

For me it is fine to change it. I just copied because it is confirmed to work.

[albin-mullvad]

Reviewed 3 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Pururun and @Rawa)

---

android/gradle/verification-metadata.xml line 29 at r1 (raw file):

         <trust file=".*[.]pom" regex="true"/>
      </trusted-artifacts>
      <ignored-keys>

Should this be kept even though key-servers enabled="false"?

---

android/gradle/verification-metadata.xml line 35 at r1 (raw file):

         <ignored-key id="8E3A02905A1AE67E7B0F9ACD3967D4EDA591B991" reason="Key couldn't be downloaded from any key server"/>
      </ignored-keys>
      <trusted-keys>

It looks like we cover a lot of dependencies. Any idea of how many are signed and verified against keys as-of this PR?

---

android/scripts/update-lockfile-keys.sh line 50 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

For me it is fine to change it. I just copied because it is confirmed to work.

@Rawa what third party dependency are you thinking of?

As you say though, there's a lot of value keeping this similar as the F-Droid version IMO. I suggest keeping as-is for now.

[albin-mullvad]

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Pururun and @Rawa)

---

.github/workflows/android-audit.yml line 96 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

I was thinking of running it in the same script, but I ran into caching issues, but I could test it again now that I am not longer generating the keys from scratch and it is not as unstable.

If we'll always or almost always run both, it could be nice to combine the scripts 🤔

[Pururun]

Reviewable status: 3 of 4 files reviewed, 4 unresolved discussions (waiting on @albin-mullvad and @Rawa)

---

.github/workflows/android-audit.yml line 96 at r1 (raw file):

Previously, albin-mullvad wrote…

If we'll always or almost always run both, it could be nice to combine the scripts 🤔

I guess in theory you would not need to update the keys if you only update the version of a dependency.
But I think we should try to combine them.

---

android/gradle/verification-metadata.xml line 29 at r1 (raw file):

Previously, albin-mullvad wrote…

Should this be kept even though key-servers enabled="false"?

That is true, removed them.

---

android/gradle/verification-metadata.xml line 35 at r1 (raw file):

Previously, albin-mullvad wrote…

It looks like we cover a lot of dependencies. Any idea of how many are signed and verified against keys as-of this PR?

I checked and if we remove all dependencies that are signed from the file we remove 5000 lines. Since every dependency is 3-5 lines that would mean ~1000 dependencies and ~300 dependencies that are not.

It is quite massive change and it would perhaps been better to manually add keys in steps, but I could not find a good way to do this.

[albin-mullvad]

Reviewed 1 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Pururun and @Rawa)

[Rawa]

Reviewed 1 of 4 files at r1, 3 of 4 files at r3, 1 of 2 files at r4, all commit messages.
Reviewable status: 5 of 6 files reviewed, all discussions resolved

[Rawa]

Reviewable status: 5 of 6 files reviewed, 1 unresolved discussion (waiting on @Pururun)

---

android/scripts/update-lockfile.sh line 30 at r4 (raw file):

export GRADLE_USER_HOME

function cleanup {

Check what happens if a key has been rotated/replaced and this script is ran again.

[Pururun]

Reviewable status: 4 of 6 files reviewed, 1 unresolved discussion (waiting on @Rawa)

---

android/scripts/update-lockfile.sh line 30 at r4 (raw file):

Previously, Rawa (David Göransson) wrote…

Check what happens if a key has been rotated/replaced and this script is ran again.

Script seems to run fine and key is replaced with the correct one.

[Rawa]

Reviewed 1 of 2 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

[albin-mullvad]

Looks good, I mostly have some nitpicking on some of the code comments and prints in order to keep things consistent.

Reviewed 3 of 4 files at r3, 1 of 2 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @Pururun)

---

android/scripts/update-lockfile.sh line 37 at r6 (raw file):

trap cleanup EXIT

echo "### Updating dependency lockfile ###"

nit: change to Updating checksums

Code quote:

Updating dependency lockfile

---

android/scripts/update-lockfile.sh line 40 at r6 (raw file):

echo "Gradle home: $GRADLE_USER_HOME"
echo "Gradle cache: $TEMP_GRADLE_PROJECT_CACHE_DIR"
echo ""

nit: move to after trap cleanup EXIT since these are shared for both the "checksum" and "key" sections. Maybe also prefix with a print saying something like:

echo "### Configuration ###"

Code quote:

echo "Gradle home: $GRADLE_USER_HOME"
echo "Gradle cache: $TEMP_GRADLE_PROJECT_CACHE_DIR"
echo ""

---

android/scripts/update-lockfile.sh line 54 at r6 (raw file):

done

echo "### Updating dependency lockfile verification keys ###"

nit: change to Updating keys

Code quote:

Updating dependency lockfile verification keys

---

android/scripts/update-lockfile.sh line 57 at r6 (raw file):

echo ""

echo "Set key servers true temporarily"

nit Temporarily enabling key servers...

Code quote:

Set key servers true temporarily

---

android/scripts/update-lockfile.sh line 60 at r6 (raw file):

sed -Ei 's,key-servers enabled="[^"]+",key-servers enabled="true",' ../gradle/verification-metadata.xml

# Generate keys

nit: remove this comment

---

android/scripts/update-lockfile.sh line 65 at r6 (raw file):

../gradlew -q -p .. --project-cache-dir "$TEMP_GRADLE_PROJECT_CACHE_DIR" -M pgp,sha256 "${GRADLE_TASKS[@]}" --export-keys --dry-run "${EXCLUDED_GRADLE_TASKS[@]}"

# Move keys from dry run file to existing file (This part is taken from: https://gitlab.com/fdroid/fdroidclient/-/blob/master/gradle/update-verification-metadata.sh)

nit: change to

Move keys from dry run file to existing file.
This part is taken from: https://gitlab.com/fdroid/fdroidclient/-/blob/master/gradle/update-verification-metadata.sh

Code quote:

Move keys from dry run file to existing file (This part is taken from: https://gitlab.com/fdroid/fdroidclient/-/blob/master/gradle/update-verification-metadata.sh)

---

android/scripts/update-lockfile.sh line 66 at r6 (raw file):

# Move keys from dry run file to existing file (This part is taken from: https://gitlab.com/fdroid/fdroidclient/-/blob/master/gradle/update-verification-metadata.sh)
# extract the middle of the new file, https://github.com/gradle/gradle/issues/18569

nit: Extract the middle of the new file due to: https://github.com/gradle/gradle/issues/18569

Code quote:

extract

---

android/scripts/update-lockfile.sh line 74 at r6 (raw file):

numLines="$(< ../gradle/verification-metadata.dryrun.xml wc -l)"
numMiddleLines="$((numLines - numTopLines - numBottomLines))"
# also remove 'version=' lines, https://github.com/gradle/gradle/issues/20192

nit: Remove 'version=' due to: https://github.com/gradle/gradle/issues/20192

Code quote:

also remove 'version=' lines, https://github.com/gradle/gradle/issues/20192

---

android/scripts/update-lockfile.sh line 77 at r6 (raw file):

< ../gradle/verification-metadata.dryrun.xml tail -n "+$numTopLinesPlus1" | head -n "$numMiddleLines" | sed 's/ version="[^"]*"//' > "$TEMP_GRADLE_PROJECT_CACHE_DIR/new.middle"

# extract the top and bottom of the old file

nit: Extract

Code quote:

extract

---

android/scripts/update-lockfile.sh line 81 at r6 (raw file):

grep -A 10000 "</trusted-keys>" ../gradle/verification-metadata.xml > "$TEMP_GRADLE_PROJECT_CACHE_DIR/old.tail"

# update verification metadata file

nit: Update

Code quote:

update

---

android/scripts/update-lockfile.sh line 84 at r6 (raw file):

cat "$TEMP_GRADLE_PROJECT_CACHE_DIR/old.head" "$TEMP_GRADLE_PROJECT_CACHE_DIR/new.middle" "$TEMP_GRADLE_PROJECT_CACHE_DIR/old.tail" > ../gradle/verification-metadata.xml

echo "sorting keyring and removing duplicates"

nit: Sorting keyring and removing duplicates...

Code quote:

sorting

---

android/scripts/update-lockfile.sh line 85 at r6 (raw file):

echo "sorting keyring and removing duplicates"
  # sort and unique the keyring

nit: Sort

Code quote:

sort

---

android/scripts/update-lockfile.sh line 103 at r6 (raw file):

    > ../gradle/verification-keyring.keys

echo "Disable key servers again"

nit: Disabling key servers...

[Pururun]

Reviewable status: complete! all files reviewed, all discussions resolved

---

android/scripts/update-lockfile.sh line 37 at r6 (raw file):

Previously, albin-mullvad wrote…

nit: change to Updating checksums

Done

---

android/scripts/update-lockfile.sh line 40 at r6 (raw file):

Previously, albin-mullvad wrote…

nit: move to after trap cleanup EXIT since these are shared for both the "checksum" and "key" sections. Maybe also prefix with a print saying something like:

echo "### Configuration ###"

Done

---

android/scripts/update-lockfile.sh line 54 at r6 (raw file):

Previously, albin-mullvad wrote…

nit: change to Updating keys

Done

---

android/scripts/update-lockfile.sh line 57 at r6 (raw file):

Previously, albin-mullvad wrote…

nit Temporarily enabling key servers...

Done

---

android/scripts/update-lockfile.sh line 60 at r6 (raw file):

Previously, albin-mullvad wrote…

nit: remove this comment

Done

---

android/scripts/update-lockfile.sh line 65 at r6 (raw file):

Previously, albin-mullvad wrote…

nit: change to

Move keys from dry run file to existing file.
This part is taken from: https://gitlab.com/fdroid/fdroidclient/-/blob/master/gradle/update-verification-metadata.sh

Done

---

android/scripts/update-lockfile.sh line 66 at r6 (raw file):

Previously, albin-mullvad wrote…

nit: Extract the middle of the new file due to: https://github.com/gradle/gradle/issues/18569

Done

---

android/scripts/update-lockfile.sh line 74 at r6 (raw file):

Previously, albin-mullvad wrote…

nit: Remove 'version=' due to: https://github.com/gradle/gradle/issues/20192

Done

---

android/scripts/update-lockfile.sh line 77 at r6 (raw file):

Previously, albin-mullvad wrote…

nit: Extract

Done

---

android/scripts/update-lockfile.sh line 81 at r6 (raw file):

Previously, albin-mullvad wrote…

nit: Update

Done

---

android/scripts/update-lockfile.sh line 84 at r6 (raw file):

Previously, albin-mullvad wrote…

nit: Sorting keyring and removing duplicates...

Done

---

android/scripts/update-lockfile.sh line 85 at r6 (raw file):

Previously, albin-mullvad wrote…

nit: Sort

Done

---

android/scripts/update-lockfile.sh line 103 at r6 (raw file):

Previously, albin-mullvad wrote…

nit: Disabling key servers...

Done

[albin-mullvad]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

---

android/scripts/update-lockfile.sh line 84 at r6 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

Done

nit: please also add the ... to the end to keep it consistent with the other prints

[Pururun]

Reviewable status: complete! all files reviewed, all discussions resolved

---

android/scripts/update-lockfile.sh line 84 at r6 (raw file):

Previously, albin-mullvad wrote…

nit: please also add the ... to the end to keep it consistent with the other prints

Missed ... part. Fixed.

[albin-mullvad]

Reviewed 1 of 1 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved