Skip to content

Commit f84cfec

Browse files
leshileshi
committed
Fixes as per shawn's comments
1 parent 78054d2 commit f84cfec

4 files changed

Lines changed: 44 additions & 65 deletions

File tree

‎u2f-ref-code/java/src/com/google/u2f/server/impl/androidattestation/Algorithm.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import java.security.cert.CertificateParsingException;
44

55
/**
6-
* Keysmaster algorithm values as taken from: keymaster_defs.h
6+
* Keysmaster algorithm values as taken from: keymaster_defs.h / KeymasterDefs.java
77
*/
88
public enum Algorithm {
99
/* Asymmetric algorithms. */

‎u2f-ref-code/java/src/com/google/u2f/server/impl/androidattestation/AndroidKeyStoreAttestation.java‎

Lines changed: 41 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -35,23 +35,22 @@ public class AndroidKeyStoreAttestation {
3535
private static final int DESCRIPTION_UNIQUE_INDEX = 4;
3636

3737
// Don't expect more than 32 bits for any INTEGER
38-
private static final int MAX_INTEGER_BITS = 32;
38+
private static final int MAX_INT_BITS = 32;
39+
private static final int MAX_LONG_BITS = 64;
3940

4041
// Tags for Authorization List
4142
private static final int AUTHZ_PURPOSE_TAG = 1;
4243
private static final int AUTHZ_ALGORITHM_TAG = 2;
4344

44-
private final Integer keymasterVersion;
45+
private final int keymasterVersion;
4546
private final byte[] attestationChallenge;
4647
private final AuthorizationList softwareAuthorizationList;
47-
private final byte[] uniqueId;
4848

4949
private AndroidKeyStoreAttestation(Integer keymasterVersion, byte[] attestationChallenge,
50-
AuthorizationList softwareAuthorizationList, byte[] uniqueId) {
50+
AuthorizationList softwareAuthorizationList) {
5151
this.keymasterVersion = keymasterVersion;
5252
this.attestationChallenge = attestationChallenge;
5353
this.softwareAuthorizationList = softwareAuthorizationList;
54-
this.uniqueId = uniqueId;
5554
}
5655

5756
/**
@@ -66,66 +65,66 @@ private AndroidKeyStoreAttestation(Integer keymasterVersion, byte[] attestationC
6665
*
6766
* AuthorizationList ::= SEQUENCE {
6867
* -- See keymaster_purpose_t for purpose values.
69-
* purpose [1] IMPLICIT SET OF INTEGER OPTIONAL,
68+
* purpose [1] EXPLICIT SET OF INTEGER OPTIONAL,
7069
* -- See keymaster_algorithm_t for algorithm values.
71-
* algorithm [2] IMPLICIT INTEGER OPTIONAL,
70+
* algorithm [2] EXPLICIT INTEGER OPTIONAL,
7271
* -- keySize is measured in bits, not bytes, and the value must be
7372
* -- positive and less than than 2^32, though realistic values are
7473
* -- much smaller.
75-
* keySize [3] IMPLICIT INTEGER OPTIONAL,
74+
* keySize [3] EXPLICIT INTEGER OPTIONAL,
7675
* -- See keymaster_block_mode_t for blockMode values.
77-
* blockMode [4] IMPLICIT SET OF INTEGER OPTIONAL,
76+
* blockMode [4] EXPLICIT SET OF INTEGER OPTIONAL,
7877
* -- See keymaster_digest_t for digest values.
79-
* digest [5] IMPLICIT SET OF INTEGER OPTIONAL,
78+
* digest [5] EXPLICIT SET OF INTEGER OPTIONAL,
8079
* -- See keymaster_padding_t for padding values.
81-
* padding [6] IMPLICIT SET OF INTEGER OPTIONAL,
82-
* callerNonce [7] IMPLICIT NULL OPTIONAL,
80+
* padding [6] EXPLICIT SET OF INTEGER OPTIONAL,
81+
* callerNonce [7] EXPLICIT NULL OPTIONAL,
8382
* -- minMacLength values must be positive and less than 2^32.
84-
* minMacLength [8] IMPLICIT INTEGER OPTIONAL,
83+
* minMacLength [8] EXPLICIT INTEGER OPTIONAL,
8584
* -- See keymaster_kdf_t for kdf values.
86-
* kdf [9] IMPLICIT SEQUENCE OF INTEGER OPTIONAL,
85+
* kdf [9] EXPLICIT SEQUENCE OF INTEGER OPTIONAL,
8786
* -- See keymaster_ec_curve_t for ecCurve values
88-
* ecCurve [10] IMPLICIT INTEGER OPTIONAL,
87+
* ecCurve [10] EXPLICIT INTEGER OPTIONAL,
8988
* -- rsaPublicExponent must be a valid RSA public exponent less
9089
* -- than 2^64.
91-
* rsaPublicExponent [200] IMPLICIT INTEGER OPTIONAL,
92-
* eciesSingleHashMode [201] IMPLICIT NULL OPTIONAL,
93-
* includeUniqueId [202] IMPLICIT NULL OPTIONAL,
90+
* rsaPublicExponent [200] EXPLICIT INTEGER OPTIONAL,
91+
* eciesSingleHashMode [201] EXPLICIT NULL OPTIONAL,
92+
* includeUniqueId [202] EXPLICIT NULL OPTIONAL,
9493
* -- See keymaster_key_blob_usage_requirements for
9594
* -- blobUsageRequirement values.
96-
* blobUsageRequirement [301] IMPLICIT INTEGER OPTIONAL,
97-
* bootloaderOnly [302] IMPLICIT NULL OPTIONAL,
95+
* blobUsageRequirement [301] EXPLICIT INTEGER OPTIONAL,
96+
* bootloaderOnly [302] EXPLICIT NULL OPTIONAL,
9897
* -- activeDateTime must be a 64-bit Java date/time value.
99-
* activeDateTime [400] IMPLICIT INTEGER OPTIONAL
98+
* activeDateTime [400] EXPLICIT INTEGER OPTIONAL
10099
* -- originationExpireDateTime must be a 64-bit Java date/time
101100
* -- value.
102-
* originationExpireDateTime [401] IMPLICIT INTEGER OPTIONAL
101+
* originationExpireDateTime [401] EXPLICIT INTEGER OPTIONAL
103102
* -- usageExpireDateTime must be a 64-bit Java date/time value.
104-
* usageExpireDateTime [402] IMPLICIT INTEGER OPTIONAL
103+
* usageExpireDateTime [402] EXPLICIT INTEGER OPTIONAL
105104
* -- minSecondsBetweenOps must be non-negative and less than 2^32.
106-
* minSecondsBetweenOps [403] IMPLICIT INTEGER OPTIONAL,
105+
* minSecondsBetweenOps [403] EXPLICIT INTEGER OPTIONAL,
107106
* -- maxUsesPerBoot must be positive and less than 2^32.
108-
* maxUsesPerBoot [404] IMPLICIT INTEGER OPTIONAL,
109-
* noAuthRequired [503] IMPLICIT NULL OPTIONAL,
107+
* maxUsesPerBoot [404] EXPLICIT INTEGER OPTIONAL,
108+
* noAuthRequired [503] EXPLICIT NULL OPTIONAL,
110109
* -- See hw_authenticator_type_t for userAuthType values. Note
111110
* -- this field is a bitmask; multiple authenticator types may be
112111
* -- ORed together.
113-
* userAuthType [504] IMPLICIT INTEGER OPTIONAL,
112+
* userAuthType [504] EXPLICIT INTEGER OPTIONAL,
114113
* -- authTimeout, if present, must be positive and less than 2^32.
115-
* authTimeout [505] IMPLICIT INTEGER OPTIONAL,
116-
* allApplications [600] IMPLICIT NULL OPTIONAL,
117-
* applicationId [601] IMPLICIT OCTET_STRING OPTIONAL,
118-
* applicationData [700] IMPLICIT OCTET_STRING OPTIONAL,
114+
* authTimeout [505] EXPLICIT INTEGER OPTIONAL,
115+
* allApplications [600] EXPLICIT NULL OPTIONAL,
116+
* applicationId [601] EXPLICIT OCTET_STRING OPTIONAL,
117+
* applicationData [700] EXPLICIT OCTET_STRING OPTIONAL,
119118
* -- creationDateTime must be a 64-bit Java date/time value.
120-
* creationDateTime [701] IMPLICIT INTEGER OPTIONAL,
119+
* creationDateTime [701] EXPLICIT INTEGER OPTIONAL,
121120
* -- See keymaster_origin_t for origin values.
122-
* origin [702] IMPLICIT INTEGER OPTIONAL,
123-
* rollbackResistant [703] IMPLICIT NULL OPTIONAL,
121+
* origin [702] EXPLICIT INTEGER OPTIONAL,
122+
* rollbackResistant [703] EXPLICIT NULL OPTIONAL,
124123
* -- rootOfTrust is included only if bootloader is not locked.
125-
* rootOfTrust [704] IMPLICIT RootOfTrust OPTIONAL
126-
* osVersion [705] IMPLICIT INTEGER OPTIONAL,
127-
* patchLevel [706] IMPLICIT INTEGER OPTIONAL,
128-
* uniqueId [707] IMPLICIT NULL OPTIONAL,
124+
* rootOfTrust [704] EXPLICIT RootOfTrust OPTIONAL
125+
* osVersion [705] EXPLICIT INTEGER OPTIONAL,
126+
* patchLevel [706] EXPLICIT INTEGER OPTIONAL,
127+
* uniqueId [707] EXPLICIT NULL OPTIONAL,
129128
* }
130129
*
131130
* RootOfTrust ::= SEQUENCE {
@@ -152,15 +151,10 @@ public static AndroidKeyStoreAttestation Parse(X509Certificate cert)
152151
DLSequence softwareEnforcedSequence = getSoftwareEncodedSequence(keyDescriptionSequence);
153152
AuthorizationList softwareAuthorizationList =
154153
extractAuthorizationList(softwareEnforcedSequence);
154+
155+
// TODO(aczeskis) Extract the tee authorization list
155156

156-
// Get the unique id
157-
byte[] uniqueId = null;
158-
if (keyDescriptionSequence.size() == DESCRIPTION_LENGTH_MAX) {
159-
uniqueId = getUniqueId(keyDescriptionSequence);
160-
}
161-
162-
return new AndroidKeyStoreAttestation(
163-
keymasterVersion, challenge, softwareAuthorizationList, uniqueId);
157+
return new AndroidKeyStoreAttestation(keymasterVersion, challenge, softwareAuthorizationList);
164158
}
165159

166160
/**
@@ -177,13 +171,6 @@ public AuthorizationList getSoftwareAuthorizationList() {
177171
return softwareAuthorizationList;
178172
}
179173

180-
/**
181-
* @return the parsed unique id or {@code null} if none was included
182-
*/
183-
public byte[] getUniqueId() {
184-
return uniqueId;
185-
}
186-
187174
/**
188175
* @return the parsed attestation challenge
189176
*/
@@ -259,7 +246,7 @@ private static int getIntFromAsn1Encodable(ASN1Encodable asn1Encodable)
259246
}
260247
ASN1Integer asn1Integer = (ASN1Integer) asn1Encodable;
261248
BigInteger bigInt = asn1Integer.getPositiveValue();
262-
if (bigInt.bitLength() > MAX_INTEGER_BITS) {
249+
if (bigInt.bitLength() > MAX_INT_BITS) {
263250
throw new CertificateParsingException("INTEGER too big");
264251
}
265252
return bigInt.intValue();
@@ -307,12 +294,6 @@ private static byte[] getAttestationChallenge(DLSequence keyDescriptionSequence)
307294
return getByteArrayFromAsn1Encodable(asn1Encodable);
308295
}
309296

310-
private static byte[] getUniqueId(DLSequence keyDescriptionSequence)
311-
throws CertificateParsingException {
312-
ASN1Encodable asn1Encodable = keyDescriptionSequence.getObjectAt(DESCRIPTION_UNIQUE_INDEX);
313-
return getByteArrayFromAsn1Encodable(asn1Encodable);
314-
}
315-
316297
private static List<Purpose> getPurpose(
317298
HashMap<Integer, ASN1Primitive> softwareEnforcedTaggedObjects)
318299
throws CertificateParsingException {

‎u2f-ref-code/java/src/com/google/u2f/server/impl/androidattestation/Purpose.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
package com.google.u2f.server.impl.androidattestation;
22

33
import java.security.cert.CertificateParsingException;
4+
45
/**
5-
* Keysmaster purpose values as taken from: keymaster_defs.h
6+
* Keysmaster purpose values as taken from: keymaster_defs.h / KeymasterDefs.java
67
*/
78
public enum Purpose {
89
KM_PURPOSE_ENCRYPT(0, "encrypt"),

‎u2f-ref-code/java/tests/com/google/u2f/server/impl/androidattestation/AndroidKeyStoreAttestationTest.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,6 @@ public void testValidCert() throws Exception {
3232
assertArrayEquals(
3333
"Incorrect challenge", "challenge".getBytes(), attestation.getAttestationChallenge());
3434

35-
// Check unique id
36-
assertArrayEquals("Incorrect unique id", "non-unique ID".getBytes(), attestation.getUniqueId());
37-
3835
// Get software authz list
3936
AuthorizationList softwareAuthorizationList = attestation.getSoftwareAuthorizationList();
4037
assertNotNull("Not expecting null software authorization list", softwareAuthorizationList);

0 commit comments

Comments
 (0)