[BUG] (Android) Return/throw an error when an insufficient IPK is provided

[tnguyen-alarm]

Feature description

I was building out commissioning on Android and noticed that when I provided an insufficient IPK (less than 16 bytes, 4 bytes specifically) via onNOCChainGeneration(), commissioning would hang, but no errors would show up in Logcat. The last Logcat message that shows up is setNOCChain() called. It took a while to figure out that the IPK was the cause of the hanging because I was receiving it from the cloud and not generating one myself.

Just a guess, but I would have expected this method to return/throw an error:

connectedhomeip/src/controller/java/CHIPDeviceController-JNI.cpp

Line 165 in b274b9e

JNI_METHOD(jint, onNOCChainGeneration)

Platform

android

Platform Version(s)

1.1.0.1

Anything else?

No response

[pgatti86]

@tnguyen-alarm how do you fix the problem?

I'm using version 1.2.0.1 but the last log message is still setNOCChain()
My IPK is a random generated array of 16 bytes.
Thanks

[tnguyen-alarm]

@pgatti86 Are you using the same IPK between the commissioner + controller + accessory device? They need to all be the same for CASE to work.

[pgatti86]

@tnguyen-alarm Thanks for your reply.
I was not using the same ipk! After your suggestion it works. Thanks

[yunhanw-google]

@tnguyen-alarm
onNOCChainGeneration has returned the error, you could raise the exception in java side when error is not 0?

do you know which exact line is failing with insufficient IPK?

here?

connectedhomeip/src/controller/java/CHIPDeviceController-JNI.cpp

Line 225 in b274b9e

ipkOptional.SetValue(commissioningParams.GetIpk().Value());

[yunhanw-google]

@tnguyen-alarm any reply on this? thanks

[tnguyen-alarm]

@yunhanw-google Sorry for the extremely late reply. I haven't been working in this area of the code for a bit so it hasn't been as high of a priority.

onNOCChainGeneration has returned the error, you could raise the exception in java side when error is not 0?

Just to confirm, you're suggesting adding a new parameter to onNOCChainGeneration for the return code? If so, that would work.

do you know which exact line is failing with insufficient IPK?

I think this is the line that is failing:

connectedhomeip/src/controller/java/CHIPDeviceController-JNI.cpp

Line 217 in b274b9e

VerifyOrReturnValue(jByteArrayIpk.byteSpan().size() == sizeof(ipkValue), CHIP_ERROR_INTERNAL.AsInteger());

I provided an insufficient IPK (less than 16 bytes, 4 bytes specifically) via onNOCChainGeneration()

In my original example, my guess is that the Java byte array is padded with 0x00's and so it sneaks by the jByteArrayIpk.byteSpan().size() == sizeof(ipkValue) check.

[yunhanw-google]

@tnguyen-alarm thanks for the reply.
In my test, when I pass 15 byte IPK from java to jni, it would not auto-padding, and it would return error

connectedhomeip/src/controller/java/CHIPDeviceController-JNI.cpp

Line 231 in 87e6da0

VerifyOrReturnValue(jByteArrayIpk.byteSpan().size() == sizeof(ipkValue), CHIP_ERROR_INTERNAL.AsInteger());

And onNOCChainGeneration do return the jint error code to java, https://github.com/project-chip/connectedhomeip/blob/master/src/controller/java/CHIPDeviceController-JNI.cpp#L179C18-L179C38

is it possible that somehow your application pad 0x00 in your ipk?

CHIP_ERROR_INTERNAL.AsInteger() is not that easy to know what happens. Maybe we should return CHIP_ERROR_INVALID_IPK?

Thanks

[yunhanw-google]

https://github.com/project-chip/connectedhomeip/pull/31491/files