diff --git a/CHANGELOG.md b/CHANGELOG.md index 07f41cc..5700f10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,19 @@ All notable changes to Jarl are documented here. The format follows ### Fixed +- A token for one view of a linked zone (such as an ACME token with + `views=external`) couldn't add records: a new record is shared, and + that reached the other view. A new name and type is now local to the + view when the user may not change it in the other views. +- A zone name in several views had to be given with `--view` even when + the user could see it in only one (`jarl record list adbraun.lol` with + a one-view token), and the error named views the user can't see. Now + the one they can see is used. +- `jarl view list` was empty for a user without `views:read`, though + they can see zones in views. It now lists those views by name, colour + and visible zone count (marked `limited` in the API), without their + settings. + - The DNSSEC tab told you to give the DS record to your registrar for every zone whose parent isn't in Jarl in the same view. Now it (and `jarl zone dnssec status`, and the API's new `ds_at`, `ds_place` and diff --git a/cmd/jarl/config_cmds.go b/cmd/jarl/config_cmds.go index 6688dc0..125f757 100644 --- a/cmd/jarl/config_cmds.go +++ b/cmd/jarl/config_cmds.go @@ -30,7 +30,12 @@ func viewCmd() *cobra.Command { return output(views, func(w io.Writer) { fmt.Fprintln(w, "#\tID\tVIEW\tMATCH-CLIENTS\tMATCH-DESTINATIONS\tZONES\tCOLOR") for i, v := range views { - fmt.Fprintf(w, "%d\t%d\t%s\t%s\t%s\t%d\t%s\n", i+1, v.ID, v.Name, orAny(v.MatchClients), orAny(v.MatchDestinations), v.Zones, orDefault(v.Color)) + clients, dests := orAny(v.MatchClients), orAny(v.MatchDestinations) + if v.Limited { + // No views:read: only the zones they can see. + clients, dests = "(not shown)", "(not shown)" + } + fmt.Fprintf(w, "%d\t%d\t%s\t%s\t%s\t%d\t%s\n", i+1, v.ID, v.Name, clients, dests, v.Zones, orDefault(v.Color)) } }) }, diff --git a/docs/acme.md b/docs/acme.md index d052c2e..423d816 100644 --- a/docs/acme.md +++ b/docs/acme.md @@ -48,7 +48,9 @@ jarl acme unset DOMAIN [VALUE...] # remove these values, or all of them * The zone is found automatically: the longest matching primary zone. * If the zone exists in several **views** (split DNS), the record is written into every view, so the CA (on the internet, in your external view) and - your internal resolvers see the same thing. `--view NAME` limits it to one. + your internal resolvers see the same thing. `--view NAME` limits it to one, + and so does a token for one view (`views=external`): in a linked zone its + records are then local to that view. * All values and views go into one changeset. Setting a value that is already there succeeds, so a retried hook doesn't fail, and removing a value that isn't there succeeds as well. diff --git a/docs/permissions.md b/docs/permissions.md index e44c269..940a6cd 100644 --- a/docs/permissions.md +++ b/docs/permissions.md @@ -160,7 +160,14 @@ records:write zones=acme.adbraun.lol types=TXT only TXT records in a ``` A user who may see some records of a zone may also see that the zone exists -(its name and type), but not its other records. +(its name and type), but not its other records. They also see the zone's +view in `jarl view list` (`GET /views`): its name, colour and how many +zones they can see there, marked `limited`, but not its settings +(`match-clients` and the rest need `views:read`). + +A zone name that is in several views needs a view (`--view`), unless the +user can see the zone in only one of them: a token for +`views=external` finds the external copy without being told. ### Roles diff --git a/docs/views.md b/docs/views.md index 3c01931..f33a166 100644 --- a/docs/views.md +++ b/docs/views.md @@ -222,7 +222,10 @@ anyway), so scripts don't need to know; `--shared` there is refused. a grant for one view only (`views=internal`) can change that view's local records and make overrides, but not a shared record, since that would change the other views too: Jarl says so and suggests a local record. A -grant without a view (the usual `zones=adbraun.lol`) covers every view. +new name and type it adds is local to its view without being asked (an +ACME token for `views=external` adds its `_acme-challenge` TXT records to +the external view only). A grant without a view (the usual +`zones=adbraun.lol`) covers every view. **Importing a zone file** into a linked zone (`jarl zone import`) adds the records like any others: shared, unless the name and type is local diff --git a/internal/api/types.go b/internal/api/types.go index b1d31fa..6f747fd 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -70,6 +70,10 @@ type View struct { model.View Zones int `json:"zone_count"` Color string `json:"color,omitempty"` // web UI colour, see model.ViewColors + // Limited is set for a view listed only because the principal can see + // zones in it (no views:read): only its name, colour and the zones + // they can see are given. + Limited bool `json:"limited,omitempty"` } // ViewColor sets the colour the web UI shows a view in. diff --git a/internal/engine/expand.go b/internal/engine/expand.go index 115bfb4..ce43a48 100644 --- a/internal/engine/expand.go +++ b/internal/engine/expand.go @@ -76,11 +76,13 @@ func (x *expander) resolveZone(zoneID int64, zone, view string) (*model.Zone, er if zone == "" { return nil, errf(KindBadRequest, "the zone is missing (zone_id or zone)") } - return FindZoneRef(x.st, zone, view) + return FindZoneRef(x.st, zone, view, func(z *model.Zone) bool { return x.p.CanSeeZone(z.Name, x.st.ViewName(z)) }) } -// FindZoneRef finds a zone by name and optional view name. -func FindZoneRef(st *model.State, zone, view string) (*model.Zone, error) { +// FindZoneRef finds a zone by name and optional view name. Without a +// view, a name in several views needs one, counting only the zones sees +// allows (nil: all): a token for one view finds that view's zone. +func FindZoneRef(st *model.State, zone, view string, sees func(*model.Zone) bool) (*model.Zone, error) { name, err := ParseZoneName(zone) if err != nil { return nil, errf(KindBadRequest, "%v", err) @@ -97,6 +99,16 @@ func FindZoneRef(st *model.State, zone, view string) (*model.Zone, error) { return z, nil } zs := st.ZonesNamed(name) + if sees != nil && len(zs) > 1 { + var vis []*model.Zone + for _, z := range zs { + if sees(z) { + vis = append(vis, z) + } + } + zs = vis // none: not found, without naming the views + } + st.SortByView(zs) switch len(zs) { case 0: return nil, errf(KindNotFound, "zone %s does not exist", dnsname.Display(name)) diff --git a/internal/engine/linked.go b/internal/engine/linked.go index a9d8a5c..d0463ac 100644 --- a/internal/engine/linked.go +++ b/internal/engine/linked.go @@ -456,12 +456,30 @@ func (x *expander) unlinkOps(m *model.Zone, summary string) []model.Op { // localFor works out whether a new record (or set) in zone z is local: // as asked, or like the records of the same name and type already there. +// A new name and type is shared, unless the principal may not change it +// in every linked view (an ACME token for the external view only): then +// it is local to z's view, which is all they may change. func (x *expander) localFor(z *model.Zone, name, typ string, want *bool) (bool, error) { - has := x.st.HasLocal(z.ID, name, typ) - if want == nil { - return has, nil + if want != nil { + return *want, nil } - return *want, nil + if x.st.HasLocal(z.ID, name, typ) { + return true, nil + } + if len(x.st.SharedRecords(z.ID, name, typ)) > 0 { + return false, nil + } + var not []string + for _, o := range x.st.LinkedWith(z) { + if !x.p.Can(recordAction(x.st, o, name, typ, true)) { + not = append(not, x.st.ViewName(o)) + } + } + if len(not) > 0 { + x.warnings = append(x.warnings, fmt.Sprintf("%s %s is local to view %s: you may not change it in %s", dnsname.Display(name), typ, x.st.ViewName(z), strings.Join(not, ", "))) + return true, nil + } + return false, nil } // shareAllOps returns the operations that start a link with zone z: it diff --git a/internal/engine/linked_test.go b/internal/engine/linked_test.go index b27fbd3..c34219e 100644 --- a/internal/engine/linked_test.go +++ b/internal/engine/linked_test.go @@ -413,3 +413,37 @@ func TestDeleteLocalOverride(t *testing.T) { t.Errorf("validation: %+v", probs) } } + +// An ACME token for one view of a linked zone: its challenge records are +// local to that view, the zone is found without naming the view, and they +// go again without touching the other view. +func TestLinkedACMEToken(t *testing.T) { + env := linkedEnv(t) + env.submit(env.admin, NewRequest(OpZoneCreate, 0, ZoneInput{Name: model.Ptr("example.com"), View: model.Ptr("external"), LinkWith: "internal"})) + u := &model.User{Username: "certbot", Grants: []model.Grant{{Resource: auth.ResRecords, Access: "write", Zones: []string{"example.com"}, Views: []string{"external"}, Names: []string{"_acme-challenge.*"}, Types: []string{"TXT"}}}} + if err := env.st.CreateUser(env.ctx, u); err != nil { + t.Fatal(err) + } + bot := &auth.Principal{User: u, Method: "token"} + name, typ, data := "_acme-challenge", "TXT", "token-value" + res := env.submit(bot, NewRequest(OpRecordCreate, 0, RecordInput{Zone: "example.com", Name: &name, Type: &typ, Data: &data})) + if !slices.Contains(env.view("external"), "_acme-challenge TXT \"token-value\" L") || slices.ContainsFunc(env.view("internal"), func(s string) bool { return strings.HasPrefix(s, "_acme-challenge") }) { + t.Errorf("challenge: internal %q external %q", env.view("internal"), env.view("external")) + } + if !slices.ContainsFunc(res.Warnings, func(w string) bool { return strings.Contains(w, "local to view external") }) { + t.Errorf("warnings %q", res.Warnings) + } + env.submit(bot, NewRequest(OpRecordDelete, 0, RecordInput{Zone: "example.com", Name: &name, Type: &typ, Data: &data})) + if slices.ContainsFunc(env.view("external"), func(s string) bool { return strings.HasPrefix(s, "_acme-challenge") }) { + t.Errorf("not deleted: %q", env.view("external")) + } + // The admin, who may change both, still gets a shared record. + env.submit(env.admin, NewRequest(OpRecordCreate, 0, RecordInput{Zone: "example.com", View: "internal", Name: model.Ptr("both"), Type: model.Ptr("A"), Data: model.Ptr("192.0.2.7")})) + if !slices.Contains(env.view("external"), "both A 192.0.2.7 S") { + t.Errorf("admin: %q", env.view("external")) + } + // Without a view, the admin is still asked which. + if e := env.submitErr(env.admin, NewRequest(OpRecordCreate, 0, RecordInput{Zone: "example.com", Name: model.Ptr("x"), Type: model.Ptr("A"), Data: model.Ptr("192.0.2.8")})); !strings.Contains(e.Message, "several views (internal, external)") { + t.Errorf("admin without a view: %v", e) + } +} diff --git a/internal/server/dsat_test.go b/internal/server/dsat_test.go index 5366580..33a5a97 100644 --- a/internal/server/dsat_test.go +++ b/internal/server/dsat_test.go @@ -49,3 +49,34 @@ func TestZoneInViews(t *testing.T) { t.Errorf("in_views = %v", z["in_views"]) } } + +// A token for records in one view of a zone that is in two: the zone is +// found without naming the view, and view list names the views it can +// see zones in, without their settings. +func TestOneViewToken(t *testing.T) { + e := newAPIEnv(t) + e.do("POST", "/views", e.token, map[string]any{"name": "internal", "match_clients": []string{"10.0.0.0/8"}}, 201) + e.do("POST", "/views", e.token, map[string]any{"name": "external"}, 201) + for _, v := range []string{"internal", "external"} { + e.do("POST", "/zones", e.token, map[string]any{"name": "adbraun.lol", "view": v, "nameservers": []string{"ns1.example.net."}}, 201) + } + e.do("POST", "/users", e.token, map[string]any{"username": "certbot", "password": "", "grant_strings": []string{"records:write zones=adbraun.lol views=external names=_acme-challenge.* types=TXT"}}, 201) + var uid int64 + for _, u := range e.list("/users", e.token) { + if u["username"] == "certbot" { + uid = int64(u["id"].(float64)) + } + } + tok := e.do("POST", "/users/"+itoa(uid)+"/tokens", e.token, map[string]any{"name": "certbot"}, 201)["token"].(string) + + if z := e.do("GET", "/zones/adbraun.lol", tok, nil, 200); z["view"] != "external" { + t.Errorf("zone without a view: %v", z["view"]) + } + e.do("GET", "/zones/adbraun.lol/records", tok, nil, 200) + views := e.list("/views", tok) + if len(views) != 1 || views[0]["name"] != "external" || views[0]["limited"] != true || views[0]["match_clients"] != nil { + t.Errorf("views = %v", views) + } + // The admin still has to say which. + e.do("GET", "/zones/adbraun.lol", e.token, nil, 400) +} diff --git a/internal/server/h_config.go b/internal/server/h_config.go index 0026d7d..30ce340 100644 --- a/internal/server/h_config.go +++ b/internal/server/h_config.go @@ -57,6 +57,8 @@ func (s *Server) listViews(w http.ResponseWriter, r *http.Request, p *auth.Princ for _, v := range st.OrderedViews() { if p.Can(auth.Action{Resource: auth.ResViews, View: v.Name}) { out = append(out, presentView(st, v, colors)) + } else if lv := limitedView(st, v, colors, p); lv != nil { + out = append(out, *lv) } } writeJSON(w, http.StatusOK, out) diff --git a/internal/server/h_zones.go b/internal/server/h_zones.go index 6b74b76..dd112e0 100644 --- a/internal/server/h_zones.go +++ b/internal/server/h_zones.go @@ -34,7 +34,7 @@ func (s *Server) zoneByPath(r *http.Request, st *model.State, p *auth.Principal) name = "." } var err error - z, err = engine.FindZoneRef(st, name, r.URL.Query().Get("view")) + z, err = engine.FindZoneRef(st, name, r.URL.Query().Get("view"), func(z *model.Zone) bool { return p.CanSeeZone(z.Name, st.ViewName(z)) }) if err != nil { return nil, err } diff --git a/internal/server/present.go b/internal/server/present.go index 81d431c..0b7f0f9 100644 --- a/internal/server/present.go +++ b/internal/server/present.go @@ -82,6 +82,22 @@ func presentView(st *model.State, v *model.View, colors map[int64]string) api.Vi return api.View{View: *v, Zones: len(st.ZonesInView(v.ID)), Color: colors[v.ID]} } +// limitedView presents a view to a principal without views:read who can +// see zones in it (the zone list names the view anyway): its name and +// colour, and how many of its zones they can see. nil when they see none. +func limitedView(st *model.State, v *model.View, colors map[int64]string, p *auth.Principal) *api.View { + n := 0 + for _, z := range st.ZonesInView(v.ID) { + if p.CanSeeZone(z.Name, v.Name) { + n++ + } + } + if n == 0 { + return nil + } + return &api.View{View: model.View{ID: v.ID, Name: v.Name, Position: v.Position}, Zones: n, Color: colors[v.ID], Limited: true} +} + func presentACL(st *model.State, a *model.ACL) api.ACL { used := engine.FindACLRefs(st, a.Name) if used == nil {