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

Example comparingcheri_cap_build & cheri_address_set #92

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

probablytom
Copy link
Contributor

cheri_cap_build and cheri_address_set are similar, but not quite the same. We were previously unsure of what the difference between the two was; this example uses both and explains the differences between the two.

It's pretty explicit about what's going on and prints detailled messages describing the capabilities it constructs & prints. I figured it'd be useful to have some detail in there so we didn't have to re-parse the code every time we wanted to remember what the difference between the functions is.

Put together mostly by @0152la; I just cut down the program we were using as a scratchpad when figuring this out and tidied it into an example.

cap_build.c Outdated Show resolved Hide resolved
cap_build.c Show resolved Hide resolved
cap_build.c Outdated Show resolved Hide resolved
@ltratt
Copy link
Collaborator

ltratt commented Nov 14, 2023

Since I'm the dummy here, I'll ask the obvious question: does cheri_cap_build allow you to create an arbitrary capability that overrides all other checks? I guess "no", but I'm not quite sure what the "but" is. My initial guess is that the capability that's created is valid but has limited permissions -- but maybe I'm wrong!

@probablytom
Copy link
Contributor Author

Took a while to demonstrate this conclusively, but the answer is no. I've got a snippet now which removes permissions from the DDC and shows that you can't use cheri_cap_build to produce a valid capability which restores any permissions the DDC doesn't already have — I think that answers your question, @ltratt. You can construct a valid capability using cheri_cap_build that can do anything the DDC can do, but the capability constructed by cheri_cap_build won't be valid if it's more permissive than the DDC.

I suspect what's happening is that the uintcap_t argument to cheri_cap_build is returned as a valid capability so long as it is no more permissive than the void *__capability argument it's given rather than the DDC, but I'm yet to confirm.

I'd been aiming to get this tidied into the example with responses to @0152la 's notes, but haven't quite finished up and needing to dash — so I'll push the code demonstrating this tomorrow.

(Also: I have a suspicion it can be used to reconstruct a capability that's been serialised to a file, which shouldn't be possible, but I'm not going to investigate that right now — maybe a diversion for next week.)

@ltratt
Copy link
Collaborator

ltratt commented Nov 16, 2023

Thanks!

I have a suspicion it can be used to reconstruct a capability that's been serialised to a file

This shouldn't be possible -- but there was once a bug in CheriBSD that allowed it because of file caching! It was fixed, though.

@djlowther
Copy link
Member

cheri_cap_build translates to the BUILD Morello instruction, which like Tom suspects turns a bit-pattern into a valid, unsealed capability, provided that that capability can be derived from the capability argument. I believe it's designed for situations like virtual memory paging -- the kernel can use it to quickly reconstitute capabilities in swapped pages rather than having to use a sequence of SCBNDS, SCVALUE, etc.

@ltratt
Copy link
Collaborator

ltratt commented Nov 20, 2023

Thanks both -- I get it now!

@probablytom
Copy link
Contributor Author

Thanks @djlowther — that's a very helpful note! We haven't found much documentation around cheri_cap_build ourselves.

I haven't played with this yet and can't budget time to investigating it right now, but perhaps you know already: does that mean that a capability written to disk by one process could be loaded by a hybrid process with a very permissive DDC, and so re-created as a valid capability by that second process? Shouldn't that be forbidden, as @ltratt mentioned? Or am I misunderstanding something?

@djlowther
Copy link
Member

djlowther commented Nov 20, 2023

The answer to your question is yes, in theory it could. However, BUILD is just a short-cut; it doesn't allow anything that a sequence of other instructions couldn't accomplish. To be precise, in ARM's pseudocode the tag is cleared if:

!CapIsTagSet(key) || CapIsSealed(key) || !CapIsSubSetOf(data,key) || CapIsBaseAboveLimit(data)

So the key has to be a valid, unsealed superset of the capability to be derived. Therefore, one could replace:

build c2, c0, c1

with (provided R3 is available as scratch)

gcbase x2, c0
scvalue c2, c1, x2
gclen x3, c0
scbnds c2, c2, x3
scvalue c2, x0          // x0 functions as an implicit GCVALUE of c0
gcperm x3, c0
mvn x3, x3
clrperm c2, c2, x3

(N.B. - this may have a different result on failure than BUILD but should be identical on success)

Moreover, being able to recreate a capability from data from an IO stream is not a problem as long as it's done explicitly and doesn't grant extra privileges to the receiving process (which the superset restriction guarantees). In fact, it's necessary to allow the kernel to swap pages (which may contain valid capabilities) to disk.

@probablytom
Copy link
Contributor Author

Fascinating! Thank you for the detailled response!

@ltratt
Copy link
Collaborator

ltratt commented Nov 21, 2023

@0152la I think this is OK to squash?

@0152la
Copy link
Contributor

0152la commented Nov 21, 2023

Needs rebasing against master first.

@ltratt
Copy link
Collaborator

ltratt commented Nov 21, 2023

@probablytom Please squash.

[Note: this doesn't mean you have to squash down to 1 commit, though you can if that makes sense. "Squashing" also implies "rebase + force push".]

probablytom added a commit to probablytom/cheri-examples that referenced this pull request Nov 21, 2023
Adds an example comparing `cheri_cap_build` and `cheri_address_set`.

`cheri_cap_build` and `cheri_address_set` are very similar, but not
quite the same. We were previously unsure of what the difference between
the two was; this example uses both and explains the differences between
the two.

Briefly put, `cheri_address_set` takes two capabilities as arguments and
returns a new capability with the metadata of the first arg and the
address of the second. `cheri_cap_build` takes a capability and a
`uintcap_t` — some bits to interpret as a capability, but not actually a
capability — and returns a new capability constructed from the bits of
the second (`uintcap_t`) argument, which is valid if and only if it is
no more permissive than the first (cap) argument.

There's a little more detail, but that's a summary. For more, there are
a couple of useful comments on github:

- capablevms#92 (comment)
- capablevms#39 (comment)
@probablytom probablytom force-pushed the example_cheri_cap_build branch 2 times, most recently from 3741aac to d7ce778 Compare November 21, 2023 12:36
@probablytom
Copy link
Contributor Author

Squashed!

@ltratt ltratt enabled auto-merge November 21, 2023 12:37
@0152la
Copy link
Contributor

0152la commented Nov 21, 2023

On my end, still says it's out of date with master.

@probablytom
Copy link
Contributor Author

Oh, and mine too. Apologies, thought I'd fixed — one sec

probablytom added a commit to probablytom/cheri-examples that referenced this pull request Nov 21, 2023
Adds an example comparing `cheri_cap_build` and `cheri_address_set`.

`cheri_cap_build` and `cheri_address_set` are very similar, but not
quite the same. We were previously unsure of what the difference between
the two was; this example uses both and explains the differences between
the two.

Briefly put, `cheri_address_set` takes two capabilities as arguments and
returns a new capability with the metadata of the first arg and the
address of the second. `cheri_cap_build` takes a capability and a
`uintcap_t` — some bits to interpret as a capability, but not actually a
capability — and returns a new capability constructed from the bits of
the second (`uintcap_t`) argument, which is valid if and only if it is
no more permissive than the first (cap) argument.

There's a little more detail, but that's a summary. For more, there are
a couple of useful comments on github:

- capablevms#92 (comment)
- capablevms#39 (comment)
@probablytom probablytom force-pushed the example_cheri_cap_build branch from d7ce778 to ae1be5c Compare November 21, 2023 12:44
@ltratt ltratt added this pull request to the merge queue Nov 21, 2023
@0152la 0152la removed this pull request from the merge queue due to a manual request Nov 21, 2023
@0152la
Copy link
Contributor

0152la commented Nov 21, 2023

I've removed it from the merge queue, since this now seems to have our whole repo history in it.

@probablytom probablytom force-pushed the example_cheri_cap_build branch from ae1be5c to d7ce778 Compare November 21, 2023 12:46
@probablytom
Copy link
Contributor Author

Yes, fortunately it looks like it was also still marked as out of sync with master so didn't auto-merge. Should be fixed now — looks correct on my end, at least — but will keep my mitts off the merge button…

@probablytom
Copy link
Contributor Author

…should be good to go… 👍

@ltratt ltratt added this pull request to the merge queue Nov 21, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 21, 2023
probablytom added a commit to probablytom/cheri-examples that referenced this pull request Nov 21, 2023
Adds an example comparing `cheri_cap_build` and `cheri_address_set`.

`cheri_cap_build` and `cheri_address_set` are very similar, but not
quite the same. We were previously unsure of what the difference between
the two was; this example uses both and explains the differences between
the two.

Briefly put, `cheri_address_set` takes two capabilities as arguments and
returns a new capability with the metadata of the first arg and the
address of the second. `cheri_cap_build` takes a capability and a
`uintcap_t` — some bits to interpret as a capability, but not actually a
capability — and returns a new capability constructed from the bits of
the second (`uintcap_t`) argument, which is valid if and only if it is
no more permissive than the first (cap) argument.

There's a little more detail, but that's a summary. For more, there are
a couple of useful comments on github:

- capablevms#92 (comment)
- capablevms#39 (comment)
@probablytom probablytom force-pushed the example_cheri_cap_build branch from 29fbcfc to c18cdf7 Compare November 21, 2023 16:08
@probablytom
Copy link
Contributor Author

Re-formatted.

@ltratt ltratt added this pull request to the merge queue Nov 21, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 21, 2023
@ltratt
Copy link
Collaborator

ltratt commented Nov 23, 2023

@probablytom Is this ready for rereview?

@0152la
Copy link
Contributor

0152la commented Nov 23, 2023

I'm confused about commit ba68b10. Looking at the code [1], this new example should only run on morello-hybrid. If you see it running on risc-v, that might be a CI issue, and I need to investigate.

[1] https://github.com/capablevms/cheri-examples/pull/92/files#diff-bfa67bbd428f0fd40485bf6e666132a2adb30f6524631aff49d2bcd30f71c5ebL75

@ltratt
Copy link
Collaborator

ltratt commented Nov 23, 2023

I'm confused about commit ba68b10. Looking at the code [1], this new example should only run on morello-hybrid. If you see it running on risc-v, that might be a CI issue, and I need to investigate.

I agree that this might be worth investigating, but I'd prefer to get this PR in now, and investigate separately: we can always reenable on RISC-V later if it turns out there's a CI issue.

@probablytom Just checking that this PR is finished from your perspective?

@probablytom
Copy link
Contributor Author

probablytom commented Nov 23, 2023

Yes, I was confused as to why it was running too. I think it's because the CI runs make -f Makefile.riscv64-purecap all, and the make rule appears to pick up every example in the root of the repo and run it as if it were riscv-compatible. Other examples have similar macros to avoid running on the incorrect architectures [1], I imagine for exactly this reason — so I think that commit [2] actually follows the precedent set by other examples so far. Afraid I haven't been able to investigate further, but I'm confident the change should fix the issue.

Given other examples do this too, if the build passes CI now I don't think it's worth making changes in the short term to get this to run without the macros…but if you're keen, my suspicions sit with that makefile.

So to answer the Q @ltratt, yes — happy with the state of this now — but also happy to make changes if there's anything else to tidy up! I'll squash when instructed.

[1] https://github.com/capablevms/cheri-examples/blob/master/general_bounds.c
[2] ba68b10

@0152la
Copy link
Contributor

0152la commented Nov 23, 2023

You're right. It seems it assumes root examples mean purecap [1]. Looking at the logic then [2], you should place this example in ./hybrid/, and remove the additional condition you added in run_tests.sh.

I think that's the best resolution, but still the fact that we have some tests compiling in .buildbot.sh, then others running and compiling in ./tests/run_tests.sh is not ideal, and should probably fix that. However, that is beyond the scope of this PR. If this works and @ltratt is fine merging as is, without the change at the top, I'm fine with that.

[1] https://github.com/capablevms/cheri-examples/blob/master/.buildbot.sh#L18
[2] https://github.com/capablevms/cheri-examples/blob/master/.buildbot.sh#L29

@ltratt
Copy link
Collaborator

ltratt commented Nov 23, 2023

Yeah, let's get this in, and then refine/move things around in another PR.

@probablytom Please squash.

probablytom added a commit to probablytom/cheri-examples that referenced this pull request Nov 23, 2023
Adds an example comparing `cheri_cap_build` and `cheri_address_set`.

`cheri_cap_build` and `cheri_address_set` are very similar, but not
quite the same. We were previously unsure of what the difference between
the two was; this example uses both and explains the differences between
the two.

Briefly put, `cheri_address_set` takes two capabilities as arguments and
returns a new capability with the metadata of the first arg and the
address of the second. `cheri_cap_build` takes a capability and a
`uintcap_t` — some bits to interpret as a capability, but not actually a
capability — and returns a new capability constructed from the bits of
the second (`uintcap_t`) argument, which is valid if and only if it is
no more permissive than the first (cap) argument.

There's a little more detail, but that's a summary. For more, there are
a couple of useful comments on github:

- capablevms#92 (comment)
- capablevms#39 (comment)
@probablytom probablytom force-pushed the example_cheri_cap_build branch from ba68b10 to 63a8bbe Compare November 23, 2023 14:31
@probablytom
Copy link
Contributor Author

Squashed!

@ltratt ltratt added this pull request to the merge queue Nov 23, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 23, 2023
@ltratt ltratt added this pull request to the merge queue Nov 23, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 23, 2023
@ltratt
Copy link
Collaborator

ltratt commented Nov 23, 2023

Please squash.

Adds an example comparing `cheri_cap_build` and `cheri_address_set`.

`cheri_cap_build` and `cheri_address_set` are very similar, but not
quite the same. We were previously unsure of what the difference between
the two was; this example uses both and explains the differences between
the two.

Briefly put, `cheri_address_set` takes two capabilities as arguments and
returns a new capability with the metadata of the first arg and the
address of the second. `cheri_cap_build` takes a capability and a
`uintcap_t` — some bits to interpret as a capability, but not actually a
capability — and returns a new capability constructed from the bits of
the second (`uintcap_t`) argument, which is valid if and only if it is
no more permissive than the first (cap) argument.

There's a little more detail, but that's a summary. For more, there are
a couple of useful comments on github:

- capablevms#92 (comment)
- capablevms#39 (comment)
@probablytom probablytom force-pushed the example_cheri_cap_build branch from a3d1d11 to e156027 Compare November 23, 2023 18:52
@probablytom
Copy link
Contributor Author

Squashed.

@ltratt ltratt added this pull request to the merge queue Nov 23, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 23, 2023
@ltratt ltratt added this pull request to the merge queue Nov 29, 2023
Merged via the queue into capablevms:master with commit 8772cc0 Nov 29, 2023
2 checks passed
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