Commit 35286350 authored by Robert Speicher's avatar Robert Speicher

Merge remote-tracking branch 'dev/master'

parents eb091439 873c4932
Please view this file on the master branch, on stable branches it's out of date.
## 10.5.3 (2018-03-01)
### Security (2 changes)
- Project can no longer be shared between groups when both member and group locks are active.
- Prevent new push rules from using non-RE2 regexes.
### Fixed (1 change)
- Fix LDAP group sync no longer configurable for regular users.
## 10.5.2 (2018-02-25)
- No changes.
......@@ -82,6 +94,18 @@ Please view this file on the master branch, on stable branches it's out of date.
- Remove unaproved typo check in sast:container report.
## 10.4.5 (2018-03-01)
### Security (2 changes)
- Project can no longer be shared between groups when both member and group locks are active.
- Prevent new push rules from using non-RE2 regexes.
### Fixed (1 change)
- Fix LDAP group sync no longer configurable for regular users.
## 10.4.4 (2018-02-16)
### Fixed (4 changes)
......@@ -183,6 +207,18 @@ Please view this file on the master branch, on stable branches it's out of date.
- Make scoped issue board specs more reliable.
## 10.3.8 (2018-03-01)
### Security (2 changes)
- Project can no longer be shared between groups when both member and group locks are active.
- Prevent new push rules from using non-RE2 regexes.
### Fixed (1 change)
- Fix LDAP group sync no longer configurable for regular users.
## 10.3.7 (2018-02-05)
### Security (1 change)
......
......@@ -2,6 +2,13 @@
documentation](doc/development/changelog.md) for instructions on adding your own
entry.
## 10.5.3 (2018-03-01)
### Security (1 change)
- Ensure that OTP backup codes are always invalidated.
## 10.5.2 (2018-02-25)
### Fixed (7 changes)
......@@ -219,6 +226,13 @@ entry.
- Adds empty state illustration for pending job.
## 10.4.5 (2018-03-01)
### Security (1 change)
- Ensure that OTP backup codes are always invalidated.
## 10.4.4 (2018-02-16)
### Security (1 change)
......@@ -443,6 +457,13 @@ entry.
- Use a background migration for issues.closed_at.
## 10.3.8 (2018-03-01)
### Security (1 change)
- Ensure that OTP backup codes are always invalidated.
## 10.3.7 (2018-02-05)
### Security (4 changes)
......
......@@ -2,6 +2,7 @@ class Projects::GroupLinksController < Projects::ApplicationController
layout 'project_settings'
before_action :authorize_admin_project!
before_action :authorize_admin_project_member!, only: [:update]
before_action :authorize_group_share!, only: [:create]
def index
redirect_to namespace_project_settings_members_path
......@@ -42,6 +43,10 @@ class Projects::GroupLinksController < Projects::ApplicationController
protected
def authorize_group_share!
access_denied! unless project.allowed_to_share_with_group?
end
def group_link_params
params.require(:group_link).permit(:group_access, :expires_at)
end
......
......@@ -450,6 +450,10 @@ module ProjectsHelper
end
end
def project_can_be_shared?
!membership_locked? || @project.allowed_to_share_with_group?
end
def membership_locked?
if @project.group && @project.group.membership_lock
true
......@@ -458,6 +462,24 @@ module ProjectsHelper
end
end
def share_project_description
share_with_group = @project.allowed_to_share_with_group?
share_with_members = !membership_locked?
project_name = content_tag(:strong, @project.name)
member_message = "You can add a new member to #{project_name}"
description =
if share_with_group && share_with_members
"#{member_message} or share it with another group."
elsif share_with_group
"You can share #{project_name} with another group."
elsif share_with_members
"#{member_message}."
end
description.to_s.html_safe
end
def readme_cache_key
sha = @project.commit.try(:sha) || 'nil'
[@project.full_path, sha, "readme"].join('-')
......
class UntrustedRegexpValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value)
return unless value
Gitlab::UntrustedRegexp.new(value)
rescue RegexpError => e
record.errors.add(attribute, "not valid RE2 syntax: #{e.message}")
end
end
- page_title "Members"
- can_admin_project_members = can?(current_user, :admin_project_member, @project)
.row.prepend-top-default
.col-lg-12
%h4
Project members
- if can?(current_user, :admin_project_member, @project)
%p
You can add a new member to
%strong= @project.name
or share it with another group.
- else
%p
Members can be added by project
%i Masters
or
%i Owners
- if project_can_be_shared?
%h4
Project members
- if can_admin_project_members
%p= share_project_description
- else
%p
Members can be added by project
%i Masters
or
%i Owners
.light
- if can?(current_user, :admin_project_member, @project)
%ul.nav-links.gitlab-tabs{ role: 'tablist' }
- if !membership_locked?
- if can_admin_project_members && project_can_be_shared?
- if !membership_locked? && @project.allowed_to_share_with_group?
%ul.nav-links.gitlab-tabs{ role: 'tablist' }
%li.active{ role: 'presentation' }
%a{ href: '#add-member-pane', id: 'add-member-tab', data: { toggle: 'tab' }, role: 'tab' } Add member
- if @project.allowed_to_share_with_group?
%li{ role: 'presentation', class: ('active' if membership_locked?) }
%a{ href: '#share-with-group-pane', id: 'share-with-group-tab', data: { toggle: 'tab' }, role: 'tab' } Share with group
.tab-content.gitlab-tab-content
- if !membership_locked?
.tab-content.gitlab-tab-content
.tab-pane.active{ id: 'add-member-pane', role: 'tabpanel' }
= render 'projects/project_members/new_project_member', tab_title: 'Add member'
.tab-pane{ id: 'share-with-group-pane', role: 'tabpanel', class: ('active' if membership_locked?) }
= render 'projects/project_members/new_shared_group', tab_title: 'Share with group'
.tab-pane{ id: 'share-with-group-pane', role: 'tabpanel', class: ('active' if membership_locked?) }
= render 'projects/project_members/new_shared_group', tab_title: 'Share with group'
- elsif !membership_locked?
.add-member= render 'projects/project_members/new_project_member', tab_title: 'Add member'
- elsif @project.allowed_to_share_with_group?
.share-with-group= render 'projects/project_members/new_shared_group', tab_title: 'Share with group'
= render 'shared/members/requests', membership_source: @project, requesters: @requesters
.clearfix
......
---
title: Project can no longer be shared between groups when both member and group locks
are active
merge_request:
author:
type: security
---
title: Prevent new push rules from using non-RE2 regexes
merge_request:
author:
type: security
---
title: Fix LDAP group sync no longer configurable for regular users
merge_request:
author:
type: fixed
class AddRegexpUsesRe2ToPushRules < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
# Default value to true for new values while keeping NULL for existing ones
add_column :push_rules, :regexp_uses_re2, :boolean
change_column_default :push_rules, :regexp_uses_re2, true
end
def down
remove_column :push_rules, :regexp_uses_re2
end
end
......@@ -2060,6 +2060,7 @@ ActiveRecord::Schema.define(version: 20180306074045) do
t.string "branch_name_regex"
t.boolean "reject_unsigned_commits"
t.boolean "commit_committer_check"
t.boolean "regexp_uses_re2", default: true
end
add_index "push_rules", ["is_sample"], name: "index_push_rules_on_is_sample", where: "is_sample", using: :btree
......
......@@ -340,7 +340,6 @@ data before running `pg_basebackup`.
echo Cleaning up old cluster directory
sudo -u postgres rm -rf /var/opt/gitlab/postgresql/data
rm -f /tmp/postgresql.trigger
echo Starting base backup as the replicator user
echo Enter the password for $USER@$HOST
......@@ -350,7 +349,6 @@ data before running `pg_basebackup`.
sudo -u postgres bash -c "cat > /var/opt/gitlab/postgresql/data/recovery.conf <<- _EOF1_
standby_mode = 'on'
primary_conninfo = 'host=$HOST port=$PORT user=$USER password=$PASSWORD sslmode=$SSLMODE'
trigger_file = '/tmp/postgresql.trigger'
_EOF1_
"
......
......@@ -66,14 +66,14 @@ The following options are available.
| Check whether committer is the current authenticated user | **Premium** 10.2 | GitLab will reject any commit that was not committed by the current authenticated user |
| Check whether commit is signed through GPG | **Premium** 10.1 | Reject commit when it is not signed through GPG. Read [signing commits with GPG][signing-commits]. |
| Prevent committing secrets to Git | **Starter** 8.12 | GitLab will reject any files that are likely to contain secrets. Read [what files are forbidden](#prevent-pushing-secrets-to-the-repository). |
| Restrict by commit message | **Starter** 7.10 | Only commit messages that match this Ruby regular expression are allowed to be pushed. Leave empty to allow any commit message. |
| Restrict by branch name | **Starter** 9.3 | Only branch names that match this Ruby regular expression are allowed to be pushed. Leave empty to allow any branch name. |
| Restrict by commit author's email | **Starter** 7.10 | Only commit author's email that match this Ruby regular expression are allowed to be pushed. Leave empty to allow any email. |
| Prohibited file names | **Starter** 7.10 | Any committed filenames that match this Ruby regular expression are not allowed to be pushed. Leave empty to allow any filenames. |
| Restrict by commit message | **Starter** 7.10 | Only commit messages that match this regular expression are allowed to be pushed. Leave empty to allow any commit message. Uses multiline mode, which can be disabled using `(?-m)`. |
| Restrict by branch name | **Starter** 9.3 | Only branch names that match this regular expression are allowed to be pushed. Leave empty to allow any branch name. |
| Restrict by commit author's email | **Starter** 7.10 | Only commit author's email that match this regular expression are allowed to be pushed. Leave empty to allow any email. |
| Prohibited file names | **Starter** 7.10 | Any committed filenames that match this regular expression are not allowed to be pushed. Leave empty to allow any filenames. |
| Maximum file size | **Starter** 7.12 | Pushes that contain added or updated files that exceed this file size (in MB) are rejected. Set to 0 to allow files of any size. |
>**Tip:**
You can check your regular expressions at <http://rubular.com>.
GitLab uses [RE2 syntax](https://github.com/google/re2/wiki/Syntax) for regular expressions in push rules. You can check your regular expressions at <https://regex-golang.appspot.com>.
## Prevent pushing secrets to the repository
......
class PushRule < ActiveRecord::Base
MatchError = Class.new(StandardError)
REGEX_COLUMNS = %i[
force_push_regex
delete_branch_regex
commit_message_regex
author_email_regex
file_name_regex
branch_name_regex
].freeze
belongs_to :project
validates :project, presence: true, unless: "is_sample?"
validates :max_file_size, numericality: { greater_than_or_equal_to: 0, only_integer: true }
validates(*REGEX_COLUMNS, untrusted_regexp: true)
before_update :convert_to_re2
FILES_BLACKLIST = YAML.load_file(Rails.root.join('ee/lib/gitlab/checks/files_blacklist.yml'))
SETTINGS_WITH_GLOBAL_DEFAULT = %i[
......@@ -43,7 +55,7 @@ class PushRule < ActiveRecord::Base
end
def commit_message_allowed?(message)
data_match?(message, commit_message_regex)
data_match?(message, commit_message_regex, multiline: true)
end
def branch_name_allowed?(branch)
......@@ -94,9 +106,15 @@ class PushRule < ActiveRecord::Base
private
def data_match?(data, regex)
def data_match?(data, regex, multiline: false)
if regex.present?
!!(data =~ Regexp.new(regex))
regexp = if allow_regex_fallback?
Gitlab::UntrustedRegexp.with_fallback(regex, multiline: multiline)
else
Gitlab::UntrustedRegexp.new(regex, multiline: multiline)
end
regexp === data
else
true
end
......@@ -104,6 +122,16 @@ class PushRule < ActiveRecord::Base
raise MatchError, "Regular expression '#{regex}' is invalid: #{e.message}"
end
def convert_to_re2
self.regexp_uses_re2 = true
end
# Allow fallback to ruby regex library
# Only supported for existing regexes due to denial of service risk
def allow_regex_fallback?
!regexp_uses_re2?
end
def read_setting_with_global_default(setting)
value = read_attribute(setting)
......
module API
class Ldap < Grape::API
before { authenticated_as_admin! }
# Admin users by default should be able to access these API endpoints.
# However, non-admin users can access these endpoints if the "Allow group
# owners to manage LDAP-related group settings" is enabled, and they own a
# group.
before { authenticated_with_ldap_admin_access! }
resource :ldap do
helpers do
......
......@@ -26,6 +26,21 @@ module EE
render_api_error!("SHA does not match HEAD of source branch: #{merge_request.diff_head_sha}", 409)
end
end
# Normally, only admin users should have access to see LDAP
# groups. However, due to the "Allow group owners to manage LDAP-related
# group settings" setting, any group owner can sync LDAP groups with
# their project.
#
# In the future, we should also check that the user has access to manage
# a specific group so that we can use the Ability class.
def authenticated_with_ldap_admin_access!
authenticate!
forbidden! unless current_user.admin? ||
::Gitlab::CurrentSettings.current_application_settings
.allow_group_owners_to_manage_ldap
end
end
end
end
......@@ -13,6 +13,71 @@ describe PushRule do
describe "Validation" do
it { is_expected.to validate_presence_of(:project) }
it { is_expected.to validate_numericality_of(:max_file_size).is_greater_than_or_equal_to(0).only_integer }
it 'validates RE2 regex syntax' do
push_rule = build(:push_rule, branch_name_regex: '(ee|ce).*\1')
expect(push_rule).not_to be_valid
expect(push_rule.errors.full_messages.join).to match /invalid escape sequence/
end
end
it 'defaults regexp_uses_re2 to true' do
push_rule = create(:push_rule)
expect(push_rule.regexp_uses_re2).to eq(true)
end
it 'updates regexp_uses_re2 to true on edit' do
push_rule = create(:push_rule, regexp_uses_re2: nil)
expect do
push_rule.update!(branch_name_regex: '.*')
end.to change(push_rule, :regexp_uses_re2).to true
end
describe '#branch_name_allowed?' do
subject(:push_rule) { create(:push_rule, branch_name_regex: '\d+\-.*')}
it 'checks branch against regex' do
expect(subject.branch_name_allowed?('123-feature')).to be true
expect(subject.branch_name_allowed?('feature-123')).to be false
end
it 'uses RE2 regex engine' do
expect_any_instance_of(Gitlab::UntrustedRegexp).to receive(:===)
subject.branch_name_allowed?('123-feature')
end
context 'with legacy regex' do
before do
push_rule.update_column(:regexp_uses_re2, nil)
end
it 'attempts to use safe RE2 regex engine' do
expect_any_instance_of(Gitlab::UntrustedRegexp).to receive(:===)
subject.branch_name_allowed?('ee-feature-ee')
end
it 'falls back to ruby regex engine' do
push_rule.update_column(:branch_name_regex, '(ee|ce).*\1')
expect(subject.branch_name_allowed?('ee-feature-ee')).to be true
expect(subject.branch_name_allowed?('ee-feature-ce')).to be false
end
end
end
describe '#commit_message_allowed?' do
subject(:push_rule) { create(:push_rule, commit_message_regex: '^Signed-off-by')}
it 'uses multiline regex' do
commit_message = "Some git commit feature\n\nSigned-off-by: Someone"
expect(subject.commit_message_allowed?(commit_message)).to be true
end
end
describe '#commit_validation?' do
......
......@@ -17,6 +17,7 @@ describe API::Ldap do
allow(Gitlab::Auth::LDAP::Config).to receive(:enabled?).and_return(true)
allow(Gitlab::Auth::LDAP::Adapter).to receive(:new).and_return(adapter)
allow(adapter).to receive_messages(groups: groups)
stub_application_setting(allow_group_owners_to_manage_ldap: false)
end
describe "GET /ldap/groups" do
......@@ -34,6 +35,20 @@ describe API::Ldap do
end
end
context 'when group owners are allowed to manage LDAP' do
before do
stub_application_setting(allow_group_owners_to_manage_ldap: true)
end
it "returns an array of ldap groups" do
get api("/ldap/groups", user)
expect(response.status).to eq 200
expect(json_response).to be_an Array
expect(json_response.length).to eq 2
expect(json_response.first['cn']).to eq 'developers'
end
end
context "when authenticated as admin" do
it "returns an array of ldap groups" do
get api("/ldap/groups", admin)
......@@ -60,6 +75,20 @@ describe API::Ldap do
end
end
context 'when group owners are allowed to manage LDAP' do
before do
stub_application_setting(allow_group_owners_to_manage_ldap: true)
end
it "returns an array of ldap groups" do
get api("/ldap/ldapmain/groups", admin)
expect(response.status).to eq 200
expect(json_response).to be_an Array
expect(json_response.length).to eq 2
expect(json_response.first['cn']).to eq 'developers'
end
end
context "when authenticated as admin" do
it "returns an array of ldap groups" do
get api("/ldap/ldapmain/groups", admin)
......
......@@ -11,7 +11,11 @@ module Gitlab
class UntrustedRegexp
delegate :===, to: :regexp
def initialize(pattern)
def initialize(pattern, multiline: false)
if multiline
pattern = "(?m)#{pattern}"
end
@regexp = RE2::Regexp.new(pattern, log_errors: false)
raise RegexpError.new(regexp.error) unless regexp.ok?
......@@ -31,6 +35,19 @@ module Gitlab
RE2.Replace(text, regexp, rewrite)
end
# Handles regular expressions with the preferred RE2 library where possible
# via UntustedRegex. Falls back to Ruby's built-in regular expression library
# when the syntax would be invalid in RE2.
#
# One difference between these is `(?m)` multi-line mode. Ruby regex enables
# this by default, but also handles `^` and `$` differently.
# See: https://www.regular-expressions.info/modifiers.html
def self.with_fallback(pattern, multiline: false)
UntrustedRegexp.new(pattern, multiline: multiline)
rescue RegexpError
Regexp.new(pattern)
end
private
attr_reader :regexp
......
......@@ -21,6 +21,18 @@ describe Projects::GroupLinksController do
end
end
context 'when project is not allowed to be shared with a group' do
before do
group.update_attributes(share_with_group_lock: false)
end
include_context 'link project to group'
it 'responds with status 404' do
expect(response).to have_gitlab_http_status(404)
end
end
context 'when user has access to group he want to link project to' do
before do
group.add_developer(user)
......
......@@ -7,18 +7,47 @@ feature 'Project > Members > Share with Group', :js do
let(:master) { create(:user) }
describe 'Share with group lock' do
shared_examples 'the project can be shared with groups' do
scenario 'the "Share with group" tab exists' do
shared_examples 'the project cannot be shared with groups' do
scenario 'user is only able to share with members' do
visit project_settings_members_path(project)
expect(page).to have_selector('#share-with-group-tab')
expect(page).not_to have_selector('#add-member-tab')
expect(page).not_to have_selector('#share-with-group-tab')
expect(page).not_to have_selector('.share-with-group')
expect(page).to have_selector('.add-member')
end
end
shared_examples 'the project cannot be shared with groups' do
scenario 'the "Share with group" tab does not exist' do
shared_examples 'the project cannot be shared with members' do
scenario 'user is only able to share with groups' do
visit project_settings_members_path(project)
expect(page).to have_selector('#add-member-tab')
expect(page).not_to have_selector('#add-member-tab')
expect(page).not_to have_selector('#share-with-group-tab')
expect(page).not_to have_selector('.add-member')
expect(page).to have_selector('.share-with-group')
end
end
shared_examples 'the project cannot be shared with groups and members' do
scenario 'no tabs or share content exists' do
visit project_settings_members_path(project)
expect(page).not_to have_selector('#add-member-tab')
expect(page).not_to have_selector('#share-with-group-tab')
expect(page).not_to have_selector('.add-member')
expect(page).not_to have_selector('.share-with-group')
end
end
shared_examples 'the project can be shared with groups and members' do
scenario 'both member and group tabs exist' do
visit project_settings_members_path(project)
expect(page).not_to have_selector('.add-member')
expect(page).not_to have_selector('.share-with-group')
expect(page).to have_selector('#add-member-tab')
expect(page).to have_selector('#share-with-group-tab')
end
end
......@@ -31,8 +60,8 @@ feature 'Project > Members > Share with Group', :js do
sign_in(master)
end
context 'when the group has "Share with group lock" disabled' do
it_behaves_like 'the project can be shared with groups'
context 'when the group has "Share with group lock" and "Member lock" disabled' do
it_behaves_like 'the project can be shared with groups and members'
scenario 'the project can be shared with another group' do
visit project_settings_members_path(project)
......@@ -56,10 +85,25 @@ feature 'Project > Members > Share with Group', :js do
it_behaves_like 'the project cannot be shared with groups'
end
context 'when the group has membership lock enabled' do
before do
project.namespace.update_column(:membership_lock, true)
end
it_behaves_like 'the project cannot be shared with members'
end
context 'when the group has membership lock and "Share with group lock" enabled' do
before do
project.namespace.update_attributes(share_with_group_lock: true, membership_lock: true)
end
it_behaves_like 'the project cannot be shared with groups and members'
end
end
context 'for a project in a subgroup', :nested_groups do
let!(:group_to_share_with) { create(:group) }
let(:root_group) { create(:group) }
let(:subgroup) { create(:group, parent: root_group) }
let(:project) { create(:project, namespace: subgroup) }
......@@ -69,9 +113,9 @@ feature 'Project > Members > Share with Group', :js do
sign_in(master)
end
context 'when the root_group has "Share with group lock" disabled' do
context 'when the subgroup has "Share with group lock" disabled' do
it_behaves_like 'the project can be shared with groups'
context 'when the root_group has "Share with group lock" and membership lock disabled' do
context 'when the subgroup has "Share with group lock" and membership lock disabled' do
it_behaves_like 'the project can be shared with groups and members'
end
context 'when the subgroup has "Share with group lock" enabled' do
......@@ -81,24 +125,67 @@ feature 'Project > Members > Share with Group', :js do
it_behaves_like 'the project cannot be shared with groups'
end
context 'when the subgroup has membership lock enabled' do
before do
subgroup.update_column(:membership_lock, true)
end
it_behaves_like 'the project cannot be shared with members'
end
context 'when the group has membership lock and "Share with group lock" enabled' do
before do
subgroup.update_attributes(share_with_group_lock: true, membership_lock: true)
end
it_behaves_like 'the project cannot be shared with groups and members'
end
end
context 'when the root_group has "Share with group lock" enabled' do
context 'when the root_group has "Share with group lock" and membership lock enabled' do
before do
root_group.update_column(:share_with_group_lock, true)
root_group.update_attributes(share_with_group_lock: true, membership_lock: true)
subgroup.reload
end
# This behaviour should be changed to disable sharing with members as well
# See: https://gitlab.com/gitlab-org/gitlab-ce/issues/42093
it_behaves_like 'the project cannot be shared with groups'
context 'when the subgroup has "Share with group lock" and membership lock disabled (parent overridden)' do
before do
subgroup.update_attributes(share_with_group_lock: false, membership_lock: false)
end
it_behaves_like 'the project can be shared with groups and members'
end
context 'when the subgroup has "Share with group lock" disabled (parent overridden)' do
it_behaves_like 'the project can be shared with groups'
# This behaviour should be changed to disable sharing with members as well
# See: https://gitlab.com/gitlab-org/gitlab-ce/issues/42093
context 'when the subgroup has membership lock enabled (parent overridden)' do
before do
subgroup.update_column(:membership_lock, true)
end
it_behaves_like 'the project cannot be shared with groups and members'
end
context 'when the subgroup has "Share with group lock" enabled' do
context 'when the subgroup has "Share with group lock" enabled (parent overridden)' do
before do
subgroup.update_column(:share_with_group_lock, true)
end
it_behaves_like 'the project cannot be shared with groups'
end
context 'when the subgroup has "Share with group lock" and membership lock enabled' do
before do
subgroup.update_attributes(membership_lock: true, share_with_group_lock: true)
end
it_behaves_like 'the project cannot be shared with groups and members'
end
end
end
end
......
......@@ -39,6 +39,14 @@ describe Gitlab::UntrustedRegexp do
expect(result).to be_falsy
end
it 'can handle regular expressions in multiline mode' do
regexp = described_class.new('^\d', multiline: true)
result = regexp === "Header\n\n1. Content"
expect(result).to be_truthy
end
end
describe '#scan' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment