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.

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

Basic auth: Blowfish/Bcrypt password hashing doesn't work #3150

Closed
michaelkunzmann-sap opened this issue Sep 27, 2018 · 15 comments
Closed

Basic auth: Blowfish/Bcrypt password hashing doesn't work #3150

michaelkunzmann-sap opened this issue Sep 27, 2018 · 15 comments

Comments

@michaelkunzmann-sap
Copy link

michaelkunzmann-sap commented Sep 27, 2018

Is this a request for help? Yes, but also might be a bug report

What keywords did you search in NGINX Ingress controller issues before filing this one?
"bcrypt"
"crypt_r() failed"


Is this a BUG REPORT or FEATURE REQUEST? BUG REPORT

NGINX Ingress controller version: nginx-ingress-0.22.1

Kubernetes version (use kubectl version):

Client Version: version.Info{Major:"1", Minor:"9", GitVersion:"v1.9.6", GitCommit:"9f8ebd171479bec0ada837d7ee641dec2f8c6dd1", GitTreeState:"clean", BuildDate:"2018-03-21T15:21:50Z", GoVersion:"go1.9.3", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"9+", GitVersion:"v1.9.7-gke.6", GitCommit:"9b635efce81582e1da13b35a7aa539c0ccb32987", GitTreeState:"clean", BuildDate:"2018-08-16T21:33:47Z", GoVersion:"go1.9.3b4", Compiler:"gc", Platform:"linux/amd64"}

Environment:

  • Cloud provider or hardware configuration: GKE
  • OS (e.g. from /etc/os-release): Linux / Ubuntu 16.04 (Local)
  • Kernel (e.g. uname -a): 4.4.0-134-generic (Local)
  • Install tools: N/A
  • Others: N/A

What happened:
I want to enable basic auth so I created a htpasswd configuration with a bcrypt hash. However, when I sign in, I get an "internal server error" with the following log entry:

2018/09/27 16:37:18 [crit] 28296#28296: *98777 crypt_r() failed (22: Invalid argument), client: xxx.xxx.xxx.xxx, server: xxx.xxx.xxx, request: "GET /favicon.ico HTTP/2.0", host: "xxx.xxx.xxx", referrer: "https://xxx.xxx.xxx/"

What you expected to happen:
Basic auth to work with bcrypt hashes, as described here: http://httpd.apache.org/docs/current/programs/htpasswd.html

Bcrypt should be supported by ingress-nginx, since md5, sha1 and crypt are not considered secure anymore for hashing passwords.

How to reproduce it (as minimally and precisely as possible):

I generated the htpasswd string using Go, like this:

import (
  ...
  "golang.org/x/crypto/bcrypt"
)
...
username := "admin"
password  := "nitJ4ln7NLDTL5XzMyPbpBXlEHJOSvF55FYIImBtinI="
bcryptHashedPassword, _ := bcrypt.GenerateFromPassword(
		[]byte(password),
		bcrypt.DefaultCost, // = 10
	)
htpasswd := fmt.Sprintf("%s:%s", username, string(bcryptHashedPassword))

Which generates the following output (contains 128bit salt):

admin:$2a$10$iOQDUvSaEY2tiHaLk980luTKCxTZ9TiyEmiqtVx5R8m9zKTHRfqru

Which I encode to base64 and put into a Secret's auth field:

apiVersion: v1
kind: Secret
metadata:
  name: basic-auth
data:
  auth: YWRtaW46JDJhJDEwJGlPUURVdlNhRVkydGlIYUxrOTgwbHVUS0N4VFo5VGl5RW1pcXRWeDVSOG05ektUSFJmcXJ1

Which is reused by an ingress rule:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  annotations:
    kubernetes.io/ingress.class: nginx
    nginx.ingress.kubernetes.io/auth-realm: Authentication required.
    nginx.ingress.kubernetes.io/auth-secret: basic-auth
    nginx.ingress.kubernetes.io/auth-type: basic
  name: myingress
spec:
  rules:
  - host: xxx.xxx.xxx
    http:
      paths:
      - backend:
          serviceName: someservice
          servicePort: 3000
        path: /
  tls:
  - hosts:
    - xxx.xxx.xxx

When I open the site in a browser, if prompts for a password but returns a 500 error, with the log output shown above.

@michaelkunzmann-sap
Copy link
Author

michaelkunzmann-sap commented Sep 27, 2018

It looks like this is a base image issue: https://trac.nginx.org/nginx/ticket/382

It also looks like nginx:alpine supports bcrypt: nginxinc/docker-nginx#29 (comment)

@aledbf
Copy link
Member

aledbf commented Sep 28, 2018

It also looks like nginx:alpine supports bcrypt: nginxinc/docker-nginx#29 (comment)

I am sorry but we have no plans to switch to alpine.

@aledbf
Copy link
Member

aledbf commented Sep 28, 2018

Closing. This bug is related to the missing support for bcrypt in debian/ubuntu https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=149452
Once this is fixed upstream we can address this issue.

@aledbf aledbf closed this as completed Sep 28, 2018
@james-stevens
Copy link

james-stevens commented Dec 11, 2019

Apache can support it :)
I'm using Slackware 14.2 and have the same issue with nginx, but bcrypt passwords work fine in Apache

This would be a problem for FIPS environments, as the default password encryption is MD5, but FIPS bans all MD5.

The SHA1 implementation in "htpasswd" does not include a salt, so is vulnerable to a rainbow attack. Therefore, bcrypt is really the only secure password mechanism available.

You really should support bcrypt.

@jfuechsl
Copy link

@aledbf could you please reopen this and discuss how we can add support for bcrypt? I'm happy to submit a PR, given some direction.

@aledbf
Copy link
Member

aledbf commented Dec 16, 2019

@jfuechsl @james-stevens this is an issue with Debian itself, not NGINX. The only course of action is to switch to a different distribution, like alpine (which is known to work) this image https://github.com/kubernetes/ingress-nginx/tree/master/images/nginx

Note: this is not a trivial task

@jfuechsl
Copy link

Thanks for the response. Looks like there isn't much to be done about it.

@james-stevens
Copy link

So its works in Apache, but there is nothing you can do to get it working in nginx.

Seems unlikely to me.

@james-stevens
Copy link

james-stevens commented Dec 17, 2019

In Apache bcrypt is done in apr-util/crypto/apr_passwd.c

static const char * const bcrypt_id = "$2y$";
APU_DECLARE(apr_status_t) apr_bcrypt_encode(const char *pw,
                                            unsigned int count,
                                            const unsigned char *salt,
                                            apr_size_t salt_len,
                                            char *out, apr_size_t out_len)
{
    char setting[40];
    if (_crypt_gensalt_blowfish_rn(bcrypt_id, count, (const char *)salt,
                                   salt_len, setting, sizeof(setting)) == NULL)
        return APR_FROM_OS_ERROR(errno);
    if (_crypt_blowfish_rn(pw, setting, out, out_len) == NULL)
        return APR_FROM_OS_ERROR(errno);
    return APR_SUCCESS;
}

The Blowfish code is in crypto/crypt_blowfish.c, from http://www.openwall.com/crypt/

And the time between my comments is 4 mins - so it took me 4 mins to find the code and write about it.

@aledbf
Copy link
Member

aledbf commented Dec 17, 2019

And the time between my comments is 4 mins - so it took me 4 mins to find the code and write about it.

I am sorry but I don't understand what are you expecting from the ingress-nginx.

@MitchK
Copy link

MitchK commented Dec 17, 2019

In Apache bcrypt is done in apr-util/crypto/apr_passwd.c

Afaik, ingress-nginx isn't developing nginx itself so they are unlikely to add it. It seems to be an issue with the Linux distribution used.

Closing. This bug is related to the missing support for bcrypt in debian/ubuntu https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=149452
Once this is fixed upstream we can address this issue.

This bug report has been open since 2002 and the last comment was 2017. Not sure how quick things are to change there. But to be fair, I also question how big the use case for Nginx password hashing nowadays is. We had a niche use case but were able to fall back to an alternative solution.

It might be good to at least add it to the documentation that nginx password hashing shouldn't be used with ingress-nginx.

@james-stevens
Copy link

Yeah - my mistake - I'm really sorry - I didn't read which project this was on carefully enough.

I'm sorry for the trouble.

@aledbf
Copy link
Member

aledbf commented Dec 28, 2019

@james-stevens @jfuechsl I am testing the migration to alpine in #4862
Please test the dev image quay.io/kubernetes-ingress-controller/nginx-ingress-controller-amd64:dev-1.17.7-2 where this issue should be fixed.

Important: due to the change in the distribution you need to change runAsUser: 33 to runAsUser: 101 in the ingress controller deployment.

@pepakriz
Copy link

I can confirm that the latest version (0.27.0) works. Thank you!

@fl-max
Copy link

fl-max commented Apr 21, 2020

Can also confirm this works now.

Example
Using Terraform to create the blowfish password:

resource "random_password" "password" {
  length           = 15
  special          = true
  override_special = "_%@"
}

resource "kubernetes_secret" "basic-auth" {
  metadata {
    name      = "basic-auth"
    namespace = "my-namespace"
  }
  data = {
    "auth"     = "admin:${bcrypt(random_password.password.result, 9)}"
  }
}

Ingress Annotations

ingress:
  annotations:
    kubernetes.io/ingress.class: nginx
    nginx.ingress.kubernetes.io/auth-type: basic
    nginx.ingress.kubernetes.io/auth-secret: basic-auth
    nginx.ingress.kubernetes.io/auth-realm: "Authentication Required"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants