N°9584 - Fix "Unable to connect with STARTTLS" error when sending emails

[Molkobain]

Base information

Question	Answer
Related to a SourceForge thread / Another PR / A GitHub Issue / Combodo ticket?	N°9584
Type of change?	Bug fix

Symptom (bug)

When sending an email from iTop with SMTP transport, the following error occurs if the SMTP server presents a certificate whose CN does not match the configured host:

Unable to connect with STARTTLS: stream_socket_enable_crypto(): Peer certificate CN='some.vertified.domain' did not match expected CN='some.uncertified.domain.alias'

Reproduction procedure (bug)

1. On iTop Community 3.2.3
2. With an Administrator account
3. Set the email configuration to use SMTP transport to an host which is an alias of the real SMTP host, encryption tls, and verify_peer = false
4. Go to the notification test page: Menu Configuration > Notifications > email.test.php
5. Enter a destination email address in the TO field
6. Click Next
7. Observe the error above

Cause (bug)

This issue appeared after the migration from Laminas to Symfony Mailer. The email_transport_smtp.verify_peer configuration parameter (default true) is supposed to allow disabling certificate verification, but has no longer any effect.

The array_key_exists('ssl', $aOptions) condition is the root cause. When using STARTTLS (encryption=tls), the connection starts unencrypted on a plain socket and only upgrades to TLS later (via the STARTTLS SMTP command, see RFC 3207 section 4). At the time getStreamOptions() is called, the 'ssl' key does not yet exist in the stream context, so the condition silently fails and the options are never written.

When PHP later calls stream_socket_enable_crypto() to upgrade the connection, it uses the SSL context options as-is — with peer verification still enabled by default — causing the CN mismatch error.

This worked with Laminas because that library configured SSL options through a dedicated configuration object, independently of whether the connection had already established an SSL context.

Proposed solution (bug and enhancement)

Remove the array_key_exists('ssl', $aOptions) guard so that SSL options are unconditionally written when verify_peer is false, regardless of whether the 'ssl' key is pre-populated which does no harm is the connection is not encrypted:

This ensures the options are present in the stream context before stream_socket_enable_crypto() is called, both for STARTTLS and for implicit TLS (smtps).

Checklist before requesting a review

• I have performed a self-review of my code
• I have tested all changes I made on an iTop instance
• I have added a unit test, otherwise I have explained why I couldn't
• Is the PR clear and detailed enough so anyone can understand without digging in the code?

Checklist before requesting a review

• Check how to make a unit that could ensure we don't have a regression on that

[Molkobain]

@odain-cbd do you have any idea how I could make a unit test for that?

[greptile-apps]

Greptile Summary

This PR fixes the STARTTLS peer-verification bypass regression introduced during the Laminas → Symfony Mailer migration. The root cause was an array_key_exists('ssl', $aOptions) guard that silently skipped writing SSL stream-context options when no 'ssl' key was present (which is always the case with STARTTLS until stream_socket_enable_crypto() is called). The fix removes that guard and extracts the logic into a new protected CreateSmtpTransport() method, accompanied by a comprehensive data-provider unit test.

Confidence Score: 5/5

Safe to merge — the change is a minimal, well-targeted fix with no new surface area and full unit-test coverage.

No P0 or P1 findings. The one-line guard removal is exactly the right fix for the described root cause, the extracted method has a correct PHP return-type declaration so any wrong-transport scenario would surface as a TypeError, and the six-case data-provider test directly covers the regression scenario and all transport modes.

No files require special attention.

Important Files Changed

Filename	Overview
sources/Core/Email/EmailSymfony.php	Extracts SMTP transport creation into CreateSmtpTransport() and removes the array_key_exists('ssl', $aOptions) guard so SSL options are written unconditionally when verify_peer=false, fixing the STARTTLS regression.
tests/php-unit-tests/unitary-tests/sources/core/Email/EmailSymfonyTest.php	Adds six data-provider cases covering STARTTLS, implicit TLS, and plain SMTP with both verify_peer=true and verify_peer=false, directly exercising the regression scenario.

Sequence Diagram

sequenceDiagram
    participant S as SendSynchronous
    participant C as CreateSmtpTransport
    participant T as EsmtpTransport
    participant St as SocketStream
    participant PHP as PHP TLS layer

    S->>C: CreateSmtpTransport(dsn, bVerifyPeer)
    C->>T: Transport::fromDsn(dsn)
    T-->>C: EsmtpTransport
    C->>St: getStream().getStreamOptions()
    St-->>C: aOptions (no 'ssl' key for STARTTLS)
    alt bVerifyPeer = false
        C->>C: aOptions['ssl']['verify_peer'] = false
        C->>C: aOptions['ssl']['verify_peer_name'] = false
        C->>C: aOptions['ssl']['allow_self_signed'] = true
    end
    C->>St: setStreamOptions(aOptions)
    C-->>S: EsmtpTransport (ssl options pre-populated)
    S->>T: send(message) → SMTP handshake
    T->>PHP: stream_socket_enable_crypto()
    Note over PHP: Reads ssl options from stream context (now correctly set before this call)

Loading

_{Reviews (2): Last reviewed commit: "N°9584 - Refactor EMailSymfony transport..." | Re-trigger Greptile}

[Copilot]

Pull request overview

Fixes SMTP STARTTLS connection failures after the Laminas → Symfony Mailer migration by ensuring SSL stream context options are applied even when the connection starts in cleartext (STARTTLS upgrade path).

Changes:

• Removes the array_key_exists('ssl', $aOptions) guard so SSL verification-disabling options are written even when the ssl context doesn’t exist yet (STARTTLS).
• Adds inline documentation explaining why SSL options must be set before stream_socket_enable_crypto() is invoked.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Molkobain]

I think this is irrelevant as it's an ssl option, whereas what your point is an inexisting iTop config parameter

[Molkobain]

@greptileai review again

[Molkobain]

@odain-cbd do you have any idea how I could make a unit test for that?

@odain-cbd I made a unit test but feel free to comment / advise on how we could do it.