Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding preprocessor checks before using DevelopmentCerts:kDac* constants #25467

Closed
wants to merge 1 commit into from
Closed

Adding preprocessor checks before using DevelopmentCerts:kDac* constants #25467

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/credentials/examples/DeviceAttestationCredsExample.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ class ExampleDACProvider : public DeviceAttestationCredentialsProvider

CHIP_ERROR ExampleDACProvider::GetDeviceAttestationCert(MutableByteSpan & out_dac_buffer)
{
#if 0x8000 <= CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID && CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID <= 0x801F
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of hardcoding the 0x8000 here, can we define the CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID_MIN/MAX or something in the ExampleDACs.h file?

It'd be nicer to have the control of the range specified close to where they're defined otherwise if we add additional product identifiers here you'd need to know to look at this spot too otherwise it won't work and you'd be wondering why

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, wouldn't it be better to simply add a GN switch not to compile ExampleDACProvider if you use a non-default product ID? This class is only for test purposes so it shouldn't be used if you have your own DAC. Btw, why is the linker error occuring? Is ExampleDACProvider instantiated in your application? Wouldn't removing ExampleDACProvider from the application be enough to resolve it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, in this PR: #25953

Closing this one.

return CopySpanToMutableSpan(DevelopmentCerts::kDacCert, out_dac_buffer);
#else
return CHIP_ERROR_NOT_IMPLEMENTED;
#endif // 0x8000 <= CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID && CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID <= 0x801F
}

CHIP_ERROR ExampleDACProvider::GetProductAttestationIntermediateCert(MutableByteSpan & out_pai_buffer)
Expand Down Expand Up @@ -122,6 +126,7 @@ CHIP_ERROR ExampleDACProvider::GetFirmwareInformation(MutableByteSpan & out_firm
CHIP_ERROR ExampleDACProvider::SignWithDeviceAttestationKey(const ByteSpan & message_to_sign,
MutableByteSpan & out_signature_buffer)
{
#if 0x8000 <= CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID && CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID <= 0x801F
Crypto::P256ECDSASignature signature;
Crypto::P256Keypair keypair;

Expand All @@ -135,6 +140,9 @@ CHIP_ERROR ExampleDACProvider::SignWithDeviceAttestationKey(const ByteSpan & mes
ReturnErrorOnFailure(keypair.ECDSA_sign_msg(message_to_sign.data(), message_to_sign.size(), signature));

return CopySpanToMutableSpan(ByteSpan{ signature.ConstBytes(), signature.Length() }, out_signature_buffer);
#else
return CHIP_ERROR_NOT_IMPLEMENTED;
#endif // 0x8000 <= CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID && CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_ID <= 0x801F
}

} // namespace
Expand Down