Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update dependencies and add CSR subject check. #8

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM node:12-alpine AS node-builder
FROM node:16-alpine AS node-builder

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 16? Wouldn't we want to have a more up-to-date version of node?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that node is available in certbot image we use as final one.


WORKDIR /app

Expand All @@ -14,7 +14,7 @@ RUN rm -rf node_modules && yarn install --production



FROM certbot/dns-rfc2136:v1.9.0
FROM certbot/dns-rfc2136:v2.7.1

RUN apk add --update nodejs npm && mkdir -p /usr/app
WORKDIR /usr/app
Expand Down
55 changes: 28 additions & 27 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"name": "dappnode-cert-api",
"version": "1.0.0",
"description": "dAppNode SSL certificate API",
"type": "module",
"repository": {
"type": "git",
"url": "git://github.com/Shard-Labs/dappnode-cert-api.git"
Expand All @@ -17,36 +18,36 @@
"author": "",
"license": "ISC",
"devDependencies": {
"@types/chai": "^4.2.14",
"@types/express": "^4.17.8",
"@types/express-rate-limit": "^5.1.0",
"@types/mocha": "^8.0.4",
"@types/supertest": "^2.0.10",
"@typescript-eslint/eslint-plugin": "^4.14.2",
"@typescript-eslint/parser": "^4.14.2",
"chai": "^4.2.0",
"@types/chai": "^4.3.7",
"@types/express": "^4.17.19",
"@types/express-rate-limit": "^6.0.0",
"@types/mocha": "^10.0.2",
"@types/supertest": "^2.0.14",
"@typescript-eslint/eslint-plugin": "^6.7.5",
"@typescript-eslint/parser": "^6.7.5",
"chai": "^4.3.10",
"csr-gen": "^0.2.1",
"eslint": "^7.19.0",
"mocha": "^8.2.1",
"prettier": "^2.2.1",
"supertest": "^6.0.1",
"ts-node": "^9.0.0",
"tsc": "^1.20150623.0",
"typescript": "^4.0.3"
"eslint": "^8.51.0",
"mocha": "^10.2.0",
"prettier": "^3.0.3",
"supertest": "^6.3.3",
"ts-node": "^10.9.1",
"tsc": "^2.0.4",
"typescript": "^5.2.2"
},
"dependencies": {
"@types/dotenv-defaults": "^2.0.0",
"@types/morgan": "^1.9.2",
"@types/multer": "^1.4.4",
"@types/rimraf": "^3.0.0",
"dotenv-defaults": "^2.0.1",
"ethers": "^5.0.24",
"express": "^4.17.1",
"express-async-handler": "^1.1.4",
"express-rate-limit": "^5.1.3",
"@types/dotenv-defaults": "^2.0.2",
"@types/morgan": "^1.9.6",
"@types/multer": "^1.4.8",
"@types/rimraf": "^4.0.5",
"dotenv-defaults": "^5.0.2",
"ethers": "^6.8.0",
"express": "^4.18.2",
"express-async-handler": "^1.2.0",
"express-rate-limit": "^7.1.1",
"morgan": "^1.10.0",
"multer": "^1.4.2",
"nodemon": "^2.0.6",
"rimraf": "^3.0.2"
"multer": "^1.4.4",
"nodemon": "^3.0.1",
"rimraf": "^5.0.5"
}
}
17 changes: 12 additions & 5 deletions src/app.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
import os from "os";
import fs from "fs";
import path from "path";
import rimraf from "rimraf";
import { rimrafSync } from "rimraf";
import express, { ErrorRequestHandler, Request, Response } from "express";
import asyncHandler from "express-async-handler";
import rateLimit from "express-rate-limit";
import morgan from "morgan";
import multer from "multer";
import config from "./config";
import config from "./config.js";
import {
shell,
HttpError,
BadRequestError,
isHex,
assertValidSignedDappnodeMessage
} from "./utils";
} from "./utils/index.js";

const maxCsrSize = 10e3; // 10 KB;
const renewalTimeThresholdMs = config.renewalTimeThresholdSec * 1000;
Expand All @@ -30,7 +30,7 @@ const upload = multer({
app.use(
rateLimit({
windowMs: parseInt(config.rateLimitWindowMs),
max: config.debug ? 0 : config.rateLimitMax
limit: config.debug ? undefined : config.rateLimitMax
})
);

Expand Down Expand Up @@ -98,7 +98,7 @@ app.post(
});
}

rimraf.sync(certBaseDir);
rimrafSync(certBaseDir);
fs.mkdirSync(certBaseDir, { recursive: true });

// Must attach Certificate Signing Request as "csr" formData
Expand All @@ -109,6 +109,13 @@ app.post(

fs.writeFileSync(csrPath, req.file.buffer);


const csrDomain = await shell(`openssl req -in ${csrPath} -subject -noout | cut -c 31-`);
if (csrDomain != "dyndns.dappnode.io") {
rimrafSync(certBaseDir);
throw new BadRequestError("CSR Subject invalid.");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this properly caught? Will it cause the app to exit?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is caught. You can see that request params were validated in the same way.

}

const command = [
"certbot",
"certonly",
Expand Down
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { app } from "./app";
import { app } from "./app.js";
import { AddressInfo } from "net";

const server = app.listen(5000, "0.0.0.0", () => {
Expand Down
8 changes: 4 additions & 4 deletions src/utils/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export * from "./asyncHandler";
export * from "./format";
export * from "./shell";
export * from "./signedDappnodeMessage";
export * from "./asyncHandler.js";
export * from "./format.js";
export * from "./shell.js";
export * from "./signedDappnodeMessage.js";
2 changes: 1 addition & 1 deletion src/utils/shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export async function shell(
{ timeout, maxBuffer }
);
return stdout.trim();
} catch (e) {
} catch (e: any) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to define any here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDK, compiler was complaining for some reason, that code worked fine before. 😅

// Rethrow a typed error, and ignore the internal NodeJS stack trace
const err: child.ExecException = e;
if (err.signal === "SIGTERM")
Expand Down
6 changes: 3 additions & 3 deletions src/utils/signedDappnodeMessage.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ethers } from "ethers";
import config from "../config";
import config from "../config.js";

export function prepareMessageFromPackage(
packageEnsName: string,
Expand All @@ -24,8 +24,8 @@ export function assertValidSignedDappnodeMessage({
config.signerPackageEnsName,
timestamp.toString()
);
const hash = ethers.utils.solidityKeccak256(["string"], [message]);
const signingAddress = ethers.utils.recoverAddress(hash, signature);
const hash = ethers.solidityPackedKeccak256(["string"], [message]);
const signingAddress = ethers.recoverAddress(hash, signature);

// validate signature
if (signingAddress.toLowerCase() !== address.toLowerCase()) {
Expand Down
10 changes: 5 additions & 5 deletions test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import csrgen from "csr-gen";

describe("app", () => {
let request: SuperAgentTest;
let wallet: ethers.Wallet;
let wallet: ethers.HDNodeWallet;

before("Create random identity", async () => {
request = agent(app);
Expand All @@ -30,16 +30,16 @@ describe("app", () => {
});

function createQueryString(timestamp: number): string {
const signingKey: ethers.utils.SigningKey = new ethers.utils.SigningKey(
const signingKey: ethers.SigningKey = new ethers.SigningKey(
wallet.privateKey
);
const signDigest = signingKey.signDigest.bind(signingKey);
const signDigest = signingKey.sign.bind(signingKey);
const signer = config.signerPackageEnsName;
const hash = ethers.utils.solidityKeccak256(
const hash = ethers.solidityPackedKeccak256(
["string"],
[prepareMessageFromPackage(signer, timestamp.toString())]
);
const signature = ethers.utils.joinSignature(signDigest(hash));
const signature = ethers.Signature.from(signDigest(hash)).serialized;
const params: RequestQueryOptions = {
address: wallet.address,
timestamp: timestamp.toString(),
Expand Down
5 changes: 2 additions & 3 deletions tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
{
"compilerOptions": {
"target": "es6",
"lib": ["es6"],
"module": "commonjs",
"target": "es2022",
"lib": ["es2022"],
"moduleResolution": "node",
"esModuleInterop": true,
"resolveJsonModule": true,
Expand Down
Loading