From 97bb1934dd9028bff8ed2e8ae664aac0b6e7d295 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 24 Jan 2017 10:24:46 +0100 Subject: [PATCH 1/3] feat: Add nonce to authentication The nonce is meant to be stored in the client applications session. It is then transferred to the server as part of the redirect url. The server in turn includes it in the ticket. This binds the ticket to the user session. This prevents an attacker from authenticating a target as themselves. It also makes it impossible to reuse a leaked ticket in a different browser session (if the session is temper resistent). --- CHANGELOG.md | 12 ++++++++++++ lib/rbsso/authentication.rb | 13 ++++++++----- lib/rbsso/client.rb | 9 ++++++++- lib/rbsso/server.rb | 5 +++-- test/rbsso/authentication_test.rb | 24 ++++++++++++++++++++---- test/rbsso/client_test.rb | 20 ++++++++++++++++++-- test/rbsso/integration_test.rb | 7 +++++-- test/rbsso/server_test.rb | 8 +++++++- test/test_data.rb | 8 ++++++++ 9 files changed, 89 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1aaaea4..e27840b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,15 @@ +#### API Changes + +Server#ticket now takes an arguments hash instead of order arguments. + +Replace: + server.ticket user, service, domain +With: + server.ticket user: user, service: service, domain: domain + +This allows us to introduce nounce as an option without introducing a +fourth argument. + ### 0.2.2 (2017-01-13) diff --git a/lib/rbsso/authentication.rb b/lib/rbsso/authentication.rb index 44cff2b..4be3e2b 100644 --- a/lib/rbsso/authentication.rb +++ b/lib/rbsso/authentication.rb @@ -8,20 +8,22 @@ def initialize(version) end end - attr_reader :user, :service, :domain, :groups, :expires + attr_reader :user, :service, :domain, :groups, :nonce, :expires - def initialize(user:, service:, domain:, groups: [], ttl: 3600, expires: nil) + def initialize(user:, service:, domain:, groups: [], nonce: nil, ttl: 3600, expires: nil) @user, @service, @domain, @groups = user, service, domain, groups + @nonce = nonce @expires = expires || (Time.now + ttl).to_i end def self.parse(string) - version, user, service, domain, expires, groups = string.split '|' + version, user, service, domain, expires, nonce, groups = string.split '|' check_version(version) new user: user, service: service, domain: domain, expires: expires.to_i, + nonce: nonce, groups: (groups || '').split(',') end @@ -34,7 +36,7 @@ def to_info end def content - [VERSION, user, service, domain, expires.to_s, groups.join(',')] + [VERSION, user, service, domain, expires.to_s, nonce, groups.join(',')] end def ==(other) @@ -42,7 +44,8 @@ def ==(other) service == other.service && domain == other.domain && groups == other.groups && - expires == other.expires + expires == other.expires && + nonce == other.nonce end def expired? diff --git a/lib/rbsso/client.rb b/lib/rbsso/client.rb index 9847907..8713758 100644 --- a/lib/rbsso/client.rb +++ b/lib/rbsso/client.rb @@ -16,6 +16,12 @@ def initialize(expected, was) end end + class NonceMismatch < RuntimeError + def initialize(expected, was) + super "Ticket nonce '#{was}' differs from session nonce '#{expected}'." + end + end + def initialize(service, key) if !key || key !~ /[0-9a-f]{64}/i raise ArgumentError, "key MUST be 32 bytes, hex encoded string, was: #{key}" @@ -25,11 +31,12 @@ def initialize(service, key) @verify_key = key end - def open(ticket_string) + def open(ticket_string, nonce: nil) ticket = RbSSO::Ticket.open ticket_string, verify_key auth = RbSSO::Authentication.parse ticket.content raise TicketExpired.new(auth.expires) if auth.expired? raise WrongService.new(service, auth.service) if auth.service != service + raise NonceMismatch.new(nonce, auth.nonce) if auth.nonce != nonce auth.to_info end diff --git a/lib/rbsso/server.rb b/lib/rbsso/server.rb index b7a6da9..69f11f2 100644 --- a/lib/rbsso/server.rb +++ b/lib/rbsso/server.rb @@ -13,10 +13,11 @@ def initialize(secret) @key = RbNaCl::SigningKey.new seed_binary end - def ticket(user, service, domain) + def ticket(user:, service:, domain:, nonce: nil) auth = RbSSO::Authentication.new user: user, service: service, - domain: domain + domain: domain, + nonce: nonce ticket = RbSSO::Ticket.sign auth, key return ticket.to_base64 end diff --git a/test/rbsso/authentication_test.rb b/test/rbsso/authentication_test.rb index edc3eea..87a642a 100644 --- a/test/rbsso/authentication_test.rb +++ b/test/rbsso/authentication_test.rb @@ -13,20 +13,36 @@ def test_to_s assert_match /^3|user|service|domain|\d*|$/, auth.to_s end + def test_to_s_with_nonce + assert_match /^3|user|service|domain|\d*|nonce|$/, + auth_with_nonce.to_s + end + def test_parse parsed = RbSSO::Authentication.parse auth.to_s assert_equal auth, parsed end + def test_parse_with_nonce + parsed = RbSSO::Authentication.parse auth_with_nonce.to_s + assert_equal auth_with_nonce, parsed + end + def test_invalid_version assert_raises RbSSO::Authentication::VersionMismatch do RbSSO::Authentication.parse '4|other versions may contain other auth' end end - def auth - RbSSO::Authentication.new user: 'user', - service: 'service', - domain: 'domain' + def auth(user: 'user', service: 'service', domain: 'domain', nonce: nil) + RbSSO::Authentication.new user: user, + service: service, + domain: domain, + nonce: nonce end + + def auth_with_nonce + auth nonce: 'nonce' + end + end diff --git a/test/rbsso/client_test.rb b/test/rbsso/client_test.rb index eb871fb..eec68e0 100644 --- a/test/rbsso/client_test.rb +++ b/test/rbsso/client_test.rb @@ -18,18 +18,34 @@ def test_check_key_content end def test_open_ticket + auth = client.open(current_ticket_string) + assert auth + end + + def test_open_ticket_with_nonce + auth = client.open(current_ticket_with_nonce_string, nonce: 'nonce') + assert auth + end + + def test_reject_expired_ticket assert_raises Client::TicketExpired do client.open(static_ticket_string) end end - def test_reject_ticket + def test_reject_wrong_service assert_raises Client::WrongService do client(service: 'other_service').open(current_ticket_string) end end - def client(service: 'service/', key: nil) + def test_reject_missing_nonce + assert_raises Client::NonceMismatch do + client.open(current_ticket_string, nonce: 'nonce') + end + end + + def client(service: 'service', key: nil) Client.new(service, key || verify_key_hex) end diff --git a/test/rbsso/integration_test.rb b/test/rbsso/integration_test.rb index 595e3c9..9ca7c23 100644 --- a/test/rbsso/integration_test.rb +++ b/test/rbsso/integration_test.rb @@ -7,11 +7,14 @@ class IntegrationTest < Minitest::Test def test_server_client_flow # server server = RbSSO::Server.new seed - ticket = server.ticket("user", "service/", "domain") + ticket = server.ticket user: "user", + service: "service/", + domain: "domain", + nonce: "nonce" # client client = RbSSO::Client.new 'service/', server.verify_key - info = client.open(ticket) + info = client.open(ticket, nonce: "nonce") assert_equal 'user', info[:name] assert_equal 'user@domain', info[:email] end diff --git a/test/rbsso/server_test.rb b/test/rbsso/server_test.rb index e3472ad..9d5f672 100644 --- a/test/rbsso/server_test.rb +++ b/test/rbsso/server_test.rb @@ -12,7 +12,13 @@ def test_check_seed def test_ticket_content assert_match /3|user|service|domain|14/, - Base64.urlsafe_decode64(server.ticket(user, service, domain)) + Base64.urlsafe_decode64(ticket) + end + + def ticket + server.ticket user: user, + service: service, + domain: domain end def server diff --git a/test/test_data.rb b/test/test_data.rb index acc2abd..61a579a 100644 --- a/test/test_data.rb +++ b/test/test_data.rb @@ -7,6 +7,10 @@ def current_content "3|user|service|domain|#{Time.now.to_i}|" end + def current_content_with_nonce + "3|user|service|domain|#{Time.now.to_i}|nonce|" + end + def signing_key seed_binary = [seed].pack('H*') RbNaCl::SigningKey.new seed_binary @@ -31,4 +35,8 @@ def current_ticket_string def static_ticket_string "loFbFifM6T_WJfe8D9Jyr80KXWxnBYNeJUoUA2PiSZi-Q_zSbFNu6gI-ujcDHTOq90GivY5nngTDz94C4zpgDjN8dXNlcnxzZXJ2aWNlfGRvbWFpbnwxNDgzOTY0NDkyfA==" end + + def current_ticket_with_nonce_string + RbSSO::Ticket.sign(current_content_with_nonce, signing_key).to_base64 + end end From bb5d3102f752bf904561e8cff70245e748590890 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 24 Jan 2017 11:42:27 +0100 Subject: [PATCH 2/3] api: ticket version 4 - nonce support --- lib/rbsso/authentication.rb | 2 +- test/rbsso/authentication_test.rb | 2 +- test/rbsso/client_test.rb | 2 +- test/test_data.rb | 24 ++++++++++++++++++------ 4 files changed, 21 insertions(+), 9 deletions(-) diff --git a/lib/rbsso/authentication.rb b/lib/rbsso/authentication.rb index 4be3e2b..0ad0e10 100644 --- a/lib/rbsso/authentication.rb +++ b/lib/rbsso/authentication.rb @@ -1,6 +1,6 @@ module RbSSO class Authentication - VERSION = 3 + VERSION = 4 class VersionMismatch < ArgumentError def initialize(version) diff --git a/test/rbsso/authentication_test.rb b/test/rbsso/authentication_test.rb index 87a642a..c75ab05 100644 --- a/test/rbsso/authentication_test.rb +++ b/test/rbsso/authentication_test.rb @@ -30,7 +30,7 @@ def test_parse_with_nonce def test_invalid_version assert_raises RbSSO::Authentication::VersionMismatch do - RbSSO::Authentication.parse '4|other versions may contain other auth' + RbSSO::Authentication.parse '2|other versions may contain other auth' end end diff --git a/test/rbsso/client_test.rb b/test/rbsso/client_test.rb index eec68e0..930eca8 100644 --- a/test/rbsso/client_test.rb +++ b/test/rbsso/client_test.rb @@ -29,7 +29,7 @@ def test_open_ticket_with_nonce def test_reject_expired_ticket assert_raises Client::TicketExpired do - client.open(static_ticket_string) + client.open(expired_ticket_string) end end diff --git a/test/test_data.rb b/test/test_data.rb index 61a579a..b635beb 100644 --- a/test/test_data.rb +++ b/test/test_data.rb @@ -1,14 +1,14 @@ module TestData - def static_content - '3|user|service|domain|1483964492|' + def expired_content + "4|user|service|domain|#{Time.now.to_i - 1000}|" end def current_content - "3|user|service|domain|#{Time.now.to_i}|" + "4|user|service|domain|#{Time.now.to_i}|" end def current_content_with_nonce - "3|user|service|domain|#{Time.now.to_i}|nonce|" + "4|user|service|domain|#{Time.now.to_i}|nonce|" end def signing_key @@ -32,11 +32,23 @@ def current_ticket_string RbSSO::Ticket.sign(current_content, signing_key).to_base64 end - def static_ticket_string - "loFbFifM6T_WJfe8D9Jyr80KXWxnBYNeJUoUA2PiSZi-Q_zSbFNu6gI-ujcDHTOq90GivY5nngTDz94C4zpgDjN8dXNlcnxzZXJ2aWNlfGRvbWFpbnwxNDgzOTY0NDkyfA==" + def expired_ticket_string + RbSSO::Ticket.sign(expired_content, signing_key).to_base64 end def current_ticket_with_nonce_string RbSSO::Ticket.sign(current_content_with_nonce, signing_key).to_base64 end + + # This is outdated content but we continue to use it to + # test the signing and base64 encoding of the tickets. + + def static_content + '3|user|service|domain|1483964492|' + end + + def static_ticket_string + "loFbFifM6T_WJfe8D9Jyr80KXWxnBYNeJUoUA2PiSZi-Q_zSbFNu6gI-ujcDHTOq90GivY5nngTDz94C4zpgDjN8dXNlcnxzZXJ2aWNlfGRvbWFpbnwxNDgzOTY0NDkyfA==" + end + end From 17a3a2e46f90aae3f916e356c52777f037365c22 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 24 Jan 2017 10:37:07 +0100 Subject: [PATCH 3/3] Version 0.3.0 Features: * Add nonce to authentication ([97bb193](/../../commit/97bb193)) API Changes: Ticket version 4 to indicate nonce support Server#ticket now takes an arguments hash instead of order arguments. Replace: `server.ticket user, service, domain` With: `server.ticket user: user, service: service, domain: domain` This allows us to introduce nounce as an option without introducing a fourth argument. --- CHANGELOG.md | 10 ++++++++++ rbsso.gemspec | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e27840b..0dff409 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ + +### 0.3.0 (2017-01-24) + + +#### Features + +* Add nonce to authentication ([97bb193](/../../commit/97bb193)) + #### API Changes +Ticket version now is 4 to indicate support for nonces. + Server#ticket now takes an arguments hash instead of order arguments. Replace: diff --git a/rbsso.gemspec b/rbsso.gemspec index d41594c..9bc5b17 100644 --- a/rbsso.gemspec +++ b/rbsso.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |s| s.name = 'rbsso' - s.version = '0.2.2' + s.version = '0.3.0' s.licenses = ['MIT'] s.summary = "Ruby implementation for ai's libsso" s.description = <<-EODESC