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

Proposal for upgradable program recovery #27460

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
96 changes: 96 additions & 0 deletions docs/src/proposals/program-recovery.md
@@ -0,0 +1,96 @@
# Program Recovery

## Problem

The Upgradable Program Loader makes closing programs an irreversible action.

After closing a program, the [`Program`] points to a non-existent [`ProgramData`] account.
Note that the `Program` account itself still exists, as it is executable, and thus immutable.

[`Program`]: https://docs.rs/solana-sdk/latest/solana_sdk/bpf_loader_upgradeable/enum.UpgradeableLoaderState.html#variant.Program
[`ProgramData`]: https://docs.rs/solana-sdk/latest/solana_sdk/bpf_loader_upgradeable/enum.UpgradeableLoaderState.html#variant.ProgramData

Below is an instance of a closed program (as of epoch 343).

```
# Irrecoverable Program account
% solana account optFiKjQpoQ3PvacwnFWaPUAqXCETMJSz2sz8HwPe9B

Public Key: optFiKjQpoQ3PvacwnFWaPUAqXCETMJSz2sz8HwPe9B
Balance: 0.00114144 SOL
Owner: BPFLoaderUpgradeab1e11111111111111111111111
Executable: true
Rent Epoch: 310
Length: 36 (0x24) bytes
0000: 02 00 00 00 89 d1 af 82 0a c8 fc e8 89 1f 84 d6 ................
0010: ee 7d 70 aa 34 60 99 85 fa 30 3a 08 37 ff b6 4d .}p.4`...0:.7..M
0020: b7 20 60 c2 . `.

# Irrecoverable ProgramData account
% solana account AGzJVXAdQkyN8mDw619DuAnMyz4ypBJNZHSY7zKKJuvh
Error: AccountNotFound: pubkey=AGzJVXAdQkyN8mDw619DuAnMyz4ypBJNZHSY7zKKJuvh
```

In Solana v1.11, the upgradable loader lacks instructions to recreate a `ProgramData` account that has been closed.
Closed programs are thus permanently irrecoverable.
Any PDAs associated with a closed program would be permanently inaccessible.

This behavior is counterintuitive.
Closed accounts of most other programs can be recreated (e.g. system program, SPL token program)
Copy link
Member

Choose a reason for hiding this comment

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

I think that SPL token accounts are a really good example of why making this change is actually more intuitive since they can be recreated even if a new authority closes the account. Furthermore, most people don't realize that closing a program is irreversible which leads me to agree that it's an unintuitive implementation


## Proposed Solution

The upgradable loader should allow the deployer to recreate `ProgramData` accounts for `Program`s with a matching `programdata_address`.

Relax constraints of `UpgradeableLoaderInstruction::DeployWithMaxDataLen`.

```diff
/// # Account references
/// 0. `[signer]` The payer account that will pay to create the ProgramData
/// account.
/// 1. `[writable]` The uninitialized ProgramData account.
-/// 2. `[writable]` The uninitialized Program account.
+/// 2. `[writable]` The Program account.
/// 3. `[writable]` The Buffer account where the program data has been
/// written. The buffer account's authority must match the program's
/// authority
/// 4. `[]` Rent sysvar.
/// 5. `[]` Clock sysvar.
/// 6. `[]` System program (`solana_sdk::system_program::id()`).
/// 7. `[signer]` The program's authority
DeployWithMaxDataLen {
/// Maximum length that the program can be upgraded to.
max_data_len: usize,
},

```

Instead of requiring the account 2 to be `Uninitialized`, the instruction should also permit `Program` accounts.

The following rules apply if an initialized program is given to `DeployWithMaxDataLen`:
- The `programdata_address` of the `Program` must match account 1.
- The program account must be a signer.

Deployers can use the `DeployWithData` instruction to recover a closed program, provided they can sign for the program ID.

## Concerns

### Program authority is reset

Closing a program now resets the program authority to the program's address.

Entities in possession of the key of a closed program now gain access to the program and its PDAs.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has serious implications for PDA ownership. The effects of this change should be audited.

I propose the following process:

  • Create a list of all existing PDAs that are owned by a closed program
  • Estimate the value stored within these PDAs

PDAs could be enumerated with this procedure:

  • List all closed programs
  • List the complete transaction history of all closed programs
  • Derive the set of PDAs that ever interacted with the program by walking transaction history (obvious if the signer bit is true in the absence of a signature)
  • Look up each PDA to see if it still exists today

Choose a reason for hiding this comment

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

Changing the authority back seems like a dangerous thing to do. I know of a number of projects who did not keep their program-deploy-key safe, as is previously didn't matter for security.

And having the key around (i.e. checked into git) allows you to deploy the program at the same address in both the real mainnet and a test cluster (in CI or whatever).

Copy link
Contributor Author

@riptl riptl Aug 30, 2022

Choose a reason for hiding this comment

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

I know of a number of projects who did not keep their program-deploy-key safe, as is previously didn't matter for security.

It still doesn't matter for deployed programs. The recovery extensions only authorize the deploy key by default if the program data account does not exist. (Making closing accounts as dangerous as resetting the deploy authority to the deploy key manually -- which is intuitive behavior, since SPL token program also behaves that way)

Closed programs rising from the dead because of compromised keys sounds dangerous though. It could compromise live programs that authorize CPIs from these closed programs. This is much harder to audit

Choose a reason for hiding this comment

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

Unfortunately, determining a programs PDAs is an impossible task in theory.

In practice it can likely be approximated quite well, but even that requires full historical chain data.

Core issue is that token accounts will be owned by PDAs, and without knowing seeds you can't calculate the PDAs. But seeds can be dependent on user input to a transaction -> there might never have been a transaction for a certain PDA, but a program can still access it, if provided the correct input.

In practice, most PDAs will be used fairly often, so you can hook a validator to output all seeds a program uses and build a database with that.

I wanted that kind of information a number of times, but its hard to come by :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, thanks for sharing. The issue sounds much more complicated considering that and the hidden CPI issue (recovered accounts being authorized to interact with live programs). It's not possible to audit the changes of this proposal with certainty.

I think this warrants a call to the dev community then to review their dead programs for PDAs and live programs for loose authorizations.

Copy link
Member

Choose a reason for hiding this comment

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

We can certainly get a list of closed programs for informational purposes but I agree that it's not possible to exhaustively determine what the revived program would be able to recover. Even if we did, I don't think it's likely that a program authority intentionally burned assets by closing the program and so I don't think it's relevant. It also would be irresponsible to publicize any private key corresponding to a mainnet account key so I'm not concerned about that scenario either.


### Other kinds of irrecoverable programs

This proposal might create a false sense of security and encourage reckless behavior.

Only a specific composition of upgradable loader accounts can be recovered using this method.
There will always be other ways to create a situation where a program is irrecoverable.

### Deliberately disabling programs

Users may have relied on the program close instruction to permanently disable ("brick") a program.
However, the program close method has not been documented as irrecoverable.

To brick a program, users should upgrade the program to an abort instruction and freeze it (by setting the authority to None).