Skip to content

Commit 89e8703

Browse files
committed
Update some docs and todos as well as code issues
1 parent 465134a commit 89e8703

File tree

2 files changed

+54
-8
lines changed

2 files changed

+54
-8
lines changed

README.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,47 @@ The Cavage/RichAnna specifications have changed over time, introducing new featu
2424
the [latest version of the specification](https://datatracker.ietf.org/doc/html/draft-richanna-http-message-signatures)
2525
and not to try to support each version in isolation.
2626

27+
## Limitations in compliance with the specification
28+
29+
As with many libraries and environments, HTTP Requests and Responses are abstracted away from the
30+
developer. This fact is noted in the specification. As such (in compliance with the specification),
31+
consumers of this library should take care to make sure that they are processing signatures that
32+
only cover fields/components whose values can be reliably resolved. Below is a list of limitations
33+
that you should be aware of when selecting a list of parameters to sign or accept.
34+
35+
### Derived component limitations
36+
37+
Many of the derived components are expected to be sourced from what are effectively http2 pseudo
38+
headers. However, if the application is not running in http2 mode or the message being signed is
39+
not being built as a http2 message, then some of these pseudo headers will not be available to the
40+
application and must be derived from a URL.
41+
42+
#### @request-target
43+
44+
The [`@request-target`](https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-message-signatures#section-2.2.5)
45+
component is intended to be the equivalent to the "request target portion of the request line".
46+
See the specification for examples of what this means. In NodeJS, this line in requests is automatically
47+
constructed for consumers, so it's not possible to know for certainty what this will be. For incoming
48+
requests, it is possible to extract, but for simplicity’s sake this library does not process the raw
49+
headers for the incoming request and, as such, cannot calculate this value with certainty. It is
50+
recommended that this component is avoided.
51+
52+
### Multiple message component contexts
53+
54+
As described in [section 7.4.4](https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-message-signatures#section-7.4.4)
55+
it is deemed that complex message context resolution is outside the scope of this library.
56+
57+
This means that it is the responsibility of the consumer of this library to construct the equivalent
58+
message context for signatures that need to be reinterpreted based on other signer contexts.
59+
60+
61+
### Padding attacks
62+
63+
As described in [section 7.5.7](https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-message-signatures-13#section-7.5.7)
64+
it is expected that the NodeJS application has taken steps to ensure that headers are valid and not
65+
"garbage". For this library to take on that obligation would be to widen the scope of the library to
66+
a complete HTTP Message validator.
67+
2768
## Examples
2869

2970
### Signing a request

src/httpbis/index.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ export function deriveComponent(component: string, params: Map<string, string |
3131
/**
3232
* Components can be derived from requests or responses (which can also be bound to their request).
3333
* The signature is essentially (component, params, signingSubject, supplementaryData)
34+
*
35+
* @todo - prefer pseudo-headers over parsed urls
3436
*/
3537
export function deriveComponent(component: string, params: Map<string, string | number | boolean>, message: Request | Response, req?: Request): string[] {
3638
// switch the context of the signing data depending on if the `req` flag was passed
@@ -94,12 +96,6 @@ export function deriveComponent(component: string, params: Map<string, string |
9496
// absent query params means use `?`
9597
return [decodeURI(search) || '?'];
9698
}
97-
case '@status': {
98-
if (isRequest(context)) {
99-
throw new Error('Cannot obtain @status component for requests');
100-
}
101-
return [context.status.toString()];
102-
}
10399
case '@query-param': {
104100
if (!isRequest(context)) {
105101
throw new Error('Cannot derive @scheme on response');
@@ -114,6 +110,12 @@ export function deriveComponent(component: string, params: Map<string, string |
114110
}
115111
return searchParams.getAll(name);
116112
}
113+
case '@status': {
114+
if (isRequest(context)) {
115+
throw new Error('Cannot obtain @status component for requests');
116+
}
117+
return [context.status.toString()];
118+
}
117119
default:
118120
throw new Error(`Unsupported component "${component}"`);
119121
}
@@ -132,8 +134,8 @@ export function extractHeader(header: string, params: Map<string, string | numbe
132134
throw new Error(`No header "${header}" found in headers`);
133135
}
134136
const values = (Array.isArray(headerTuple[1]) ? headerTuple[1] : [headerTuple[1]]);
135-
if (params.has('bs') && params.has('sf')) {
136-
throw new Error('Invalid combination of parameters');
137+
if (params.has('bs') && (params.has('sf') || params.has('key'))) {
138+
throw new Error('Cannot have both `bs` and (implicit) `sf` parameters');
137139
}
138140
if (params.has('sf') || params.has('key')) {
139141
// strict encoding of field
@@ -234,6 +236,9 @@ export function createSigningParameters(config: SignConfig): Parameters {
234236
break;
235237
}
236238
case 'alg': {
239+
// if there is no alg, but it's listed as a required parameter, we should probably
240+
// throw an error - the problem is that if it's in the default set of params, do we
241+
// really want to throw if there's no keyid?
237242
const alg = config.paramValues?.alg ?? config.key.alg ?? null;
238243
if (alg) {
239244
value = alg.toString();

0 commit comments

Comments
 (0)