Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add an admin option to shared secret registration (breaks backwards compat) #909

Merged
merged 6 commits into from
Jul 6, 2016
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
32 changes: 26 additions & 6 deletions scripts/register_new_matrix_user
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,26 @@ import urllib2
import yaml


def request_registration(user, password, server_location, shared_secret):
def request_registration(user, password, server_location, shared_secret, admin=False):
mac = hmac.new(
key=shared_secret,
msg=user,
digestmod=hashlib.sha1,
).hexdigest()
)

mac.update(user)
mac.update("\x00")
mac.update(password)
mac.update("\x00")
mac.update("admin" if admin else "notadmin")

mac = mac.hexdigest()

data = {
"user": user,
"password": password,
"mac": mac,
"type": "org.matrix.login.shared_secret",
"admin": admin,
}

server_location = server_location.rstrip("/")
Expand Down Expand Up @@ -68,7 +76,7 @@ def request_registration(user, password, server_location, shared_secret):
sys.exit(1)


def register_new_user(user, password, server_location, shared_secret):
def register_new_user(user, password, server_location, shared_secret, admin):
if not user:
try:
default_user = getpass.getuser()
Expand Down Expand Up @@ -99,7 +107,14 @@ def register_new_user(user, password, server_location, shared_secret):
print "Passwords do not match"
sys.exit(1)

request_registration(user, password, server_location, shared_secret)
if not admin:
admin = raw_input("Make admin [no]: ")
if admin in ("y", "yes", "true"):
admin = True
else:
admin = False

request_registration(user, password, server_location, shared_secret, bool(admin))


if __name__ == "__main__":
Expand All @@ -119,6 +134,11 @@ if __name__ == "__main__":
default=None,
help="New password for user. Will prompt if omitted.",
)
parser.add_argument(
"-a", "--admin",
action="store_true",
help="Register new user as an admin. Will prompt if omitted.",
)

group = parser.add_mutually_exclusive_group(required=True)
group.add_argument(
Expand Down Expand Up @@ -151,4 +171,4 @@ if __name__ == "__main__":
else:
secret = args.shared_secret

register_new_user(args.user, args.password, args.server_url, secret)
register_new_user(args.user, args.password, args.server_url, secret, args.admin)
4 changes: 3 additions & 1 deletion synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ def register(
password=None,
generate_token=True,
guest_access_token=None,
make_guest=False
make_guest=False,
admin=False,
):
"""Registers a new client on the server.

Expand Down Expand Up @@ -141,6 +142,7 @@ def register(
# If the user was a guest then they already have a profile
None if was_guest else user.localpart
),
admin=admin,
)
else:
# autogen a sequential user ID
Expand Down
20 changes: 16 additions & 4 deletions synapse/rest/client/v1/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,24 +324,36 @@ def _do_shared_secret(self, request, register_json, session):
raise SynapseError(400, "Shared secret registration is not enabled")

user = register_json["user"].encode("utf-8")
password = register_json["password"].encode("utf-8")
admin = register_json.get("admin", None)

# Its important to check as we use null bytes as HMAC field separators
if "\x00" in user:
raise SynapseError(400, "Invalid user")
if "\x00" in password:
raise SynapseError(400, "Invalid password")

# str() because otherwise hmac complains that 'unicode' does not
# have the buffer interface
got_mac = str(register_json["mac"])

want_mac = hmac.new(
key=self.hs.config.registration_shared_secret,
msg=user,
digestmod=sha1,
).hexdigest()

password = register_json["password"].encode("utf-8")
)
want_mac.update(user)
want_mac.update("\x00")
want_mac.update(password)
want_mac.update("\x00")
want_mac.update("admin" if admin else "notadmin")
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if someone creates a user with password = "password" and admin=False and then someone nicks their mac and registers a user with password = "passwordnot" and admin=True?

Copy link
Contributor

Choose a reason for hiding this comment

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

Similar thoughts on shifting characters between the "user" and "password" fields.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, leo just brought that up in the sytest PR. I've added null separators, which I think makes it fine?

want_mac = want_mac.hexdigest()

if compare_digest(want_mac, got_mac):
handler = self.handlers.registration_handler
user_id, token = yield handler.register(
localpart=user,
password=password,
admin=bool(admin),
)
self._remove_session(session)
defer.returnValue({
Expand Down
52 changes: 29 additions & 23 deletions synapse/storage/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def add_refresh_token_to_user(self, user_id, token):
@defer.inlineCallbacks
def register(self, user_id, token, password_hash,
was_guest=False, make_guest=False, appservice_id=None,
create_profile_with_localpart=None):
create_profile_with_localpart=None, admin=False):
"""Attempts to register an account.

Args:
Expand All @@ -104,6 +104,7 @@ def register(self, user_id, token, password_hash,
make_guest,
appservice_id,
create_profile_with_localpart,
admin
)
self.get_user_by_id.invalidate((user_id,))
self.is_guest.invalidate((user_id,))
Expand All @@ -118,36 +119,41 @@ def _register(
make_guest,
appservice_id,
create_profile_with_localpart,
admin,
):
now = int(self.clock.time())

next_id = self._access_tokens_id_gen.get_next()

try:
if was_guest:
txn.execute("UPDATE users SET"
" password_hash = ?,"
" upgrade_ts = ?,"
" is_guest = ?"
" WHERE name = ?",
[password_hash, now, 1 if make_guest else 0, user_id])
self._simple_update_one_txn(
txn,
"users",
keyvalues={
"name": user_id,
},
updatevalues={
"password_hash": password_hash,
"upgrade_ts": now,
"is_guest": 1 if make_guest else 0,
"appservice_id": appservice_id,
"admin": 1 if admin else 0,
}
)
else:
txn.execute("INSERT INTO users "
"("
" name,"
" password_hash,"
" creation_ts,"
" is_guest,"
" appservice_id"
") "
"VALUES (?,?,?,?,?)",
[
user_id,
password_hash,
now,
1 if make_guest else 0,
appservice_id,
])
self._simple_insert_txn(
txn,
"users",
values={
"name": user_id,
"password_hash": password_hash,
"creation_ts": now,
"is_guest": 1 if make_guest else 0,
"appservice_id": appservice_id,
"admin": 1 if admin else 0,
}
)
except self.database_engine.module.IntegrityError:
raise StoreError(
400, "User ID already taken.", errcode=Codes.USER_IN_USE
Expand Down