diff --git a/packages/core/acme-client/CHANGELOG.md b/packages/core/acme-client/CHANGELOG.md index 6f08cb2f..57d0328c 100644 --- a/packages/core/acme-client/CHANGELOG.md +++ b/packages/core/acme-client/CHANGELOG.md @@ -1,9 +1,10 @@ # Changelog -## v5.4.0 +## v5.4.0 (2024-07-16) * `added` Directory URLs for [Google](https://cloud.google.com/certificate-manager/docs/overview) ACME provider -* `fixed` Invalidate ACME directory cache after 24 hours +* `fixed` Invalidate ACME provider directory cache after 24 hours +* `fixed` Retry HTTP requests on server errors or when rate limited - [#89](https://github.com/publishlab/node-acme-client/issues/89) ## v5.3.1 (2024-05-22) @@ -13,7 +14,7 @@ ## v5.3.0 (2024-02-05) * `added` Support and tests for satisfying `tls-alpn-01` challenges -* `changed` Replace `jsrsasign` with `@peculiar/x509` for certificate and CSR generation and parsing +* `changed` Replace `jsrsasign` with `@peculiar/x509` for certificate and CSR handling * `changed` Method `getChallengeKeyAuthorization()` now returns `$token.$thumbprint` when called with a `tls-alpn-01` challenge * Previously returned base64url encoded SHA256 digest of `$token.$thumbprint` erroneously * This change is not considered breaking since the previous behavior was incorrect diff --git a/packages/core/acme-client/package.json b/packages/core/acme-client/package.json index 1ca90e1b..07414ce5 100644 --- a/packages/core/acme-client/package.json +++ b/packages/core/acme-client/package.json @@ -2,7 +2,7 @@ "name": "acme-client", "description": "Simple and unopinionated ACME client", "author": "nmorsman", - "version": "5.3.1", + "version": "5.4.0", "main": "src/index.js", "types": "types/index.d.ts", "license": "MIT", @@ -15,23 +15,23 @@ "types" ], "dependencies": { - "@peculiar/x509": "^1.10.0", + "@peculiar/x509": "^1.11.0", "asn1js": "^3.0.5", "axios": "^1.7.2", - "debug": "^4.1.1", + "debug": "^4.3.5", "node-forge": "^1.3.1" }, "devDependencies": { - "@types/node": "^20.12.12", + "@types/node": "^20.14.10", "chai": "^4.4.1", "chai-as-promised": "^7.1.2", "eslint": "^8.57.0", "eslint-config-airbnb-base": "^15.0.0", "eslint-plugin-import": "^2.29.1", "jsdoc-to-markdown": "^8.0.1", - "mocha": "^10.4.0", + "mocha": "^10.6.0", "nock": "^13.5.4", - "tsd": "^0.31.0" + "tsd": "^0.31.1" }, "scripts": { "build-docs": "jsdoc2md src/client.js > docs/client.md && jsdoc2md src/crypto/index.js > docs/crypto.md && jsdoc2md src/crypto/forge.js > docs/forge.md", diff --git a/packages/core/acme-client/src/axios.js b/packages/core/acme-client/src/axios.js index e5ea3c66..b62c92b1 100644 --- a/packages/core/acme-client/src/axios.js +++ b/packages/core/acme-client/src/axios.js @@ -3,10 +3,14 @@ */ const axios = require('axios'); +const { parseRetryAfterHeader } = require('./util'); +const { log } = require('./logger'); const pkg = require('./../package.json'); +const { AxiosError } = axios; + /** - * Instance + * Defaults */ const instance = axios.create(); @@ -19,6 +23,9 @@ instance.defaults.acmeSettings = { httpChallengePort: 80, httpsChallengePort: 443, tlsAlpnChallengePort: 443, + + retryMaxAttempts: 5, + retryDefaultDelay: 5, }; /** @@ -30,6 +37,85 @@ instance.defaults.acmeSettings = { instance.defaults.adapter = 'http'; +/** + * Retry requests on server errors or when rate limited + * + * https://datatracker.ietf.org/doc/html/rfc8555#section-6.6 + */ + +function isRetryableError(error) { + return (error.code !== 'ECONNABORTED') + && (error.code !== 'ERR_NOCK_NO_MATCH') + && (!error.response + || (error.response.status === 429) + || ((error.response.status >= 500) && (error.response.status <= 599))); +} + +/* https://github.com/axios/axios/blob/main/lib/core/settle.js */ +function validateStatus(response) { + const validator = response.config.retryValidateStatus; + + if (!response.status || !validator || validator(response.status)) { + return response; + } + + throw new AxiosError( + `Request failed with status code ${response.status}`, + (Math.floor(response.status / 100) === 4) ? AxiosError.ERR_BAD_REQUEST : AxiosError.ERR_BAD_RESPONSE, + response.config, + response.request, + response, + ); +} + +/* Pass all responses through the error interceptor */ +instance.interceptors.request.use((config) => { + if (!('retryValidateStatus' in config)) { + config.retryValidateStatus = config.validateStatus; + } + + config.validateStatus = () => false; + return config; +}); + +/* Handle request retries if applicable */ +instance.interceptors.response.use(null, async (error) => { + const { config, response } = error; + + if (!config) { + return Promise.reject(error); + } + + /* Pick up errors we want to retry */ + if (isRetryableError(error)) { + const { retryMaxAttempts, retryDefaultDelay } = instance.defaults.acmeSettings; + config.retryAttempt = ('retryAttempt' in config) ? (config.retryAttempt + 1) : 1; + + if (config.retryAttempt <= retryMaxAttempts) { + const code = response ? `HTTP ${response.status}` : error.code; + log(`Caught ${code}, retry attempt ${config.retryAttempt}/${retryMaxAttempts} to URL ${config.url}`); + + /* Attempt to parse Retry-After header, fallback to default delay */ + let retryAfter = response ? parseRetryAfterHeader(response.headers['retry-after']) : 0; + + if (retryAfter > 0) { + log(`Found retry-after response header with value: ${response.headers['retry-after']}, waiting ${retryAfter} seconds`); + } + else { + retryAfter = (retryDefaultDelay * config.retryAttempt); + log(`Unable to locate or parse retry-after response header, waiting ${retryAfter} seconds`); + } + + /* Wait and retry the request */ + await new Promise((resolve) => { setTimeout(resolve, (retryAfter * 1000)); }); + return instance(config); + } + } + + /* Validate and return response */ + return validateStatus(response); +}); + /** * Export instance */ diff --git a/packages/core/acme-client/src/http.js b/packages/core/acme-client/src/http.js index cf56d940..0e1b4dcd 100644 --- a/packages/core/acme-client/src/http.js +++ b/packages/core/acme-client/src/http.js @@ -70,9 +70,11 @@ class HttpClient { */ async getDirectory() { - const age = (Math.floor(Date.now() / 1000) - this.directoryTimestamp); + const now = Math.floor(Date.now() / 1000); + const age = (now - this.directoryTimestamp); if (!this.directoryCache || (age > this.directoryMaxAge)) { + log(`Refreshing ACME directory, age: ${age}`); const resp = await this.request(this.directoryUrl, 'get'); if (resp.status >= 400) { @@ -84,6 +86,7 @@ class HttpClient { } this.directoryCache = resp.data; + this.directoryTimestamp = now; } return this.directoryCache; @@ -108,7 +111,7 @@ class HttpClient { * * https://datatracker.ietf.org/doc/html/rfc8555#section-7.2 * - * @returns {Promise} nonce + * @returns {Promise} Nonce */ async getNonce() { diff --git a/packages/core/acme-client/src/util.js b/packages/core/acme-client/src/util.js index be2cdd49..242b47eb 100644 --- a/packages/core/acme-client/src/util.js +++ b/packages/core/acme-client/src/util.js @@ -84,9 +84,12 @@ function retry(fn, { attempts = 5, min = 5000, max = 30000 } = {}) { } /** - * Parse URLs from link header + * Parse URLs from Link header * - * @param {string} header Link header contents + * https://datatracker.ietf.org/doc/html/rfc8555#section-7.4.2 + * https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Link + * + * @param {string} header Header contents * @param {string} rel Link relation, default: `alternate` * @returns {string[]} Array of URLs */ @@ -102,6 +105,37 @@ function parseLinkHeader(header, rel = 'alternate') { return results.filter((r) => r); } +/** + * Parse date or duration from Retry-After header + * + * https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After + * + * @param {string} header Header contents + * @returns {number} Retry duration in seconds + */ + +function parseRetryAfterHeader(header) { + const sec = parseInt(header, 10); + const date = new Date(header); + + /* Seconds into the future */ + if (Number.isSafeInteger(sec) && (sec > 0)) { + return sec; + } + + /* Future date string */ + if (date instanceof Date && !Number.isNaN(date)) { + const now = new Date(); + const diff = Math.ceil((date.getTime() - now.getTime()) / 1000); + + if (diff > 0) { + return diff; + } + } + + return 0; +} + /** * Find certificate chain with preferred issuer common name * - If issuer is found in multiple chains, the closest to root wins @@ -161,14 +195,16 @@ function findCertificateChainForIssuer(chains, issuer) { function formatResponseError(resp) { let result; - if (resp.data.error) { - result = resp.data.error.detail || resp.data.error; - } - else { - result = resp.data.detail || JSON.stringify(resp.data); + if (resp.data) { + if (resp.data.error) { + result = resp.data.error.detail || resp.data.error; + } + else { + result = resp.data.detail || JSON.stringify(resp.data); + } } - return result.replace(/\n/g, ''); + return (result || '').replace(/\n/g, ''); } /** @@ -296,6 +332,7 @@ async function retrieveTlsAlpnCertificate(host, port, timeout = 30000) { module.exports = { retry, parseLinkHeader, + parseRetryAfterHeader, findCertificateChainForIssuer, formatResponseError, getAuthoritativeDnsResolver, diff --git a/packages/core/acme-client/test/10-http.spec.js b/packages/core/acme-client/test/10-http.spec.js index 6967d9ec..743b7b6e 100644 --- a/packages/core/acme-client/test/10-http.spec.js +++ b/packages/core/acme-client/test/10-http.spec.js @@ -12,33 +12,12 @@ const pkg = require('./../package.json'); describe('http', () => { let testClient; + const endpoint = `http://${uuid()}.example.com`; const defaultUserAgent = `node-${pkg.name}/${pkg.version}`; const customUserAgent = 'custom-ua-123'; - const primaryEndpoint = `http://${uuid()}.example.com`; - const defaultUaEndpoint = `http://${uuid()}.example.com`; - const customUaEndpoint = `http://${uuid()}.example.com`; - - /** - * HTTP mocking - */ - - before(() => { - const defaultUaOpts = { reqheaders: { 'User-Agent': defaultUserAgent } }; - const customUaOpts = { reqheaders: { 'User-Agent': customUserAgent } }; - - nock(primaryEndpoint) - .persist().get('/').reply(200, 'ok'); - - nock(defaultUaEndpoint, defaultUaOpts) - .persist().get('/').reply(200, 'ok'); - - nock(customUaEndpoint, customUaOpts) - .persist().get('/').reply(200, 'ok'); - }); - - after(() => { - axios.defaults.headers.common['User-Agent'] = defaultUserAgent; + afterEach(() => { + nock.cleanAll(); }); /** @@ -54,7 +33,8 @@ describe('http', () => { */ it('should http get', async () => { - const resp = await testClient.request(primaryEndpoint, 'get'); + nock(endpoint).get('/').reply(200, 'ok'); + const resp = await testClient.request(endpoint, 'get'); assert.isObject(resp); assert.strictEqual(resp.status, 200); @@ -66,28 +46,76 @@ describe('http', () => { */ it('should request using default user-agent', async () => { - const resp = await testClient.request(defaultUaEndpoint, 'get'); + nock(endpoint).matchHeader('user-agent', defaultUserAgent).get('/').reply(200, 'ok'); + axios.defaults.headers.common['User-Agent'] = defaultUserAgent; + const resp = await testClient.request(endpoint, 'get'); assert.isObject(resp); assert.strictEqual(resp.status, 200); assert.strictEqual(resp.data, 'ok'); }); - it('should not request using custom user-agent', async () => { - await assert.isRejected(testClient.request(customUaEndpoint, 'get')); + it('should reject using custom user-agent', async () => { + nock(endpoint).matchHeader('user-agent', defaultUserAgent).get('/').reply(200, 'ok'); + axios.defaults.headers.common['User-Agent'] = customUserAgent; + await assert.isRejected(testClient.request(endpoint, 'get')); }); it('should request using custom user-agent', async () => { + nock(endpoint).matchHeader('user-agent', customUserAgent).get('/').reply(200, 'ok'); axios.defaults.headers.common['User-Agent'] = customUserAgent; - const resp = await testClient.request(customUaEndpoint, 'get'); + const resp = await testClient.request(endpoint, 'get'); assert.isObject(resp); assert.strictEqual(resp.status, 200); assert.strictEqual(resp.data, 'ok'); }); - it('should not request using default user-agent', async () => { - axios.defaults.headers.common['User-Agent'] = customUserAgent; - await assert.isRejected(testClient.request(defaultUaEndpoint, 'get')); + it('should reject using default user-agent', async () => { + nock(endpoint).matchHeader('user-agent', customUserAgent).get('/').reply(200, 'ok'); + axios.defaults.headers.common['User-Agent'] = defaultUserAgent; + await assert.isRejected(testClient.request(endpoint, 'get')); + }); + + /** + * Retry on HTTP errors + */ + + it('should retry on 429 rate limit', async () => { + let rateLimitCount = 0; + + nock(endpoint).persist().get('/').reply(() => { + rateLimitCount += 1; + + if (rateLimitCount < 3) { + return [429, 'Rate Limit Exceeded', { 'Retry-After': 1 }]; + } + + return [200, 'ok']; + }); + + assert.strictEqual(rateLimitCount, 0); + const resp = await testClient.request(endpoint, 'get'); + + assert.isObject(resp); + assert.strictEqual(resp.status, 200); + assert.strictEqual(resp.data, 'ok'); + assert.strictEqual(rateLimitCount, 3); + }); + + it('should retry on 5xx server error', async () => { + let serverErrorCount = 0; + + nock(endpoint).persist().get('/').reply(() => { + serverErrorCount += 1; + return [500, 'Internal Server Error', { 'Retry-After': 1 }]; + }); + + assert.strictEqual(serverErrorCount, 0); + const resp = await testClient.request(endpoint, 'get'); + + assert.isObject(resp); + assert.strictEqual(resp.status, 500); + assert.strictEqual(serverErrorCount, 4); }); }); diff --git a/packages/core/acme-client/test/10-util.spec.js b/packages/core/acme-client/test/10-util.spec.js new file mode 100644 index 00000000..c9768be8 --- /dev/null +++ b/packages/core/acme-client/test/10-util.spec.js @@ -0,0 +1,145 @@ +/** + * Utility method tests + */ + +const dns = require('dns').promises; +const fs = require('fs').promises; +const path = require('path'); +const { assert } = require('chai'); +const util = require('./../src/util'); +const { readCertificateInfo } = require('./../src/crypto'); + +describe('util', () => { + const testCertPath1 = path.join(__dirname, 'fixtures', 'certificate.crt'); + const testCertPath2 = path.join(__dirname, 'fixtures', 'letsencrypt.crt'); + + it('retry()', async () => { + let attempts = 0; + const backoffOpts = { + min: 100, + max: 500, + }; + + await assert.isRejected(util.retry(() => { + throw new Error('oops'); + }, backoffOpts)); + + const r = await util.retry(() => { + attempts += 1; + + if (attempts < 3) { + throw new Error('oops'); + } + + return 'abc'; + }, backoffOpts); + + assert.strictEqual(r, 'abc'); + assert.strictEqual(attempts, 3); + }); + + it('parseLinkHeader()', () => { + const r1 = util.parseLinkHeader(';rel="alternate"'); + assert.isArray(r1); + assert.strictEqual(r1.length, 1); + assert.strictEqual(r1[0], 'https://example.com/a'); + + const r2 = util.parseLinkHeader(';rel="test"'); + assert.isArray(r2); + assert.strictEqual(r2.length, 0); + + const r3 = util.parseLinkHeader('; rel="test"', 'test'); + assert.isArray(r3); + assert.strictEqual(r3.length, 1); + assert.strictEqual(r3[0], 'http://example.com/c'); + + const r4 = util.parseLinkHeader(`; rel="alternate", + ; rel="nope", + ;rel="alternate", + ; rel="alternate"`); + assert.isArray(r4); + assert.strictEqual(r4.length, 3); + assert.strictEqual(r4[0], 'https://example.com/a'); + assert.strictEqual(r4[1], 'https://example.com/b'); + assert.strictEqual(r4[2], 'https://example.com/c'); + }); + + it('parseRetryAfterHeader()', () => { + const r1 = util.parseRetryAfterHeader(''); + assert.strictEqual(r1, 0); + + const r2 = util.parseRetryAfterHeader('abcdef'); + assert.strictEqual(r2, 0); + + const r3 = util.parseRetryAfterHeader('123'); + assert.strictEqual(r3, 123); + + const r4 = util.parseRetryAfterHeader('123.456'); + assert.strictEqual(r4, 123); + + const r5 = util.parseRetryAfterHeader('-555'); + assert.strictEqual(r5, 0); + + const r6 = util.parseRetryAfterHeader('Wed, 21 Oct 2015 07:28:00 GMT'); + assert.strictEqual(r6, 0); + + const now = new Date(); + const future = new Date(now.getTime() + 123000); + const r7 = util.parseRetryAfterHeader(future.toUTCString()); + assert.isTrue(r7 > 100); + }); + + it('findCertificateChainForIssuer()', async () => { + const certs = [ + (await fs.readFile(testCertPath1)).toString(), + (await fs.readFile(testCertPath2)).toString(), + ]; + + const r1 = util.findCertificateChainForIssuer(certs, 'abc123'); + const r2 = util.findCertificateChainForIssuer(certs, 'example.com'); + const r3 = util.findCertificateChainForIssuer(certs, 'E6'); + + [r1, r2, r3].forEach((r) => { + assert.isString(r); + assert.isNotEmpty(r); + }); + + assert.strictEqual(readCertificateInfo(r1).issuer.commonName, 'example.com'); + assert.strictEqual(readCertificateInfo(r2).issuer.commonName, 'example.com'); + assert.strictEqual(readCertificateInfo(r3).issuer.commonName, 'E6'); + }); + + it('formatResponseError()', () => { + const e1 = util.formatResponseError({ data: { error: 'aaa' } }); + assert.strictEqual(e1, 'aaa'); + + const e2 = util.formatResponseError({ data: { error: { detail: 'bbb' } } }); + assert.strictEqual(e2, 'bbb'); + + const e3 = util.formatResponseError({ data: { detail: 'ccc' } }); + assert.strictEqual(e3, 'ccc'); + + const e4 = util.formatResponseError({ data: { a: 123 } }); + assert.strictEqual(e4, '{"a":123}'); + + const e5 = util.formatResponseError({}); + assert.isString(e5); + assert.isEmpty(e5); + }); + + it('getAuthoritativeDnsResolver()', async () => { + /* valid domain - should not use global default */ + const r1 = await util.getAuthoritativeDnsResolver('example.com'); + assert.instanceOf(r1, dns.Resolver); + assert.isNotEmpty(r1.getServers()); + assert.notDeepEqual(r1.getServers(), dns.getServers()); + + /* invalid domain - fallback to global default */ + const r2 = await util.getAuthoritativeDnsResolver('invalid.xtldx'); + assert.instanceOf(r2, dns.Resolver); + assert.deepStrictEqual(r2.getServers(), dns.getServers()); + }); + + /* TODO: Figure out how to test this */ + it('retrieveTlsAlpnCertificate()'); +}); diff --git a/packages/core/acme-client/test/fixtures/letsencrypt.crt b/packages/core/acme-client/test/fixtures/letsencrypt.crt new file mode 100644 index 00000000..7efd44ca --- /dev/null +++ b/packages/core/acme-client/test/fixtures/letsencrypt.crt @@ -0,0 +1,23 @@ +-----BEGIN CERTIFICATE----- +MIIDzzCCA1WgAwIBAgISA0ghDoSv5DpT3Pd3lqwjbVDDMAoGCCqGSM49BAMDMDIx +CzAJBgNVBAYTAlVTMRYwFAYDVQQKEw1MZXQncyBFbmNyeXB0MQswCQYDVQQDEwJF +NjAeFw0yNDA2MTAxNzEyMjZaFw0yNDA5MDgxNzEyMjVaMBQxEjAQBgNVBAMTCWxl +bmNyLm9yZzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABEHJ3DjN7pYV3mftHzaP +V/WI0RhOJnSI5AIFEPFHDi8UowOINRGIfm9FHGIDqrb4Rmyvr9JrrqBdFGDen8BW +6OGjggJnMIICYzAOBgNVHQ8BAf8EBAMCB4AwHQYDVR0lBBYwFAYIKwYBBQUHAwEG +CCsGAQUFBwMCMAwGA1UdEwEB/wQCMAAwHQYDVR0OBBYEFIdCTnxqmpOELDyzPaEM +seB36lUOMB8GA1UdIwQYMBaAFJMnRpgDqVFojpjWxEJI2yO/WJTSMFUGCCsGAQUF +BwEBBEkwRzAhBggrBgEFBQcwAYYVaHR0cDovL2U2Lm8ubGVuY3Iub3JnMCIGCCsG +AQUFBzAChhZodHRwOi8vZTYuaS5sZW5jci5vcmcvMG8GA1UdEQRoMGaCCWxlbmNy +Lm9yZ4IPbGV0c2VuY3J5cHQuY29tgg9sZXRzZW5jcnlwdC5vcmeCDXd3dy5sZW5j +ci5vcmeCE3d3dy5sZXRzZW5jcnlwdC5jb22CE3d3dy5sZXRzZW5jcnlwdC5vcmcw +EwYDVR0gBAwwCjAIBgZngQwBAgEwggEFBgorBgEEAdZ5AgQCBIH2BIHzAPEAdgA/ +F0tP1yJHWJQdZRyEvg0S7ZA3fx+FauvBvyiF7PhkbgAAAZADWfneAAAEAwBHMEUC +IGlp+dPU2hLT2suTMYkYMlt/xbzSnKLZDA/wYSsPACP7AiEAxbAzx6mkzn0cs0hh +ti6sLf0pcbmDhxHdlJRjuo6SQZEAdwDf4VbrqgWvtZwPhnGNqMAyTq5W2W6n9aVq +AdHBO75SXAAAAZADWfqrAAAEAwBIMEYCIQCrAmDUrlX3oGhri1qCIb65Cuf8h2GR +LC1VfXBenX7dCAIhALXwbhCQ1vO1WLv4CqyihMHOwFaICYqN/N6ylaBlVAM4MAoG +CCqGSM49BAMDA2gAMGUCMFdgjOXGl+hE2ABDsAeuNq8wi34yTMUHk0KMTOjRAfy9 +rOCGQqvP0myoYlyzXOH9uQIxAMdkG1ZWBZS1dHavbPf1I/MjYpzX6gy0jVHIXXu5 +aYWylBi/Uf2RPj0LWFZh8tNa1Q== +-----END CERTIFICATE----- diff --git a/packages/core/acme-client/test/setup.js b/packages/core/acme-client/test/setup.js index 9be01fc0..749a7ec7 100644 --- a/packages/core/acme-client/test/setup.js +++ b/packages/core/acme-client/test/setup.js @@ -29,6 +29,13 @@ if (process.env.ACME_TLSALPN_PORT) { axios.defaults.acmeSettings.tlsAlpnChallengePort = process.env.ACME_TLSALPN_PORT; } +/** + * Greatly reduce retry duration while testing + */ + +axios.defaults.acmeSettings.retryMaxAttempts = 3; +axios.defaults.acmeSettings.retryDefaultDelay = 1; + /** * External account binding */