Skip to content

Commit 3b838c4

Browse files
committed
fix: address code review feedback
- Config file ownership verification - SSL CA validation requirement - Missing Optional import - Payload size check on decoded data - Token ID tracking for failed auth - Clarified error messages Signed-off-by: Scott R. Shinn <scott@atomicorp.com>
1 parent 47bbaaa commit 3b838c4

17 files changed

Lines changed: 703 additions & 24 deletions

chelon-1.0.0.tar.gz

32.3 KB
Binary file not shown.

chelon.spec

Lines changed: 76 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
Name: chelon
22
Version: 1.0.0
33
Release: 2%{?dist}
4-
Summary: Remote GPG package signing service (Chelon)
4+
Summary: Remote GPG package signing service
55

66
License: GPL-2.0-or-later
77
Vendor: Atomicorp, Inc.
@@ -11,15 +11,25 @@ Source0: %{name}-%{version}.tar.gz
1111

1212
BuildArch: noarch
1313

14-
# Runtime dependencies (all from Fedora repos)
14+
%description
15+
Chelon is a secure remote signing service for RPM packages and repository
16+
metadata. Build servers send package hashes to Chelon via HTTPS API and
17+
receive GPG signatures in response, eliminating the need for private keys on
18+
build infrastructure.
19+
20+
This is a meta-package that can install both server and client components.
21+
22+
#
23+
# Server subpackage
24+
#
25+
%package server
26+
Summary: Chelon signing service server
1527
Requires: python3
1628
Requires: python3-flask
1729
Requires: python3-gnupg
1830
Requires: python3-pydantic
1931
Requires: gnupg2
2032
Requires: systemd
21-
22-
# Needed for user/group creation in %pre
2333
Requires(pre): shadow-utils
2434

2535
# Prevent auto-generated requires for user/group (we create them in %pre)
@@ -28,11 +38,24 @@ Requires(pre): shadow-utils
2838
Provides: user(chelon)
2939
Provides: group(chelon)
3040

31-
%description
32-
Chelon is a secure remote signing service for RPM packages and repository
33-
metadata. Build servers send package hashes to Chelon via HTTPS API and
34-
receive GPG signatures in response, eliminating the need for private keys on
35-
build infrastructure.
41+
%description server
42+
Chelon signing service server component. This package contains the signing
43+
service daemon, systemd unit, and admin tools for managing tokens and audit logs.
44+
45+
Install this package on the signing server (e.g., gamera).
46+
47+
#
48+
# Client subpackage
49+
#
50+
%package client
51+
Summary: Chelon signing client tools
52+
Requires: python3
53+
54+
%description client
55+
Chelon signing client tools. This package contains command-line tools for
56+
signing RPM packages and repository metadata using a remote Chelon service.
57+
58+
Install this package on build servers and workstations that need to sign packages.
3659

3760
%prep
3861
%setup -q
@@ -44,6 +67,7 @@ build infrastructure.
4467
# Create directory structure
4568
install -d %{buildroot}%{_bindir}
4669
install -d %{buildroot}%{_datadir}/%{name}/server
70+
install -d %{buildroot}%{_datadir}/%{name}/client
4771
install -d %{buildroot}%{_sysconfdir}/%{name}
4872
install -d %{buildroot}%{_unitdir}
4973
install -d %{buildroot}%{_localstatedir}/lib/%{name}
@@ -54,50 +78,78 @@ install -m 644 server/signing_engine.py %{buildroot}%{_datadir}/%{name}/server/
5478
install -m 644 server/auth.py %{buildroot}%{_datadir}/%{name}/server/
5579
install -m 644 server/audit.py %{buildroot}%{_datadir}/%{name}/server/
5680

57-
# Install CLI tools
81+
# Install server admin tool
5882
install -m 755 tools/chelon-admin %{buildroot}%{_bindir}/
5983

84+
# Install client tools
85+
install -m 755 tools/chelon-sign-rpm %{buildroot}%{_bindir}/
86+
install -m 755 tools/chelon-sign-repomd %{buildroot}%{_bindir}/
87+
install -m 644 tools/chelon_client.py %{buildroot}%{_datadir}/%{name}/client/
88+
6089
# Install systemd unit
6190
install -m 644 systemd/chelon.service %{buildroot}%{_unitdir}/
6291

6392
# Install default config
6493
install -m 600 config/chelon.conf %{buildroot}%{_sysconfdir}/%{name}/
6594

66-
%pre
95+
#
96+
# Server scriptlets
97+
#
98+
%pre server
6799
# Create chelon user if it doesn't exist
68100
getent group chelon >/dev/null || groupadd -r chelon
69101
getent passwd chelon >/dev/null || \
70102
useradd -r -g chelon -d %{_localstatedir}/lib/%{name} -s /sbin/nologin \
71103
-c "Chelon signing service" chelon
72104
exit 0
73105

74-
%post
106+
%post server
75107
%systemd_post chelon.service
76108
# Fix ownership of data directory
77109
chown -R chelon:chelon %{_localstatedir}/lib/%{name} 2>/dev/null || true
78110

79-
%preun
111+
%preun server
80112
%systemd_preun chelon.service
81113

82-
%postun
114+
%postun server
83115
%systemd_postun_with_restart chelon.service
84-
85-
%files
116+
# Only remove user if package is being erased (not upgraded)
117+
if [ $1 -eq 0 ]; then
118+
userdel chelon 2>/dev/null || true
119+
groupdel chelon 2>/dev/null || true
120+
fi
121+
122+
#
123+
# File lists
124+
#
125+
%files server
86126
%doc README.md
87-
%attr(0755, root, root) %{_datadir}/%{name}/
127+
%{_datadir}/%{name}/server/
88128
%{_bindir}/chelon-admin
89129
%{_unitdir}/chelon.service
90-
%config(noreplace) %attr(0600, chelon, chelon) %{_sysconfdir}/%{name}/chelon.conf
91-
%dir %attr(0750, chelon, chelon) %{_localstatedir}/lib/%{name}
92-
%dir %attr(0750, root, chelon) %{_sysconfdir}/%{name}/
130+
%attr(0750,root,chelon) %dir %{_sysconfdir}/%{name}
131+
%attr(0600,chelon,chelon) %config(noreplace) %{_sysconfdir}/%{name}/chelon.conf
132+
%attr(0750,chelon,chelon) %dir %{_localstatedir}/lib/%{name}
133+
134+
%files client
135+
%doc README.md
136+
%{_bindir}/chelon-sign-rpm
137+
%{_bindir}/chelon-sign-repomd
138+
%{_datadir}/%{name}/client/
93139

94140
%changelog
95-
* Wed Jan 07 2026 Atomicorp <support@atomicorp.com> - 1.0.0-2
141+
* Tue Jan 07 2026 Atomicorp <support@atomicorp.com> - 1.0.0-2
142+
- Split into server and client subpackages
143+
- Add client signing tools (chelon-sign-rpm, chelon-sign-repomd)
96144
- Add binary data signing support
97145
- Update HTTP API endpoints and request/response formats
98-
- Introduce new client tools for interacting with the signing service
99-
* Tue Jan 06 2026 Atomicorp <support@atomicorp.com> - 1.0.0-1
100-
- Initial release as Chelon
146+
- Unified logging to journald/syslog
147+
- Enhanced audit logging with request tracing
148+
- Fixed hardcoded admin tool paths
149+
- Code review fixes: config ownership, SSL validation, payload size checks
150+
151+
* Mon Jan 06 2026 Atomicorp <support@atomicorp.com> - 1.0.0-1
152+
- Initial package
101153
- Flask-based HTTP API for remote signing
102154
- Support for Legacy and Modern GPG keys
103155
- Token-based authentication
5.34 KB
Binary file not shown.
9.78 KB
Binary file not shown.
11.1 KB
Binary file not shown.
4.46 KB
Binary file not shown.

tests/gnupg.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
class GPG:
2+
def __init__(self, *args, **kwargs):
3+
pass

tests/test_config.conf

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
DATA_DIR=/tmp/chelon-test-data

tests/test_config_security.py

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
2+
import os
3+
import sys
4+
import unittest
5+
import tempfile
6+
import json
7+
import importlib.util
8+
from pathlib import Path
9+
10+
# Add server to path
11+
sys.path.insert(0, os.path.join(os.path.dirname(__file__), '../server'))
12+
13+
# Mock dependencies to avoid import errors (gnupg, etc)
14+
# We need to mock them BEFORE loading chelon_service
15+
sys.modules['signing_engine'] = MagicMock()
16+
sys.modules['auth'] = MagicMock()
17+
sys.modules['audit'] = MagicMock()
18+
sys.modules['gnupg'] = MagicMock()
19+
20+
class TestConfigSecurity(unittest.TestCase):
21+
22+
def setUp(self):
23+
self.temp_dir = tempfile.mkdtemp()
24+
self.config_path = os.path.join(self.temp_dir, 'chelon.conf')
25+
with open(self.config_path, 'w') as f:
26+
f.write("LEGACY_PASSPHRASE=secret\n")
27+
28+
# Set environment
29+
os.environ['CHELON_CONFIG'] = self.config_path
30+
31+
# Path to service file
32+
self.service_path = os.path.join(os.path.dirname(__file__), '../server/chelon-service.py')
33+
34+
def tearDown(self):
35+
import shutil
36+
shutil.rmtree(self.temp_dir)
37+
# Clean up sys.modules to ensure fresh import each time if needed,
38+
# though we are manually loading it.
39+
if "chelon_service" in sys.modules:
40+
del sys.modules["chelon_service"]
41+
42+
def load_service_module(self):
43+
spec = importlib.util.spec_from_file_location("chelon_service", self.service_path)
44+
module = importlib.util.module_from_spec(spec)
45+
sys.modules["chelon_service"] = module
46+
spec.loader.exec_module(module)
47+
return module
48+
49+
@patch('os.stat')
50+
@patch('sys.exit')
51+
def test_insecure_permissions(self, mock_exit, mock_stat):
52+
# Mock world-readable permissions (0o644 -> world read is 0o004)
53+
mock_stat.return_value.st_mode = 0o100644
54+
55+
# Checking if import triggers exit
56+
try:
57+
self.load_service_module()
58+
except Exception:
59+
pass
60+
61+
# Should call exit(1) during module load
62+
mock_exit.assert_called_with(1)
63+
64+
@patch('os.stat')
65+
@patch('sys.exit')
66+
def test_secure_permissions(self, mock_exit, mock_stat):
67+
# Mock secure permissions (0o600)
68+
mock_stat.return_value.st_mode = 0o100600
69+
70+
module = self.load_service_module()
71+
72+
# Should NOT call exit
73+
mock_exit.assert_not_called()
74+
75+
# Config should be loaded
76+
self.assertTrue(module.config)
77+
78+
@patch('os.stat')
79+
def test_payload_size_limit(self, mock_stat):
80+
# Mock secure permissions so module loads
81+
mock_stat.return_value.st_mode = 0o100600
82+
83+
module = self.load_service_module()
84+
85+
# Mock token auth to bypass authentication
86+
module.token_auth = MagicMock()
87+
module.token_auth.validate_token.return_value = {
88+
'token_id': 'test',
89+
'permissions': ['sign:rpm']
90+
}
91+
92+
# Create request context
93+
app = module.app
94+
with app.test_request_context(
95+
'/api/v1/sign/rpm',
96+
method='POST',
97+
json={'data': 'A' * (10 * 1024 * 1024 + 1)}, # 10MB + 1 byte
98+
headers={'Authorization': 'Bearer test_token'}
99+
):
100+
# The service calls request.get_json(), which we mocked via test_request_context json arg
101+
response, status = module._handle_signing('sign_rpm')
102+
self.assertEqual(status, 413)
103+
self.assertIn('Payload too large', response.json['error'])
104+
105+
if __name__ == '__main__':
106+
unittest.main()

0 commit comments

Comments
 (0)