From 33579637aa1a51c32e6b523fa327ea0c3876483b Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Thu, 8 Sep 2022 01:23:08 -0400 Subject: [PATCH] Allow null ICAC in `AndroidDeviceControllerWrapper::AllocateNew` (#22458) * Allow null ICAC in `AndroidDeviceControllerWrapper::AllocateNew` - Passing null for existing ICAC, when a NOC and RCAC were provided and desired to be used casued confusing behavior of NOC/RCAC being ignored and a completely different cert chain being generated. Fixes #22455 Changes done: - Add code to just use empty internal ICAC when null is provided in Java - Add log to tell users when an ephemeral NOC chain is generated. Testing done: - CI still passes - Tested in unit/integration with our internal infrastructure that uses this API both with/without ICAC * Restyled --- .../java/AndroidDeviceControllerWrapper.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/controller/java/AndroidDeviceControllerWrapper.cpp b/src/controller/java/AndroidDeviceControllerWrapper.cpp index dc223d989859df..f737e9bae81a34 100644 --- a/src/controller/java/AndroidDeviceControllerWrapper.cpp +++ b/src/controller/java/AndroidDeviceControllerWrapper.cpp @@ -209,8 +209,7 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew( // The lifetime of the ephemeralKey variable must be kept until SetupParams is saved. Crypto::P256Keypair ephemeralKey; - if (rootCertificate != nullptr && intermediateCertificate != nullptr && nodeOperationalCertificate != nullptr && - keypairDelegate != nullptr) + if (rootCertificate != nullptr && nodeOperationalCertificate != nullptr && keypairDelegate != nullptr) { CHIPP256KeypairBridge * nativeKeypairBridge = wrapper->GetP256KeypairBridge(); nativeKeypairBridge->SetDelegate(keypairDelegate); @@ -224,21 +223,20 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew( setupParams.hasExternallyOwnedOperationalKeypair = true; JniByteArray jniRcac(env, rootCertificate); - JniByteArray jniIcac(env, intermediateCertificate); JniByteArray jniNoc(env, nodeOperationalCertificate); // Make copies of the cert that outlive the scope so that future factor init does not // cause loss of scope from the JNI refs going away. Also, this keeps the certs // handy for debugging commissioner init. wrapper->mRcacCertificate = std::vector(jniRcac.byteSpan().begin(), jniRcac.byteSpan().end()); - if (!jniIcac.byteSpan().empty()) + + // Intermediate cert could be missing. Let's only copy it if present + wrapper->mIcacCertificate.clear(); + if (intermediateCertificate != nullptr) { + JniByteArray jniIcac(env, intermediateCertificate); wrapper->mIcacCertificate = std::vector(jniIcac.byteSpan().begin(), jniIcac.byteSpan().end()); } - else - { - wrapper->mIcacCertificate.clear(); - } wrapper->mNocCertificate = std::vector(jniNoc.byteSpan().begin(), jniNoc.byteSpan().end()); @@ -248,6 +246,9 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew( } else { + ChipLogProgress(Controller, + "No existing credentials provided: generating ephemeral local NOC chain with OperationalCredentialsIssuer"); + *errInfoOnFailure = ephemeralKey.Initialize(); if (*errInfoOnFailure != CHIP_NO_ERROR) { @@ -289,7 +290,7 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew( { return nullptr; } - ChipLogProgress(Support, "Setting up group data for Fabric Index %u with Compressed Fabric ID:", + ChipLogProgress(Controller, "Setting up group data for Fabric Index %u with Compressed Fabric ID:", static_cast(wrapper->Controller()->GetFabricIndex())); ChipLogByteSpan(Support, compressedFabricIdSpan);