Bug 2393738 - Review Request: smtprelay - Simple Golang SMTP relay/proxy server
Summary: Review Request: smtprelay - Simple Golang SMTP relay/proxy server
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: 42
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Mikel Olasagasti Uranga
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2025-09-07 16:48 UTC by Neel Chauhan
Modified: 2025-09-09 13:38 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2025-09-09 13:38:01 UTC
Type: ---
Embargoed:
mikel: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 9533171 to 9533176 (1.21 KB, patch)
2025-09-08 05:04 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 9533176 to 9534346 (1.04 KB, patch)
2025-09-08 14:55 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 9534688 to 9534711 (1.39 KB, patch)
2025-09-08 16:57 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 9534688 to 9534711 (1.39 KB, patch)
2025-09-08 17:07 UTC, Fedora Review Service
no flags Details | Diff

Comment 1 Neel Chauhan 2025-09-07 16:57:47 UTC
Koji Build: https://koji.fedoraproject.org/koji/taskinfo?taskID=136871421

Comment 2 Mikel Olasagasti Uranga 2025-09-07 21:18:12 UTC
> %preun
> %systemd_preun caddy.service
> 
> %postun
> %systemd_postun_with_restart caddy.service

`caddy.service` seems a leftover.

Comment 3 Mikel Olasagasti Uranga 2025-09-07 21:31:47 UTC
The spec is correct once the `caddy.service` problem is fixed, but I've a few questions:

> AmbientCapabilities=CAP_NET_ADMIN CAP_NET_BIND_SERVICE

Why `CAP_NET_ADMIN` is required?

> %attr(0644,root,root) %config(noreplace) %{_sysconfdir}/smtprelay.ini

wouldn't it better to be 0600 as the file stores passwords?

Also, why run this as root rather than creating a `smtprelay` user/group?

Comment 5 Fedora Review Service 2025-09-08 05:02:53 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9533171
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2393738-smtprelay/fedora-rawhide-x86_64/09533171-smtprelay/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 6 Fedora Review Service 2025-09-08 05:04:58 UTC
Created attachment 2105950 [details]
The .spec file difference from Copr build 9533171 to 9533176

Comment 7 Fedora Review Service 2025-09-08 05:05:00 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9533176
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2393738-smtprelay/fedora-rawhide-x86_64/09533176-smtprelay/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 8 Mikel Olasagasti Uranga 2025-09-08 11:18:00 UTC
- Add the following after "%global gomodulesmode GO111MODULE=on"

export GO_LDFLAGS="-X main.appVersion=%{version} \
                   -X main.buildTime=$(date -d "@${SOURCE_DATE_EPOCH}" +%Y-%m-%dT%H:%M:%SZ)"

Otherwise `smtprelay --version` doesn't report a correct value.

- (optional) Adjust permissions for smtprelay.ini

From: 
> %attr(0644,root,root) %config(noreplace) %{_sysconfdir}/smtprelay.ini

To:
%attr(0640,root,smtprelay) %config(noreplace) %{_sysconfdir}/smtprelay.ini

- (optional) You can enhance the service with the following params:

CapabilityBoundingSet=CAP_NET_BIND_SERVICE
NoNewPrivileges=yes
ProtectSystem=strict
ProtectHome=yes
PrivateTmp=yes
PrivateDevices=yes
RestrictSUIDSGID=yes
Restart=on-failure
RestartSec=5s

Comment 9 Neel Chauhan 2025-09-08 13:23:47 UTC
Spec URL: https://neelc.org/fedora/smtprelay3/smtprelay.spec
SRPM URL: https://neelc.org/fedora/smtprelay3/smtprelay-1.12.0-1.fc44.src.rpm

I made the changes (again). Please approve/comment.

Comment 10 Fedora Review Service 2025-09-08 14:55:13 UTC
Created attachment 2106001 [details]
The .spec file difference from Copr build 9533176 to 9534346

Comment 11 Fedora Review Service 2025-09-08 14:55:15 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9534346
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2393738-smtprelay/fedora-rawhide-x86_64/09534346-smtprelay/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 12 Mikel Olasagasti Uranga 2025-09-08 15:13:11 UTC
Based on the documentation, the user and group creation requires some extra bits:

https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/#_creation_of_users_and_groups_with_scriptlets

- The %install part is not there:

install -p -D -m 0644 %{SOURCE4} %{buildroot}%{_sysusersdir}/smtprelay.conf

- The %files part is missing:

%{_sysusersdir}/smtprelay.conf

Beware that file needs to end with ".conf".

I think this is the last change required.

Comment 13 Neel Chauhan 2025-09-08 16:14:56 UTC
Spec URL: https://neelc.org/fedora/smtprelay4/smtprelay.spec
SRPM URL: https://neelc.org/fedora/smtprelay4/smtprelay-1.12.0-1.fc44.src.rpm

Is this good? I added the lines.

Comment 14 Maxwell G 2025-09-08 16:16:11 UTC
(Drive by suggestion about the config file. Thanks Mikel for the detailed review!)

The smtprelay.ini is 0644, but I assume it has sensitive data (i.e., mail server credentials) and shouldn't be world readable. I suppose the proper solution would be to make it `%attr(0640,root,smtprelay)`. Also, I see an `allowed_users` option is allowed which could point to another config file. Would it make sense to store smtprelay.ini in /etc/smtprelay instead of the parent /etc directory so, if needed, users could drop that additional file in the same directory?

```
diff --git a/smtprelay.spec b/smtprelay.spec
index 9bb231e..d02022e 100644
--- a/smtprelay.spec
+++ b/smtprelay.spec
@@ -54,11 +54,16 @@ export GO_LDFLAGS="-X main.appVersion=%{version} \
 
 %install
 %go_vendor_license_install -c %{S:2}
+# Binary
 install -m 0755 -vd                             %{buildroot}%{_bindir}
 install -m 0755 -vp %{gobuilddir}/bin/*         %{buildroot}%{_bindir}/
+# Config file
 install -m 0755 -vd                             %{buildroot}%{_sysconfdir}
-install -m 0755 -vp smtprelay.ini %{buildroot}%{_sysconfdir}/
-install -D -p -m 0640 %{S:3} %{buildroot}%{_unitdir}/smtprelay.service
+install -m 0750 -vd                             %{buildroot}%{_sysconfdir}/smtprelay
+install -m 0640 -vp smtprelay.ini %{buildroot}/%{_sysconfdir}/smtprelay
+# Systemd unit and sysusers
+install -D -p -m 0644 %{S:3} %{buildroot}%{_unitdir}/smtprelay.service
+install -D -p -m 0644 %{S:4} %{buildroot}%{_sysusersdir}/smtprelay.conf
 
 %check
 %go_vendor_license_check -c %{S:2}
@@ -82,7 +87,9 @@ install -D -p -m 0640 %{S:3} %{buildroot}%{_unitdir}/smtprelay.service
 %license vendor/modules.txt
 %doc README.md SECURITY.md
 %{_bindir}/smtprelay
-%attr(0644,root,root) %config(noreplace) %{_sysconfdir}/smtprelay.ini
+%dir %attr(0750,root,smtprelay) %config(noreplace) %{_sysconfdir}/smtprelay
+%attr(0640,root,smtprelay) %config(noreplace) %{_sysconfdir}/smtprelay/smtprelay.ini
+%{_sysusersdir}/smtprelay.conf
 %{_unitdir}/smtprelay.service
 
 %changelog
```

Comment 15 Neel Chauhan 2025-09-08 16:28:11 UTC
Spec URL: https://neelc.org/fedora/smtprelay/smtprelay.spec
SRPM URL: https://neelc.org/fedora/smtprelay/smtprelay-1.12.0-1.fc44.src.rpm

Good suggestions, Maxwell!

I have updated my spec file with your suggestions too.

Comment 16 Fedora Review Service 2025-09-08 16:57:57 UTC
Created attachment 2106019 [details]
The .spec file difference from Copr build 9534688 to 9534711

Comment 17 Fedora Review Service 2025-09-08 16:57:59 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9534688
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2393738-smtprelay/fedora-rawhide-x86_64/09534688-smtprelay/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 18 Fedora Review Service 2025-09-08 17:07:58 UTC
Created attachment 2106021 [details]
The .spec file difference from Copr build 9534688 to 9534711

Comment 19 Fedora Review Service 2025-09-08 17:08:01 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9534711
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2393738-smtprelay/fedora-rawhide-x86_64/09534711-smtprelay/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 20 Mikel Olasagasti Uranga 2025-09-09 09:16:04 UTC
Thanks for the suggestion Maxwell and thanks Neel for you patience :)

Golang Package Review
==============

This package was generated using go2rpm and Go Vendor Tools, which simplifies the review.

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated


- [x] The latest version is packaged or packaging an earlier version is justified.
- [x] The License tag reflects the package contents and uses the correct identifiers.
- [x] The package builds successfully in mock.
- [x] Package is installable (checked by fedora-review).
- [x] There are no relevant rpmlint errors.
- [x] The package runs tests in %check.
- [x] `%goipath` is set correctly.
- [x] The package's binaries don't conflict with binaries already in the distribution. (Some Go projects include utility binaries with very generic names)
- [x] There are no `%{_bindir}/*` wildcards in %files. (go2rpm includes these by default)
- [x] The package does not use `%gometa -f` if it has dependents that still build for %ix86.
- [x] The package complies with the Golang and general Packaging Guidelines.

Package approved! On import, don't forget to do the following:

- [ ] Add the package to release-monitoring.org
- [ ] Give go-sig privileges (at least commit) on the package
- [ ] Close the review bug by referencing its ID in the rpm changelog and the Bodhi ticket.
- [ ] Consider configuring Packit service to help with maintenance, you can use https://src.fedoraproject.org/fork/mikelo2/rpms/google-guest-agent/blob/c6d7e14023aa4115e86aaea3f10596c58e1bc81a/f/.packit.yaml as template

Thanks!

Comment 21 Fedora Admin user for bugzilla script actions 2025-09-09 13:20:16 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/smtprelay

Comment 22 Fedora Update System 2025-09-09 13:33:53 UTC
FEDORA-2025-0ea8c734f1 (smtprelay-1.12.0-1.fc44) has been submitted as an update to Fedora 44.
https://bodhi.fedoraproject.org/updates/FEDORA-2025-0ea8c734f1

Comment 23 Fedora Update System 2025-09-09 13:38:01 UTC
FEDORA-2025-0ea8c734f1 (smtprelay-1.12.0-1.fc44) has been pushed to the Fedora 44 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.