Skip to content

contract audit proposal#168

Open
pixelplex wants to merge 1 commit intocanton-foundation:mainfrom
pixelplex:contracts_audit_proposal
Open

contract audit proposal#168
pixelplex wants to merge 1 commit intocanton-foundation:mainfrom
pixelplex:contracts_audit_proposal

Conversation

@pixelplex
Copy link

No description provided.

Copy link
Contributor

@meiersi-da meiersi-da left a comment

Choose a reason for hiding this comment

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

Thanks a lot. This is a good first stab at the problem.

I think the biggest missing item is support for computing the target vetting state of wallet providers. See my comments.

It might make sense to call out their computation in the intro, and consider the main goal to compute vetting states that allow as many benign apps to be used by users of as many wallet providers as possible, while minimizing the chance of users being exposed to a malicious app.


## Abstract

This CIP proposes a standardized protocol for secure interaction between three parties in the Daml application ecosystem: **AppProviders**, **WalletProviders**, and **SecurityAuditors**. The standard defines how AppProviders publish Daml packages (.dar files) with cryptographically verifiable build metadata, how they request security audits from independent SecurityAuditors, and how WalletProviders can discover and verify audit results to make informed security decisions before integrating third-party packages.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This CIP proposes a standardized protocol for secure interaction between three parties in the Daml application ecosystem: **AppProviders**, **WalletProviders**, and **SecurityAuditors**. The standard defines how AppProviders publish Daml packages (.dar files) with cryptographically verifiable build metadata, how they request security audits from independent SecurityAuditors, and how WalletProviders can discover and verify audit results to make informed security decisions before integrating third-party packages.
This CIP proposes a standardized protocol for secure interaction between three parties in the Daml application ecosystem: **App Providers**, **Wallet Providers**, and **Security Auditors**. The standard defines how AppProviders publish Daml packages (.dar files) with cryptographically verifiable build metadata, how they request security audits from independent SecurityAuditors, and how WalletProviders can discover and verify audit results to make informed security decisions before integrating third-party packages.

We usually don't use camel case for roles in flow-text. See https://github.com/canton-foundation/cips/blob/main/cip-0056/cip-0056.md#overview for prior art on defining roles.

#### Parties

- **AppProvider**: An organization or individual developing Daml applications and publishing Daml packages (.dar files)
- **WalletProvider**: A service provider managing digital assets on behalf of users, integrating third-party Daml applications
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- **WalletProvider**: A service provider managing digital assets on behalf of users, integrating third-party Daml applications
- **Validator Node Provider**: A service provider providing users with access t to a Canton Network validator node hosting their parties.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please check all text wrt "managing digital assets", as that is something reserved to properly licensed asset managers. Neither wallet providers nor node provider ever do that; and they do not want to do that as that requires special licenses in most jurisdictions.


#### Package Metadata Format

The `metadata.json` file for each package version follows this JSON schema:
Copy link
Contributor

Choose a reason for hiding this comment

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

consider using JSON schema to specify the schema itself, as done by Ledger for ClearSign here: https://github.com/LedgerHQ/clear-signing-erc7730-registry/tree/master/specs

app-provider-repo/
├── README.md
├── packages/
│ ├── v1.0.0/
Copy link
Contributor

Choose a reason for hiding this comment

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

use full package name, e.g., splice-amulet-0.1.16 to avoid conflicts.

Also support a _<pkgId> suffix for the case where an app provider did accidentally publish multiple packages for the same version and needs to publish metadata about both of them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Consider making this the default format so that we never have a conflict, but still get the good human readability.

├── audits.json
└── build-configs/
├── v1.0.0/
│ └── build-config.json
Copy link
Contributor

Choose a reason for hiding this comment

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

why are build configs separate? seems like extra work to maintain the copy of the structure for what could have just been another file in the same directory.

"severity_rating": "none",
"expiration_date": "2027-02-27T00:00:00Z"
}
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not inline the audits as well into the package itself to avoid repeating the same structure?

| 1.0.0 | SecurityAuditorCompany | 2026-02-20 | ✓ Passed | [Audit Report](https://github.com/auditor/audit-reports/blob/main/reports/app-provider-name/1.0.0/audit-report.json) |
| 1.1.0 | SecurityAuditorCompany | 2026-02-26 | ✓ Passed | [Audit Report](https://github.com/auditor/audit-reports/blob/main/reports/app-provider-name/1.1.0/audit-report.json) |
| 1.1.0 | AnotherAuditor | 2026-02-25 | ✓ Passed | [Audit Report](https://github.com/another-auditor/audits/blob/main/reports/app-provider/1.1.0/audit.json) |

Copy link
Contributor

Choose a reason for hiding this comment

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

What about exposing this as a TypeScript script that produces the summary from the information in the packages/ dir?

Comment on lines +265 to +269
"authorization_logic",
"party_confidentiality",
"contract_integrity",
"dependency_safety",
"daml_best_practices"
Copy link
Contributor

Choose a reason for hiding this comment

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

Where are these properties defined?

"decision_notes": "string"
}
]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need a separate process for this?

I was thinking that a wallet provider would

  1. define the app repositories for apps to integrate, using git submodules
  2. define the auditor repositories to use, using git submodules
  3. a trust policy that defines how to compute the target vetting state on their nodes from the data in the above repositories

On the last point, I believe we are missing one crucial part of metadata in the App Provider's repos: which vetting state to use for their apps. I'd suggest to address this by introducing a top-level directory that lists the apps, and then within that directory define the packages that are contributed by this app.

Here's a sketch how that would look like for the apps contributed by Splice:

canton-coin/
  packages/
    splice-util-0.1.5_5a58024e2cc488ca9e0c952ec7ef41da3a1ed0a78ba23bacd819e5b30afb5546/
    splice-amulet-0.1.16_c208d7ead1e4e9b610fc2054d0bf00716144ad444011bce0b02dcd6cd0cb8a23/
    ...
  vetting-states/
    mainnet.json
    devnet.json
    testnet.json


global-sync-governance/
  packages/
    splice-dso-governance-0.1.22_5c28530209b9ab37c5f187132cd826709bb18b0efe28411488ab750870414738/
  vetting-states/
    mainnet.json
    testnet.json
    devnet.json

...

The ... at the bottom refers to like app definitions for the token standard APIs and other packages in in https://github.com/hyperledger-labs/splice/blob/main/daml/dars.lock

Copy link
Contributor

Choose a reason for hiding this comment

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

I would then also like to see a script for computing the target vetting state as part of the reference implementation of this CIP. The target vetting state will need to be computed in a single pass to take into account dependencies.

I'd probably even consider that wallet providers will want to check-in the vetting state command to use, as that gives a great way on the diff of adding/removing/updating apps.


We do not require authentication on audit result URLs because:
- All data is expected to be public
- URLs are hard to guess (based on package hashes and identifiers)
Copy link
Contributor

Choose a reason for hiding this comment

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

package hashes are part of the publicly available topology state. So everybody sees all package hashes.

The question is though why the mechanisms of git repo access controls are not sufficient.

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.

2 participants