JAMES-4207 Do not announce capabilities after authentication

[felixauringer]

RFC 5804 states that capabilities must be sent after a SASL security layer was negotiated. This is not the case for any of the authentication mechanisms offered by James.

This reverts parts of this commit and fixes JAMES-4207.

[chibenwa]

No specific objection.

Do we need to backport this fix in 3.9.x and 3.8.x ?

[felixauringer]

The additional announcement has been introduced this year and is not included in any release, I think.

Do you still remember what the initial reason was for announcing capabilities after authentication? Was there some client that expected James to do so?
Regardless, the ABNF in the RFC is quite clear that it should only happen if a SASL security layer has been negotiated.

[chibenwa]

The additional announcement has been introduced this year and is not included in any release, I think.

It was backported to previous release branches I presume, not yet released

Do you still remember what the initial reason was for announcing capabilities after authentication? Was there some client that expected James to do so?

A security report originating from a large mailing company (this is PMC private so I cannot disclose it)

This was not a security issue but more the fact we do not re-advertize capability after STARTTLS

Regardless, the ABNF in the RFC is quite clear that it should only happen if a SASL security layer has been negotiated.

Then fair sorry I may have misunderstood the ABNF or read the RFC too quickly.

[chibenwa]

Once a SASL security layer is established, the server MUST re-issue the capability results, followed by an OK response. 

My interpretation was SASL = AUTH hence the code. I disregarded "security layer"

But it is immediatly followed by

This is necessary to protect against man-in-the-middle attacks that alter the capabilities list prior to SASL negotiation. The capability results MUST include all SASL mechanisms the server was capable of negotiating with that client.

So understanding this as Once a *STARTTLS* security layer is established, the server MUST re-issue the capability results, followed by an OK response. is indeed a better call.

Thanks for catching it.

[felixauringer]

My interpretation was SASL = AUTH hence the code. I disregarded "security layer"

That was mine too, at first. However, the real SASL security layers seem to be very rarely used nowadays. If you search for security layer in SASL RFCs, you'll find that most do not provide a security layer.

RFC 4616 (SASL PLAIN):

The PLAIN SASL mechanism does not provide a security layer.

RFC 7628 (SASL OAUTHBEARER):

SASL mechanisms using this document as their definition do not provide a data security layer; that is, they cannot provide integrity or confidentiality protection for application messages after the initial authentication.

[felixauringer]

It was backported to previous release branches I presume, not yet released

Hm, then it should maybe be backported. My webmail client (Roundcube) couldn't use Managesieve with the master branch. Luckily, it's a really small change.

This was not a security issue but more the fact we do not re-advertize capability after STARTTLS

Makes sense, after STARTTLS, re-advertizing is indeed necessary.

Then fair sorry I may have misunderstood the ABNF or read the RFC too quickly.

No worries, I also overlooked it when first reading it and searched for the error in the Roundcube codebase for a while because I was convinced that your patch fulfilled the RFC.

[chibenwa]

Backports shall be trivial. Are you ok taking a look at it?

[felixauringer]

Backports shall be trivial. Are you ok taking a look at it?

As far as I can see, your original commit was never backported. The branches 3.8.x and 3.9.x do not send managesieve capabilities after STARTTLS.

[felixauringer]

As far as I can see, your original commit was never backported. The branches 3.8.x and 3.9.x do not send managesieve capabilities after STARTTLS.

At least found out during the cherry-pick that you adapted some tests. I have reverted those now, too.