Bug 533258 - Californium/Leshan DTLS PSK identity oracle
Summary: Californium/Leshan DTLS PSK identity oracle
Status: RESOLVED FIXED
Alias: None
Product: Community
Classification: Eclipse Foundation
Component: Vulnerability Reports (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Security vulnerabilitied reported against Eclipse projects CLA
QA Contact:
URL:
Whiteboard:
Keywords: security
Depends on:
Blocks:
 
Reported: 2018-04-05 08:20 EDT by Phoebe Queen CLA
Modified: 2021-09-29 16:46 EDT (History)
7 users (show)

See Also:


Attachments
valid and invalid PSK identity handshake pcaps (2.69 KB, application/x-zip-compressed)
2018-04-05 08:20 EDT, Phoebe Queen CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Phoebe Queen CLA 2018-04-05 08:20:34 EDT
Created attachment 273448 [details]
valid and invalid PSK identity handshake pcaps

There is a discrepancy in behaviour during the DTLS handshake between the use of a valid or an invalid PSK allows an attacker to distinguish valid identities from random guesses.

This allows attackers to use an affected service as an oracle to validate guesses for PSK Identities without prior knowledge of any valid credential pairs, rendering weak configurations particularly prone to brute force attacks more generally.

In the case where an invalid identity is used, the server immediately returns with a Fatal Handshake Failure alert, terminating the handshake protocol.

In the case where a valid identity is used (with a random, invalid PSK), the handshake is not terminated and no alert is returned.

This issue was identified using the (at the time of disclosure) latest version of the Leshan LWM2M demonstration server, configured with the valid user "admin".

Review of the internal dependencies indicated this was built with Californium version 2.0.0-M8

Packet captures of the handshakes have been included for reference.

Duplication of this PoC can be performed using the openssl command line tool.

Invalid case: (note the "SSL alert number 40")
=============

$ openssl s_client -dtls -psk deadbeef -psk_identity InvalidIdentity -connect localhost:5684 -debug
PSK key given, setting client callback
CONNECTED(00000003)
…
write to 0x55a404dd8f20 [0x55a404dea100] (241 bytes => 241 (0xF1))
…
read from 0x55a404dd8f20 [0x55a404de0ec3] (16717 bytes => 60 (0x3C))
…
write to 0x55a404dd8f20 [0x55a404dea100] (273 bytes => 273 (0x111))
…
read from 0x55a404dd8f20 [0x55a404de0ec3] (16717 bytes => 120 (0x78))
…
psk_client_cb
NULL received PSK identity hint, continuing anyway
created identity 'Invalid' len=7
created PSK len=4
write to 0x55a404dd8f20 [0x55a404dea100] (141 bytes => 141 (0x8D))
…
140575335506112:error:14102410:SSL routines:dtls1_read_bytes:sslv3 alert handshake failure:../ssl/record/rec_layer_d1.c:772:SSL alert number 40
---
no peer certificate available
---
No client certificate CA names sent
---
SSL handshake has read 195 bytes and written 655 bytes
Verification: OK
---
New, TLSv1.0, Cipher is PSK-AES128-CBC-SHA256
Secure Renegotiation IS NOT supported
Compression: NONE
Expansion: NONE
No ALPN negotiated
SSL-Session:
    Protocol  : DTLSv1.2
    Cipher    : PSK-AES128-CBC-SHA256
    Session-ID: 5ABAA4572A428074D53BA001337EF3D4470DBA42BB26BA2B9799A03B50947E00
    Session-ID-ctx: 
    Master-Key: 002EC77AD77E7EC45728B3DFF600B60212983F0B13040E9D2E66C132C507A6672EE365E7B79CEBEDC3409C962648B959
    PSK identity: InvalidIdentity
    PSK identity hint: None
    SRP username: None
    Start Time: 1522181207
    Timeout   : 7200 (sec)
    Verify return code: 0 (ok)
    Extended master secret: no
---

Valid Identity case:
====================

$ openssl s_client -dtls -psk deadbeef -psk_identity admin -connect localhost:5684 -debug
PSK key given, setting client callback
CONNECTED(00000003)
…
write to 0x564080b62f20 [0x564080b74100] (241 bytes => 241 (0xF1))
…
read from 0x564080b62f20 [0x564080b6aec3] (16717 bytes => 60 (0x3C))
…
write to 0x564080b62f20 [0x564080b74100] (273 bytes => 273 (0x111))
…
read from 0x564080b62f20 [0x564080b6aec3] (16717 bytes => 120 (0x78))
psk_client_cb
NULL received PSK identity hint, continuing anyway
created identity 'admin' len=5
created PSK len=4
write to 0x564080b62f20 [0x564080b74100] (139 bytes => 139 (0x8B))
…
read from 0x564080b62f20 [0x564080b6aec3] (16717 bytes => -1 (0xFFFFFFFFFFFFFFFF))
…
write to 0x564080b62f20 [0x564080b74100] (32 bytes => 32 (0x20))
0000 - 16 fe fd 00 00 00 00 00-00 00 04 00 13 10 00 00   ................
0010 - 07 00 02 00 00 00 00 00-07 00 05 61 64 6d 69 6e   ...........admin
write to 0x564080b62f20 [0x564080b74100] (14 bytes => 14 (0xE))
…
# Note, handshake hangs here repeatedly attempting to complete.
Comment 1 Wayne Beaton CLA 2018-04-05 10:54:41 EDT
/cc project leads for the referenced projects.
Comment 2 Achim Kraus CLA 2018-04-05 12:40:15 EDT
Thanks!

FMPOV we have two use-cases:
- production, there it should be fixed
- test/sandbox, there we had already the opposite request to send more alerts with details, especially for those, who don't want to setup a leshan/scandium server local.

So I would prefer to make the behaviour configurable with the default not to send the alerts.
Comment 3 Kai Hudalla CLA 2018-04-06 03:24:27 EDT
(In reply to Achim Kraus from comment #2)
> Thanks!
> 
> FMPOV we have two use-cases:
> - production, there it should be fixed

So, my understanding is that scandium should abort the handshake with a "handshake_failure" in both cases, i.e. either if the client uses a PSK identity unknown at the server side or if the client uses a valid PSK identity but provides the wrong secret.

@Simon, Achim: do we agree that this is the expected behavior?


> - test/sandbox, there we had already the opposite request to send more
> alerts with details, especially for those, who don't want to setup a
> leshan/scandium server local.
> 

Are we still talking about handshake failures or are we talking about errors occurring when processing "application" records?

> So I would prefer to make the behaviour configurable with the default not to
> send the alerts.

As noted above, my understanding is that we always send a fatal alert when the handshake fails due to an authentication problem. However, in case of MAC validation problems, we should not send an alert by default, but we could make this configurable for debugging purposes.
Comment 4 Achim Kraus CLA 2018-04-06 04:18:51 EDT
RFC4279, section 2

https://tools.ietf.org/html/rfc4279#section-2

> If the server does not recognize the PSK identity, it MAY respond
> with an "unknown_psk_identity" alert message.  Alternatively, if the
> server wishes to hide the fact that the PSK identity was not known,
> it MAY continue the protocol as if the PSK identity existed but the
> key was incorrect: that is, respond with a "decrypt_error" alert.

From Kai:

> So, my understanding is that scandium should abort the handshake with a
> "handshake_failure" in both cases, i.e. either if the client uses a PSK
> identity unknown at the server side or if the client uses a valid 
> PSK identity but provides the wrong secret.

> @Simon, Achim: do we agree that this is the expected behavior?

I think, the first step would be to change it according the second variant specified by the RFC. But I still think, it would be also good to have a more speaky variant enabled by configuration.

> Are we still talking about handshake failures or
> are we talking about errors occurring when processing "application" records?

Yes, we are still talking about handshake records and NOT "application" records!

> DEBUG [DTLSConnector]: Discarding Handshake (22) record from peer
> [/127.0.0.1:51591]: MAC validation failed  

is reported in the log, if the secret is different.

See also:
https://github.com/eclipse/californium/issues/592#issuecomment-374909093

Just to mention finally:

THIS IS NO VULNERABILITY OF THE SANDBOXES!

The secrets used by the sandboxes are very public :-).
Comment 5 Kai Hudalla CLA 2018-04-06 04:30:31 EDT
(In reply to Achim Kraus from comment #4)
> RFC4279, section 2
> 
> https://tools.ietf.org/html/rfc4279#section-2
> 
> > If the server does not recognize the PSK identity, it MAY respond
> > with an "unknown_psk_identity" alert message.  Alternatively, if the
> > server wishes to hide the fact that the PSK identity was not known,
> > it MAY continue the protocol as if the PSK identity existed but the
> > key was incorrect: that is, respond with a "decrypt_error" alert.
> 
> From Kai:
> 
> > So, my understanding is that scandium should abort the handshake with a
> > "handshake_failure" in both cases, i.e. either if the client uses a PSK
> > identity unknown at the server side or if the client uses a valid 
> > PSK identity but provides the wrong secret.
> 
> > @Simon, Achim: do we agree that this is the expected behavior?
> 
> I think, the first step would be to change it according the second variant
> specified by the RFC. 

Ok, I agree, the default behavior in both cases should be to return a "decrypt_error" fatal alert.

> But I still think, it would be also good to have a
> more speaky variant enabled by configuration.
> 

You mean, returning an "unknown_psk_identity" fatal alert if the client uses an invalid identity and returning a "decrypt_error" if a valid identity but wrong secret is provided, right?

> > Are we still talking about handshake failures or
> > are we talking about errors occurring when processing "application" records?
> 
> Yes, we are still talking about handshake records and NOT "application"
> records!
> 
> > DEBUG [DTLSConnector]: Discarding Handshake (22) record from peer
> > [/127.0.0.1:51591]: MAC validation failed  
> 
> is reported in the log, if the secret is different.
> 
> See also:
> https://github.com/eclipse/californium/issues/592#issuecomment-374909093
> 
> Just to mention finally:
> 
> THIS IS NO VULNERABILITY OF THE SANDBOXES!
> 
> The secrets used by the sandboxes are very public :-).
Comment 6 Achim Kraus CLA 2018-04-06 04:56:00 EDT
> You mean, returning an "unknown_psk_identity" fatal alert if the client
> uses an invalid identity and returning a "decrypt_error" if a valid
> identity but wrong secret is provided, right?

Yes. See discussion in

https://dev.eclipse.org/mhonarc/lists/leshan-dev/msg00913.html
Comment 7 Simon Bernard CLA 2018-04-06 05:52:21 EDT
I agree this could be valuable to have a debug mode which send alert with details.

About production mode : 

Pre-Shared Key Ciphersuites for Transport Layer Security (TLS) says :
(https://tools.ietf.org/html/rfc4279#section-2)

>   If the server does not recognize the PSK identity, it MAY respond
>   with an "unknown_psk_identity" alert message.  Alternatively, if the
>   server wishes to hide the fact that the PSK identity was not known,
>   it MAY continue the protocol as if the PSK identity existed but the
>   key was incorrect: that is, respond with a "decrypt_error" alert.

We want to hide identity is not known.

In TLS this means sending an alert but in DTLS : 
https://tools.ietf.org/html/rfc6347#section-4.1.2.7

>   In general, invalid records
>   SHOULD be silently discarded, thus preserving the association;
>   however, an error MAY be logged for diagnostic purposes.
>   Implementations which choose to generate an alert instead, MUST
>   generate fatal level alerts to avoid attacks where the attacker
>   repeatedly probes the implementation to see how it responds to
>   various types of error.  Note that if DTLS is run over UDP, then any
>   implementation which does this will be extremely susceptible to
>   denial-of-service (DoS) attacks because UDP forgery is so easy.
>   Thus, this practice is NOT RECOMMENDED for such transports.

Unlike TLS, in DTLS instead of sending "decrypt_error" we should discard invalid records.
So, I feel the more secure way is to ignore the message for unknown identity too.


If we choose the other solution : "define as default behavior to return a "decrypt_error" fatal alert in both case". (So sending decrypt error on invalid FINISHED message too, right ?)

We should keep in mind that :
1) We could expose ourself to timing attack as identity unknown could be really faster than decrypt FINISHED message. (So we should try to implement it in a way to avoid that)
2) How do we implement the specific behavior for FINISHED message as we can not know this is a FINISHED message before to decode the encrypted record ? The only information we have is the record type "Handshake (22)". So we could consider to send alert and close the connection for all invalid record with a Handshake(22) type, but this means that just spoofing IP and sending a Handshake(22) record with invalid MAC could close an established session.

(I looked at DTLS 1.3 spec and I don't see any advice which could help to make this choice)
(I could ask the question on tls mailing list if you feel this is necessary ?)
Comment 8 Achim Kraus CLA 2018-04-06 06:32:28 EDT
RFC6347 4.1.2.7 seems to be FMPOV very generic and protects generally against spoofing. But this doesn't really cover the situation, that the handshake expects a special message, which then is delivered with the wrong content.
Anyway, what ever, a mode where the behaviour doesn't differ between unknown id or invalid secret (with or without alert), should be possible.
Comment 9 Kai Hudalla CLA 2018-04-06 07:10:05 EDT
(In reply to Simon Bernard from comment #7)
> I agree this could be valuable to have a debug mode which send alert with
> details.
> 
> About production mode : 
> 
> Pre-Shared Key Ciphersuites for Transport Layer Security (TLS) says :
> (https://tools.ietf.org/html/rfc4279#section-2)
> 
> >   If the server does not recognize the PSK identity, it MAY respond
> >   with an "unknown_psk_identity" alert message.  Alternatively, if the
> >   server wishes to hide the fact that the PSK identity was not known,
> >   it MAY continue the protocol as if the PSK identity existed but the
> >   key was incorrect: that is, respond with a "decrypt_error" alert.
> 
> We want to hide identity is not known.
> 
> In TLS this means sending an alert but in DTLS : 
> https://tools.ietf.org/html/rfc6347#section-4.1.2.7
> 
> >   In general, invalid records
> >   SHOULD be silently discarded, thus preserving the association;
> >   however, an error MAY be logged for diagnostic purposes.
> >   Implementations which choose to generate an alert instead, MUST
> >   generate fatal level alerts to avoid attacks where the attacker
> >   repeatedly probes the implementation to see how it responds to
> >   various types of error.  Note that if DTLS is run over UDP, then any
> >   implementation which does this will be extremely susceptible to
> >   denial-of-service (DoS) attacks because UDP forgery is so easy.
> >   Thus, this practice is NOT RECOMMENDED for such transports.
> 
> Unlike TLS, in DTLS instead of sending "decrypt_error" we should discard
> invalid records.

My understanding of the corresponding section in the DTLS spec is that it specifically suggests to ignore "application" records for which e.g. the MAC validation fails instead of sending alerts. However, this is probably also relevant for the FINISHED message of the handshake.

> So, I feel the more secure way is to ignore the message for unknown identity
> too.
> 

I would still go for failing the handshake with a fatal alert.

> 
> If we choose the other solution : "define as default behavior to return a
> "decrypt_error" fatal alert in both case". (So sending decrypt error on
> invalid FINISHED message too, right ?)
> 
> We should keep in mind that :
> 1) We could expose ourself to timing attack as identity unknown could be
> really faster than decrypt FINISHED message. (So we should try to implement
> it in a way to avoid that)

We could simply continue the handshake with a (randomly) modified version of the identifier and let it fail when the FINISHED message is processed.

> 2) How do we implement the specific behavior for FINISHED message as we can
> not know this is a FINISHED message before to decode the encrypted record ?
> The only information we have is the record type "Handshake (22)". So we
> could consider to send alert and close the connection for all invalid record
> with a Handshake(22) type, but this means that just spoofing IP and sending
> a Handshake(22) record with invalid MAC could close an established session.
> 

No, I think we can simply continue to silently ignore messages for which MAC validation fails. This is a different problem than the one addressed by this issue, isn't it?

> (I looked at DTLS 1.3 spec and I don't see any advice which could help to
> make this choice)
> (I could ask the question on tls mailing list if you feel this is necessary
> ?)
Comment 10 Simon Bernard CLA 2018-04-06 08:55:05 EDT
(In reply to Kai Hudalla from comment #9)

> My understanding of the corresponding section in the DTLS spec is that it specifically suggests to ignore "application" records for which e.g. the MAC validation fails instead of sending alerts. However, this is probably also relevant for the FINISHED message of the handshake.

> I would still go for failing the handshake with a fatal alert.

Maybe I missed something but this means that you want to ignore the RFC recommendation ? Any argument about why you prefer this solution ?

> We could simply continue the handshake with a (randomly) modified version of the identifier and let it fail when the FINISHED message is processed.

Of course this is a way to fix the 1) issue.

> No, I think we can simply continue to silently ignore messages for which MAC validation fails. This is a different problem than the one addressed by this issue, isn't it?

The way we can detect the psk is invalid is when MAC validation of FINISHED message failed. Is there another way ?
Comment 11 Kai Hudalla CLA 2018-04-06 10:13:37 EDT
(In reply to Simon Bernard from comment #10)
> (In reply to Kai Hudalla from comment #9)
> 
> > My understanding of the corresponding section in the DTLS spec is that it specifically suggests to ignore "application" records for which e.g. the MAC validation fails instead of sending alerts. However, this is probably also relevant for the FINISHED message of the handshake.
> 
> > I would still go for failing the handshake with a fatal alert.
> 
> Maybe I missed something but this means that you want to ignore the RFC
> recommendation ? Any argument about why you prefer this solution ?

I still believe that the spec's recommendation to silently ignore records failing MAC validation is targeted at "application" records.

> 
> > We could simply continue the handshake with a (randomly) modified version of the identifier and let it fail when the FINISHED message is processed.
> 
> Of course this is a way to fix the 1) issue.
> 
> > No, I think we can simply continue to silently ignore messages for which MAC validation fails. This is a different problem than the one addressed by this issue, isn't it?
> 
> The way we can detect the psk is invalid is when MAC validation of FINISHED
> message failed. Is there another way ?

You are right, I guess. It has been quite some time since I have been working on this ;-)

> 2) How do we implement the specific behavior for FINISHED message as we can
> not know this is a FINISHED message before to decode the encrypted record ?
> The only information we have is the record type "Handshake (22)". So we
> could consider to send alert and close the connection for all invalid record
> with a Handshake(22) type, but this means that just spoofing IP and sending
> a Handshake(22) record with invalid MAC could close an established session.
>

I am not sure if this is the case. My understanding is that an existing (established) session is only replaced by a newly negotiated session after the handshake has succeeded, right? In that case an attacker would only be able to "abort" an ongoing handshake, but not an established session, or wouldn't it?
Comment 12 Achim Kraus CLA 2018-04-06 10:42:37 EDT
Just my last 2 cents:

ANY solution, which handles the "unknown id" or "invalid secret", with or without alert, in the same way, would be great!

The RFCs too often give more a hint and/or could be interpreted in different ways. So I'm not sure, how long it would take to get answers from the IETF groups. 
So, Simone, if you prefer one of the solutions, go for it. Otherwise, if you prefer to ask the core/tls list, what their intention was, I would also agree on that. The issue seems not to be too time-critical.
Comment 13 Simon Bernard CLA 2018-04-06 10:50:06 EDT
> I still believe that the spec's recommendation to silently ignore records failing MAC validation is targeted at "application" records.

@Kai, You previously said that this is relevant to do this for FINISHED message too. And I find nothing in the spec which say that this is only for record of type APPLICATION_DATA(23) (this is a generic chapter on record layer)

About 2) Only ongoing handshake should be aborted on our side. But we will send a fatal message error to the foreign peer and this will close the connection on its side. (decrypt_error is alway fatal refering to  https://tools.ietf.org/html/rfc5246#section-7.2.2)
Comment 14 Kai Hudalla CLA 2018-04-06 11:24:27 EDT
(In reply to Simon Bernard from comment #13)
> > I still believe that the spec's recommendation to silently ignore records failing MAC validation is targeted at "application" records.
> 
> @Kai, You previously said that this is relevant to do this for FINISHED
> message too. And I find nothing in the spec which say that this is only for
> record of type APPLICATION_DATA(23) (this is a generic chapter on record
> layer)

As I said earlier: this is my understanding of the purpose of the recommendation. I am aware that the spec does not explicitly restrict the advice to application records only. However, FMPOV there is a difference between the two phases "handshake" vs. "application" and during handshake it is desirable IMHO to terminate the handshake (not an established session) explicitly, if the handshake fails. 

> 
> About 2) Only ongoing handshake should be aborted on our side. But we will
> send a fatal message error to the foreign peer and this will close the
> connection on its side. (decrypt_error is alway fatal refering to 
> https://tools.ietf.org/html/rfc5246#section-7.2.2)

I do not believe that the peer will actually accept the alert we send in this case because it will be from the wrong epoch from his point of view, wouldn't it? The peer will be in an established session (epoch 1) whereas the alert we send back as part of the failed handshake with the attacker will still be epoch 0, right?

This is different after the handshake and simply ignoring records with a wrong MAC makes a lot of sense then because sending a fatal alert in this case would indeed trigger the peer to close the connection, I guess.
Comment 15 Simon Bernard CLA 2018-04-06 11:56:47 EDT
> The peer will be in an established session (epoch 1) whereas the alert we send back as part of the failed handshake with the attacker will still be epoch 0, right?


No, we are in a case where we start a renegociation. So we do an handshake in an encrypted session, so epoch is 1. Attacker creates an encrypted record of type HANDSHAKE(22) with invalid MAC. So we will send an encrypted alert  (epoch 1). So the foreign peer would not ignore it.

We can imagine strategy like send alert only if we successfully start a handshake and so ignore invalid MAC of first record of type (HANDSHAKE), but this + the fake identity for timing attack + ignoring rfc recommendation just to help devices with bad credentials during a handshake. This sounds not good to me.
Comment 16 Kai Hudalla CLA 2018-04-06 12:13:16 EDT
(In reply to Simon Bernard from comment #15)
> > The peer will be in an established session (epoch 1) whereas the alert we send back as part of the failed handshake with the attacker will still be epoch 0, right?
> 
> 
> No, we are in a case where we start a renegociation.
> So we do an handshake
> in an encrypted session, so epoch is 1. Attacker creates an encrypted record
> of type HANDSHAKE(22) with invalid MAC. So we will send an encrypted alert 
> (epoch 1). So the foreign peer would not ignore it.
> 

I think the server ignores handshake records if it currently is not in an ongoing handshake with the peer indicated by the source address. So, this attack would only work in a case where a peer is indeed renegotiating a session (which is discouraged by https://tools.ietf.org/html/rfc7925#section-17) with the server. However, the server would only react with a fatal alert if it is expecting the peer's FINISHED message, I guess. If we reject attempts to renegotiate sessions as mandated by rfc7925 then we should be quite safe, shouldn't we?

> We can imagine strategy like send alert only if we successfully start a
> handshake and so ignore invalid MAC of first record of type (HANDSHAKE), but
> this + the fake identity for timing attack + ignoring rfc recommendation
> just to help devices with bad credentials during a handshake. This sounds
> not good to me.
Comment 17 Simon Bernard CLA 2018-04-06 13:02:00 EDT
Removing the ability to renegociate credentials in an existing connection should remove the issue 2)


I'm not againt removing it.
@Achim, your opinion about removing it ?

I still feel that discarding is better : 
- this avoid the fake identity solution (which I'm not even sure if it solve the timing attack issue as generating a random identity is maybe measurable)
- this keep the code simple
- this respect RFC recommendation
- the less you give information the safer you are
- I don't see "sending an alert" for bad credentials as a real benefit. 

But if we remove the renegociation it's maybe acceptable?
Anyway as after 1 day of discussion there is no consensus, waiting I will ask the question on the TLS mailing list.
Comment 18 Simon Bernard CLA 2018-04-06 17:09:15 EDT
Link to the tls mailing list discussion : 
http://ietf.10.n7.nabble.com/DTLS-how-to-handle-unknown-identity-and-bad-psk-td561060.html
Comment 19 Kai Hudalla CLA 2018-04-09 04:23:50 EDT
(In reply to Simon Bernard from comment #17)
> Removing the ability to renegociate credentials in an existing connection
> should remove the issue 2)
> 
> 
> I'm not againt removing it.
> @Achim, your opinion about removing it ?
> 
> I still feel that discarding is better : 
> - this avoid the fake identity solution (which I'm not even sure if it solve
> the timing attack issue as generating a random identity is maybe measurable)
> - this keep the code simple
> - this respect RFC recommendation
> - the less you give information the safer you are
> - I don't see "sending an alert" for bad credentials as a real benefit. 
> 
> But if we remove the renegociation it's maybe acceptable?
> Anyway as after 1 day of discussion there is no consensus, waiting I will
> ask the question on the TLS mailing list.

It seems that the first reply supports your point of view, Simon :-)
I was not aware of the fact that it seems to be generally acceptable for the DTLS handshake to simply stall, i.e. time out, instead of "properly bringing it to an end". I do agree that following this strategy (silently ignoring a record containing an unknown PSK identity) will be much easier to implement. I still think that we should prevent re-negotiation in any case (not the least because recognizing it and handling it properly has led to a lot of ugly code, IIRC).
Comment 20 Achim Kraus CLA 2018-04-09 04:38:04 EDT
Though the reported issue seems to be very easy, 
"process wrong id or secret in that same way",
this discussion seems for me to be far out of scope.

> No, we are in a case where we start a renegociation. So we do an handshake in 
> an encrypted session, so epoch is 1. Attacker creates an encrypted record of
> type HANDSHAKE(22) with invalid MAC. So we will send an encrypted alert
> (epoch 1). So the foreign peer would not ignore it.

This will only be an issue, if the attacker is able to attack exactly at that time, when the dtls server waits for the FINISHED in epoch 1 (for the first handshake). Otherwise, if the dtls server has already successful processed the FINISHED and receives an HANDSHAKE (or any other epoch 1 traffic) with a invalid MAC, the implementation should just apply 

> 4.1.2.7. Handling Invalid Records
> Unlike TLS, DTLS is resilient in the face of invalid records (e.g.,
> invalid formatting, length, MAC, etc.). In general, invalid records
> SHOULD be silently discarded, thus preserving the association;

so discard the record, if not waiting for FINISH epoch 1, and preserve the current association. But this is seems to be no conflict with reporting or not reporting alerts.
Comment 21 Simon Bernard CLA 2018-04-09 05:29:20 EDT
> this discussion seems for me to be far out of scope.

We agree about  "process wrong id or secret in that same way".
There is 2 way to achieve that. As Kai and me didn't agree about the way to choose, we have a discussion which led us to this kind of problem. This seems not out of scope.

Anyway, as silently discarding seems :
- easier/cleaner to implement.
- adviced by one of the author of the DTLS specification.

I will go for this implementation in a first time. (with an option for explicit alert for test/sandbox use case)

If the needs to have alert for bad PSK credentials arises, we could consider to implement it as an improvement.

About removing renegociation, I opened an issue : https://github.com/eclipse/californium/issues/604
Comment 22 Phoebe Queen CLA 2018-04-09 17:51:13 EDT
Hi,

Not sure if appropriate to comment here following up on my own report, but in case it helps:

From my pov as a security tester type person, silently discarding malformed records (so long as you never respond to a partially well-formed record) is the most reliable way to avoid a side channel. There's nothing for me to time or measure a discrepancy against, and there's an inherent reduction of things implementers can get wrong.

Also, with the fact that UDP can trivially be spoofed from any location, it's entirely feasible (and at least somewhat accounted for within DTLS' attack model) that an attacker on the side could send invalid records imitating the original connection during a valid DTLS handshake in order to derail connection attempts if the server returns Fatal to invalid records -- simply discarding bad records and accepting valid records in accordance with the protocol prevents this "man on the side" type of DOS.
Comment 23 Kai Hudalla CLA 2018-04-10 02:43:54 EDT
(In reply to Phoebe Queen from comment #22)
> Hi,
> 
> Not sure if appropriate to comment here following up on my own report, but
> in case it helps:
> 

Oh, it helps a lot, Pheobe :-) Since we are not the security experts (we seem to mainly try to follow relevant RFCs and combine that with some common sense), any hints from people more knowledgeable than us is very welcome.
 
> From my pov as a security tester type person, silently discarding malformed
> records (so long as you never respond to a partially well-formed record) is
> the most reliable way to avoid a side channel. There's nothing for me to
> time or measure a discrepancy against, and there's an inherent reduction of
> things implementers can get wrong.
> 

That makes sense and I have to admit that during this discussion I have changed my mind from my original position to what Simon has suggested from the beginning.

> Also, with the fact that UDP can trivially be spoofed from any location,
> it's entirely feasible (and at least somewhat accounted for within DTLS'
> attack model) that an attacker on the side could send invalid records
> imitating the original connection during a valid DTLS handshake in order to
> derail connection attempts if the server returns Fatal to invalid records --
> simply discarding bad records and accepting valid records in accordance with
> the protocol prevents this "man on the side" type of DOS.

So, Simon, you have my full support for silently ignoring all malformed records, regardless of type :-) It also seems that we agree that we want to remove support for re-negotiation [1]. Thanks for taking us through this discourse, Simon. I really appreciate it!

[1] https://github.com/eclipse/californium/issues/604
Comment 24 Simon Bernard CLA 2018-04-12 06:12:53 EDT
It should be fixed in master : https://github.com/eclipse/californium/pull/605
Comment 25 Kai Hudalla CLA 2018-04-12 06:42:35 EDT
(In reply to Simon Bernard from comment #24)
> It should be fixed in master :
> https://github.com/eclipse/californium/pull/605

Thanks, Simon :-)

According to the Eclipse Security Policy [1] we are supposed to disclose the "vulnerability" at some point in time. My understanding is that it would be sufficient to disclose it as part of the release notes of a next milestone, WDYT?

[1] https://eclipse.org/security/policy.php
Comment 26 Simon Bernard CLA 2018-04-12 08:13:57 EDT
It sounds good to me.

This make me think : Should we backport this on the 1.0.x branch ?
Comment 27 Kai Hudalla CLA 2018-04-12 08:20:42 EDT
(In reply to Simon Bernard from comment #26)
> It sounds good to me.
> 
> This make me think : Should we backport this on the 1.0.x branch ?

FMPOV: yes, absolutely. The 1.0.x version is in use in the field already, so we should definitely provide a bug fix release for this.
Comment 28 Simon Bernard CLA 2018-04-13 10:55:47 EDT
I backported the fix for 1.0.x : https://github.com/eclipse/californium/pull/611
Comment 29 Wayne Beaton CLA 2018-04-13 13:43:26 EDT
Does this require a CVE?
Comment 30 Kai Hudalla CLA 2018-04-16 03:43:37 EDT
(In reply to Wayne Beaton from comment #29)
> Does this require a CVE?

That's a good question. I do not have any experience with issuing CVEs so I have no clue, under what circumstances a CVE would be required. Maybe you can shed some light on it, Wayne? Or maybe you can point us to a resource providing some guidelines?
Comment 31 Phoebe Queen CLA 2018-04-26 04:52:00 EDT
I'd argue a CVE identifier would probably allow easier referencing for this vulnerability for instances where this framework is in use, and may raise some awareness of safer configuration in production. I also believe that this issue probably meets all of the CVE inclusion criteria.

A table with a decision tree for inclusion in CVE is linked below

https://cve.mitre.org/cve/editorial_policies/counting_rules.html
Comment 32 Wayne Beaton CLA 2018-04-26 11:44:11 EDT
(In reply to Kai Hudalla from comment #30)
> That's a good question. I do not have any experience with issuing CVEs so I
> have no clue, under what circumstances a CVE would be required. Maybe you
> can shed some light on it, Wayne? Or maybe you can point us to a resource
> providing some guidelines?

Sorry for the delay. I haven't included this in our documentation yet. Bug 496426 Comment 2 describes the process.

The short version is that it's between you and your PMC to sort it out. Jens has experience here.

> A table with a decision tree for inclusion in CVE is linked below
> 
> https://cve.mitre.org/cve/editorial_policies/counting_rules.html

Good advice. I'll add this to the (future) documentation.
Comment 33 Wayne Beaton CLA 2019-05-09 13:08:52 EDT
I've added some content to the handbook to describe the process. 

We're well past time to disclose this issue; do you want a CVE?

https://www.eclipse.org/projects/handbook/#vulnerability
Comment 34 Wayne Beaton CLA 2019-12-18 21:26:46 EST
The Eclipse Foundation's policy requires the disclosure of vulnerabilities after 90 days. I've removed the committers-only flag.
Comment 35 Wayne Beaton CLA 2021-09-20 16:15:17 EDT
Are we done here?

Have you decided whether or not a CVE is required?
Comment 36 Achim Kraus CLA 2021-09-29 05:14:35 EDT
> Are we done here?

Since release 2.0, yes. 
See https://github.com/eclipse/californium/blob/2.0.x/scandium-core/src/main/java/org/eclipse/californium/scandium/DTLSConnector.java#L2615-L2632

Regardless of a wrong identity or secret, the FINISH is simply ignored.

> Have you decided whether or not a CVE is required?

FMPOV, I prefer to have CVE for actual vulnerabilities. 
But not for "rust", which may lead somehow in an unfixed future to trouble. The strength of PSK is in my opinion the strength of the secret and keeping it secret. The "privacy" of the identity is some side stuff. If the first ("strong secret") is considered, it's not that relevant. 

My conclusion. no CVE.
Comment 37 Wayne Beaton CLA 2021-09-29 16:46:44 EDT
Declaring victory...