BoGo AppDataAfterChangeCipherSpec-DTLS* fails

NEW
Unassigned

Status

NSS
Libraries
P3
normal
a year ago
3 months ago

People

(Reporter: jld, Unassigned)

Tracking

(Blocks: 1 bug)

trunk

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

a year ago
The BoGo tests AppDataAfterChangeCipherSpec-DTLS* fail with NSS returning SSL_ERROR_RX_UNEXPECTED_APPLICATION_DATA.  I looked into it a while ago and left myself this note in config.json: "Is this a protocol violation that Boring tolerates? NSS does not."

Specifically, the TLS versions of the test do expect an error (and get one), but the DTLS versions have this comment in the Go source:

> // BoringSSL's DTLS implementation will drop the out-of-order
> // application data.

So this might be a BoGo/Boring bug, but filing it here in case it's not and to get a bug number.
I think this may actually be a bug in NSS. Is the sequence of events:

CCS [epoch 0]
App Data [epoch 1]
Finished [epoch 1]

If so, then I think that yes, NSS should be special-casing this and dropping the app data rather than generating an alert, because this is something that can happen if you send CCS and Finished in separate packets and then you get re-ordering. Shouldn't be a security issue, though.
Flags: needinfo?(jld)

Updated

a year ago
Flags: needinfo?(martin.thomson)
(Reporter)

Comment 2

a year ago
The sequence of records BoGo sends to NSS, after the initial handshake:

Handshake, epoch 0, sequence 5
      CCS, epoch 0, sequence 6
 App Data, epoch 1, sequence 0
Handshake, epoch 1, sequence 1
 App Data, epoch 1, sequence 2

The next thing is NSS responding with an alert (epoch 1, sequence 1).  I'm just looking at a hex dump so I can't see the encrypted handshake, but I'm assuming that (1,1) is a Finished.

If (1,0) and (1,1) had been reordered by the network, the sequence numbers would be the other way around; I don't know what NSS does in that case.
Flags: needinfo?(jld)
Yeah, this looks like the situation I was expecting. Martin, comments?
I would be perfectly happy for NSS to reject that.  The record clearly appears in sequence before the handshake data, which is not permitted by the spec.

I'd be interested in whether reordering causes this to be accepted or not.

If (1,0) is lost, then I might also be OK with that being rejected, but I suspect we won't be rejecting that.

We really need to write some test cases for this stuff, perhaps:

(1,0), (1,1)
(1,1), (1,0)
(1,1) with (1,0) lost

Note that we can do this with simple rewriting rules now, at least for TLS 1.3.
Flags: needinfo?(martin.thomson)
MT, the RSN isn't relevant here. The handshake doesn't attempt to enforce RSN sequentiality. So I believe that even if the RSNs were "as expected" you would still get rejection.
Wan-Teh, can you give us your opinion of the security impact of this, if any?
Flags: needinfo?(wtc)

Comment 7

a year ago
Eric: I don't think your proposed fix has security concerns.

I suggest you handle this new case in dtls_IsRelevant(), which
should return PR_FALSE.
Flags: needinfo?(wtc)

Updated

11 months ago
Group: crypto-core-security
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.