Server should support alternate types of Channel Bindings

[GuidoKiener]

Problem

According to RFC 5929 there are different channel binding types (e.g. "tls-unique", "tls-exporter", "tls-end-point-server") for TLS connections that can be used with SASL mechanisms like SCRAM-SHA-*-PLUS or GS2-KRB5-PLUS. After a TLS connection is established a server can currently set only one binding data with sasl_setprop(conn, SASL_CHANNEL_BINDING, &cb);.

Depending on protocol agreements a client can select e.g. between:

• "tls-unique" and "tls-end-point-server" when using TLS 1.2
• "tls-exporter" and "tls-end-point-server" when using TLS 1.3

If a server does not support the desired channel binding type of the client then the authentication will fail or the client is obliged to try another channel binding type or even another worse SASL mechanism. Therefore it's useful to add a new property or callback function that allows the server to select the desired channel binding data.

Proposal: New Callback Function

My preferred solution is adding a new callback function:

/* callback to change channel binding type (e.g. "tls-end-point-server")
 * of the server.  The callback is used by plugins like SCRAM-SHA-*-PLUS or
 * GS2-KRB5-PLUS when the desired binding type of the client does not match
 * the binding type set with property SASL_CHANNEL_BINDING. A server should
 * check the requested 'cbindingname' of the 'plugin' and overwrite the
 * channel binding data of property SASL_CHANNEL_BINDING within the callback
 * function.
 *  plugin        -- name of plugin
 *  cbindingname  -- name of desired channel binding type
 */
typedef int sasl_server_channel_binding_t(sasl_conn_t* conn,
                      void* context,
                      const char* plugin,
                      const char* cbindingname);
#define SASL_CB_SERVER_CHANNEL_BINDING (0x8006)

This solution is flexibel and a server application can react to specific plugins and binding types.

Alternative Proposal: New property SASL_CHANNEL_BINDING_2

Another way is to add a new property SASL_CHANNEL_BINDING_2 that allows the server to set an alternative channel binding type that is selected by the plugins SCRAM-SHA-*-PLUS or GS2-KRB5-PLUS.

This solution is easy and ok for the next years, but maybe not flexible enough for future security extensions.

Common problem

The property SASL_CHANNEL_BINDING sets only a reference to the channel binding data and the user has to care for the lifecycle of the channel binding data. A less error-prone coding style is to allocate memory and store a copy of the channel binding data within the struct sasl_server_params. This will help users to implement the callback function for channel binding.

cc: @Neustradamus, @quanah, @ksmurchison, @aamelnikov, @dilyanpalauzov : Feedback is welcome. I would contribute the patches.

[ksmurchison]

I also believe that a new callback is the better solution. I don't have any strong feelings on whether the application of the library is responsible for the channel binding data.

[GuidoKiener]

Thanks for your comment. Since all applications using cyrus-sasl have managed the lifecycle problem of the channel binding data there is not really a need to change the policy of the ownership of the binding data. I just stumbled over this pitfall with my first reference implemenation and people are able to fix it. Nevertheless some common members of struct sasl_conn store a copy (like service, serverFQDN, security properties ...) and others do not. It's confusing. At the end I'm not sad if we keep it like it is.

[GuidoKiener]

@ksmurchison, Hi Ken, do you like to review the patch? I tested the patch in my projects and with a patched cyrus-imapd variant.

[GuidoKiener]

@ksmurchison, Hi Ken, do you like to review the patch? I tested the patch in my projects and with a patched cyrus-imapd variant.

@ksmurchison or @quanah: Do you have time to review the patch?

[ksmurchison]

I will try but I'm not an expert on channel binding. @aamelnikov can you give a look?

[Neustradamus]

@aamelnikov: Can you look tickets here?

• https://github.com/cyrusimap/cyrus-sasl/issues/742
• https://github.com/cyrusimap/cyrus-sasl/issues/823

@GuidoKiener has done a PR here, can you look too?

• https://github.com/cyrusimap/cyrus-sasl/pull/824

@ksmurchison requests same.

[Neustradamus]

@quanah: I have relaunched @aamelnikov.