Siyao Meng created HDDS-17249:
---------------------------------
Summary: 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
Reporter: Siyao Meng
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]
