Use js_content to respond to challenge requests (#14)

* serve challenge responses with js_content to simplify config
* ensure challenge token is only base64url chars
* remove duplicate content
* fix README.md section that mentioned the deleted config var and outdated location block syntax
* enforce a length limit on the challenge token value; enforce only GET requests
* improve security with a regex match on the acme-challenge location
* update location example in readme

README.md

@@ -25,12 +25,6 @@ You can use environment variables or NGINX configuration variables to control th
         value: Space-separated list of hostnames, e.g. `www1.mydomain.com www2.mydomain.com`\
         default: none (you must specify this!)
 
-### NGINX Variables Only (not allowed as environment variable)
-   - `njs_acme_challenge_dir`\
-        NGINX variable with the path to where store HTTP-01 challenges.\
-        value: Any valid system path writable by the `nginx` user.\
-        default: none (you must specify this!)
-
 ### Optional Variables
    - `NJS_ACME_VERIFY_PROVIDER_HTTPS`\
         Verifies the ACME provider SSL certificate when connecting.\
@@ -47,6 +41,11 @@ You can use environment variables or NGINX configuration variables to control th
         value: Any valid system path writable by the `nginx` user. \
         default: `/etc/nginx/njs-acme/`
 
+   - `NJS_ACME_CHALLENGE_DIR`\
+        Path to store ACME-related challenge responses.\
+        value: Any valid system path writable by the `nginx` user. \
+        default: `${NJS_ACME_DIR}/challenge/`
+
    - `NJS_ACME_ACCOUNT_PRIVATE_JWK`\
         Path to fetch/store the account private JWK.\
         value: Path to the private JWK\
@@ -83,17 +82,15 @@ There are a few pieces that are required to be present in your `nginx.conf` file
 
 ### `server` Section
 * Set the hostname or hostnames (space-separated) to generate the certificate.
+This may also be the environment variable `NJS_ACME_SERVER_NAMES`.
   ```nginx
   set $njs_acme_server_names proxy.nginx.com;
   ```
-* Set your email address to use to configure your ACME account.
+* Set your email address to use to configure your ACME account. This may also
+be the environment variable `NJS_ACME_ACCOUNT_EMAIL`.
   ```nginx
   set $njs_acme_account_email [email protected];
   ```
-* Set the directory to store challenges. This is also used in a `location{}` block below.
-  ```nginx
-  set $njs_acme_challenge_dir /etc/nginx/njs-acme/challenge;
-  ```
 * Set and use variables to hold the certificate and key paths using Javascript.
   ```nginx
   js_set $dynamic_ssl_cert acme.js_cert;
@@ -103,11 +100,10 @@ There are a few pieces that are required to be present in your `nginx.conf` file
   ssl_certificate_key $dynamic_ssl_key;
   ```
 ### `location` Blocks
-* Location to handle ACME challenge requests. `$njs_acme_challenge_dir` is used here.
+* Location to handle ACME challenge requests.
   ```nginx
-  location ^~ /.well-known/acme-challenge/ {
-    default_type "text/plain";
-    root $njs_acme_challenge_dir;
+  location ~ "^/\.well-known/acme-challenge/[-_A-Za-z0-9]{22,128}$" {
+    js_content acme.challengeResponse;
   }
   ```
 * Location, that when requested, inspects the stored certificate (if present) and will request a new certificate if necessary. The included `docker-compose.yml` shows how to use a `healthcheck:` configuration for the NGINX service to periodically request this endpoint.

examples/nginx.conf

@@ -1,5 +1,4 @@
 daemon off;
-#master_process off;
 user nginx;
 
 load_module modules/ngx_http_js_module.so;
@@ -24,7 +23,6 @@ http {
   resolver_timeout 5s;
 
   server {
-
     listen 0.0.0.0:8000; # testing with 8000 should be 80 in prod, pebble usees httpPort in integration-tests/pebble/config.json
     listen 443 ssl http2;
     server_name proxy.nginx.com;
@@ -34,7 +32,6 @@ http {
     ## Mandatory Variables
     set $njs_acme_server_names proxy.nginx.com;
     set $njs_acme_account_email [email protected];
-    set $njs_acme_challenge_dir /etc/nginx/njs-acme/challenge; # must be an nginx variable, not an environment variable
     ## Optional Variables
     #set $njs_acme_dir /etc/nginx/njs-acme;
     #set $njs_acme_account_private_jwk /etc/nginx/njs-acme/account_private_key.json;
@@ -51,16 +48,14 @@ http {
       return 200 "hello server_name:$server_name\nssl_session_id:$ssl_session_id\n";
     }
 
-    location ^~ /.well-known/acme-challenge/ {
-      default_type "text/plain";
-      root $njs_acme_challenge_dir;
+    location ~ "^/\.well-known/acme-challenge/[-_A-Za-z0-9]{22,128}$" {
+      js_content acme.challengeResponse;
     }
 
     location = /acme/auto {
       js_content acme.clientAutoMode;
     }
 
-
     location = /csr/new {
       js_content acme.createCsrHandler;
     }
@@ -69,3 +64,4 @@ http {
       js_content acme.acmeNewAccount;
     }
   }
+}

integration-tests/nginx.conf

@@ -50,9 +50,8 @@ http {
       return 200 "hello server_name:$server_name\nssl_server_name:$ssl_server_name\nssl_session_id:$ssl_session_id\n";
     }
 
-    location ^~ /.well-known/acme-challenge/ {
-      default_type "text/plain";
-      root /etc/nginx/examples/;
+    location ~ "^/\.well-known/acme-challenge/[-_A-Za-z0-9]{22,128}$" {
+      js_content acme.challengeResponse;
     }
 
     location = /check-cert {

src/index.ts

@@ -8,6 +8,7 @@ import {
   getVariable,
   joinPaths,
   acmeDir,
+  acmeChallengeDir,
   acmeAccountPrivateJWKPath,
   acmeDirectoryURI,
   acmeVerifyProviderHTTPS,
@@ -131,25 +132,13 @@ async function clientAutoMode(r: NginxHTTPRequest): Promise<void> {
     fs.writeFileSync(pkeyPath, pkeyPem)
     log.info(`Wrote Private key to ${pkeyPath}`)
 
-    // this is the only variable that has to be set in nginx.conf
-    const challengePath = r.variables.njs_acme_challenge_dir
+    const challengePath = acmeChallengeDir(r)
 
-    if (challengePath === undefined || challengePath.length === 0) {
-      return r.return(
-        500,
-        "Nginx variable 'njs_acme_challenge_dir' must be set"
-      )
-    }
-    log.info('Issuing a new Certificate:', params)
-    const fullChallengePath = joinPaths(
-      challengePath,
-      '.well-known/acme-challenge'
-    )
     try {
-      fs.mkdirSync(fullChallengePath, { recursive: true })
+      fs.mkdirSync(challengePath, { recursive: true })
     } catch (e) {
       log.error(
-        `Error creating directory to store challenges at ${fullChallengePath}. Ensure the ${challengePath} directory is writable by the nginx user.`
+        `Error creating directory to store challenges. Ensure the ${challengePath} directory is writable by the nginx user.`
       )
 
       return r.return(500, 'Cannot create challenge directory')
@@ -164,12 +153,15 @@ async function clientAutoMode(r: NginxHTTPRequest): Promise<void> {
         log.info(
           `Writing challenge file so nginx can serve it via .well-known/acme-challenge/${challenge.token}`
         )
-
-        const path = joinPaths(fullChallengePath, challenge.token)
+        ngx.log(
+          ngx.INFO,
+          `njs-acme: [auto] Writing challenge file so nginx can serve it via ${challengePath}/${challenge.token}`
+        )
+        const path = joinPaths(challengePath, challenge.token)
         fs.writeFileSync(path, keyAuthorization)
       },
       challengeRemoveFn: async (_authz, challenge, _keyAuthorization) => {
-        const path = joinPaths(fullChallengePath, challenge.token)
+        const path = joinPaths(challengePath, challenge.token)
         try {
           fs.unlinkSync(path)
           log.info(`removed challenge ${path}`)
@@ -327,13 +319,61 @@ function read_cert_or_key(prefix: string, domain: string, suffix: string) {
   return { path, data }
 }
 
+/*
+ * Demonstrates using js_content to serve challenge responses.
+ */
+async function challengeResponse(r: NginxHTTPRequest): Promise<void> {
+  const challengeUriPrefix = '/.well-known/acme-challenge/'
+
+  // Only support GET requests
+  if (r.method !== 'GET') {
+    return r.return(400, 'Bad Request')
+  }
+
+  // Here is the challenge token spec:
+  // https://datatracker.ietf.org/doc/html/draft-ietf-acme-acme-07#section-8.3
+  // - greater than 128 bits or ~22 base-64 encoded characters.
+  //   Let's Encrypt uses a 43-character string.
+  // - base64url characters only
+
+  // Ensure we're not given a token that is too long (128 chars to be future-proof)
+  if (r.uri.length > 128 + challengeUriPrefix.length) {
+    return r.return(400, 'Bad Request')
+  }
+
+  // Ensure this handler is only receiving /.well-known/acme-challenge/
+  // requests, and not other requests through some kind of configuration
+  // mistake.
+  if (!r.uri.startsWith(challengeUriPrefix)) {
+    return r.return(400, 'Bad Request')
+  }
+
+  const token = r.uri.substring(challengeUriPrefix.length)
+
+  // Token must only contain base64url chars
+  if (token.match(/[^a-zA-Z0-9-_]/)) {
+    return r.return(400, 'Bad Request')
+  }
+
+  try {
+    return r.return(
+      200,
+      // just return the contents of the token file
+      fs.readFileSync(joinPaths(acmeChallengeDir(r), token), 'utf8')
+    )
+  } catch (e) {
+    return r.return(404, 'Not Found')
+  }
+}
+
 export default {
   js_cert,
   js_key,
   acmeNewAccount,
+  challengeResponse,
   clientNewAccount,
   clientAutoMode,
   createCsrHandler,
   LogLevel,
-  Logger
+  Logger,
 }

src/utils.ts

@@ -765,6 +765,7 @@ export function getVariable(
     | 'njs_acme_account_email'
     | 'njs_acme_server_names'
     | 'njs_acme_dir'
+    | 'njs_acme_challenge_dir'
     | 'njs_acme_account_private_jwk'
     | 'njs_acme_directory_uri'
     | 'njs_acme_verify_provider_https',
@@ -800,6 +801,19 @@ export function acmeDir(r: NginxHTTPRequest): string {
   return getVariable(r, 'njs_acme_dir', '/etc/nginx/njs-acme')
 }
 
+/**
+ * Return the path where ACME challenges are stored
+ * @param r request
+ * @returns configured path or default
+ */
+export function acmeChallengeDir(r: NginxHTTPRequest): string {
+  return getVariable(
+    r,
+    'njs_acme_challenge_dir',
+    joinPaths(acmeDir(r), 'challenge')
+  )
+}
+
 /**
  * Returns the path for the account private JWK
  * @param r {NginxHTTPRequest}