Skip to content

Commit 825e5f8

Browse files
authored
docs(security): address post-merge feedback from Robin on PR 204 (#253)
## Summary Addresses all feedback from @robin-kunzler posted on PR #204 after merge. - **`identity-and-access-management.mdx`**: Link both mentions of "ICP JavaScript agent" to the developer-tools page (`#javascript--typescript` section), rather than the GitHub repo as originally suggested; internal docs link is more useful for readers - **`data-integrity-and-authenticity.md`**: Add link to `calling-from-clients.md` after the update/query/composite_query intro paragraph; append certified-variables guide link at the end of the certified data concept paragraph - **`https-outcalls.md`**: Link inline "pricing" directly to `cycles-costs.md#https-outcalls` anchor - **`observability-and-monitoring.md`**: Restore the two original portal sections ("Expose metrics from your canister" and "Do not publicly reveal a canister's cycles balance") that had been collapsed into a single generic section during the 1:1 port - **`references/message-execution-properties.md`**: Convert Property 1–11 from bold bullet points to `####` subheadings so cross-links (e.g. "Property 5") can anchor directly - **`references/index.md`**: Add `message-execution-properties.md` to the Specifications section so it appears in the navigation (Robin reported the page was not findable) **Note on developer-tools links**: The two links in `identity-and-access-management.mdx` currently point to `references/developer-tools.md#javascript--typescript`. When PR #251 (`infra/developer-tools-section`) merges, those paths will move to `developer-tools/index.md#javascript--typescript`. A comment has been left on PR #251 to flag this. Whichever PR merges second handles the one-line update; the build will catch it. ## Sync recommendation `sync from dfinity/portal — building-apps/security/observability-and-monitoring.mdx`
1 parent 191e8ef commit 825e5f8

9 files changed

Lines changed: 68 additions & 36 deletions

docs/guides/security/data-integrity-and-authenticity.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,14 @@ sidebar:
99

1010
### Security concern
1111

12-
ICP offers three modes of operation for canisters: `update`, `query`, and `composite_query`. For simplicity, this guide treats `composite_query` methods as query methods for the rest of this section.
12+
ICP offers three modes of operation for canisters: `update`, `query`, and `composite_query`. For simplicity, this guide treats `composite_query` methods as query methods for the rest of this section. For more information, view the [detailed overview between update and query calls](../canister-calls/inter-canister-calls.md#query-vs-update-calls).
1313

1414
Update calls are slow and expensive but provide integrity guarantees as their responses include a threshold signature signed by the subnet.
1515

1616
On the other hand, query calls are fast since a single replica formulates the response, but **there is no integrity guarantee, since the response can be manipulated by a single replica or boundary node.** For example, if the NNS app fetches proposal information from the governance canister via query calls and the responding node is malicious, it can mask an ill-intentioned proposal that causes irrevocable damage as innocuous by modifying the proposal payload in the response and mislead voters into voting yes. Another consequence of query calls is that users can't rely on [canister_inspect_message](../../references/ic-interface-spec/canister-interface.md#system-api-inspect-message) as a guard. **This makes query calls, in their raw form, unfit to serve data for security-critical applications.**
1717

1818
### Using certified variables for secure queries
19-
In certain use cases, there is a third option whereby query results can return data that has been certified by the subnet in an earlier update call. This is the concept of certified data, and it requires changes to the update call to create the certification, the query call to return the certificate, and the frontend to verify the certificate. Using certified data provides query-like response times with update-like certified responses.
19+
In certain use cases, there is a third option whereby query results can return data that has been certified by the subnet in an earlier update call. This is the concept of certified data, and it requires changes to the update call to create the certification, the query call to return the certificate, and the frontend to verify the certificate. Using certified data provides query-like response times with update-like certified responses. This forms the core of [certified variables](../backends/certified-variables.md).
2020

2121
Some examples of certified variables are asset certification in [Internet Identity](https://github.com/dfinity/internet-identity/blob/b29a6f68bbe5a49d048e12bc7a3263a9f43d080b/src/internet_identity/src/main.rs#L775-L808), [NNS app](https://github.com/dfinity/nns-dapp/blob/372c3562127d70c2fde059bc9c268e8ae858583e/rs/src/assets.rs#L121-L145), or the [canister signature implementation in Internet Identity](https://github.com/dfinity/ic-canister-sig-creation).
2222

docs/guides/security/https-outcalls.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ See the [HTTPS outcalls guide](../backends/https-outcalls.md) for more details.
7070

7171
### Security concern
7272

73-
The pricing of HTTPS outcalls is determined by the size of the HTTP request and the maximal response size, among other variables. Thus, if big requests are made, this could quickly drain the canister's cycles balance. This can be risky in scenarios where HTTPS outcalls are triggered by user actions (rather than a heartbeat or timer invocation).
73+
The [pricing](../../references/cycles-costs.md#https-outcalls) of HTTPS outcalls is determined by the size of the HTTP request and the maximal response size, among other variables. Thus, if big requests are made, this could quickly drain the canister's cycles balance. This can be risky in scenarios where HTTPS outcalls are triggered by user actions (rather than a heartbeat or timer invocation).
7474

7575
### Recommendation
7676

docs/guides/security/identity-and-access-management.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ Implementing user authentication and canister calls yourself in your web app is
7979

8080
### Recommendation
8181

82-
- Consider using an identity provider such as [Internet Identity](https://github.com/dfinity/internet-identity) for authentication, and use the ICP JavaScript agent for making canister calls.
82+
- Consider using an identity provider such as [Internet Identity](https://github.com/dfinity/internet-identity) for authentication, and use the [ICP JavaScript agent](../../references/developer-tools.md#javascript--typescript) for making canister calls.
8383

8484
- You may consider alternative authentication frameworks on ICP for authentication.
8585

@@ -99,7 +99,7 @@ The auth-client supports [idle timeouts](https://js.icp.build/auth/latest/api/cl
9999

100100
### Security concern
101101

102-
`agent.fetchRootKey()` can be used in the ICP JavaScript agent to fetch the root subnet threshold public key from a status call in test environments. This key is used to verify threshold signatures on certified data received through canister update calls. Using this method in a production web app gives an attacker the option to supply their own public key, invalidating all authenticity guarantees of update responses.
102+
`agent.fetchRootKey()` can be used in the [ICP JavaScript agent](../../references/developer-tools.md#javascript--typescript) to fetch the root subnet threshold public key from a status call in test environments. This key is used to verify threshold signatures on certified data received through canister update calls. Using this method in a production web app gives an attacker the option to supply their own public key, invalidating all authenticity guarantees of update responses.
103103

104104
### Recommendation
105105

docs/guides/security/inter-canister-calls.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ This is also explained in the [community conversation on security best practices
1313

1414
### Security concern
1515

16-
Traps and panics roll back the canister state, as described in [Property 5](../../references/message-execution-properties.md#message-execution-properties). So any state change followed by a trap or panic can be risky. This is an important concern when inter-canister calls are made. If a trap occurs after an await to an inter-canister call, then the state is reverted to the snapshot before the inter-canister call's callback invocation, and not to the state before the entire call.
16+
Traps and panics roll back the canister state, as described in [Property 5](../../references/message-execution-properties.md#property-5). So any state change followed by a trap or panic can be risky. This is an important concern when inter-canister calls are made. If a trap occurs after an await to an inter-canister call, then the state is reverted to the snapshot before the inter-canister call's callback invocation, and not to the state before the entire call.
1717

1818
More precisely, suppose some state changes are applied and then an inter-canister call is issued. Also, assume that these state changes leave the canister in an inconsistent state, and that state is only made consistent again in the callback. Now if there is a trap in the callback, this leaves the canister in an inconsistent state.
1919

@@ -128,11 +128,11 @@ GoldDAO's GLDT-swap has an implementation of journaling. In their case, the jour
128128

129129
### Security concern
130130

131-
As described in the [properties of message executions on ICP](../../references/message-execution-properties.md), messages (but not entire calls) are processed atomically. In particular, as described in Property 4 in that document, messages from interleaving calls do not have a reliable execution ordering. Thus, the state of the canister (and other canisters) may change between the time an inter-canister call is started and the time when it returns, which may lead to issues if not handled correctly. These issues are generally called 'reentrancy bugs' (see the [Ethereum best practices on reentrancy](https://consensysdiligence.github.io/smart-contract-best-practices/attacks/reentrancy/)). Note, however, that the messaging guarantees, and thus the bugs, on ICP are different from Ethereum.
131+
As described in the [properties of message executions on ICP](../../references/message-execution-properties.md), messages (but not entire calls) are processed atomically. In particular, as described in [Property 4](../../references/message-execution-properties.md#property-4) in that document, messages from interleaving calls do not have a reliable execution ordering. Thus, the state of the canister (and other canisters) may change between the time an inter-canister call is started and the time when it returns, which may lead to issues if not handled correctly. These issues are generally called 'reentrancy bugs' (see the [Ethereum best practices on reentrancy](https://consensysdiligence.github.io/smart-contract-best-practices/attacks/reentrancy/)). Note, however, that the messaging guarantees, and thus the bugs, on ICP are different from Ethereum.
132132

133133
Here are two concrete and somewhat similar types of bugs to illustrate potential reentrancy security issues:
134134

135-
- **Time-of-check time-of-use issues:** These occur when some condition on global state is checked before an inter-canister call and then wrongly assuming the condition still holds when the call returns. For example, one might check if there is sufficient balance on some account, then issue an inter-canister call, and finally make a transfer as part of the callback message. When the second inter-canister call starts, it is possible that the condition that was checked initially no longer holds, because other ledger transfers may have happened before the callback of the first call is executed (see also Property 4 above).
135+
- **Time-of-check time-of-use issues:** These occur when some condition on global state is checked before an inter-canister call and then wrongly assuming the condition still holds when the call returns. For example, one might check if there is sufficient balance on some account, then issue an inter-canister call, and finally make a transfer as part of the callback message. When the second inter-canister call starts, it is possible that the condition that was checked initially no longer holds, because other ledger transfers may have happened before the callback of the first call is executed (see also [Property 4](../../references/message-execution-properties.md#property-4)).
136136

137137
- **Double-spending issues**: Such issues occur when a transfer is issued twice, often because of unfavorable message scheduling. For example, suppose you check if a caller is eligible for a refund, and if so, transfer some refund amount to them. When the refund ledger call returns successfully, you set a flag in the canister storage indicating that the caller has been refunded. This is vulnerable to double-spending because the refund method can be called twice by the caller in parallel, in which case it is possible that the messages before issuing the transfer (including the eligibility check) are scheduled before both callbacks. A detailed explanation of this issue can be found in the [community conversation on security best practices](https://www.youtube.com/watch?v=PneRzDmf_Xw&list=PLuhDt1vhGcrez-f3I0_hvbwGZHZzkZ7Ng&index=2&t=4s).
138138

@@ -294,7 +294,7 @@ Finally, note that the same guard can be used in several methods to restrict par
294294

295295
### Security concern
296296

297-
As stated by the [Property 6](../../references/message-execution-properties.md#message-execution-properties), inter-canister calls can fail in which case they result in a **reject**. See [reject codes](../../references/ic-interface-spec/https-interface.md#reject-codes) for more detail. The caller must correctly deal with the reject cases, as they can happen in normal operation, because of insufficient cycles on the sender or receiver side, or because some data structures like message queues are full.
297+
As stated by the [Property 6](../../references/message-execution-properties.md#property-6), inter-canister calls can fail in which case they result in a **reject**. See [reject codes](../../references/ic-interface-spec/https-interface.md#reject-codes) for more detail. The caller must correctly deal with the reject cases, as they can happen in normal operation, because of insufficient cycles on the sender or receiver side, or because some data structures like message queues are full.
298298

299299
1. The call was issued as a bounded-wait (best-effort response) call, and the system responded with a `SYS_UNKNOWN` reject code. In this case, the caller cannot be a priori sure whether the call took effect or not.
300300
2. The system responded with a `CANISTER_ERROR` reject code. This indicates a bug in the ledger canister. In this case, it is still possible that the call had a partial effect on the ledger canister.

docs/guides/security/observability-and-monitoring.md

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,24 @@ sidebar:
55
order: 9
66
---
77

8-
## Monitor your canister
8+
## Expose metrics from your canister
99

1010
### Security concern
1111

12-
Without monitoring, it can be hard to detect attacks or vulnerabilities that are being actively exploited. For example, a sudden increase in cycles consumption could indicate a DoS attack, while unexpected changes in canister state could indicate a security breach.
12+
In case of attacks, it is great to be able to obtain relevant metrics from canisters, such as the number of accounts, size of internal data structures, stable memory, etc.
1313

1414
### Recommendation
1515

16-
- Monitor your canister's cycles balance regularly, set up alerts for sudden changes in cycles consumption, and add an endpoint to expose health indicators. See the [DoS prevention best practices](./dos-prevention.md) for more context on cycles monitoring.
16+
[Expose metrics from your canister](https://mmapped.blog/posts/01-effective-rust-canisters.html#expose-metrics) (from [effective Rust canisters](https://mmapped.blog/posts/01-effective-rust-canisters.html)).
1717

18-
- Consider emitting logs for security-relevant events (e.g., access control failures, unexpected state transitions). Since logs are stored in the canister, they provide a tamperproof audit trail.
18+
## Do not publicly reveal a canister's cycles balance
1919

20-
- See [effective Rust canisters](https://mmapped.blog/posts/01-effective-rust-canisters.html) for general patterns on canister observability.
20+
### Security concern
21+
22+
Publicly revealing the canister's cycles balance allows an attacker to measure the number of instructions spent by executing the canister methods on the attacker's input. Then the attacker might be able to learn which code paths were taken during execution and derive secret information based on that. Moreover, the attacker can learn which methods and their inputs consume a lot of cycles to mount a cycles-draining attack (see also [protect against draining the cycles balance](./dos-prevention.md#handle-expensive-calls)).
23+
24+
### Recommendation
25+
26+
Your canisters should not publicly expose their cycles balance (available through the system API), i.e., they should only expose their cycles balance to their controllers or other trusted principals.
2127

2228
<!-- Upstream: sync from dfinity/portal building-apps/security/observability-and-monitoring.mdx -->

docs/references/glossary.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
title: "Glossary"
33
description: "Definitions of ICP-specific terms: canister, cycle, principal, subnet, and more"
44
sidebar:
5-
order: 15
5+
order: 17
66
---
77

88
# Glossary

docs/references/index.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ Technical reference material for ICP development. These pages cover exact specif
3333
## Specifications
3434

3535
- **[IC Interface Specification](ic-interface-spec/index.md)**: System API, HTTPS interface, certified data, management canister, and formal specification of the Internet Computer.
36+
- **[Message Execution Properties](message-execution-properties.md)**: The 11 properties governing atomicity, ordering, inter-canister call delivery, and cycle handling in ICP message execution.
3637
- **[HTTP Gateway Specification](http-gateway-protocol-spec.md)**: How boundary nodes serve canister HTTP responses with certification verification.
3738
- **[Candid Specification](candid-spec.md)**: The Candid interface description language: type system, encoding, and subtyping rules.
3839
- **[Internet Identity Specification](internet-identity-spec.md)**: Delegation chains, passkey management, and canister signatures.

0 commit comments

Comments
 (0)