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

multiple collaborators per certificate #944

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 3 additions & 1 deletion openfl/component/aggregator/aggregator.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def __init__(self,
aggregator_uuid,
federation_uuid,
authorized_cols,
cn_mapping,

init_state_path,
best_state_path,
Expand Down Expand Up @@ -75,6 +76,7 @@ def __init__(self,

# if the collaborator requests a delta, this value is set to true
self.authorized_cols = authorized_cols
self.cn_mapping = cn_mapping
self.uuid = aggregator_uuid
self.federation_uuid = federation_uuid
self.assigner = assigner
Expand Down Expand Up @@ -225,7 +227,7 @@ def valid_collaborator_cn_and_id(self, cert_common_name,
# FIXME: '' instead of None is just for protobuf compatibility.
# Cleaner solution?
if self.single_col_cert_common_name == '':
return (cert_common_name == collaborator_common_name
return (cert_common_name == self.cn_mapping[collaborator_common_name]
Copy link
Author

Choose a reason for hiding this comment

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

TODO: handle error

Copy link
Author

Choose a reason for hiding this comment

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

possible key error

and collaborator_common_name in self.authorized_cols)
# otherwise, common_name must be in whitelist and
# collaborator_common_name must be in authorized_cols
Expand Down
10 changes: 9 additions & 1 deletion openfl/federated/plan/plan.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,15 @@ def parse(plan_config_path: Path, cols_config_path: Path = None,
gandlf_config['output_dir'] = gandlf_config.get('output_dir', '.')
plan.config['task_runner']['settings']['gandlf_config'] = gandlf_config

plan.authorized_cols = Plan.load(cols_config_path).get(
cols_info = Plan.load(cols_config_path).get(
'collaborators', []
)
if isinstance(cols_info, list):
plan.cn_mapping = {col: col for col in cols_info}
plan.authorized_cols = cols_info
else:
plan.cn_mapping = cols_info
plan.authorized_cols = list(cols_info.keys())

# TODO: Does this need to be a YAML file? Probably want to use key
# value as the plan hash
Expand Down Expand Up @@ -223,6 +229,7 @@ def __init__(self):
"""Initialize."""
self.config = {} # dictionary containing patched plan definition
self.authorized_cols = [] # authorized collaborator list
self.cn_mapping = {} # expected cert common name for each collaborator
self.cols_data_paths = {} # collaborator data paths dict

self.collaborator_ = None # collaborator object
Expand Down Expand Up @@ -338,6 +345,7 @@ def get_aggregator(self, tensor_dict=None):
defaults[SETTINGS]['aggregator_uuid'] = self.aggregator_uuid
defaults[SETTINGS]['federation_uuid'] = self.federation_uuid
defaults[SETTINGS]['authorized_cols'] = self.authorized_cols
defaults[SETTINGS]['cn_mapping'] = self.cn_mapping
defaults[SETTINGS]['assigner'] = self.get_assigner()
defaults[SETTINGS]['compression_pipeline'] = self.get_tensor_pipe()
defaults[SETTINGS]['straggler_handling_policy'] = self.get_straggler_handling_policy()
Expand Down
3 changes: 2 additions & 1 deletion openfl/interface/collaborator.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ def register_data_path(collaborator_name, data_path=None, silent=False):
def generate_cert_request_(collaborator_name,
silent, skip_package):
"""Generate certificate request for the collaborator."""
# TODO: this should take an extra argument: common_name
generate_cert_request(collaborator_name, silent, skip_package)


Expand Down Expand Up @@ -304,7 +305,7 @@ def certify(collaborator_name, silent, request_pkg=None, import_=False):
from openfl.utilities.utils import rmtree

common_name = f'{collaborator_name}'.lower()

# TODO: read and parse CSR and use the actual CN
if not import_:
if request_pkg:
Path(f'{CERT_DIR}/client').mkdir(parents=True, exist_ok=True)
Expand Down
1 change: 1 addition & 0 deletions openfl/interface/interactive_api/experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ def _prepare_plan(self, model_provider, data_loader,
self.plan.authorized_cols = [
name for name, info in shard_registry.items() if info['is_online']
]
self.plan.cn_mapping = {name:name for name in self.plan.authorized_cols}
# Network part of the plan
# We keep in mind that an aggregator FQND will be the same as the directors FQDN
# We just choose a port randomly from plan hash
Expand Down
2 changes: 1 addition & 1 deletion openfl/transport/grpc/aggregator_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def validate_collaborator(self, request, context):
common_name, collaborator_common_name):
# Random delay in authentication failures
sleep(5 * random()) # nosec
context.abort(
context.abort( # TODO? add the expected CN in the error msg
StatusCode.UNAUTHENTICATED,
f'Invalid collaborator. CN: |{common_name}| '
f'collaborator_common_name: |{collaborator_common_name}|')
Expand Down
Loading