Fix stream SSE uploads with S3 encrypt type

[jespino]

When you try to upload files stream based with encryption activated, minio-go
try to upload the parts using the ServerSideEncryption header, but it isn't
supported for uploadPart with encrypt type S3, only is needed for customer
keys. For S3 encryption we simply add the header at the creation, and in the
following uploadPart we don't pass the header.

I'm not sure, but maybe we need to exclude the encrypt KMS type too (I'm not
sure).

Here is an example of it failing before the patch, and working after it.

package main

import (
	"log"
	"os"

	minio "github.com/minio/minio-go"
	"github.com/minio/minio-go/pkg/encrypt"
)

func main() {
	s3Client, err := minio.New("s3.amazonaws.com", "API_KEY", "API_SECRET", true)
	if err != nil {
		log.Fatalln(err)
	}

	object, err := os.Open("test.txt")
	if err != nil {
		log.Fatalln(err)
	}
	defer object.Close()

	options := minio.PutObjectOptions{ServerSideEncryption: encrypt.NewSSE()}

	n, err := s3Client.PutObject("my-bucket", "my-objectname", object, -1, options)
	if err != nil {
		log.Fatalln(err)
	}

	log.Println("Uploaded", "my-objectname", " of size: ", n, "Successfully.")
}

[aead]

@jespino I think you're also right about KMS - but we should check to be sure...
Thanks for the PR 😄

[jespino]

You are welcome, I'm implementing the encrypt.NewSSEKMS to try it ;) If I realize that is needed for KMS too, I will send another PR, if you want :)

[aead]

That would be awesome :) @jespino

[houndci-bot]

func parameter keyId should be keyID

[houndci-bot]

func parameter keyId should be keyID

[houndci-bot]

const sseKmsKeyId should be sseKmsKeyID

[jespino]

I have created what I think is needed, but I don't know too much about KMS, so I don't know if the context as an optional string is enough.

[aead]

Since::

The encryption context can be any value that you want, provided that the header adheres to the Base64-encoded JSON format.

I think we can follow this approach:

NewSSEKMS(keyID string, context interface{}) (ServerSide, error) {
      // if context is nil -> no context
      // if context implements json.Marshaler -> marshal
      // json.Marshal(context) 
}

[jespino]

I like this approach and then we don't need the contextPresent proposed in the comment below. I'll do this changes tomorrow.

[aead]

Maybe we don't need two functions right now - removing a function breaks API while we can always add if requested.

[jespino]

Ok, perfect, I'll leave one only.

[aead]

Can we use a bool var - like contextPresent instead of a *string and nil checks?

[houndci-bot]

if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary)

[aead]

LGTM