Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

c18n: Compartmentalisation policies #2054

Closed
wants to merge 5 commits into from
Closed

c18n: Compartmentalisation policies #2054

wants to merge 5 commits into from

Conversation

dpgao
Copy link
Contributor

@dpgao dpgao commented Mar 17, 2024

Implements a compartmentalisation policy format. Fixes a bug that causes vfork to corrupt stack state. Improves trampoline generation code. Adds an option to output verbose utrace information.

Load a compartment policy file via the environment variable
LD_C18N_COMPARTMENT_POLICY.

Currently, the policy allows one to
(a) define logical compartments comprising multiple libraries,
(b) specify which symbols are trusted by a particular compartment, and
(c) specify which libraries are allowed to link against any particular
symbol.
Previously, after a process calls vfork, the child process then calls
execve. This call causes a domain transition that changes the top of the
stack as recorded by RTLD. This change is visible in the parent process,
which then wakes up and sees that the top of the stack has been altered,
which is something that should never happen.

Trust execve so that calling it does not cause a domain transition.
Let trampoline hooks consume the trampoline header and trusted stack
directly.

Remove the dependency on mempcpy.
Copy link
Collaborator

@bsdjhb bsdjhb left a comment

Choose a reason for hiding this comment

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

I know Brooks would like an alternate file format, but that can wait for later (and I agree some existing structured text like UCL or JSON would be better long term).

default:
return (0);
}
if (ut->verbose)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's a bit unusual to have this flag be set when the trace is logged rather than being something you can choose at inspection time (that is a flag passed in from kdump/truss) since you have the data in the utrace log always. You can always adjust that later though. (That is, I think ideally you would have verbose or not come from a flag passed to kdump or truss, or even if this is a temporary thing a hacky thing would be to check an env var here to determine verbose vs non-verbose rather than having it be a property of when you are running the program since you might decide later when examining a dump via kdump that you'd like to see the extra detail for some reason.)

@brooksdavis
Copy link
Member

I know Brooks would like an alternate file format, but that can wait for later (and I agree some existing structured text like UCL or JSON would be better long term).

I'm fine with a new format being something for the next release. We likely need to discuss with TA-1 teams before we do a V2.

@dpgao
Copy link
Contributor Author

dpgao commented Mar 20, 2024

Closing with 81ffb8b merged into dev. dc35f2f is omitted because it has unresolved issues and it is not essential to compartmentalisation policies.

@dpgao dpgao closed this Mar 20, 2024
@dpgao dpgao deleted the c18n-latest branch March 20, 2024 19:55
Copy link
Member

@jrtc27 jrtc27 left a comment

Choose a reason for hiding this comment

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

This is still a really unfriendly format with all its whitespace-sensitive syntax...

}

static bool
eat_token(struct cursor *cur, char delim, char *buf, size_t size)
Copy link
Member

Choose a reason for hiding this comment

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

eat takes a const char *token but eat_token doesn't? That's confusing naming. This isn't eat_token (what eat is), this is get_token / lex_token?

string_base_push(&com->libs, buf);
else
policy_error(&cur);

Copy link
Member

Choose a reason for hiding this comment

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

You still have strange blank lines here


.section .rodata
.globl c18n_default_policy
.type c18n_default_policy,#object
Copy link
Member

Choose a reason for hiding this comment

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

If it's MI, use % not #. # is often a comment character (aarch64 is unusual in using // instead in assembly, normally that only works if you pre-process it), and whilst @ is what most architectures default to, it doesn't work on arm.

@dpgao
Copy link
Contributor Author

dpgao commented Mar 20, 2024

@jrtc27 Thanks for the reviews. I'll address them in a future PR. I also plan to rewrite the parser to relax the syntax, but probably better after we decide on what policy language to use.

bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Aug 9, 2024
New features:
  CTSRD-CHERI#1941 uudecode filter: support file name and file mode in raw mode
  CTSRD-CHERI#1943 7-zip reader: translate Windows permissions into UNIX
        permissions
  CTSRD-CHERI#1962 zstd filter now supports the "long" write option
  CTSRD-CHERI#2012 add trailing letter b to bsdtar(1) substitute pattern
  CTSRD-CHERI#2031 PCRE2 support
  CTSRD-CHERI#2054 add support for long options "--group" and "--owner" to tar(1)

Security fixes:
  CTSRD-CHERI#2101 Fix possible vulnerability in tar error reporting introduced
        in f27c173

Important bugfixes:
  CTSRD-CHERI#1974 ISO9660: preserve the natural order of links
  CTSRD-CHERI#2105 rar5: fix infinite loop if during rar5 decompression the last
        block produced no data
  CTSRD-CHERI#2027 xz filter: fix incorrect eof at the end of an lzip member
  CTSRD-CHERI#2043 zip: fix end-of-data marker processing when decompressing zip
        archives

Obtained from:		libarchive
Libarchive commit:	4fcc02d906cca4b9e21a78a833f1142a2689ec52
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Aug 9, 2024
Libarchive 3.7.3

New features:
  CTSRD-CHERI#1941 uudecode filter: support file name and file mode in raw mode
  CTSRD-CHERI#1943 7-zip reader: translate Windows permissions into UNIX
        permissions
  CTSRD-CHERI#1962 zstd filter now supports the "long" write option
  CTSRD-CHERI#2012 add trailing letter b to bsdtar(1) substitute pattern
  CTSRD-CHERI#2031 PCRE2 support
  CTSRD-CHERI#2054 add support for long options "--group" and "--owner" to tar(1)

Security fixes:
  CTSRD-CHERI#2101 Fix possible vulnerability in tar error reporting introduced
        in f27c173

Important bugfixes:
  CTSRD-CHERI#1974 ISO9660: preserve the natural order of links
  CTSRD-CHERI#2105 rar5: fix infinite loop if during rar5 decompression the last
        block produced no data
  CTSRD-CHERI#2027 xz filter: fix incorrect eof at the end of an lzip member
  CTSRD-CHERI#2043 zip: fix end-of-data marker processing when decompressing zip
        archives

PR:		278315 (exp-run)
MFC after:	1 week
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Aug 9, 2024
Libarchive 3.7.3

New features:
  CTSRD-CHERI#1941 uudecode filter: support file name and file mode in raw mode
  CTSRD-CHERI#1943 7-zip reader: translate Windows permissions into UNIX
        permissions
  CTSRD-CHERI#1962 zstd filter now supports the "long" write option
  CTSRD-CHERI#2012 add trailing letter b to bsdtar(1) substitute pattern
  CTSRD-CHERI#2031 PCRE2 support
  CTSRD-CHERI#2054 add support for long options "--group" and "--owner" to tar(1)

Security fixes:
  CTSRD-CHERI#2101 Fix possible vulnerability in tar error reporting introduced
        in f27c173

Important bugfixes:
  CTSRD-CHERI#1974 ISO9660: preserve the natural order of links
  CTSRD-CHERI#2105 rar5: fix infinite loop if during rar5 decompression the last
        block produced no data
  CTSRD-CHERI#2027 xz filter: fix incorrect eof at the end of an lzip member
  CTSRD-CHERI#2043 zip: fix end-of-data marker processing when decompressing zip
        archives

PR:		278315 (exp-run)
MFC after:	1 week
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.

4 participants