nginx-http-auth-digest icon indicating copy to clipboard operation
nginx-http-auth-digest copied to clipboard

IPv6 Crash

Open Nazar78 opened this issue 1 year ago • 6 comments

Hi, I have issues using IPv6 authentication. It works in IPv4.

It initially crashed the workers, seen no similar issues opened or closed but then I realized it's in the ongoing pending 2019 PR https://github.com/samizdatco/nginx-http-auth-digest/pull/34. I then tried the patch compiling from the Debian 11 Nginx 1.18.0-6.1+deb11u3 source. Saw the warnings but it didn't error out as mentioned in the PR, so I proceed using it.

However after a few refresh, albeit successful authentication, it will eventually end up with a error 401. It's also being logged with 'ignoring authentication request - in evasion period' which I guess it never clears the auth_digest_maxtries counters despite successful authentication due to the warnings during the build.

I'm not very familiar with coding hoping this can be fixed soon, Thank you.

Nazar78 avatar Apr 06 '23 23:04 Nazar78

I somewhat got it working with IPv6.

Instead of using sockaddr_storage as mentioned in this pending 2019 PR https://github.com/samizdatco/nginx-http-auth-digest/pull/34, I made all the ngx_memcpy occurrences for the source size to use the destination size if it's larger, idea sourced from https://stackoverflow.com/questions/1184291/creating-a-wrapper-for-strncpy-to-insert-terminating-null.

Tested with both IPv4 and IPv6 in evasive mode and it works as expected. However honestly I'm not sure of the implications by using a smaller size for the IPv6 evasive mode or this is even the correct approach solving this issue as I'm not well versed in C.

Anyone can advise?

Nazar78 avatar Apr 08 '23 21:04 Nazar78

static size_t ngx_http_auth_digest_get_copy_size(size_t source_size, size_t dest_size) {
  return (dest_size <= source_size ? dest_size : source_size);
}
...
  ngx_memcpy(&testnode.src_addr, r->connection->sockaddr,
            ngx_http_auth_digest_get_copy_size(sizeof(r->connection->sockaddr), sizeof(&testnode)));
...
    ngx_memcpy(&node->src_addr, r->connection->sockaddr,
               ngx_http_auth_digest_get_copy_size(sizeof(r->connection->sockaddr), sizeof(&node)));
...
  ngx_memcpy(&testnode.src_addr, r->connection->sockaddr,
             ngx_http_auth_digest_get_copy_size(sizeof(r->connection->sockaddr), sizeof(&testnode)));

Nazar78 avatar Apr 08 '23 21:04 Nazar78

Created a fork https://github.com/samizdatco/nginx-http-auth-digest/compare/master...Nazar78:nginx-http-auth-digest:master.

Nazar78 avatar Apr 08 '23 21:04 Nazar78

I think that there are more places where addresses are used that need to be updated to fit ipv6. I'm afraid that I currently don't use this module anymore and don't have the time to look into this.

erikdubbelboer avatar Apr 15 '23 11:04 erikdubbelboer

Thank you for the reply. Just a question, do you happen to know which is the latest fork still being maintained?

Nazar78 avatar Apr 15 '23 19:04 Nazar78

That is this one as far as I know.

erikdubbelboer avatar Apr 17 '23 12:04 erikdubbelboer