Merge branch 'auditLog' into 'master'

record security relevant events

See merge request !61

.gitlab-ci.yml

@@ -3,6 +3,9 @@ stages:
 
 image: $CI_REGISTRY/immerda/container-images/ruby/devel:2.5
 
+variables:
+  GIT_SUBMODULE_STRATEGY: recursive
+
 simple_test:
   stage: simpletest
   tags:

.gitmodules

+[submodule "i18n"]
+	path = i18n
+	url = https://code.immerda.ch/immerda/apps/i18n_i5a.git

i18n @ b3a9493d

+Subproject commit b3a9493dc03becc57bb8f9243726e2443da8c11d

lib/iapi.rb

@@ -7,6 +7,7 @@ require 'thin'
 require 'singleton'
 require 'active_record'
 require 'bcrypt'
+require 'i18n'
 
 require 'iapi/utils/logger'
 require 'iapi/utils/mail_crypt'
@@ -46,6 +47,7 @@ require 'iapi/managers/jabber_manager'
 require 'iapi/managers/mail_manager'
 require 'iapi/managers/saml_manager'
 require 'iapi/managers/user_manager'
+require 'iapi/managers/transaction_manager'
 
 require 'iapi/delayed_jobs'
 require 'iapi/mailer'
@@ -96,6 +98,7 @@ class IApi < Sinatra::Base
     set :server, "thin"
     set :logger, IApiLog.logger
     set :logging, IApiLog.level
+    I18n.load_path = Dir["#{File.dirname(__FILE__)}/../i18n/*.yml"]
   end
 
   def self.run!

lib/iapi/mailer.rb

@@ -2,3 +2,4 @@ require 'iapi/mailers/base_mailer'
 require 'iapi/mailers/welcome_mail'
 require 'iapi/mailers/recovery_token_mail'
 require 'iapi/mailers/nudge_mail'
+require 'iapi/mailers/audit_mail'

lib/iapi/mailers/audit_mail.rb

+class AuditMail < BaseMailer
+  SUPPORTED_LANG = []
+  def notify(recipient, event, msg, language = nil)
+    language = if SUPPORTED_LANG.include? language then language else 'multi' end
+    mail(
+      to: recipient,
+      subject: "Einstellungsänderung / Changed settings",
+    ) do |format|
+      # https://github.com/rails/rails/issues/22045
+      self.action_name = language
+      # The msg is not rendered on purpose. It has some malicious potential,
+      # as it allows an attacker that compromised the account to embed content.
+      format.text {
+        render locals: { recipient: recipient, event: event }
+      }
+    end
+  end
+end

lib/iapi/mailers/views/audit_mail/multi.text.erb

+# encoding: utf-8
+Hallo <%= recipient %>
+
+[de]
+
+Soeben wurde folgende Kontoeinstellungen vorgenommen:
+
+<%= t(event, scope: :audit_event, locale: :de) %>
+
+Sollte diese Änderung nicht von dir stammen, kontaktiere uns so schnell wie möglich via admin@immerda.ch.
+
+Um deine Kontoeinstellungen zu überprüfen, gehe auf https://www.immerda.ch, klicke auf Dienste, dann Einstellungen.
+
+[en]
+
+The following settings have been changed on your account:
+
+<%= t(event, scope: :audit_event, locale: :en) %>
+
+If you did not make these changes, please contact us as quickly as possible via admin@immerda.ch.
+
+To configure your account and audit changes visit https://immerda.ch, click on the link called Dienste, then Einstellungen.
+
+immerda admin team

lib/iapi/managers/transaction_manager.rb

+class TransactionManager
+  class << self
+    def get_user_transactions(scope, user, user_secret_key, from=nil, to=nil)
+      from = DateTime.parse(from) if from.present?
+      to = DateTime.parse(to) if to.present?
+      entries = UserTransaction.where(email_user: user, scope: scope)
+      entries = entries.order(created_at: 'DESC')
+      entries = entries.where('created_at >= ?', from) if from
+      entries = entries.where('created_at < ?', to) if to
+      entries = entries.pluck(:encrypted, :value, :created_at)
+      entries.map do |e|
+        encrypted, value, created_at = e
+        value = Base64.decode64(value)
+        if encrypted
+          value = Sodium::Box.open(
+            IApiConf.transaction_log_key.pub,
+            user_secret_key,
+            value)
+        end
+        [value.force_encoding("utf-8"), created_at]
+      end
+    end
+
+    def add_user_transaction(scope, user, value, ttl=nil)
+      key = user.iapi_public_key
+      ttl = ttl.to_i if ttl.present?
+      data = {email_user: user, scope: scope, value: value}
+      if ttl.present?
+        data[:remove_at] = Time.now + ttl.days
+      end
+      if key
+        data[:value] = Sodium::Box.close(
+          key,
+          IApiConf.transaction_log_key.sec, data[:value])
+        data[:encrypted] = true
+      end
+      data[:value] = Base64.encode64(data[:value])
+      UserTransaction.create!(data)
+    end
+
+    def add_audit_log(user, event, msg = "")
+      add_user_transaction('audit_log', user, {event: event, msg: msg}.to_json, 90)
+      AuditMail.notify(user.email, event, msg).deliver
+    # Catching every exception is a bit dangerous, but I don't see an
+    # alternative as there are so many things that could happen.
+    rescue Exception => e
+      IApiLog.error("Failed to create audit event #{e}")
+    end
+  end
+end

lib/iapi/managers/user_manager.rb

@@ -90,7 +90,7 @@ class UserManager
             email, pass, keep_recovery_token: keep_token)
           if res[:state] == 'success'
             if options[:email_recovery_token]
-              generate_recovery_token_and_send_by_mail(email, pass, recovery_email)
+              generate_recovery_token_and_send_by_mail(email, pass, recovery_email, true)
             end
 
             return_token = options[:return_recovery_token]
@@ -99,7 +99,7 @@ class UserManager
               return { state: 'success' }
             end
 
-            mail_crypt_token = UserManager.generate_recovery_token(email, pass)
+            mail_crypt_token = UserManager.generate_recovery_token(email, pass, true)
             if mail_crypt_token[:state] == 'success'
               welcome_mail(email_user, options[:language])
               return {state: 'success',
@@ -122,6 +122,7 @@ class UserManager
             user.set_password(new_pwd)
           end
           user.save!
+          TransactionManager.add_audit_log(user, "pwchange")
           return { state: 'success' }
         end
       end
@@ -156,6 +157,7 @@ class UserManager
                   secret_box: app_pw[:secret_box],
               })
               if success
+                TransactionManager.add_audit_log(user, "apppw-add", pw_name)
                 return {state: 'success',
                         new_pw: app_pw[:pw]}
               end
@@ -172,14 +174,15 @@ class UserManager
       with_active_mailbox(email) do |user|
         if user.mail_crypt_enabled?
           if user.mail_crypt_extra_boxes.where(name: pw_name).destroy_all
-              return {state: 'success'}
+            TransactionManager.add_audit_log(user, "apppw-del", pw_name)
+            return {state: 'success'}
           end
         end
       end
       return { state: 'fail'}
     end
 
-    def generate_recovery_token(email, pwd)
+    def generate_recovery_token(email, pwd, no_audit=false)
       return { state: 'fail', msg: 'No issuer key found!' } unless IApiConf.mail_crypt_issuer_key
       with_active_mailbox(email) do |user|
         if res = AuthManager.mail_crypt_authenticate(pwd, user.mail_crypt_secret_box)
@@ -191,6 +194,9 @@ class UserManager
             user.mail_crypt_recovery_token = nil
             user.save!
           end
+          unless no_audit
+            TransactionManager.add_audit_log(user, "recoverytoken-gen")
+          end
           return {
             state: 'success',
             mail_crypt_recovery_token: token,
@@ -202,11 +208,11 @@ class UserManager
       return { state: 'fail'}
     end
 
-    def generate_recovery_token_and_send_by_mail(email, pass, receiver)
+    def generate_recovery_token_and_send_by_mail(email, pass, receiver, no_audit=false)
       unless allowed_to_receive_recovery_token(receiver)
         return {state: 'fail', msg: "invalid_recovery_token_receiver"}
       end
-      res = UserManager.generate_recovery_token(email, pass)
+      res = UserManager.generate_recovery_token(email, pass, no_audit)
       if res[:state] == 'success'
         recovery_token_mail(
           email, receiver, res[:mail_crypt_recovery_token])
@@ -266,6 +272,7 @@ class UserManager
             totp: secret,
             totp_last: current_totp,
           )
+          TransactionManager.add_audit_log(user, "totp-add", name)
           return { state: 'success' }
         else
           IApiLog.error "Error while verifying initial TOTP for #{email}"
@@ -279,6 +286,7 @@ class UserManager
       if (u = EmailUser.active_by_email(email)) && (t = u.totps.find_by(name: name))
         unless u.totps.count == 1 && u.web_authn_credentials.present?
           t.destroy!
+          TransactionManager.add_audit_log(u, "totp-del", name)
           return { state: 'success' }
         else
           IApiLog.error "Refuse to delete last TOTP for '#{email}' - User still has WebAuthnCredentials"
@@ -292,6 +300,7 @@ class UserManager
     def set_recovery_email(email, recovery_email)
       with_active_mailbox(email) do |user|
         user.set_recovery_email!(recovery_email.downcase)
+        TransactionManager.add_audit_log(user, "recoveryemail")
         return { state: 'success' }
       end
     end
@@ -341,6 +350,7 @@ class UserManager
           credential.update!(
             name: name,
             public_key: WebAuthn.standard_encoder.encode(webauthn_credential.credential.public_key))
+          TransactionManager.add_audit_log(user, "webauthn-add", name)
           return { state: 'success' }
         end
       end
@@ -351,6 +361,16 @@ class UserManager
       { state: 'webauthn verification failed' }
     end
 
+    def delete_webauthn_credential(email, name)
+      if user = EmailUser.active_by_email(email)
+        if user && (c = user.web_authn_credentials.find_by(name: name))
+          c.destroy!
+          TransactionManager.add_audit_log(user, "webauthn-del", name)
+          return true
+        end
+      end
+    end
+
     ############################
     # Admin API
     ###########################

lib/iapi/routes/resource.rb

@@ -19,6 +19,10 @@ class IApi < Sinatra::Base
     end
   end
 
+  def security_critical_resource?(kind)
+    ['mail_alias'].include?(kind)
+  end
+
   namespace '/resource' do
     get '/available' do
       return json result: 'success', available:
@@ -106,6 +110,10 @@ class IApi < Sinatra::Base
             self_create: true,
             allow_protected_alias:
               IApiConf.acl.has_access_to_admin_api?(authenticated_user.email))
+          # Log creation of security critical resources
+          if security_critical_resource?(kind)
+            TransactionManager.add_audit_log(owner, "#{kind}-add", res.to_api[:email])
+          end
           return json result: 'success', uid: res.uid
         end
       end
@@ -132,7 +140,14 @@ class IApi < Sinatra::Base
 
     post '/delete' do
       if (_, res = authorize_access)
+        owners = res.owners.to_a
+        kind = res.class.resource_name
         if res.destroy
+          if security_critical_resource?(kind)
+            owners.each do |o|
+              TransactionManager.add_audit_log(o, "#{kind}-del", res.to_api[:email])
+            end
+          end
           return json result: 'success'
         end
       end

lib/iapi/routes/transaction.rb

@@ -90,27 +90,12 @@ class IApi < Sinatra::Base
   namespace '/user_transaction' do
     get '/get' do
       scope = params['scope']
-      unless (verify_acl(scope, ['bill']))
+      unless (verify_acl(scope, ['bill','audit_log']))
         return client_error('invalid_scope')
       end
 
-      from = DateTime.parse(params['from']) if params['from']
-      to = DateTime.parse(params['to']) if params['to']
-      entries = UserTransaction.where(email_user: authenticated_user, scope: scope)
-      entries = entries.where('created_at >= ?', from) if from
-      entries = entries.where('created_at < ?', to) if to
-      entries = entries.pluck(:encrypted, :value, :created_at)
-      entries = entries.map do |e|
-        encrypted, value, created_at = e
-        value = Base64.decode64(value)
-        if encrypted
-          value = Sodium::Box.open(
-            IApiConf.transaction_log_key.pub,
-            api_user_secret_key,
-            value)
-        end
-        [value.force_encoding("utf-8"), created_at]
-      end
+      entries = TransactionManager.get_user_transactions(
+        scope, authenticated_user, api_user_secret_key, params['from'], params['to'])
       json(result: 'success', values: entries)
     end
 
@@ -128,22 +113,8 @@ class IApi < Sinatra::Base
           return client_error('invalid_target_user')
         end
       end
-      key = user.iapi_public_key
-      value = parsed_body['value']
-      ttl = parsed_body['ttl']
-      ttl = ttl.to_i if ttl.present?
-      data = {email_user: user, scope: scope, value: value}
-      if ttl.present?
-        data[:remove_at] = Time.now + ttl.days
-      end
-      if key
-        data[:value] = Sodium::Box.close(
-          key,
-          IApiConf.transaction_log_key.sec, data[:value])
-        data[:encrypted] = true
-      end
-      data[:value] = Base64.encode64(data[:value])
-      UserTransaction.create!(data)
+      TransactionManager.add_user_transaction(
+        scope, user, parsed_body['value'], parsed_body['ttl'])
       json result: 'success'
     end
   end

lib/iapi/routes/users.rb

@@ -351,9 +351,7 @@ class IApi < Sinatra::Base
         'deleting webauthn'
       )
       if name = parsed_body['webauthn_name']
-        u = authenticated_user
-        if u && (c = u.web_authn_credentials.find_by(name: name))
-          c.destroy!
+        if UserManager.delete_webauthn_credential(authenticated_user.email, name)
           return json result: 'success'
         end
       end

scripts/update_prod.sh

@@ -3,6 +3,7 @@
 base_path=$(dirname $(dirname $(readlink -f $0)))
 
 su - iapi -c "git pull -q"
+su - iapi -c "git submodule update --init"
 su - iapi -c "HOME=${base_path} scl enable rh-ruby25 -- ${base_path}/scripts/prepare_for_prod.sh"
 su - iapi -c "HOME=${base_path} scl enable rh-ruby25 'bundle exec ./db/migrate.rb'"
 

spec/spec_helper.rb

@@ -207,5 +207,7 @@ module RSpecMixin
   def app() IApi end
 end
 
+BaseMailer.delivery_method = :test
+
 # For RSpec 2.x and 3.x
 RSpec.configure { |c| c.include RSpecMixin }

spec/users/password_spec.rb

@@ -9,11 +9,13 @@ describe "iApi users change password" do
         eu = EmailUser.by_email(email)
         old_pwd = eu.password_crypt
         old_sb = eu.mail_crypt_secret_box
-        post_as('users', '/users/change_password',
-          email: email,
-          current_password: 'pwduser1',
-          new_password: new_pwd,
-        )
+        expect {
+          post_as('users', '/users/change_password',
+            email: email,
+            current_password: 'pwduser1',
+            new_password: new_pwd,
+          )
+        }.to change { AuditMail.deliveries.count }.by(1)
         expect(last_response).to be_ok
         eu = EmailUser.by_email(email)
         if eu.mail_crypt_enabled?

spec/users/totp_spec.rb

@@ -6,13 +6,15 @@ describe "iApi users totp route" do
       it 'sets a totp with correct current value' do
         email = 'user1@example.com'
         secret = ROTP::Base32.random_base32
-        post_as('users', '/users/create_totp',
-          password: 'pwduser1',
-          email: email,
-          name: 'mytotp',
-          secret: secret,
-          current_totp: ROTP::TOTP.new(secret).now.to_s,
-        )
+        expect {
+          post_as('users', '/users/create_totp',
+            password: 'pwduser1',
+            email: email,
+            name: 'mytotp',
+            secret: secret,
+            current_totp: ROTP::TOTP.new(secret).now.to_s,
+          )
+        }.to change { AuditMail.deliveries.count }.by(1)
         expect(last_response).to be_ok
         eu = EmailUser.by_email(email)
         expect(eu.has_2fa?).to be_truthy
@@ -70,11 +72,13 @@ describe "iApi users totp route" do
         expect(last_response).to be_ok
         eu = EmailUser.by_email(email)
         expect(eu.has_2fa?).to be_truthy
-        post_as('users', '/users/delete_totp',
-          password: 'pwduser1',
-          email: email,
-          name: 'mytotp'
-        )
+        expect {
+          post_as('users', '/users/delete_totp',
+            password: 'pwduser1',
+            email: email,
+            name: 'mytotp'
+          )
+        }.to change { AuditMail.deliveries.count }.by(1)
         expect(last_response).to be_ok
         eu = EmailUser.by_email(email)
         expect(eu.has_2fa?).to be_falsy

spec/users/webauthn_spec.rb

@@ -25,10 +25,12 @@ describe "iApi users route" do
         )
         expect(last_response).to be_ok
 
-        post_as('users', '/users/init_webauthn_registration',
-          email: email,
-          password: 'pwduser1',
-        )
+        expect {
+          post_as('users', '/users/init_webauthn_registration',
+            email: email,
+            password: 'pwduser1',
+          )
+        }.to change { AuditMail.deliveries.count }.by(0)
         expect(last_response).to be_ok
         cred_opts = last_response.parsed_body['credential_options']
         client = WebAuthn::FakeClient.new(IApiConf.webauthn_register_origin)
@@ -38,14 +40,16 @@ describe "iApi users route" do
         )
         response = public_key_credential['response']
 
-        post_as('users', '/users/verify_webauthn_registration',
-          email: email,
-          password: 'pwduser1',
-          webauthn_name: 'mykey',
-          attestation_object: response['attestationObject'],
-          client_data_json:   response['clientDataJSON'],
-          current_challenge: cred_opts['challenge'],
-        )
+        expect {
+          post_as('users', '/users/verify_webauthn_registration',
+            email: email,
+            password: 'pwduser1',
+            webauthn_name: 'mykey',
+            attestation_object: response['attestationObject'],
+            client_data_json:   response['clientDataJSON'],
+            current_challenge: cred_opts['challenge'],
+          )
+        }.to change { AuditMail.deliveries.count }.by(1)
         expect(last_response).to be_ok
 
         get_as('users', '/users/webauthn_credentials',
@@ -300,11 +304,13 @@ describe "iApi users route" do
         key = eu.web_authn_credentials.find_by(name: 'mykey')
         expect(key).to be_present
 
-        post_as('users', '/users/delete_webauthn_credential',
-          email: email,
-          password: 'pwduser1',
-          webauthn_name: 'mykey',
-        )
+        expect {
+          post_as('users', '/users/delete_webauthn_credential',
+            email: email,
+            password: 'pwduser1',
+            webauthn_name: 'mykey',
+          )
+        }.to change { AuditMail.deliveries.count }.by(1)
 
         expect(last_response).to be_ok
         key = EmailUser.active_by_email(email).web_authn_credentials.find_by(name: 'mykey')

tests/replay/scripts/encrypted-pw-change-and-recovery-gen.log

@@ -23,7 +23,7 @@
     storagehost: storage1.example.com
     mail_crypt_enabled: true
     recovery_email: ''
-    keep_recovery_token: true
+    keep_recovery_token: false
     admin_email: "user1@example.com"
     token: "%ADMIN_API_TOKEN%"
 :response:
@@ -122,6 +122,18 @@
   :body:
   - '{"email":"change-pw-test@private.example.com","result":"success","saml_response":"%%SAML_USER_REPLY%%","saml_acs_url":"%IGNORE%","login_token":"%IGNORE%"}'
 ---
+:request:
+  :method: :get
+  :path: "/user_transaction/get"
+  :payload:
+    email: change-pw-test@private.example.com
+    scope: audit_log
+    token: "%API_TOKEN%"
+:response:
+  :status: 200
+  :body:
+  - '{"result":"success", "values":[["{\"event\":\"recoverytoken-gen\",\"msg\":\"\"}", "%IGNORE%"], ["{\"event\":\"recoverytoken-gen\",\"msg\":\"\"}", "%IGNORE%"], ["{\"event\":\"recoveryemail\",\"msg\":\"\"}", "%IGNORE%"], ["{\"event\":\"pwchange\",\"msg\":\"\"}", "%IGNORE%"]]}'
+---
 :request:
   :method: :get
   :path: "/users_admin/info"