-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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.
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
[Android] Attestation trust store bridge #24381
Closed
panliming-tuya
wants to merge
72
commits into
project-chip:master
from
panliming-tuya:attestation-store-bridge
Closed
Changes from 66 commits
Commits
Show all changes
72 commits
Select commit
Hold shift + click to select a range
11b5724
[Android] Added mechanism to override device attestation failure base…
panliming-tuya ea40f1a
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 5ed16c0
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya d36248a
platform jar keep name of method parameters
panliming-tuya 1c38401
Restyled by whitespace
restyled-commits 587ac5f
Restyled by google-java-format
restyled-commits 66bedd6
Restyled by clang-format
restyled-commits d9be2ba
Restyled by gn
restyled-commits afdf205
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya e8f0495
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 640ddbd
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 3c4f499
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 48ce640
fix copyright
panliming-tuya eb95edd
Merge branch 'additional-verification-after-attestation' of https://g…
panliming-tuya 5e8723b
fix and modify comments
panliming-tuya 79024b4
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 2270c9d
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 47f079e
Use setters instead of adding parameters to methods
panliming-tuya 27a3862
fix NetworkCredentials NPE
panliming-tuya beb066a
Do not expose deviceController raw pointer
panliming-tuya f9d3dd5
add sample
panliming-tuya 9e6a181
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 38b0c0c
Create AttestationTrustStoreBridge when we know we have PAA certs.
panliming-tuya 0ee733e
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 8481cde
fix jni method
panliming-tuya e6211db
fix certs loss of scope and add some comments
panliming-tuya 3bacfe3
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 3b40467
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya b26afb9
implement destructor
panliming-tuya 27584d6
fix destructor crash
panliming-tuya a654ede
revoke vscode setting change
panliming-tuya 6db2e8b
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 52c5c30
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 4958adb
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 213164e
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya a610fa5
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 59c8ca6
Restyled by whitespace
panliming-tuya 3811015
Restyled by google-java-format
panliming-tuya 8c01083
Restyled by clang-format
panliming-tuya 5dcbae5
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 8837adf
fix conflict
panliming-tuya 212dc10
add unit of failSafeExpiryTimeout
panliming-tuya 9d41d26
add sample code
panliming-tuya 37f760a
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 747645d
Restyled by whitespace
panliming-tuya 17348b2
Restyled by google-java-format
panliming-tuya e5972ae
Restyled by clang-format
panliming-tuya 9d915df
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 993b5cf
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya f069618
fix comments
panliming-tuya f485757
remove android attestation trust store
panliming-tuya c027231
Merge branch 'master' into additional-verification-after-attestation
panliming-tuya 2b094c8
restyle
panliming-tuya a870918
[android] support attestation trust store delegate
panliming-tuya b76318b
restyled.
panliming-tuya af9da6d
fix jni crash
panliming-tuya d764674
Modify the timing of setting the Attestation trust store
panliming-tuya 73c27a1
fix conflict
panliming-tuya 731c941
setAttestationTrustStoreDelegate add comments
panliming-tuya b0b7e1c
Remove redundant variables
panliming-tuya 7de0a34
Release jni class references
panliming-tuya d3a8940
Merge branch 'attestation-store-bridge' of https://github.com/panlimi…
panliming-tuya 4ed1234
Fix objects leak when calling 'setAttestationTrustStoreDelegate' twice
panliming-tuya a8b9a86
Remove unused include
panliming-tuya f2d3c35
Modify private to protected
panliming-tuya 7e29657
Optimize variable initialization
panliming-tuya 4c219c2
fix compile error
panliming-tuya 67dc3cf
Moving the alloc of AttestationTrustStoreBridge before clear
panliming-tuya 454d160
Merge branch 'master' into attestation-store-bridge
panliming-tuya 3f82e30
Add Android example
panliming-tuya 6541582
Merge branch 'attestation-store-bridge' of https://github.com/panlimi…
panliming-tuya 3ede0b5
Merge branch 'master' into attestation-store-bridge
panliming-tuya File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
/** | ||
* | ||
* Copyright (c) 2023 Project CHIP Authors | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
#include "AttestationTrustStoreBridge.h" | ||
#include <credentials/CHIPCert.h> | ||
#include <lib/support/CHIPJNIError.h> | ||
#include <lib/support/CodeUtils.h> | ||
#include <lib/support/JniReferences.h> | ||
#include <lib/support/JniTypeWrappers.h> | ||
#include <lib/support/logging/CHIPLogging.h> | ||
|
||
using namespace chip; | ||
|
||
AttestationTrustStoreBridge::~AttestationTrustStoreBridge() | ||
{ | ||
if (mAttestationTrustStoreDelegate != nullptr) | ||
{ | ||
JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); | ||
VerifyOrReturn(env != nullptr, ChipLogError(Controller, "Could not get JNIEnv for current thread")); | ||
env->DeleteGlobalRef(mAttestationTrustStoreDelegate); | ||
mAttestationTrustStoreDelegate = nullptr; | ||
} | ||
} | ||
|
||
CHIP_ERROR AttestationTrustStoreBridge::GetProductAttestationAuthorityCert(const chip::ByteSpan & skid, | ||
chip::MutableByteSpan & outPaaDerBuffer) const | ||
{ | ||
VerifyOrReturnError(skid.size() == chip::Crypto::kSubjectKeyIdentifierLength, CHIP_ERROR_INVALID_ARGUMENT); | ||
|
||
constexpr size_t paaCertAllocatedLen = chip::Credentials::kMaxDERCertLength; | ||
Platform::ScopedMemoryBuffer<uint8_t> paaCert; | ||
VerifyOrReturnError(paaCert.Alloc(paaCertAllocatedLen), CHIP_ERROR_NO_MEMORY); | ||
|
||
MutableByteSpan paaDerBuffer{paaCert.Get(), paaCertAllocatedLen}; | ||
ReturnErrorOnFailure(GetPaaCertFromJava(skid, paaDerBuffer)); | ||
|
||
uint8_t skidBuf[chip::Crypto::kSubjectKeyIdentifierLength] = { 0 }; | ||
chip::MutableByteSpan candidateSkidSpan{ skidBuf }; | ||
VerifyOrReturnError(CHIP_NO_ERROR == chip::Crypto::ExtractSKIDFromX509Cert(paaDerBuffer, candidateSkidSpan), | ||
CHIP_ERROR_INTERNAL); | ||
|
||
// Make sure the skid of the paa cert is match. | ||
if (skid.data_equal(candidateSkidSpan)) | ||
{ | ||
// Found a match | ||
return CopySpanToMutableSpan(paaDerBuffer, outPaaDerBuffer); | ||
} | ||
return CHIP_ERROR_CA_CERT_NOT_FOUND; | ||
} | ||
|
||
CHIP_ERROR AttestationTrustStoreBridge::GetPaaCertFromJava(const chip::ByteSpan & skid, | ||
chip::MutableByteSpan & outPaaDerBuffer) const | ||
{ | ||
JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); | ||
jclass attestationTrustStoreDelegateCls = nullptr; | ||
jbyteArray javaSkid = nullptr; | ||
jmethodID getProductAttestationAuthorityCertMethod = nullptr; | ||
|
||
JniReferences::GetInstance().GetClassRef(env, "chip/devicecontroller/AttestationTrustStoreDelegate", | ||
panliming-tuya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
attestationTrustStoreDelegateCls); | ||
VerifyOrReturnError(attestationTrustStoreDelegateCls != nullptr, CHIP_JNI_ERROR_TYPE_NOT_FOUND); | ||
JniClass attestationTrustStoreDelegateJniCls(attestationTrustStoreDelegateCls); | ||
|
||
JniReferences::GetInstance().FindMethod(env, mAttestationTrustStoreDelegate, "getProductAttestationAuthorityCert", "([B)[B", | ||
&getProductAttestationAuthorityCertMethod); | ||
VerifyOrReturnError(getProductAttestationAuthorityCertMethod != nullptr, CHIP_JNI_ERROR_METHOD_NOT_FOUND); | ||
|
||
JniReferences::GetInstance().N2J_ByteArray(env, skid.data(), static_cast<jsize>(skid.size()), javaSkid); | ||
VerifyOrReturnError(javaSkid != nullptr, CHIP_ERROR_NO_MEMORY); | ||
|
||
jbyteArray javaPaaCert = | ||
(jbyteArray) env->CallObjectMethod(mAttestationTrustStoreDelegate, getProductAttestationAuthorityCertMethod, javaSkid); | ||
JniByteArray paaCertBytes(env, javaPaaCert); | ||
CopySpanToMutableSpan(paaCertBytes.byteSpan(), outPaaDerBuffer); | ||
|
||
return CHIP_NO_ERROR; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
/** | ||
* | ||
* Copyright (c) 2023 Project CHIP Authors | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
#include <lib/support/JniReferences.h> | ||
#include <lib/support/Span.h> | ||
|
||
class AttestationTrustStoreBridge : public chip::Credentials::AttestationTrustStore | ||
{ | ||
public: | ||
AttestationTrustStoreBridge(jobject attestationTrustStoreDelegate) : | ||
mAttestationTrustStoreDelegate(attestationTrustStoreDelegate) | ||
{} | ||
~AttestationTrustStoreBridge(); | ||
|
||
CHIP_ERROR GetProductAttestationAuthorityCert(const chip::ByteSpan & skid, | ||
panliming-tuya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
chip::MutableByteSpan & outPaaDerBuffer) const override; | ||
|
||
protected: | ||
jobject mAttestationTrustStoreDelegate = nullptr; | ||
|
||
CHIP_ERROR GetPaaCertFromJava(const chip::ByteSpan & skid, chip::MutableByteSpan & outPaaDerBuffer) const; | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
25 changes: 25 additions & 0 deletions
25
src/controller/java/src/chip/devicecontroller/AttestationTrustStoreDelegate.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
package chip.devicecontroller; | ||
|
||
import javax.annotation.Nullable; | ||
|
||
/** | ||
* Delegate for attestation trust store for device attestation verifiers. | ||
* | ||
* <p>API is synchronous. This implementation will replace the built-in attestation trust store, | ||
* please make sure you have the required paa certificate before commissioning. | ||
*/ | ||
public interface AttestationTrustStoreDelegate { | ||
/** | ||
* Look-up a product attestation authority (PAA) cert by subject key identifier (SKID). | ||
* | ||
* <p>The implementations of this interface must have access to a set of PAAs to trust. | ||
* | ||
* <p>Interface is synchronous, and therefore this should not be used unless to expose a PAA store | ||
* that is both fully local and quick to access. | ||
* | ||
* @param skid Buffer containing the subject key identifier (SKID) of the PAA to look-up | ||
* @return If found, the result should return paa cert in x.509 format, if not found, return null. | ||
*/ | ||
@Nullable | ||
byte[] getProductAttestationAuthorityCert(byte[] skid); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assumes it was
new'ed
which it doesn't have to be. We should not delete unless we know it was allocated dynamically with new.The pattern used by previous code:
is also problematic, unless the API clearly states the requirement that the arguments must be allocated with
new
and ownership is transfered to the class.Suggest using
std::unique_ptr
instead if ownership transfer is desired. It would make it clearer.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with that, using
std::unique_ptr
could make it clearer.However, it requires all platforms
AttestationTrustStore *
andDeviceAttestationVerifier *
to be modified into smart pointers.Could we fix it in following PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now the issue is that you force a new being done outside, which is not guaranteed to be done by all...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK,I will fix it. Due to the need to modify the code for multiple platforms, I need to work with other teams in my company, which will take some time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @panliming-tuya