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

Need to make error labeling more unique #2780

Open
erights opened this issue Apr 1, 2021 · 3 comments
Open

Need to make error labeling more unique #2780

erights opened this issue Apr 1, 2021 · 3 comments
Assignees
Labels
bug Something isn't working endo marshal package: marshal

Comments

@erights
Copy link
Member

erights commented Apr 1, 2021

Documenting a bug left over from the combination of
endojs/endo#649
and
#2769
By "currently" I mean, after both of these PRs, where an error might appear on the console as

RemoteError(error:device:vatAdmin#10001)#54

There are currently two numbers used to identify errors on console output. The 54 above is the errorTagNum coming from Endo's assert.js module. The 10001 is an errorIdNum coming from a marshal instance's errorIdNum variable. It would be helpful if they were more unique. However, fixing either or both turned into a larger discussion, so we're postponing it from those PRs and filing this bug instead. The tensions are between uniqueness, determinism, and leakage of information.

In the Endo assert.js module is a per-realm errorTagNum variable that only influences what appears on console output. Recall that console output is closely held. Being able to read that output is considered a similar level of privilege as running the code under a debugger. Thus, they don't leak to non-privileged objects within their vats. This suggests that assert could use a genuinely random seed unless provided an explicit one, and then generate a random enough sequence for use in debugging. The Agoric sparse-ints package looks plausible for this purpose, since there is no information leakage danger for this one. That is also why it is ok for this to be per-realm state. Currently, the errorTagNum "seed" is always 0 and the PRNG is +1. However, using any local source of entropy would be problematic for deterministic replay, even though the visible semantics of the program would be unchanged. For example, it would prevent replicated execution from producing identical snapshots.

In the agoric-sdk marshal.js module is a per-marshal-instance errorIdNum variable that a marshal instance uses to generate an errorId for serialization of errors it sends out, to log the association of this error with this marshal-assigned errorId, and to attach this errorId to the unserialized error at whatever site unserializes it. Currently, the errorIdNum "seed" is always 10000 and the PRNG is +1. This leaks way too much information. @warner points out that each vat has one marshal instance serializing messages that don't get unserialized until they arrive at their destination machine. Thus, these sequence numbers reveal to each destination machine some information about how many messages that source vat sent to other destination machines. To plug this leak, we'd need an unpredictable sequence, such as we'd get with a cryptographically secure PRNG (not the sparseInt package), seeded with good entropy. Of course, when the source is a public chain the information that would leak is already public, so the seed can be predictably deterministic. For a non-chain, using genuine entropy should be non-problematic.

@warner you have a great way of explaining the architecture needed to resolve these tensions in terms of how layers of the system interact with those above and below. Could you explain how to approach this issue in these terms?

@erights erights added the bug Something isn't working label Apr 1, 2021
@erights erights self-assigned this Apr 1, 2021
@erights
Copy link
Member Author

erights commented Apr 1, 2021

Attn @dtribble @kriskowal @warner

@erights
Copy link
Member Author

erights commented Apr 2, 2021

See @warner 's extended comment at #2769 (comment)

@Tartuffo
Copy link
Contributor

@erights This is in the "Up Next" pipeline, but does not have a MN-1 label. If it is needed for MN-1, please label, otherwise move from "Up Next" to "Product Backlog".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working endo marshal package: marshal
Projects
None yet
Development

No branches or pull requests

2 participants