Skip to content

Commit

Permalink
Allow null ICAC in AndroidDeviceControllerWrapper::AllocateNew (#22458
Browse files Browse the repository at this point in the history
)

* 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
  • Loading branch information
tcarmelveilleux authored and pull[bot] committed Jul 18, 2023
1 parent 9e248e1 commit 3357963
Showing 1 changed file with 10 additions and 9 deletions.
19 changes: 10 additions & 9 deletions src/controller/java/AndroidDeviceControllerWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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<uint8_t>(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<uint8_t>(jniIcac.byteSpan().begin(), jniIcac.byteSpan().end());
}
else
{
wrapper->mIcacCertificate.clear();
}

wrapper->mNocCertificate = std::vector<uint8_t>(jniNoc.byteSpan().begin(), jniNoc.byteSpan().end());

Expand All @@ -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)
{
Expand Down Expand Up @@ -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<unsigned>(wrapper->Controller()->GetFabricIndex()));
ChipLogByteSpan(Support, compressedFabricIdSpan);

Expand Down

0 comments on commit 3357963

Please sign in to comment.