Skip to content

Fix: encryptProjectConfig now supports AES-GCM#52

Merged
logwolvy merged 1 commit intomainfrom
encryption-update
Jul 29, 2025
Merged

Fix: encryptProjectConfig now supports AES-GCM#52
logwolvy merged 1 commit intomainfrom
encryption-update

Conversation

@jkosanam
Copy link
Collaborator

@jkosanam jkosanam commented Jul 22, 2025

Summary

This PR is responsible for updating the encryption algorithm from AES-CBC to AES-GCM to mitigate a security issue.

JIRA ticket: https://do-internal.atlassian.net/browse/SERVERLESS-3237
VULN: https://do-internal.atlassian.net/browse/VULN-2603
Solution proposal doc: https://docs.google.com/document/d/1viBJyKt7FeCEFggydmqfEHJQ6VYNcIGUEeY4ud5mog4/edit?tab=t.0

Testing

  • Added unit tests.
  • Verified the deploy, watch and get-metadata commands.
  • Tested the above commands using all 4 runtimes i.e go, python, php and node.

@jkosanam jkosanam requested a review from sanpj2292 July 22, 2025 03:41
@jkosanam jkosanam self-assigned this Jul 22, 2025
Comment on lines +176 to +180
const decipher = crypto.createDecipheriv(
'aes-256-gcm',
key,
Buffer.from(ivHex, 'hex')
);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semgrep identified an issue in your code:
The call to 'createDecipheriv' with the Galois Counter Mode (GCM) mode of operation is missing an expected authentication tag length. If the expected authentication tag length is not specified or otherwise checked, the application might be tricked into verifying a shorter-than-expected authentication tag. This can be abused by an attacker to spoof ciphertexts or recover the implicit authentication key of GCM, allowing arbitrary forgeries.

To resolve this comment:

✨ Commit Assistant fix suggestion

Suggested change
const decipher = crypto.createDecipheriv(
'aes-256-gcm',
key,
Buffer.from(ivHex, 'hex')
);
const decipher = crypto.createDecipheriv(
'aes-256-gcm',
key,
Buffer.from(ivHex, 'hex'),
{ authTagLength: 16 } // Specify the expected authentication tag length in bytes
);
View step-by-step instructions
  1. Pass an additional options object with the authTagLength property set to 16 as the fourth parameter to crypto.createDecipheriv. For example:
    const decipher = crypto.createDecipheriv('aes-256-gcm', key, Buffer.from(ivHex, 'hex'), { authTagLength: 16 });
  2. Make sure that the value for authTagLength matches the length of the authentication tag you use when calling decipher.setAuthTag.
    In AES-GCM, a typical tag length is 16 bytes (128 bits). This helps prevent attackers from supplying shorter authentication tags when decrypting.
  3. Review and update any other instances of createDecipheriv with a GCM mode in your codebase to use the authTagLength option as well.
💬 Ignore this finding

Reply with Semgrep commands to ignore this finding.

  • /fp <comment> for false positive
  • /ar <comment> for acceptable risk
  • /other <comment> for all other reasons

Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by gcm-no-tag-length.

Questions about this issue? Reach out to Product Security in #prodsec-tools.

You can view more details about this finding in the Semgrep AppSec Platform.

@jkosanam jkosanam marked this pull request as ready for review July 22, 2025 11:37
Copy link
Collaborator

@sanpj2292 sanpj2292 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@logwolvy logwolvy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. We need to get e2e test passing before merging this.

@logwolvy logwolvy force-pushed the encryption-update branch from 415745d to 1096162 Compare July 29, 2025 10:44
@logwolvy logwolvy merged commit 21a95f4 into main Jul 29, 2025
12 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants