drand / tlock

Timelock Encryption made practical. The Go `tlock` library and the `tle` cmd line tool home to encrypt towards the future.
Apache License 2.0
510 stars 24 forks source link

Rework the API #7

Closed AnomalRoil closed 2 years ago

AnomalRoil commented 2 years ago

So, this one is going to be a rabbit hole, but I'm not really happy with the current state of the API, bringing Network stuff around all the place.

We should try to stick to the Go way of doing stuff.

On my side I would ideally like to be able use it as follows:

    network, err := tlock.NewHttpNetwork("testnet0-api.drand.cloudflare.com", chainhash)
    if err != nil {
        panic(err)
    }

    futureday := time.Date(2023, time.February, 10, 12, 0, 0, 0, time.UTC)

    roundNumber := network.RoundFor(futureday)

    enc, err := tlock.NewEncrypter(network.Public(), roundNumber)
    if err != nil {
        panic(err)
    }

    ciphertext, err := enc.Encrypt([]byte("Test"))
    if err != nil {
        panic(err)
    }

    log.Printf("plaintext encrypted: %v\n", ciphertext.Armor())

    dec, err := tlock.NewDecrypter(network.GetBeaconAt(ciphertext.Round()))
    if err != nil {
        panic(err)
    }

    plaintext, err := dec.Decrypt(ciphertext)
    if err != nil {
        panic(err)
    }

    log.Printf("ciphertext decrypted: %s\n", string(plaintext))

Where tlock.NewEncrypter(pk PublicKey, round uint64) and tlock.NewDecrypter(beacon chain.Beacon) should also be io.Writer and io.Reader, respectively.


Another option might be to implement a new Timelock recipient type for age using their plugin system, it looks fairly easy:

We would need to:

CluEleSsUK commented 2 years ago

It might make sense to build the decrypter with the ciphertext and decrypt using the network - you have the ciphertext now but might not have the beacon until the future and might want to try multiple times e.g.

    dec, err := tlock.NewDecrypter(ciphertext))
    if err != nil {
        panic(err)
    }

    plaintext, err := dec.Decrypt(network.GetBeaconAt(cipherText.Round())

If the Round is already embedded in the ciphertext, the api could even be simplified to func Decrypt(network Network), albeit that's not as flexible

nikkolasg commented 2 years ago

Nice - while reading this, I think we can make it even more ergonomic and Go friendly, like the json encoder:

ciphertext, err := tlock.NewEncryptor(network).Encrypt([]byte("Test"), roundNumber)
fmt.Println("ciphertext: ",ciphertext.Armor(), " for round ",ciphertext.Round);
plaintext, err := tlock.NewDecryptor(network).Decrypt(ciphertext)

Main difference is letting the encryptor/decryptor use the Network for doing what it needs to do, and that we can encrypt multiple times for different round number using the same encryptor. Round number is a parameter that we need when encrypting the ciphertext, and not when we instantiate an encryptor, we don't need it at this point, I don't think it should be part of the constructor.

Note the decrypt() doesn't need the round number since it's already part of the ciphertext ?

If we want to be able to use directly a beacon retrieved from somewhere, either we implement a Network for it, or we can have tlock.DecryptFrom(publickey, beacon, ciphertext)

AnomalRoil commented 2 years ago

@CluEleSsUK My main issue with that is that I'd like it to be as agnostic of everything else as possible: what if the beacon value is provided by reading a file and not by doing a HTTP query?

But I guess something like

func NewDecrypter(network Network) (Decrypter, error)

func (*Decrypter) DecryptReader(src io.Reader) (io.Reader, error)

func (*Decrypter) Decrypt(ciphertext []byte) ([]byte, error)

would be fine too.

AnomalRoil commented 2 years ago

@nikkolasg Ahaha, you were too quick. Yeah, that sound good.

ardan-bkennedy commented 2 years ago

We found a way to keep the existing API's and handle chunks. We would like to finish this and have you review. Keeps things simple.

ardan-bkennedy commented 2 years ago

This new version now supports streaming. @AnomalRoil @nikkolasg

nikkolasg commented 2 years ago

I don't think this issue was specifically about chunks, but more about the ergonomics of the API.

func EncryptWithDuration(ctx context.Context, out io.Writer, in io.Reader, encoder Encoder, network Network, encrypter Encrypter, duration time.Duration, armor bool) error {

this function has 8 parameters, it's unclear to me if this can be considered "simple". It mixes information from the encryption step (encrypter, duration), to the "getting the beacon" step(context, network), to the encoding step (encoder, armor)). What we were proposing is to have clearer separations between each steps.

ardan-bkennedy commented 2 years ago

The API has been discovered over time by identifying what we needed to perform Encryption/Decryption.

I purposely separated the network, encoder, and encryper from each other. None of these should import the other. They each have their own independent purpose which makes them easier to implement.

From an API perspective, you just need to construct the encoder, encrypter, and network and provide it.

var encoder base.Encoder
var encrypter aead.Encrypter
network := http.NewNetwork(flags.Network, flags.Chain)

tlock.Decrypt(ctx, out, in, encoder, network, encrypter, flags.Armor)

I feel this is very Go like in that there is a separation of concern. The API is also very precise in what it needs to perform the work. Having this number of parameters isn't a smell to me.

In one API call, the user can encrypt/decrypt their data.

ardan-bkennedy commented 2 years ago

One other note. The main goal of this initial project was to build a CLI tool with the define command line API. Exposing the API we created for the CLI tool in its own package I think was a nice add. Creating an API that exposes smaller parts of the functionality so others can implement their own workflows is a great idea, but a different scope of work.

ardan-bkennedy commented 2 years ago

In an attempt to reduce the surface area of the API, I made the following changes. I also found a way to reduce the size of the Network interface as well.

Now you construct a tlock encrypter providing the network, dataEncrypter, and encoder. The Encrypt API is now just destination, source, roundNumber and if you want to armor or not. You can use the network API to convert a duration to a round number.

network := http.NewNetwork(flags.Network, flags.Chain)

tl := tlock.NewEncrypter(network, aead.DataEncrypter{}, base.Encoder{})

roundNumber, err := network.RoundNumber(ctx, time.Now().Add(time.Hour))
if err != nil {
    return fmt.Errorf("round number: %w", err)
}

err := tl.Encrypt(ctx, out, in, roundNumber, flags.Armor)

Decrypt is similar and has an even smaller surface area.

tl := tlock.NewDecrypter(network, aead.DataDecrypter{}, base.Decoder{})
tl..Decrypt(ctx, out, in, flags.Armor)

The API is still a do it all API which IMHO is great for developers like me who don't understand all the interworkings and don't have time. I just want to encrypt and decrypt in a time sensitive manner. For a long running service, a Encrypter or Decrypter can be constructed once and perfoming the encrypt/decrypt operation is more precise.

AnomalRoil commented 2 years ago

In my opinion the encoding/armoring is important for the CLI tool, but shouldn't be part of the Encrypt/Decrypt API for the library.

ardan-bkennedy commented 2 years ago

I see, so remove that functionality from the package API and move it to the CLI tool?

AnomalRoil commented 2 years ago

Please see #16 I've done it more or less.

ardan-bkennedy commented 2 years ago

Yea, this looks great!!

ardan-bkennedy commented 2 years ago

I have merged these changes into main with code cleanup.