Skip to content

Commit

Permalink
Add a helper to generate valid random PINs to the Darwin framework. (#…
Browse files Browse the repository at this point in the history
…20703)

Adds a public SDK API to test whether a PIN is valid, and uses it in
the Darwin framework and in CommissioningWindowOpener so we don't try
to open commissioning windows for invalid PINs.

Fixes #20662
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Feb 12, 2024
1 parent 008d93c commit 1164035
Show file tree
Hide file tree
Showing 10 changed files with 58 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
pairingCode = [controller openPairingWindowWithPIN:mNodeId
duration:mCommissioningWindowTimeoutMs
discriminator:mDiscriminator
setupPIN:arc4random()
setupPIN:[MTRSetupPayload generateRandomPIN]
error:&error];
}

Expand Down
5 changes: 5 additions & 0 deletions src/controller/CommissioningWindowOpener.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(NodeId deviceId, S

if (setupPIN.HasValue())
{
if (!SetupPayload::IsValidSetupPIN(setupPIN.Value()))
{
return CHIP_ERROR_INVALID_ARGUMENT;
}

mCommissioningWindowOption = CommissioningWindowOption::kTokenWithProvidedPIN;
mSetupPayload.setUpPINCode = setupPIN.Value();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ - (IBAction)overrideControls:(id)sender

- (IBAction)openPairingWindow:(id)sender
{
uint32_t setupPIN = arc4random();
NSUInteger setupPIN = [MTRSetupPayload generateRandomPIN];
[_deviceSelector forSelectedDevices:^(uint64_t deviceId) {
if (MTRGetConnectedDeviceWithID(deviceId, ^(MTRBaseDevice * _Nullable chipDevice, NSError * _Nullable error) {
if (chipDevice) {
Expand Down
8 changes: 7 additions & 1 deletion src/darwin/Framework/CHIP/MTRDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,13 @@ - (NSString *)openPairingWindowWithPIN:(uint64_t)deviceID
return rv;
}

setupPIN &= ((1 << chip::kSetupPINCodeFieldLengthInBits) - 1);
if (!chip::CanCastTo<uint32_t>(setupPIN) || !chip::SetupPayload::IsValidSetupPIN(static_cast<uint32_t>(setupPIN))) {
MTR_LOG_ERROR("Error: Setup pin %lu is not valid", setupPIN);
if (error) {
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_INTEGER_VALUE];
}
return rv;
}

dispatch_sync(_chipWorkQueue, ^{
VerifyOrReturn([self checkIsRunning:error]);
Expand Down
4 changes: 4 additions & 0 deletions src/darwin/Framework/CHIP/MTRSetupPayload.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ typedef NS_ENUM(NSUInteger, MTROptionalQRCodeInfoType) {
@property (nonatomic, strong) NSString * serialNumber;
- (nullable NSArray<MTROptionalQRCodeInfo *> *)getAllOptionalVendorData:(NSError * __autoreleasing *)error;

/**
* Generate a random Matter-valid setup PIN.
*/
+ (NSUInteger)generateRandomPIN;
@end

NS_ASSUME_NONNULL_END
19 changes: 19 additions & 0 deletions src/darwin/Framework/CHIP/MTRSetupPayload.mm
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,25 @@ - (void)getSerialNumber:(chip::SetupPayload)setupPayload
return allOptionalData;
}

+ (NSUInteger)generateRandomPIN
{
do {
// Make sure the thing we generate is in the right range.
uint32_t setupPIN = arc4random_uniform(chip::kSetupPINCodeMaximumValue) + 1;
if (chip::SetupPayload::IsValidSetupPIN(setupPIN)) {
return setupPIN;
}

// We got pretty unlikely with our random number generation. Just try
// again. The chance that this loop does not terminate in a reasonable
// amount of time is astronomically low, assuming arc4random_uniform is not
// broken.
} while (1);

// Not reached.
return chip::kSetupPINCodeUndefinedValue;
}

#pragma mark - NSSecureCoding

static NSString * const MTRSetupPayloadCodingKeyVersion = @"MTRSP.ck.version";
Expand Down
3 changes: 0 additions & 3 deletions src/protocols/secure_channel/PASESession.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ extern const char * kSpake2pR2ISessionInfo;

constexpr uint16_t kPBKDFParamRandomNumberSize = 32;

constexpr uint32_t kSetupPINCodeMaximumValue = 99999998;
constexpr uint32_t kSetupPINCodeUndefinedValue = 0;

using namespace Crypto;

struct PASESessionSerialized;
Expand Down
20 changes: 15 additions & 5 deletions src/setup_payload/SetupPayload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,20 @@ bool PayloadContents::isValidManualCode() const
return CheckPayloadCommonConstraints();
}

bool PayloadContents::IsValidSetupPIN(uint32_t setupPIN)
{
// SHALL be restricted to the values 0x0000001 to 0x5F5E0FE (00000001 to 99999998 in decimal), excluding the invalid Passcode
// values.
if (setupPIN == kSetupPINCodeUndefinedValue || setupPIN > kSetupPINCodeMaximumValue || setupPIN == 11111111 ||
setupPIN == 22222222 || setupPIN == 33333333 || setupPIN == 44444444 || setupPIN == 55555555 || setupPIN == 66666666 ||
setupPIN == 77777777 || setupPIN == 88888888 || setupPIN == 12345678 || setupPIN == 87654321)
{
return false;
}

return true;
}

bool PayloadContents::CheckPayloadCommonConstraints() const
{
// A version not equal to 0 would be invalid for v1 and would indicate new format (e.g. version 2)
Expand All @@ -119,11 +133,7 @@ bool PayloadContents::CheckPayloadCommonConstraints() const
return false;
}

// SHALL be restricted to the values 0x0000001 to 0x5F5E0FE (00000001 to 99999998 in decimal), excluding the invalid Passcode
// values.
if (setUpPINCode < 0x0000001 || setUpPINCode > 0x5F5E0FE || setUpPINCode == 11111111 || setUpPINCode == 22222222 ||
setUpPINCode == 33333333 || setUpPINCode == 44444444 || setUpPINCode == 55555555 || setUpPINCode == 66666666 ||
setUpPINCode == 77777777 || setUpPINCode == 88888888 || setUpPINCode == 12345678 || setUpPINCode == 87654321)
if (!IsValidSetupPIN(setUpPINCode))
{
return false;
}
Expand Down
5 changes: 5 additions & 0 deletions src/setup_payload/SetupPayload.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ constexpr uint8_t kBPKFSaltTag = 0x02;
constexpr uint8_t kNumberOFDevicesTag = 0x03;
constexpr uint8_t kCommissioningTimeoutTag = 0x04;

constexpr uint32_t kSetupPINCodeMaximumValue = 99999998;
constexpr uint32_t kSetupPINCodeUndefinedValue = 0;

// clang-format off
const int kTotalPayloadDataSizeInBits =
kVersionFieldLengthInBits +
Expand Down Expand Up @@ -124,6 +127,8 @@ struct PayloadContents
bool isShortDiscriminator = false;
bool operator==(PayloadContents & input) const;

static bool IsValidSetupPIN(uint32_t setupPIN);

private:
bool CheckPayloadCommonConstraints() const;
};
Expand Down
1 change: 1 addition & 0 deletions src/tools/spake2p/Cmd_GenVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include <lib/support/CHIPArgParser.hpp>
#include <lib/support/CHIPMem.h>
#include <protocols/secure_channel/PASESession.h>
#include <setup_payload/SetupPayload.h>

namespace {

Expand Down

0 comments on commit 1164035

Please sign in to comment.