Add the respective code comment

In D49039#1118150, @asomers wrote:

LGTM. Except that the commit message contains a typo: "uprivileged_user" => "unprivileged_user"

I guess it could be helpful for a couple of test cases from tests/sys/netinet/fibs_multibind_test.c and/or tests/sys/netinet/socket_afinet.c.

@ngie , it would be appreciated if you could find time to consider this. It seems that your opinion is necessary to move forward.

Update kyuafile.5 after landing of D48190

Improve the comment regarding the retry mechansim

tests: Use recently added support of is.exclusive coming from ATF side

This is a follow-up to https://reviews.freebsd.org/D48190, where it was discovered that this support is missing. It's obvious that our test suite does not use it, and this patch is for consistency and not to say "not mapped" in the kyuafile.5. Also, I found that Kyua 0.11 release notes state that it should work, so actually this is a fix.

In D48190#1100132, @ngie wrote:

I think submitting this upstream (freebsd/kyua) and filing an issue capturing the feedback/concerns would probably be ok.

In D48087#1096440, @kp wrote:

One trivial remark about 'pip? # e.g. to have scapy installed'. We have a scapy package and that's how I've always installed it. Right now it's py311-scapy-2.6.1.

In D48190#1099037, @ngie wrote:

It honestly seems like this documentation should live in another place, e.g., a "test driver" [1] (kyua-atf-driver(4)?) manpage. Putting all of this information in a single manpage seems like it would really clutter up what's being explained here, and the more "test drivers" get bolted on to kyua, the more complex the documentation will become.

1. I'm not sold on the name "test driver", but it's the best I can offer for the concept right now.

This is a follow-up to https://reviews.freebsd.org/D47671, where we discussed documenting of the mapping.

In D47671#1098794, @ngie wrote:

If you could commit it directly to the freebsd/src:main, that would be helpful :) (I think it's better to get it in sooner so it gets some bake time on FreeBSD).

Now the wording is away from the internals and closer to the end users:

> ./kyua help | grep prepare
  prepare                 Prepare env and resolve requirements before testing.

My current favorite is the prepare term proposed by Kristof. So, I've changed it in the latest version of the patch for the sake of further testing and discussion. Everything is open for renaming. The internals still use requirement resolver terminology, I could not quickly find the replacement and probably it's not needed due to "prepare" is kind of higher level, anyway this is the encapsulated part with the only exposure via CLI help output.

Rename "kyua rr" to "kyua prepare"; Drop "kyua test --rr"

Make keyvalue_contention test case more accurate

Tiny code improvements, no functional change

Add keyvalue_contention test case

Fix jm_h_cut_occurrences() logic

libjail: Correctly differentiate no<boolparam> from keyvalue-based ones

Each buffer can be handled as a set of key=value\n strings

By the way, now I'm leaning towards resolve subcommand as you mentioned:

> kyua resolve kmods
> kyua test { --resolve | -r } kmods testprog

In D48087#1096189, @markj wrote:

As usual, I will quibble a bit about naming - the kyua subcommands are mostly english words, so "rr" is a bit odd. Why not "resolve" or even "resolve-requirements" (or "resolve-reqs")?

This is the very first working version of the Project B: https://lists.freebsd.org/archives/freebsd-testing/2024-November/000395.html.

In D47671#1095969, @ngie wrote:

Thanks! Please update the differential revision with the tests. Memory serves me correctly, CI on main might start failing if the tests aren't introduced.

In D47671#1095814, @ngie wrote:

Please open a PR against freebsd/atf .
Also: this deserves a test (upstream).

In D47668#1095800, @jamie wrote:

In D47668#1095796, @igoro wrote:

• The allowed chars for each buffer are very limited by default, it covers Base64, k=v\n format, and some extra bytes. It can be changed via security.jail.meta_allowedchars sysctl. For convenience (as it seems to me for now), setting it to an empty string allows everything.

Why is this a kernel issue? Aside from NUL, because it preserves the C-string nature, allowed characters would seem only to be a concern on the user side.

In D47671#1087433, @markj wrote:

The change itself looks fine to me, but don't we need to document this somewhere?

The latest update aggregates the recent discussions:

• Rename metaext/metaint to meta/env. meta is expected to be used for "tagging" and hidden from the jail. env is intended for "configuring" and readable by the jail through security.jail.env sysctl.
• The allowed chars for each buffer are very limited by default, it covers Base64, k=v\n format, and some extra bytes. It can be changed via security.jail.meta_allowedchars sysctl. For convenience (as it seems to me for now), setting it to an empty string allows everything.
• The tests and man page are upgraded respectively.

Rename to meta/env, add meta_allowedchars.

In D46653#1091544, @ngie wrote:

Regarding the change proposed on the GitHub PR. I think it's better to merge the existing PR to keep FreeBSD src/contrib/kyua and the github/freebsd/kyua in sync with absolutely the same commit. And I would open a new PR for the change requested with a reference to the previous GitHub discussion/PR. What do you think is the best here from the organizational perspective?

I don't think it would be a good idea to merge the PR as-is. I'm trying to avoid violating POLA as much as humanly possible for 0.14 and taking a look at the output definitely violates POLA. There are enough Linux/MacOS things to deal with -- I'd rather not get more questions with a UX change like has been made on main...

In D46653#1091513, @ngie wrote:

For the record, stuff like this should really be committed to freebsd/kyua first, then backported. As of right now this change is not in what's intended to go in to 0.14 and in order to do the upgrade I would need to reapply this patch. I provided a ship-it earlier, but I was swayed by the comment in the PR about this change being confusing for naive results parsers: https://github.com/freebsd/kyua/pull/230 .

In D47668#1090028, @jamie wrote:

In D47668#1089529, @igoro wrote:

Yeah, it seems that on the Jail Call of 26-Nov we came to a conclusion that for now we would keep it very simple like two buffers per jail managed from the user-land side, while keeping a wide spectrum of opportunities to extend it in the future having a more specific production need in mind. Thus, we can postpone thinking about extra complexity on the kernel side.

If I had managed to make that call, I have to say that at least the conclusion wouldn't have been unanimous. The single blob is indeed simpler to implement, but not simpler to use. Still, you mention extension, and that's a possibility. I'm thinking something like:

meta="foo=bar\0baz=bletch"
meta.foo="bar"
meta.baz="bletch"

In D47668#1089218, @crest_freebsd_rlwinm.de wrote:

In D47668#1088649, @igoro wrote:

It seems there is an agreement to split it onto two buffers per jail: both are readable by parent jail, while only one is readable by a jail itself. The updated patch reflects this concept. For now metaext and metaint naming is used as external/internal concept. The naming is open for discussion.

There is a third useful combination: (privileged) access host access only e.g. as a place to store API token or similar things without ever writing them to a file.

The problem having a fixed set of sysctls with with their designated access permissions is that all accesses have to go through a single writer and use the same data format because a single sysctl is just one string. This means anyone else is required closely coupled to that writer unless there is some standardised patch and query interface (e.g. JSON patch and JSON query) and I can't imagine anyone wanting to deal with that complexity. A flat key=value store avoids this by allowing multiple writers to lay exclusive claim to their part of the keyspace, but it doesn't absolve the need for access control.

It seems there is an agreement to split it onto two buffers per jail: both are readable by parent jail, while only one is readable by a jail itself. The updated patch reflects this concept. For now metaext and metaint naming is used as external/internal concept. The naming is open for discussion.

v2: Split onto two buffers per jail: external & internal

Great, I can continue from any point where you decide to stop and land your commits.

Thank you all for the discussion. I've refreshed the context for myself and I would like to share it:

• We have got a new -J flag for sysctl(8). It filters the variables by CTLFLAG_PRISON.
• It reminded me of a couple of existing sysctlS which are not listed with the -J flag.
• That's because they are read-only (CTLFLAG_RD) without CTLFLAG_PRISON flag set. The reasoning follows.
• The CTLFLAG_PRISON has been used as a way to allow jailed roots modify a sysctl. It works very simple way. If a sysctl sys call request means modification, i.e. a new buffer (the req->newptr) is provided, then the sys call will check if the user has PRIV_SYSCTL_WRITE privilege. It does not work for a jailed root, that's why the sys call will check for another privilege (PRIV_SYSCTL_WRITEJAIL) if the sysctl in question has CTLFLAG_PRISON flag set. That's the only use case for this flag for now.
• And it's fine that read-only per-jail variables do not have this flag, it's not needed for them.
• As long as I found this little inconsistency I proposed to update the meaning of this flag (sysctl.9) and add it to some read-only variables which are, kind of, expected to be found in the sysctl -aJ output.

Pivot the patch.

I would like to share my thoughts regarding this.

Currently, there is no accessible way to attach metadata to jails. This is commonly used elsewhere, for example in Kubernetes, to allow non-unique properties to enrich load balancers, schedulers, and volume provisioners to make more informed decisions.

Looks good to me too.

The email (https://lists.freebsd.org/archives/freebsd-testing/2024-November/000399.html) was understood to mean that you wanted to offer help with the development and were asking about a review. This was interpreted as a request to take over the project entirely, which is why I shared the full plan I intended to follow—to avoid prolonging the process, reduce the number of iterations, and clearly outline all expectations upfront. Please, let me know if you did not intend to work on this project and the email was misunderstood.

I see the currently proposed change as the following outcome:

• 13.x users should stay as is with old test suite and old Kyua from base or port/pkg. And the pkg is expected to be still cooked till 13 branch EOL.
• 14.x users are expected to find kyua pkg missing after upgrade to the latest port branch. They are expected to do such upgrade with OS upgrade as well, i.e. up to 14.2, which has the test suite aligned with Kyua in its base. 14.0 and 14.1 users of older Q branches are expected to use the same older suite and pkg. Even if they upgrade only ports having old 14.0 or 14.1 then they still have the test suite runnable with Kyua in their base.
• Just in case, the port is still available for anyone till 13 EOL (30 April 2026).
• A port/pkg update is expected to show the deprecation notice. It should cover attended cases.

IGNORE it for 14.0+

Thank you for working on this. If you plan to go ahead then please consider the following points.

I had to double check the things. I've delved into the details and discovered that the existing code does not allow to use CTLFLAG_PRISON as a flag which means that a variable varies per jail. I've stumbled upon the *allow* variables like these ones:

SYSCTL_PROC(_security_jail, OID_AUTO, set_hostname_allowed,
     CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_MPSAFE,
     NULL, PR_ALLOW_SET_HOSTNAME, sysctl_jail_default_allow, "I",
     "Processes in jail can set their hostnames (deprecated)");

Bump PORTREVISION with the expectation that users will see the deprecation notice upon upgrade.

Update the deprecation notice as suggested.

Please, check the updated patch as the follow-up of your email.

Covered by https://reviews.freebsd.org/D47334.