Skip to content

Feature: Enable self-signed certificates #1973

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.

Sign up for GitHub

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

Open
blitztide wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Feature: Enable self-signed certificates #1973

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a97f16
Feature: Enable self-signed certificates
blitztide Aug 1, 2025
c7cf101
Bugfix: extra checks and removing forced SSL
Sep 6, 2025
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
35 changes: 35 additions & 0 deletions utils/novnc_proxy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ usage() {
echo " Default: 6080 (on all interfaces)"
echo " --vnc VNC_HOST:PORT VNC server host:port proxy target"
echo " Default: localhost:5900"
echo " --self-sign hostname Generate self-signed certificates for hostname"
Copy link
Contributor

@CendioZeijlon CendioZeijlon Sep 5, 2025

Choose a reason for hiding this comment

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

Wouldn't it be better to not force users to provide the hostname argument here? If they care enough, they will probably generate a more detailed certificate.

Copy link
Author

@blitztide blitztide Sep 5, 2025

Choose a reason for hiding this comment

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

What would you suggest for the default value?

Copy link
Author

@blitztide blitztide Sep 6, 2025

Choose a reason for hiding this comment

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

I was using it to point to something behind a reverse proxy, for testing, so hostname is pretty critical for this.
Once fully set up, then a proper SSL would be generated.

Copy link
Contributor

@CendioZeijlon CendioZeijlon Sep 8, 2025

Choose a reason for hiding this comment

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

I would suggest something equally generic to the other properties like CN=noVNC-host or perhaps CN=${HOSTNAME} if that environment variable is set.

I was using it to point to something behind a reverse proxy, for testing, so hostname is pretty critical for this.
Once fully set up, then a proper SSL would be generated.

Why is CN in the certificate important in this case?

echo " Requires OpenSSL to be installed"
echo " --cert CERT Path to combined cert/key file, or just"
echo " the cert file if used with --key"
echo " Default: self.pem"
Expand Down Expand Up @@ -51,6 +53,7 @@ HOST=""
PORT="6080"
LISTEN="$PORT"
VNC_DEST="localhost:5900"
SELF_SIGN=""
CERT=""
KEY=""
WEB=""
Expand Down Expand Up @@ -90,6 +93,7 @@ while [ "$*" ]; do
case $param in
--listen) LISTEN="${OPTARG}"; shift ;;
--vnc) VNC_DEST="${OPTARG}"; shift ;;
--self-sign) SELF_SIGN="${OPTARG}"; shift ;;
--cert) CERT="${OPTARG}"; shift ;;
--key) KEY="${OPTARG}"; shift ;;
--web) WEB="${OPTARG}"; shift ;;
Expand Down Expand Up @@ -128,6 +132,14 @@ if [ -z "${HOST}" ]; then
fi
fi

# Check if (cert | key) & self-sign are set, as they are incompatible
if [ -n "$CERT" ] || [ -n "$KEY" ] && [ -n "$SELF_SIGN" ]; then
echo "Arguments --cert and --key and incompatible with --self-sign"
echo ""
usage
exit 1
fi

trap "cleanup" TERM QUIT INT EXIT

# Find vnc.html
Expand All @@ -147,6 +159,29 @@ else
die "Could not find vnc.html"
fi

# Create self-signed certificates
if [ -n "${SELF_SIGN}" ]; then
# Check if OpenSSL is installed
which openssl > /dev/null
if [ $? != 0 ]; then
echo "Unable to find OpenSSL, please ensure you have OpenSSL installed and available in \$PATH"
exit 1
fi
# Check that the file doesn't already exist
if [ -f $(pwd)/self.pem ]; then
Copy link
Contributor

@CendioZeijlon CendioZeijlon Sep 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

You should probably check for the existence of all files that are generated by your OpenSSL command.

Copy link
Contributor

@CendioZeijlon CendioZeijlon Sep 9, 2025

Choose a reason for hiding this comment

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

It's also allowed to set -keyout self.pem so that both cert and pubkey end up in the same file. Would probably simplify existence checks a bit.

read -p "$(pwd)/self.pem aleady exists, overwrite? (Y/N) " overwrite
if [ "$overwrite" != "Y" ]; then
echo "Not overwriting $(pwd)/self.pem"
exit 1
fi
fi
echo "Generating Certificate for: ${SELF_SIGN}"
openssl req -x509 -newkey rsa:4096 -keyout key.pem -out self.pem -sha256 -days 3650 -nodes -subj "/C=XX/ST=NoVNC/L=NoVNC/O=NoVNC/OU=NoVNC/CN=${SELF_SIGN}"
Copy link
Contributor

@CendioZeijlon CendioZeijlon Sep 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

key.pem should be called self.key since key.pem might as well be the key for a valid certificate.


Comment on lines +166 to +180
Copy link
Contributor

@CendioZeijlon CendioZeijlon Sep 8, 2025

Choose a reason for hiding this comment

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

Wouldn't it be more convenient if --self-sign only created a certificate if one was not found and echoed some info otherwise, Skip generating self-signed certificate. $(pwd)/self.pem and $(pwd)/key.pem already exists ? This way you could just manually delete the old files if you want to regenerate a cert.

Right now, if I e.g. run novnc_proxy --self-sign as a part of a larger script, I would have to manage the input to read on each subsequent run.

I know this maybe reverts parts of what you added here; sorry about that.

Copy link
Author

@blitztide blitztide Sep 8, 2025

Choose a reason for hiding this comment

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

--self-signed is more likely to be used in a test scenario.

If you are running scripts to spawn something, then certificate management will also be used in the higher level script.

Copy link
Contributor

@CendioZeijlon CendioZeijlon Sep 9, 2025

Choose a reason for hiding this comment

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

I hear you, I still think forcing users to make a choice every time they run with --self-sign adds unnecessary inconvenience. It's more likely that they will just want to continue where they left off on subsequent runs.

CERT=$(pwd)/self.pem
KEY=$(pwd)/key.pem
fi

# Find self.pem
if [ -n "${CERT}" ]; then
if [ ! -e "${CERT}" ]; then
Expand Down