Skip to content

Commit

Permalink
feat(DPoP): send a dpop-nonce when the proof's iat check fails and no…
Browse files Browse the repository at this point in the history 
…nces are configured but not required
  • Loading branch information
@panva
panva committed Feb 13, 2024
1 parent 4d635e2 commit 1b073c0
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 32 deletions.
1 change: 1 addition & 0 deletions certification/fapi/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ const fapi = new Provider(ISSUER, {
},
dPoP: {
enabled: true,
nonceSecret: crypto.randomBytes(32),
},
mTLS: {
enabled: true,
Expand Down
5 changes: 4 additions & 1 deletion certification/oidc/configuration.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ export default {
},
claimsParameter: { enabled: true },
deviceFlow: { enabled: true },
dPoP: { enabled: true },
dPoP: {
enabled: true,
nonceSecret: crypto.randomBytes(32),
},
encryption: { enabled: true },
jwtUserinfo: { enabled: true },
introspection: { enabled: true },
Expand Down
8 changes: 6 additions & 2 deletions lib/helpers/validate_dpop.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export default async (ctx, accessToken) => {
throw new Error('features.dPoP.nonceSecret configuration is missing');
}

const nextNonce = DPoPNonces?.nextNonce();
let payload;
let protectedHeader;
try {
Expand All @@ -65,6 +66,10 @@ export default async (ctx, accessToken) => {
const now = epochTime();
const diff = Math.abs(now - payload.iat);
if (diff > DPOP_OK_WINDOW) {
if (nextNonce) {
ctx.set('DPoP-Nonce', nextNonce);
throw new UseDpopNonce('DPoP proof iat is not recent enough, use a DPoP nonce instead');
}
throw new InvalidDpopProof('DPoP proof iat is not recent enough');
}
} else if (!DPoPNonces) {
Expand Down Expand Up @@ -96,13 +101,12 @@ export default async (ctx, accessToken) => {
}
}
} catch (err) {
if (err instanceof InvalidDpopProof) {
if (err instanceof InvalidDpopProof || err instanceof UseDpopNonce) {
throw err;
}
throw new InvalidDpopProof('invalid DPoP key binding', err.message);
}

const nextNonce = DPoPNonces?.nextNonce();
if (!payload.nonce && requireNonce) {
ctx.set('DPoP-Nonce', nextNonce);
throw new UseDpopNonce('nonce is required in the DPoP proof');
Expand Down
71 changes: 42 additions & 29 deletions test/dpop/dpop.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,35 +270,48 @@ describe('features.dPoP', () => {
.expect('WWW-Authenticate', /algs="ES256 PS256"/);
});

it('iat too old', async function () {
await this.agent.get('/me')
.set('DPoP', await new SignJWT({ htm: 'GET', htu: `${this.provider.issuer}${this.suitePath('/me')}`, ath: this.ath })
.setProtectedHeader({ alg: 'ES256', typ: 'dpop+jwt', jwk: this.jwk })
.setIssuedAt(epochTime() - 301)
.setJti(randomUUID())
.sign(this.keypair.privateKey))
.set('Authorization', `DPoP ${this.access_token}`)
.expect(401)
.expect({ error: 'invalid_dpop_proof', error_description: 'DPoP proof iat is not recent enough' })
.expect('WWW-Authenticate', /^DPoP /)
.expect('WWW-Authenticate', /error="invalid_dpop_proof"/)
.expect('WWW-Authenticate', /algs="ES256 PS256"/);
});
for (const enabled of [true, false]) {
describe(`with DPoP-Nonces ${enabled ? 'enabled' : 'disabled'}`, () => {
before(function () {
({ DPoPNonces: this.DPoPNonces } = i(this.provider));
if (enabled) {
i(this.provider).DPoPNonces = this.DPoPNonces;
} else {
i(this.provider).DPoPNonces = undefined;
}
});

it('iat too in the future', async function () {
await this.agent.get('/me')
.set('DPoP', await new SignJWT({ htm: 'GET', htu: `${this.provider.issuer}${this.suitePath('/me')}`, ath: this.ath })
.setProtectedHeader({ alg: 'ES256', typ: 'dpop+jwt', jwk: this.jwk })
.setIssuedAt(epochTime() + 301)
.setJti(randomUUID())
.sign(this.keypair.privateKey))
.set('Authorization', `DPoP ${this.access_token}`)
.expect(401)
.expect({ error: 'invalid_dpop_proof', error_description: 'DPoP proof iat is not recent enough' })
.expect('WWW-Authenticate', /^DPoP /)
.expect('WWW-Authenticate', /error="invalid_dpop_proof"/)
.expect('WWW-Authenticate', /algs="ES256 PS256"/);
});
after(function () {
i(this.provider).DPoPNonces = this.DPoPNonces;
});

for (const offset of [301, -301]) {
it(`iat too ${offset > 0 ? 'far in the future' : 'old'}`, async function () {
await this.agent.get('/me')
.set('DPoP', await new SignJWT({ htm: 'GET', htu: `${this.provider.issuer}${this.suitePath('/me')}`, ath: this.ath })
.setProtectedHeader({ alg: 'ES256', typ: 'dpop+jwt', jwk: this.jwk })
.setIssuedAt(epochTime() - 301)
.setJti(randomUUID())
.sign(this.keypair.privateKey))
.set('Authorization', `DPoP ${this.access_token}`)
.expect(401)
.expect('WWW-Authenticate', /^DPoP /)
.expect('WWW-Authenticate', /algs="ES256 PS256"/)
.expect('WWW-Authenticate', /DPoP proof iat is not recent enough/)
.expect(({ headers }) => {
if (enabled) {
expect(headers).to.have.property('dpop-nonce').that.matches(/^[\w-]{43}$/);
expect(headers).to.have.property('www-authenticate').that.matches(/error="use_dpop_nonce"/);
expect(headers).to.have.property('www-authenticate').that.matches(/use a DPoP nonce instead/);
} else {
expect(headers).not.to.have.property('dpop-nonce');
expect(headers).to.have.property('www-authenticate').that.matches(/error="invalid_dpop_proof"/);
}
});
});
}
});
}
});

it('acts like an RS checking the DPoP proof and thumbprint now', async function () {
Expand Down Expand Up @@ -982,7 +995,7 @@ describe('features.dPoP', () => {
.set('DPoP', await DPoP(this.keypair, `${this.provider.issuer}${this.suitePath('/request')}`, 'POST'))
.type('form')
.expect(400)
.expect('dpop-nonce', /^[\w-]{43}$/)
.expect('DPoP-Nonce', /^[\w-]{43}$/)
.expect({ error: 'use_dpop_nonce', error_description: 'nonce is required in the DPoP proof' })
.expect(({ headers }) => { nonce = headers['dpop-nonce']; });

Expand Down

0 comments on commit 1b073c0

Please sign in to comment.