Revert "sm: implement post-CER/CEA TLS upgrade" (#243)

[fiorix]

Summary

• Reverts sm: implement post-CER/CEA TLS upgrade (RFC 6733 §6.2) #243 (commit 64dabe9).

Why

@vingarzan pointed out on #238 that the PR was based on a misreading
of RFC 6733:

• §5.6: TLS handshake SHOULD begin BEFORE any Diameter messages, not
  after CER/CEA. The pre-existing TLS-on-listen path already implements
  this and is the spec-compliant approach.
• §6.2 ("Diameter Header") does not mention TLS. The quote in the
  original PR description was fabricated.
• §1.1.3 deprecates RFC 3588, where post-CER TLS upgrade originated.
• §6.10: Inband-Security-Id in CER/CEA is "NOT RECOMMENDED."

The merged code implements removed/deprecated behavior and should not
have landed. Reopening #238 for further discussion if a legitimate
need surfaces.

Test plan

• go test ./... passes after revert
• All RFC 6733 §5.6 pre-CER TLS paths remain intact

[tridentsx]

Apology and Clarification

First, I want to apologize for the incorrect RFC citation in the original PR. The quote I attributed to RFC 6733 §6.2 was wrong — §6.2 covers "Diameter Answer Processing," not TLS. That was my mistake, likely from confusing RFC 3588 language with RFC 6733, and I should have verified the reference before submitting. I'm sorry for the confusion this caused.

That said, I'd like to revisit the conclusion that post-CER/CEA TLS upgrade is "removed/deprecated behavior that should not have landed," because the RFC tells a more nuanced story.

MUST vs SHOULD vs NOT RECOMMENDED (RFC 2119)

RFC 6733 §1.1.2 states that keywords are interpreted per RFC 2119. That document defines:

• MUST / MUST NOT (§1, §2): "an absolute requirement/prohibition of the specification"
• SHOULD / SHOULD NOT (§3, §4): "there may exist valid reasons in particular circumstances to ignore a particular item, but the full implications must be understood and carefully weighed"
• NOT RECOMMENDED (§4): Equivalent to SHOULD NOT — "there may exist valid reasons in particular circumstances when the particular behavior is acceptable or even useful"

These are not interchangeable. The RFC authors deliberately chose SHOULD over MUST in the relevant sections.

What RFC 6733 actually says (verified quotes)

§5.6 — Peer State Machine (page 69):

For TLS/TCP and DTLS/SCTP usage, a TLS/TCP and DTLS/SCTP handshake SHOULD begin when both ends are in the closed state prior to any Diameter message exchanges.

This is SHOULD, not MUST. The RFC explicitly allows exceptions.

§5.6 — Backward compatibility scenario (same paragraph, page 69):

When connecting to responders that do not conform to this document (i.e., older Diameter implementations that are not prepared to received TLS/TCP and DTLS/SCTP connections in the closed state), the initial TLS/TCP and DTLS/SCTP connection attempt will fail. The initiator MAY then attempt to connect via TCP or SCTP and initiate the TLS/TCP and DTLS/SCTP handshake when both ends are in the open state. If the handshake is successful, all further messages will be sent via TLS/TCP and DTLS/SCTP. If the handshake fails, both ends move to the closed state.

This describes a scenario where TLS is initiated after CER/CEA (i.e., in the open state), but does not itself specify the signaling mechanism.

§13 — Security Considerations (page 140) provides the signaling requirement:

For TLS/TCP and DTLS/SCTP connections to be established in the open state, the CER/CEA exchange MUST include an Inband-Security-ID AVP with a value of TLS/TCP and DTLS/SCTP. The TLS/TCP and DTLS/SCTP handshake will begin when both ends successfully reach the open state, after completion of the CER/CEA exchange. If the TLS/TCP and DTLS/SCTP handshake is successful, all further messages will be sent via TLS/TCP and DTLS/SCTP. If the handshake fails, both ends MUST move to the closed state.

§5.6 describes when you may do it (backward compatibility with RFC 3588 peers). §13 describes how to do it correctly (Inband-Security-Id in CER/CEA, TLS handshake after open state is reached). Together they define the post-CER/CEA TLS upgrade mechanism with MUST-level requirements for correct implementation.

§6.10 — Inband-Security-Id AVP (page 84):

The use of this AVP in CER and CEA messages is NOT RECOMMENDED.

NOT RECOMMENDED, not MUST NOT.

§1.1.3 — Changes from RFC 3588 (page 11):

Deprecated the use of the Inband-Security AVP for negotiating Transport Layer Security (TLS). [...] This new approach augments the existing in-band security negotiation, but it does not completely replace it. The old method is kept for backward compatibility reasons.

The RFC explicitly states the old method is retained.

§2.1 — Transport (page 22):

If the Diameter peer does not support receiving TLS/TCP and DTLS/SCTP connections on port 5658 (i.e., the peer complies only with RFC 3588), then the initiator MAY revert to using TCP or SCTP on port 3868. Note that this scheme is kept only for the purpose of backward compatibility and that there are inherent security vulnerabilities when the initial CER/CEA messages are sent unprotected (see Section 5.6).

MAY, with an acknowledged tradeoff — not a prohibition.

§11.3.13 — IANA Considerations (page 138):

The use of this AVP has been deprecated.

"Deprecated" in IETF context means discouraged for new use, not removed or prohibited.

Important constraint: plain connections only

Reading §2.1 and §5.6 together, the post-CER/CEA TLS upgrade path applies to plain TCP/SCTP connections when interoperating with RFC 3588 peers. Per §2.1, a Diameter node "MUST always be prepared to receive connections on port 3868" and RFC 6733 nodes should also accept on port 5658 for TLS. An RFC 3588 peer only accepts connections on 3868 (plain). The initiator connecting to such a peer establishes a plain TCP/SCTP connection, then uses Inband-Security-Id in CER/CEA to negotiate TLS upgrade. The flow is:

1. Initiator connects to the RFC 3588 peer on port 3868 via plain TCP/SCTP
2. CER/CEA exchange includes Inband-Security-Id=TLS
3. After reaching open state, TLS handshake occurs on the existing connection

The implementation constraint is: only offer/accept Inband-Security-Id=TLS upgrade on connections that are not already TLS-protected. If TLS is already active, the AVP is meaningless. This is a guard the implementation should enforce.

Summary

Claim in revert PR	Actual RFC 6733 text
"TLS handshake SHOULD begin BEFORE any Diameter messages, not after CER/CEA"	Correct — but SHOULD, not MUST (§5.6, p.69)
"§6.2 does not mention TLS"	Correct — my citation was wrong. I apologize.
"§6.10: Inband-Security-Id is NOT RECOMMENDED"	Correct — but NOT RECOMMENDED ≠ MUST NOT per RFC 2119 §4
"§1.1.3 deprecates RFC 3588, where post-CER TLS originated"	Partially correct — but §1.1.3 also says "it does not completely replace it. The old method is kept for backward compatibility reasons."
"Implements removed/deprecated behavior"	Incorrect — §5.6 describes the scenario, §13 provides MUST-level requirements for how to perform it correctly

What I think the right path forward is

I acknowledge the original PR had real problems: the wrong RFC citation, and the implementation issues raised in review. But the feature itself — post-CER/CEA TLS upgrade for backward compatibility with RFC 3588 peers — is defined across §5.6 and §13 of RFC 6733 with normative text describing both when it applies and how to do it correctly.

I'd be happy to resubmit with:

• Correct RFC references (§5.6 for the scenario, §13 for the signaling mechanism, §2.1 for the transport fallback)
• Clear documentation that this is a backward-compatibility mechanism, not the recommended default
• Scoped to plain (non-TLS) connections only — reject Inband-Security-Id=TLS if TLS is already active
• All the implementation fixes from the original review addressed properly

---

All citations verified against the full text of RFC 6733 and RFC 2119.

Also as a clarification may of the PR's I have submitted is due to finding running a compliance suite thats very detailed and has 25 plus years evolution.

[vingarzan]

First I will just admit right away that I didn't fully read all the above, because there are way too many things to follow and my brain hurts while trying to weigh everything in 😝 (I'm getting old...). Pasting all the arguments into my AI-buddy might help, but... I won't...

So I just wanted to say that I made the observation primarily because I implemented TLS and D-TLS in my stack (also Go-lang) and I remembered it starting before CEx, so a simpler just-transport topic. Hence this was a potential incompatibility, which I just wanted to verify against my mental model, as well as for interoperability.

And a few more short points, maybe they help:

• RFC 3588 came out in 2003 (draft ~2002). I don't remember many stacks really implementing TLS back then, so I wonder if there is much benefit in backwards compatibility. But if there is even a single stack which does that and you need to interconnect with it, of course, you must go for it.
• RFC 6733 came out in 2012 - so also pretty old. Should RFC 3588 stacks be upgraded, if they are still around?
• The arguments about starting TLS early, to also protect the CEx, made sense to me.
• IRL, Diameter peer connections are not really dynamic, but it's mostly a manual process, due to security procedures. Neither are there so many implementations. So the diversity of peers is not that high.
• There are enough complications around SCTP support and the many features there to get right in the state machines.
• So I for one prefer smaller and easier to manage code. And drag others into the future, if you need to 😉.

@tridentsx Could you maybe share what that compliance suite is? I would be interested in checking it out a bit, but as you could probably tell, I'm not much for strict compliance, rather would be more practical. Of course, I would also understand if this is some private telecom stuff, which you can't share.

[tridentsx]

For the suite I cant share it proprietary.

The standard though is not ambigous it clearly states in the RFC that should work but is not the preferred way , so the question is what is the goal of this project to be RFC compliant or not ? Is this affecting your application in any way ? Not having it does affect mine !

When it comes to arguments about age of standards we still use ss7 signalling established in early 1980's, diameter is a telco protocol and telocs are extremly conservative and there are more products out there compliant to old 3588 spec than 6733

rfc6733 section 1.1.3

1.1.3. Changes from RFC 3588

This document obsoletes RFC 3588 but is fully backward compatible
with that document. The changes introduced in this document focus on
fixing issues that have surfaced during the implementation of
Diameter (RFC 3588). An overview of some the major changes are given
below.

o Deprecated the use of the Inband-Security AVP for negotiating
Transport Layer Security (TLS) [RFC5246]. It has been generally
considered that bootstrapping of TLS via Inband-Security AVP
creates certain security risks because it does not completely
protect the information carried in the CER/CEA (Capabilities-
Exchange-Request/Capabilities-Exchange-Answer). This version of
Diameter adopts the common approach of defining a well-known
secured port that peers should use when communicating via TLS/TCP
and DTLS/SCTP. This new approach augments the existing in-band
security negotiation, but it does not completely replace it. The
old method is kept for backward compatibility reasons.

[vingarzan]

Like I said: if there is even a single stack which does that and you need to interconnect with it, of course, you must go for it. I'm not against you, was just bringing up the philosophical point about better security and all that blah-blah-blah of "newer" options. And I might be biased, because in my experience with Diameter dating back to 2004, I never saw in-prod RFC3588 stacks using TLS. Which doesn't mean that it wasn't supported or used somewhere maybe.

If it is RFC6733 compliant too, it's not affecting me or anyone else, of course.

Yes, telcos are backwards and cheap in sooo many ways 🙃, including in keeping their SS7 mindbogglingly exploits, such that Diameter still feels modern... Well, at least we don't have to do this in COBOL 🤣!