TrueCloudLab/frostfs-node

Fork 27

Support secondary keys in FrostFSID contract #1436

New issue

Open

opened 2024-10-17 13:21:35 +00:00 by alexvanin · 8 comments

alexvanin commented

2024-10-17 13:21:35 +00:00

Owner

FrostFSID contract supports multiple additional keys for subject. It means, that single subject may operate with different wallet files. Requests signed by these wallet must be handled correctly by node and inner ring.

Describe the solution you'd like

(if necessary) Update frostfs-cli or frostfs-adm to support addition of secondary keys to a subject

Update code of subject fetchers:

use GetSubjectByKey in FormFrostfsIDRequestProperties
use GetSubjectByKey in Groups
maybe something else.

Describe alternatives you've considered

None.

Additional context

In private installation, there might be service account that exists on all network nodes with different unique wallets. Instead of having multiple subjects in the same service group, it is possible to keep single data-path subject and register several wallets for it.

## Is your feature request related to a problem? Please describe. FrostFSID contract supports multiple [additional keys](https://git.frostfs.info/TrueCloudLab/frostfs-contract/src/commit/3f4f8feca799c45e2a74374bf6046758cfd584e9/frostfsid/frostfsid_contract.go#L26) for subject. It means, that single subject may operate with different wallet files. Requests signed by these wallet must be handled correctly by node and inner ring. ## Describe the solution you'd like (if necessary) Update frostfs-cli or frostfs-adm to support addition of secondary keys to a subject Update code of subject fetchers: * use `GetSubjectByKey` in [FormFrostfsIDRequestProperties](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/c0a2f20eee5e6279e71cfc3d83f8bb89d0a2ef75/pkg/ape/request/frostfsid.go#L17) * use `GetSubjectByKey` in [Groups](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/c0a2f20eee5e6279e71cfc3d83f8bb89d0a2ef75/pkg/ape/request/frostfsid.go#L40) * maybe something else. ## Describe alternatives you've considered None. ## Additional context In private installation, there might be service account that exists on all network nodes with different unique wallets. Instead of having multiple subjects in the same service group, it is possible to keep single data-path subject and register several wallets for it.

alexvanin added the

frostfs-ir

frostfs-node

triage

labels 2024-10-17 13:21:36 +00:00

alexvanin commented

2024-10-17 13:22:20 +00:00

Author

Owner

Similar to https://git.frostfs.info/TrueCloudLab/frostfs-s3-gw/issues/518

fyrchik commented

2024-10-17 13:41:51 +00:00

Owner

Can we have the same key listed as the main one for one account and as an additional one for another?
If yes, is it what we want?

Can we have the same key listed as the main one for one account and as an additional one for another? If yes, is it what we want?

alexvanin commented

2024-10-17 13:54:09 +00:00

Author

Owner

Can we have the same key listed as the main one for one account and as an additional one for another?

I think contract allows it.

If yes, is it what we want?

I would rather restrict it on contract level, but may be there are other opinions. I will create new issue in contract repo.

> Can we have the same key listed as the main one for one account and as an additional one for another? I think contract allows it. > If yes, is it what we want? I would rather restrict it on contract level, but may be there are other opinions. I will create new issue in contract repo.

alexvanin referenced this issue from TrueCloudLab/frostfs-contract

2024-10-17 14:03:15 +00:00

Restrict adding primary key of one subject as additional key of another subject #118

alexvanin commented

2024-10-17 14:03:29 +00:00

Author

Owner

Contract discussion here: TrueCloudLab/frostfs-contract#118

Contract discussion here: https://git.frostfs.info/TrueCloudLab/frostfs-contract/issues/118

fyrchik commented

2024-10-17 14:40:32 +00:00

Owner

Restricting it on a contract level is a MUST, otherwise we cannot really rely on it anywhere. This can lead to all kinds of surprising behaviours.

👍 2

fyrchik added a new dependency 2024-10-17 14:40:40 +00:00

TrueCloudLab/frostfs-contract#118 - Restrict adding primary key of one subject as additional key of another subject

fyrchik commented

2024-11-18 06:18:54 +00:00

Owner

Requests signed by these wallet must be handled correctly by node and inner ring.

Could you describe the correct behaviour w.r.t to APE rules?

>Requests signed by these wallet must be handled correctly by node and inner ring. Could you describe the correct behaviour w.r.t to APE rules?

fyrchik added this to the v0.44.0 milestone 2024-11-18 08:26:20 +00:00

alexvanin commented

2024-11-18 09:15:55 +00:00

Author

Owner

Could you describe the correct behaviour w.r.t to APE rules?

Consider Alice having primary key key_1 and secondary keys key_2 and key_3.

Examples:

Request is signed by key_2. Before checking policies, APE identifies that key_2 is associated with Alice and her public key from key_1. Node searches for all policies attached to Alice and her groups. If chain rule contains condition with public key of key_1, this condition should not match (*)
The same applies to the request that contains bearer token with allowImpersonate flag and signed by key_3.
As for Inner Ring, during container creation, it should verify, that ownerID of container is set to Alice (address of key_1) even if request is signed by key_2 or key_3.
During all container owner check (if there any), key_2 and key_3 signatures must be interpreted as key_1 signature and key_1 owner.

(*) User should be able to make condition to match all it's keys. For me, this is primary case we should implement. S3 / IAM policy converter uses public key condition. It will be broken after secondary key support.

To fix it later, we have to introduce new condition and use it in converter. Also migrate existing rules.

The alternative is to match public_key=key_1 with any secondary key.

> Could you describe the correct behaviour w.r.t to APE rules? Consider Alice having primary key `key_1` and secondary keys `key_2` and `key_3`. Examples: * Request is signed by `key_2`. Before checking policies, APE identifies that `key_2` is associated with Alice and her public key from `key_1`. Node searches for all policies attached to Alice and her groups. If chain rule contains condition with public key of `key_1`, this condition should not match (*) * The same applies to the request that contains bearer token with `allowImpersonate` flag and signed by `key_3`. * As for Inner Ring, during container creation, it should verify, that ownerID of container is set to Alice (address of `key_1`) even if request is signed by `key_2` or `key_3`. * During all container owner check (if there any), `key_2` and `key_3` signatures must be interpreted as `key_1` signature and `key_1` owner. --- (*) User should be able to make condition to match all it's keys. For me, this is primary case we should implement. S3 / IAM policy converter [uses public key condition](https://git.frostfs.info/TrueCloudLab/policy-engine/src/commit/a3bc3099bd5bfefd702b44070d58982e477a56e9/iam/converter_native.go#L238). **It will be broken after secondary key support**. To fix it later, we have to introduce new condition and use it in converter. Also migrate existing rules. The alternative is to match `public_key=key_1` with any secondary key.

fyrchik commented

2024-11-19 12:57:52 +00:00

Owner

@alexvanin, please, look at TrueCloudLab/frostfs-contract#129
It solves the problem for container creation and it seems sufficient for now.

@alexvanin, please, look at https://git.frostfs.info/TrueCloudLab/frostfs-contract/pulls/129 It solves the problem for container creation and it seems sufficient for now.

👍 1