RFC: Opinionated public API for sel

[jhenahan]

• Motivation
• Executive Summary
• Example - Sel.PublicKey
  • Key generation
    • Proposed Changes
  • De/serialization
    • Proposed Changes
  • Transmission/Display
    • Proposed Changes
  • Accessors and constructors
    • Proposed Changes

Motivation

Consistent naming conventions aid discoverability and reduce surprise. Opinionated APIs can encourage consistency of practice and help prevent user error.

Executive Summary

1. Define a naming convention for common functions (e.g., serde, construction)
2. Define a consistent API for transmission/display (i.e., raw bytes, String, Text, Show, Display, etc.)
3. Define a part of speech for "accessors" (e.g., to retrieve the signature on a signed message, should we say signature or getSignature?) and how to report access failure

Example - Sel.PublicKey

Key generation

Cipher:

newKeyPair :: IO (Cipher.PublicKey, Cipher.SecretKey)

Seal: None, implication is to import PublicKey.Cipher and use those functions

Signature:

generateKeyPair :: IO (Signature.PublicKey, Signature.SecretKey)

Proposed Changes

In each module, define

{-# LANGUAGE NoFieldSelectors #-}
data KeyPair = KeyPair { public :: PublicKey, secret :: SecretKey }

-- Encourage `OverloadedRecordDot` for access, but we can also export
public = (.public)
secret = (.secret)
-- for qualified use.
--
-- This bit is substantially optional, and is mainly suggested to avoid silly
-- things like `signaturePublicKey someKeyPair` to avoid name collisions

and a constructor keyPair :: IO KeyPair.

Encourage qualified imports and OverloadedRecordDot so one writes

{-# LANGUAGE ImportQualifiedPost, OverloadedRecordDot #-}
import Sel.PublicKey.Cipher qualified as Cipher
import Sel.PublicKey.Seal qualified as Seal
import Sel.PublicKey.Signature qualified as Signature

foo :: IO ()
foo = do
  c <- Cipher.keyPair
  s <- Seal.keyPair
  sig <- Signature.keyPair
  print $ c.public
  print $ s.secret
  print $ Signature.public sig

Tuples make sense when there's no name for a thing, but keypairs definitely have a name. The author, at least, is more likely to remember someKeyPair.public before fst someKeyPair.

De/serialization

Cipher:

publicKeyToHexByteString :: Cipher.PublicKey -> StrictByteString
unsafeSecretKeyToHexByteString :: Cipher.SecretKey -> StrictByteString
keyPairFromHexByteStrings :: StrictByteString -> StrictByteString -> Either Text (Cipher.PublicKey, Cipher.SecretKey)

Seal: None, implication is to import PublicKey.Cipher and use those functions

Signature: None in current code, but #173 exports

encodePublicKeyHexByteString :: Signature.PublicKey -> StrictByteString
decodePublicKeyHexByteString :: StrictByteString -> Either KeyMaterialDecodeError Signature.PublicKey

unsafeEncodeSecretKeyHexByteString :: Signature.SecretKey -> StrictByteString
decodeSecretKeyHexByteString :: StrictByteString -> Either KeyMaterialDecodeError Signature.SecretKey

Proposed Changes

1. Define a consistent vocabulary for serde functions, and expose them uniformly. In particular, expose at least separate encoding and decoding functions for each half of a keypair. Public keys especially are shared often, so it's unusual that there's no way to recover a Cipher.PublicKey without bytes for a Cipher.SecretKey on hand.

2. Re-export the Cipher functions and types in Seal for consistency. It feels weird to import a seemingly separate module to get at "core" functions and types of the module at hand.

3. With an expressed recommendation for qualified imports, shorter names would also be agreeable, e.g., Signature.encodePublicKey, Cipher.decodeSecretKey, etc.

   1. Goofier option: Virtual record fields via HasField like hexBytes, hexString, bytes. Not an especially serious suggestion, just covering options.

4. Use a richer type for reporting errors in decoding, either specific to each module or a catch-all "all the sorts of problems we run into when decoding bytestrings" sum type. This is exposed in feat(Signature): add serde functions, hide internals #173 as KeyMaterialDecodingError, but could be renamed to simply DecodingError and cover the failure modes of, e.g.,

cipherTextFromHexByteString :: StrictByteString -> Either Text Cipher.CipherText

The identified common failure modes in Sel.PublicKey are:

• Bytes are not hex-encoded
• Byte array length mismatch (e.g., the target pointer has size n, but the byte array being decoded has length /= n); there are some more unusual situations that still fit largely into this case, such as around cryptoBoxMACBytes for cipher text, but these could have their own cases if necessary

Transmission/Display

Cipher:

Show Cipher.PublicKey
-- Display via ShowInstance
Show Cipher.SecretKey
-- Display via OpaqueInstance

-- but

Show Cipher.CipherText
-- via binary -> Base16 -> ByteString -> String
Display Cipher.CipherText
-- via binary -> Base16 -> Text

cipherTextToBinary :: CipherText -> StrictByteString
cipherTextToHexByteString :: CipherText -> StrictByteString
cipherTextToHexText :: CipherText -> Text

Seal: None, implication is to import PublicKey.Cipher and use those functions

Signature: None in current code, but #173 exports

Show Signature.PublicKey -- encodePublicKeyHexByteString, unpackChars
-- Display via ShowInstance
Show Signature.SecretKey
-- Display via OpaqueInstance

-- No functions for transmitting raw binary data

Proposed Changes

1. If transmitting raw binary data is a desirable property, expose it for all types, e.g., encodePublicKeyBinary, encodeCipherTextBinary or whatever conventional name.
   1. I'm not entirely sure if this is an expected usecase or if the cipherTextToBinary is just there for the implementation of the other two. If it's just a shared bit of code, recommend dropping it from the exports and recommending hexadecimal encoding everywhere.
2. Choose one of Show or Display to implement for each type, and reuse that definition for the other instance; suggest display for Text output rather than multiple functions.

Accessors and constructors

Cipher:

encrypt :: StrictByteString -> PublicKey -> SecretKey -> IO (Nonce, CipherText)
decrypt :: CipherText -> PublicKey -> SecretKey -> Nonce -> Maybe StrictByteString

Seal:

seal :: StrictByteString -> PublicKey -> IO CipherText
open :: CipherText -> PublicKey -> SecretKey -> Maybe StrictByteString

Signature:

signMessage :: StrictByteString -> SecretKey -> IO SignedMessage
openMessage :: SignedMessage -> PublicKey -> Maybe StrictByteString
getSignature :: SignedMessage -> StrictByteString
unsafeGetMessage :: SignedMessage -> StrictByteString
mkSignature :: StrictByteString -> StrictByteString -> SignedMessage

Proposed Changes

1. Prefer descriptive nouns over verbs for "pure" computations. E.g., decryptedMessage, verifiedMessage, signature, unverifiedMessage, signedMessage. (EDIT: Elsewhere, it was suggested that a more precise (though still informal) rule of thumb might be "use nouns when there is no rejection/failure case" or when the function "looks like" a Church encoding)
   1. The main thing I'd like to avoid is prefix noise like get and mk. We do not say getLength, why say getMessage?
   2. In the particular case of mkSignature, the name is just a bit wrong since it's actually building a SignedMessage.
   3. Verbs seem reasonable for IO actions since they "do" things.
   4. If consistency with the libsodium vocabulary is desirable, verbs make more sense. In that case, consider identifiers like seal and openSeal, though this is more complicated in Cipher since it uses crypto_box_easy and crypto_box_open_easy, which are not exactly nice to rephrase.
2. Use a more descriptive type for reporting failure for convenient tracking of status.
   1. Example: feat(Signature): add serde functions, hide internals #173 defines SignatureVerification internally so that (if added to the public API) a user could chain computations dependent on the signature being valid for the message and keep track of that information directly rather than the arguably lossy Maybe.
   2. This is dependent on how these accessors are meant to be used. I'm sympathetic to the idea that we'd rather lose specificity of failures via Maybe to allow simpler chaining across modules. That means I can chain Cipher.decrypt and Seal.open and Signature.openMessage without intentionally flattening their failure modes to Maybe as might be required with a more descriptive type.
   3. Alternatively, we could get away with some Either e with the right choice of e.