bytes.Reader already providers a ReaderAt interface, and Size on a byte slice, so this seems unnecessary.

Dropped this patch now.

Also, I 'downgraded' the parameters to the encryption function to not take a content.ReaderAt but a io.ReaderAt instead.

In a tree that has all the image encryption code I tried to then convert the io.ReaderAt to a plain io.Reader but that doesn't fit with the function for reading (layer blobs) from the content.Store since this one returns a ReaderAt interface via the content.Provider interface. I suppose we can leave it like this unless we somehow wrap that content.ReaderAt with some sort of helper that makes it looks like io.Reader interface.

It seems likely that x/crypto/openpgp will be deprecated, see golang/go#30141 so really not sure about using it.

I think they reverted on that decision 3 days ago :)

golang/go#30141 (comment)

Why is the nonce base64 encoded at this point? It seems weird to me to make CipherOptions a string map and put the decoding everywhere it is used, rather than putting the keys in explicitly, in which case Go will convert byte types from base64 automatically.

CipherOptions is a map[string]string but the Nonce is a []byte. It looks like that map only carries the Nonce at the moment, so it could be converted to map[string][]byte and we probably wouldn't have to do the conversion then...

I converted the map to map[string][]byte, which resolve the issue: d647440

The size should not be hard coded. Miscreant provides a GenerateKey function https://godoc.org/github.com/miscreant/miscreant-go#GenerateKey

ah again so don't use generateKey but the size should be got from somewhere, as it is dependent on the encryption function.

Added a patch on top of this series that resolves the issue.

Miscreatnt provides a GenerateNonce function https://godoc.org/github.com/miscreant/miscreant-go#GenerateNonce

Hm, we are using NewStreamEncryptor which returns us

type StreamEncryptor struct {
	// cipher.AEAD instance underlying this STREAM
	a cipher.AEAD

	// Nonce encoder instance which computes per-message nonces
	n *nonceEncoder32
}

https://github.com/miscreant/miscreant.go/blob/master/stream.go#L25

I am missing a function that gives me access to 'a'.

... might as well raise an issue with the miscreant project

miscreant/miscreant.go#4

Ah sorry I was misreading the API docs.

So we leave it as it is now ?

typo "invocation"

Fixed.

this comment is too pgp specific

Golang JSON already marshalls and unmarshals bytes to base64 encoded strings, so this should not be necessary at all.

I kept the two functions but simplified them: c05cb18

rand.Read uses io.ReadFull itself see https://golang.org/pkg/crypto/rand/#Read so you just need to use that.

Calling miscreant's GenerateKey() now, which is good too since it checks for proper size...

stefanberger@9f28224

any reason this one function shouldn't just go into your utils package (next file)? or conversely any reason the utility functions in the separate encryption/utils pkg shouldn't just end up in this file?

The function Uint64ToStringArray in this file only has one user in images/encryption/gpg.go and could either move in there or we can merge it into images/encryption/utils/utils.go where there are more crypto related functions, though. Either way is fine by me. If you don't have a preference I'd probably move it into gpg.go.