Skip to content

Commit

Permalink
fix: bug in DPoP confirmation claim verification
Browse files Browse the repository at this point in the history
  • Loading branch information
matthieubosquet committed Feb 25, 2021
1 parent 0cbb504 commit fbdeb4a
Show file tree
Hide file tree
Showing 8 changed files with 177 additions and 119 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@solid/identity-token-verifier",
"version": "0.5.1",
"version": "0.6.0",
"description": "Verifies Solid access tokens via their WebID claim, and thus asserts ownership of WebIDs.",
"license": "MIT",
"keywords": [
Expand Down
6 changes: 2 additions & 4 deletions src/guards/DPoPJWKGuard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ export function isDPoPPublicJWK(x: unknown): asserts x is DPoPPublicJWK {
*/

export function isECPublicJWK(x: unknown): asserts x is ECPublicJWK {
asserts.areObjectPropertiesOf(x, ["kid", "kty", "crv", "x", "y"]);
asserts.isString(x.kid);
asserts.areObjectPropertiesOf(x, ["kty", "crv", "x", "y"]);
asserts.isLiteral(x.kty, "EC" as const);
asserts.isLiteralType(x.crv, curve);
asserts.isString(x.x);
Expand All @@ -61,9 +60,8 @@ export function isECPublicJWK(x: unknown): asserts x is ECPublicJWK {
*/

export function isRSAPublicJWK(x: unknown): asserts x is RSAPublicJWK {
asserts.areObjectPropertiesOf(x, ["alg", "kid", "kty", "n", "e"]);
asserts.areObjectPropertiesOf(x, ["alg", "kty", "n", "e"]);
asserts.isLiteralType(x.alg, rsaAlgorithm);
asserts.isString(x.kid);
asserts.isLiteral(x.kty, "RSA" as const);
asserts.isString(x.n);
asserts.isString(x.e);
Expand Down
18 changes: 14 additions & 4 deletions src/lib/DPoP.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import EmbeddedJWK from "jose/jwk/embedded";
import calculateThumbprint from "jose/jwk/thumbprint";
import jwtVerify from "jose/jwt/verify";
import { asserts } from "ts-guards";
import { isDPoPBoundAccessTokenPayload, isDPoPToken } from "../guards";
Expand All @@ -11,7 +12,7 @@ import type {
import { asymetricCryptographicAlgorithm } from "../types";
import { clockToleranceInSeconds, maxAgeInMilliseconds } from "./Defaults";

function isValidProof(
async function isValidProof(
accessToken: AccessToken,
dpop: DPoPToken,
method: RequestMethod,
Expand All @@ -21,8 +22,17 @@ function isValidProof(
asserts.isObjectPropertyOf(accessToken.payload, "cnf");
isDPoPBoundAccessTokenPayload(accessToken.payload);

// Check DPoP is bound to the access token
asserts.isLiteral(dpop.header.jwk.kid, accessToken.payload.cnf.jkt);
/*
* Check DPoP is bound to the access token
* The value in "jkt" MUST be the base64url encoding [RFC7515] of the
* JWK SHA-256 Thumbprint (according to [RFC7638]) of the public key to
* which the access token is bound.
* https://tools.ietf.org/html/draft-fett-oauth-dpop-04#section-7
*/
asserts.isLiteral(
await calculateThumbprint(dpop.header.jwk),
accessToken.payload.cnf.jkt
);

// Check DPoP Token claims method, url and unique token id
asserts.isLiteral(dpop.payload.htm, method);
Expand Down Expand Up @@ -69,7 +79,7 @@ export async function verify(

isDPoPToken(dpop);

isValidProof(accessToken, dpop, method, url, isDuplicateJTI);
await isValidProof(accessToken, dpop, method, url, isDuplicateJTI);

return dpop;
}
30 changes: 3 additions & 27 deletions src/types/DPoPJWK.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,7 @@ export type PrivateKeyProperties = typeof privateKeyProperties extends Set<
* - Must have kid, kty, crv, x, y & d
* - d is the private part of the key, it must be ommited from the public embedded DPoP JWK
*/
export type ECJWK = Required<
Pick<JWK, "kid" | "kty" | "crv" | "x" | "y" | "d">
> & {
kid: string;
kty: "EC";
crv: Curve;
x: string;
y: string;
d: string;
};
export type ECJWK = Required<Pick<JWK, "kty" | "crv" | "x" | "y" | "d">>;

export type ECPublicJWK = Omit<ECJWK, PrivateKeyProperties>;

Expand All @@ -94,23 +85,8 @@ export type ECPublicJWK = Omit<ECJWK, PrivateKeyProperties>;
* - d is the private part of the key, it must be ommited from the public embedded DPoP JWK
*/
export type RSAJWK = Required<
Pick<
JWK,
"kid" | "kty" | "alg" | "n" | "e" | "d" | "p" | "q" | "dp" | "dq" | "qi"
>
> & {
kid: string;
kty: "RSA";
alg: RSAAlgorithm;
n: string;
e: string;
d: string;
p: string;
q: string;
dp: string;
dq: string;
qi: string;
};
Pick<JWK, "kty" | "alg" | "n" | "e" | "d" | "p" | "q" | "dp" | "dq" | "qi">
>;

export type RSAPublicJWK = Omit<RSAJWK, PrivateKeyProperties>;

Expand Down
189 changes: 130 additions & 59 deletions test/DPoP.test.ts
Original file line number Diff line number Diff line change
@@ -1,143 +1,214 @@
import jwtVerify from "jose/jwt/verify";
import { verify } from "../src/lib/DPoP";
import {
dpopTokenHeaderUnsupported,
dpopTokenEC,
dpopTokenRSA,
} from "./fixture/DPoPToken";
import type { DPoPToken } from "../src/types";
/*
* import {
* dpopTokenHeaderUnsupported,
* dpopTokenEC,
* } from "./fixture/DPoPToken";
*/
import { encodeToken } from "./fixture/EncodeToken";

jest.mock("jose/jwt/verify");

describe("DPoP proof", () => {
const accessToken: any = { payload: { cnf: { jkt: "confirmed_ID" } } };
const wrongAccessToken: any = { payload: { cnf: { jkt: "unconfirmed_ID" } } };
const dpop: DPoPToken = {
header: {
typ: "dpop+jwt",
alg: "ES256",
jwk: {
kty: "EC",
x: "l8tFrhx-34tV3hRICRDY9zCkDlpBhF42UQUfWVAWBFs",
y: "9VE4jf_Ok_o64zbTTlcuNJajHmt6v9TDVrU0CdvGRDA",
crv: "P-256",
},
},
payload: {
jti: "e1j3V_bKic8-LAEB",
htm: "GET",
htu: "https://resource.example.org/protectedresource",
iat: 1562262618,
},
signature:
"lNhmpAX1WwmpBvwhok4E74kWCiGBNdavjLAeevGy32H3dbF0Jbri69Nm2ukkwb-uyUI4AUg1JSskfWIyo4UCbQ",
};

const dpopRSA: DPoPToken = {
header: {
typ: "dpop+jwt",
alg: "RS256",
jwk: {
alg: "RS256",
kty: "RSA",
e: "AQAB",
n:
"12oBZRhCiZFJLcPg59LkZZ9mdhSMTKAQZYq32k_ti5SBB6jerkh-WzOMAO664r_qyLkqHUSp3u5SbXtseZEpN3XPWGKSxjsy-1JyEFTdLSYe6f9gfrmxkUF_7DTpq0gn6rntP05g2-wFW50YO7mosfdslfrTJYWHFhJALabAeYirYD7-9kqq9ebfFMF4sRRELbv9oi36As6Q9B3Qb5_C1rAzqfao_PCsf9EPsTZsVVVkA5qoIAr47lo1ipfiBPxUCCNSdvkmDTYgvvRm6ZoMjFbvOtgyts55fXKdMWv7I9HMD5HwE9uW839PWA514qhbcIsXEYSFMPMV6fnlsiZvQQ",
},
},
payload: dpop.payload,
signature: "",
};

const dpopWrongKeyType: DPoPToken = {
header: {
typ: "dpop+jwt",
alg: "RS256",
jwk: {
alg: "RS256",
kty: "UNSUPPORTED_KEY_TYPE",
e: "AQAB",
n:
"12oBZRhCiZFJLcPg59LkZZ9mdhSMTKAQZYq32k_ti5SBB6jerkh-WzOMAO664r_qyLkqHUSp3u5SbXtseZEpN3XPWGKSxjsy-1JyEFTdLSYe6f9gfrmxkUF_7DTpq0gn6rntP05g2-wFW50YO7mosfdslfrTJYWHFhJALabAeYirYD7-9kqq9ebfFMF4sRRELbv9oi36As6Q9B3Qb5_C1rAzqfao_PCsf9EPsTZsVVVkA5qoIAr47lo1ipfiBPxUCCNSdvkmDTYgvvRm6ZoMjFbvOtgyts55fXKdMWv7I9HMD5HwE9uW839PWA514qhbcIsXEYSFMPMV6fnlsiZvQQ",
},
},
payload: dpop.payload,
signature: "",
};

it("Checks conforming proof", async () => {
describe("DPoP proof", () => {
it("Checks conforming proof with EC Key", async () => {
(jwtVerify as jest.Mock).mockResolvedValueOnce({
payload: dpopTokenEC.payload,
protectedHeader: dpopTokenEC.header,
payload: dpop.payload,
protectedHeader: dpop.header,
});

expect(
await verify(
encodeToken(dpopTokenEC),
accessToken,
encodeToken(dpop),
{
payload: {
cnf: { jkt: "0ZcOCORZNYy-DWpqq30jZyJGHTN0d2HglBV3uiguA4I" },
},
} as any,
"GET",
"https://example.com",
"https://resource.example.org/protectedresource",
() => false
)
).toStrictEqual(dpopTokenEC);
).toStrictEqual(dpop);
});

it("Checks conforming proof with RSA JWK", async () => {
it("Checks conforming proof with RSA Key", async () => {
(jwtVerify as jest.Mock).mockResolvedValueOnce({
payload: dpopTokenRSA.payload,
protectedHeader: dpopTokenRSA.header,
payload: dpopRSA.payload,
protectedHeader: dpopRSA.header,
});

expect(
await verify(
encodeToken(dpopTokenRSA),
accessToken,
encodeToken(dpopRSA),
{
payload: {
cnf: { jkt: "cbaZgHZazjgQq0Q2-Hy_o2-OCDpPu02S30lNhTsNU1Q" },
},
} as any,
"GET",
"https://example.com",
"https://resource.example.org/protectedresource",
() => false
)
).toStrictEqual(dpopTokenRSA);
).toStrictEqual(dpopRSA);
});

it("Throws on unsupported JWK", async () => {
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
it("Fails unsupported Key type", async () => {
(jwtVerify as jest.Mock).mockResolvedValueOnce({
payload: dpopTokenRSA.payload,
protectedHeader: dpopTokenHeaderUnsupported,
payload: dpopWrongKeyType.payload,
protectedHeader: dpopWrongKeyType.header,
});
const unsupportedKeyType = {
header: dpopTokenHeaderUnsupported,
payload: dpopTokenRSA.payload,
signature: "",
};
/* eslint-enable @typescript-eslint/no-unsafe-assignment */

await expect(
verify(
encodeToken(unsupportedKeyType),
accessToken,
encodeToken(dpopRSA),
{
payload: {
cnf: { jkt: "cbaZgHZazjgQq0Q2-Hy_o2-OCDpPu02S30lNhTsNU1Q" },
},
} as any,
"GET",
"https://example.com",
"https://resource.example.org/protectedresource",
() => false
)
).rejects.toThrow("Expected EC or RSA, got:\nUNSUPPORTED_KEY_TYPE");
});

it("Throws on wrong method", async () => {
(jwtVerify as jest.Mock).mockResolvedValueOnce({
payload: dpopTokenEC.payload,
protectedHeader: dpopTokenEC.header,
payload: dpop.payload,
protectedHeader: dpop.header,
});

await expect(
verify(
encodeToken(dpopTokenEC),
accessToken,
encodeToken(dpop),
{
payload: {
cnf: { jkt: "0ZcOCORZNYy-DWpqq30jZyJGHTN0d2HglBV3uiguA4I" },
},
} as any,
"POST",
"https://example.com",
"https://resource.example.org/protectedresource",
() => false
)
).rejects.toThrow("Expected POST, got:\nGET");
});

it("Throws on wrong URL", async () => {
it("Throws on wrong url", async () => {
(jwtVerify as jest.Mock).mockResolvedValueOnce({
payload: dpopTokenEC.payload,
protectedHeader: dpopTokenEC.header,
payload: dpop.payload,
protectedHeader: dpop.header,
});

await expect(
verify(
encodeToken(dpopTokenEC),
accessToken,
encodeToken(dpop),
{
payload: {
cnf: { jkt: "0ZcOCORZNYy-DWpqq30jZyJGHTN0d2HglBV3uiguA4I" },
},
} as any,
"GET",
"https://example.coms",
"https://resource.example.org/otherresource",
() => false
)
).rejects.toThrow(
"Expected https://example.coms, got:\nhttps://example.com"
"Expected https://resource.example.org/otherresource, got:\nhttps://resource.example.org/protectedresource"
);
});

it("Throws on duplicate JTI", async () => {
(jwtVerify as jest.Mock).mockResolvedValueOnce({
payload: dpopTokenEC.payload,
protectedHeader: dpopTokenEC.header,
payload: dpop.payload,
protectedHeader: dpop.header,
});

await expect(
verify(
encodeToken(dpopTokenEC),
accessToken,
encodeToken(dpop),
{
payload: {
cnf: { jkt: "0ZcOCORZNYy-DWpqq30jZyJGHTN0d2HglBV3uiguA4I" },
},
} as any,
"GET",
"https://example.com",
"https://resource.example.org/protectedresource",
() => true
)
).rejects.toThrow("Expected false, got:\ntrue");
});

it("Throws on wrong confirmation claim", async () => {
(jwtVerify as jest.Mock).mockResolvedValueOnce({
payload: dpopTokenEC.payload,
protectedHeader: dpopTokenEC.header,
payload: dpop.payload,
protectedHeader: dpop.header,
});

await expect(
verify(
encodeToken(dpopTokenEC),
wrongAccessToken,
encodeToken(dpop),
{ payload: { cnf: { jkt: "UNCONFIRMED_KEY_THUMBPRINT" } } } as any,
"GET",
"https://example.com",
() => true
"https://resource.example.org/protectedresource",
() => false
)
).rejects.toThrow("Expected unconfirmed_ID, got:\nconfirmed_ID");
).rejects.toThrow(
"Expected UNCONFIRMED_KEY_THUMBPRINT, got:\n0ZcOCORZNYy-DWpqq30jZyJGHTN0d2HglBV3uiguA4I"
);
});
});
Loading

0 comments on commit fbdeb4a

Please sign in to comment.