putObject request without Content-MD5 header on bucket with object lock should fail

[tPl0ch]

Expected Behavior

From the AWS putObject API documentation:

The Content-MD5 header is required for any request to upload an object with a retention period configured using Amazon S3 Object Lock. For more information about Amazon S3 Object Lock, see Amazon S3 Object Lock Overview in the Amazon S3 User Guide.

minio should fail when no Content-MD5 OR x-amz-checksum- HTTP header is present in the request

Content-MD5 OR x-amz-checksum- HTTP header is required for Put Object requests with Object Lock parameters (Service: S3, Status Code: 400, Request ID: XXX, Extended Request ID: XXX)

Current Behavior

No error is raised. See logs in steps to reproduce.

Possible Solution

Raise an error with status code 400 and given error message.

Steps to Reproduce (for bugs)

1. Set-up bucket with versioning and object lock

mc mb -p my-minio/bucket-locked --with-lock;
mc version enable my-minio/bucket-locked;
mc retention set --default GOVERNANCE "30d" my-minio/bucket-locked;

2. Send putObject request without Content-MD5

127.0.0.1:9000 [REQUEST s3.PutObject] [2023-01-25T19:37:35.014] [Client IP: 172.28.0.1]
2023-01-25T19:37:35.253128312Z 127.0.0.1:9000 PUT /bucket-locked/wXZz1YHBMK
2023-01-25T19:37:35.253131454Z 127.0.0.1:9000 Proto: HTTP/1.1
2023-01-25T19:37:35.253133764Z 127.0.0.1:9000 Host: 127.0.0.1:9000
2023-01-25T19:37:35.253136183Z 127.0.0.1:9000 Expect: 100-continue
2023-01-25T19:37:35.253138510Z 127.0.0.1:9000 X-Amz-Acl: private
2023-01-25T19:37:35.253141070Z 127.0.0.1:9000 X-Amz-Content-Sha256: UNSIGNED-PAYLOAD
2023-01-25T19:37:35.253143255Z 127.0.0.1:9000 X-Amz-Date: 20230125T193735Z
2023-01-25T19:37:35.253145891Z 127.0.0.1:9000 Authorization: AWS4-HMAC-SHA256 Credential=TESTKEY/20230125/ca-central-1/s3/aws4_request, SignedHeaders=amz-sdk-invocation-id;amz-sdk-request;content-length;content-type;host;x-amz-acl;x-amz-content-sha256;x-amz-date, Signature=46fc14513cb4f4e90516fe0a612f8f2e48e73b6d7ad2500605185dc4637ab285
2023-01-25T19:37:35.253149020Z 127.0.0.1:9000 Content-Type: application/octet-stream
2023-01-25T19:37:35.253151736Z 127.0.0.1:9000 Content-Length: 193985
2023-01-25T19:37:35.253154308Z 127.0.0.1:9000 User-Agent: aws-sdk-java/2.16.61 Linux/5.15.0-58-generic OpenJDK_64-Bit_Server_VM/17.0.2+8-86 Java/17.0.2 scala/2.13.8 vendor/Oracle_Corporation io/async http/NettyNio cfg/retry-mode/legacy
2023-01-25T19:37:35.253157244Z 127.0.0.1:9000 Amz-Sdk-Invocation-Id: af6aafda-a4dc-e107-3174-9e66843101bc
2023-01-25T19:37:35.253159744Z 127.0.0.1:9000 Amz-Sdk-Request: attempt=1; max=3
2023-01-25T19:37:35.253162453Z 127.0.0.1:9000 <BLOB>
2023-01-25T19:37:35.253164998Z 127.0.0.1:9000 [RESPONSE] [2023-01-25T19:37:35.252] [ Duration 237.751ms  ↑ 190 KiB  ↓ 0 B ]
2023-01-25T19:37:35.253167863Z 127.0.0.1:9000 200 OK
2023-01-25T19:37:35.253170071Z 127.0.0.1:9000 Strict-Transport-Security: max-age=31536000; includeSubDomains
2023-01-25T19:37:35.253172314Z 127.0.0.1:9000 Vary: Origin,Accept-Encoding
2023-01-25T19:37:35.253174422Z 127.0.0.1:9000 X-Amz-Request-Id: 173DA4741A3A057A
2023-01-25T19:37:35.253176668Z 127.0.0.1:9000 X-Content-Type-Options: nosniff
2023-01-25T19:37:35.253178985Z 127.0.0.1:9000 X-Xss-Protection: 1; mode=block
2023-01-25T19:37:35.253181269Z 127.0.0.1:9000 Accept-Ranges: bytes
2023-01-25T19:37:35.253183580Z 127.0.0.1:9000 Content-Security-Policy: block-all-mixed-content
2023-01-25T19:37:35.253185864Z 127.0.0.1:9000 Server: MinIO
2023-01-25T19:37:35.253188132Z 127.0.0.1:9000 x-amz-version-id: 1f94375b-f43d-403b-9c19-b27ae02c1e34
2023-01-25T19:37:35.253190454Z 127.0.0.1:9000 Content-Length: 0
2023-01-25T19:37:35.253192585Z 127.0.0.1:9000 ETag: "3b1e073fc4c50e8b759fda098a7df138"
2023-01-25T19:37:35.253200249Z 127.0.0.1:9000 <BLOB>

Context

I am trying to use minio as a backend for integration testing an S3 Scala library that should behave like the real thing. I understand that this is neither a primary nor revenue generating use-case. We came across a bug that slipped through due to this misaligned behavior.

Regression

Not sure TBH...

Your Environment

version: "3.7"

services:
  minio:
    image: quay.io/minio/minio
    ports:
      - "9000:9000"
      - "9001:9001"
    volumes:
      - ./minio/data:/data
    environment:
      - MINIO_ROOT_USER=TESTKEY
      - MINIO_ROOT_PASSWORD=TESTSECRET
    healthcheck:
      test: ["CMD", "curl", "-f", "http://localhost:9000/minio/health/live"]
      interval: 30s
      timeout: 20s
      retries: 3
    command: server /data --console-address ":9001"

  mc:
    image: quay.io/minio/mc
    volumes:
      - ./minio/export:/export
      - ./minio/export-locked:/export-locked
    depends_on:
      - minio
    environment:
      - MINIO_ROOT_USER=TESTKEY
      - MINIO_ROOT_PASSWORD=TESTSECRET
    entrypoint: >
      /bin/sh -c "
      echo Waiting for minio service to start...;
      curl --retry 10 --retry-delay 10 -s -o /dev/null http://minio:9000/minio/health/live

      echo Minio is started;
      /usr/bin/mc config host add my-minio http://minio:9000 $${MINIO_ROOT_USER} $${MINIO_ROOT_PASSWORD};
      /usr/bin/mc mb -p my-minio/bucket-1;
      /usr/bin/mc mb -p my-minio/bucket-locked --with-lock;
      /usr/bin/mc version enable my-minio/bucket-locked;
      /usr/bin/mc retention set --default GOVERNANCE "30d" my-minio/bucket-locked;
      /usr/bin/mc mirror export/ my-minio/bucket-1;
      /usr/bin/mc mirror export-locked/ my-minio/bucket-locked;
      /usr/bin/mc admin trace --verbose my-minio/bucket-locked;
      "

[harshavardhana]

which version of mc is this? and also the server version? @tPl0ch

[tPl0ch]

@harshavardhana

minio

minio version RELEASE.2023-01-12T02-06-16Z (commit-id=7bc95c47a322971aff7d4d4c270dcf28a933e84b)
2023-01-25T19:56:16.452536917Z Runtime: go1.19.4 linux/amd64

mc

mc version RELEASE.2023-01-11T03-14-16Z (commit-id=14c2e506fa78b53fb6db88bcf87d8f6d3fb6989e)

[harshavardhana]

Okay I see now we had this on purpose since it didn't make sense to force clients to add ContentMd5 @tPl0ch

md5 is slow and CPU heavy we really don't want client resources to be spent on this - so it was relaxed.

how does it help you for us to return an error? is it just compatibility for compatibility's sake?

[tPl0ch]

As I mentioned before, we use minio as a local and ci development tool for our test suite to point our live clients at:

I am trying to use minio as a backend for integration testing an S3 Scala library that should behave like the real thing. I understand that this is neither a primary nor revenue generating use-case. We came across a bug that slipped through due to this misaligned behavior.

I tried to add a regression test for this use case and it was not possible with minio. I could work around that by using a failing stub instead of the integration client itself, but I thought it was something worth reporting.

Okay I see now we had this on purpose since it didn't make sense to force clients to add ContentMd5

This MD5 check would only be required when object locking with retention is enabled on the bucket though, which might mean that most of the clients remain unaffected by that change. But yes, in theory it's a BC breaking change that can affect existing clients that depend on the current behavior. @harshavardhana

[klauspost]

What a random feature. Now that we have content checksums we can enforce this without too major of a downside, since they are much faster than MD5.

The clients will already add CRC when it can and with MD5 disabled.

My main regression concern would be streaming uploads that are sent without any checksums on TLS. We would need streaming checksums for this.

If TLS | x-amz-checksum- | md5 | sha256 signed v4 then we accept the upload. This means we only reject plain HTTP without any kind of checksum and UNSIGNED-PAYLOAD.