(bugfix) Validate Django-managed SSH private keys
Validate uploaded SSH private keys with ssh-keygen before saving them so invalid, malformed, or unsupported key material is rejected in the control panel instead of failing later during rsync. Auto-populate the public key when it is omitted, add an edit flow for existing SSH credentials, and cover create, update, and invalid-key paths with view tests.
This commit is contained in:
@@ -1,5 +1,10 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
|
import subprocess
|
||||||
|
from pathlib import Path
|
||||||
|
from tempfile import TemporaryDirectory
|
||||||
|
|
||||||
from django import forms
|
from django import forms
|
||||||
from django.conf import settings
|
from django.conf import settings
|
||||||
|
|
||||||
@@ -157,6 +162,18 @@ class SshCredentialForm(forms.ModelForm):
|
|||||||
model = SshCredential
|
model = SshCredential
|
||||||
fields = ("name", "private_key", "public_key", "known_hosts", "notes")
|
fields = ("name", "private_key", "public_key", "known_hosts", "notes")
|
||||||
|
|
||||||
|
def clean_private_key(self) -> str:
|
||||||
|
private_key = self.cleaned_data["private_key"].strip()
|
||||||
|
public_key = validate_ssh_private_key(private_key)
|
||||||
|
self.derived_public_key = public_key
|
||||||
|
return f"{private_key}\n"
|
||||||
|
|
||||||
|
def clean(self):
|
||||||
|
cleaned_data = super().clean()
|
||||||
|
if cleaned_data.get("private_key") and not cleaned_data.get("public_key") and hasattr(self, "derived_public_key"):
|
||||||
|
cleaned_data["public_key"] = self.derived_public_key
|
||||||
|
return cleaned_data
|
||||||
|
|
||||||
|
|
||||||
class RetentionApplyForm(forms.Form):
|
class RetentionApplyForm(forms.Form):
|
||||||
kind = forms.ChoiceField(choices=(("scheduled", "Scheduled"), ("manual", "Manual"), ("all", "All")))
|
kind = forms.ChoiceField(choices=(("scheduled", "Scheduled"), ("manual", "Manual"), ("all", "All")))
|
||||||
@@ -201,3 +218,35 @@ class ScheduleConfigForm(forms.ModelForm):
|
|||||||
except ValueError as exc:
|
except ValueError as exc:
|
||||||
raise forms.ValidationError(str(exc)) from exc
|
raise forms.ValidationError(str(exc)) from exc
|
||||||
return cron_expr
|
return cron_expr
|
||||||
|
|
||||||
|
|
||||||
|
def validate_ssh_private_key(private_key: str) -> str:
|
||||||
|
with TemporaryDirectory() as tmp:
|
||||||
|
key_path = Path(tmp) / "identity"
|
||||||
|
key_path.write_text(f"{private_key}\n", encoding="utf-8")
|
||||||
|
os.chmod(key_path, 0o600)
|
||||||
|
try:
|
||||||
|
result = subprocess.run(
|
||||||
|
["ssh-keygen", "-y", "-f", str(key_path)],
|
||||||
|
check=False,
|
||||||
|
stdin=subprocess.DEVNULL,
|
||||||
|
stdout=subprocess.PIPE,
|
||||||
|
stderr=subprocess.PIPE,
|
||||||
|
text=True,
|
||||||
|
timeout=5,
|
||||||
|
)
|
||||||
|
except FileNotFoundError as exc:
|
||||||
|
raise forms.ValidationError("ssh-keygen is not available in this container.") from exc
|
||||||
|
except subprocess.TimeoutExpired as exc:
|
||||||
|
raise forms.ValidationError("Could not validate SSH private key before timeout.") from exc
|
||||||
|
|
||||||
|
if result.returncode != 0:
|
||||||
|
message = result.stderr.strip() or "OpenSSH could not read this private key."
|
||||||
|
if "passphrase" in message.lower():
|
||||||
|
message = "Encrypted SSH private keys are not supported for unattended backups."
|
||||||
|
raise forms.ValidationError(f"Invalid SSH private key: {message}")
|
||||||
|
|
||||||
|
public_key = result.stdout.strip()
|
||||||
|
if not public_key:
|
||||||
|
raise forms.ValidationError("Invalid SSH private key: no public key could be derived.")
|
||||||
|
return public_key
|
||||||
|
|||||||
@@ -1,16 +1,16 @@
|
|||||||
{% extends "pobsync_backend/base.html" %}
|
{% extends "pobsync_backend/base.html" %}
|
||||||
|
|
||||||
{% block title %}New SSH Key | pobsync{% endblock %}
|
{% block title %}{% if credential %}SSH Key | {{ credential.name }}{% else %}New SSH Key{% endif %} | pobsync{% endblock %}
|
||||||
|
|
||||||
{% block content %}
|
{% block content %}
|
||||||
<h1>New SSH Key</h1>
|
<h1>{% if credential %}SSH Key: {{ credential.name }}{% else %}New SSH Key{% endif %}</h1>
|
||||||
|
|
||||||
<section class="actions" aria-label="SSH key form actions">
|
<section class="actions" aria-label="SSH key form actions">
|
||||||
<a class="button-link" href="{% url 'ssh_credentials' %}">Back to SSH keys</a>
|
<a class="button-link" href="{% url 'ssh_credentials' %}">Back to SSH keys</a>
|
||||||
</section>
|
</section>
|
||||||
|
|
||||||
<section class="panel">
|
<section class="panel">
|
||||||
<h2>Create SSH Credential</h2>
|
<h2>{% if credential %}Edit SSH Credential{% else %}Create SSH Credential{% endif %}</h2>
|
||||||
<form method="post" class="form-grid">
|
<form method="post" class="form-grid">
|
||||||
{% csrf_token %}
|
{% csrf_token %}
|
||||||
{{ form.non_field_errors }}
|
{{ form.non_field_errors }}
|
||||||
@@ -25,7 +25,7 @@
|
|||||||
{% endfor %}
|
{% endfor %}
|
||||||
|
|
||||||
<div class="actions">
|
<div class="actions">
|
||||||
<button type="submit">Save SSH key</button>
|
<button type="submit">{% if credential %}Save SSH key{% else %}Create SSH key{% endif %}</button>
|
||||||
</div>
|
</div>
|
||||||
</form>
|
</form>
|
||||||
</section>
|
</section>
|
||||||
|
|||||||
@@ -25,7 +25,7 @@
|
|||||||
<tbody>
|
<tbody>
|
||||||
{% for credential in credentials %}
|
{% for credential in credentials %}
|
||||||
<tr>
|
<tr>
|
||||||
<td>{{ credential.name }}</td>
|
<td><a href="{% url 'edit_ssh_credential' credential.id %}">{{ credential.name }}</a></td>
|
||||||
<td>{{ credential.public_key|yesno:"yes,no" }}</td>
|
<td>{{ credential.public_key|yesno:"yes,no" }}</td>
|
||||||
<td>{{ credential.known_hosts|yesno:"yes,no" }}</td>
|
<td>{{ credential.known_hosts|yesno:"yes,no" }}</td>
|
||||||
<td>{{ credential.hosts.count }}</td>
|
<td>{{ credential.hosts.count }}</td>
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ from __future__ import annotations
|
|||||||
from datetime import datetime, timezone
|
from datetime import datetime, timezone
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from tempfile import TemporaryDirectory
|
from tempfile import TemporaryDirectory
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
from django.contrib.auth import get_user_model
|
from django.contrib.auth import get_user_model
|
||||||
from django.test import TestCase, override_settings
|
from django.test import TestCase, override_settings
|
||||||
@@ -85,24 +86,66 @@ class ViewTests(TestCase):
|
|||||||
def test_ssh_credentials_view_creates_key(self) -> None:
|
def test_ssh_credentials_view_creates_key(self) -> None:
|
||||||
self.client.force_login(self.staff_user)
|
self.client.force_login(self.staff_user)
|
||||||
|
|
||||||
response = self.client.post(
|
with patch("pobsync_backend.forms.validate_ssh_private_key", return_value="DERIVED PUBLIC KEY"):
|
||||||
reverse("create_ssh_credential"),
|
response = self.client.post(
|
||||||
{
|
reverse("create_ssh_credential"),
|
||||||
"name": "backup-key",
|
{
|
||||||
"private_key": "PRIVATE KEY",
|
"name": "backup-key",
|
||||||
"public_key": "PUBLIC KEY",
|
"private_key": "PRIVATE KEY",
|
||||||
"known_hosts": "web-01.example.test ssh-ed25519 AAAATEST",
|
"public_key": "",
|
||||||
"notes": "production backup key",
|
"known_hosts": "web-01.example.test ssh-ed25519 AAAATEST",
|
||||||
},
|
"notes": "production backup key",
|
||||||
follow=True,
|
},
|
||||||
)
|
follow=True,
|
||||||
|
)
|
||||||
|
|
||||||
self.assertRedirects(response, reverse("ssh_credentials"))
|
self.assertRedirects(response, reverse("ssh_credentials"))
|
||||||
self.assertContains(response, "SSH credential saved for backup-key.")
|
self.assertContains(response, "SSH credential saved for backup-key.")
|
||||||
self.assertContains(response, "backup-key")
|
self.assertContains(response, "backup-key")
|
||||||
credential = SshCredential.objects.get(name="backup-key")
|
credential = SshCredential.objects.get(name="backup-key")
|
||||||
self.assertEqual(credential.private_key, "PRIVATE KEY")
|
self.assertEqual(credential.private_key, "PRIVATE KEY\n")
|
||||||
self.assertEqual(credential.public_key, "PUBLIC KEY")
|
self.assertEqual(credential.public_key, "DERIVED PUBLIC KEY")
|
||||||
|
|
||||||
|
def test_ssh_credentials_view_rejects_invalid_key(self) -> None:
|
||||||
|
self.client.force_login(self.staff_user)
|
||||||
|
|
||||||
|
response = self.client.post(
|
||||||
|
reverse("create_ssh_credential"),
|
||||||
|
{
|
||||||
|
"name": "bad-key",
|
||||||
|
"private_key": "not a private key",
|
||||||
|
"public_key": "",
|
||||||
|
"known_hosts": "",
|
||||||
|
"notes": "",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
self.assertEqual(response.status_code, 200)
|
||||||
|
self.assertContains(response, "Invalid SSH private key")
|
||||||
|
self.assertFalse(SshCredential.objects.exists())
|
||||||
|
|
||||||
|
def test_ssh_credentials_view_updates_existing_key(self) -> None:
|
||||||
|
self.client.force_login(self.staff_user)
|
||||||
|
credential = SshCredential.objects.create(name="backup-key", private_key="OLD KEY")
|
||||||
|
|
||||||
|
with patch("pobsync_backend.forms.validate_ssh_private_key", return_value="UPDATED PUBLIC KEY"):
|
||||||
|
response = self.client.post(
|
||||||
|
reverse("edit_ssh_credential", args=[credential.id]),
|
||||||
|
{
|
||||||
|
"name": "backup-key",
|
||||||
|
"private_key": "UPDATED KEY",
|
||||||
|
"public_key": "",
|
||||||
|
"known_hosts": "",
|
||||||
|
"notes": "rotated",
|
||||||
|
},
|
||||||
|
follow=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
self.assertRedirects(response, reverse("ssh_credentials"))
|
||||||
|
credential.refresh_from_db()
|
||||||
|
self.assertEqual(credential.private_key, "UPDATED KEY\n")
|
||||||
|
self.assertEqual(credential.public_key, "UPDATED PUBLIC KEY")
|
||||||
|
self.assertEqual(credential.notes, "rotated")
|
||||||
|
|
||||||
def test_global_config_form_creates_default_config(self) -> None:
|
def test_global_config_form_creates_default_config(self) -> None:
|
||||||
self.client.force_login(self.staff_user)
|
self.client.force_login(self.staff_user)
|
||||||
|
|||||||
@@ -82,6 +82,29 @@ def create_ssh_credential(request):
|
|||||||
"pobsync_backend/ssh_credential_form.html",
|
"pobsync_backend/ssh_credential_form.html",
|
||||||
{
|
{
|
||||||
"form": form,
|
"form": form,
|
||||||
|
"credential": None,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@staff_member_required
|
||||||
|
def edit_ssh_credential(request, credential_id: int):
|
||||||
|
credential = get_object_or_404(SshCredential, id=credential_id)
|
||||||
|
if request.method == "POST":
|
||||||
|
form = SshCredentialForm(request.POST, instance=credential)
|
||||||
|
if form.is_valid():
|
||||||
|
saved_credential = form.save()
|
||||||
|
messages.success(request, f"SSH credential saved for {saved_credential.name}.")
|
||||||
|
return redirect("ssh_credentials")
|
||||||
|
else:
|
||||||
|
form = SshCredentialForm(instance=credential)
|
||||||
|
|
||||||
|
return render(
|
||||||
|
request,
|
||||||
|
"pobsync_backend/ssh_credential_form.html",
|
||||||
|
{
|
||||||
|
"form": form,
|
||||||
|
"credential": credential,
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ urlpatterns = [
|
|||||||
path("config/global/", views.edit_global_config, name="edit_global_config"),
|
path("config/global/", views.edit_global_config, name="edit_global_config"),
|
||||||
path("ssh-credentials/", views.ssh_credentials, name="ssh_credentials"),
|
path("ssh-credentials/", views.ssh_credentials, name="ssh_credentials"),
|
||||||
path("ssh-credentials/new/", views.create_ssh_credential, name="create_ssh_credential"),
|
path("ssh-credentials/new/", views.create_ssh_credential, name="create_ssh_credential"),
|
||||||
|
path("ssh-credentials/<int:credential_id>/", views.edit_ssh_credential, name="edit_ssh_credential"),
|
||||||
path("hosts/new/", views.create_host_config, name="create_host_config"),
|
path("hosts/new/", views.create_host_config, name="create_host_config"),
|
||||||
path("hosts/<str:host>/", views.host_detail, name="host_detail"),
|
path("hosts/<str:host>/", views.host_detail, name="host_detail"),
|
||||||
path("hosts/<str:host>/config/", views.edit_host_config, name="edit_host_config"),
|
path("hosts/<str:host>/config/", views.edit_host_config, name="edit_host_config"),
|
||||||
|
|||||||
Reference in New Issue
Block a user