Skip to content

Commit

Permalink
Ignore cookies that fail decryption (#1470)
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewmcgarvey authored Apr 12, 2021
1 parent 4c0e776 commit b19f716
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 36 deletions.
20 changes: 16 additions & 4 deletions spec/lucky/cookies/cookie_jar_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,26 @@ describe Lucky::CookieJar do
jar.get?(:name).should be_nil
end

it "raises helpful error if trying to read unencrypted values" do
it "returns nil if fails to decrypt value" do
jar = Lucky::CookieJar.empty_jar

jar.set_raw(:name, "Jane")

expect_raises Exception, "cookies.get_raw(:name).value" do
jar.get?(:name)
end
jar.get?(:name).should be_nil
end

it "parses encrypted cookies as expected" do
# meant to be a regression test to make sure we don't
# accidentally break cookie decryption
#
# this cookie was created with Lucky 0.27
cookie_key = "cookie_key"
cookie_value = "bHVja3k=--hY71kbRfob4pb9NS7wJpWKOBRhF+kwYPsHRQQanyXzGSKsCO6MIHCZfRBxDRqqm6"
cookies = HTTP::Cookies.new
cookies[cookie_key] = cookie_value
jar = Lucky::CookieJar.from_request_cookies(cookies)

JSON.parse(jar.get(cookie_key)).should eq({"key" => "value", "abc" => "123"})
end

describe "#set" do
Expand Down
2 changes: 1 addition & 1 deletion spec/spec_helper.cr
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Lucky::Session.configure do |settings|
end

Lucky::Server.configure do |settings|
settings.secret_key_base = Random::Secure.base64(32)
settings.secret_key_base = "EPzB4/PA/JZxEhISPr7Ad5X+G73exX+qg8IKFjqwdx0="
settings.host = "0.0.0.0"
settings.port = 8080
end
Expand Down
41 changes: 10 additions & 31 deletions src/lucky/cookies/cookie_jar.cr
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
class Lucky::CookieJar
MAX_COOKIE_SIZE = 4096
LUCKY_ENCRYPTION_PREFIX = Base64.strict_encode("lucky") + "--"
LEGACY_LUCKY_ENCRYPTION_PREFIX = Base64.encode("lucky") + "--"
MAX_COOKIE_SIZE = 4096
LUCKY_ENCRYPTION_PREFIX = Base64.strict_encode("lucky") + "--"
alias Key = String | Symbol
private property cookies
private property set_cookies
Expand Down Expand Up @@ -143,41 +142,21 @@ class Lucky::CookieJar
end
end

private def decrypt(cookie_value : String, cookie_name : String) : String
if encrypted_with_lucky?(cookie_value)
base_64_encrypted_part = cookie_value.lchop(LUCKY_ENCRYPTION_PREFIX)
decoded = Base64.decode(base_64_encrypted_part)
String.new(encryptor.decrypt(decoded))
elsif encrypted_with_legacy?(cookie_value)
decrypt(update_from_legacy_value(cookie_value), cookie_name)
else
raise <<-ERROR
It looks like this cookie's value is not encrypted by Lucky. This likely means the cookie was set by something other than Lucky, like a client side script.
You can access the raw value by using 'get_raw':
cookies.get_raw(:#{cookie_name}).value
ERROR
end
end
private def decrypt(cookie_value : String, cookie_name : String) : String?
return unless encrypted_with_lucky?(cookie_value)

private def update_from_legacy_value(value : String) : String
decoded_value = URI.decode_www_form(value)
LUCKY_ENCRYPTION_PREFIX + decoded_value.lchop(LEGACY_LUCKY_ENCRYPTION_PREFIX)
base_64_encrypted_part = cookie_value.lchop(LUCKY_ENCRYPTION_PREFIX)
decoded = Base64.decode(base_64_encrypted_part)
String.new(encryptor.decrypt(decoded))
rescue e
# an error happened while decrypting the cookie
# we will treat that as if no cookie was passed
end

private def encrypted_with_lucky?(value : String) : Bool
value.starts_with?(LUCKY_ENCRYPTION_PREFIX)
end

# legacy encrypted values had a \n between the encoded lucky and -- and were also www form encoded
# this allows apps made before 0.27.0 to not have to log all users out
private def encrypted_with_legacy?(value : String) : Bool
decoded_value = URI.decode_www_form(value)
decoded_value.starts_with?(LEGACY_LUCKY_ENCRYPTION_PREFIX)
end

@_encryptor : Lucky::MessageEncryptor?

private def encryptor : Lucky::MessageEncryptor
Expand Down

0 comments on commit b19f716

Please sign in to comment.