[
https://issues.apache.org/jira/browse/HDDS-17249?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17249:
------------------------------
Attachment: MC-1-check-bucket-again-when-applying-snapshot-create.patch
> Snapshot create does not check the bucket again at apply
> --------------------------------------------------------
>
> Key: HDDS-17249
> URL: https://issues.apache.org/jira/browse/HDDS-17249
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 1.4.0, 2.0.0, 1.4.1, 2.1.0, 2.2.0, 2.1.1, 2.1.2, 2.2.1
> Reporter: Siyao Meng
> Priority: Major
> Attachments:
> MC-1-check-bucket-again-when-applying-snapshot-create.patch
>
>
> h3. Mechanism
> {{OMSnapshotCreateRequest.preExecute}} resolves the bucket and, when admin
> authorization is enabled, checks that the caller owns it or is an admin. The
> CreateSnapshot request then carries only the caller, the volume and bucket
> names, the snapshot name and ID and the creation time.
> {{OMSnapshotCreateRequest.validateAndUpdateCache}} looks the bucket up again
> by name under the bucket lock ({{getBucketInfo}}, which reads the bucket
> table cache) and uses the result without checking it:
> * {{omBucketInfo.getUsedBytes()}} is called on the lookup result without a
> null check, so a missing bucket ends in a {{NullPointerException}} instead of
> {{BUCKET_NOT_FOUND}}.
> * The snapshot is created on whatever bucket the lookup returns, and the
> owner or admin check is not made against that bucket.
> The dereference came with the snapshot referenced size (HDDS-9159); the owner
> or admin check has been in {{preExecute}} only since the request was added.
> h3. Trigger
> At apply, the bucket table cache has no entry for the bucket, or holds a
> bucket other than the one {{preExecute}} checked (a different objectID).
> h3. Impact
> Reproduced on unmodified source through the request classes, with the bucket
> table cache entry changed directly before {{validateAndUpdateCache}}:
> * No bucket: {{validateAndUpdateCache}} throws {{NullPointerException: Cannot
> invoke "OmBucketInfo.getUsedBytes()" because "omBucketInfo" is null}}.
> * A bucket with another owner and objectID: the apply returns OK and the
> snapshot is created on it, with no owner or admin check against it.
> h3. Reproduction
> No standalone reproduction is attached. The two tests in the patch run the
> real {{preExecute}} and {{validateAndUpdateCache}} against a real
> {{OmMetadataManagerImpl}} and change only the bucket table cache entry in
> between; both fail on unmodified source (see Patch).
> h3. Patch
> [^MC-1-check-bucket-again-when-applying-snapshot-create.patch], against
> 20d95ed37fe35013b5320713665db03c06ee36fa. It also applies to master at
> 64d4a791502.
> {{validateAndUpdateCache}} now throws {{OMException(BUCKET_NOT_FOUND)}} when
> {{getBucketInfo}} returns null and, when admin authorization is enabled,
> re-checks owner or admin against the bucket it found, with the rule
> {{preExecute}} uses (moved into a small helper used by both). The exception
> takes the request's normal error path: the snapshot chain and the snapshot
> table cache are untouched, the locks and the snapshot limit reservation taken
> by {{OmSnapshotManager.snapshotLimitCheck}} are released, and an error
> response is returned. Other bucket requests also check authorization at
> apply, for example {{OMBucketSetPropertyRequest}} with the same owner or
> admin rule.
> A stricter alternative is to bind the snapshot to the bucket objectID
> recorded in {{preExecute}}, in a new optional {{CreateSnapshotRequest}}
> field, and reject at apply when it differs; that would also reject a recreate
> by the same owner, which the authorization check accepts. The null check is
> needed in either variant. The open pull request apache/ozone#10965
> (HDDS-16060) adds a check on the same {{omBucketInfo}} right after this
> lookup; the two changes touch the same lines, and whichever lands second
> keeps the null check ahead of that check.
> The check changes what an OM applies for such an entry. With the current
> upgrade, which goes through OM prepare, OMs of two versions do not apply the
> same entries; under zero downtime upgrade this apply change, like others,
> would need a version flag.
> Covered by {{testValidateAndUpdateCacheBucketNotFound}} and
> {{testValidateAndUpdateCacheChecksCurrentBucketOwner}} (owner on the bucket
> {{preExecute}} checked, replacement bucket with another owner, and an admin
> on that bucket) in the existing {{TestOMSnapshotCreateRequest}}. Without the
> change they fail with the {{NullPointerException}} above and with "expected:
> <PERMISSION_DENIED> but was: <OK>". With it the snapshot request, response
> and service unit suites pass (270 tests), as do
> {{TestOzoneManagerSnapshotAcl}} and {{TestOzoneManagerHASnapshot}} (67
> tests), and checkstyle is clean.
> Found by TLA+ model checking and code review of the OM volume, bucket and
> snapshot lifecycle paths under HDDS-15926, on commit
> 20d95ed37fe35013b5320713665db03c06ee36fa. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. Related but not a
> duplicate: HDDS-16744 (snapshot limit counter not released for a retried
> create). The attached patch is a proposal for review. Generated with Specula
> (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
