Skip to content

Commit

Permalink
conway: fix security policies
Browse files Browse the repository at this point in the history
  • Loading branch information
janmazak committed Nov 28, 2023
1 parent bc84029 commit 2289e9c
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 92 deletions.
154 changes: 68 additions & 86 deletions src/securityPolicy.c
Original file line number Diff line number Diff line change
Expand Up @@ -1018,77 +1018,72 @@ security_policy_t policyForSignTxCertificate(
DENY(); // should not be reached
}

security_policy_t _policyForSignTxCertificateStakeCredential(
sign_tx_signingmode_t txSigningMode,
const ext_credential_t* stakeCredential
// applicable to credentials that are witnessed in this tx
static bool forbiddenCredential(
sign_tx_signingmode_t txSigningMode,
const ext_credential_t* credential
)
{
switch (stakeCredential->type) {
case EXT_CREDENTIAL_KEY_PATH:
DENY_UNLESS(bip44_isOrdinaryStakingKeyPath(&stakeCredential->keyPath));
DENY_IF(violatesSingleAccountOrStoreIt(&stakeCredential->keyPath));
switch (txSigningMode) {
case SIGN_TX_SIGNINGMODE_ORDINARY_TX:
case SIGN_TX_SIGNINGMODE_PLUTUS_TX:
PROMPT();
break;

case SIGN_TX_SIGNINGMODE_MULTISIG_TX:
DENY();
break;

// certain combinations of tx signing mode and credential type are not allowed
// either because they don't make sense or are dangerous
switch (txSigningMode) {
case SIGN_TX_SIGNINGMODE_MULTISIG_TX:
switch (credential->type) {
case EXT_CREDENTIAL_KEY_PATH:
case EXT_CREDENTIAL_KEY_HASH:
// everything is expected to be governed by native scripts
return true;
default:
// in POOL_REGISTRATION signing modes, this certificate should have already been
// reported as invalid (only pool registration certificate is allowed)
ASSERT(false);
break;
}
break;

case EXT_CREDENTIAL_KEY_HASH:
switch (txSigningMode) {
case SIGN_TX_SIGNINGMODE_PLUTUS_TX:
PROMPT();
break;

case SIGN_TX_SIGNINGMODE_ORDINARY_TX:
case SIGN_TX_SIGNINGMODE_MULTISIG_TX:
DENY();
break;
case SIGN_TX_SIGNINGMODE_PLUTUS_TX:
// everything allowed, txs are too complex for a HW wallet to understand
// and there might be third-party key hashes in the tx
break;

case SIGN_TX_SIGNINGMODE_ORDINARY_TX:
switch (credential->type) {
case EXT_CREDENTIAL_KEY_HASH:
case EXT_CREDENTIAL_SCRIPT_HASH:
// keys must be given by path, otherwise the user does not know
// if the hash corresponds to some of his keys,
// and might inadvertently sign several certificates with a single witness
return true;
default:
// in POOL_REGISTRATION signing modes, this certificate should have already been
// reported as invalid (only pool registration certificate is allowed)
ASSERT(false);
break;
}
break;

case EXT_CREDENTIAL_SCRIPT_HASH:
switch (txSigningMode) {
case SIGN_TX_SIGNINGMODE_MULTISIG_TX:
case SIGN_TX_SIGNINGMODE_PLUTUS_TX:
PROMPT();
break;
default:
// this should not be called in POOL_REGISTRATION signing modes
ASSERT(false);
break;
}

case SIGN_TX_SIGNINGMODE_ORDINARY_TX:
DENY();
break;
return false;
}

default:
// in POOL_REGISTRATION signing modes, this certificate should have already been
// reported as invalid (only pool registration certificate is allowed)
ASSERT(false);
break;
}
security_policy_t _policyForSignTxCertificateStakeCredential(
sign_tx_signingmode_t txSigningMode,
const ext_credential_t* stakeCredential
)
{
DENY_IF(forbiddenCredential(txSigningMode, stakeCredential));

switch (stakeCredential->type) {
case EXT_CREDENTIAL_KEY_PATH:
DENY_UNLESS(bip44_isOrdinaryStakingKeyPath(&stakeCredential->keyPath));
DENY_IF(violatesSingleAccountOrStoreIt(&stakeCredential->keyPath));
break;

default:
ASSERT(false);
// the rest is OK, forbidden credentials have been dealt with above
break;
}

DENY(); // should not be reached
PROMPT();
}

// for certificates concerning stake keys and stake delegation
Expand Down Expand Up @@ -1128,27 +1123,22 @@ security_policy_t policyForSignTxCertificateVoteDelegation(
}

security_policy_t policyForSignTxCertificateCommitteeAuth(
sign_tx_signingmode_t txSigningMode,
const ext_credential_t* coldCredential,
const ext_credential_t* hotCredential
)
{
switch (coldCredential->type) {

case EXT_CREDENTIAL_KEY_HASH:
// the user would not know if it is his key or not
DENY();
break;
DENY_IF(forbiddenCredential(txSigningMode, coldCredential));

switch (coldCredential->type) {
case EXT_CREDENTIAL_KEY_PATH:
DENY_UNLESS(bip44_isCommitteeColdKeyPath(&coldCredential->keyPath));
break;

case EXT_CREDENTIAL_SCRIPT_HASH:
// allowed
DENY_IF(violatesSingleAccountOrStoreIt(&coldCredential->keyPath));
break;

default:
ASSERT(false);
// the rest is OK, forbidden credentials have been dealt with above
break;
}

switch (hotCredential->type) {
Expand All @@ -1170,52 +1160,42 @@ security_policy_t policyForSignTxCertificateCommitteeAuth(
}

security_policy_t policyForSignTxCertificateCommitteeResign(
sign_tx_signingmode_t txSigningMode,
const ext_credential_t* coldCredential
)
{
switch (coldCredential->type) {

case EXT_CREDENTIAL_KEY_HASH:
// the user would not know if it is his key or not
DENY();
break;
DENY_IF(forbiddenCredential(txSigningMode, coldCredential));

switch (coldCredential->type) {
case EXT_CREDENTIAL_KEY_PATH:
DENY_UNLESS(bip44_isCommitteeColdKeyPath(&coldCredential->keyPath));
break;

case EXT_CREDENTIAL_SCRIPT_HASH:
// allowed
DENY_IF(violatesSingleAccountOrStoreIt(&coldCredential->keyPath));
break;

default:
ASSERT(false);
// the rest is OK, forbidden credentials have been dealt with above
break;
}

PROMPT();
}

security_policy_t policyForSignTxCertificateDRep(
const ext_credential_t* drepCredential
sign_tx_signingmode_t txSigningMode,
const ext_credential_t* dRepCredential
)
{
switch (drepCredential->type) {

case EXT_CREDENTIAL_KEY_HASH:
// the user would not know if it is his key or not
DENY();
break;
DENY_IF(forbiddenCredential(txSigningMode, dRepCredential));

switch (dRepCredential->type) {
case EXT_CREDENTIAL_KEY_PATH:
DENY_UNLESS(bip44_isDRepKeyPath(&drepCredential->keyPath));
break;

case EXT_CREDENTIAL_SCRIPT_HASH:
// allowed
DENY_UNLESS(bip44_isDRepKeyPath(&dRepCredential->keyPath));
DENY_IF(violatesSingleAccountOrStoreIt(&dRepCredential->keyPath));
break;

default:
ASSERT(false);
// the rest is OK, forbidden credentials have been dealt with above
break;
}

PROMPT();
Expand Down Expand Up @@ -1534,7 +1514,6 @@ static inline security_policy_t _ordinaryWitnessPolicy(const bip44_path_t* path,
case PATH_COMMITTEE_HOT_KEY:
// these have to be shown because the tx might contain
// an action proposal that cannot be fully shown on the device
// TODO what about violation of single account policy?
DENY_IF(violatesSingleAccountOrStoreIt(path));
WARN_UNLESS(bip44_isPathReasonable(path));
SHOW();
Expand Down Expand Up @@ -1596,6 +1575,9 @@ static inline security_policy_t _plutusWitnessPolicy(const bip44_path_t* path, b
case PATH_ORDINARY_STAKING_KEY:
case PATH_MULTISIG_SPENDING_KEY:
case PATH_MULTISIG_STAKING_KEY:
case PATH_DREP_KEY:
case PATH_COMMITTEE_COLD_KEY:
case PATH_COMMITTEE_HOT_KEY:
WARN_UNLESS(bip44_isPathReasonable(path));
SHOW();
break;
Expand Down
3 changes: 3 additions & 0 deletions src/securityPolicy.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,16 @@ security_policy_t policyForSignTxCertificateVoteDelegation(
const ext_drep_t* drep
);
security_policy_t policyForSignTxCertificateCommitteeAuth(
sign_tx_signingmode_t txSigningMode,
const ext_credential_t* coldCredential,
const ext_credential_t* hotCredential
);
security_policy_t policyForSignTxCertificateCommitteeResign(
sign_tx_signingmode_t txSigningMode,
const ext_credential_t* coldCredential
);
security_policy_t policyForSignTxCertificateDRep(
sign_tx_signingmode_t txSigningMode,
const ext_credential_t* drepCredential
);
#ifdef APP_FEATURE_POOL_RETIREMENT
Expand Down
17 changes: 14 additions & 3 deletions src/signTx.c
Original file line number Diff line number Diff line change
Expand Up @@ -1112,26 +1112,34 @@ static void _setDRep(
{
switch (extDRep->type) {

case EXT_CREDENTIAL_KEY_PATH:
case EXT_DREP_KEY_PATH:
drep->type = DREP_KEY_HASH;
bip44_pathToKeyHash(
&extDRep->keyPath,
drep->keyHash, SIZEOF(drep->keyHash)
);
break;

case EXT_CREDENTIAL_KEY_HASH:
case EXT_DREP_KEY_HASH:
drep->type = DREP_KEY_HASH;
STATIC_ASSERT(SIZEOF(drep->keyHash) == SIZEOF(extDRep->keyHash), "bad script hash container size");
memmove(drep->keyHash, extDRep->keyHash, SIZEOF(extDRep->keyHash));
break;

case EXT_CREDENTIAL_SCRIPT_HASH:
case EXT_DREP_SCRIPT_HASH:
drep->type = DREP_SCRIPT_HASH;
STATIC_ASSERT(SIZEOF(drep->scriptHash) == SIZEOF(extDRep->scriptHash), "bad script hash container size");
memmove(drep->scriptHash, extDRep->scriptHash, SIZEOF(extDRep->scriptHash));
break;

case EXT_DREP_ALWAYS_ABSTAIN:
drep->type = DREP_ALWAYS_ABSTAIN;
break;

case EXT_DREP_ALWAYS_NO_CONFIDENCE:
drep->type = DREP_ALWAYS_NO_CONFIDENCE;
break;

default:
ASSERT(false);
break;
Expand Down Expand Up @@ -1348,6 +1356,7 @@ static void _handleCertificateVoteDeleg()
static void _handleCertificateCommitteeAuth()
{
security_policy_t policy = policyForSignTxCertificateCommitteeAuth(
ctx->commonTxData.txSigningMode,
&BODY_CTX->stageData.certificate.committeeColdCredential,
&BODY_CTX->stageData.certificate.committeeHotCredential
);
Expand All @@ -1371,6 +1380,7 @@ static void _handleCertificateCommitteeAuth()
static void _handleCertificateCommitteeResign()
{
security_policy_t policy = policyForSignTxCertificateCommitteeResign(
ctx->commonTxData.txSigningMode,
&BODY_CTX->stageData.certificate.committeeColdCredential
);
TRACE("Policy: %d", (int) policy);
Expand All @@ -1393,6 +1403,7 @@ static void _handleCertificateCommitteeResign()
static void _handleCertificateDRep()
{
security_policy_t policy = policyForSignTxCertificateDRep(
ctx->commonTxData.txSigningMode,
&BODY_CTX->stageData.certificate.drepCredential
);
TRACE("Policy: %d", (int) policy);
Expand Down
3 changes: 0 additions & 3 deletions src/signTx_ui.c
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,6 @@ void signTx_handleCertificateVoteDeleg_ui_runStep()
set_light_confirmation(true);
display_prompt("Delegate\nvote", "", this_fn, respond_with_user_reject);
#endif // HAVE_BAGL
break;
}
UI_STEP(HANDLE_CERTIFICATE_VOTE_DELEG_STEP_DISPLAY_STAKE_CRED) {
_displayCredential(
Expand Down Expand Up @@ -811,7 +810,6 @@ void signTx_handleCertificateCommitteeResign_ui_runStep()
set_light_confirmation(true);
display_prompt("Resign from\ncommittee", "", this_fn, respond_with_user_reject);
#endif // HAVE_BAGL
break;
}
UI_STEP(HANDLE_CERTIFICATE_COMM_RESIGN_STEP_DISPLAY_COLD_CRED) {
_displayCredential(
Expand Down Expand Up @@ -877,7 +875,6 @@ void signTx_handleCertificateCommitteeAuth_ui_runStep()
set_light_confirmation(true);
display_prompt("Authorize committee", "", this_fn, respond_with_user_reject);
#endif // HAVE_BAGL
break;
}
UI_STEP(HANDLE_CERTIFICATE_COMM_AUTH_STEP_DISPLAY_COLD_CRED) {
_displayCredential(
Expand Down

0 comments on commit 2289e9c

Please sign in to comment.