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

add support for URL signing #11

Closed
willnorris opened this Issue Jan 25, 2015 · 21 comments

Comments

Projects
None yet
3 participants
@willnorris
Copy link
Owner

willnorris commented Jan 25, 2015

Instead of just URL whitelisting, @teodor-pripoae has been working on support for URL signing, which would allow proxying from any remote host without risk of the proxy being abused. Beginning work can be seen at teodor-pripoae/imageproxy@willnorris:master...0b31315. I think the implementation can be simplified a bit, but starting this issue to track this feature.

@toubib

This comment has been minimized.

Copy link
Contributor

toubib commented May 11, 2015

Hi,

I'm very interested by this feature. Are you working on it ? Can I help in any ways ?

Thanks

@willnorris

This comment has been minimized.

Copy link
Owner

willnorris commented May 11, 2015

I have not yet started on it. What @teodor-pripoae has is definitely the right approach in terms of simply using an hmac over the request. A few things I would probably change from his implementation:

  • move the signature into Options
  • run the HMAC over Request (we probably need to give it a String method for this). That way the signature is independent of whatever path you're running the image proxy behind (edit: actually on second thought, I don't think his implementation used the image proxy path either)
  • specify the HMAC secret as a file which contains the actual data. Otherwise, it leaks the secret to anyone on the machine that could just run ps
@willnorris

This comment has been minimized.

Copy link
Owner

willnorris commented May 12, 2015

Okay, so I'm taking a first pass at these changes. One thing I ran into that hasn't been addressed is the interaction between the host whitelist and the signature. My current thinking is:

If the proxy has either a whitelist or signature key specified, then one of the two must be valid for all requests. If the remote URL matches a whitelisted host, serve the request. If the signature is valid, serve the request. Otherwise, deny the request.

The net effect of that is that if only a host whitelist is specified, then all requests must match the whitelist. If only a signature key is specified, then all requests must be signed.

If the proxy has neither a whitelist or a signature key, then serve all requests.

Does that match your expectations or at least sound reasonable?

@willnorris

This comment has been minimized.

Copy link
Owner

willnorris commented May 12, 2015

I have a working implementation in the signature branch: https://github.com/willnorris/imageproxy/compare/signature

There's still at least one thing I'm going to change... Currently, the signature covers both the remote URL and the options (minus the signature itself, of course). I'm afraid this is going to be complicated for any code that is constructing these signatures (since it would have to know the canonicalization rules for converting Options to a string), so I'm going to back it off to just sign the URL.

@willnorris

This comment has been minimized.

Copy link
Owner

willnorris commented May 12, 2015

okay, it now only signs the URL. Take a look and see if that will work for you (still need to write some docs though)

@toubib

This comment has been minimized.

Copy link
Contributor

toubib commented May 12, 2015

I think whitelist and signing can be exclusive because whitelist is useless if you sign your requests, and signing is useful only if used on all request, so I'm ok with your implementation.

It's also fine for me to sign only the url, my only goal is to prevent free use of the proxy without to have to restrict domains, because I can't have a specific domain list in my use case.

I'll do some tests and come back to you then.

@willnorris

This comment has been minimized.

Copy link
Owner

willnorris commented May 12, 2015

docs added in 6446923 if that helps.

@toubib

This comment has been minimized.

Copy link
Contributor

toubib commented May 12, 2015

I fail to generate correct signature.

I tried with openssl/base64 (it's not url safe but that's not the problem here) tools or many web encoders and nothing look like your examples.

$ echo "https://octodex.github.com/images/codercat.jpg" | openssl dgst -sha256 -hmac "secret key"
(stdin)= a8fca8ca044207d1557dd41b48658fb8a7a7083903db8a540188499c2a42da03

$ echo a8fca8ca044207d1557dd41b48658fb8a7a7083903db8a540188499c2a42da03|base64
YThmY2E4Y2EwNDQyMDdkMTU1N2RkNDFiNDg2NThmYjhhN2E3MDgzOTAzZGI4YTU0MDE4ODQ5OWMyYTQyZGEwMwo=

It doesn't look like XyMwWKIC5JPCtlYOQ2f4yMBTqpjtUsfI67Sp7huXIYY=

Tried with http://www.freeformatter.com/hmac-generator.html#ad-output and http://www.ebase64encode.org but it doesn't work neither (and it adds a '\n' at the end).

I will try to reuse your go code to create the signature.

@toubib

This comment has been minimized.

Copy link
Contributor

toubib commented May 12, 2015

Ok it works with this code:

package main

import (
        "os"
        "fmt"
        "crypto/hmac"
        "crypto/sha256"
        "encoding/base64"
)

func main() {
        mac := hmac.New(sha256.New, []byte(os.Args[1]))
        mac.Write([]byte(os.Args[2]))
        want := mac.Sum(nil)
        fmt.Println("result: ",base64.URLEncoding.EncodeToString(want))
}
@toubib

This comment has been minimized.

Copy link
Contributor

toubib commented May 12, 2015

I've found how to work with openssl, it needs the binary output option:

 /usr/local/go/bin/go run main.go "test" "https://www.google.fr/images/srpr/logo11w.png"
result:  RYifAJRfbhsitJeOrDNxWURCCkPsVR4ihCPXNv-ePbA=

echo -n "https://www.google.fr/images/srpr/logo11w.png" | openssl dgst -sha256 -hmac "test" -binary|base64| tr '/+' '_-'
RYifAJRfbhsitJeOrDNxWURCCkPsVR4ihCPXNv-ePbA=
@toubib

This comment has been minimized.

Copy link
Contributor

toubib commented May 12, 2015

And in java:

import org.apache.commons.codec.binary.Base64;
import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;

class EncodeUrl {

  public static String encode(String key, String data) throws Exception {
    Mac sha256_HMAC = Mac.getInstance("HmacSHA256");
    SecretKeySpec secret_key = new SecretKeySpec(key.getBytes(), "HmacSHA256");
    sha256_HMAC.init(secret_key);

    return Base64.encodeBase64URLSafeString(sha256_HMAC.doFinal(data.getBytes()));
  }

  public static void main(String [] args) throws Exception {
    System.out.println(encode(args[0], args[1]));
  }

}
$ java -cp commons-codec-1.10.jar:. EncodeUrl test https://www.google.fr/images/srpr/logo11w.png
RYifAJRfbhsitJeOrDNxWURCCkPsVR4ihCPXNv-ePbA
@willnorris

This comment has been minimized.

Copy link
Owner

willnorris commented May 12, 2015

And in ruby (modified from this gist):

require 'openssl'
require 'Base64'

key = "test"
data = "https://www.google.fr/images/srpr/logo11w.png"
puts Base64.urlsafe_encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('sha256'), key, data)).strip()
% ruby sign.rb
RYifAJRfbhsitJeOrDNxWURCCkPsVR4ihCPXNv-ePbA=
@willnorris

This comment has been minimized.

Copy link
Owner

willnorris commented May 12, 2015

also, your java example is missing the padding at the end. Which is at least mentioned in the docs, but still annoying.

@willnorris

This comment has been minimized.

Copy link
Owner

willnorris commented May 12, 2015

Looking at https://code.google.com/p/go/issues/detail?id=4237, it shouldn't be too hard to have this accept signatures without padding... working on adding that now.

@willnorris

This comment has been minimized.

Copy link
Owner

willnorris commented May 13, 2015

fixed in c8583cf ... now accepts signatures without padding.

@willnorris

This comment has been minimized.

Copy link
Owner

willnorris commented May 13, 2015

@toubib any other thoughts or feedback before merging this into master?

@toubib

This comment has been minimized.

Copy link
Contributor

toubib commented May 13, 2015

It's ok for me

@willnorris

This comment has been minimized.

Copy link
Owner

willnorris commented May 14, 2015

merged into master. thanks for helping test this! 🤘

@willnorris willnorris closed this May 14, 2015

@rooty123

This comment has been minimized.

Copy link

rooty123 commented Apr 21, 2017

What will happen if someone is going to abuse server by sending signed requests with variable range of dimensions and options? How signing will help in that case? I mean the intruder is able to eat all disk space if you have on-disk caching enabled.

@willnorris

This comment has been minimized.

Copy link
Owner

willnorris commented Apr 21, 2017

What will happen if someone is going to abuse server by sending signed requests with variable range of dimensions and options? How signing will help in that case? I mean the intruder is able to eat all disk space if you have on-disk caching enabled.

That's correct, the URL signing doesn't protect against this. It was designed to lock down which remote images can be proxied, not necessarily as a protection against an attacker trying to fill disk space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment