Soundtrack of this PR: https://www.youtube.com/watch?v=-Zbx8QFhDK4&t=5s

Motivation

There need to be three fog url objects, one for the report server, which also appears in public address. And one for the view server, and one for the ledger server. There are also some query parameters in the public address one that are needed, and the way in which these affect the other two needs to be implemented as specified.

In this PR

• Add FogUri object which conceptually is what is in the public address for a fog address.
• Add query parameter support in this object and tests that it works
• Add conversions which derive FogViewUri and FogLedgerUri objects from the FogUri object, based on the shard id value.
• Test that conversions preserve the query parameters that they are supposed to and not the ones they aren't.

This resolves Fog-20, Fog-95. This implements the internal RFC around fog query parameters.

Future Work

The sig will eventually be used when validating fog report cert chains. The sig is not yet computed and that step is not yet integrated with account key and public address. That is Fog-106

This makes transaction builder and validation aware of BlockVersion. It also plumbs this config into consensus.

This commit adds support for the change in fog sample paykit and mobilecoind

See also: https://github.com/mobilecoinfoundation/mcips/pull/26

Checklist:

• [x] Rebase on master
• [x] Do the responder id parts
• [x] I made the block version be a parameter to things like form_block, but it might be better to do it differently, e.g. block version is announced on enclave startup, and can change after form_block but not in between calls to form block?
• [x] There is a place where I am taking a max of latest_block.version and configured block version, but perhaps it should only ever use the configured block version.
• [x] In many tests, I decided it was worth testing all acceptable values of block version, and in other places, I only tested BlockVersion(1) or BlockVersion::MAX because that seemed like the highest value testing. Review these decisions.

This PR implements the transaction math part of confidential token ids. MCIP (still draft) is here: https://github.com/mobilecoinfoundation/mcips/pull/25

Still TODO:

• [x] Add the proof of knowledge of opening
• [x] Rebase on master
• [x] Use the mob id constant and token id types in master
• [ ] Rename Amount -> MaskedAmount (that may come in a separate PR afterwards)
• [x] More tests

More TODO

• [x] Remove proof of knowledge of opening
• [x] Undo breaking changes to protos
• [x] Use the block version feature test in transaction builder and validation
• [x] Make sample paykit support multiple asset balances
• [x] Make mobilecoind track token ids
• [x] Make slam and fog distro respect token ids
• [x] rebase again
• [x] Make it possible to bootstrap with token ids, and test in CD

In digestible crate, part of the design is to allow that new optional and repeated fields can be added to structs corresponding to protobufs, without changing the hashes of old objects. By making the hash algorithm similar to protobuf in this regard, we make it easier to evolve the blockchain without breaking backwards compatibility.

Unfortunately, there are some gaps still in what kinds of schema evolution digestible allows vs. what protobuf allows. Protobuf allows that new bytes and string fields can be added to a struct without a breaking change, but digestible doesn't, because it doesn't currently omit empty bytes or string fields when omitting them would be allowed. We didn't think about this when implementing the allow_omit stuff, because we were focused on containers -- but bytes and strings are primitives in protobuf.

Currently, NO primitives can be added unless they are Optional in rust. Wrapping things in Optional<...> also works for Digestible, but it doesn't always work well for things like prost.

This commit makes it so that strings and byte vectors have the "omit if empty" behavior and can be added easily to our prosty structures.

I believe that there are currently no string objects in our blockchain. I believe that there are no byte objects that are being hashed by digestible that are not either fixed size, or wrapped in optional.

This commit does not adjust the behavior of integers like u64 and u32, which could also arguably be omitted if zero. However we have a fine workaround for that in place which is the digestible(omit_when = 0) which Eran made. Unfortunately it is fairly difficult to make that work when the value to be omitted is not a literal.

If we decide that it is safe to merge this, this will simplify the changes needed to support masked token ids.

There are other approaches we could take like

• adding more attributes to digestible, or,
• representing this type using Option<GenericArray<u8, U4>>.

But the latter requires a bunch of repr-bytes stuff with prost and serde macros which in the long run we would like to reduce.

Dan Burkert was non-committal about supporting custom prost::Message implementations: https://github.com/tokio-rs/prost/pull/421#issuecomment-758867131 and suggested that this is an "at your own risk" undertaking not covered by semver guarantees. That makes the stuff where prost::Message is derived using a repr-bytes implementation somewhat uncomfortable. Hopefully in the future prost will just allow us to use things like Option<[u8; 99]> using const generics. Or, we may be able to just move everything over to Vec<u8> and then fixup the error handling.

Additionally, we considered it surprising that Vec<u8> behaves differently from Vec<T> for digestible crate, around whether it is omitted when empty. The documentation mentions the behavior of Vec<T> but not that of Vec. By merging this we would eliminate that footgun.

TODOs:

• [ ] grep this repo for all derive(Digestible) and look for String/Vec<u8> fields
• [ ] Do the same in full-service

Tested: cargo build -v, cargo build --release -v on my dev machine running Ubuntu.

I verified all the rustc commands include -D warnings -C target-cpu=skylake

Fixes #580

Motivation

This PR contains the core functionality of the Fog Overseer service. The service monitors the active Fog Ingest cluster and makes sure that there is at least one active node. If there is no active node, then it performs the automatic failover logic. This logic is described in detail in the worker.rs documentation.

Note that this PR contains the core logic: the monitoring logic is in worker.rs and the rest of the files basically just expose that functionality to clients via a Rocket HTTPS server. The tests test 5 different scenarios in an end to end fashion. Note that there are still some key features that need to be implemented before we can deploy this service. Namely, we'll need to implement the following items:

1. Sentry alerting: in some cases, we need to send alerts to humans when Overseer encounters certain (unlikely) Fog Ingest cluster states.
2. Prometheus metrics: as noted in the design doc, we want to capture Fog ingest cluster state metrics like number of ingress keys, number of responsive Fog ingest nodes, etc.
3. Get info api: in addition to storing Fog ingest cluster state in Prometheus, we want to expose a get info api that allows clients to get the current Fog Ingest cluster info in real time.
4. Config refresh: currently, we pass the list of Fog Ingest node uris to overseer through a CLI config. To allow for overseer to look at a new set of node urls during blue / green deployment without having to restart it, we'll move this list of uris to an environment variable that can be dynamically refreshed. This way, we can update the list of uris without having to restart overseer.

I figured that it would be best if I implemented these 4 features in future PRs, since this PR is already quite large. However, if you find that any of these features should be in this PR, I'm open to discussing.

In this PR

• Implements the core functionality of the Fog Overseer Service
• Creates 5 end to end tests that cover the core scenarios we expect Fog Overseer to help with.

Future Work

• Implement the 4 features listed above.

TOB-MCCT-5 is about a "token id oracle" described by TOB during review of confidential tokens design. The idea is that the consensus enclave fee map can be changed by the untrusted side, by restarting the node. If the attacker controls a node and is able to get the user to submit their Tx once with one fee map, and once with another, then the priority value leaks information about what the token id of the Tx is.

In principle they could do this without the user knowing -- they would restart the node and it would not be able to attest to the peers after changing the fee map, but it could still accept Tx proposals and see the WellFormedTxContext which conatins the priority number. Then they turn the node back to normal. The user's app retries when their transaction doesn't land, and the same Tx comes back to their node, now with the regular fee map.

This requires having root on a consensus node, but the threat model is that attackers with root on the consensus node should not be able to see the user's token id.

We view this as a time-of-check-vs-time-of-use (TOCTOU) bug, and we try to fix it a way that those bugs are sometimes fixed. The user, who checked the fee map, also makes an assertion about what the fee-map is when they try to use the system (by submitting a Tx). Before acting on the data (which the attacker has potentially tampered with), the enclave checks if the user's assertion is correct, and rejects the Tx without creating a WellFormedTxContext if the assertion is not correct. (This is sort of like dealing with races when manipulating atomic variables by using compare-and-swap.)

It should be straightforward to add this check, but unfortunately, the status quo client_propose_tx has a design flaw. It does not have a Request protobuf as the RPC argument, the argument it takes is directly an (encrypted) Tx. So, we cannot add any additional data to that protobuf in a non-breaking way without adding things to Tx. And that is untenable in this case because it adds complexity in sensitive areas of the code. We really just want to add this check next to the Tx.

To fix this, we introduce a ClientProposeTxRequest object, which contains a Tx, and the client's minimum fee map digest. We introduce client_propose_tx_v2 which encrypts this Request object instead of a naked Tx. This will come in handy if we want to add anything else like this in the future.

Clients will not be able to use client_propose_tx_v2 until after the V3 service is live, because existing consensus nodes don't have this endpoint. However, in the next release of clients after the v3 release is live, they could all start using client_propose_tx_v2 and we could deprecate / remove client_propose_tx.

I made the fog-sample-paykit exercise the client_propose_tx_v2 route, since the test client uses that and so we can see that it is working in CD without making mobilecoind or anything else use it yet.

I am not sure if this change requires an MCIP. It is a protocol / api change, but not a big one, and users are unlikely to notice. If it does require an MCIP we can write one pretty easily. Let me know what you think.

Also, lmk if you think there's a better approach than the route I took here.

The idea here is to add a github action that clones some remote repository, such as android-bindings and then go to the mobilecoin submodule there and checkout the branch going through CI, and try to build it. If the build fails, then it will try to build using the base branch of the PR (e.g. master in most cases). Then, if the master build succeeded and the branch build failed, it will comment on the PR indicating that this happened. The reason it checks if master builds, is because if that is already broken then it is out of scope for the current PR to deal with that - bit maybe we can remove this behavior. If the build is fixed after it was initially broken, the action should also comment on the PR to indicate the issue was resolved. Right now there's a bug that I still need to fix where if master is broken and this PR fixes the issue, I am not sure a comment gets created. I can work on that if people think this is worth pursuing further. I figured it's not high priority since usually what will happen is a PR on mobilecoin will break the remote repo (e.g. an API change that breaks android-sdk). This will be indicated in the PR, but the PR will move forward and get merged. Then someone from the SDK team can be notified that this PR caused a breakage, and they can go and uprev mobilecoin in android-sdk and fix the breakage.

We currently have a lot of code that is like manually grabbing prost::encoding::*; and using it to create custom serializations with prost.

This is a hazard, because that is not part of the public-facing interface of prost and it is not documented. It also changes periodically as upstream refactors the library. The only reason these symbols are publicly visible at all is that there is no way that prost-derive can function if the proc-macro generated code cannot access these symbols. When we grab prost::encodings::*; it is creating a maintanence hazard that will make it very hard ever to uprev prost again.

In the past, every cryptographic primitive that went into prost or serde required custom bindings code, which was very long. ReprBytes32 was a quick stopgap to prevent the code duplication and take us down a route that would be more maintainable.

It turns out that we have some types that are not exactly 32 bytes, and then we have started grabbing the prost::encodings::* again to support those.

Separately from this, we have had a problem in keys crate where we need to be able to get the bytes of any Kex public key. Previously, Kex was only implemented for X25519 and was not implemented for Ristretto, which prevented us from writing generic (curve agnostic) code in a lot of places.

In https://github.com/mobilecoinofficial/mobilecoin/pull/74 we fixed this, but we needed to introduce a trait GetBytes as a stopgap.

I propose that GetBytes can go away and be replaced with this version of ReprBytes. I propose that ReprBytes32 can also go away and be replaced with this version of ReprBytes.

I believe that ReprBytes should potentially be moved out of mobilecoin repo and treated as a "fundamental trait" in the RustCrypto ecosystem. Then, crates like prost and serde would integrate with ReprBytes trait, and e.g. Dalek crates would not have to know about prost or Dalek would not have to know about prost or serde, they would only have to know about ReprBytes.

Even if RustCrypto folks don't want to take ownership of this, this will reduce a lot of nasty code duplication in our repo now, and may help to clean up the keys crate, and make it able to achieve its goals of abstracting both X25519 and Ristretto.

Alternatives: In keys crate, the main alternative is, try to make everything implement AsRef<[u8]> and Into<Vec<u8>>. Because these are the "standard library interfaces" for doing this. However, traits like

/// A trait which all public key structures should implement.
pub trait PublicKey:
    Clone
    + Debug
    + DeserializeOwned
    + Digestible
    + Eq
    + Hash
    + Into<Vec<u8>>
    + PartialEq
    + PartialOrd
    + Ord
    + Serialize
    + Sized
    + for<'bytes> TryFrom<&'bytes [u8]>
{
    /// Retrieve the size of this public key's raw bytes.
    fn size() -> usize;
}

Are bad because they force a requirement on extern crate alloc. Because they have a trait bound on a core trait Into<Vec<u8>>, there is no way that a consumer of this API can implement the trait unless they also have access to alloc crate.

Suppose we wanted to take keys crate and add an alloc feature, so that it can be used in microcontroller environments without a standard allocator. All existing elliptic curve implementations do not require an allocator, and all of the code in RustCrypto is friendly to no-alloc development.

If we try to simply do #[cfg(feature = "alloc")] to make the Into<Vec<u8>> go away, then we are creating global feature coordination problems. Anything that implements keys traits will have to have an alloc feature, and that feature must be configured exactly the same way as the way that keys crate is configured. This is exactly the same problem that serde has, and the source of an enormous number of build problems, for us, for months. This would not be an "additive" use of features.

Cargo expects / requires that when a crate has features, turning them on is "additive", only adding things to the API, and not removing them. Otherwise, cargo's global feature unification is unsound and causes build breakage. Making a trait bound stricter when a feature is turned on is not additive, and the fact that serde itself does this, is a bug.

(My criticism of serde, in this way, with respect to non-additive std::error::Error, is the reason that they added this release of serde: https://github.com/serde-rs/serde/releases/tag/v1.0.100. I think the reason that it ended up this way in the first place is that serde is older than no_std, and when they created no_std they failed / neglected to move std::error::Error to core.)

So I think that we should expect to redesign the keys crate so that it does not have any dependence on Into<Vec<u8>> if we expect that it will eventually be taken up by RustCrypto folks. I think that a trait like ReprBytes can be an adequate replacement for it.

When we planned out the way that we build enclaves, one of the main questions was "how do we unit test the mission critical code that lives in enclaves". We split this logic into _impl crates that can compile as std, without having any sgx-only bits inside them.

Currently, none of these crates support logging, because the mc-common::logger module is std-only. We built an mc-sgx-slog interface which sends slog structures logs over an OCALL to untrusted to be logged on the global logger. But because this code was enclave-specific, there was no way to actually use this in the enclave/impl crates without breaking the tests. And so mc-sgx-slog has been basically unused. None of the _impl crates use it, and none of the tests even use the [test_with_logger] functionality, because they can't actually use the logger in the code under test.

After this revision we make the following changes:

• mc-common depends unconditionally on slog, and configures it in a no_std mode.
• mc-common unconditionally re-exports mc-common::logger, giving the mc-common::logger::log and mc-common::logger::slog parts, because these are portable to the enclaves.
• Everything that depends on slog now gets it from the mc-common re-export instead of duplicating the dependency in its cargo toml.
• slog is patched to fix minor problems in its no_std build when dynamic_keys feature is turned on.
• Functionality in mc-common::logger that depends on slog_scope is gated on log feature as before, and this requires std.
• Functionality in mc-common::logger that depends on loggers feature is still gated on loggers as before.
• The mc-sgx-slog::untrusted module is moved to the mc-sgx-urts crate with the other logging related stuff. This also means that some times that a server needs to depend mc-sgx-slog "only for linkage" are removed, because it gets the needed code from mc-sgx-urts now.
• mc-sgx-urts now sends eprintln and panic messages to global_log instead of eprintln.
• consensus-enclave-impl owns a Logger object where it sends log messages
• Tests in consensus-enclave-impl use test_with_logger and build the enclave using the test logger, to demonstrate that the functionality works.

The biggest drawback in this commit is that it makes the mc-common::logger module messy, but this module was already messy and was re-exporting a lot of complicated and variously portable stuff from slog-* crates. The alternative here is that everything starts depending on my patched slog, and if they don't all get the right version then the build breaks. In this commit, we have tried to keep that module as nice as possible without breaking the API that it defined.

We propose to consider changes to the API of the mc-common::logger module in another commit, because that will be onerous and will require touching almost the entire project, anything that uses the logger. By scoping this commit so that we don't change the API of mc-common we make it possible to land it and unblock other work that needs logging in enclaves.

It seems that the logger module needs to be redesigned with the idea that there are three different components that different consumers need:

• slog itself, which is totally portable
• slog_scope, which is not portable to enclave, depends on std and a quite complex ArcSwap crate. This is used to manage the lifetime of the global logger instance. It also depends on thread local vectors of logger pointers, to manage the scope loggers. This stuff is no_std compatible only in nightly compiler because it relies on thread_local. But we already have several pieces of code that are nightly only. IMO We should consider whether to patch this crate to make it no_std compatible, which would simplify stuff somewhat. We would have to get rid of ArcSwap but that might be for the best anyways, it's difficult for me to see why we need all that, we could require that set_global_logger is only called once and not allow swapping out global logger on the fly. I have never seen a legitimate use-case for that in any project. But it's not for sure that we even need the global_log API in enclaves, we can probably get by without it.
• sentry etc., which depends on std and an http stack, which we cannot compile with libmobilecoin or for the phones.

this increases the amount of time the test waits for mobilecoind and the mint auditor to catchup to network activity, and adds a named constant so that this can easily be adjusted in the future

Motivation

Implements Prometheus metrics for Fog View Router (FVR). Previously, upon receiving a request, the router "opened" a Prometheus metrics session, but it never closed it. This PR closes the session and refactors some underlying util logic for cleanliness.

Bumps prost from 0.11.2 to 0.11.5.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Bumps blake2 from 0.10.4 to 0.10.6.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Bumps blake2 from 0.10.4 to 0.10.6.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

The router_handlers::handle_requests method does the bulk of the work of the Fog Ledger Router. Currently any errors in this method are logged, but no more. We need to enumerate what kinds of errors this method can produce and discuss what other actions we might need to take over and above logging to handle them.

Each connection to a Key Image Store is currently creating its own grpcio environment. We should instead create this once and re-use it for all Key Image Store connections.