Properly validate atproto did:web (#2776)

* Properly validate atproto did:web

* explicit why there is no protection against localhost fetches in did:web resolver

.changeset/breezy-ducks-allow.md

@@ -0,0 +1,5 @@
+---
+"@atproto/did": patch
+---
+
+Disallow path component in Web DID's (as per spec)

.changeset/hip-beds-flow.md

@@ -0,0 +1,5 @@
+---
+"@atproto/did": patch
+---
+
+Properly parse localhost did:web

.changeset/olive-rabbits-occur.md

@@ -0,0 +1,5 @@
+---
+"@atproto/did": patch
+---
+
+Code optimizations and documentation. Rename `check*` utility function to `assert*`.

packages/did/src/atproto.ts

@@ -1,12 +1,11 @@
 import { InvalidDidError } from './did-error.js'
 import { Did } from './did.js'
 import {
-  checkDidPlc,
-  checkDidWeb,
+  assertDidPlc,
+  assertDidWeb,
   DID_PLC_PREFIX,
   DID_WEB_PREFIX,
   isDidPlc,
-  isDidWeb,
 } from './methods.js'
 
 // This file contains atproto-specific DID validation utilities.
@@ -30,17 +29,17 @@ export function isAtprotoDid(input: unknown): input is AtprotoDid {
 }
 
 export function asAtprotoDid(input: unknown): AtprotoDid {
-  checkAtprotoDid(input)
+  assertAtprotoDid(input)
   return input
 }
 
-export function checkAtprotoDid(input: unknown): asserts input is AtprotoDid {
+export function assertAtprotoDid(input: unknown): asserts input is AtprotoDid {
   if (typeof input !== 'string') {
     throw new InvalidDidError(typeof input, `DID must be a string`)
   } else if (input.startsWith(DID_PLC_PREFIX)) {
-    checkDidPlc(input)
+    assertDidPlc(input)
   } else if (input.startsWith(DID_WEB_PREFIX)) {
-    checkDidWeb(input)
+    assertAtprotoDidWeb(input)
   } else {
     throw new InvalidDidError(
       input,
@@ -49,27 +48,37 @@ export function checkAtprotoDid(input: unknown): asserts input is AtprotoDid {
   }
 }
 
-/**
- * @see {@link https://atproto.com/specs/did#blessed-did-methods}
- */
-export function isAtprotoDidWeb(input: unknown): input is Did<'web'> {
-  // Optimization: make cheap checks first
-  if (typeof input !== 'string') {
-    return false
-  }
+export function assertAtprotoDidWeb(
+  input: unknown,
+): asserts input is Did<'web'> {
+  assertDidWeb(input)
 
-  // Path are not allowed
   if (input.includes(':', DID_WEB_PREFIX.length)) {
-    return false
+    throw new InvalidDidError(
+      input,
+      `Atproto does not allow path components in Web DIDs`,
+    )
   }
 
-  // Port numbers are not allowed, except for localhost
   if (
     input.includes('%3A', DID_WEB_PREFIX.length) &&
     !input.startsWith('did:web:localhost%3A')
   ) {
+    throw new InvalidDidError(
+      input,
+      `Atproto does not allow port numbers in Web DIDs, except for localhost`,
+    )
+  }
+}
+
+/**
+ * @see {@link https://atproto.com/specs/did#blessed-did-methods}
+ */
+export function isAtprotoDidWeb(input: unknown): input is Did<'web'> {
+  try {
+    assertAtprotoDidWeb(input)
+    return true
+  } catch {
     return false
   }
-
-  return isDidWeb(input)
 }

packages/did/src/did-document.ts

@@ -117,35 +117,27 @@ export type DidDocument<Method extends string = string> = z.infer<
 
 // @TODO: add other refinements ?
 export const didDocumentValidator = didDocumentSchema
-  .superRefine((data, ctx) => {
-    if (data.service) {
-      for (let i = 0; i < data.service.length; i++) {
-        if (data.service[i].id === data.id) {
+  // Ensure that every service id is unique
+  .superRefine(({ id: did, service }, ctx) => {
+    if (service) {
+      const visited = new Set()
+
+      for (let i = 0; i < service.length; i++) {
+        const current = service[i]
+
+        const serviceId = current.id.startsWith('#')
+          ? `${did}${current.id}`
+          : current.id
+
+        if (!visited.has(serviceId)) {
+          visited.add(serviceId)
+        } else {
           ctx.addIssue({
             code: z.ZodIssueCode.custom,
-            message: `Service id must be different from the document id`,
+            message: `Duplicate service id (${current.id}) found in the document`,
             path: ['service', i, 'id'],
           })
         }
       }
     }
   })
-  .superRefine((data, ctx) => {
-    if (data.service) {
-      const normalizedIds = data.service.map((s) =>
-        s.id?.startsWith('#') ? `${data.id}${s.id}` : s.id,
-      )
-
-      for (let i = 0; i < normalizedIds.length; i++) {
-        for (let j = i + 1; j < normalizedIds.length; j++) {
-          if (normalizedIds[i] === normalizedIds[j]) {
-            ctx.addIssue({
-              code: z.ZodIssueCode.custom,
-              message: `Duplicate service id (${normalizedIds[j]}) found in the document`,
-              path: ['service', j, 'id'],
-            })
-          }
-        }
-      }
-    }
-  })

packages/did/src/did.ts

@@ -83,7 +83,7 @@ type AsDidMethodInternal<
  * Check if the input is a valid DID method name, at the position between
  * `start` (inclusive) and `end` (exclusive).
  */
-export function checkDidMethod(
+export function assertDidMethod(
   input: string,
   start = 0,
   end = input.length,
@@ -130,7 +130,7 @@ export function extractDidMethod<D extends Did>(did: D) {
  * Check if the input is a valid DID method-specific identifier, at the position
  * between `start` (inclusive) and `end` (exclusive).
  */
-export function checkDidMsid(
+export function assertDidMsid(
   input: string,
   start = 0,
   end = input.length,
@@ -207,7 +207,7 @@ export function checkDidMsid(
   }
 }
 
-export function checkDid(input: unknown): asserts input is Did {
+export function assertDid(input: unknown): asserts input is Did {
   if (typeof input !== 'string') {
     throw new InvalidDidError(typeof input, `DID must be a string`)
   }
@@ -226,13 +226,13 @@ export function checkDid(input: unknown): asserts input is Did {
     throw new InvalidDidError(input, `Missing colon after method name`)
   }
 
-  checkDidMethod(input, DID_PREFIX_LENGTH, idSep)
-  checkDidMsid(input, idSep + 1, length)
+  assertDidMethod(input, DID_PREFIX_LENGTH, idSep)
+  assertDidMsid(input, idSep + 1, length)
 }
 
 export function isDid(input: unknown): input is Did {
   try {
-    checkDid(input)
+    assertDid(input)
     return true
   } catch (err) {
     if (err instanceof DidError) {
@@ -245,7 +245,7 @@ export function isDid(input: unknown): input is Did {
 }
 
 export function asDid(input: unknown): Did {
-  checkDid(input)
+  assertDid(input)
   return input
 }
 
@@ -253,7 +253,7 @@ export const didSchema = z
   .string()
   .superRefine((value: string, ctx: z.RefinementCtx): value is Did => {
     try {
-      checkDid(value)
+      assertDid(value)
       return true
     } catch (err) {
       ctx.addIssue({

packages/did/src/methods/plc.ts

@@ -8,23 +8,22 @@ const DID_PLC_LENGTH = 32
 export { DID_PLC_PREFIX }
 
 export function isDidPlc(input: unknown): input is Did<'plc'> {
-  // Optimization: make cheap checks first
+  // Optimization: equivalent to try/catch around "assertDidPlc"
   if (typeof input !== 'string') return false
-
-  try {
-    checkDidPlc(input)
-    return true
-  } catch {
-    return false
+  if (input.length !== DID_PLC_LENGTH) return false
+  if (!input.startsWith(DID_PLC_PREFIX)) return false
+  for (let i = DID_PLC_PREFIX_LENGTH; i < DID_PLC_LENGTH; i++) {
+    if (!isBase32Char(input.charCodeAt(i))) return false
   }
+  return true
 }
 
 export function asDidPlc(input: unknown): Did<'plc'> {
-  checkDidPlc(input)
+  assertDidPlc(input)
   return input
 }
 
-export function checkDidPlc(input: unknown): asserts input is Did<'plc'> {
+export function assertDidPlc(input: unknown): asserts input is Did<'plc'> {
   if (typeof input !== 'string') {
     throw new InvalidDidError(typeof input, `DID must be a string`)
   }
@@ -40,12 +39,16 @@ export function checkDidPlc(input: unknown): asserts input is Did<'plc'> {
     throw new InvalidDidError(input, `Invalid did:plc prefix`)
   }
 
-  let c: number
+  // The following check is not necessary, as the check below is more strict:
+
+  // assertDidMsid(input, DID_PLC_PREFIX.length)
+
   for (let i = DID_PLC_PREFIX_LENGTH; i < DID_PLC_LENGTH; i++) {
-    c = input.charCodeAt(i)
-    // Base32 encoding ([a-z2-7])
-    if ((c < 0x61 || c > 0x7a) && (c < 0x32 || c > 0x37)) {
+    if (!isBase32Char(input.charCodeAt(i))) {
       throw new InvalidDidError(input, `Invalid character at position ${i}`)
     }
   }
 }
+
+const isBase32Char = (c: number): boolean =>
+  (c >= 0x61 && c <= 0x7a) || (c >= 0x32 && c <= 0x37) // [a-z2-7]

packages/did/src/methods/web.ts

@@ -1,5 +1,5 @@
 import { InvalidDidError } from '../did-error.js'
-import { Did, checkDidMsid } from '../did.js'
+import { Did, assertDidMsid } from '../did.js'
 
 export const DID_WEB_PREFIX = `did:web:` satisfies Did<'web'>
 
@@ -11,7 +11,7 @@ export function isDidWeb(input: unknown): input is Did<'web'> {
   if (typeof input !== 'string') return false
 
   try {
-    checkDidWeb(input)
+    assertDidWeb(input)
     return true
   } catch {
     return false
@@ -19,11 +19,11 @@ export function isDidWeb(input: unknown): input is Did<'web'> {
 }
 
 export function asDidWeb(input: unknown): Did<'web'> {
-  checkDidWeb(input)
+  assertDidWeb(input)
   return input
 }
 
-export function checkDidWeb(input: unknown): asserts input is Did<'web'> {
+export function assertDidWeb(input: unknown): asserts input is Did<'web'> {
   if (typeof input !== 'string') {
     throw new InvalidDidError(typeof input, `DID must be a string`)
   }
@@ -41,12 +41,16 @@ export function didWebToUrl(did: string): URL {
   }
 
   // Make sure every char is valid (per DID spec)
-  checkDidMsid(did, DID_WEB_PREFIX.length)
+  assertDidMsid(did, DID_WEB_PREFIX.length)
 
   try {
     const msid = did.slice(DID_WEB_PREFIX.length)
     const parts = msid.split(':').map(decodeURIComponent)
-    return new URL(`https://${parts.join('/')}`)
+    const url = new URL(`https://${parts.join('/')}`)
+    if (url.hostname === 'localhost') {
+      url.protocol = 'http:'
+    }
+    return url
   } catch (cause) {
     throw new InvalidDidError(did, 'Invalid Web DID', cause)
   }

packages/internal/did-resolver/src/methods/plc.ts

@@ -6,7 +6,7 @@ import {
   fetchOkProcessor,
 } from '@atproto-labs/fetch'
 import { pipe } from '@atproto-labs/pipe'
-import { Did, checkDidPlc, didDocumentValidator } from '@atproto/did'
+import { Did, assertDidPlc, didDocumentValidator } from '@atproto/did'
 
 import { DidMethod, ResolveDidOptions } from '../did-method.js'
 
@@ -43,7 +43,7 @@ export class DidPlcMethod implements DidMethod<'plc'> {
   async resolve(did: Did<'plc'>, options?: ResolveDidOptions) {
     // Although the did should start with `did:plc:` (thanks to typings), we
     // should still check if the msid is valid.
-    checkDidPlc(did)
+    assertDidPlc(did)
 
     const url = new URL(`/${did}`, this.plcDirectoryUrl)
 

packages/internal/did-resolver/src/methods/web.ts

@@ -30,6 +30,11 @@ export class DidWebMethod implements DidMethod<'web'> {
   async resolve(did: Did<'web'>, options?: ResolveDidOptions) {
     const didDocumentUrl = buildDidWebDocumentUrl(did)
 
+    // Note we do not explicitly check for "localhost" here. Instead, we rely on
+    // the injected 'fetch' function to handle the URL. If the URL is
+    // "localhost", or resolves to a private IP address, the fetch function is
+    // responsible for handling it.
+
     return this.fetch(didDocumentUrl, {
       redirect: 'error',
       headers: { accept: 'application/did+ld+json,application/json' },