Skip to content

Commit

Permalink
Add chip-tool support for octet string arguments with non-ASCII bytes…
Browse files Browse the repository at this point in the history 
… in them. (#7363)

For an octet-string argument we support three formats:

1. hex: followed by hex representation of the bytes.
2. str: followed by just the chars, if they are all nicely printable
3. Just the bytes/chars, if they are all nicely printable.

Format 1 is needed when the value contains non-printable chars, and
especially nulls.  Formats 2 and 3 are easier to use for the typical use case
of Wi-Fi SSIDs and passwords.

Formats 1 and 2 match chip-device-ctrl.
  • Loading branch information
@bzbarsky-apple @pull
bzbarsky-apple authored and pull[bot] committed Jun 18, 2021
1 parent f16387b commit b5a29d3
Show file tree
Hide file tree
Showing 8 changed files with 147 additions and 74 deletions.
87 changes: 35 additions & 52 deletions examples/chip-tool/commands/clusters/Commands.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11516,9 +11516,8 @@ class NetworkCommissioningAddThreadNetwork : public ModelCommand

chip::Controller::NetworkCommissioningCluster cluster;
cluster.Associate(device, endpointId);
return cluster.AddThreadNetwork(onSuccessCallback->Cancel(), onFailureCallback->Cancel(),
chip::ByteSpan(chip::Uint8::from_char(mOperationalDataset), strlen(mOperationalDataset)),
mBreadcrumb, mTimeoutMs);
return cluster.AddThreadNetwork(onSuccessCallback->Cancel(), onFailureCallback->Cancel(), mOperationalDataset, mBreadcrumb,
mTimeoutMs);
}

private:
Expand All @@ -11527,7 +11526,7 @@ class NetworkCommissioningAddThreadNetwork : public ModelCommand
OnNetworkCommissioningClusterAddThreadNetworkResponse, this);
chip::Callback::Callback<DefaultFailureCallback> * onFailureCallback =
new chip::Callback::Callback<DefaultFailureCallback>(OnDefaultFailureResponse, this);
char * mOperationalDataset;
chip::ByteSpan mOperationalDataset;
uint64_t mBreadcrumb;
uint32_t mTimeoutMs;
};
Expand Down Expand Up @@ -11558,9 +11557,8 @@ class NetworkCommissioningAddWiFiNetwork : public ModelCommand

chip::Controller::NetworkCommissioningCluster cluster;
cluster.Associate(device, endpointId);
return cluster.AddWiFiNetwork(
onSuccessCallback->Cancel(), onFailureCallback->Cancel(), chip::ByteSpan(chip::Uint8::from_char(mSsid), strlen(mSsid)),
chip::ByteSpan(chip::Uint8::from_char(mCredentials), strlen(mCredentials)), mBreadcrumb, mTimeoutMs);
return cluster.AddWiFiNetwork(onSuccessCallback->Cancel(), onFailureCallback->Cancel(), mSsid, mCredentials, mBreadcrumb,
mTimeoutMs);
}

private:
Expand All @@ -11569,8 +11567,8 @@ class NetworkCommissioningAddWiFiNetwork : public ModelCommand
OnNetworkCommissioningClusterAddWiFiNetworkResponse, this);
chip::Callback::Callback<DefaultFailureCallback> * onFailureCallback =
new chip::Callback::Callback<DefaultFailureCallback>(OnDefaultFailureResponse, this);
char * mSsid;
char * mCredentials;
chip::ByteSpan mSsid;
chip::ByteSpan mCredentials;
uint64_t mBreadcrumb;
uint32_t mTimeoutMs;
};
Expand Down Expand Up @@ -11600,8 +11598,7 @@ class NetworkCommissioningDisableNetwork : public ModelCommand

chip::Controller::NetworkCommissioningCluster cluster;
cluster.Associate(device, endpointId);
return cluster.DisableNetwork(onSuccessCallback->Cancel(), onFailureCallback->Cancel(),
chip::ByteSpan(chip::Uint8::from_char(mNetworkID), strlen(mNetworkID)), mBreadcrumb,
return cluster.DisableNetwork(onSuccessCallback->Cancel(), onFailureCallback->Cancel(), mNetworkID, mBreadcrumb,
mTimeoutMs);
}

Expand All @@ -11611,7 +11608,7 @@ class NetworkCommissioningDisableNetwork : public ModelCommand
OnNetworkCommissioningClusterDisableNetworkResponse, this);
chip::Callback::Callback<DefaultFailureCallback> * onFailureCallback =
new chip::Callback::Callback<DefaultFailureCallback>(OnDefaultFailureResponse, this);
char * mNetworkID;
chip::ByteSpan mNetworkID;
uint64_t mBreadcrumb;
uint32_t mTimeoutMs;
};
Expand Down Expand Up @@ -11641,9 +11638,7 @@ class NetworkCommissioningEnableNetwork : public ModelCommand

chip::Controller::NetworkCommissioningCluster cluster;
cluster.Associate(device, endpointId);
return cluster.EnableNetwork(onSuccessCallback->Cancel(), onFailureCallback->Cancel(),
chip::ByteSpan(chip::Uint8::from_char(mNetworkID), strlen(mNetworkID)), mBreadcrumb,
mTimeoutMs);
return cluster.EnableNetwork(onSuccessCallback->Cancel(), onFailureCallback->Cancel(), mNetworkID, mBreadcrumb, mTimeoutMs);
}

private:
Expand All @@ -11652,7 +11647,7 @@ class NetworkCommissioningEnableNetwork : public ModelCommand
OnNetworkCommissioningClusterEnableNetworkResponse, this);
chip::Callback::Callback<DefaultFailureCallback> * onFailureCallback =
new chip::Callback::Callback<DefaultFailureCallback>(OnDefaultFailureResponse, this);
char * mNetworkID;
chip::ByteSpan mNetworkID;
uint64_t mBreadcrumb;
uint32_t mTimeoutMs;
};
Expand Down Expand Up @@ -11716,9 +11711,7 @@ class NetworkCommissioningRemoveNetwork : public ModelCommand

chip::Controller::NetworkCommissioningCluster cluster;
cluster.Associate(device, endpointId);
return cluster.RemoveNetwork(onSuccessCallback->Cancel(), onFailureCallback->Cancel(),
chip::ByteSpan(chip::Uint8::from_char(mNetworkID), strlen(mNetworkID)), mBreadcrumb,
mTimeoutMs);
return cluster.RemoveNetwork(onSuccessCallback->Cancel(), onFailureCallback->Cancel(), mNetworkID, mBreadcrumb, mTimeoutMs);
}

private:
Expand All @@ -11727,7 +11720,7 @@ class NetworkCommissioningRemoveNetwork : public ModelCommand
OnNetworkCommissioningClusterRemoveNetworkResponse, this);
chip::Callback::Callback<DefaultFailureCallback> * onFailureCallback =
new chip::Callback::Callback<DefaultFailureCallback>(OnDefaultFailureResponse, this);
char * mNetworkID;
chip::ByteSpan mNetworkID;
uint64_t mBreadcrumb;
uint32_t mTimeoutMs;
};
Expand Down Expand Up @@ -11757,8 +11750,7 @@ class NetworkCommissioningScanNetworks : public ModelCommand

chip::Controller::NetworkCommissioningCluster cluster;
cluster.Associate(device, endpointId);
return cluster.ScanNetworks(onSuccessCallback->Cancel(), onFailureCallback->Cancel(),
chip::ByteSpan(chip::Uint8::from_char(mSsid), strlen(mSsid)), mBreadcrumb, mTimeoutMs);
return cluster.ScanNetworks(onSuccessCallback->Cancel(), onFailureCallback->Cancel(), mSsid, mBreadcrumb, mTimeoutMs);
}

private:
Expand All @@ -11767,7 +11759,7 @@ class NetworkCommissioningScanNetworks : public ModelCommand
OnNetworkCommissioningClusterScanNetworksResponse, this);
chip::Callback::Callback<DefaultFailureCallback> * onFailureCallback =
new chip::Callback::Callback<DefaultFailureCallback>(OnDefaultFailureResponse, this);
char * mSsid;
chip::ByteSpan mSsid;
uint64_t mBreadcrumb;
uint32_t mTimeoutMs;
};
Expand Down Expand Up @@ -11797,8 +11789,7 @@ class NetworkCommissioningUpdateThreadNetwork : public ModelCommand

chip::Controller::NetworkCommissioningCluster cluster;
cluster.Associate(device, endpointId);
return cluster.UpdateThreadNetwork(onSuccessCallback->Cancel(), onFailureCallback->Cancel(),
chip::ByteSpan(chip::Uint8::from_char(mOperationalDataset), strlen(mOperationalDataset)),
return cluster.UpdateThreadNetwork(onSuccessCallback->Cancel(), onFailureCallback->Cancel(), mOperationalDataset,
mBreadcrumb, mTimeoutMs);
}

Expand All @@ -11808,7 +11799,7 @@ class NetworkCommissioningUpdateThreadNetwork : public ModelCommand
OnNetworkCommissioningClusterUpdateThreadNetworkResponse, this);
chip::Callback::Callback<DefaultFailureCallback> * onFailureCallback =
new chip::Callback::Callback<DefaultFailureCallback>(OnDefaultFailureResponse, this);
char * mOperationalDataset;
chip::ByteSpan mOperationalDataset;
uint64_t mBreadcrumb;
uint32_t mTimeoutMs;
};
Expand Down Expand Up @@ -11839,9 +11830,8 @@ class NetworkCommissioningUpdateWiFiNetwork : public ModelCommand

chip::Controller::NetworkCommissioningCluster cluster;
cluster.Associate(device, endpointId);
return cluster.UpdateWiFiNetwork(
onSuccessCallback->Cancel(), onFailureCallback->Cancel(), chip::ByteSpan(chip::Uint8::from_char(mSsid), strlen(mSsid)),
chip::ByteSpan(chip::Uint8::from_char(mCredentials), strlen(mCredentials)), mBreadcrumb, mTimeoutMs);
return cluster.UpdateWiFiNetwork(onSuccessCallback->Cancel(), onFailureCallback->Cancel(), mSsid, mCredentials, mBreadcrumb,
mTimeoutMs);
}

private:
Expand All @@ -11850,8 +11840,8 @@ class NetworkCommissioningUpdateWiFiNetwork : public ModelCommand
OnNetworkCommissioningClusterUpdateWiFiNetworkResponse, this);
chip::Callback::Callback<DefaultFailureCallback> * onFailureCallback =
new chip::Callback::Callback<DefaultFailureCallback>(OnDefaultFailureResponse, this);
char * mSsid;
char * mCredentials;
chip::ByteSpan mSsid;
chip::ByteSpan mCredentials;
uint64_t mBreadcrumb;
uint32_t mTimeoutMs;
};
Expand Down Expand Up @@ -12207,10 +12197,8 @@ class OperationalCredentialsAddOpCert : public ModelCommand

chip::Controller::OperationalCredentialsCluster cluster;
cluster.Associate(device, endpointId);
return cluster.AddOpCert(
onSuccessCallback->Cancel(), onFailureCallback->Cancel(), chip::ByteSpan(chip::Uint8::from_char(mNoc), strlen(mNoc)),
chip::ByteSpan(chip::Uint8::from_char(mICACertificate), strlen(mICACertificate)),
chip::ByteSpan(chip::Uint8::from_char(mIPKValue), strlen(mIPKValue)), mCaseAdminNode, mAdminVendorId);
return cluster.AddOpCert(onSuccessCallback->Cancel(), onFailureCallback->Cancel(), mNoc, mICACertificate, mIPKValue,
mCaseAdminNode, mAdminVendorId);
}

private:
Expand All @@ -12219,9 +12207,9 @@ class OperationalCredentialsAddOpCert : public ModelCommand
OnOperationalCredentialsClusterOpCertResponse, this);
chip::Callback::Callback<DefaultFailureCallback> * onFailureCallback =
new chip::Callback::Callback<DefaultFailureCallback>(OnDefaultFailureResponse, this);
char * mNoc;
char * mICACertificate;
char * mIPKValue;
chip::ByteSpan mNoc;
chip::ByteSpan mICACertificate;
chip::ByteSpan mIPKValue;
chip::NodeId mCaseAdminNode;
uint16_t mAdminVendorId;
};
Expand Down Expand Up @@ -12249,8 +12237,7 @@ class OperationalCredentialsOpCSRRequest : public ModelCommand

chip::Controller::OperationalCredentialsCluster cluster;
cluster.Associate(device, endpointId);
return cluster.OpCSRRequest(onSuccessCallback->Cancel(), onFailureCallback->Cancel(),
chip::ByteSpan(chip::Uint8::from_char(mCSRNonce), strlen(mCSRNonce)));
return cluster.OpCSRRequest(onSuccessCallback->Cancel(), onFailureCallback->Cancel(), mCSRNonce);
}

private:
Expand All @@ -12259,7 +12246,7 @@ class OperationalCredentialsOpCSRRequest : public ModelCommand
OnOperationalCredentialsClusterOpCSRResponse, this);
chip::Callback::Callback<DefaultFailureCallback> * onFailureCallback =
new chip::Callback::Callback<DefaultFailureCallback>(OnDefaultFailureResponse, this);
char * mCSRNonce;
chip::ByteSpan mCSRNonce;
};

/*
Expand Down Expand Up @@ -15873,16 +15860,15 @@ class WriteTestClusterOctetString : public ModelCommand

chip::Controller::TestClusterCluster cluster;
cluster.Associate(device, endpointId);
return cluster.WriteAttributeOctetString(onSuccessCallback->Cancel(), onFailureCallback->Cancel(),
chip::ByteSpan(chip::Uint8::from_char(mValue), strlen(mValue)));
return cluster.WriteAttributeOctetString(onSuccessCallback->Cancel(), onFailureCallback->Cancel(), mValue);
}

private:
chip::Callback::Callback<DefaultSuccessCallback> * onSuccessCallback =
new chip::Callback::Callback<DefaultSuccessCallback>(OnDefaultSuccessResponse, this);
chip::Callback::Callback<DefaultFailureCallback> * onFailureCallback =
new chip::Callback::Callback<DefaultFailureCallback>(OnDefaultFailureResponse, this);
char * mValue;
chip::ByteSpan mValue;
};

/*
Expand Down Expand Up @@ -16661,17 +16647,15 @@ class TrustedRootCertificatesAddTrustedRootCertificate : public ModelCommand

chip::Controller::TrustedRootCertificatesCluster cluster;
cluster.Associate(device, endpointId);
return cluster.AddTrustedRootCertificate(
onSuccessCallback->Cancel(), onFailureCallback->Cancel(),
chip::ByteSpan(chip::Uint8::from_char(mRootCertificate), strlen(mRootCertificate)));
return cluster.AddTrustedRootCertificate(onSuccessCallback->Cancel(), onFailureCallback->Cancel(), mRootCertificate);
}

private:
chip::Callback::Callback<DefaultSuccessCallback> * onSuccessCallback =
new chip::Callback::Callback<DefaultSuccessCallback>(OnDefaultSuccessResponse, this);
chip::Callback::Callback<DefaultFailureCallback> * onFailureCallback =
new chip::Callback::Callback<DefaultFailureCallback>(OnDefaultFailureResponse, this);
char * mRootCertificate;
chip::ByteSpan mRootCertificate;
};

/*
Expand All @@ -16697,17 +16681,16 @@ class TrustedRootCertificatesRemoveTrustedRootCertificate : public ModelCommand

chip::Controller::TrustedRootCertificatesCluster cluster;
cluster.Associate(device, endpointId);
return cluster.RemoveTrustedRootCertificate(
onSuccessCallback->Cancel(), onFailureCallback->Cancel(),
chip::ByteSpan(chip::Uint8::from_char(mTrustedRootIdentifier), strlen(mTrustedRootIdentifier)));
return cluster.RemoveTrustedRootCertificate(onSuccessCallback->Cancel(), onFailureCallback->Cancel(),
mTrustedRootIdentifier);
}

private:
chip::Callback::Callback<DefaultSuccessCallback> * onSuccessCallback =
new chip::Callback::Callback<DefaultSuccessCallback>(OnDefaultSuccessResponse, this);
chip::Callback::Callback<DefaultFailureCallback> * onFailureCallback =
new chip::Callback::Callback<DefaultFailureCallback>(OnDefaultFailureResponse, this);
char * mTrustedRootIdentifier;
chip::ByteSpan mTrustedRootIdentifier;
};

/*
Expand Down
79 changes: 74 additions & 5 deletions examples/chip-tool/commands/common/Command.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@
#include <sys/socket.h>
#include <sys/types.h>

#include <lib/core/CHIPSafeCasts.h>
#include <support/BytesToHex.h>
#include <support/CHIPMem.h>
#include <support/CodeUtils.h>
#include <support/SafeInt.h>
#include <support/ScopedBuffer.h>
#include <support/logging/CHIPLogging.h>

bool Command::InitArguments(int argc, char ** argv)
Expand Down Expand Up @@ -78,7 +83,7 @@ static bool ParseAddressWithInterface(const char * addressString, Command::Addre
return true;
}

bool Command::InitArgument(size_t argIndex, const char * argValue)
bool Command::InitArgument(size_t argIndex, char * argValue)
{
bool isValidArgument = false;

Expand All @@ -91,10 +96,63 @@ bool Command::InitArgument(size_t argIndex, const char * argValue)
break;
}

case ArgumentType::String: {
case ArgumentType::CharString: {
const char ** value = reinterpret_cast<const char **>(arg.value);
*value = const_cast<char *>(argValue);
isValidArgument = (strcmp(argValue, *value) == 0);
*value = argValue;
isValidArgument = true;
break;
}

case ArgumentType::OctetString: {
auto * value = static_cast<chip::ByteSpan *>(arg.value);
// We support two ways to pass an octet string argument. If it happens
// to be all-ASCII, you can just pass it in. Otherwise you can pass in
// 0x followed by the hex-encoded bytes.
size_t argLen = strlen(argValue);
static constexpr char hexPrefix[] = "hex:";
constexpr size_t prefixLen = ArraySize(hexPrefix) - 1; // Don't count the null
if (strncmp(argValue, hexPrefix, prefixLen) == 0)
{
// Hex-encoded. Decode it into a temporary buffer first, so if we
// run into errors we can do correct "argument is not valid" logging
// that actually shows the value that was passed in. After we
// determine it's valid, modify the passed-in value to hold the
// right bytes, so we don't need to worry about allocating storage
// for this somewhere else. This works because the hex
// representation is always longer than the octet string it encodes,
// so we have enough space in argValue for the decoded version.
chip::Platform::ScopedMemoryBuffer<uint8_t> buffer;
if (!buffer.Calloc(argLen)) // Bigger than needed, but it's fine.
{
isValidArgument = false;
break;
}

size_t octetCount = chip::Encoding::HexToBytes(argValue + prefixLen, argLen - prefixLen, buffer.Get(), argLen);
if (octetCount == 0)
{
isValidArgument = false;
break;
}

memcpy(argValue, buffer.Get(), octetCount);
*value = chip::ByteSpan(chip::Uint8::from_char(argValue), octetCount);
isValidArgument = true;
}
else
{
// Just ASCII. Check for the "str:" prefix.
static constexpr char strPrefix[] = "str:";
constexpr size_t strPrefixLen = ArraySize(strPrefix) - 1; // Don't count the null
if (strncmp(argValue, strPrefix, strPrefixLen) == 0)
{
// Skip the prefix
argValue += strPrefixLen;
argLen -= strPrefixLen;
}
*value = chip::ByteSpan(chip::Uint8::from_char(argValue), argLen);
isValidArgument = true;
}
break;
}

Expand Down Expand Up @@ -237,7 +295,18 @@ size_t Command::AddArgument(const char * name, const char * value)
size_t Command::AddArgument(const char * name, char ** value)
{
Argument arg;
arg.type = ArgumentType::String;
arg.type = ArgumentType::CharString;
arg.name = name;
arg.value = reinterpret_cast<void *>(value);

mArgs.emplace_back(arg);
return mArgs.size();
}

size_t Command::AddArgument(const char * name, chip::ByteSpan * value)
{
Argument arg;
arg.type = ArgumentType::OctetString;
arg.name = name;
arg.value = reinterpret_cast<void *>(value);

Expand Down
Loading

0 comments on commit b5a29d3

Please sign in to comment.