Skip to content

Commit

Permalink
Updated PASE PasscodeId to Always be 0 to Match Spec (#15924)
Browse files Browse the repository at this point in the history
-- Removed PasscodeId from all APIs.
  -- When opening an ECM window, the passcodeID to use in PASE PBKDFParamRequest is always 0.
  -- Non-zero values of passcodeID in PASE PBKDFParamRequest are forbidden.
  -- Removed PasscodeId field in OpenCommissioningWindow command needs.
  • Loading branch information
emargolis authored and pull[bot] committed Jul 17, 2023
1 parent d587b35 commit 56ac593
Show file tree
Hide file tree
Showing 39 changed files with 56 additions and 142 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ server cluster AdministratorCommissioning = 60 {
INT16U discriminator = 2;
INT32U iterations = 3;
OCTET_STRING salt = 4;
INT16U passcodeID = 5;
}

timed command OpenBasicCommissioningWindow(OpenBasicCommissioningWindowRequest): DefaultSuccess = 1;
Expand Down
1 change: 0 additions & 1 deletion examples/bridge-app/bridge-common/bridge-app.matter
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ server cluster AdministratorCommissioning = 60 {
INT16U discriminator = 2;
INT32U iterations = 3;
OCTET_STRING salt = 4;
INT16U passcodeID = 5;
}

timed command OpenBasicCommissioningWindow(OpenBasicCommissioningWindowRequest): DefaultSuccess = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ server cluster AdministratorCommissioning = 60 {
INT16U discriminator = 2;
INT32U iterations = 3;
OCTET_STRING salt = 4;
INT16U passcodeID = 5;
}

timed command OpenBasicCommissioningWindow(OpenBasicCommissioningWindowRequest): DefaultSuccess = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ server cluster AdministratorCommissioning = 60 {
INT16U discriminator = 2;
INT32U iterations = 3;
OCTET_STRING salt = 4;
INT16U passcodeID = 5;
}

timed command OpenBasicCommissioningWindow(OpenBasicCommissioningWindowRequest): DefaultSuccess = 1;
Expand Down
1 change: 0 additions & 1 deletion examples/lighting-app/lighting-common/lighting-app.matter
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ server cluster AdministratorCommissioning = 60 {
INT16U discriminator = 2;
INT32U iterations = 3;
OCTET_STRING salt = 4;
INT16U passcodeID = 5;
}

timed command OpenBasicCommissioningWindow(OpenBasicCommissioningWindowRequest): DefaultSuccess = 1;
Expand Down
1 change: 0 additions & 1 deletion examples/lock-app/lock-common/lock-app.matter
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ server cluster AdministratorCommissioning = 60 {
INT16U discriminator = 2;
INT32U iterations = 3;
OCTET_STRING salt = 4;
INT16U passcodeID = 5;
}

timed command OpenBasicCommissioningWindow(OpenBasicCommissioningWindowRequest): DefaultSuccess = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ server cluster AdministratorCommissioning = 60 {
INT16U discriminator = 2;
INT32U iterations = 3;
OCTET_STRING salt = 4;
INT16U passcodeID = 5;
}

timed command OpenBasicCommissioningWindow(OpenBasicCommissioningWindowRequest): DefaultSuccess = 1;
Expand Down
1 change: 0 additions & 1 deletion examples/pump-app/pump-common/pump-app.matter
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ server cluster AdministratorCommissioning = 60 {
INT16U discriminator = 2;
INT32U iterations = 3;
OCTET_STRING salt = 4;
INT16U passcodeID = 5;
}

timed command OpenBasicCommissioningWindow(OpenBasicCommissioningWindowRequest): DefaultSuccess = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ server cluster AdministratorCommissioning = 60 {
INT16U discriminator = 2;
INT32U iterations = 3;
OCTET_STRING salt = 4;
INT16U passcodeID = 5;
}

timed command OpenBasicCommissioningWindow(OpenBasicCommissioningWindowRequest): DefaultSuccess = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ server cluster AdministratorCommissioning = 60 {
INT16U discriminator = 2;
INT32U iterations = 3;
OCTET_STRING salt = 4;
INT16U passcodeID = 5;
}

timed command OpenBasicCommissioningWindow(OpenBasicCommissioningWindowRequest): DefaultSuccess = 1;
Expand Down
1 change: 0 additions & 1 deletion examples/thermostat/thermostat-common/thermostat.matter
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ server cluster AdministratorCommissioning = 60 {
INT16U discriminator = 2;
INT32U iterations = 3;
OCTET_STRING salt = 4;
INT16U passcodeID = 5;
}

timed command OpenBasicCommissioningWindow(OpenBasicCommissioningWindowRequest): DefaultSuccess = 1;
Expand Down
1 change: 0 additions & 1 deletion examples/tv-app/tv-common/tv-app.matter
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ server cluster AdministratorCommissioning = 60 {
INT16U discriminator = 2;
INT32U iterations = 3;
OCTET_STRING salt = 4;
INT16U passcodeID = 5;
}

timed command OpenBasicCommissioningWindow(OpenBasicCommissioningWindowRequest): DefaultSuccess = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ server cluster AdministratorCommissioning = 60 {
INT16U discriminator = 2;
INT32U iterations = 3;
OCTET_STRING salt = 4;
INT16U passcodeID = 5;
}

timed command OpenBasicCommissioningWindow(OpenBasicCommissioningWindowRequest): DefaultSuccess = 1;
Expand Down
1 change: 0 additions & 1 deletion examples/window-app/common/window-app.matter
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ server cluster AdministratorCommissioning = 60 {
INT16U discriminator = 2;
INT32U iterations = 3;
OCTET_STRING salt = 4;
INT16U passcodeID = 5;
}

timed command OpenBasicCommissioningWindow(OpenBasicCommissioningWindowRequest): DefaultSuccess = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback(
auto & discriminator = commandData.discriminator;
auto & iterations = commandData.iterations;
auto & salt = commandData.salt;
auto & passcodeID = commandData.passcodeID;

Optional<StatusCode> status = Optional<StatusCode>::Missing();
Spake2pVerifier verifier;
Expand Down Expand Up @@ -124,7 +123,7 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback(
VerifyOrExit(verifier.Deserialize(pakeVerifier) == CHIP_NO_ERROR,
status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR));
VerifyOrExit(Server::GetInstance().GetCommissioningWindowManager().OpenEnhancedCommissioningWindow(
commissioningTimeout, discriminator, verifier, iterations, salt, passcodeID) == CHIP_NO_ERROR,
commissioningTimeout, discriminator, verifier, iterations, salt) == CHIP_NO_ERROR,
status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR));
ChipLogProgress(Zcl, "Commissioning window is now open");

Expand Down
14 changes: 6 additions & 8 deletions src/app/server/CommissioningWindowManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ void CommissioningWindowManager::ResetState()
mUseECM = false;

mECMDiscriminator = 0;
mECMPasscodeID = 0;
mECMIterations = 0;
mECMSaltLength = 0;
mWindowStatus = app::Clusters::AdministratorCommissioning::CommissioningWindowStatus::kWindowNotOpen;
Expand Down Expand Up @@ -183,8 +182,8 @@ CHIP_ERROR CommissioningWindowManager::OpenCommissioningWindow()
{
ReturnErrorOnFailure(SetTemporaryDiscriminator(mECMDiscriminator));
ReturnErrorOnFailure(
mPairingSession.WaitForPairing(mECMPASEVerifier, mECMIterations, ByteSpan(mECMSalt, mECMSaltLength), mECMPasscodeID,
keyID, Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()), this));
mPairingSession.WaitForPairing(mECMPASEVerifier, mECMIterations, ByteSpan(mECMSalt, mECMSaltLength), keyID,
Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()), this));
}
else
{
Expand All @@ -202,9 +201,9 @@ CHIP_ERROR CommissioningWindowManager::OpenCommissioningWindow()
VerifyOrReturnError(kSpake2p_VerifierSerialized_Length == serializedVerifierLen, CHIP_ERROR_INVALID_ARGUMENT);
ReturnErrorOnFailure(verifier.Deserialize(ByteSpan(serializedVerifier)));

ReturnErrorOnFailure(
mPairingSession.WaitForPairing(verifier, iterationCount, ByteSpan(salt, saltLen), kDefaultCommissioningPasscodeId,
keyID, Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()), this));
ReturnErrorOnFailure(mPairingSession.WaitForPairing(verifier, iterationCount, ByteSpan(salt, saltLen), keyID,
Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()),
this));
}

ReturnErrorOnFailure(StartAdvertisement());
Expand Down Expand Up @@ -241,7 +240,7 @@ CHIP_ERROR CommissioningWindowManager::OpenBasicCommissioningWindow(uint16_t com

CHIP_ERROR CommissioningWindowManager::OpenEnhancedCommissioningWindow(uint16_t commissioningTimeoutSeconds, uint16_t discriminator,
Spake2pVerifier & verifier, uint32_t iterations,
ByteSpan salt, PasscodeId passcodeID)
ByteSpan salt)
{
// Once a device is operational, it shall be commissioned into subsequent fabrics using
// the operational network only.
Expand All @@ -256,7 +255,6 @@ CHIP_ERROR CommissioningWindowManager::OpenEnhancedCommissioningWindow(uint16_t
mCommissioningTimeoutSeconds = commissioningTimeoutSeconds;

mECMDiscriminator = discriminator;
mECMPasscodeID = passcodeID;
mECMIterations = iterations;

memcpy(&mECMPASEVerifier, &verifier, sizeof(Spake2pVerifier));
Expand Down
4 changes: 1 addition & 3 deletions src/app/server/CommissioningWindowManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ class CommissioningWindowManager : public SessionEstablishmentDelegate, public a
CommissioningWindowAdvertisement advertisementMode = chip::CommissioningWindowAdvertisement::kAllSupported);

CHIP_ERROR OpenEnhancedCommissioningWindow(uint16_t commissioningTimeoutSeconds, uint16_t discriminator,
Spake2pVerifier & verifier, uint32_t iterations, chip::ByteSpan salt,
PasscodeId passcodeID);
Spake2pVerifier & verifier, uint32_t iterations, chip::ByteSpan salt);

void CloseCommissioningWindow();

Expand Down Expand Up @@ -113,7 +112,6 @@ class CommissioningWindowManager : public SessionEstablishmentDelegate, public a
bool mUseECM = false;
Spake2pVerifier mECMPASEVerifier;
uint16_t mECMDiscriminator = 0;
PasscodeId mECMPasscodeID = kDefaultCommissioningPasscodeId;
// mListeningForPASE is true only when we are listening for
// PBKDFParamRequest messages.
bool mListeningForPASE = false;
Expand Down
4 changes: 1 addition & 3 deletions src/app/tests/TestCommissionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,9 @@ void CheckCommissioningWindowManagerEnhancedWindowTask(intptr_t context)
constexpr uint32_t kIterations = chip::kSpake2p_Min_PBKDF_Iterations;
uint8_t salt[chip::kSpake2p_Min_PBKDF_Salt_Length];
chip::ByteSpan saltData(salt);
constexpr chip::PasscodeId kPasscodeID = 1;
uint16_t currentDiscriminator;

err = commissionMgr.OpenEnhancedCommissioningWindow(kNoCommissioningTimeout, newDiscriminator, verifier, kIterations, saltData,
kPasscodeID);
err = commissionMgr.OpenEnhancedCommissioningWindow(kNoCommissioningTimeout, newDiscriminator, verifier, kIterations, saltData);
NL_TEST_ASSERT(suite, err == CHIP_NO_ERROR);
NL_TEST_ASSERT(suite,
commissionMgr.CommissioningWindowStatus() ==
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ limitations under the License.
<arg name="Discriminator" type="INT16U"/>
<arg name="Iterations" type="INT32U"/>
<arg name="Salt" type="OCTET_STRING"/>
<arg name="PasscodeID" type="INT16U"/>
</command>

<command source="client" code="0x01" name="OpenBasicCommissioningWindow" mustUseTimedInvoke="true" optional="false">
Expand Down
7 changes: 2 additions & 5 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -444,11 +444,10 @@ void DeviceController::OnOpenPairingWindowFailureResponse(void * context, CHIP_E
}

CHIP_ERROR DeviceController::ComputePASEVerifier(uint32_t iterations, uint32_t setupPincode, const ByteSpan & salt,
Spake2pVerifier & outVerifier, PasscodeId & outPasscodeId)
Spake2pVerifier & outVerifier)
{
ReturnErrorOnFailure(PASESession::GeneratePASEVerifier(outVerifier, iterations, salt, /* useRandomPIN= */ false, setupPincode));

outPasscodeId = mPAKEVerifierID++;
return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -531,7 +530,6 @@ CHIP_ERROR DeviceController::OpenCommissioningWindowInternal()
request.discriminator = mSetupPayload.discriminator;
request.iterations = mCommissioningWindowIteration;
request.salt = salt;
request.passcodeID = mPAKEVerifierID++;

// TODO: What should the timed invoke timeout here be?
uint16_t timedInvokeTimeoutMs = 10000;
Expand Down Expand Up @@ -847,8 +845,7 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re
exchangeCtxt = mSystemState->ExchangeMgr()->NewContext(session.Value(), &device->GetPairing());
VerifyOrExit(exchangeCtxt != nullptr, err = CHIP_ERROR_INTERNAL);

// TODO: Need to determine how PasscodeId is provided for a non-default case. i.e. ECM
err = device->GetPairing().Pair(params.GetPeerAddress(), params.GetSetupPINCode(), kDefaultCommissioningPasscodeId, keyID,
err = device->GetPairing().Pair(params.GetPeerAddress(), params.GetSetupPINCode(), keyID,
Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()), exchangeCtxt, this);
SuccessOrExit(err);

Expand Down
6 changes: 2 additions & 4 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,11 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate,
* @param[in] setupPincode The desired PIN code to use
* @param[in] salt The 16-byte salt for verifier computation
* @param[out] outVerifier The Spake2pVerifier to be populated on success
* @param[out] outPasscodeId The passcode ID to be populated on success
*
* @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error
*/
CHIP_ERROR ComputePASEVerifier(uint32_t iterations, uint32_t setupPincode, const ByteSpan & salt, Spake2pVerifier & outVerifier,
PasscodeId & outPasscodeId);
CHIP_ERROR ComputePASEVerifier(uint32_t iterations, uint32_t setupPincode, const ByteSpan & salt,
Spake2pVerifier & outVerifier);

/**
* @brief
Expand Down Expand Up @@ -413,7 +412,6 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate,
static void OnOpenPairingWindowFailureResponse(void * context, CHIP_ERROR error);

CHIP_ERROR ProcessControllerNOCChain(const ControllerInitParams & params);
PasscodeId mPAKEVerifierID = 1;
};

/**
Expand Down
1 change: 0 additions & 1 deletion src/controller/data_model/controller-clusters.matter
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ client cluster AdministratorCommissioning = 60 {
INT16U discriminator = 2;
INT32U iterations = 3;
OCTET_STRING salt = 4;
INT16U passcodeID = 5;
}

timed command OpenBasicCommissioningWindow(OpenBasicCommissioningWindowRequest): DefaultSuccess = 1;
Expand Down
12 changes: 5 additions & 7 deletions src/controller/java/CHIPDeviceController-JNI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ using namespace chip::Controller;
#define CDC_JNI_CALLBACK_LOCAL_REF_COUNT 256

static void * IOThreadMain(void * arg);
static CHIP_ERROR N2J_PaseVerifierParams(JNIEnv * env, jlong setupPincode, jint passcodeId, jbyteArray pakeVerifier,
jobject & outParams);
static CHIP_ERROR N2J_PaseVerifierParams(JNIEnv * env, jlong setupPincode, jbyteArray pakeVerifier, jobject & outParams);
static CHIP_ERROR N2J_NetworkLocation(JNIEnv * env, jstring ipAddress, jint port, jobject & outLocation);
static CHIP_ERROR GetChipPathIdValue(jobject chipPathId, uint32_t wildcardValue, uint32_t & outValue);
static CHIP_ERROR ParseAttributePathList(jobject attributePathList,
Expand Down Expand Up @@ -668,7 +667,6 @@ JNI_METHOD(jobject, computePaseVerifier)
CHIP_ERROR err = CHIP_NO_ERROR;
jobject params;
jbyteArray verifierBytes;
PasscodeId passcodeId;
Spake2pVerifier verifier;
Spake2pVerifierSerialized serializedVerifier;
MutableByteSpan serializedVerifierSpan(serializedVerifier);
Expand All @@ -677,7 +675,7 @@ JNI_METHOD(jobject, computePaseVerifier)
ChipLogProgress(Controller, "computePaseVerifier() called");

AndroidDeviceControllerWrapper * wrapper = AndroidDeviceControllerWrapper::FromJNIHandle(handle);
err = wrapper->Controller()->ComputePASEVerifier(iterations, setupPincode, jniSalt.byteSpan(), verifier, passcodeId);
err = wrapper->Controller()->ComputePASEVerifier(iterations, setupPincode, jniSalt.byteSpan(), verifier);
SuccessOrExit(err);

err = verifier.Serialize(serializedVerifierSpan);
Expand All @@ -686,7 +684,7 @@ JNI_METHOD(jobject, computePaseVerifier)
err = JniReferences::GetInstance().N2J_ByteArray(env, serializedVerifier, kSpake2p_VerifierSerialized_Length, verifierBytes);
SuccessOrExit(err);

err = N2J_PaseVerifierParams(env, setupPincode, static_cast<jlong>(passcodeId), verifierBytes, params);
err = N2J_PaseVerifierParams(env, setupPincode, verifierBytes, params);
SuccessOrExit(err);
return params;
exit:
Expand Down Expand Up @@ -908,7 +906,7 @@ void * IOThreadMain(void * arg)
return NULL;
}

CHIP_ERROR N2J_PaseVerifierParams(JNIEnv * env, jlong setupPincode, jint passcodeId, jbyteArray paseVerifier, jobject & outParams)
CHIP_ERROR N2J_PaseVerifierParams(JNIEnv * env, jlong setupPincode, jbyteArray paseVerifier, jobject & outParams)
{
CHIP_ERROR err = CHIP_NO_ERROR;
jmethodID constructor;
Expand All @@ -922,7 +920,7 @@ CHIP_ERROR N2J_PaseVerifierParams(JNIEnv * env, jlong setupPincode, jint passcod
constructor = env->GetMethodID(paramsClass, "<init>", "(JI[B)V");
VerifyOrExit(constructor != nullptr, err = CHIP_JNI_ERROR_METHOD_NOT_FOUND);

outParams = (jobject) env->NewObject(paramsClass, constructor, setupPincode, passcodeId, paseVerifier);
outParams = (jobject) env->NewObject(paramsClass, constructor, setupPincode, paseVerifier);

VerifyOrExit(!env->ExceptionCheck(), err = CHIP_JNI_ERROR_EXCEPTION_THROWN);
exit:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ public void readPath(
public native byte[] convertX509CertToMatterCert(byte[] x509Cert);

/**
* Generates a new PASE verifier and passcode ID for the given setup PIN code.
* Generates a new PASE verifier for the given setup PIN code.
*
* @param devicePtr a pointer to the device object for which to generate the PASE verifier
* @param setupPincode the PIN code to use
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,16 @@
public final class PaseVerifierParams {

private final long setupPincode;
private final int passcodeId;
private final byte[] pakeVerifier;

/**
* Constructor
*
* @param setupPincode the PIN code associated with this verifier
* @param passcodeId the passcode ID for this generated verifier
* @param pakeVerifier the encoded verifier (concatenation of w0 and L)
*/
public PaseVerifierParams(long setupPincode, int passcodeId, byte[] pakeVerifier) {
public PaseVerifierParams(long setupPincode, byte[] pakeVerifier) {
this.setupPincode = setupPincode;
this.passcodeId = passcodeId;
this.pakeVerifier = pakeVerifier.clone();
}

Expand All @@ -28,11 +25,6 @@ public long getSetupPincode() {
return setupPincode;
}

/** Returns the passcode ID for this generated verifier. */
public int getPasscodeId() {
return passcodeId;
}

/**
* Returns the encoded PAKE verifier (the concatenation of w0 and L, as described in section 3.10
* (PAKE) of the Matter specification).
Expand All @@ -49,21 +41,14 @@ public boolean equals(Object other) {
return false;
} else {
PaseVerifierParams that = (PaseVerifierParams) other;
return setupPincode == that.setupPincode
&& passcodeId == that.passcodeId
&& Arrays.equals(pakeVerifier, that.pakeVerifier);
return setupPincode == that.setupPincode && Arrays.equals(pakeVerifier, that.pakeVerifier);
}
}

@Override
public int hashCode() {
int result = Objects.hash(setupPincode, passcodeId);
int result = Objects.hash(setupPincode);
result = 31 * result + Arrays.hashCode(pakeVerifier);
return result;
}

@Override
public String toString() {
return "PaseVerifierParams{ passcodeId=" + passcodeId + " }";
}
}
Loading

0 comments on commit 56ac593

Please sign in to comment.