Skip to content

Commit

Permalink
Update some docs and todos as well as code issues
Browse files Browse the repository at this point in the history
  • Loading branch information
dhensby committed Oct 3, 2022
1 parent 9324ca4 commit 8f18da0
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 8 deletions.
41 changes: 41 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,47 @@ The Cavage/RichAnna specifications have changed over time, introducing new featu
the [latest version of the specification](https://datatracker.ietf.org/doc/html/draft-richanna-http-message-signatures)
and not to try to support each version in isolation.

## Limitations in compliance with the specification

As with many libraries and environments, HTTP Requests and Responses are abstracted away from the
developer. This fact is noted in the specification. As such (in compliance with the specification),
consumers of this library should take care to make sure that they are processing signatures that
only cover fields/components whose values can be reliably resolved. Below is a list of limitations
that you should be aware of when selecting a list of parameters to sign or accept.

### Derived component limitations

Many of the derived components are expected to be sourced from what are effectively http2 pseudo
headers. However, if the application is not running in http2 mode or the message being signed is
not being built as a http2 message, then some of these pseudo headers will not be available to the
application and must be derived from a URL.

#### @request-target

The [`@request-target`](https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-message-signatures#section-2.2.5)
component is intended to be the equivalent to the "request target portion of the request line".
See the specification for examples of what this means. In NodeJS, this line in requests is automatically
constructed for consumers, so it's not possible to know for certainty what this will be. For incoming
requests, it is possible to extract, but for simplicity’s sake this library does not process the raw
headers for the incoming request and, as such, cannot calculate this value with certainty. It is
recommended that this component is avoided.

### Multiple message component contexts

As described in [section 7.4.4](https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-message-signatures#section-7.4.4)
it is deemed that complex message context resolution is outside the scope of this library.

This means that it is the responsibility of the consumer of this library to construct the equivalent
message context for signatures that need to be reinterpreted based on other signer contexts.


### Padding attacks

As described in [section 7.5.7](https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-message-signatures-13#section-7.5.7)
it is expected that the NodeJS application has taken steps to ensure that headers are valid and not
"garbage". For this library to take on that obligation would be to widen the scope of the library to
a complete HTTP Message validator.

## Examples

### Signing a request
Expand Down
21 changes: 13 additions & 8 deletions src/httpbis/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ export function deriveComponent(component: string, params: Map<string, string |
/**
* Components can be derived from requests or responses (which can also be bound to their request).
* The signature is essentially (component, params, signingSubject, supplementaryData)
*
* @todo - prefer pseudo-headers over parsed urls
*/
export function deriveComponent(component: string, params: Map<string, string | number | boolean>, message: Request | Response, req?: Request): string[] {
// switch the context of the signing data depending on if the `req` flag was passed
Expand Down Expand Up @@ -94,12 +96,6 @@ export function deriveComponent(component: string, params: Map<string, string |
// absent query params means use `?`
return [decodeURI(search) || '?'];
}
case '@status': {
if (isRequest(context)) {
throw new Error('Cannot obtain @status component for requests');
}
return [context.status.toString()];
}
case '@query-param': {
if (!isRequest(context)) {
throw new Error('Cannot derive @scheme on response');
Expand All @@ -114,6 +110,12 @@ export function deriveComponent(component: string, params: Map<string, string |
}
return searchParams.getAll(name);
}
case '@status': {
if (isRequest(context)) {
throw new Error('Cannot obtain @status component for requests');
}
return [context.status.toString()];
}
default:
throw new Error(`Unsupported component "${component}"`);
}
Expand All @@ -132,8 +134,8 @@ export function extractHeader(header: string, params: Map<string, string | numbe
throw new Error(`No header "${header}" found in headers`);
}
const values = (Array.isArray(headerTuple[1]) ? headerTuple[1] : [headerTuple[1]]);
if (params.has('bs') && params.has('sf')) {
throw new Error('Invalid combination of parameters');
if (params.has('bs') && (params.has('sf') || params.has('key'))) {
throw new Error('Cannot have both `bs` and (implicit) `sf` parameters');
}
if (params.has('sf') || params.has('key')) {
// strict encoding of field
Expand Down Expand Up @@ -234,6 +236,9 @@ export function createSigningParameters(config: SignConfig): Parameters {
break;
}
case 'alg': {
// if there is no alg, but it's listed as a required parameter, we should probably
// throw an error - the problem is that if it's in the default set of params, do we
// really want to throw if there's no keyid?
const alg = config.paramValues?.alg ?? config.key.alg ?? null;
if (alg) {
value = alg.toString();
Expand Down

0 comments on commit 8f18da0

Please sign in to comment.