feat: allow build-time overrides of attestation server url and static fingerprint override #234
Conversation
this allows the attestation server url to be overriden at AOSP/GrapheneOS build time.
when the fingerprint override resource is set at build time, dynamically look up the device info for the current device, checking that the AVB fingerprint matches the one the Auditor app was built with. uses a singleton to allow lazily-initialized lookups of the device name and override fingerprint in a static context.
| @@ -507,6 +506,10 @@ public boolean onPrepareOptionsMenu(final Menu menu) { | |||
| return true; | |||
| } | |||
|
|
|||
| final String tutorial_url() { | |||
There was a problem hiding this comment.
Should make this private since Java defaults to package public. Don't need to make it final since if we want to do that it probably makes more sense to just make the classes final but it isn't really needed. It doesn't have the usual meaning of an immutable variable binding here but rather preventing an override by a class inheriting this one.
| @@ -127,6 +124,18 @@ static void cancel(final Context context) { | |||
| scheduler.cancel(FIRST_RUN_JOB_ID); | |||
| } | |||
|
|
|||
| final String domain() { | |||
app/src/main/res/values/strings.xml
Outdated
| @@ -1,5 +1,6 @@ | |||
| <resources> | |||
| <string name="app_name">Auditor</string> | |||
| <string name="url">attestation.app</string> | |||
There was a problem hiding this comment.
Probably better to call this base_url and it can include the https:// prefix.
app/src/main/res/values/strings.xml
Outdated
| @@ -1,6 +1,7 @@ | |||
| <resources> | |||
| <string name="app_name">Auditor</string> | |||
| <string name="url">attestation.app</string> | |||
| <string name="avb_fingerprint_override">NOT OVERRIDEN</string> | |||
There was a problem hiding this comment.
This approach won't really work because Auditor requires 2 devices. The standard GrapheneOS and stock OS entries should also be left working for a custom build. Instead, new entries should be added to the non-stock OS table and the person doing it will need to use their build on both ends since our Auditor builds won't trust their Auditor builds (signing key fingerprint of the app is verified remotely via key attestation) and won't have the entries for their OS builds.
There was a problem hiding this comment.
this is here so there's still a local check of the fingerprint before the verification is submitted. I'm unsure how new entries can be added to the table without source code modification at build time. do you have thoughts?
There was a problem hiding this comment.
I took a pass at resolving this by setting up maps in the XML resources to connect device identifiers with fingerprints. this should allow someone building a custom os for several devices to inject their own map of keys / DeviceInfo without source code modification. as I needed to validate the parser anyway, I've moved the existing static maps into their own resource maps, but I'm open to thoughts on that -- it is useful for ensuring the parser doesn't break sometime in the future.
| return getString(R.string.url); | ||
| } | ||
|
|
||
| final String challenge_url() { |
There was a problem hiding this comment.
Method names should also match the code/naming style of the rest of the codebase even if they're private methods
| final String challenge_url() { | |
| final String challengeUrl() { |
|
Turning |
we're actually already passing the Context around so there's no need for a singleton.
this allows multiple devices to be configured the same even when they have different AVB fingerprints, and therefore should be able to validate each other.
they got increased when I was moving the tests around to figure out what would make them actually function.
thestinger
force-pushed
the
main
branch
7 times, most recently
from
ac2dde3 to
6a0e6c2
Compare
thestinger
force-pushed
the
main
branch
2 times, most recently
from
72363db to
34e6d8d
Compare
thestinger
force-pushed
the
main
branch
3 times, most recently
from
3581800 to
a5f4f7b
Compare
thestinger
force-pushed
the
main
branch
3 times, most recently
from
06aa66d to
632bd88
Compare
pulls the attestation server url and overriden fingerprint from res/values/strings, allowing them to be overriden at AOSP/GrapheneOS build time. because the device info/fingerprint can't be known statically, this PR introduces a dynamic look up for the device info and fingerprint via a singleton that gets its context set by the main activity, avoiding null pointer issues by ensuring static accesses for Resources in AttestationProtocol.java occur as late as possible.