serve sftp: Add support for public key with auth proxy - fixes #3572

This commit is contained in:
Paul Tinsley 2019-12-24 11:34:35 -06:00 committed by Nick Craig-Wood
parent 63128834da
commit f2a789ea98
7 changed files with 168 additions and 40 deletions

View File

@ -246,7 +246,7 @@ var connServeFunction = []byte("(*Conn).Serve(")
func (s *server) CheckPasswd(user, pass string) (ok bool, err error) {
var VFS *vfs.VFS
if s.proxy != nil {
VFS, _, err = s.proxy.Call(user, pass)
VFS, _, err = s.proxy.Call(user, pass, false)
if err != nil {
fs.Infof(nil, "proxy login failed: %v", err)
return false, nil

View File

@ -108,7 +108,7 @@ type Proxy struct {
// cacheEntry is what is stored in the vfsCache
type cacheEntry struct {
vfs *vfs.VFS // stored VFS
pwHash []byte // bcrypt hash of the password
pwHash []byte // bcrypt hash of the password/publicKey
}
// New creates a new proxy with the Options passed in
@ -162,12 +162,21 @@ func (p *Proxy) run(in map[string]string) (config configmap.Simple, err error) {
}
// call runs the auth proxy and returns a cacheEntry and an error
func (p *Proxy) call(user, pass string, passwordBytes []byte) (value interface{}, err error) {
func (p *Proxy) call(user, auth string, isPublicKey bool) (value interface{}, err error) {
var config configmap.Simple
// Contact the proxy
config, err := p.run(map[string]string{
if isPublicKey {
config, err = p.run(map[string]string{
"user": user,
"pass": pass,
"public_key": auth,
})
} else {
config, err = p.run(map[string]string{
"user": user,
"pass": auth,
})
}
if err != nil {
return nil, err
}
@ -208,10 +217,11 @@ func (p *Proxy) call(user, pass string, passwordBytes []byte) (value interface{}
if err != nil {
return nil, false, err
}
// The bcrypt cost is a compromise between security and speed. The password is looked up on every
// transaction for WebDAV so we store it lightly hashed. An attacker would find it easier to go after
// the unencrypted password in memory most likely.
pwHash, err := bcrypt.GenerateFromPassword(passwordBytes, bcrypt.MinCost)
pwHash, err := bcrypt.GenerateFromPassword([]byte(auth), bcrypt.MinCost)
if err != nil {
return nil, false, err
}
@ -227,17 +237,15 @@ func (p *Proxy) call(user, pass string, passwordBytes []byte) (value interface{}
return value, nil
}
// Call runs the auth proxy with the given input, returning a *vfs.VFS
// and the key used in the VFS cache.
func (p *Proxy) Call(user, pass string) (VFS *vfs.VFS, vfsKey string, err error) {
var passwordBytes = []byte(pass)
// Call runs the auth proxy with the username and password/public key provided
// returning a *vfs.VFS and the key used in the VFS cache.
func (p *Proxy) Call(user, auth string, isPublicKey bool) (VFS *vfs.VFS, vfsKey string, err error) {
// Look in the cache first
value, ok := p.vfsCache.GetMaybe(user)
// If not found then call the proxy for a fresh answer
if !ok {
value, err = p.call(user, pass, passwordBytes)
value, err = p.call(user, auth, isPublicKey)
if err != nil {
return nil, "", err
}
@ -249,14 +257,14 @@ func (p *Proxy) Call(user, pass string) (VFS *vfs.VFS, vfsKey string, err error)
return nil, "", errors.Errorf("proxy: value is not cache entry: %#v", value)
}
// Check the password is correct in the cached entry. This
// Check the password / public key is correct in the cached entry. This
// prevents an attack where subsequent requests for the same
// user don't have their auth checked. It does mean that if
// the password is changed, the user will have to wait for
// cache expiry (5m) before trying again.
err = bcrypt.CompareHashAndPassword(entry.pwHash, passwordBytes)
err = bcrypt.CompareHashAndPassword(entry.pwHash, []byte(auth))
if err != nil {
return nil, "", errors.Wrap(err, "proxy: incorrect password")
return nil, "", errors.Wrap(err, "proxy: incorrect password / public key")
}
return entry.vfs, user, nil

View File

@ -1,6 +1,10 @@
package proxy
import (
"crypto/rand"
"crypto/rsa"
"encoding/base64"
"log"
"strings"
"testing"
@ -10,6 +14,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/crypto/bcrypt"
"golang.org/x/crypto/ssh"
)
func TestRun(t *testing.T) {
@ -68,13 +73,13 @@ func TestRun(t *testing.T) {
const testUser = "testUser"
const testPass = "testPass"
t.Run("call", func(t *testing.T) {
t.Run("call w/Password", func(t *testing.T) {
// check cache empty
assert.Equal(t, 0, p.vfsCache.Entries())
defer p.vfsCache.Clear()
passwordBytes := []byte(testPass)
value, err := p.call(testUser, testPass, passwordBytes)
value, err := p.call(testUser, testPass, false)
require.NoError(t, err)
entry, ok := value.(cacheEntry)
require.True(t, ok)
@ -95,12 +100,12 @@ func TestRun(t *testing.T) {
assert.Equal(t, value, cacheValue)
})
t.Run("Call", func(t *testing.T) {
t.Run("Call w/Password", func(t *testing.T) {
// check cache empty
assert.Equal(t, 0, p.vfsCache.Entries())
defer p.vfsCache.Clear()
vfs, vfsKey, err := p.Call(testUser, testPass)
vfs, vfsKey, err := p.Call(testUser, testPass, false)
require.NoError(t, err)
require.NotNil(t, vfs)
assert.Equal(t, "proxy-"+testUser, vfs.Fs().Name())
@ -121,7 +126,7 @@ func TestRun(t *testing.T) {
})
// now try again from the cache
vfs, vfsKey, err = p.Call(testUser, testPass)
vfs, vfsKey, err = p.Call(testUser, testPass, false)
require.NoError(t, err)
require.NotNil(t, vfs)
assert.Equal(t, "proxy-"+testUser, vfs.Fs().Name())
@ -131,7 +136,7 @@ func TestRun(t *testing.T) {
assert.Equal(t, 1, p.vfsCache.Entries())
// now try again from the cache but with wrong password
vfs, vfsKey, err = p.Call(testUser, testPass+"wrong")
vfs, vfsKey, err = p.Call(testUser, testPass+"wrong", false)
require.Error(t, err)
require.Contains(t, err.Error(), "incorrect password")
require.Nil(t, vfs)
@ -142,4 +147,89 @@ func TestRun(t *testing.T) {
})
privateKey, privateKeyErr := rsa.GenerateKey(rand.Reader, 2048)
if privateKeyErr != nil {
log.Fatal("error generating test private key " + privateKeyErr.Error())
}
publicKey, publicKeyError := ssh.NewPublicKey(&privateKey.PublicKey)
if privateKeyErr != nil {
log.Fatal("error generating test public key " + publicKeyError.Error())
}
publicKeyString := base64.StdEncoding.EncodeToString(publicKey.Marshal())
t.Run("Call w/PublicKey", func(t *testing.T) {
// check cache empty
assert.Equal(t, 0, p.vfsCache.Entries())
defer p.vfsCache.Clear()
value, err := p.call(testUser, publicKeyString, true)
require.NoError(t, err)
entry, ok := value.(cacheEntry)
require.True(t, ok)
// check publicKey is correct in entry
require.NoError(t, err)
require.NotNil(t, entry.vfs)
f := entry.vfs.Fs()
require.NotNil(t, f)
assert.Equal(t, "proxy-"+testUser, f.Name())
assert.True(t, strings.HasPrefix(f.String(), "Local file system"))
// check it is in the cache
assert.Equal(t, 1, p.vfsCache.Entries())
cacheValue, ok := p.vfsCache.GetMaybe(testUser)
assert.True(t, ok)
assert.Equal(t, value, cacheValue)
})
t.Run("call w/PublicKey", func(t *testing.T) {
// check cache empty
assert.Equal(t, 0, p.vfsCache.Entries())
defer p.vfsCache.Clear()
vfs, vfsKey, err := p.Call(
testUser,
publicKeyString,
true,
)
require.NoError(t, err)
require.NotNil(t, vfs)
assert.Equal(t, "proxy-"+testUser, vfs.Fs().Name())
assert.Equal(t, testUser, vfsKey)
// check it is in the cache
assert.Equal(t, 1, p.vfsCache.Entries())
cacheValue, ok := p.vfsCache.GetMaybe(testUser)
assert.True(t, ok)
cacheEntry, ok := cacheValue.(cacheEntry)
assert.True(t, ok)
assert.Equal(t, vfs, cacheEntry.vfs)
// Test Get works while we have something in the cache
t.Run("Get", func(t *testing.T) {
assert.Equal(t, vfs, p.Get(testUser))
assert.Nil(t, p.Get("unknown"))
})
// now try again from the cache
vfs, vfsKey, err = p.Call(testUser, publicKeyString, true)
require.NoError(t, err)
require.NotNil(t, vfs)
assert.Equal(t, "proxy-"+testUser, vfs.Fs().Name())
assert.Equal(t, testUser, vfsKey)
// check cache is at the same level
assert.Equal(t, 1, p.vfsCache.Entries())
// now try again from the cache but with wrong public key
vfs, vfsKey, err = p.Call(testUser, publicKeyString+"wrong", true)
require.Error(t, err)
require.Contains(t, err.Error(), "incorrect public key")
require.Nil(t, vfs)
require.Equal(t, "", vfsKey)
// check cache is at the same level
assert.Equal(t, 1, p.vfsCache.Entries())
})
}

View File

@ -8,10 +8,10 @@ import (
"crypto/rsa"
"crypto/subtle"
"crypto/x509"
"encoding/base64"
"encoding/pem"
"fmt"
"io/ioutil"
"log"
"net"
"os"
"path/filepath"
@ -119,8 +119,13 @@ func (s *server) acceptConnections() {
func (s *server) serve() (err error) {
var authorizedKeysMap map[string]struct{}
// ensure the user isn't trying to use conflicting flags
if proxyflags.Opt.AuthProxy != "" && s.opt.AuthorizedKeys != "" && s.opt.AuthorizedKeys != DefaultOpt.AuthorizedKeys {
return errors.New("--auth-proxy and --authorized-keys cannot be used at the same time")
}
// Load the authorized keys
if s.opt.AuthorizedKeys != "" {
if s.opt.AuthorizedKeys != "" && proxyflags.Opt.AuthProxy == "" {
authKeysFile := env.ShellExpand(s.opt.AuthorizedKeys)
authorizedKeysMap, err = loadAuthorizedKeys(authKeysFile)
// If user set the flag away from the default then report an error
@ -142,7 +147,7 @@ func (s *server) serve() (err error) {
fs.Debugf(describeConn(c), "Password login attempt for %s", c.User())
if s.proxy != nil {
// query the proxy for the config
_, vfsKey, err := s.proxy.Call(c.User(), string(pass))
_, vfsKey, err := s.proxy.Call(c.User(), string(pass), false)
if err != nil {
return nil, err
}
@ -164,7 +169,21 @@ func (s *server) serve() (err error) {
PublicKeyCallback: func(c ssh.ConnMetadata, pubKey ssh.PublicKey) (*ssh.Permissions, error) {
fs.Debugf(describeConn(c), "Public key login attempt for %s", c.User())
if s.proxy != nil {
return nil, errors.New("public key login not allowed when using auth proxy")
//query the proxy for the config
_, vfsKey, err := s.proxy.Call(
c.User(),
base64.StdEncoding.EncodeToString(pubKey.Marshal()),
true,
)
if err != nil {
return nil, err
}
// just return the Key so we can get it back from the cache
return &ssh.Permissions{
Extensions: map[string]string{
"_vfsKey": vfsKey,
},
}, nil
}
if _, ok := authorizedKeysMap[string(pubKey.Marshal())]; ok {
return &ssh.Permissions{
@ -220,7 +239,7 @@ func (s *server) serve() (err error) {
// accepted.
s.listener, err = net.Listen("tcp", s.opt.ListenAddr)
if err != nil {
log.Fatal("failed to listen for connection", err)
return errors.Wrap(err, "failed to listen for connection")
}
fs.Logf(nil, "SFTP server listening on %v\n", s.listener.Addr())

View File

@ -17,7 +17,7 @@ import (
"github.com/rclone/rclone/fs"
"github.com/rclone/rclone/fs/config/configmap"
"github.com/rclone/rclone/fs/config/obscure"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
const (
@ -45,7 +45,7 @@ func TestSftp(t *testing.T) {
opt.Pass = testPass
w := newServer(f, &opt)
assert.NoError(t, w.serve())
require.NoError(t, w.serve())
// Read the host and port we started on
addr := w.Addr()

View File

@ -159,7 +159,7 @@ func (w *WebDAV) getVFS(ctx context.Context) (VFS *vfs.VFS, err error) {
// auth does proxy authorization
func (w *WebDAV) auth(user, pass string) (value interface{}, err error) {
VFS, _, err := w.proxy.Call(user, pass)
VFS, _, err := w.proxy.Call(user, pass, false)
if err != nil {
return nil, err
}

View File

@ -22,9 +22,9 @@ The server will log errors. Use -v to see access logs.
--bwlimit will be respected for file transfers. Use --stats to
control the stats printing.
You must provide some means of authentication, either with --user/--pass,
an authorized keys file (specify location with --authorized-keys - the
default is the same as ssh) or set the --no-auth flag for no
You must provide some means of authentication, either with `--user`/`--pass`,
an authorized keys file (specify location with `--authorized-keys` - the
default is the same as ssh), an `--auth-proxy`, or set the --no-auth flag for no
authentication when logging in.
Note that this also implements a small number of shell commands so
@ -183,11 +183,13 @@ rclone will use that program to generate backends on the fly which
then are used to authenticate incoming requests. This uses a simple
JSON based protocl with input on STDIN and output on STDOUT.
> **PLEASE NOTE:** `--auth-proxy` and `--authorized-keys` cannot be used together, if `--auth-proxy` is set the authorized keys option will be ignored.
There is an example program
[bin/test_proxy.py](https://github.com/rclone/rclone/blob/master/test_proxy.py)
in the rclone source code.
The program's job is to take a `user` and `pass` on the input and turn
The program's job is to take a `user` and `pass` or `public_key` on the input and turn
those into the config for a backend on STDOUT in JSON format. This
config will have any default parameters for the backend added, but it
won't use configuration from environment variables or command line
@ -200,7 +202,7 @@ This config generated must have this extra parameter
And it may have this parameter
- `_obscure` - comma separated strings for parameters to obscure
For example the program might take this on STDIN
If password authentication was used by the client, input to the proxy process (on STDIN) would look similar to this:
```
{
@ -209,7 +211,16 @@ For example the program might take this on STDIN
}
```
And return this on STDOUT
If public-key authentication was used by the client, input to the proxy process (on STDIN) would look similar to this:
```
{
"user": "me",
"public_key": "AAAAB3NzaC1yc2EAAAADAQABAAABAQDuwESFdAe14hVS6omeyX7edc+4BlQz1s6tWT5VxBu1YlR9w39BUAom4qDKuH+uqLMDIaS5F7D6lNwOuPylvyV/LgMFsgJV4QZ52Kws7mNgdsCEDTvfLz5Pt9Qtp6Gnah3kA0cmbXcfQFaO50Ojnz/W1ozg2z5evKmGtyYMtywTXvH/KVh5WjhbpQ/ERgu+1pbgwWkpWNBM8TCO8D85PSpxtkdpEdkaiGtKA6U+6ZOtdCqd88EasyMEBWLVSx9bvqMVsD8plYstXOm5CCptGWWqckZBIqp0YBP6atw/ANRESD3cIJ4dOO+qlWkLR5npAZZTx2Qqh+hVw6qqTFB+JQdf"
}
```
And as an example return this on STDOUT
```
{
@ -223,7 +234,7 @@ And return this on STDOUT
```
This would mean that an SFTP backend would be created on the fly for
the `user` and `pass` returned in the output to the host given. Note
the `user` and `pass`/`public_key` returned in the output to the host given. Note
that since `_obscure` is set to `pass`, rclone will obscure the `pass`
parameter before creating the backend (which is required for sftp
backends).
@ -235,8 +246,8 @@ in the output and the user to `user`. For security you'd probably want
to restrict the `host` to a limited list.
Note that an internal cache is keyed on `user` so only use that for
configuration, don't use `pass`. This also means that if a user's
password is changed the cache will need to expire (which takes 5 mins)
configuration, don't use `pass` or `public_key`. This also means that if a user's
password or public-key is changed the cache will need to expire (which takes 5 mins)
before it takes effect.
This can be used to build general purpose proxies to any kind of