Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/cli.yml
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ jobs:
shell: 'script -q -e -c "bash {0}"'
run: |
set -e
docker exec -it cosmos-openc3-redis-1 sh -c "echo -e 'AUTH openc3 openc3password\nset OPENC3__TOKEN 5e884898da28047151d0e56f8dc6292773603d0d6aabbdd62a11ef721d1542d8' | redis-cli"
docker exec -it cosmos-openc3-redis-1 sh -c "echo -e 'AUTH openc3 openc3password\nset OPENC3__TOKEN \"\$argon2id\$v=19\$m=8,t=1,p=1\$KEDp3bRbyFK3lJLMa99kzQ\$INGoDEdgRbAG/wVie/ftzh//If91eeMTQQ5HoyKcfvY\"' | redis-cli"
# list shows all the available file names
./openc3.sh cli script list | tee /dev/tty | grep "INST/procedures/stash.rb"
# spawning a script prints only a PID
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/playwright.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ on:
branches:
- "**"

env:
OPENC3_ARGON2_PROFILE: "unsafe_cheapest"

jobs:
openc3-build-test:
if: ${{ github.actor != 'dependabot[bot]' }}
Expand Down
2 changes: 1 addition & 1 deletion docs.openc3.com/docs/development/json-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,5 +128,5 @@ Note that you will need a valid session token in the `Authorization` header to a
```bash
curl http://localhost:2900/openc3-api/auth/verify \
-H 'Content-Type: application/json' \
-d '{"token": "your-password-here"}'
-d '{"password": "your-password-here"}'
```
10 changes: 10 additions & 0 deletions docs.openc3.com/docs/getting-started/upgrading.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,16 @@ The process should loook something like this:

5. Test and verify functionality and commit your changes.

## Migrating From COSMOS 6 to COSMOS 7

### Passwords

If you are using COSMOS Enterprise, you can skip this section. COSOMS 7 Core introduces some security enhancements around the handling of the user password.

First, we have switched the password hashing algorithm from SHA-256 to the industry standard argon2id. Before you can log in using COSMOS 7, you need to migrate the stored password hash used for user authentication. To do this, with Redis running, set the `OPENC3_API_PASSWORD` environment variable to your current password and run `openc3.sh cli migratepassword`. You can do this at any point of the upgrade process while COSMOS is running (e.g. either before tearing down COSMOS 6 or after starting up COSMOS 7).

Second, the JSON API no longer accepts plaintext passwords. You must instead use a session token. Please see the note at the bottom of our [JSON API documentation](../development/json-api#further-debugging) for how to acquire a session token for the API.

## Migrating From COSMOS 5 to COSMOS 6

:::info Developers Only
Expand Down
6 changes: 3 additions & 3 deletions openc3-cosmos-cmd-tlm-api/app/controllers/auth_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def token_exists

def verify
begin
if OpenC3::AuthModel.verify_no_service(params[:token], no_password: false)
if OpenC3::AuthModel.verify_no_service(params[:password], no_password: false)
render :plain => OpenC3::AuthModel.generate_session()
else
head :unauthorized
Expand All @@ -46,7 +46,7 @@ def verify

def verify_service
begin
if OpenC3::AuthModel.verify(params[:token], service_only: true)
if OpenC3::AuthModel.verify(params[:password], service_only: true)
render :plain => OpenC3::AuthModel.generate_session()
else
head :unauthorized
Expand All @@ -60,7 +60,7 @@ def verify_service
def set
begin
# Set throws an exception if it fails for any reason
OpenC3::AuthModel.set(params[:token], params[:old_token])
OpenC3::AuthModel.set(params[:password], params[:old_password])
OpenC3::Logger.info("Password changed", user: username())
render :plain => OpenC3::AuthModel.generate_session()
rescue StandardError => e
Expand Down
32 changes: 20 additions & 12 deletions openc3-cosmos-cmd-tlm-api/spec/controllers/auth_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
json = JSON.parse(response.body, allow_nan: true, create_additions: true)
expect(json).to eql({"result" => false})

post :set, params: { token: 'PASSWORD' }
post :set, params: { password: 'PASSWORD' }
expect(response).to have_http_status(:ok)

get :token_exists
Expand All @@ -41,23 +41,23 @@
end

describe "set" do
it "requires old_token after initial set" do
post :set, params: { token: 'PASSWORD' }
it "requires old_password after initial set" do
post :set, params: { password: 'PASSWORD' }
expect(response).to have_http_status(:ok)

post :set, params: { token: 'PASSWORD2' }
post :set, params: { password: 'PASSWORD2' }
expect(response).to have_http_status(:error)
json = JSON.parse(response.body, allow_nan: true, create_additions: true)
expect(json["status"]).to eql 'error'
expect(json["message"]).to eql 'old_token must not be nil or empty'
expect(json["message"]).to eql 'old_password must not be nil or empty'

post :set, params: { token: 'PASSWORD2', old_token: 'BAD' }
post :set, params: { password: 'PASSWORD2', old_password: 'BAD' }
expect(response).to have_http_status(:error)
json = JSON.parse(response.body, allow_nan: true, create_additions: true)
expect(json["status"]).to eql 'error'
expect(json["message"]).to eql 'old_token incorrect'
expect(json["message"]).to eql 'old_password incorrect'

post :set, params: { token: 'PASSWORD2', old_token: 'PASSWORD' }
post :set, params: { password: 'PASSWORD2', old_password: 'PASSWORD' }
expect(response).to have_http_status(:ok)
end
end
Expand All @@ -68,15 +68,23 @@
expect(response).to have_http_status(:unauthorized)
end

it "validates the set token" do
post :set, params: { token: 'PASSWORD' }
it "validates the set password" do
post :set, params: { password: 'PASSWORD' }
expect(response).to have_http_status(:ok)

post :verify, params: { token: 'PASSWORD' }
post :verify, params: { password: 'PASSWORD' }
expect(response).to have_http_status(:ok)

post :verify, params: { token: 'BAD' }
post :verify, params: { password: 'BAD' }
expect(response).to have_http_status(:unauthorized)
end

it "validates the service password" do
post :verify_service, params: { password: 'BAD' }
expect(response).to have_http_status(:unauthorized)

post :verify_service, params: { password: 'openc3service' }
expect(response).to have_http_status(:ok)
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
<v-row>
<v-btn
type="submit"
@click.prevent="() => verifyPassword()"
@click.prevent="() => verify()"
size="large"
color="success"
:disabled="!formValid"
Expand Down Expand Up @@ -140,7 +140,7 @@ export default {
},
mounted: function () {
if (localStorage.openc3Token) {
this.verifyPassword(localStorage.openc3Token, true)
this.verify(localStorage.openc3Token, true)
}
},
methods: {
Expand All @@ -159,12 +159,12 @@ export default {
window.location = '/'
}
},
verifyPassword: function (token, noAlert) {
token ||= this.password
verify: function (password, noAlert) {
password ||= this.password
this.showAlert = false
Api.post('/openc3-api/auth/verify', {
data: {
token,
password,
},
...this.options,
})
Expand All @@ -174,6 +174,11 @@ export default {
.catch((error) => {
if (error?.status === 401) {
this.alert = 'Incorrect password'
} else if (
error?.response?.data?.message === 'invalid password hash'
) {
this.alert =
'Please see the migration guide for upgrading to COSMOS 7 in our docs.'
} else {
this.alert = error.message || 'Something went wrong...'
}
Expand All @@ -185,8 +190,8 @@ export default {
this.showAlert = false
Api.post('/openc3-api/auth/set', {
data: {
old_token: this.oldPassword,
token: this.password,
old_password: this.oldPassword,
password: this.password,
},
...this.options,
})
Expand Down
1 change: 1 addition & 0 deletions openc3/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ gem 'rack-cors', '~> 3.0'
gem 'rails', '~> 7.2.0'
gem 'tzinfo-data'
gem 'ruby-termios', '>= 1.0'
gem 'argon2', '~> 2.3', '>= 2.3.2'
21 changes: 21 additions & 0 deletions openc3/bin/openc3cli
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ require 'redis'
require 'erb'
require 'irb'
require 'irb/completion'
require 'digest'
require 'argon2'

$redis_url = "redis://#{ENV['OPENC3_REDIS_HOSTNAME']}:#{ENV['OPENC3_REDIS_PORT']}"

Expand Down Expand Up @@ -869,6 +871,22 @@ def cli_script(args=[])
exit(ret_code)
end

def migrate_password_hash
password = ENV['OPENC3_API_PASSWORD']
argon2_profile = ENV["OPENC3_ARGON2_PROFILE"]&.to_sym || :rfc_9106_low_memory
if password.nil? or password.empty?
abort "OPENC3_API_PASSWORD environment variable is required for password migration"
end
redis = Redis.new(url: $redis_url, username: ENV['OPENC3_REDIS_USERNAME'], password: ENV['OPENC3_REDIS_PASSWORD'])
old_pw_hash = redis.get('OPENC3__TOKEN')
abort "Password hash already migrated" if old_pw_hash.start_with?('$argon2')
abort "OPENC3_API_PASSWORD incorrect" unless old_pw_hash == Digest::SHA256.hexdigest(password)
new_pw_hash = Argon2::Password.create(password, profile: argon2_profile)
redis.set('OPENC3__TOKEN', new_pw_hash)
puts "Password migrated successfully."
exit 0
end

if not ARGV[0].nil? # argument(s) given

# Handle each task
Expand Down Expand Up @@ -1331,6 +1349,9 @@ if not ARGV[0].nil? # argument(s) given
end
run_migrations(ARGV[1])

when 'migratepassword'
migrate_password_hash()

else # Unknown task
print_usage()
abort("Unknown task: #{ARGV[0]}")
Expand Down
74 changes: 46 additions & 28 deletions openc3/lib/openc3/models/auth_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,29 +20,41 @@
# This file may also be used under the terms of a commercial license
# if purchased from OpenC3, Inc.

require 'digest'
require 'argon2'
require 'securerandom'
require 'openc3/utilities/store'

module OpenC3
class AuthModel
PRIMARY_KEY = 'OPENC3__TOKEN'
SESSIONS_KEY = 'OPENC3__SESSIONS'
ARGON2_PROFILE = ENV["OPENC3_ARGON2_PROFILE"]&.to_sym || :rfc_9106_low_memory

TOKEN_CACHE_TIMEOUT = 5
# Redis keys
PRIMARY_KEY = 'OPENC3__TOKEN' # for argon2 password hash
SESSIONS_KEY = 'OPENC3__SESSIONS' # for hash containing session tokens

# The length of time in minutes to keep redis values in memory
PW_HASH_CACHE_TIMEOUT = 5
SESSION_CACHE_TIMEOUT = 5
@@token_cache = nil
@@token_cache_time = nil

# Cached argon2 password hash
@@pw_hash_cache = nil
@@pw_hash_cache_time = nil

# Cached session tokens
@@session_cache = nil
@@session_cache_time = nil

MIN_TOKEN_LENGTH = 8
MIN_PASSWORD_LENGTH = 8

def self.set?(key = PRIMARY_KEY)
Store.exists(key) == 1
end

# Checks whether the provided token is a valid user password, service password, or session token.
# @param token [String] the plaintext password or session token to check (required)
# @param no_password [Boolean] enforces use of a session token or service password (default: true)
# @param service_only [Boolean] enforces use of a service password (default: false)
# @return [Boolean] whether the provided password/token is valid
def self.verify(token, no_password: true, service_only: false)
# Handle a service password - Generally only used by ScriptRunner
# TODO: Replace this with temporary service tokens
Expand All @@ -54,55 +66,61 @@ def self.verify(token, no_password: true, service_only: false)
return verify_no_service(token, no_password: no_password)
end

# Checks whether the provided token is a valid user password or session token.
# @param token [String] the plaintext password or session token to check (required)
# @param no_password [Boolean] enforces use of a session token (default: true)
# @return [Boolean] whether the provided password/token is valid
def self.verify_no_service(token, no_password: true)
return false if token.nil? or token.empty?

# Check cached session tokens and password hash
time = Time.now
return true if @@session_cache and (time - @@session_cache_time) < SESSION_CACHE_TIMEOUT and @@session_cache[token]
token_hash = hash(token)
return true if @@token_cache and (time - @@token_cache_time) < TOKEN_CACHE_TIMEOUT and @@token_cache == token_hash
unless no_password
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing this unless no_password is actually a bug in my last PR

return true if @@pw_hash_cache and (time - @@pw_hash_cache_time) < PW_HASH_CACHE_TIMEOUT and Argon2::Password.verify_password(token, @@pw_hash_cache)
end

# Check sessions
# Check stored session tokens
@@session_cache = Store.hgetall(SESSIONS_KEY)
@@session_cache_time = time
return true if @@session_cache[token]

return false if no_password

# Check Direct password
@@token_cache = Store.get(PRIMARY_KEY)
@@token_cache_time = time
return true if @@token_cache == token_hash

return false
# Check stored password hash
pw_hash = Store.get(PRIMARY_KEY)
raise "invalid password hash" unless pw_hash.start_with?("$argon2") # Catch users who didn't run the migration utility when upgrading to COSMOS 7
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This triggers an alert on the login page that tells them to look at the migration guide, which tells them to run the password migration utility

@@pw_hash_cache = pw_hash
@@pw_hash_cache_time = time
return Argon2::Password.verify_password(token, @@pw_hash_cache)
end

def self.set(token, old_token, key = PRIMARY_KEY)
raise "token must not be nil or empty" if token.nil? or token.empty?
raise "token must be at least 8 characters" if token.length < MIN_TOKEN_LENGTH
def self.set(password, old_password, key = PRIMARY_KEY)
raise "password must not be nil or empty" if password.nil? or password.empty?
raise "password must be at least 8 characters" if password.length < MIN_PASSWORD_LENGTH

if set?(key)
raise "old_token must not be nil or empty" if old_token.nil? or old_token.empty?
raise "old_token incorrect" unless verify_no_service(old_token, no_password: false)
raise "old_password must not be nil or empty" if old_password.nil? or old_password.empty?
raise "old_password incorrect" unless verify_no_service(old_password, no_password: false)
end
Store.set(key, hash(token))
pw_hash = Argon2::Password.create(password, profile: ARGON2_PROFILE)
Store.set(key, pw_hash)
@@pw_hash_cache = nil
@@pw_hash_cache_time = nil
end

# Creates a new session token. DO NOT CALL BEFORE VERIFYING.
def self.generate_session
token = SecureRandom.urlsafe_base64(nil, false)
Store.hset(SESSIONS_KEY, token, Time.now.iso8601)
return token
end

# Terminates every session.
def self.logout
Store.del(SESSIONS_KEY)
@@sessions_cache = nil
@@sessions_cache_time = nil
end

def self.hash(token)
Digest::SHA256.hexdigest token
@@session_cache = nil
@@session_cache_time = nil
Comment on lines +122 to +123
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sessions_cache typo was a bug that allowed sessions to persist for 5min after logging out

end
end
end
2 changes: 1 addition & 1 deletion openc3/lib/openc3/utilities/authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def token(include_bearer: true)
end

def _make_auth_request(password)
Faraday.new.post(_generate_auth_url, '{"token": "' + password + '"}', {'Content-Type' => 'application/json'})
Faraday.new.post(_generate_auth_url, '{"password": "' + password + '"}', {'Content-Type' => 'application/json'})
end

def _generate_auth_url
Expand Down
1 change: 1 addition & 0 deletions openc3/openc3.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ spec = Gem::Specification.new do |s|
s.add_runtime_dependency 'rubyzip', '~> 3.0'
s.add_runtime_dependency 'uuidtools', '~> 2.2'
s.add_runtime_dependency 'yard', '~> 0.9'
s.add_runtime_dependency 'argon2', '~> 2.3'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryanmelt if the dependency is here does it also have to be the Gemfile? I think no because of the gemspec line in the Gemfile. But maybe the Gemfile has support the >= 2.3.2 and that's why it's in both? I also see that tzinfo-data is in both ... is that an oversight? We should probably add some comments one way or another.

# faraday includes faraday-net_http as the default adapter
s.add_runtime_dependency 'aws-sdk-s3', '< 2'
s.add_runtime_dependency 'cbor', '~> 0.5.10'
Expand Down
2 changes: 1 addition & 1 deletion openc3/python/openc3/utilities/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def __init__(self):
if not password:
raise OpenC3AuthenticationError("Authentication requires environment variable OPENC3_API_PASSWORD")
self.service = password == OPENC3_SERVICE_PASSWORD
response = Session().post(self._generate_auth_url(), json={"token": password}, headers={"Content-Type": "application/json"})
response = Session().post(self._generate_auth_url(), json={"password": password}, headers={"Content-Type": "application/json"})
self._token = response.text
if not self._token:
raise OpenC3AuthenticationError("Authentication failed. Please check the password in the environment variable OPENC3_API_PASSWORD")
Expand Down
Loading
Loading