Docker: an image a selector of the caller's grant hides is not found (404)

crates/server/src/images/dispatch.rs

@@ -400,40 +400,64 @@ impl Registry {
     }
 
     /// Whether the caller may `action` the target's image: the repository
-    /// permission and its content selectors, then a registry token's scope
-    /// (checked against the name the client used).
+    /// permission and its content selectors ([`Self::rbac_decision`]), then
+    /// a registry token's scope (checked against the name the client used).
     pub(crate) async fn decide(&self, caller: &Caller, target: &Target, action: Level) -> Decision {
-        if !self
-            .rbac_allows(&caller.principal, &target.repo, &target.image, action)
+        match self
+            .rbac_decision(&caller.principal, &target.repo, &target.image, action)
             .await
         {
-            return Decision::Deny;
+            Decision::Allow => {}
+            refused => return refused,
         }
         match &caller.claims {
             Some(claims) if !claims_allow(claims, &target.name, action) => Decision::NeedsScope,
             _ => Decision::Allow,
         }
     }
 
-    /// The RBAC part of [`Self::decide`].
-    pub(crate) async fn rbac_allows(
+    /// The RBAC part of [`Self::decide`]: the repository permission, then
+    /// its content selectors. An image a selector hides from a caller who
+    /// may read the repository is [`Decision::Hidden`] to a read, and
+    /// [`Decision::Deny`] to a push or a delete.
+    async fn rbac_decision(
         &self,
         principal: &Principal,
         repo: &Repository,
         image: &str,
         action: Level,
-    ) -> bool {
+    ) -> Decision {
         let authz = &self.inner.authz;
         let allowed = match action {
             Level::Read => authz.can_read(principal, repo).await,
             Level::Write => authz.can_write(principal, repo).await,
             Level::Delete => authz.can_delete(principal, repo).await,
         };
-        allowed
-            && authz
-                .package_filter_for(principal, repo, action)
-                .await
-                .allows(image)
+        if !allowed {
+            Decision::Deny
+        } else if authz
+            .package_filter_for(principal, repo, action)
+            .await
+            .allows(image)
+        {
+            Decision::Allow
+        } else if action == Level::Read {
+            Decision::Hidden
+        } else {
+            Decision::Deny
+        }
+    }
+
+    /// Whether RBAC allows the caller `action` on `repo`'s `image`
+    /// ([`Self::rbac_decision`]).
+    pub(crate) async fn rbac_allows(
+        &self,
+        principal: &Principal,
+        repo: &Repository,
+        image: &str,
+        action: Level,
+    ) -> bool {
+        self.rbac_decision(principal, repo, image, action).await == Decision::Allow
     }
 
     /// Where a push or delete addressed to `target` lands (the repository
@@ -453,7 +477,12 @@ impl Registry {
         let landing = match target.repo.kind() {
             RepoKind::Hosted => target.clone(),
             RepoKind::Proxy | RepoKind::Group => {
-                let decision = self.decide(caller, target, Level::Read).await;
+                // A push of an image a selector hides is refused, not
+                // answered as a name that does not exist.
+                let decision = match self.decide(caller, target, Level::Read).await {
+                    Decision::Hidden => Decision::Deny,
+                    decision => decision,
+                };
                 if decision != Decision::Allow {
                     return Err(self.refusal(
                         caller,
@@ -523,7 +552,9 @@ impl Registry {
 
     /// The refusal of `permission` on `name`: a challenge (401) when
     /// credentials or a token for the scope could help, a denial (403) for
-    /// an authenticated caller the permission is missing for.
+    /// an authenticated caller the permission is missing for, and for an
+    /// image a selector hides the answer to a name that does not exist
+    /// (404 `NAME_UNKNOWN`).
     pub(crate) fn refusal(
         &self,
         caller: &Caller,
@@ -535,6 +566,9 @@ impl Registry {
         let authenticated = mutation_credential(&caller.principal);
         let error = caller.claims.as_ref().map(|_| "insufficient_scope");
         match decision {
+            Decision::Hidden => Failure::new(ImageError::NameUnknown {
+                name: name.to_string(),
+            }),
             Decision::NeedsScope => challenge(site, name, permission, error),
             Decision::Deny if !authenticated => challenge(site, name, permission, error),
             _ => Failure::new(ImageError::Denied {
@@ -1067,6 +1101,11 @@ pub(crate) enum Decision {
     /// RBAC allows it, but the registry token presented does not cover it:
     /// the client should fetch a token for this scope.
     NeedsScope,
+    /// The caller may read the repository, but a content selector of its
+    /// grant hides the image: a read is answered as for a name that does
+    /// not exist (404), as the other formats answer a package a selector
+    /// hides.
+    Hidden,
     Deny,
 }
 

crates/server/tests/registry.rs

@@ -3515,13 +3515,87 @@ async fn group_browse_lists_what_its_pulls_serve() {
     assert_eq!(listed(reader), [("other/app".to_string(), 1)]);
 }
 
+/// An image a selector of the caller's grant hides is not found (#703): a
+/// read of it answers 404 `NAME_UNKNOWN` on every endpoint, as a name that
+/// does not exist, where it answered 403, while a push outside the
+/// selector is still refused (403), and a caller who may not read the
+/// repository at all is refused as before.
+#[tokio::test]
+async fn an_image_a_selector_hides_is_not_found() {
+    let srv = common::start().await;
+    let repo = repo_name("sel");
+    create_repo(
+        &srv,
+        json!({"name": repo, "kind": "hosted", "format": "docker"}),
+    )
+    .await;
+    let admin = admin_oci(&srv).await;
+    let digest = admin
+        .push_image(&format!("{repo}/secret/app"), "1", "amd64", b"secret layer")
+        .await;
+    admin
+        .push_image(&format!("{repo}/public/app"), "1", "amd64", b"public layer")
+        .await;
+    let selector = srv.create_selector(&["public/*"]).await;
+    let caller = srv.caller().await;
+    srv.grant_role(caller.id, &[(&repo, "write", Some(selector))])
+        .await;
+    let oci = Oci::new(&srv, Some((caller.username.clone(), caller.token.clone())));
+
+    let hidden = format!("{repo}/secret/app");
+    for path in [
+        "manifests/1".to_string(),
+        format!("manifests/{digest}"),
+        format!("blobs/{}", sha(b"secret layer")),
+        "tags/list".to_string(),
+        format!("referrers/{digest}"),
+    ] {
+        let resp = oci.get(&hidden, &path).await;
+        assert_eq!(resp.status(), 404, "{path}");
+        let body: Value = resp.json().await.unwrap();
+        assert_eq!(body["errors"][0]["code"], "NAME_UNKNOWN", "{path}: {body}");
+        assert_eq!(oci.head(&hidden, &path).await.status(), 404, "HEAD {path}");
+    }
+    assert_eq!(
+        oci.get(&format!("{repo}/public/app"), "manifests/1")
+            .await
+            .status(),
+        200
+    );
+    // No push outside the selector, and no delete of what it hides.
+    let resp = oci
+        .request(
+            Method::POST,
+            &format!("/v2/{repo}/secret/new/blobs/uploads/"),
+            "pull,push",
+            &format!("{repo}/secret/new"),
+        )
+        .await
+        .send()
+        .await
+        .unwrap();
+    assert_eq!(resp.status(), 403);
+    let resp = oci.delete(&hidden, &format!("manifests/{digest}")).await;
+    assert!(
+        resp.status() == 401 || resp.status() == 403,
+        "{}",
+        resp.status()
+    );
+
+    // Who may not read the repository is refused, not told it is absent.
+    let stranger = srv.caller().await;
+    let oci = Oci::new(&srv, Some((stranger.username, stranger.token)));
+    assert_eq!(oci.get(&hidden, "manifests/1").await.status(), 403);
+}
+
 /// A group shows a caller what it could pull from its members directly, no
 /// more (#648): with a member it may not read, a selector on the group's
 /// grant or one on a member's, a hidden image's manifests, blobs, tags and
 /// referrers answer as an image of a name like it that does not exist
-/// (404, or 403 for a name outside a selector of the group's own grant,
-/// which grants no `pull` on it), and the browse API's image list (its
-/// count too), image page and chart page leave it out.
+/// (404; a selector of the group's own grant answers so too, #703, and a
+/// readable name's referrers of an unknown subject are an empty index), and
+/// the browse API's image list (its count too), image page and chart page
+/// leave it out.
 #[tokio::test]
 async fn groups_show_a_caller_only_the_images_it_may_read() {
     let srv = common::start().await;
@@ -3613,7 +3687,7 @@ async fn groups_show_a_caller_only_the_images_it_may_read() {
                 let (expected, absent_body) = (resp.status(), resp.text().await.unwrap());
                 // Referrers of an unknown subject are an empty index.
                 assert!(
-                    matches!(expected.as_u16(), 403 | 404) || path.starts_with("referrers/"),
+                    expected == 404 || path.starts_with("referrers/"),
                     "{label}: {absent} {path}: {expected}"
                 );
                 assert_eq!(status, expected, "{label}: {name} {path}: {body}");

docs/changes/2.2.0/703.fixed.md

@@ -0,0 +1,7 @@
+A docker pull of an image a content selector of the caller's grant hides
+was refused with 403 (`DENIED`, 401 for an anonymous caller) when the
+selector was on the repository pulled from, a group's included; through a
+group's member it answered 404. Such an image is now not found on every
+endpoint (manifests, blobs, tags and referrers answer 404 `NAME_UNKNOWN`),
+as a package a selector hides is in the other formats. A push or a delete
+outside the selector is still refused with 403.

docs/docker.md

@@ -83,7 +83,11 @@ docker push forge.example.com/docker-hosted/team/app:1.3
 
 A registry token grants only what the caller may do when it is issued:
 `pull` needs `read` on the repository, `push` needs `write`, and `delete`
-needs `delete`; content selectors match image names (`team/*`). One token
+needs `delete`; content selectors match image names (`team/*`). An image a
+selector hides from a caller who may read the repository is not found: a
+pull, its tags or its referrers answer 404 `NAME_UNKNOWN`, as for a name
+that does not exist (also through a group), while a push or a delete of it
+is refused with 403. One token
 request may name at most 32 repositories (clients ask for one to three); a
 request naming more is refused with `400 UNSUPPORTED`. Every
 request is authorized again against the current permissions, and the user