Skip to content

feat(pki-118): add spiffe machine auth#5610

Open
PrestigePvP wants to merge 3 commits intoInfisical:mainfrom
PrestigePvP:tre/PKI-118-add-spiffe-machine-auth
Open

feat(pki-118): add spiffe machine auth#5610
PrestigePvP wants to merge 3 commits intoInfisical:mainfrom
PrestigePvP:tre/PKI-118-add-spiffe-machine-auth

Conversation

@PrestigePvP
Copy link
Contributor

Context

  • Adds SPIFFE machine auth mechanism
  • Ability for remote bundle usage
  • Ability to use static JWKs for verifcation

Screenshots

TODO

Steps to verify the change

Type

  • Fix
  • Feature
  • Improvement
  • Breaking
  • Docs
  • Chore

Checklist

  • Title follows the conventional commit format: type(scope): short description (scope is optional, e.g., fix: prevent crash on sync or fix(api): handle null response).
  • Tested locally
  • Updated docs (if needed)
  • Read the contributing guide

@maidul98
Copy link
Collaborator

maidul98 commented Mar 6, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 6, 2026

Greptile Summary

This PR introduces SPIFFE machine authentication with a complete service, router, schema, and frontend components. However, it has critical blockers that prevent it from functioning:

Critical Issues:

  • Empty database migration: The 20260305201413_add-spiffe-machine-auth.ts migration has no implementation. The identity_spiffe_auths table will never be created, causing all SPIFFE auth operations to fail at runtime.

Security Issues:

  • DNS rebinding SSRF: blockLocalAndPrivateIpAddresses() validates the hostname once (line 190), but fetchRemoteBundleJwks() performs a separate DNS resolution (line 197). An attacker controlling DNS can exploit this time-of-check-time-of-use window to redirect the HTTPS request to internal services (e.g., AWS metadata, internal cluster services).

  • Algorithm confusion vulnerability: The JWT alg header value (attacker-controlled) is passed directly to importJWK() (line 86) without an allowlist. This enables algorithm-substitution attacks (e.g., "none" algorithm).

  • Unsafe KeyLike to jwt.Secret cast: importJWK() returns KeyLike | Uint8Array, but line 89 casts it unsafely to jwt.Secret. Depending on the version and whether a CryptoKey or KeyObject is returned, this can fail at runtime.

  • Native regex violations (2 locations): Both identity-spiffe-auth-fns.ts and identity-spiffe-auth-validators.ts use native JavaScript regex instead of the re2 package, violating project security standards.

These must be resolved before merging.

Confidence Score: 0/5

  • Not safe to merge — critical blocker (empty migration) and multiple high-severity security issues (SSRF, algorithm confusion, unsafe casts, native regex).
  • The empty migration is a hard blocker that breaks the entire feature at runtime. Additionally, there are five security concerns: DNS rebinding SSRF, algorithm-confusion attacks, unsafe type casting that can cause runtime failures, and native regex that bypasses ReDoS protection. These must be resolved before shipping.
  • All files in this feature require attention: backend/src/db/migrations/20260305201413_add-spiffe-machine-auth.ts (empty — critical), backend/src/services/identity-spiffe-auth/identity-spiffe-auth-service.ts (JWT verification, SSRF window), backend/src/services/identity-spiffe-auth/identity-spiffe-auth-fns.ts (native regex), backend/src/services/identity-spiffe-auth/identity-spiffe-auth-validators.ts (native regex).

Last reviewed commit: 96245d2

Comment on lines +5 to +17
const SPIFFE_ID_REGEX = /^spiffe:\/\/([^/]+)(\/.*)?$/;

export const isValidSpiffeId = (value: string): boolean => {
return SPIFFE_ID_REGEX.test(value);
};

export const extractTrustDomainFromSpiffeId = (spiffeId: string): string => {
const match = spiffeId.match(SPIFFE_ID_REGEX);
if (!match) {
throw new Error(`Invalid SPIFFE ID: ${spiffeId}`);
}
return match[1];
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Security: Use re2 package instead of native regex

Per project security rules, all regex must use the re2 package to prevent ReDoS. The SPIFFE_ID_REGEX is a native JavaScript regex used at lines 8 and 12.

Replace with:

import RE2 from "re2";

const SPIFFE_ID_REGEX = new RE2(String.raw`^spiffe://([^/]+)(/.*)?$`);

export const isValidSpiffeId = (value: string): boolean => {
  return SPIFFE_ID_REGEX.test(value);
};

export const extractTrustDomainFromSpiffeId = (spiffeId: string): string => {
  const match = SPIFFE_ID_REGEX.exec(spiffeId);
  if (!match) throw new Error(`Invalid SPIFFE ID: ${spiffeId}`);
  return match[1];
};

Comment on lines +27 to +31
export const validateTrustDomain = z
.string()
.trim()
.min(1, "Trust domain is required")
.regex(/^[a-zA-Z0-9]([a-zA-Z0-9.-]*[a-zA-Z0-9])?$/, "Invalid trust domain format");
Copy link
Contributor

Choose a reason for hiding this comment

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

Security: Use re2 package instead of native regex

Per project security rules, all regex must use the re2 package. The .regex() call at line 31 uses a native JavaScript regex.

Replace with:

import RE2 from "re2";

const TRUST_DOMAIN_RE2 = new RE2(String.raw`^[a-zA-Z0-9]([a-zA-Z0-9.\-]*[a-zA-Z0-9])?$`);

export const validateTrustDomain = z
  .string()
  .trim()
  .min(1, "Trust domain is required")
  .refine((val) => TRUST_DOMAIN_RE2.test(val), "Invalid trust domain format");

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.

2 participants