Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add padded url safe base64 encoding b64_url_pad for random_id outputs #352

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

BIwashi
Copy link

@BIwashi BIwashi commented Jan 10, 2023

proposal

I added b64_url_pad for random_id outputs.

Why

When creating a signed URL for CloudCDN, I used random_id b64_url.
google_compute_backend_bucket_signed_url_key | Resources | hashicorp/google | Terraform Registry

resource "random_id" "url_signature" {
  byte_length = 16
}

resource "google_compute_backend_bucket_signed_url_key" "backend_key" {
  name           = "test-key"
  key_value      = random_id.url_signature.b64_url
  backend_bucket = google_compute_backend_bucket.test_backend.name
}

I have confirmed that this works.
However, when I created them from the Google Cloud console or gcloud commanda, the keys created were URL safe, base64 encoded, and padded with =.

Furthermore, the official Google Cloud sample code assumes padded keys, so I could not use the keys created with b64_url without modification. In addition, when I tried to create a signed URL using the gcloud compute sign-url command, an error occurred because it used a key without padding.

failed gcloud command

> gcloud compute sign-url \
  "https://example.com/test.png" \
  --key-name test-key \
  --key-file key-file \
  --expires-in 5m \
  --validate
ERROR: gcloud crashed (Error): Incorrect padding

If you would like to report this issue, please run the following command:
  gcloud feedback

To check gcloud for common problems, please run the following command:
  gcloud info --run-diagnostics

fixed google cloud sample code for golang
Use signed URLs  |  Cloud CDN  |  Google Cloud

// readKeyFile reads the base64url-encoded key file and decodes it.
func readKeyFile(path string) ([]byte, error) {
        b, err := ioutil.ReadFile(path)
        if err != nil {
                return nil, fmt.Errorf("failed to read key file: %+v", err)
        }
-       d := make([]byte, base64.URLEncoding.DecodedLen(len(b)))
-       n, err := base64.URLEncoding.Decode(d, b)
+       d := make([]byte, base64.RawURLEncoding.DecodedLen(len(b)))
+       n, err := base64.RawURLEncoding.Decode(d, b)
        if err != nil {
                return nil, fmt.Errorf("failed to base64url decode: %+v", err)
        }
        return d[:n], nil
}

Change

before

resource "random_id" "url_signature" {
  byte_length = 16
}

resource "google_compute_backend_bucket_signed_url_key" "backend_key" {
  name           = "test-key"
  key_value      = random_id.url_signature.b64_url
  backend_bucket = google_compute_backend_bucket.test_backend.name
}

after

resource "random_id" "url_signature" {
  byte_length = 16
}

resource "google_compute_backend_bucket_signed_url_key" "backend_key" {
  name           = "test-key"
  key_value      = random_id.url_signature.b64_url_pad
  backend_bucket = google_compute_backend_bucket.test_backend.name
}

Name

At first, I thought of changing the output of b64_url to have padding and creating a new b64_raw_url with no padding.
The reason is that Go code also uses base64.URLEncoding to encode those with padding and base64.RawURLEncoding for those without padding.

The reason for using b64_url_pad instead of b64_raw_url is to avoid modifying the already used b64_url.
In the long run, I think it is better to add b64_raw_url, but if you accept this PR, I would appreciate your consideration.

@BIwashi BIwashi requested a review from a team as a code owner January 10, 2023 18:12
@hashicorp-cla
Copy link

hashicorp-cla commented Jan 10, 2023

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants