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.

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

Move the access control ownership and initialization to Server. #14680

Merged
merged 2 commits into from
Feb 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 0 additions & 27 deletions examples/platform/linux/AppMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@
#include <credentials/examples/DefaultDeviceAttestationVerifier.h>
#include <credentials/examples/DeviceAttestationCredsExample.h>

#include <access/examples/ExampleAccessControlDelegate.h>

#include <lib/support/CHIPMem.h>
#include <lib/support/ScopedBuffer.h>
#include <setup_payload/QRCodeSetupPayloadGenerator.h>
Expand Down Expand Up @@ -66,27 +64,6 @@ using namespace chip::DeviceLayer;
using namespace chip::Inet;
using namespace chip::Transport;

class GeneralStorageDelegate : public PersistentStorageDelegate
{
CHIP_ERROR SyncGetKeyValue(const char * key, void * buffer, uint16_t & size) override
{
ChipLogProgress(NotSpecified, "Retrieved value from general storage.");
return PersistedStorage::KeyValueStoreMgr().Get(key, buffer, size);
}

CHIP_ERROR SyncSetKeyValue(const char * key, const void * value, uint16_t size) override
{
ChipLogProgress(NotSpecified, "Stored value in general storage");
return PersistedStorage::KeyValueStoreMgr().Put(key, value, size);
}

CHIP_ERROR SyncDeleteKeyValue(const char * key) override
{
ChipLogProgress(NotSpecified, "Delete value in general storage");
return PersistedStorage::KeyValueStoreMgr().Delete(key);
}
};

#if defined(ENABLE_CHIP_SHELL)
using chip::Shell::Engine;
#endif
Expand All @@ -111,8 +88,6 @@ void EventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg)
ChipLogProgress(DeviceLayer, "Receive kCHIPoBLEConnectionEstablished");
}
}

GeneralStorageDelegate gAclStorageDelegate;
} // namespace

#if CHIP_DEVICE_CONFIG_ENABLE_WPA
Expand Down Expand Up @@ -161,8 +136,6 @@ int ChipLinuxAppInit(int argc, char ** argv)

PrintOnboardingCodes(LinuxDeviceOptions::GetInstance().payload);

Access::Examples::SetAccessControlDelegateStorage(&gAclStorageDelegate);

#if defined(PW_RPC_ENABLED)
rpc::Init();
ChipLogProgress(NotSpecified, "PW_RPC initialized.");
Expand Down
13 changes: 4 additions & 9 deletions src/access/examples/ExampleAccessControlDelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1069,12 +1069,13 @@ class AccessControlDelegate : public AccessControl::Delegate
CHIP_ERROR err = LoadFromFlash();
if (err != CHIP_NO_ERROR)
{
ChipLogDetail(DataManagement, "AccessControl: unable to load stored ACL entries; using empty list instead");
for (auto & storage : EntryStorage::acl)
{
storage.Clear();
}
}
return err;
return CHIP_NO_ERROR;
}

CHIP_ERROR Finish() override
Expand Down Expand Up @@ -1309,17 +1310,11 @@ namespace chip {
namespace Access {
namespace Examples {

AccessControl::Delegate & GetAccessControlDelegate()
AccessControl::Delegate & GetAccessControlDelegate(PersistentStorageDelegate * storageDelegate)
{
static AccessControlDelegate accessControlDelegate;
return accessControlDelegate;
}

void SetAccessControlDelegateStorage(chip::PersistentStorageDelegate * storageDelegate)
{
ChipLogDetail(DataManagement, "Examples::SetAccessControlDelegateStorage");
AccessControlDelegate & accessControlDelegate = static_cast<AccessControlDelegate &>(GetAccessControlDelegate());
accessControlDelegate.SetStorageDelegate(storageDelegate);
return accessControlDelegate;
}

} // namespace Examples
Expand Down
13 changes: 10 additions & 3 deletions src/access/examples/ExampleAccessControlDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,16 @@ namespace chip {
namespace Access {
namespace Examples {

AccessControl::Delegate & GetAccessControlDelegate();

void SetAccessControlDelegateStorage(chip::PersistentStorageDelegate * storageDelegate);
/**
* @brief Get a global instance of the access control delegate implemented in this module.
*
* NOTE: This function should be followed by an ::Init() method call. This function does
* not manage lifecycle considerations.
*
* @param storageDelegate Storage instance to access persisted ACL data.
* @return a reference to the AccessControl::Delegate singleton.
*/
AccessControl::Delegate & GetAccessControlDelegate(PersistentStorageDelegate * storageDelegate);

} // namespace Examples
} // namespace Access
Expand Down
2 changes: 1 addition & 1 deletion src/access/tests/TestAccessControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ using Entry = AccessControl::Entry;
using EntryIterator = AccessControl::EntryIterator;
using Target = Entry::Target;

AccessControl accessControl(Examples::GetAccessControlDelegate());
AccessControl accessControl(Examples::GetAccessControlDelegate(nullptr));

constexpr ClusterId kOnOffCluster = 0x0006;
constexpr ClusterId kLevelControlCluster = 0x0008;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
*/

#include <access/AccessControl.h>
#include <access/examples/ExampleAccessControlDelegate.h>

#include <app-common/zap-generated/af-structs.h>
#include <app-common/zap-generated/cluster-objects.h>
Expand Down Expand Up @@ -501,16 +500,9 @@ CHIP_ERROR AccessControlAttribute::WriteExtension(AttributeValueDecoder & aDecod

AccessControlAttribute gAttribute;

AccessControl gAccessControl(Examples::GetAccessControlDelegate());
harimau-qirex marked this conversation as resolved.
Show resolved Hide resolved

} // namespace

void MatterAccessControlPluginServerInitCallback()
{
registerAttributeAccessOverride(&gAttribute);

// TODO: move access control setup to lower level
// (it's OK and convenient here during development)
gAccessControl.Init();
SetAccessControl(gAccessControl);
}
9 changes: 8 additions & 1 deletion src/app/server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

#include <app/server/Server.h>

#include <access/examples/ExampleAccessControlDelegate.h>

#include <app/EventManagement.h>
#include <app/InteractionModelEngine.h>
#include <app/server/Dnssd.h>
Expand Down Expand Up @@ -92,7 +94,7 @@ Server::Server() :
.devicePool = &mDevicePool,
.dnsResolver = nullptr,
}), mCommissioningWindowManager(this), mGroupsProvider(mDeviceStorage),
mAttributePersister(mDeviceStorage)
mAttributePersister(mDeviceStorage), mAccessControl(Access::Examples::GetAccessControlDelegate(&mDeviceStorage))
{}

CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint16_t unsecureServicePort)
Expand Down Expand Up @@ -128,6 +130,11 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint
SuccessOrExit(err);
SetGroupDataProvider(&mGroupsProvider);

// Access control must be initialized after mDeviceStorage.
err = mAccessControl.Init();
SuccessOrExit(err);
Access::SetAccessControl(mAccessControl);

// Init transport before operations with secure session mgr.
err = mTransports.Init(UdpListenParameters(DeviceLayer::UDPEndPointManager())
.SetAddressType(IPAddressType::kIPv6)
Expand Down
3 changes: 3 additions & 0 deletions src/app/server/Server.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#pragma once

#include <access/AccessControl.h>
#include <app/CASEClientPool.h>
#include <app/CASESessionManager.h>
#include <app/DefaultAttributePersistenceProvider.h>
Expand Down Expand Up @@ -194,6 +195,8 @@ class Server
app::DefaultAttributePersistenceProvider mAttributePersister;
GroupDataProviderListener mListener;

Access::AccessControl mAccessControl;

// TODO @ceille: Maybe use OperationalServicePort and CommissionableServicePort
uint16_t mSecuredServicePort;
uint16_t mUnsecuredServicePort;
Expand Down