Basic Auth does not seem to work with SHA1 hashs

[www.google.com/accounts/o8/id?id=AItOawlOMc4TegxQewE17mpLWT0RLKdQIHsGX88]

We are using Basic Authentication to protect part of our site. Initially, we set up the passwords using the SHA1 option. However, authentication failed. Using MD5 worked OK.

I think NGINX module is not generating the SHA1 version of the password from the HTTP header correctly. This is the log dump:

2011/11/04 14:01:22 [debug] 26969#0: *6 rc: 0 user: "SR*" salt: "{SHA}VAf27VQjI3EG7889p+LAY9HqMOo="
2011/11/04 14:01:22 [debug] 26969#0: *6 encrypted: "{SgJ8nAZK5bVg"
2011/11/04 14:01:22 [error] 26969#0: *6 user "istewartson": password mismatch, client: 62.73.161.20, server: site400.the
clubuk.com, request: "GET /guides/index.html HTTP/1.1", host: "SR", referrer: "​http://**SR**/report/chapter-1.html"

Looking at the source, we think the code in src/core/ngx_crypt.c in the function ngx_crypt_ssha not creating the hashed version of the password correctly - it should be {SHA}followed by 20 characters - the encrypted line in the above debug dump looks wrong.

I've remove some sensitive data and replaced it by SR (host and user names).

[Maxim Dounin]

nginx doesn't support {SHA} password scheme, it does support {SSHA} one (salted variant of {SHA}).

The {SHA} scheme shouldn't be used for security reasons as it's vulnerable to attacks using ​rainbow tables. Use either {SSHA} or {MD5} instead if you care about compatibility with other platforms, or crypt() schemes provided by your OS if you aren't.

[Louis Opter]

Hello,

I would like to re-open this, it really sucks to not have {SHA} support for user who already have htpasswd files using {SHA} in their hands.

I have a bunch of htpasswd files from Apache, all using {SHA} and I don't have the original passwords anymore, so I can't recreate them with another scheme.

I understand that {SHA} is less secure but well, since Nginx supports {PLAIN} and that the code is very similar to {SSHA}, I really don't see any reason for not adding it.

I'm attaching a patch to add {SHA} support in Nginx, it just works against trunk@5008 and also Nginx 1.2.1.

[Louis Opter]

Add support for {SHA} in htpasswd files

[Maxim Dounin]

If you want to use particular hashes as is and understand consequences, you may just do s/SHA/SSHA/g in your password files, as SHA password hashes are SSHA ones with empty salt. (This also makes adding support as trivial as adding another string constant, but see below.)

Unfortunately, most people doesn't understand that SHA password scheme is almost as unsecure as PLAIN one. Instead, unaware people tend to think that SHA is better than MD5 and prefer SHA over $apr1$ - as seen many times with Apache and it's htpasswd which offers both. That's why support would be a bad idea and isn't added.

Closing this again as 'wontfix' to refect that no support for {SHA} passwords is intentional. See above for a workaround you may use if you know fore sure you want to use unsalted passwords.

[is]

Changing SHA password hash to SSHA one with empty salt just hides the security issue.
I believe SHA password hash scheme should be added to support fast migration from old setup.

[Maxim Dounin]

The argument that s/SHA/SSHA/g hides the problem was discussed and it is considered valid. Below is slightly cleaned up patch to add {SHA} support.

We also need some documentation to note that {SHA} is bad from security point of view, but supported for compatibility reasons.

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1359991300 -14400
# Node ID 5ec9f96e039e065a3174b8e50a2b8a176cf0d5f5
# Parent  44025ae9fc6736ff5d6e062025e3e7bab37f784f
Added support for {SHA} passwords (ticket #50).

Note: use of {SHA} passwords is discouraged as {SHA} password scheme is
vulnerable to attacks using ranbow tables.  Use of {SSHA}, $apr1$ or
crypt algorithms as supported by OS is recommended instead.

The {SHA} password scheme support is added to avoid need of changing of
the password scheme recorded in password files from {SHA} to {SSHA}
as it hides security problem with passwords used.

Patch by Louis Opter, with minor changes.

diff --git a/src/core/ngx_crypt.c b/src/core/ngx_crypt.c
--- a/src/core/ngx_crypt.c
+++ b/src/core/ngx_crypt.c
@@ -24,6 +24,8 @@ static ngx_int_t ngx_crypt_plain(ngx_poo
 
 static ngx_int_t ngx_crypt_ssha(ngx_pool_t *pool, u_char *key, u_char *salt,
     u_char **encrypted);
+static ngx_int_t ngx_crypt_sha(ngx_pool_t *pool, u_char *key, u_char *salt,
+    u_char **encrypted);
 
 #endif
 
@@ -43,6 +45,9 @@ ngx_crypt(ngx_pool_t *pool, u_char *key,
 #if (NGX_HAVE_SHA1)
     } else if (ngx_strncmp(salt, "{SSHA}", sizeof("{SSHA}") - 1) == 0) {
         return ngx_crypt_ssha(pool, key, salt, encrypted);
+
+    } else if (ngx_strncmp(salt, "{SHA}", sizeof("{SHA}") - 1) == 0) {
+        return ngx_crypt_sha(pool, key, salt, encrypted);
 #endif
     }
 
@@ -241,6 +246,38 @@ ngx_crypt_ssha(ngx_pool_t *pool, u_char 
     return NGX_OK;
 }
 
+
+static ngx_int_t
+ngx_crypt_sha(ngx_pool_t *pool, u_char *key, u_char *salt, u_char **encrypted)
+{
+    size_t      len;
+    ngx_str_t   encoded, decoded;
+    ngx_sha1_t  sha1;
+    u_char      digest[20];
+
+    /* "{SHA}" base64(SHA1(key)) */
+
+    decoded.len = sizeof(digest);
+    decoded.data = digest;
+
+    ngx_sha1_init(&sha1);
+    ngx_sha1_update(&sha1, key, ngx_strlen(key));
+    ngx_sha1_final(digest, &sha1);
+
+    len = sizeof("{SHA}") - 1 + ngx_base64_encoded_length(decoded.len) + 1;
+
+    *encrypted = ngx_pnalloc(pool, len);
+    if (*encrypted == NULL) {
+        return NGX_ERROR;
+    }
+
+    encoded.data = ngx_cpymem(*encrypted, "{SHA}", sizeof("{SHA}") - 1);
+    ngx_encode_base64(&encoded, &decoded);
+    encoded.data[encoded.len] = '\0';
+
+    return NGX_OK;
+}
+
 #endif /* NGX_HAVE_SHA1 */
 
 #endif /* NGX_CRYPT */

[Louis Opter]

Replying to Maxim Dounin:

The argument that s/SHA/SSHA/g hides the problem was discussed and it is considered valid. Below is slightly cleaned up patch to add {SHA} support.

We also need some documentation to note that {SHA} is bad from security point of view, but supported for compatibility reasons.

Thanks, I'm glad that this is going to be merged!

Once we have a target release for this patch, I'll gladly update wiki.nginx.org. I don't know if there is something else I can edit?

[Ruslan Ermilov]

Replying to Louis Opter <kalessin@kalessin.fr>:

Thanks, I'm glad that this is going to be merged!

Once we have a target release for this patch, I'll gladly update wiki.nginx.org. I don't know if there is something else I can edit?

An update for official docs is ready. It's not yet available from nginx.org awaiting the code commit to happen, but you can see it here in the interim:

​http://pp.nginx.com/ru/libxslt/en/docs/http/ngx_http_auth_basic_module.html

[Maxim Dounin]

In 5035/nginx:

(The changeset message doesn't reference this ticket)

[Maxim Dounin]

The patch was committed as [e4441ebe05d5].