ncw/swift

Issue when providing an invalid token to authenticate (SIGSEGV)

giovannipizzi opened this issue · 4 comments

While testing my PR #1888 on the rclone repository I found what I think is a bug (and I imagine it's in the swift library, that's why I'm opening it here - if it's a bug in rclone, feel free to move this issue there).

Basically (when using the alternate auth method and providing StorageUrl and AuthToken only) if I provide an invalid token (wrong or expired), I get the following segmentation fault:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x16fcfe2]

goroutine 20 [running]:
github.com/ncw/rclone/swift.(*auth).Request(0xc4202f8320, 0xc42022e9a0, 0xc420026600, 0xc42027c900, 0x1bf32c0)
	<autogenerated>:1 +0x32
github.com/ncw/rclone/vendor/github.com/ncw/swift.(*Connection).authenticate(0xc42022e9a0, 0x1bf32c0, 0xc42022eaf0)
	/Users/pizzi/go/src/github.com/ncw/rclone/vendor/github.com/ncw/swift/swift.go:468 +0x161
github.com/ncw/rclone/vendor/github.com/ncw/swift.(*Connection).getUrlAndAuthToken(0xc42022e9a0, 0xc4201e43ce, 0x43, 0xc42017e040, 0xc4201e43ce, 0x43, 0x0, 0x0, 0x0, 0x0)
	/Users/pizzi/go/src/github.com/ncw/rclone/vendor/github.com/ncw/swift/swift.go:522 +0x10b
github.com/ncw/rclone/vendor/github.com/ncw/swift.(*Connection).Call(0xc42022e9a0, 0xc4201e43ce, 0x43, 0x0, 0x0, 0x0, 0x0, 0x1ba5acf, 0x3, 0xc4201820c0, ...)
	/Users/pizzi/go/src/github.com/ncw/rclone/vendor/github.com/ncw/swift/swift.go:677 +0x1b1
github.com/ncw/rclone/vendor/github.com/ncw/swift.(*Connection).storage(0xc42022e9a0, 0x0, 0x0, 0x0, 0x0, 0x1ba5acf, 0x3, 0xc4201820c0, 0x0, 0xc420190f00, ...)
	/Users/pizzi/go/src/github.com/ncw/rclone/vendor/github.com/ncw/swift/swift.go:779 +0x112
github.com/ncw/rclone/vendor/github.com/ncw/swift.(*Connection).Containers(0xc42022e9a0, 0xc420042be0, 0x0, 0x2210af0, 0x0, 0x0, 0x2)
	/Users/pizzi/go/src/github.com/ncw/rclone/vendor/github.com/ncw/swift/swift.go:877 +0x18c
github.com/ncw/rclone/vendor/github.com/ncw/swift.(*Connection).ContainersAll(0xc42022e9a0, 0x0, 0x0, 0x0, 0xc420042f08, 0x0, 0x0)
	/Users/pizzi/go/src/github.com/ncw/rclone/vendor/github.com/ncw/swift/swift.go:914 +0x128
github.com/ncw/rclone/swift.(*Fs).listContainers(0xc4202f0c40, 0x0, 0x0, 0xc420042f6c, 0x2, 0x2, 0xc420042f68, 0x2)
	/Users/pizzi/go/src/github.com/ncw/rclone/swift/swift.go:425 +0x5e
github.com/ncw/rclone/swift.(*Fs).List(0xc4202f0c40, 0x0, 0x0, 0x0, 0xc420025300, 0x0, 0x0, 0x0)
	/Users/pizzi/go/src/github.com/ncw/rclone/swift/swift.go:447 +0x4c
github.com/ncw/rclone/fs.ListDirSorted(0x2192d00, 0xc4202f0c40, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
	/Users/pizzi/go/src/github.com/ncw/rclone/fs/operations.go:629 +0x63
github.com/ncw/rclone/fs.walk.func2(0xc4202bb1f0, 0xc4201c7380, 0x1bf1a68, 0x2192d00, 0xc4202f0c40, 0x0, 0xc4202bb1e8, 0xc4202f8340, 0xc4202bb200, 0xc4202beea0, ...)
	/Users/pizzi/go/src/github.com/ncw/rclone/fs/walk.go:120 +0x1e3
created by github.com/ncw/rclone/fs.walk
	/Users/pizzi/go/src/github.com/ncw/rclone/fs/walk.go:112 +0x265

My very little experience with go makes me think that clone asks the swift library to authenticate. The library tries once; if it manages, all goes well (as this is the behaviour when the token is valid). Otherwise, it tries to reauthenticate, but in this case there is some unallocated memory that is referenced.

I don't know if this is due to a problem in the swift library itself, or because in rclone some internal variables of the connection object are not set up properly, or something else...

ncw commented

I think this is a problem with rclone, not the swift library

In this code

	if storageURL != "" {
		f.c.StorageUrl = storageURL
		f.c.Auth = newAuth(f.c.Auth, storageURL)
	}

I think f.c.Auth is nil at this point hence the crash when c.Auth.Request is called, this tries to call the method on the embedded Authenticator which is nil and KABOOM.

Not 100% sure what the fix is though - can you investigate from here?

I can try to give a look in my spare time, but I don't know how I can help, as I said I don't really know go :-)
Maybe there is a simple solution:
in that snippet of code, if storageURL != "" and f.c.Auth is nil, then print an error message and quit: in the end it means that the storage URL was set, but no authentication was specified, meaning the user specified a AuthToken? In that case, rclone could say "you gave me directly the auth token for the swift config XXX, but this is invalid or expired, please get a new valid token and try again".

Would this be a viable solution?

ncw commented

Would this be a viable solution?

Alas, no, because that would mean it would always error if authenticating with a token.

I think what it needs is a different kind of authenticator...

I'm going to attempt to move this issue to rclone - hold tight!

ncw commented

This issue was moved to rclone/rclone#1919