diff --git a/src/securityPolicy.c b/src/securityPolicy.c index e5efa102..7abe6b95 100644 --- a/src/securityPolicy.c +++ b/src/securityPolicy.c @@ -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 @@ -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) { @@ -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(); @@ -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(); @@ -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; diff --git a/src/securityPolicy.h b/src/securityPolicy.h index c52a7053..a804fa0a 100644 --- a/src/securityPolicy.h +++ b/src/securityPolicy.h @@ -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 diff --git a/src/signTx.c b/src/signTx.c index 485a125f..f775555c 100644 --- a/src/signTx.c +++ b/src/signTx.c @@ -1112,7 +1112,7 @@ 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, @@ -1120,18 +1120,26 @@ static void _setDRep( ); 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; @@ -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 ); @@ -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); @@ -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); diff --git a/src/signTx_ui.c b/src/signTx_ui.c index 9bd1a23d..2ae6f2ab 100644 --- a/src/signTx_ui.c +++ b/src/signTx_ui.c @@ -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( @@ -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( @@ -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(