Skip to content

Commit

Permalink
additional test for negative case, test coverage for policy, remove c…
Browse files Browse the repository at this point in the history 
…omment
  • Loading branch information
@marktheunissen
marktheunissen committed Nov 21, 2024
1 parent 10b0c4c commit 913700e
Show file tree
Hide file tree
Showing 5 changed files with 671 additions and 44 deletions.
211 changes: 182 additions & 29 deletions functional_tests.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5643,9 +5643,6 @@ func testPresignedPostPolicy() {
"policy": "",
}

// Seed random based on current time.
rand.Seed(time.Now().Unix())

// Instantiate new minio client object
c, err := minio.New(os.Getenv(serverEndpoint),
&minio.Options{
Expand Down Expand Up @@ -5692,38 +5689,13 @@ func testPresignedPostPolicy() {
}

policy := minio.NewPostPolicy()

if err := policy.SetBucket(""); err == nil {
logError(testName, function, args, startTime, "", "SetBucket did not fail for invalid conditions", err)
return
}
if err := policy.SetKey(""); err == nil {
logError(testName, function, args, startTime, "", "SetKey did not fail for invalid conditions", err)
return
}
if err := policy.SetExpires(time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC)); err == nil {
logError(testName, function, args, startTime, "", "SetExpires did not fail for invalid conditions", err)
return
}
if err := policy.SetContentType(""); err == nil {
logError(testName, function, args, startTime, "", "SetContentType did not fail for invalid conditions", err)
return
}
if err := policy.SetContentLengthRange(1024*1024, 1024); err == nil {
logError(testName, function, args, startTime, "", "SetContentLengthRange did not fail for invalid conditions", err)
return
}
if err := policy.SetUserMetadata("", ""); err == nil {
logError(testName, function, args, startTime, "", "SetUserMetadata did not fail for invalid conditions", err)
return
}

policy.SetBucket(bucketName)
policy.SetKey(objectName)
policy.SetExpires(time.Now().UTC().AddDate(0, 0, 10)) // expires in 10 days
policy.SetContentType("binary/octet-stream")
policy.SetContentLengthRange(10, 1024*1024)
policy.SetUserMetadata(metadataKey, metadataValue)
policy.SetContentEncoding("gzip")

// Add CRC32C
checksum := minio.ChecksumCRC32C.ChecksumBytes(buf)
Expand Down Expand Up @@ -5865,6 +5837,186 @@ func testPresignedPostPolicy() {
logSuccess(testName, function, args, startTime)
}

// testPresignedPostPolicyWrongFile tests that when we have a policy with a checksum, we cannot POST the wrong file
func testPresignedPostPolicyWrongFile() {
// initialize logging params
startTime := time.Now()
testName := getFuncName()
function := "PresignedPostPolicy(policy)"
args := map[string]interface{}{
"policy": "",
}

// Instantiate new minio client object
c, err := minio.New(os.Getenv(serverEndpoint),
&minio.Options{
Creds: credentials.NewStaticV4(os.Getenv(accessKey), os.Getenv(secretKey), ""),
Transport: createHTTPTransport(),
Secure: mustParseBool(os.Getenv(enableHTTPS)),
})
if err != nil {
logError(testName, function, args, startTime, "", "MinIO client object creation failed", err)
return
}

// Enable tracing, write to stderr.
// c.TraceOn(os.Stderr)

// Set user agent.
c.SetAppInfo("MinIO-go-FunctionalTest", appVersion)

// Generate a new random bucket name.
bucketName := randString(60, rand.NewSource(time.Now().UnixNano()), "minio-go-test-")

// Make a new bucket in 'us-east-1' (source bucket).
err = c.MakeBucket(context.Background(), bucketName, minio.MakeBucketOptions{Region: "us-east-1"})
if err != nil {
logError(testName, function, args, startTime, "", "MakeBucket failed", err)
return
}

defer cleanupBucket(bucketName, c)

// Generate 33K of data.
reader := getDataReader("datafile-33-kB")
defer reader.Close()

objectName := randString(60, rand.NewSource(time.Now().UnixNano()), "")
// Azure requires the key to not start with a number
metadataKey := randString(60, rand.NewSource(time.Now().UnixNano()), "user")
metadataValue := randString(60, rand.NewSource(time.Now().UnixNano()), "")

buf, err := io.ReadAll(reader)
if err != nil {
logError(testName, function, args, startTime, "", "ReadAll failed", err)
return
}

policy := minio.NewPostPolicy()
policy.SetBucket(bucketName)
policy.SetKey(objectName)
policy.SetExpires(time.Now().UTC().AddDate(0, 0, 10)) // expires in 10 days
policy.SetContentType("binary/octet-stream")
policy.SetContentLengthRange(10, 1024*1024)
policy.SetUserMetadata(metadataKey, metadataValue)

// Add CRC32C of the 33kB file that the policy will explicitly allow.
checksum := minio.ChecksumCRC32C.ChecksumBytes(buf)
err = policy.SetChecksum(checksum)
if err != nil {
logError(testName, function, args, startTime, "", "SetChecksum failed", err)
return
}

args["policy"] = policy.String()

presignedPostPolicyURL, formData, err := c.PresignedPostPolicy(context.Background(), policy)
if err != nil {
logError(testName, function, args, startTime, "", "PresignedPostPolicy failed", err)
return
}

// At this stage, we have a policy that allows us to upload datafile-33-kB.
// Test that uploading datafile-10-kB, with a different checksum, fails as expected
filePath := getMintDataDirFilePath("datafile-10-kB")
if filePath == "" {
// Make a temp file with 10 KB data.
file, err := os.CreateTemp(os.TempDir(), "PresignedPostPolicyTest")
if err != nil {
logError(testName, function, args, startTime, "", "TempFile creation failed", err)
return
}
if _, err = io.Copy(file, getDataReader("datafile-10-kB")); err != nil {
logError(testName, function, args, startTime, "", "Copy failed", err)
return
}
if err = file.Close(); err != nil {
logError(testName, function, args, startTime, "", "File Close failed", err)
return
}
filePath = file.Name()
}
fileReader := getDataReader("datafile-10-kB")
defer fileReader.Close()
buf10k, err := io.ReadAll(fileReader)
if err != nil {
logError(testName, function, args, startTime, "", "ReadAll failed", err)
return
}
otherChecksum := minio.ChecksumCRC32C.ChecksumBytes(buf10k)

var formBuf bytes.Buffer
writer := multipart.NewWriter(&formBuf)
for k, v := range formData {
if k == "x-amz-checksum-crc32c" {
v = otherChecksum.Encoded()
}
writer.WriteField(k, v)
}

// Add file to post request
f, err := os.Open(filePath)
defer f.Close()
if err != nil {
logError(testName, function, args, startTime, "", "File open failed", err)
return
}
w, err := writer.CreateFormFile("file", filePath)
if err != nil {
logError(testName, function, args, startTime, "", "CreateFormFile failed", err)
return
}
_, err = io.Copy(w, f)
if err != nil {
logError(testName, function, args, startTime, "", "Copy failed", err)
return
}
writer.Close()

httpClient := &http.Client{
Timeout: 30 * time.Second,
Transport: createHTTPTransport(),
}
args["url"] = presignedPostPolicyURL.String()

req, err := http.NewRequest(http.MethodPost, presignedPostPolicyURL.String(), bytes.NewReader(formBuf.Bytes()))
if err != nil {
logError(testName, function, args, startTime, "", "HTTP request failed", err)
return
}

req.Header.Set("Content-Type", writer.FormDataContentType())

// Make the POST request with the form data.
res, err := httpClient.Do(req)
if err != nil {
logError(testName, function, args, startTime, "", "HTTP request failed", err)
return
}
defer res.Body.Close()
if res.StatusCode != http.StatusForbidden {
logError(testName, function, args, startTime, "", "HTTP request unexpected status", errors.New(res.Status))
return
}

// Read the response body, ensure it has checksum failure message
resBody, err := io.ReadAll(res.Body)
if err != nil {
logError(testName, function, args, startTime, "", "ReadAll failed", err)
return
}

// Normalize the response body, because S3 uses quotes around the policy condition components
// in the error message, MinIO does not.
resBodyStr := strings.ReplaceAll(string(resBody), `"`, "")
if !strings.Contains(resBodyStr, "Policy Condition failed: [eq, $x-amz-checksum-crc32c, aHnJMw==]") {
logError(testName, function, args, startTime, "", "Unexpected response body", errors.New(resBodyStr))
return
}

logSuccess(testName, function, args, startTime)
}

// Tests copy object
func testCopyObject() {
// initialize logging params
Expand Down Expand Up @@ -14977,6 +15129,7 @@ func main() {
testGetObjectReadAtFunctional()
testGetObjectReadAtWhenEOFWasReached()
testPresignedPostPolicy()
testPresignedPostPolicyWrongFile()
testCopyObject()
testComposeObjectErrorCases()
testCompose10KSources()
Expand Down
41 changes: 28 additions & 13 deletions post-policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (p *PostPolicy) SetExpires(t time.Time) error {

// SetKey - Sets an object name for the policy based upload.
func (p *PostPolicy) SetKey(key string) error {
if strings.TrimSpace(key) == "" || key == "" {
if strings.TrimSpace(key) == "" {
return errInvalidArgument("Object name is empty.")
}
policyCond := policyCondition{
Expand Down Expand Up @@ -118,7 +118,7 @@ func (p *PostPolicy) SetKeyStartsWith(keyStartsWith string) error {

// SetBucket - Sets bucket at which objects will be uploaded to.
func (p *PostPolicy) SetBucket(bucketName string) error {
if strings.TrimSpace(bucketName) == "" || bucketName == "" {
if strings.TrimSpace(bucketName) == "" {
return errInvalidArgument("Bucket name is empty.")
}
policyCond := policyCondition{
Expand All @@ -135,7 +135,7 @@ func (p *PostPolicy) SetBucket(bucketName string) error {

// SetCondition - Sets condition for credentials, date and algorithm
func (p *PostPolicy) SetCondition(matchType, condition, value string) error {
if strings.TrimSpace(value) == "" || value == "" {
if strings.TrimSpace(value) == "" {
return errInvalidArgument("No value specified for condition")
}

Expand All @@ -156,7 +156,7 @@ func (p *PostPolicy) SetCondition(matchType, condition, value string) error {

// SetTagging - Sets tagging for the object for this policy based upload.
func (p *PostPolicy) SetTagging(tagging string) error {
if strings.TrimSpace(tagging) == "" || tagging == "" {
if strings.TrimSpace(tagging) == "" {
return errInvalidArgument("No tagging specified.")
}
_, err := tags.ParseObjectXML(strings.NewReader(tagging))
Expand All @@ -178,7 +178,7 @@ func (p *PostPolicy) SetTagging(tagging string) error {
// SetContentType - Sets content-type of the object for this policy
// based upload.
func (p *PostPolicy) SetContentType(contentType string) error {
if strings.TrimSpace(contentType) == "" || contentType == "" {
if strings.TrimSpace(contentType) == "" {
return errInvalidArgument("No content type specified.")
}
policyCond := policyCondition{
Expand Down Expand Up @@ -211,7 +211,7 @@ func (p *PostPolicy) SetContentTypeStartsWith(contentTypeStartsWith string) erro

// SetContentDisposition - Sets content-disposition of the object for this policy
func (p *PostPolicy) SetContentDisposition(contentDisposition string) error {
if strings.TrimSpace(contentDisposition) == "" || contentDisposition == "" {
if strings.TrimSpace(contentDisposition) == "" {
return errInvalidArgument("No content disposition specified.")
}
policyCond := policyCondition{
Expand All @@ -226,6 +226,23 @@ func (p *PostPolicy) SetContentDisposition(contentDisposition string) error {
return nil
}

// SetContentEncoding - Sets content-encoding of the object for this policy
func (p *PostPolicy) SetContentEncoding(contentEncoding string) error {
if strings.TrimSpace(contentEncoding) == "" {
return errInvalidArgument("No content encoding specified.")
}
policyCond := policyCondition{
matchType: "eq",
condition: "$Content-Encoding",
value: contentEncoding,
}
if err := p.addNewPolicy(policyCond); err != nil {
return err
}
p.formData["Content-Encoding"] = contentEncoding
return nil
}

// SetContentLengthRange - Set new min and max content length
// condition for all incoming uploads.
func (p *PostPolicy) SetContentLengthRange(minLen, maxLen int64) error {
Expand All @@ -246,7 +263,7 @@ func (p *PostPolicy) SetContentLengthRange(minLen, maxLen int64) error {
// SetSuccessActionRedirect - Sets the redirect success url of the object for this policy
// based upload.
func (p *PostPolicy) SetSuccessActionRedirect(redirect string) error {
if strings.TrimSpace(redirect) == "" || redirect == "" {
if strings.TrimSpace(redirect) == "" {
return errInvalidArgument("Redirect is empty")
}
policyCond := policyCondition{
Expand All @@ -264,7 +281,7 @@ func (p *PostPolicy) SetSuccessActionRedirect(redirect string) error {
// SetSuccessStatusAction - Sets the status success code of the object for this policy
// based upload.
func (p *PostPolicy) SetSuccessStatusAction(status string) error {
if strings.TrimSpace(status) == "" || status == "" {
if strings.TrimSpace(status) == "" {
return errInvalidArgument("Status is empty")
}
policyCond := policyCondition{
Expand All @@ -282,10 +299,10 @@ func (p *PostPolicy) SetSuccessStatusAction(status string) error {
// SetUserMetadata - Set user metadata as a key/value couple.
// Can be retrieved through a HEAD request or an event.
func (p *PostPolicy) SetUserMetadata(key, value string) error {
if strings.TrimSpace(key) == "" || key == "" {
if strings.TrimSpace(key) == "" {
return errInvalidArgument("Key is empty")
}
if strings.TrimSpace(value) == "" || value == "" {
if strings.TrimSpace(value) == "" {
return errInvalidArgument("Value is empty")
}
headerName := fmt.Sprintf("x-amz-meta-%s", key)
Expand All @@ -304,7 +321,7 @@ func (p *PostPolicy) SetUserMetadata(key, value string) error {
// SetUserMetadataStartsWith - Set how an user metadata should starts with.
// Can be retrieved through a HEAD request or an event.
func (p *PostPolicy) SetUserMetadataStartsWith(key, value string) error {
if strings.TrimSpace(key) == "" || key == "" {
if strings.TrimSpace(key) == "" {
return errInvalidArgument("Key is empty")
}
headerName := fmt.Sprintf("x-amz-meta-%s", key)
Expand All @@ -326,8 +343,6 @@ func (p *PostPolicy) SetChecksum(c Checksum) error {
p.formData[amzChecksumAlgo] = c.Type.String()
p.formData[c.Type.Key()] = c.Encoded()

// Needed for S3 compatibility. MinIO ignores the checksum keys in the policy.
// https://github.com/minio/minio/blob/RELEASE.2024-08-29T01-40-52Z/cmd/postpolicyform.go#L60-L65
policyCond := policyCondition{
matchType: "eq",
condition: fmt.Sprintf("$%s", amzChecksumAlgo),
Expand Down
Loading

0 comments on commit 913700e

Please sign in to comment.