Am Donnerstag, 23. Mai 2013, 11:12:20 schrieb Richard Brady:
> The syn_branch global parameter results in the use of a "synonym"
> branch parameter in the Via header for statelessly forwarded requests
> as a performance optimisation.
>
> This was originally done by setting branch=0 which, while not strictly
> compliant with 3261 (8.1.1.7 and 16.6 item 8), would not cause any
> problems with a downstream proxy or UA compliant to either 3261 or
> 2543 since it does not contain the cookie and is therefore
> self-evidently not guaranteed to be unique.
>
> Then a patch was added (commit ebb3b08) to change it from branch=0 to
> branch=z9hG4bKcydzigwkX which includes the cookie, thus declaring
> itself to be unique when it clearly is not.
>
> This takes the optimisation too far as it deliberately misleads
> downstream proxies with regard to the uniqueness of the parameter and
> breaks the mechanism in 17.2.3 for identifying unreliable (non-unique)
> branch parameters for transaction matching which should then fall back
> to header inspection but does not. The result is that unrelated
> requests can be identified as duplicates of each other.
>
> Also note 3261 section 16.11 on Stateless Proxy behaviour: "The
> requirement for unique branch IDs across space and time applies to
> stateless proxies as well."
>
> Has the commit above introduced a bug and should it be reverted?
>
> And should the next major release have a default of syn_branch=0?
> Since with syn_branch=1 the branch=0 version has been known to cause
> interop issues in the past (see below) and I can confirm the
> branch=z9hG4bKcydzigwkX version causes interop issues in the present.
Hello Richard,
syn_branch=0 should be made the default, this is what we use as default since
years in our backend. The performance concerns that were the reason for
introducing it long ago are nowadays not valid anymore (even on embedded
systems).
I would suggest to remove this parameter completely, one less if/else case in
the module code and also a a parameter less to document and learn for our
users.
Henning
I created another module that links with OpenSSL.
The current list of (non-obsolete) modules that link with OpenSSL is:
- websocket
- auth_ephemeral
- tls
- stun
- outbound
- osp
- auth_identity
FYI, for the modules I've created the usage of OpenSSL is:
- websocket: SHA1() is used to create the key in the WebSocket handshake
response.
- auth_ephemeral: HMAC(EVP_sha1(), ...) is used to calculate the password
based on the username and secret key and openssl/sha.h is included for
"#define SHA_DIGEST_LENGTH"
- outbound: HMAC(EVP_sha1(), ...) is used to encode the flow token and
RAND_bytes() is used to get cryptographically strong pseudo-random bytes
for the secret key
- stun: not sure about this as a lot of the code was copied from core
--
Peter Dunkley
Technical Director
Crocodile RCS Ltd
Module: sip-router
Branch: master
Commit: ba2a6ac4230dd9169943f55a9c06af3faa694356
URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=ba2a6ac…
Author: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com>
Committer: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com>
Date: Tue May 28 00:20:20 2013 +0100
modules/auth_ephemeral: updated to handle usernames from the web-service that just consist of timestamps
- tidied up the diagnostic output
---
modules/auth_ephemeral/README | 10 ++++--
modules/auth_ephemeral/authorize.c | 31 +++++++++++--------
.../auth_ephemeral/doc/auth_ephemeral_admin.xml | 12 ++++---
3 files changed, 31 insertions(+), 22 deletions(-)
diff --git a/modules/auth_ephemeral/README b/modules/auth_ephemeral/README
index 932c886..a28fea5 100644
--- a/modules/auth_ephemeral/README
+++ b/modules/auth_ephemeral/README
@@ -104,7 +104,7 @@ Chapter 1. Admin Guide
The request should contain the following parameters:
* service - specifies the desired service (msrp, sip, etc)
- * username - a user identifier for the service
+ * username - an optional user identifier for the service
* ttl - an optional TTL request for the lifetime of the credentials,
in seconds.
@@ -114,9 +114,11 @@ GET /?service=sip&username=foobar;&ttl=86400;
1.1.2. Response
The response should include the following parameters:
- * username - the username to use, which is a combination of the
- username parameter from the request, with a timestamp in time_t
- format, colon-separated.
+ * username - the username to use with the service, which is a
+ combination of the username parameter from the request and a
+ timestamp in time_t format, colon-separated. If a username was not
+ included in the request this parameter will just include the
+ timestamp.
* password - the password to use; this value is computed from the
secret key and the returned username value, by performing
base64(hmac-sha1(secret key, returned username)).
diff --git a/modules/auth_ephemeral/authorize.c b/modules/auth_ephemeral/authorize.c
index ea7152f..9d2dd65 100644
--- a/modules/auth_ephemeral/authorize.c
+++ b/modules/auth_ephemeral/authorize.c
@@ -46,7 +46,6 @@ static inline int get_ha1(struct username* _username, str* _domain,
unsigned char password[base64_enc_len(hmac_len)];
str spassword;
- LM_INFO("using secret: %.*s\n", _secret->len, _secret->s);
if (HMAC(EVP_sha1(), _secret->s, _secret->len,
(unsigned char *) _username->whole.s,
_username->whole.len, hmac_sha1, &hmac_len) == NULL) {
@@ -57,11 +56,11 @@ static inline int get_ha1(struct username* _username, str* _domain,
spassword.len = base64_enc(hmac_sha1, hmac_len, password,
base64_enc_len(hmac_len));
spassword.s = (char *) password;
- LM_INFO("calculated password: %.*s\n", spassword.len, spassword.s);
+ LM_DBG("calculated password: %.*s\n", spassword.len, spassword.s);
eph_auth_api.calc_HA1(HA_MD5, &_username->whole, _domain, &spassword,
0, 0, _ha1);
- LM_INFO("HA1 string calculated: %s\n", _ha1);
+ LM_DBG("calculated HA1: %s\n", _ha1);
return 0;
}
@@ -73,6 +72,8 @@ static int do_auth(struct sip_msg* msg, struct hdr_field *h, str *realm,
char ha1[256];
auth_body_t *cred = (auth_body_t*) h->parsed;
+ LM_DBG("secret: %.*s\n", secret->len, secret->s);
+
ret = get_ha1(&cred->digest.username, realm, secret, ha1);
if (ret < 0)
{
@@ -101,12 +102,10 @@ static int do_auth(struct sip_msg* msg, struct hdr_field *h, str *realm,
static int verify_timestamp(str* username)
{
- int pos = 0;
+ int pos = 0, cur_time = (int) time(NULL);
unsigned int expires;
str time_str = {0, 0};
- LM_INFO("username: %.*s\n", username->len, username->s);
-
while (pos < username->len && username->s[pos] != ':')
pos++;
@@ -117,19 +116,19 @@ static int verify_timestamp(str* username)
}
else
{
- LM_ERR("unable to extract timestamp from username\n");
- return -1;
+ time_str.s = username->s;
+ time_str.len = username->len;
}
- LM_INFO("username timestamp: %.*s\n", time_str.len, time_str.s);
-
+ LM_DBG("username timestamp: %.*s\n", time_str.len, time_str.s);
if (str2int(&time_str, &expires) < 0)
{
LM_ERR("unable to convert timestamp to int\n");
return -1;
}
- if ((int) time(NULL) > expires)
+ LM_DBG("current time: %d\n", cur_time);
+ if (cur_time > expires)
{
LM_WARN("username has expired\n");
return -1;
@@ -144,6 +143,10 @@ static int digest_authenticate(struct sip_msg* msg, str *realm,
struct hdr_field* h;
int ret;
struct secret *secret_struct = secret_list;
+ str username;
+
+ LM_DBG("realm: %.*s\n", realm->len, realm->s);
+ LM_DBG("method: %.*s\n", method->len, method->s);
ret = eph_auth_api.pre_auth(msg, realm, hftype, &h, NULL);
switch(ret) {
@@ -175,8 +178,10 @@ static int digest_authenticate(struct sip_msg* msg, str *realm,
return AUTH_OK;
}
- if (verify_timestamp(&((auth_body_t*) h->parsed)->digest.username.whole)
- < 0)
+ username = ((auth_body_t *) h->parsed)->digest.username.whole;
+ LM_DBG("username: %.*s\n", username.len, username.s);
+
+ if (verify_timestamp(&username) < 0)
{
LM_ERR("invalid timestamp in username\n");
return AUTH_ERROR;
diff --git a/modules/auth_ephemeral/doc/auth_ephemeral_admin.xml b/modules/auth_ephemeral/doc/auth_ephemeral_admin.xml
index 678320b..ba2c282 100644
--- a/modules/auth_ephemeral/doc/auth_ephemeral_admin.xml
+++ b/modules/auth_ephemeral/doc/auth_ephemeral_admin.xml
@@ -56,8 +56,8 @@
(msrp, sip, etc)</para>
</listitem>
<listitem>
- <para><emphasis>username</emphasis> - a user identifier for the
- service</para>
+ <para><emphasis>username</emphasis> - an optional user identifier for
+ the service</para>
</listitem>
<listitem>
<para><emphasis>ttl</emphasis> - an optional TTL request for the
@@ -77,9 +77,11 @@ GET /?service=sip&username=foobar;&ttl=86400;
The response should include the following parameters:
<itemizedlist>
<listitem>
- <para><emphasis>username</emphasis> - the username to use, which is a
- combination of the username parameter from the request, with a timestamp
- in time_t format, colon-separated.</para>
+ <para><emphasis>username</emphasis> - the username to use with the
+ service, which is a combination of the username parameter from the
+ request and a timestamp in time_t format, colon-separated. If a username
+ was not included in the request this parameter will just include the
+ timestamp.</para>
</listitem>
<listitem>
<para><emphasis>password</emphasis> - the password to use; this value is
i noticed this when compiling ctl command:
parse_listen_id.c: In function 'str2s':
parse_listen_id.c:66:17: warning: variable 'init' set but not used [-Wunused-but-set-variable]
-- juha