Skip to content

feat(r-p1): make the portal configuration a release prerequisite, and say so everywhere - #272

Merged
guimard merged 1 commit into
mainfrom
doc/r-p1-release-prerequisite
Sep 14, 2026
Merged

guimard merged 1 commit into
mainfrom
doc/r-p1-release-prerequisite

Conversation

@guimard

@guimard guimard commented Sep 8, 2026

Copy link
Copy Markdown
Member

Item 5 of #268 — the release gate, and the only item that changes what the
product claims.

The decision

With pamAccessServerGroups empty — the shipped default, and the
multi-group model doc/bastion-architecture.md recommended — server_group is
read from the request body, so any enrolled host of the project that is
compromised can declare itself a bastion on /pam/authorize and obtain a hop
voucher for a user. allowed_bastions empty still means "accept any", for
Ansible compatibility.

#268 named two honest positions. This PR takes the first: release
prerequisite
, not accepted residual risk. A shipped default that contradicts
the configuration the architecture document recommends is a condition of
service, and it is not recoverable after the fact — a host compromised during
the window already has its vouchers.

Record

  • CE03 loses its "ou mode hérité explicitement accepté" escape and becomes
    blocking; CE06 and CE16 are marked blocking.
  • CE21 is new: pamAccessAllowedRps + bastions under a client_id of their
    own. Without it the upstream audience binding does nothing at all — an empty
    list means "no change".
  • MT40 and MT50 become publication prerequisites; MT53 is the
    pamAccessAllowedRps row the plan had no entry for.
  • The R-P1 sheet says the residual score is no longer conditioned on an
    assumption.

Three operator docs said the opposite

doc/llng-configuration.md, doc/admin-guide.md and
doc/bastion-architecture.md all told the reader to leave
pamAccessServerGroups empty "for the usual model of one client_id per
project covering several server groups"
. That model is precisely the
configuration in which the gap is exploitable. All three now say the opposite —
including what it costs: one client_id per server group, because an
unmapped one is refused. Saying "this is now required" without saying "and here
is the enrolment work it implies" would only move the surprise.

Enforcement (what is possible, and what is not)

The product can neither set nor verify the portal half. These are LLNG Manager
settings, and reading them back from a host would need an API that publishes the
project's bastions, server groups and RPs — which is exactly what an SSO must
not offer. So the enforcement is declarative, and it is on every path an
operator takes:

where what
ob-bastion-setup, ob-backend-setup, ob-desktop-setup printed in the summary, every run
ob-post-upgrade printed — it is how an existing host reaches 0.7.0, the only path to hosts set up before any of this existed
ob-builder writes PORTAL-CHECKLIST.md next to each artefact, pre-filled with that deployment's client_id and server_group
UPGRADE-NOTES.md new B0; Part B stops being "only if you are also moving the portal"

The builder checklist matters most for the Ansible role, whose setup task runs
with nobody reading its stdout.

The text is shipped once, at
/usr/lib/open-bastion/ob-portal-prerequisites.txt, and packaged in the .deb
and the .rpm as well as by cmake — a helper that existed in the tree and not
in the package is #263, and a warning nobody receives is worse than none.

Tests

tests/test_ob_portal_prerequisites.sh runs the code rather than reading it:
it executes each script's printer, runs ob-post-upgrade --dry-run, and drives
ob-builder's run_outputs_for_role with the renderers stubbed to check both
artefact kinds get a checklist with no unresolved @@PLACEHOLDER@@.

$ bash tests/test_ob_mutation.sh
  PASS: portal-prereq-bastion-setup — tests/test_ob_portal_prerequisites.sh catches it
  PASS: portal-prereq-post-upgrade — tests/test_ob_portal_prerequisites.sh catches it
  PASS: portal-prereq-builder-ansible — tests/test_ob_portal_prerequisites.sh catches it
Tests run: 14, passed: 14, failed: 0

Every tests/test_ob_*.sh suite green, ctest 22/22.

Also

The dossier's CE02 row was repaired: unescaped pipes inside the
^/ssh/(admin|certs|revoke) regex had collapsed it into the neighbouring
columns, taking R-S4 and R-P2 with it. It is the table this PR adds a row to.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@guimard
guimard force-pushed the doc/r-p1-release-prerequisite branch from ac59bbf to 00c8cf8 Compare September 8, 2026 12:41
@orontee

orontee commented Sep 10, 2026

Copy link
Copy Markdown

Is it expected that the CI job "Build Debian Package" fails silently? See https://github.com/linagora/open-bastion/actions/runs/34227502715/job/102065857202

CPack: Create package using DEB
CPack: Install projects
CPack: - Run preinstall target for: open-bastion
CPack: - Install project: open-bastion []
CMake Error: failed to create symbolic link '/home/runner/work/open-bastion/open-bastion/build/_CPack_Packages/Linux/DEB/open-bastion-0.6.2-Linux/usr/local/sbin/ob-standalone-setup': No such file or directory
CMake Error: failed to create symbolic link '/home/runner/work/open-bastion/open-bastion/build/_CPack_Packages/Linux/DEB/open-bastion-0.6.2-Linux/usr/local/share/man/man8/ob-standalone-setup.8': No such file or directory
CPack: Create package
CPack: - package: /home/runner/work/open-bastion/open-bastion/build/open-bastion-0.6.2-Linux.deb generated.

Note that cd build; cpack -G DEB fails on my workstation too with same error.

@guimard

guimard commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

@orontee Good catch, thanks — no, it is not expected, and it is not specific to this PR: main's CI shows the same two lines.

The cpack .deb really ships without ob-standalone-setup and its man page. The two links were created by install(CODE) against CMAKE_INSTALL_FULL_SBINDIR / CMAKE_INSTALL_FULL_MANDIR, which are fixed at configure time (/usr/local/...), while cpack installs under /usr: the parent directory does not exist in the staging tree, create_symlink fails, execute_process ignores it and cpack still exits 0.

The released packages are not affected: dpkg-buildpackage and rpmbuild configure with prefix /usr, so the paths match there (and both list the link explicitly).

Fix in #273: the links are now installed with install(FILES) and a relative destination, and the CI job fails when cpack reports a CMake Error.

… say so everywhere

R-P1 is the release gate of #268 and the only item in it that changes what the
product claims. With `pamAccessServerGroups` empty -- the shipped default, and
the multi-group model doc/bastion-architecture.md recommended -- `server_group`
is read from the request body, so any enrolled host of the project that is
compromised can declare itself a bastion on /pam/authorize and obtain a hop
voucher for a user. `allowed_bastions` empty still means "accept any", for
Ansible compatibility.

Two positions were tenable: make the configuration a documented prerequisite, or
accept it as a signed residual risk. This takes the first. A shipped default
that contradicts the configuration the architecture document recommends is not a
residual risk, it is a condition of service, and it is not recoverable after the
fact: a host compromised during the window already has its vouchers.

CE03 loses its "ou mode hérité explicitement accepté" escape and becomes
blocking, CE06 and CE16 are marked blocking, and CE21 is new --
`pamAccessAllowedRps` plus bastions enrolled under a client_id of their own,
without which the upstream audience binding does nothing at all, since an empty
list means "no change". MT40 and MT50 become publication prerequisites and MT53
is their `pamAccessAllowedRps` counterpart, which the plan had no row for. The
R-P1 sheet says the residual score is no longer conditioned on an assumption.

Three operator documents told the reader to leave `pamAccessServerGroups`
empty -- "for the usual model of one client_id per project covering several
server groups". That model is precisely the configuration in which the gap is
exploitable, so all three now say the opposite, including what it costs: one
client_id per server group, because an unmapped one is refused. Saying "this is
now required" without saying "and here is the enrolment work it implies" would
just move the surprise.

The product can neither set nor verify the portal half. These are Manager
settings, and reading them back from a host would need an API that publishes the
project's bastions, server groups and RPs -- which is exactly what an SSO must
not offer. The only enforcement available is declarative, so it is put on every
path an operator takes:

  - ob-bastion-setup, ob-backend-setup and ob-desktop-setup print the
    requirement in their summary, every run. A desktop is not a bastion, but it
    is an enrolled host, which is the whole point of the risk;
  - ob-post-upgrade prints it too -- it is the path an existing host takes to
    0.7.0, the only one that reaches hosts set up before any of this existed;
  - ob-builder writes PORTAL-CHECKLIST.md next to each artefact, pre-filled with
    that deployment's client_id and server_group. That one matters most for the
    Ansible role, whose setup task runs with nobody reading its stdout;
  - UPGRADE-NOTES.md gets B0, and Part B stops being "only if you are also
    moving the portal".

The text is shipped once, at /usr/lib/open-bastion/ob-portal-prerequisites.txt,
and packaged in the deb and the rpm as well as by cmake -- a helper that existed
in the tree and not in the package is #263, and a warning nobody receives is
worse than none.

tests/test_ob_portal_prerequisites.sh runs the code rather than reading it: it
executes each script's printer, runs ob-post-upgrade --dry-run, and drives
ob-builder's run_outputs_for_role with the renderers stubbed to check both
artefact kinds get a checklist with no unresolved placeholder. Three catalogue
entries, all caught; the whole catalogue is 14/14.

The dossier's CE02 row was also repaired: unescaped pipes inside the
`^/ssh/(admin|certs|revoke)` regex had collapsed it into the neighbouring
columns, taking R-S4 and R-P2 with it. It is the table this commit adds a row
to, and an evaluator reads it.

Closes part 5 of #268.
@guimard
guimard force-pushed the doc/r-p1-release-prerequisite branch from 00c8cf8 to 9c7a7f6 Compare September 10, 2026 20:56
@orontee

orontee commented Sep 14, 2026

Copy link
Copy Markdown

@guimard I confirm that #273 fixed the package build.

@guimard

guimard commented Sep 14, 2026

Copy link
Copy Markdown
Member Author

@orontee Good catch, thanks — no, it is not expected, and it is not specific to this PR: main's CI shows the same two lines.

The cpack .deb really ships without ob-standalone-setup and its man page. The two links were created by install(CODE) against CMAKE_INSTALL_FULL_SBINDIR / CMAKE_INSTALL_FULL_MANDIR, which are fixed at configure time (/usr/local/...), while cpack installs under /usr: the parent directory does not exist in the staging tree, create_symlink fails, execute_process ignores it and cpack still exits 0.

The released packages are not affected: dpkg-buildpackage and rpmbuild configure with prefix /usr, so the paths match there (and both list the link explicitly).

Fix in #273: the links are now installed with install(FILES) and a relative destination, and the CI job fails when cpack reports a CMake Error.

Let's merge #272 then ?

@orontee

orontee commented Sep 14, 2026

Copy link
Copy Markdown

It's ok for me.

@guimard
guimard merged commit 72bb02a into main Sep 14, 2026
16 checks passed
@guimard
guimard deleted the doc/r-p1-release-prerequisite branch September 15, 2026 04:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants