and switch usages to ByteBufferWriter::Write
This is part of getting rid of "pointer + length" arguments.
Bug: webrtc:42225170
Change-Id: I65a9b9550868022c0eb1f63b547195dadfbea678
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/377461
Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43916}
Propagate field trials using Environment with intent to change various types, BasicPortAllocator in particular, to take Environment at construction.
Bug: webrtc:42220378
Change-Id: I488aa82aa606e38f16aa22a032c60f4d191ede72
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/377040
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43887}
This reverts commit 4de5839c1117e5bb96148c8575a74a69bde02768.
Reason for revert: Bug fixed + DCHECK added
Original change's description:
> Revert "Move piggybacking controller from P2PTC to DTLS transport"
>
> This reverts commit 29e639e0a495a537c610182ab9b04aed8cf10426.
>
> Reason for revert: found bug accessing variable after it has been moved.
>
> Original change's description:
> > Move piggybacking controller from P2PTC to DTLS transport
> >
> > The DTLS-STUN piggybacking controller is associated with both the DTLS
> > transport and the ICE transport (P2PTransportChannel). It turned out to
> > be more closely associated with the DTLS transport and requires less
> > plumbing when moved there.
> >
> > The config option to enable the feature remains as part of the ICE
> > transport config since the ICE transport does not know its "upstream"
> > DTLS transport and hence can not query the config from it.
> >
> > BUG=webrtc:367395350
> >
> > Change-Id: Iafd5abd8b65855bcf32bf840414d96513d8e6300
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/375283
> > Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
> > Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#43823}
>
> Bug: webrtc:367395350
> Change-Id: I2d83de8890b0aa230dd9e21cb5ce2eb03c8d3564
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/375861
> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#43824}
Bug: webrtc:367395350
Change-Id: I4b4acccf15de565736b072ca2de88a1551a6378e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/375862
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43825}
This reverts commit 29e639e0a495a537c610182ab9b04aed8cf10426.
Reason for revert: found bug accessing variable after it has been moved.
Original change's description:
> Move piggybacking controller from P2PTC to DTLS transport
>
> The DTLS-STUN piggybacking controller is associated with both the DTLS
> transport and the ICE transport (P2PTransportChannel). It turned out to
> be more closely associated with the DTLS transport and requires less
> plumbing when moved there.
>
> The config option to enable the feature remains as part of the ICE
> transport config since the ICE transport does not know its "upstream"
> DTLS transport and hence can not query the config from it.
>
> BUG=webrtc:367395350
>
> Change-Id: Iafd5abd8b65855bcf32bf840414d96513d8e6300
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/375283
> Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
> Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#43823}
Bug: webrtc:367395350
Change-Id: I2d83de8890b0aa230dd9e21cb5ce2eb03c8d3564
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/375861
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43824}
The DTLS-STUN piggybacking controller is associated with both the DTLS
transport and the ICE transport (P2PTransportChannel). It turned out to
be more closely associated with the DTLS transport and requires less
plumbing when moved there.
The config option to enable the feature remains as part of the ICE
transport config since the ICE transport does not know its "upstream"
DTLS transport and hence can not query the config from it.
BUG=webrtc:367395350
Change-Id: Iafd5abd8b65855bcf32bf840414d96513d8e6300
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/375283
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43823}
This will cause automation to not pick it up when searching for
headers.
Bug: webrtc:42226155
Change-Id: I4e93cd4eca13af32f76201df784b20a80ac9baed
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/375581
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43816}
DTLS 1.3 encrypts more parts of the handshake so we move from
deep packet inspection to looking at the state of DTLS to
decide whether to intercept the packet.
BUG=webrtc:367395350
Change-Id: Idb1eda0437f24002f48381af5d6a167a4a153381
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374501
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43794}
the lack of this led to a DTLS restart in the default case.
Also reduce the log level from "ERROR" to "INFO".
BUG=webrtc:367395350
Change-Id: I52cd74f508ca4b4c28b11220ea82d95081558eee
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374503
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43752}
This change puts the DTLS handshake as payload of STUN packets with a custom STUN attribute (registered with the IANA) and starts the DTLS handshake before the ICE transport becomes writable. Effectively, STUN acts as a transport layer for DTLS during the handshake phase.
This will theoretically reduce the call setup time by one RTT for aggressive nomination or two RTTs for regular nomination.
The latest DTLS packet (flight) is cached and sent on every STUN request or response. DTLS packets are extracted from every authenticated STUN request or response and handled to the DTLS layer for processing.
The caching also increases the resilience to packet loss as STUN pacing is more aggressive (every 20ms) than the exponential backoff used by DTLS which should reduce call setup time in lossy networks.
If the other side of the connection does not support this feature the fallback to normal DTLS happens as soon as the ICE transport becomes writable. This also handles edge-cases like fragmentation of the DTLS handshake.
The feature is only supported when ECDSA certificates are used since RSA certificates are too large to transport as STUN attributes. The observed attributes for the server and client flights with the certificates were around 600 to 650 bytes. This may be further reduced by using raw public keys defined in RFC 7250.
This feature is disabled by default and guarded by the field trial
WebRTC-IceHandshakeDtls
and requires experimentation and standardization before roll-out in the browser.
Parts of this landed in
https://webrtc-review.googlesource.com/c/src/+/370679
BUG=webrtc:367395350
Change-Id: I4809438b2a267c4690a9b2bd6f1766d2f959500d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362480
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43742}
where we can not extract any message_seq for piggybacking.
BUG=webrtc:367395350
Change-Id: I2e42d3b1304b5cf204c34bc8a136f17c363e011c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/372881
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43731}
I ran the formatter on everything to make find leftovers, and these are
the last files.
git ls-files | grep -E '^.*\.(h|cc|mm|c|m)$' | xargs clang-format -i
No-Iwyu: Includes didn't change and it isn't related to formatting
Bug: webrtc:42225392
Change-Id: I8bbe8bd3c97b76c691e38b8ca290417202a61b6c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374161
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43720}
This patchs adds a field trial for enabling DTLS1.3, WebRTC-ForceDtls13
- "Enabled" set max version to DTLS1.3
- "Only" set min & max version to DTLS1.3
Wireup a FieldTrialsView so that this does not
use the global string.
Also convert the WebRTC-DisableTlsSessionTicketKillswitch
from global string to FieldTrialsView.
BUG=webrtc:383141571
Change-Id: Ia775efc1dcbffd01bfddb6030490438cb8de89d7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/372261
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43616}
Modify openssl_stream_adapter to check/set
timer regardless of dtls state. This is needed for DTLS1.3
orelse handshake will never complete if last client
packet is lost (e.g if retransmit is not triggered after
writable) as show by TestHandshakeLoseSecondClientPacket.
TestHandshakeLoseSecondClientPacket works with/without this
patch if using DTLS1.2.
BUG=webrtc:383141571
Change-Id: I2757783c9e79686d1fbe0eff12341ab9e3863fdd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/372201
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43610}
which implements the handshaking logic of the DTLS-STUN piggybacking.
Not wired up yet, split from
https://webrtc-review.googlesource.com/c/src/+/362480
BUG=webrtc:367395350
Change-Id: I9ee8ff17af4ec96fb891d9852ac50825155735a8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370679
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43606}
- add DTLS1.3 ciphers (without KeyType)
- remove code in dtls_transport.cc that tries to parse DTLS packet
- cleanup some test
- start on test for packet loss during dtls handshake (more to come!)
After this patch is submitted, it is possible
to set max version = dtls1.3 and it will active
but DON'T do it yet.
BUG=webrtc:383141571
Change-Id: I6f9a120c53415ccee7a560ea83bd0c2636702997
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/371300
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43595}
removing the last vestiges of the p2p/ dependencies and stop depending
on them for the "webrtc" static library.
BUG=webrtc:42226155
Change-Id: I0b6ac36c0a22054c229a94f55fa6690580b9d47f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/371342
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Cr-Commit-Position: refs/heads/main@{#43581}
removing the webrtc need for having sources in it.
BUG=webrtc:42226155
Change-Id: I40fbde9064f4fa629c7c6b0cf99f23ab1726da75
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370820
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43540}
This reverts commit 03f56d75d5a4bbbc6b6fe93e119f73c69ff98267.
Reason for revert: Breaks downstream project.
Original change's description:
> Remove stun_prober
>
> The STUN prober shows the old RFC 3489 way of determining the NAT type
> by pinging two different servers. This is known to be faulty as pointed
> out by
> https://datatracker.ietf.org/doc/html/rfc5389#section-2
>
> Chromium dependency removed in
> https://chromium-review.googlesource.com/c/chromium/src/+/6036622
>
> BUG=None
>
> Change-Id: I2b61dfe2ff899ce71ec9d2253dc836c5908cf8c6
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/368182
> Commit-Queue: Philipp Hancke <phancke@meta.com>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#43503}
Bug: None
Change-Id: I08d01d4c9d882aca883e1c889aed8bddbca65b91
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370540
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Jeremy Leconte <jleconte@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43506}
to be deleted when downstream consumers are upgraded
BUG=webrtc:367395350
Change-Id: I35f1fefdc6535ad443b86176ea600455c2361834
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370284
Commit-Queue: Philipp Hancke <phancke@meta.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43502}
for (partially) parsing DTLS packets and extracting the msg_seqs
BUG=webrtc:367395350
Change-Id: Ieb0fc121c6dc82118ced5939c1a9ebe2d72e3cb3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370181
Commit-Queue: Philipp Hancke <phancke@meta.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43501}
and misc cleanup
BUG=webrtc:367395350
No-Iwyu: remaining IWYU failure is deep inside gtest which is unrelated to the changes and needs to be investigated separately
Change-Id: I5c2b7a6cc6b15fc5474c55eb98635cb9145b7373
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370180
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Cr-Commit-Position: refs/heads/main@{#43498}
With L4S in WebRTC, only RTP packets are supposed to be send with ECT(1)
Bug: webrtc:42225697
Change-Id: If10bf74a867d3ea04fd1fb931cdc2a6380176270
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/367220
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43343}
The const-ref result of .str() must be copied into the returned
value, whereas the result of .Release() can be moved.
Bug: webrtc:374845009
Change-Id: I3abc98be30ce9947127c7664f5ffa6846b772ea2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366480
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43288}
and use uint8_t instead of unsigned char. Follow-up from
https://webrtc-review.googlesource.com/c/src/+/365274
BUG=webrtc:357776213
Change-Id: Ibc97e5cc85316ba69b4133b7f3c42e3afbdd7abd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365540
Commit-Queue: Philipp Hancke <phancke@meta.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Jeremy Leconte <jleconte@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43263}
This is a reland of commit 65ae3245f9380e46b1d755f3f452ba63ab6cdf8d
with more backward compat which also fixes the off-by-one issue which caused wrong SRTP keys to be extracted.
Original change's description:
> Spanify SRTP key export
>
> and simplify the interface used as this is only used for exporting
> SRTP keys and passing arcane OpenSSL arguments around does not make
> much sense.
>
> BUG=webrtc:357776213
>
> Change-Id: I9e5a94fe368b77975e48b6dd5ab6a2d2575d6382
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/364521
> Commit-Queue: Philipp Hancke <phancke@meta.com>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Florent Castelli <orphis@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#43198}
Bug: webrtc:357776213
Change-Id: I5d43dc23f90ef630834fb400751979fcc5e18203
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365180
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Jeremy Leconte <jleconte@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Cr-Commit-Position: refs/heads/main@{#43225}
This reverts commit 65ae3245f9380e46b1d755f3f452ba63ab6cdf8d.
Reason for revert: breaks downstream compilation
Original change's description:
> Spanify SRTP key export
>
> and simplify the interface used as this is only used for exporting
> SRTP keys and passing arcane OpenSSL arguments around does not make
> much sense.
>
> BUG=webrtc:357776213
>
> Change-Id: I9e5a94fe368b77975e48b6dd5ab6a2d2575d6382
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/364521
> Commit-Queue: Philipp Hancke <phancke@meta.com>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Florent Castelli <orphis@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#43198}
Bug: webrtc:357776213
Change-Id: I03ffcda3d6821718f355b243ce78a9c54b4036f3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365062
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Jeremy Leconte <jleconte@webrtc.org>
Owners-Override: Jeremy Leconte <jleconte@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43202}
and simplify the interface used as this is only used for exporting
SRTP keys and passing arcane OpenSSL arguments around does not make
much sense.
BUG=webrtc:357776213
Change-Id: I9e5a94fe368b77975e48b6dd5ab6a2d2575d6382
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/364521
Commit-Queue: Philipp Hancke <phancke@meta.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43198}
This has been deprecated since November 2022.
Bug: None
Change-Id: Ia547489b1f703d0744ab7ffc096eeadbb937974a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/364381
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Auto-Submit: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43140}
Move it away from the "proprietary" SSL_CIPHER_get_id and looking up the cipher based on that towards SSL_CIPHER_standard_name.
SSL_CIPHER_get_id and the associated GetSslCipherSuite API is kept around for
WebRTC.PeerConnection.SslCipherSuite.*
UMA metrics and metrics compability (despite not yielding the IANA ids it promises).
BUG=None
Change-Id: Iaa357e3e31dc90abea688cf6ca10c0b40582ef38
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/363202
Reviewed-by: David Benjamin <davidben@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43097}
Before this cl, ReadyToSend signaled false if sending a packet failed and transport->GetError() returns ECONN.
ECONN may be reported by the TCP connection (TcpConnection) if the remote closed the connection. TcpConnection will attempt to reconnect and should change the writable state if it fail.
Changing the state in the context of sending packets may cause recursive
calls and seems to cause problems with incorrect states.
It is simpler if RtpTransport::SendPacket ignore these failures and
upper layers treat these lost packets similar to if the packets had been
lost via UDP.
For safety, this change can be reverted by field trial WebRTC-SetReadyToSendFalseIfSendFail/Enabled/.
Bug: webrtc:361124449 b/359989715
Change-Id: I8e7016dfb4301862286215c4512aa8ac03a16685
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/360120
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42868}