Improve null safety (#46)

9 files changed, 85 insertions, 90 deletions

diff --git a/WORKSPACE b/WORKSPACE
index e99b0ee..1b893a1 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -27,23 +27,18 @@ load("@rules_jvm_external//:defs.bzl", "maven_install")
 
 maven_install(
     artifacts = [
-        # Bouncy Castle Cryptography APIs used for certificate verification
-        "org.bouncycastle:bcpkix-jdk15on:1.61",
-        "org.bouncycastle:bcprov-jdk15on:1.61",
-
-        # Gson used for decoding certificate status list
         "com.google.code.gson:gson:2.8.5",
-
-        "com.google.guava:guava:32.0.1-jre",
         "com.google.errorprone:error_prone_annotations:2.3.1",
-
+        "com.google.guava:guava:32.0.1-jre",
         "com.squareup.okhttp3:okhttp:4.10.0",
+        "org.bouncycastle:bcpkix-jdk15on:1.61",
+        "org.bouncycastle:bcprov-jdk15on:1.61",
+        "org.jspecify:jspecify:0.2.0",
 
-        # Test libraries
-        "junit:junit:4.12",
+        "com.google.testparameterinjector:test-parameter-injector:1.11",
         "com.google.truth:truth:1.0",
         "com.google.truth.extensions:truth-java8-extension:1.0",
-        "com.google.testparameterinjector:test-parameter-injector:1.11",
+        "junit:junit:4.12",
     ],
     repositories = [
         "https://repo1.maven.org/maven2/",
diff --git a/server/build.gradle b/server/build.gradle
index 77ff17a..fa3eba8 100644
--- a/server/build.gradle
+++ b/server/build.gradle
@@ -25,16 +25,15 @@ repositories {
 }
 
 dependencies {
-    // Bouncy Castle Cryptography APIs used for certificate verification
-    compile 'org.bouncycastle:bcpkix-jdk15on:1.61'
-    compile 'com.google.guava:guava:32.0.1-jre'
-    compile 'com.google.errorprone:error_prone_annotations:2.3.1'
-    // Gson used for decoding certificate status list
     compile 'com.google.code.gson:gson:2.8.5'
+    compile 'com.google.errorprone:error_prone_annotations:2.3.1'
+    compile 'com.google.guava:guava:32.0.1-jre'
     compile 'com.squareup.okhttp3:okhttp:4.10.0'
-    // JUnit, Truth and Truth8 used for testing
-    testCompile 'junit:junit:4.12'
+    compile 'org.bouncycastle:bcpkix-jdk15on:1.61'
+    compile 'org.jspecify:jspecify:0.2.0'
+
+    testCompile 'com.google.testparameterinjector:test-parameter-injector:1.11'
     testCompile 'com.google.truth:truth:1.0'
     testCompile 'com.google.truth.extensions:truth-java8-extension:1.0'
-    testCompile 'com.google.testparameterinjector:test-parameter-injector:1.11'
+    testCompile 'junit:junit:4.12'
 }
diff --git a/server/src/main/java/com/android/example/KeyAttestationExample.java b/server/src/main/java/com/android/example/KeyAttestationExample.java
index 51785f8..60dbeaf 100644
--- a/server/src/main/java/com/android/example/KeyAttestationExample.java
+++ b/server/src/main/java/com/android/example/KeyAttestationExample.java
@@ -147,7 +147,7 @@ public class KeyAttestationExample {
     authorizationList.rootOfTrust.ifPresent(
         rootOfTrust -> {
           System.out.println(indent + "Root Of Trust:");
-          print(authorizationList.rootOfTrust, indent + "\t");
+          print(rootOfTrust, indent + "\t");
         });
     print(authorizationList.osVersion, indent + "OS Version");
     print(authorizationList.osPatchLevel, indent + "OS Patch Level");
@@ -179,8 +179,10 @@ public class KeyAttestationExample {
         indent + "Verified Boot Key: " + Base64.toBase64String(rootOfTrust.verifiedBootKey));
     System.out.println(indent + "Device Locked: " + rootOfTrust.deviceLocked);
     System.out.println(indent + "Verified Boot State: " + rootOfTrust.verifiedBootState.name());
-    System.out.println(
-        indent + "Verified Boot Hash: " + Base64.toBase64String(rootOfTrust.verifiedBootHash));
+    rootOfTrust.verifiedBootHash.ifPresent(
+        verifiedBootHash ->
+            System.out.println(
+                indent + "Verified Boot Hash: " + Base64.toBase64String(verifiedBootHash)));
   }
 
   private static void print(AttestationApplicationId attestationApplicationId, String indent) {
diff --git a/server/src/main/java/com/google/android/attestation/AttestationApplicationId.java b/server/src/main/java/com/google/android/attestation/AttestationApplicationId.java
index 564e8f7..c235d47 100644
--- a/server/src/main/java/com/google/android/attestation/AttestationApplicationId.java
+++ b/server/src/main/java/com/google/android/attestation/AttestationApplicationId.java
@@ -21,7 +21,6 @@ import static com.google.android.attestation.Constants.ATTESTATION_PACKAGE_INFO_
 import static com.google.android.attestation.Constants.ATTESTATION_PACKAGE_INFO_VERSION_INDEX;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -35,6 +34,7 @@ import org.bouncycastle.asn1.ASN1Set;
 import org.bouncycastle.asn1.DEROctetString;
 import org.bouncycastle.asn1.DERSequence;
 import org.bouncycastle.asn1.DERSet;
+import org.jspecify.nullness.Nullable;
 
 /**
  * This data structure reflects the Android platform's belief as to which apps are allowed to use
@@ -47,9 +47,9 @@ public class AttestationApplicationId {
   public final List<AttestationPackageInfo> packageInfos;
   public final List<byte[]> signatureDigests;
 
-  private AttestationApplicationId(DEROctetString attestationApplicationId) throws IOException {
+  private AttestationApplicationId(byte[] attestationApplicationId) {
     ASN1Sequence attestationApplicationIdSequence =
-        (ASN1Sequence) ASN1Sequence.fromByteArray(attestationApplicationId.getOctets());
+        ASN1Sequence.getInstance(attestationApplicationId);
     ASN1Set attestationPackageInfos =
         (ASN1Set)
             attestationApplicationIdSequence.getObjectAt(
@@ -75,16 +75,8 @@ public class AttestationApplicationId {
     this.signatureDigests = signatureDigests;
   }
 
-  static AttestationApplicationId createAttestationApplicationId(
-      DEROctetString attestationApplicationId) {
-    if (attestationApplicationId == null) {
-      return null;
-    }
-    try {
-      return new AttestationApplicationId(attestationApplicationId);
-    } catch (IOException e) {
-      return null;
-    }
+  static AttestationApplicationId createAttestationApplicationId(byte[] attestationApplicationId) {
+    return new AttestationApplicationId(attestationApplicationId);
   }
 
   ASN1Sequence toAsn1Sequence() {
@@ -103,7 +95,7 @@ public class AttestationApplicationId {
   }
 
   @Override
-  public boolean equals(Object object) {
+  public boolean equals(@Nullable Object object) {
     if (object instanceof AttestationApplicationId) {
       AttestationApplicationId that = (AttestationApplicationId) object;
       return this.packageInfos.equals(that.packageInfos)
@@ -149,7 +141,7 @@ public class AttestationApplicationId {
     }
 
     @Override
-    public boolean equals(Object object) {
+    public boolean equals(@Nullable Object object) {
       if (object instanceof AttestationPackageInfo) {
         AttestationPackageInfo that = (AttestationPackageInfo) object;
         return this.packageName.equals(that.packageName) && this.version == that.version;
diff --git a/server/src/main/java/com/google/android/attestation/AuthorizationList.java b/server/src/main/java/com/google/android/attestation/AuthorizationList.java
index 2a7dddb..f43a184 100644
--- a/server/src/main/java/com/google/android/attestation/AuthorizationList.java
+++ b/server/src/main/java/com/google/android/attestation/AuthorizationList.java
@@ -412,19 +412,17 @@ public class AuthorizationList {
     this.rollbackResistant =
         findBooleanAuthorizationListEntry(authorizationMap, KM_TAG_ROLLBACK_RESISTANT);
     this.rootOfTrust =
-        Optional.ofNullable(
-            RootOfTrust.createRootOfTrust(
-                (ASN1Sequence) findAuthorizationListEntry(authorizationMap, KM_TAG_ROOT_OF_TRUST),
-                attestationVersion));
+        findAuthorizationListEntry(authorizationMap, KM_TAG_ROOT_OF_TRUST)
+            .map(ASN1Sequence.class::cast)
+            .map(rootOfTrust -> RootOfTrust.createRootOfTrust(rootOfTrust, attestationVersion));
     this.osVersion = findOptionalIntegerAuthorizationListEntry(authorizationMap, KM_TAG_OS_VERSION);
     this.osPatchLevel =
         findOptionalIntegerAuthorizationListEntry(authorizationMap, KM_TAG_OS_PATCH_LEVEL);
     this.attestationApplicationId =
-        Optional.ofNullable(
-            AttestationApplicationId.createAttestationApplicationId(
-                (DEROctetString)
-                    findAuthorizationListEntry(
-                        authorizationMap, KM_TAG_ATTESTATION_APPLICATION_ID)));
+        findAuthorizationListEntry(authorizationMap, KM_TAG_ATTESTATION_APPLICATION_ID)
+            .map(ASN1OctetString.class::cast)
+            .map(ASN1OctetString::getOctets)
+            .map(AttestationApplicationId::createAttestationApplicationId);
     this.attestationApplicationIdBytes =
         findOptionalByteArrayAuthorizationListEntry(
             authorizationMap, KM_TAG_ATTESTATION_APPLICATION_ID);
@@ -519,14 +517,15 @@ public class AuthorizationList {
     return authorizationMap;
   }
 
-  private static ASN1Primitive findAuthorizationListEntry(
+  private static Optional<ASN1Primitive> findAuthorizationListEntry(
       Map<Integer, ASN1Primitive> authorizationMap, int tag) {
-    return authorizationMap.getOrDefault(tag, null);
+    return Optional.ofNullable(authorizationMap.get(tag));
   }
 
   private static ImmutableSet<Integer> findIntegerSetAuthorizationListEntry(
       Map<Integer, ASN1Primitive> authorizationMap, int tag) {
-    ASN1Set asn1Set = (ASN1Set) findAuthorizationListEntry(authorizationMap, tag);
+    ASN1Set asn1Set =
+        findAuthorizationListEntry(authorizationMap, tag).map(ASN1Set.class::cast).orElse(null);
     if (asn1Set == null) {
       return ImmutableSet.of();
     }
@@ -541,8 +540,9 @@ public class AuthorizationList {
 
   private static Optional<Integer> findOptionalIntegerAuthorizationListEntry(
       Map<Integer, ASN1Primitive> authorizationMap, int tag) {
-    ASN1Primitive entry = findAuthorizationListEntry(authorizationMap, tag);
-    return Optional.ofNullable(entry).map(ASN1Parsing::getIntegerFromAsn1);
+    return findAuthorizationListEntry(authorizationMap, tag)
+        .map(ASN1Integer.class::cast)
+        .map(ASN1Parsing::getIntegerFromAsn1);
   }
 
   private static Optional<Instant> findOptionalInstantMillisAuthorizationListEntry(
@@ -553,19 +553,21 @@ public class AuthorizationList {
 
   private static Optional<Long> findOptionalLongAuthorizationListEntry(
       Map<Integer, ASN1Primitive> authorizationMap, int tag) {
-    ASN1Integer longEntry = ((ASN1Integer) findAuthorizationListEntry(authorizationMap, tag));
-    return Optional.ofNullable(longEntry).map(value -> value.getValue().longValue());
+    return findAuthorizationListEntry(authorizationMap, tag)
+        .map(ASN1Integer.class::cast)
+        .map(value -> value.getValue().longValue());
   }
 
   private static boolean findBooleanAuthorizationListEntry(
       Map<Integer, ASN1Primitive> authorizationMap, int tag) {
-    return null != findAuthorizationListEntry(authorizationMap, tag);
+    return findAuthorizationListEntry(authorizationMap, tag).isPresent();
   }
 
   private static Optional<byte[]> findOptionalByteArrayAuthorizationListEntry(
       Map<Integer, ASN1Primitive> authorizationMap, int tag) {
-    ASN1OctetString entry = (ASN1OctetString) findAuthorizationListEntry(authorizationMap, tag);
-    return Optional.ofNullable(entry).map(ASN1OctetString::getOctets);
+    return findAuthorizationListEntry(authorizationMap, tag)
+        .map(ASN1OctetString.class::cast)
+        .map(ASN1OctetString::getOctets);
   }
 
   private static ImmutableSet<UserAuthType> findUserAuthType(
diff --git a/server/src/main/java/com/google/android/attestation/BUILD b/server/src/main/java/com/google/android/attestation/BUILD
index 541c87b..55d9d30 100644
--- a/server/src/main/java/com/google/android/attestation/BUILD
+++ b/server/src/main/java/com/google/android/attestation/BUILD
@@ -18,5 +18,6 @@ java_library(
         "@maven//:com_squareup_okhttp3_okhttp",
         "@maven//:org_bouncycastle_bcpkix_jdk15on",
         "@maven//:org_bouncycastle_bcprov_jdk15on",
+        "@maven//:org_jspecify_jspecify",
     ],
 )
diff --git a/server/src/main/java/com/google/android/attestation/RootOfTrust.java b/server/src/main/java/com/google/android/attestation/RootOfTrust.java
index df3dbd5..a07100c 100644
--- a/server/src/main/java/com/google/android/attestation/RootOfTrust.java
+++ b/server/src/main/java/com/google/android/attestation/RootOfTrust.java
@@ -24,6 +24,7 @@ import static com.google.android.attestation.Constants.ROOT_OF_TRUST_VERIFIED_BO
 import static com.google.android.attestation.Constants.ROOT_OF_TRUST_VERIFIED_BOOT_KEY_INDEX;
 import static com.google.android.attestation.Constants.ROOT_OF_TRUST_VERIFIED_BOOT_STATE_INDEX;
 
+import java.util.Optional;
 import org.bouncycastle.asn1.ASN1Boolean;
 import org.bouncycastle.asn1.ASN1Encodable;
 import org.bouncycastle.asn1.ASN1Enumerated;
@@ -38,7 +39,7 @@ public class RootOfTrust {
   public final byte[] verifiedBootKey;
   public final boolean deviceLocked;
   public final VerifiedBootState verifiedBootState;
-  public final byte[] verifiedBootHash;
+  public final Optional<byte[]> verifiedBootHash;
 
   private RootOfTrust(ASN1Sequence rootOfTrust, int attestationVersion) {
     this.verifiedBootKey =
@@ -50,20 +51,20 @@ public class RootOfTrust {
         verifiedBootStateToEnum(
             ASN1Parsing.getIntegerFromAsn1(
                 rootOfTrust.getObjectAt(ROOT_OF_TRUST_VERIFIED_BOOT_STATE_INDEX)));
-    if (attestationVersion >= 3) {
-      this.verifiedBootHash =
-          ((ASN1OctetString) rootOfTrust.getObjectAt(ROOT_OF_TRUST_VERIFIED_BOOT_HASH_INDEX))
-              .getOctets();
-    } else {
-      this.verifiedBootHash = null;
-    }
+    this.verifiedBootHash =
+        attestationVersion >= 3
+            ? Optional.of(
+                ASN1OctetString.getInstance(
+                        rootOfTrust.getObjectAt(ROOT_OF_TRUST_VERIFIED_BOOT_HASH_INDEX))
+                    .getOctets())
+            : Optional.empty();
   }
 
   private RootOfTrust(
       byte[] verifiedBootKey,
       boolean deviceLocked,
       VerifiedBootState verifiedBootState,
-      byte[] verifiedBootHash) {
+      Optional<byte[]> verifiedBootHash) {
     this.verifiedBootKey = verifiedBootKey;
     this.deviceLocked = deviceLocked;
     this.verifiedBootState = verifiedBootState;
@@ -71,9 +72,6 @@ public class RootOfTrust {
   }
 
   static RootOfTrust createRootOfTrust(ASN1Sequence rootOfTrust, int attestationVersion) {
-    if (rootOfTrust == null) {
-      return null;
-    }
     return new RootOfTrust(rootOfTrust, attestationVersion);
   }
 
@@ -81,7 +79,7 @@ public class RootOfTrust {
       byte[] verifiedBootKey,
       boolean deviceLocked,
       VerifiedBootState verifiedBootState,
-      byte[] verifiedBootHash) {
+      Optional<byte[]> verifiedBootHash) {
     return new RootOfTrust(verifiedBootKey, deviceLocked, verifiedBootState, verifiedBootHash);
   }
 
@@ -127,10 +125,11 @@ public class RootOfTrust {
 
   public ASN1Sequence toAsn1Sequence() {
     ASN1Encodable[] rootOfTrustElements;
-    if (this.verifiedBootHash != null) {
+    byte[] verifiedBootHash = this.verifiedBootHash.orElse(null);
+    if (verifiedBootHash != null) {
       rootOfTrustElements = new ASN1Encodable[4];
       rootOfTrustElements[ROOT_OF_TRUST_VERIFIED_BOOT_HASH_INDEX] =
-          new DEROctetString(this.verifiedBootHash);
+          new DEROctetString(verifiedBootHash);
     } else {
       rootOfTrustElements = new ASN1Encodable[3];
     }
diff --git a/server/src/test/java/com/google/android/attestation/AttestationApplicationIdTest.java b/server/src/test/java/com/google/android/attestation/AttestationApplicationIdTest.java
index 5d18f06..3701186 100644
--- a/server/src/test/java/com/google/android/attestation/AttestationApplicationIdTest.java
+++ b/server/src/test/java/com/google/android/attestation/AttestationApplicationIdTest.java
@@ -17,10 +17,10 @@ package com.google.android.attestation;
 
 import static com.google.common.truth.Truth.assertThat;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.Assert.assertThrows;
 
 import com.google.android.attestation.AttestationApplicationId.AttestationPackageInfo;
 import com.google.common.collect.ImmutableList;
-import org.bouncycastle.asn1.DEROctetString;
 import org.bouncycastle.util.encoders.Base64;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -31,17 +31,16 @@ import org.junit.runners.JUnit4;
 public class AttestationApplicationIdTest {
 
   // Generated from certificate with RSA Algorithm and StrongBox Security Level
-  private static final DEROctetString ATTESTATION_APPLICATION_ID =
-      new DEROctetString(
-          Base64.decode(
-              "MIIBszGCAYswDAQHYW5kcm9pZAIBHTAZBBRjb20uYW5kcm9pZC5rZXljaGFpbgIBHTAZBBRjb20uYW5kcm9p"
-                  + "ZC5zZXR0aW5ncwIBHTAZBBRjb20ucXRpLmRpYWdzZXJ2aWNlcwIBHTAaBBVjb20uYW5kcm9pZC5keW"
-                  + "5zeXN0ZW0CAR0wHQQYY29tLmFuZHJvaWQuaW5wdXRkZXZpY2VzAgEdMB8EGmNvbS5hbmRyb2lkLmxv"
-                  + "Y2FsdHJhbnNwb3J0AgEdMB8EGmNvbS5hbmRyb2lkLmxvY2F0aW9uLmZ1c2VkAgEdMB8EGmNvbS5hbm"
-                  + "Ryb2lkLnNlcnZlci50ZWxlY29tAgEdMCAEG2NvbS5hbmRyb2lkLndhbGxwYXBlcmJhY2t1cAIBHTAh"
-                  + "BBxjb20uZ29vZ2xlLlNTUmVzdGFydERldGVjdG9yAgEdMCIEHWNvbS5nb29nbGUuYW5kcm9pZC5oaW"
-                  + "RkZW5tZW51AgEBMCMEHmNvbS5hbmRyb2lkLnByb3ZpZGVycy5zZXR0aW5ncwIBHTEiBCAwGqPLCBE0"
-                  + "UBxF8UIqvGbCQiT9Xe1f3I8X5pcXb9hmqg=="));
+  private static final byte[] ATTESTATION_APPLICATION_ID =
+      Base64.decode(
+          "MIIBszGCAYswDAQHYW5kcm9pZAIBHTAZBBRjb20uYW5kcm9pZC5rZXljaGFpbgIBHTAZBBRjb20uYW5kcm9p"
+              + "ZC5zZXR0aW5ncwIBHTAZBBRjb20ucXRpLmRpYWdzZXJ2aWNlcwIBHTAaBBVjb20uYW5kcm9pZC5keW"
+              + "5zeXN0ZW0CAR0wHQQYY29tLmFuZHJvaWQuaW5wdXRkZXZpY2VzAgEdMB8EGmNvbS5hbmRyb2lkLmxv"
+              + "Y2FsdHJhbnNwb3J0AgEdMB8EGmNvbS5hbmRyb2lkLmxvY2F0aW9uLmZ1c2VkAgEdMB8EGmNvbS5hbm"
+              + "Ryb2lkLnNlcnZlci50ZWxlY29tAgEdMCAEG2NvbS5hbmRyb2lkLndhbGxwYXBlcmJhY2t1cAIBHTAh"
+              + "BBxjb20uZ29vZ2xlLlNTUmVzdGFydERldGVjdG9yAgEdMCIEHWNvbS5nb29nbGUuYW5kcm9pZC5oaW"
+              + "RkZW5tZW51AgEBMCMEHmNvbS5hbmRyb2lkLnByb3ZpZGVycy5zZXR0aW5ncwIBHTEiBCAwGqPLCBE0"
+              + "UBxF8UIqvGbCQiT9Xe1f3I8X5pcXb9hmqg==");
 
   private static final ImmutableList<AttestationPackageInfo> EXPECTED_PACKAGE_INFOS =
       ImmutableList.of(
@@ -72,12 +71,15 @@ public class AttestationApplicationIdTest {
   }
 
   @Test
-  public void testCreateEmptyAttestationApplicationIdFromEmptyOrInvalidInput() {
-    assertThat(AttestationApplicationId.createAttestationApplicationId(null)).isNull();
-    assertThat(
+  public void createAttestationApplicationId_nullOrInvalidInput_throwsException() {
+    assertThrows(
+        NullPointerException.class,
+        () -> AttestationApplicationId.createAttestationApplicationId(null));
+    assertThrows(
+        IllegalArgumentException.class,
+        () ->
             AttestationApplicationId.createAttestationApplicationId(
-                new DEROctetString("Invalid DEROctet String".getBytes(UTF_8))))
-        .isNull();
+                "Invalid DEROctet String".getBytes(UTF_8)));
   }
 
   @Test
diff --git a/server/src/test/java/com/google/android/attestation/RootOfTrustTest.java b/server/src/test/java/com/google/android/attestation/RootOfTrustTest.java
index 3f952a6..8cc4e72 100644
--- a/server/src/test/java/com/google/android/attestation/RootOfTrustTest.java
+++ b/server/src/test/java/com/google/android/attestation/RootOfTrustTest.java
@@ -16,6 +16,8 @@
 package com.google.android.attestation;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
+import static org.junit.Assert.assertThrows;
 
 import com.google.android.attestation.RootOfTrust.VerifiedBootState;
 import java.io.IOException;
@@ -58,11 +60,12 @@ public class RootOfTrustTest {
     assertThat(rootOfTrust.verifiedBootKey).isEqualTo(EXPECTED_VERIFIED_BOOT_KEY);
     assertThat(rootOfTrust.deviceLocked).isEqualTo(EXPECTED_DEVICE_LOCKED);
     assertThat(rootOfTrust.verifiedBootState).isEqualTo(EXPECTED_VERIFIED_BOOT_STATE);
-    assertThat(rootOfTrust.verifiedBootHash).isEqualTo(EXPECTED_VERIFIED_BOOT_HASH);
+    assertThat(rootOfTrust.verifiedBootHash).hasValue(EXPECTED_VERIFIED_BOOT_HASH);
   }
 
   @Test
   public void testCreateEmptyRootOfTrust() {
-    assertThat(RootOfTrust.createRootOfTrust(null, ATTESTATION_VERSION)).isNull();
+    assertThrows(
+        NullPointerException.class, () -> RootOfTrust.createRootOfTrust(null, ATTESTATION_VERSION));
   }
 }