Skip to content

Commit

Permalink
Fix double-wrapping of IssuerSignedItem.
Browse files Browse the repository at this point in the history
The root problem is that both encodeStaticAuthData() and
decodeStaticAuthData() have a bug where IssuerSignedItem is wrapped in
a #6.24-tagged bstr twice.

For self-signed credentials provisioned on the device via
Utility.provisionSelfSignedCredential() this turned out to not be a
problem because the two bugs cancel each other out.

However for applications where the StaticAuthData (e.g. MSO and the
digest id mapping) is generated outside the device - the usual case
for production-level mDLs - this ends up being a problem because the
generated StaticAuthData CBOR fails to decode because of the bug in
decodeStaticAuthData().

Fix this by slightly reworking the API so IssuerSignedItem blobs are
passed around, not IssuedSignedItemBytes. Specifically this affects
the following public APIs:

 - Utility.mergeIssuerSigned()
 - Utility.decodeStaticAuthData()
 - Utility.encodeStaticAuthData()
 - DeviceResponseGenerator.addDocument()

This change won't affect the common path where the mdoc application is
just using DeviceResponseGenerator.addDocument() together with
Utility.decodeStaticAuthData().

Additionally, since this is a bug in how StaticAuthData is encoded and
this is stored on disk, you'll need to uninstall the reference app and
install it again to force creation of a new self-signed credential.

Test: New unit tests and all unit tests pass.
Test: Manually tested with reference apps.
  • Loading branch information
davidz25 committed Jul 11, 2022
1 parent 07f15d3 commit 0e74620
Show file tree
Hide file tree
Showing 5 changed files with 312 additions and 96 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,230 @@
/*
* Copyright 2022 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.android.identity;

import android.util.Pair;

import androidx.test.ext.junit.runners.AndroidJUnit4;
import androidx.test.filters.SmallTest;

import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;

import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import co.nstant.in.cbor.CborBuilder;
import co.nstant.in.cbor.builder.MapBuilder;
import co.nstant.in.cbor.model.DataItem;
import co.nstant.in.cbor.model.SimpleValue;
import co.nstant.in.cbor.model.UnicodeString;

@SuppressWarnings("deprecation")
@RunWith(AndroidJUnit4.class)
public class StaticAuthDataTest {

@Test
@SmallTest
public void testDecodeStaticAuthData() throws Exception {
/* Manually builds up CBOR that conforms to EXAMPLE_STATIC_AUTH_DATA_HEX
* defined in the bottom of this class. Then checks that decodeStaticAuthData()
* decodes it correctly.
*/

CborBuilder builder = new CborBuilder();
MapBuilder<CborBuilder> outerBuilder = builder.addMap();
outerBuilder.addKey("org.namespace");
outerBuilder.end();

DataItem issuerSignedItem = new CborBuilder()
.addMap()
.put("digestID", 42)
.put("random", new byte[] {0x50, 0x51, 0x52})
.put("elementIdentifier", "dataElementName")
.put(new UnicodeString("elementValue"), SimpleValue.NULL)
.end()
.build().get(0);
byte[] encodedIssuerSignedItem = Util.cborEncode(issuerSignedItem);

DataItem issuerSignedItem2 = new CborBuilder()
.addMap()
.put("digestID", 43)
.put("random", new byte[] {0x53, 0x54, 0x55})
.put("elementIdentifier", "dataElementName2")
.put(new UnicodeString("elementValue"), SimpleValue.NULL)
.end()
.build().get(0);
byte[] encodedIssuerSignedItem2 = Util.cborEncodeWithoutCanonicalizing(issuerSignedItem2);

DataItem digestIdMappingItem = new CborBuilder()
.addMap()
.putArray("org.namespace")
.add(Util.cborBuildTaggedByteString(encodedIssuerSignedItem))
.add(Util.cborBuildTaggedByteString(encodedIssuerSignedItem2))
.end()
.end()
.build().get(0);

byte[] staticAuthData = Util.cborEncode(new CborBuilder()
.addMap()
.put(new UnicodeString("digestIdMapping"), digestIdMappingItem)
// Make issuerAuth look like a valid COSE_Sign1
.putArray("issuerAuth") // IssuerAuth
.addArray().end()
.add(SimpleValue.NULL)
.add(new byte[] {0x01, 0x02})
.end()
.end()
.build().get(0));

Assert.assertEquals(EXAMPLE_STATIC_AUTH_DATA_HEX, Util.toHex(staticAuthData));

// Now check that decodeStaticAuthData() correctly decodes CBOR conforming to this CDDL
Pair<Map<String, List<byte[]>>, byte[]> val = Utility.decodeStaticAuthData(staticAuthData);
Map<String, List<byte[]>> digestIdMapping = val.first;
byte[] encodedStaticAuthData = val.second;

// Check that the IssuerSignedItem instances are correctly decoded and the order
// of the map matches what was put in above
Assert.assertEquals(1, digestIdMapping.size());
List<byte[]> list = digestIdMapping.get("org.namespace");
Assert.assertNotNull(list);
Assert.assertEquals(2, list.size());
Assert.assertEquals("{\n" +
" 'random' : [0x50, 0x51, 0x52],\n" +
" 'digestID' : 42,\n" +
" 'elementValue' : null,\n" +
" 'elementIdentifier' : 'dataElementName'\n" +
"}", Util.cborPrettyPrint(list.get(0)));
Assert.assertEquals("{\n" +
" 'digestID' : 43,\n" +
" 'random' : [0x53, 0x54, 0x55],\n" +
" 'elementIdentifier' : 'dataElementName2',\n" +
" 'elementValue' : null\n" +
"}", Util.cborPrettyPrint(list.get(1)));
}

@Test
@SmallTest
public void testEncodeStaticAuthData() throws Exception {
/* Uses encodeStaticAuthData() to build up CBOR that conforms to
* EXAMPLE_STATIC_AUTH_DATA_HEX defined in the bottom of this class.
* Then checks that the same bytes are produced.
*/

DataItem issuerSignedItem = new CborBuilder()
.addMap()
.put("digestID", 42)
.put("random", new byte[] {0x50, 0x51, 0x52})
.put("elementIdentifier", "dataElementName")
.put(new UnicodeString("elementValue"), SimpleValue.NULL)
.end()
.build().get(0);
byte[] encodedIssuerSignedItem = Util.cborEncode(issuerSignedItem);

DataItem issuerSignedItem2 = new CborBuilder()
.addMap()
.put("digestID", 43)
.put("random", new byte[] {0x53, 0x54, 0x55})
.put("elementIdentifier", "dataElementName2")
.put(new UnicodeString("elementValue"), SimpleValue.NULL)
.end()
.build().get(0);
byte[] encodedIssuerSignedItem2 = Util.cborEncodeWithoutCanonicalizing(issuerSignedItem2);

Map<String, List<byte[]>> issuerSignedMapping = new HashMap<>();
issuerSignedMapping.put("org.namespace",
Arrays.asList(encodedIssuerSignedItem, encodedIssuerSignedItem2));

byte[] encodedIssuerAuth = Util.cborEncode(new CborBuilder()
.addArray()
.addArray()
.end()
.add(SimpleValue.NULL)
.add(new byte[] {0x01, 0x02})
.end()
.build().get(0));

byte[] staticAuthData = Utility.encodeStaticAuthData(issuerSignedMapping, encodedIssuerAuth);

Assert.assertEquals(EXAMPLE_STATIC_AUTH_DATA_HEX, Util.toHex(staticAuthData));
}

// If you go to cbor.me with the hex value below you'll get
//
// {
// "issuerAuth": [[], null, << 1, 2 >>],
// "digestIdMapping": {
// "org.namespace": [
// 24(<< {
// "random": h'505152',
// "digestID": 42,
// "elementValue": null,
// "elementIdentifier": "dataElementName"
// } >>),
// 24(<< {
// "digestID": 43,
// "random": h'535455',
// "elementIdentifier": "dataElementName2",
// "elementValue": null
// } >>)
// ]
// }
// }
//
// which by inspection matches this CDDL:
//
// StaticAuthData = {
// "digestIdMapping": DigestIdMapping,
// "issuerAuth" : IssuerAuth
// }
//
// DigestIdMapping = {
// NameSpace =&gt; [ + IssuerSignedItemBytes ]
// }
//
// ; Defined in ISO 18013-5
// ;
// NameSpace = String
// DataElementIdentifier = String
// DigestID = uint
// IssuerAuth = COSE_Sign1 ; The payload is MobileSecurityObjectBytes
//
// IssuerSignedItemBytes = #6.24(bstr .cbor IssuerSignedItem)
//
// IssuerSignedItem = {
// "digestID" : uint, ; Digest ID for issuer data auth
// "random" : bstr, ; Random value for issuer data auth
// "elementIdentifier" : DataElementIdentifier, ; Data element identifier
// "elementValue" : NULL ; Placeholder for Data element value
// }
//
// TODO: We should add support for 'emb cbor' in Util.cborPrettyPrint() so we can just
// compare against a printed value instead of a hexdump.
//
private final String EXAMPLE_STATIC_AUTH_DATA_HEX =
"a26a697373756572417574688380f64201026f64696765737449644d61707069"
+ "6e67a16d6f72672e6e616d65737061636582d8185847a46672616e646f6d4350"
+ "5152686469676573744944182a6c656c656d656e7456616c7565f671656c656d"
+ "656e744964656e7469666965726f64617461456c656d656e744e616d65d81858"
+ "48a4686469676573744944182b6672616e646f6d4353545571656c656d656e74"
+ "4964656e7469666965727064617461456c656d656e744e616d65326c656c656d"
+ "656e7456616c7565f6";
}
13 changes: 3 additions & 10 deletions identity/src/androidTest/java/com/android/identity/UtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -614,10 +614,9 @@ public void replaceLineTest() {
// This test makes sure that Util.issuerSignedItemBytesSetValue() preserves the map order.
//
@Test
public void testIssuerSignedItemBytesSetValue() {
public void testIssuerSignedItemSetValue() {
DataItem di;
byte[] encoded;
byte[] encodedWithValueTagged;
byte[] encodedWithValue;
byte[] encodedDataElement = Util.cborEncodeString("A String");

Expand All @@ -639,9 +638,7 @@ public void testIssuerSignedItemBytesSetValue() {
" 'elementValue' : null,\n" +
" 'elementIdentifier' : 'foo'\n" +
"}", Util.cborPrettyPrint(encoded));
encodedWithValueTagged = Util.issuerSignedItemBytesSetValue(
Util.cborEncode(Util.cborBuildTaggedByteString(encoded)), encodedDataElement);
encodedWithValue = Util.cborExtractTaggedCbor(encodedWithValueTagged);
encodedWithValue = Util.issuerSignedItemSetValue(encoded, encodedDataElement);
assertEquals("{\n" +
" 'random' : [0x01, 0x02, 0x03],\n" +
" 'digestID' : 42,\n" +
Expand All @@ -665,17 +662,13 @@ public void testIssuerSignedItemBytesSetValue() {
" 'elementIdentifier' : 'foo',\n" +
" 'elementValue' : null\n" +
"}", Util.cborPrettyPrint(encoded));
encodedWithValueTagged = Util.issuerSignedItemBytesSetValue(
Util.cborEncode(Util.cborBuildTaggedByteString(encoded)), encodedDataElement);
encodedWithValue = Util.cborExtractTaggedCbor(encodedWithValueTagged);
encodedWithValue = Util.issuerSignedItemSetValue(encoded, encodedDataElement);
assertEquals("{\n" +
" 'digestID' : 42,\n" +
" 'random' : [0x01, 0x02, 0x03],\n" +
" 'elementIdentifier' : 'foo',\n" +
" 'elementValue' : 'A String'\n" +
"}", Util.cborPrettyPrint(encodedWithValue));


}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ public DeviceResponseGenerator(@Constants.DeviceResponseStatus long statusCode)
* Adds a new document to the device response.
*
* <p>Issuer-signed data is provided in <code>issuerSignedData</code> which
* maps from namespaces into a list of bytes of IssuerSignedItemBytes CBOR where
* each encoded CBOR blob contains the digest-id, element name, issuer-generated
* random value and finally the element value. These IssuerSignedItemBytes must
* be encoded so their digest match with the digests in the
* <code>MobileSecurityObject</code> in the <code>issuerAuth</code> parameter.
* maps from namespaces into a list of bytes of IssuerSignedItem CBOR as
* defined in 18013-5 where each contains the digest-id, element name,
* issuer-generated random value and finally the element value. Each IssuerSignedItem
* must be encoded so the digest of them in a #6.24 bstr matches with the digests in
* the <code>MobileSecurityObject</code> in the <code>issuerAuth</code> parameter.
*
* <p>The <code>encodedIssuerAuth</code> parameter contains the bytes of the
* <code>IssuerAuth</code> CBOR as defined in <em>ISO/IEC 18013-5</em>
Expand Down Expand Up @@ -100,9 +100,9 @@ public DeviceResponseGenerator(@Constants.DeviceResponseStatus long statusCode)
MapBuilder<CborBuilder> insOuter = issuerNameSpacesBuilder.addMap();
for (String ns : issuerSignedData.keySet()) {
ArrayBuilder<MapBuilder<CborBuilder>> insInner = insOuter.putArray(ns);
for (byte[] encodedIssuerSignedItemBytes : issuerSignedData.get(ns)) {
DataItem issuerSignedItemBytes = Util.cborDecode(encodedIssuerSignedItemBytes);
insInner.add(issuerSignedItemBytes);
for (byte[] encodedIssuerSignedItem : issuerSignedData.get(ns)) {
// We'll do the #6.24 wrapping here.
insInner.add(Util.cborBuildTaggedByteString(encodedIssuerSignedItem));
}
insInner.end();
}
Expand Down Expand Up @@ -151,11 +151,15 @@ public DeviceResponseGenerator(@Constants.DeviceResponseStatus long statusCode)

/**
* Like {@link #addDocument(String, byte[], byte[], byte[], Map, byte[])} but takes a
* {@link CredentialDataResult} instead.
* {@link CredentialDataResult} instead and merges the results into the "elementValue"
* entry of each IssuerSignedItem value.
*
* <p>Note: The <code>issuerSignedData</code> and <code>encodedIssuerAuth</code> are
* parameters usually obtained via {@link Utility#decodeStaticAuthData(byte[])}.
*
* @param docType The type of the document to send.
* @param credentialDataResult The device- and issuer-signed data elements to include.
* @param issuerSignedMapping A mapping from namespaces to an array of IssuerSignedItemBytes
* @param issuerSignedMapping A mapping from namespaces to an array of IssuerSignedItem
* CBOR for the namespace. The "elementValue" value in each
* IssuerSignedItem CBOR must be set to the NULL value.
* @param encodedIssuerAuth the bytes of <code>COSE_Sign1</code> signed by the issuing
Expand Down
52 changes: 13 additions & 39 deletions identity/src/main/java/com/android/identity/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -1464,64 +1464,38 @@ byte[] getPopSha256FromAuthKeyCert(@NonNull X509Certificate cert) {
}
}

static @NonNull
DataItem calcIssuerSignedItemBytes(long digestID,
@NonNull byte[] random,
@NonNull String elementIdentifier,
@NonNull DataItem elementValue) {
DataItem issuerSignedItem = new CborBuilder()
.addMap()
.put("digestID", digestID)
.put("random", random)
.put("elementIdentifier", elementIdentifier)
.put(new UnicodeString("elementValue"), elementValue)
.end()
.build().get(0);
DataItem issuerSignedItemBytes = Util.cborBuildTaggedByteString(
Util.cborEncode(issuerSignedItem));
return issuerSignedItemBytes;
}

/**
* @param encodedIssuerSignedItemBytes encoded CBOR conforming to IssuerSignedItemBytes.
* @return Same as given CBOR but with elementValue set to NULL.
*
* Clears elementValue in IssuerSignedItemBytes CBOR.
*
* Throws if the given encodedIssuerSignedItemBytes isn't IssuersignedItemBytes.
* @param encodedIssuerSignedItem encoded CBOR conforming to IssuerSignedItem.
* @return Same as given CBOR but with elementValue set to NULL.
*/
static @NonNull
byte[] issuerSignedItemBytesClearValue(
@NonNull byte[] encodedIssuerSignedItemBytes) {
byte[] issuerSignedItemClearValue(@NonNull byte[] encodedIssuerSignedItem) {
byte[] encodedNullValue = Util.cborEncode(SimpleValue.NULL);
return issuerSignedItemBytesSetValue(encodedIssuerSignedItemBytes, encodedNullValue);
return issuerSignedItemSetValue(encodedIssuerSignedItem, encodedNullValue);
}

/**
* @param encodedIssuerSignedItemBytes encoded CBOR conforming to IssuerSignedItemBytes.
* @param encodedElementValue the value to set elementValue to.
* @return Same as given CBOR but with elementValue set to given value.
*
* Sets elementValue in IssuerSignedItemBytes CBOR.
* Sets elementValue in IssuerSignedItem CBOR.
*
* Throws if the given encodedIssuerSignedItemBytes isn't IssuersignedItemBytes.
*
* @param encodedIssuerSignedItem encoded CBOR conforming to IssuerSignedItem.
* @param encodedElementValue the value to set elementValue to.
* @return Same as given CBOR but with elementValue set to given value.
*/
static @NonNull
byte[] issuerSignedItemBytesSetValue(
@NonNull byte[] encodedIssuerSignedItemBytes,
byte[] issuerSignedItemSetValue(
@NonNull byte[] encodedIssuerSignedItem,
@NonNull byte[] encodedElementValue) {
DataItem issuerSignedItemBytes = Util.cborDecode(encodedIssuerSignedItemBytes);
DataItem issuerSignedItemElem =
Util.cborExtractTaggedAndEncodedCbor(issuerSignedItemBytes);
DataItem issuerSignedItemElem = Util.cborDecode(encodedIssuerSignedItem);
Map issuerSignedItem = castTo(Map.class, issuerSignedItemElem);
DataItem elementValue = Util.cborDecode(encodedElementValue);
issuerSignedItem.put(new UnicodeString("elementValue"), elementValue);

// By using the non-canonical encoder the order is preserved.
DataItem newIssuerSignedItemBytes = Util.cborBuildTaggedByteString(
Util.cborEncodeWithoutCanonicalizing(issuerSignedItem));

return Util.cborEncode(newIssuerSignedItemBytes);
return Util.cborEncodeWithoutCanonicalizing(issuerSignedItem);
}

static @NonNull
Expand Down
Loading

0 comments on commit 0e74620

Please sign in to comment.