otp
otp copied to clipboard
erlang
Ssl: implement custom hello extensions
Work in progress: no tests yet
This PR implements custom hello extensions -- a way the user can add extensions encoders and decoders without having to modify ssl code.
Usage:
For example, this module implements two extensions with types 16#4201 and 16#4202:
-module(example_ext).
-export(encode_extension/3, decode_extension/4]).
-spec encode_extension(Type::integer(), CbOpts::any(), Context::map()) ->
binary() | {last, binary()} | undefined.
encode_extension(16#4201, #{custom_01 := V}, _) ->
<<V:16>>;
encode_extension(16#4202, #{custom_02 := V}, #{version := {3,3}) ->
{last, <<"test", V:16>>};
encode_extension(16#4202, _, _} ->
undefined.
-spec decode_extension(Type::integer(), ExtData::binary(), Context::map()) ->
{Key::atom(), any()} | undefined.
decode_extension(16#4201, <<V:16>>, _) ->
{custom_01, #{custom_01 => V}};
decode_extension(16#4202, <<_:4/binary, V:16>>, #{version := {3,3}) ->
{test, V};
decode_extension(16#4202, _, _) ->
undefined.
User may send and receive extensions in server/client hello:
CustomExts = {[16#4201, 16#4202], example_ext, #{custom_01 => 78, custom_02 => 9873}},
{ok, S, Ext} = ssl:connect("localhost", 12345, [{custom_extensions, CustomExts}, {handshake, hello}]),
#{test := T} = Ext
This may be also useful for testing (e.g. add or remove some extensions to see how the other side reacts to this).
Will this let me add the DTLS-SRTP extension? Is it possible to dynamically change the server hello extension based on the client hello extension (SRTP_AEAD_AES_128_GCM or SRTP_AES128_CM_HMAC_SHA1_80)?
-define(USE_SRTP, 14).
encode_extensions([use_srtp_aes | Rest], Acc) ->
ExtData = <<0,2,0,1,0>>,
Len = byte_size(ExtData),
encode_extensions(Rest, <<?UINT16(?USE_SRTP), ?UINT16(Len), ExtData/binary, Acc/binary>>);
encode_extensions([use_srtp_gcm | Rest], Acc) ->
ExtData = <<0,2,0,7,0>>,
Len = byte_size(ExtData),
encode_extensions(Rest, <<?UINT16(?USE_SRTP), ?UINT16(Len), ExtData/binary, Acc/binary>>).
Will this let me add the DTLS-SRTP extension?
Yes! I did this exactly while working on dtls/srtp.
Is it possible to dynamically change the server hello extension based on the client hello extension (SRTP_AEAD_AES_128_GCM or SRTP_AES128_CM_HMAC_SHA1_80)?
Should work with {handshake, hello} and updating options on handshake_continue. I didn't check it, but server hello should be formed with updated options (otherwise it's a bug).
We discussed this draft in our team meeting, and we are wondering why you want to do this as customization? Just because Ericsson does not prioritize this extension at the moment it does not mean it will not benefit others and ought to be part of the standard implementation. We do not like everybody to roll their own.
why you want to do this as customization? Just because Ericsson does not prioritize this extension at the moment it does not mean it will not benefit others and ought to be part of the standard implementation.
I get your point. At first I was going to post a PR with just use_srtp extension (see https://github.com/stolen/otp/commit/e4b416708ffb5ee0e75793c3c627983b83db5081 for the code). But then I realized I'm not sure I'm doing that right:
- Encoded MKI field is just empty (I have no idea how it should work at all), and no option for it
- I was unsure if option name was clear (is there any naming convention for ssl_options?)
- should protection profiles be integers or 2-byte binaries?
- What if I need other behaviour few years later but on older OTP release?
There are lots of not implemented extensions, and eventual implementing (almost) all of them may create quite a mess in documentation, header files, code for validate_option, maybe_add_*_extensions + encode_extensions, decode_extensions...
use_srtp and maybe some of other extensions are rarely used, do not affect usual handshake, but require complex code anyway.
We do not like everybody to roll their own.
Yes, it's a good point for you as maintainers. I tried to implement this feature in a way that will make adding extensions to a mainline easier. So, in my imagination, users will be able to implement their extensions (without having an OTP fork), ensure the implementation is good for their use cases, and then suggest a new MR. Some extensions may be in draft RFCs, so be a subject for future incompatible syntax changes. Also (in my imagination) some users may need homegrown extensions with no RFCs documenting them.
Maybe it would make more sense to have a callback for unrecognized extensions by the default implementation?
Can we have both? Official support for dtls-srtp with dynamic server response based on client hello and support for custom hello extensions?
When it comes to naming conventions currently the options are {describing_name_with_underscores, Value}. And yes we would like to rework the option handling further. Legacy options, backwards compatibility and options dependency's makes it challenging.
@benbro Depends on what you mean? We are not against officially supporting the dtls-srp extension. It is probably not a very big job it just not top priority for us to make the implementation. If we allow handling of unsupported extensions and then implement an earlier not supported extension the previously callback for unsupported extensions would no longer be called. However the original suggestion could override the handling of already implemented extension handling which I do not think is a good idea.
to have a callback for unrecognized extensions by the default implementation
I thought about that, and I'm still unsure. If callback is called only for unrecognized extensions, that would mean:
- user code may break in minor release (so, extra code for handling different extension formats)
- no way to replace stock parser (if otp has some stubs or not-always-correct assumptions or peer violates the syntax)
- inconsistency when encoding extensions — either known extension is not encoded despite explicitly listed or decoder is not called while encoder still is.
Yes, that would also mean users will not notice extensions are finally upstream.
Maybe it would be convenient to have the option to choose if callback should handle known extensions, but keeping a list of known ones may be some extra pain when implementing new extensions.
override the handling of already implemented extension handling which I do not think is a good idea.
Yep, overriding stock handler is a way to shoot into own foot. Maybe this functionality should have a clear warning in documentation about that.
In addition to that maybe overriding every extension handler should be explicit too. Something like [{override, Type}] instead of just [Type] (or something harder to add in advance).
Just to be clear as this is still a draft I will not be able to make it for OTP-24 as the deadline is approaching real fast. We can probably make a solution in the lifetime of OTP-24 before OTP-25 but not making any promises. A general solution for possible handling any extension is far more work than just adding the one you are interested in. And then we have to consider the override possibilities and all the complexity that it adds. If such a thing should be added I think we probably would only want "unknown" extensions to be handled by callback and they should be ignored if the callback has no handling. But most extensions have quite a lot of impact on the protocol and I am not at the moment convinced that it should be left to the user.
Widespread TLS fingerprinting is also a thing now. Being able to specify the order of the (custom) extensions would be nice. In an ideal world this would never be needed, but sadly the real world is a terrible place.
Is there an update on support for the USE_SRTP extension or custom extensions?
I would also be interested in callbacks for unrecognized extensions, in order to implement quic_transport_parameters extension (https://www.rfc-editor.org/rfc/rfc9001.html#quic_parameters).
In this case since OTP does not have any QUIC protocol implementation, I can't imagine that officially supporting the quic_transport_parameters extension is an option. In such cases something like the callbacks suggested in this thread would work well.
Is there any update on the status of this PR?
@benbro we have not been watching this so closely as it has been in Draft-mode. But now we have #6392
@ljmarks we do plan to make adjustments to ease the implementation of the quick protocol, but alas many other things have had higher priority.
@oskarkook I agree that the world can be awful at times ;) I think a better way to support that would be to extend the pause functionality on the server side to be able to return the hello message as opposed to only the hello extensions. Although it would not be apparent how the interface should look as we in general want to avoid having records in APIs (apart from ANSN-1 specs that are pretty static) as it creates compile-time dependencies.
@stolen Do you think there will be any continuous work on this or should we abandon it for #6392 and possible other new PRs?
#6392 is sufficient for our needs, and I don't plan to continue work on custom extensions.