Code review comment for ~lucaskanashiro/ubuntu/+source/postfix:update-to-version-3.4.11

Revision history for this message
Bryce Harrington (bryce) wrote :

* Changelog:
  - [√] old content and logical tag match as expected
  - [√] changelog entry correct version and targeted codename
  - [√] changelog entries correct
  - [x] update-maintainer has been run

* Actual changes:
  - [x] no upstream changes to consider
  - [√] no further upstream version to consider
  - [√] debian changes look safe

* Old Delta:
  - [√] dropped changes are ok to be dropped
  - [√] nothing else to drop
  - [√] changes forwarded upstream/debian (if appropriate)

* New Delta:
  - [-] no new patches added
  - [√] patches match what was proposed upstream
  - [√] patches correctly included in debian/patches/series
  - [√] patches have correct DEP3 metadata

* Build/Test:
  - [√] build is ok
  - [√] verified PPA package installs/uninstalls
  - [√] autopkgtest against the PPA package passes
  - [√] sanity checks test fine

Don't forget to run update-maintainer

I examined the dropped patch. It looks like some of it still should apply, however it looks like the rationale to include it was purely just to fix a build problem, so I agree if it now builds ok then the need for the patch is gone. So dropping it LGTM.

I didn't attempt to rebuild the package, but I did verify it installs from the PPA into lxc, and I ran the test cases for eoan and focal. The behavior is still a bit different but it appears to work:

root@triage-eoan:/home/bryce# posttls-finger -t30 -T180 -c -L verbose,summary -w smtp.sdeziel.info:465
posttls-finger: initializing the client-side TLS engine
posttls-finger: using DANE RR: _465._tcp.smtp.sdeziel.info -> dane-ta.le-authority-x3.sdeziel.info IN TLSA 2 1 1 60:B8:75:75:44:7D:CB:A2:A3:6B:7D:11:AC:09:FB:24:A9:DB:40:6F:EE:12:D2:CC:90:18:05:17:61:6E:8A:18
posttls-finger: setting up TLS connection to smtp.sdeziel.info[24.212.252.42]:465
posttls-finger: smtp.sdeziel.info[24.212.252.42]:465: TLS cipher list "aNULL:-aNULL:HIGH:MEDIUM:+RC4:@STRENGTH:!aNULL"
posttls-finger: smtp.sdeziel.info[24.212.252.42]:465: depth=1 matched trust anchor public-key sha256 digest=60:B8:75:75:44:7D:CB:A2:A3:6B:7D:11:AC:09:FB:24:A9:DB:40:6F:EE:12:D2:CC:90:18:05:17:61:6E:8A:18
posttls-finger: smtp.sdeziel.info[24.212.252.42]:465: depth=0 chain is trust-anchor signed
posttls-finger: smtp.sdeziel.info[24.212.252.42]:465: depth=0 verify=1 subject=/CN=smtp.sdeziel.info
posttls-finger: smtp.sdeziel.info[24.212.252.42]:465: subjectAltName: imap.sdeziel.info
posttls-finger: smtp.sdeziel.info[24.212.252.42]:465: subjectAltName: mail.sdeziel.info
posttls-finger: smtp.sdeziel.info[24.212.252.42]:465: Matched subjectAltName: smtp.sdeziel.info
posttls-finger: smtp.sdeziel.info[24.212.252.42]:465 CommonName smtp.sdeziel.info
posttls-finger: smtp.sdeziel.info[24.212.252.42]:465: subject_CN=smtp.sdeziel.info, issuer_CN=Let's Encrypt Authority X3, fingerprint=C9:7A:27:B3:13:62:4C:ED:5C:C8:CE:6D:9D:E8:E7:3A:F2:73:AE:9D, pkey_fingerprint=59:B1:2C:D2:78:CD:55:A1:11:F5:D5:AA:DB:87:1E:16:00:EC:52:33
posttls-finger: Verified TLS connection established to smtp.sdeziel.info[24.212.252.42]:465: TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256

And on focal:

root@triage-focal:/home/bryce# posttls-finger -t30 -T180 -c -L verbose,summary -w smtp.sdeziel.info:465
posttls-finger: initializing the client-side TLS engine
posttls-finger: warning: connect to private/tlsmgr: No such file or directory
posttls-finger: warning: connect to private/tlsmgr: No such file or directory
posttls-finger: warning: problem talking to server private/tlsmgr: No such file or directory
posttls-finger: warning: no entropy for TLS key generation: disabling TLS support
posttls-finger: using DANE RR: _465._tcp.smtp.sdeziel.info -> dane-ta.le-authority-x3.sdeziel.info IN TLSA 2 1 1 60:B8:75:75:44:7D:CB:A2:A3:6B:7D:11:AC:09:FB:24:A9:DB:40:6F:EE:12:D2:CC:90:18:05:17:61:6E:8A:18
posttls-finger: warning: lost connection while sending QUIT command

I gather from the bug report that the warnings can be ignored; you might mention that in the test case to avoid confusion. One way to do this might be to show the test case run's output from before installing the fixed package, and again after. The important thing is to highlight the incorrect output and the correct output.

The regression section in the SRU is good, it highlights what to look for if there are regressions, namely DNS troubles.

Since update-maintainer is trivial, and since the SRU text is separate from the package upload, please go ahead and upload after fixing, no need for another round of review.

review: Needs Fixing

« Back to merge proposal