Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make REST API hash algorithm configurable #1447

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Make REST API hash algorithm configurable #1447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/apps/relay/mainrelay.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ turn_params_t turn_params = {
0, /* mobility */
TURN_CREDENTIALS_NONE, /* ct */
0, /* use_auth_secret_with_timestamp */
SHATYPE_DEFAULT, /* auth_secret_with_timestamp_shatype */
0, /* max_bps */
0, /* bps_capacity */
0, /* bps_capacity_allocated */
Expand Down Expand Up @@ -1424,6 +1425,7 @@ enum EXTRA_OPTS {
PROMETHEUS_PORT_OPT,
PROMETHEUS_ENABLE_USERNAMES_OPT,
AUTH_SECRET_OPT,
AUTH_SECRET_SHA256_OPT,
NO_AUTH_PINGS_OPT,
NO_DYNAMIC_IP_LIST_OPT,
NO_DYNAMIC_REALMS_OPT,
Expand Down Expand Up @@ -1545,6 +1547,7 @@ static const struct myoption long_options[] = {
{"prometheus-username-labels", optional_argument, NULL, PROMETHEUS_ENABLE_USERNAMES_OPT},
#endif
{"use-auth-secret", optional_argument, NULL, AUTH_SECRET_OPT},
{"use-auth-secret-sha256", optional_argument, NULL, AUTH_SECRET_SHA256_OPT},
{"static-auth-secret", required_argument, NULL, STATIC_AUTH_SECRET_VAL_OPT},
{"no-auth-pings", optional_argument, NULL, NO_AUTH_PINGS_OPT},
{"no-dynamic-ip-list", optional_argument, NULL, NO_DYNAMIC_IP_LIST_OPT},
Expand Down Expand Up @@ -2204,6 +2207,9 @@ static void set_option(int c, char *value) {
turn_params.ct = TURN_CREDENTIALS_LONG_TERM;
use_lt_credentials = 1;
break;
case AUTH_SECRET_SHA256_OPT:
turn_params.auth_secret_with_timestamp_shatype = SHATYPE_SHA256;
break;
case NO_AUTH_PINGS_OPT:
turn_params.no_auth_pings = 1;
break;
Expand Down
1 change: 1 addition & 0 deletions src/apps/relay/mainrelay.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ typedef struct _turn_params_ {
vint mobility;
turn_credential_type ct;
int use_auth_secret_with_timestamp;
int auth_secret_with_timestamp_shatype;
band_limit_t max_bps;
band_limit_t bps_capacity;
band_limit_t bps_capacity_allocated;
Expand Down
25 changes: 12 additions & 13 deletions src/apps/relay/userdb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,20 +552,19 @@ int get_user_key(int in_oauth, int *out_oauth, int *max_session_time, uint8_t *u

hmac[0] = 0;

stun_attr_ref sar = stun_attr_get_first_by_type_str(
ioa_network_buffer_data(nbh), ioa_network_buffer_get_size(nbh), STUN_ATTRIBUTE_MESSAGE_INTEGRITY);
if (!sar) {
return -1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is changing the behavior from using the packet to drive the hash choice, to using the commandline param / config file param.

I'm not sure that changing this behavior will be backwards compatible with existing users.

Can you elaborate on the compatibility considerations for this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was my understanding that the token is not controlled by the specification for TURN and that the hash choice being based on the packet was a decision made by coturn developers. As coturn currently only seems to support SHA-1 for integrity of the messages as per spec, I thought defaulting to SHA-1 would keep the existing behavior unless the command-line parameter is used. Please see the discussion in #1293. I'm not sure this is the right way to do it. So please clarify any assumptions I might be wrong about.

}

int sarlen = stun_attr_get_len(sar);
switch (sarlen) {
case SHA1SIZEBYTES:
switch (turn_params.auth_secret_with_timestamp_shatype) {
case SHATYPE_SHA1:
hmac_len = SHA1SIZEBYTES;
break;
case SHA256SIZEBYTES:
case SHA384SIZEBYTES:
case SHA512SIZEBYTES:
case SHATYPE_SHA256:
hmac_len = SHA256SIZEBYTES;
break;
case SHATYPE_SHA384:
hmac_len = SHA384SIZEBYTES;
break;
case SHATYPE_SHA512:
hmac_len = SHA512SIZEBYTES;
break;
default:
return -1;
};
Expand All @@ -576,7 +575,7 @@ int get_user_key(int in_oauth, int *out_oauth, int *max_session_time, uint8_t *u

if (secret) {
if (stun_calculate_hmac(usname, strlen((char *)usname), (const uint8_t *)secret, strlen(secret), hmac,
&hmac_len, SHATYPE_DEFAULT) >= 0) {
&hmac_len, turn_params.auth_secret_with_timestamp_shatype) >= 0) {
size_t pwd_length = 0;
char *pwd = base64_encode(hmac, hmac_len, &pwd_length);

Expand Down