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

Enforce key format in keyset creation #918

Merged
merged 4 commits into from
Nov 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/en/pact-functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ Return ID if called during current pact execution, failing if not.
Obtain current pact build version.
```lisp
pact> (pact-version)
"4.1"
"4.1.1"
```

Top level only: this function will fail if used in module code.
Expand Down Expand Up @@ -1754,7 +1754,7 @@ Retreive any accumulated events and optionally clear event state. Object returne
*→* `[string]`


Queries, or with arguments, sets execution config flags. Valid flags: ["AllowReadInLocal","DisableHistoryInTransactionalMode","DisableModuleInstall","DisablePact40","DisablePactEvents","OldReadOnlyBehavior","PreserveModuleIfacesBug","PreserveModuleNameBug","PreserveNsModuleInstallBug","PreserveShowDefs"]
Queries, or with arguments, sets execution config flags. Valid flags: ["AllowReadInLocal","DisableHistoryInTransactionalMode","DisableModuleInstall","DisablePact40","DisablePactEvents","EnforceKeyFormats","OldReadOnlyBehavior","PreserveModuleIfacesBug","PreserveModuleNameBug","PreserveNsModuleInstallBug","PreserveShowDefs"]
```lisp
pact> (env-exec-config ['DisableHistoryInTransactionalMode]) (env-exec-config)
["DisableHistoryInTransactionalMode"]
Expand Down
39 changes: 36 additions & 3 deletions src/Pact/Native/Keysets.hs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,18 @@
-- Builtins for working with keysets.
--

module Pact.Native.Keysets where
module Pact.Native.Keysets
( keyDefs
, readKeysetDef
)

import Control.Lens
where

import Control.Lens
import Control.Monad
import Data.Foldable
import qualified Data.ByteString.Char8 as BS
import Data.Char
import Data.Text (Text)

import Pact.Eval
Expand Down Expand Up @@ -60,7 +68,32 @@ keyDefs =


readKeySet' :: FunApp -> Text -> Eval e KeySet
readKeySet' i key = parseMsgKey i "read-keyset" key
readKeySet' i key = do
ks <- parseMsgKey i "read-keyset" key
whenExecutionFlagSet FlagEnforceKeyFormats $ enforceKeyFormats i ks
return ks

-- | A predicate for public key format validation.
type KeyFormat = PublicKey -> Bool

-- | Current "Kadena" ED-25519 key format: 64-length hex.
ed25519Hex :: KeyFormat
ed25519Hex (PublicKey k) = BS.length k == 64 && BS.all isHexDigitLower k

-- | Lower-case hex numbers.
isHexDigitLower :: Char -> Bool
isHexDigitLower c =
-- adapted from GHC.Unicode#isHexDigit
isDigit c || (fromIntegral (ord c - ord 'a')::Word) <= 5

-- | Supported key formats.
keyFormats :: [KeyFormat]
keyFormats = [ed25519Hex]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these formats be scheme-aware?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Schemes are not wallet-supported atm and with this change will need upgrades for support; they also need to be self-identifying so that e.g. an ETH sig will be smth like eth:0xadf5adf4a... at which point this mechanism will naturally support adding that.


enforceKeyFormats :: HasInfo i => i -> KeySet -> Eval e ()
enforceKeyFormats i (KeySet ks _p) = traverse_ go ks
where
go k = unless (any ($ k) keyFormats) $ evalError' i "Invalid keyset"

defineKeyset :: GasRNativeFun e
defineKeyset g0 fi as = case as of
Expand Down
2 changes: 2 additions & 0 deletions src/Pact/Types/Runtime.hs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ data ExecutionFlag
| FlagPreserveShowDefs
-- | Disable Pact 4.0 features
| FlagDisablePact40
-- | Enforce key formats. "Positive" polarity to not break legacy repl tests.
| FlagEnforceKeyFormats
deriving (Eq,Ord,Show,Enum,Bounded)

-- | Flag string representation
Expand Down
51 changes: 51 additions & 0 deletions tests/pact/keysets.repl
Original file line number Diff line number Diff line change
Expand Up @@ -203,3 +203,54 @@
"Scoped acquire of O succeeds"
"Capability acquired"
(test-capability (O)))

;;
;; keyset formats
;;

(env-exec-config ["EnforceKeyFormats"])
(env-data
{ 'bad: ['foo]
, 'short: ["12440d374865bdf0a3349634a70d1317fc279e7e13db98f2199ac5e7378975"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add one character to this short test to cover the largest possible number of failure cases?

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 PR has merged but I manually checked and that case works.

, 'long: ["12440d374865bdf0a3349634a70d1317fc279e7e13db98f2199ac5e7378975eaea"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto for shortening this long one by one character. Since we're operating in the string domain it's characters, not after-decoded bytes, that are the fundamental unit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Manually checked.

, 'badchars: ["x2440d374865bdf0a3349634a70 1317fc279e7e13db9!f2199ac5e7378975ea"]
, 'ucase: ["12440D374865BDF0A3349634A70D1317FC279E7E13DB98F2199AC5E7378975EA"]
, 'good: ["12440d374865bdf0a3349634a70d1317fc279e7e13db98f2199ac5e7378975ea"]
, 'mixed: ['foo "12440d374865bdf0a3349634a70d1317fc279e7e13db98f2199ac5e7378975ea"]
, 'good2: ["12440d374865bdf0a3349634a70d1317fc279e7e13db98f2199ac5e7378975ea"
"fdd198807260fa07b86f97a918ff7fe3542d98b9ca41a76f509e886dba3ae177"]

})
(expect-failure
"enforce kadena key format with flag: fail single"
"Invalid keyset"
(read-keyset 'bad))
(expect-failure
"enforce kadena key format with flag: fail short"
"Invalid keyset"
(read-keyset 'short))
(expect-failure
"enforce kadena key format with flag: fail long"
"Invalid keyset"
(read-keyset 'long))
(expect-failure
"enforce kadena key format with flag: fail badchars"
"Invalid keyset"
(read-keyset 'badchars))
(expect-failure
"enforce kadena key format with flag: fail uppercase"
"Invalid keyset"
(read-keyset 'ucase))

(expect-failure
"enforce kadena key format with flag: fail one bad one good"
"Invalid keyset"
(read-keyset 'mixed))
(expect-that
"enforce kadena key format with flag: success single"
(constantly true)
(read-keyset 'good))
(expect-that
"enforce kadena key format with flag: success 2"
(constantly true)
(read-keyset 'good2))