New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[security] deployment keys access to repositories #288

Closed
gituser opened this Issue Oct 3, 2014 · 23 comments

Comments

Projects
None yet
2 participants
@gituser

gituser commented Oct 3, 2014

hello.

i've been using for quite some time older version of your's redmine_git_hosting plugin with redmine 2.2

yesterday i've decided to give a go with this new version and had some problems migrating from older version.

mainly it was because of the new scheme of keys naming, e.g:

redmine_deploy_key_10@....
or
redmine
@....

the problem here is in gitolite.conf you have access keys without @ at all, so if you add just 1 deployment key to ANY project it gets access to all projects:

repo    somerepo2
  RW+                            = redmine_user2 redmine_user2_deploy_key_10
  RW                             = redmine_user
  config redminegitolite.projectid = somerepo2
  config redminegitolite.repositoryid = 
  config redminegitolite.repositorykey = xxx2
  config http.uploadpack = true
  config multimailhook.enabled = false

repo    somerepo
  RW+                            = redmine_user2 redmine_user2_deploy_key_10
  RW                             = redmine_user
  config redminegitolite.projectid = somerepo
  config redminegitolite.repositoryid = somerepo
  config redminegitolite.repositorykey = xxxx
  config http.uploadpack = true
  config multimailhook.enabled = false

keydir of gitolite-admin contains this:

redmine_user2_deploy_key_10@redmine_1412321657_532576.pub
redmine_user2_deploy_key_10@redmine_1412226560_727957.pub

I'm using gitolite gitolite3 v3.6.1-0-g3455375 on git 1.7.2.5
also tested on v2 and it was pretty much same..

@gituser

This comment has been minimized.

Show comment
Hide comment
@gituser

gituser Oct 8, 2014

any update on this?

gituser commented Oct 8, 2014

any update on this?

@n-rodriguez

This comment has been minimized.

Show comment
Hide comment
@n-rodriguez

n-rodriguez Dec 20, 2014

Member

Hi there!

I'm sorry, I don't see the point...
Actually, you cannot have this :

redmine_user2_deploy_key_10@redmine_1412321657_532576.pub
redmine_user2_deploy_key_10@redmine_1412226560_727957.pub

since the key name is computed and incremental.

Member

n-rodriguez commented Dec 20, 2014

Hi there!

I'm sorry, I don't see the point...
Actually, you cannot have this :

redmine_user2_deploy_key_10@redmine_1412321657_532576.pub
redmine_user2_deploy_key_10@redmine_1412226560_727957.pub

since the key name is computed and incremental.

@n-rodriguez n-rodriguez self-assigned this Dec 20, 2014

@gituser

This comment has been minimized.

Show comment
Hide comment
@gituser

gituser Dec 20, 2014

hi, finally you got back to me, thanks :)

it happened after migration from old redmine_git_hosting supported by kubitron.

it might be an issue in your migration script.

i've tried creating new keys they seems to be working normal so I guess I have to-recreate all SSH-keys at some point or is there more elegant way of renaming them?

gituser commented Dec 20, 2014

hi, finally you got back to me, thanks :)

it happened after migration from old redmine_git_hosting supported by kubitron.

it might be an issue in your migration script.

i've tried creating new keys they seems to be working normal so I guess I have to-recreate all SSH-keys at some point or is there more elegant way of renaming them?

@n-rodriguez

This comment has been minimized.

Show comment
Hide comment
@n-rodriguez

n-rodriguez Dec 20, 2014

Member
redmine$ RAILS_ENV=production bundle exec rake redmine_git_hosting:rename_ssh_keys
Member

n-rodriguez commented Dec 20, 2014

redmine$ RAILS_ENV=production bundle exec rake redmine_git_hosting:rename_ssh_keys
@gituser

This comment has been minimized.

Show comment
Hide comment
@gituser

gituser Dec 20, 2014

unfortunately it didn't work out...
keys are still with @ sign, I'll try to create them manually and checkout..

gituser commented Dec 20, 2014

unfortunately it didn't work out...
keys are still with @ sign, I'll try to create them manually and checkout..

@n-rodriguez

This comment has been minimized.

Show comment
Hide comment
@n-rodriguez

n-rodriguez Dec 20, 2014

Member

keys are still with @ sign, I'll try to create them manually and checkout..

the @ in filename is normal : in 0.7 branch SSH keys are stored in a single directory in a 'flat' design so we need to be sure of the uniqueness of the keys : thus the @ with the timestamp.

Member

n-rodriguez commented Dec 20, 2014

keys are still with @ sign, I'll try to create them manually and checkout..

the @ in filename is normal : in 0.7 branch SSH keys are stored in a single directory in a 'flat' design so we need to be sure of the uniqueness of the keys : thus the @ with the timestamp.

@gituser

This comment has been minimized.

Show comment
Hide comment
@gituser

gituser Dec 20, 2014

problem is:
authorized_keys generated without @, so gitolite basically adds redmine__ for each repo without particular timestamp, so some keys granted to specific repo gets access to all repos..

gituser commented Dec 20, 2014

problem is:
authorized_keys generated without @, so gitolite basically adds redmine__ for each repo without particular timestamp, so some keys granted to specific repo gets access to all repos..

@n-rodriguez

This comment has been minimized.

Show comment
Hide comment
@n-rodriguez

n-rodriguez Dec 20, 2014

Member

so some keys granted to specific repo gets access to all repos..

I don't see how. Do you have a real example?

Member

n-rodriguez commented Dec 20, 2014

so some keys granted to specific repo gets access to all repos..

I don't see how. Do you have a real example?

@gituser

This comment has been minimized.

Show comment
Hide comment
@gituser

gituser Dec 20, 2014

ok, here is how to reproduce it:

  1. create a deployment key
  2. it will be like keydir/redmine_deploy_key@redmine_.pub
  3. for some reason it adds number of the existing before key, so for example if you had:
    keydir/redmine_user_deploy_key_11@redmine_123123123_1231.pub and earlier you had a key with the same username and number but different timestamp keydir/redmine_user_deploy_key_11@redmine_132312315_35344.pub assigned to different project, in the end your created key will get access to all repositories because gitolite cuts off everything after @ sign.

gituser commented Dec 20, 2014

ok, here is how to reproduce it:

  1. create a deployment key
  2. it will be like keydir/redmine_deploy_key@redmine_.pub
  3. for some reason it adds number of the existing before key, so for example if you had:
    keydir/redmine_user_deploy_key_11@redmine_123123123_1231.pub and earlier you had a key with the same username and number but different timestamp keydir/redmine_user_deploy_key_11@redmine_132312315_35344.pub assigned to different project, in the end your created key will get access to all repositories because gitolite cuts off everything after @ sign.
@gituser

This comment has been minimized.

Show comment
Hide comment
@gituser

gituser Dec 20, 2014

i've just tested this by creating 2 deployment keys, and one of the keys matched the same number (11) of the existing keys for some reason, so after creation it's already have access to all projects where redmine_user_deploy_key_11@.* is specified.

gituser commented Dec 20, 2014

i've just tested this by creating 2 deployment keys, and one of the keys matched the same number (11) of the existing keys for some reason, so after creation it's already have access to all projects where redmine_user_deploy_key_11@.* is specified.

@n-rodriguez

This comment has been minimized.

Show comment
Hide comment
@n-rodriguez

n-rodriguez Dec 20, 2014

Member

I've done the same and here what I get :

-rw-r--r-- 1 redmine-jbox redmine-jbox 403 Dec 20 22:33 redmine_nicolas_deploy_key_1@redmine_1419111213_815098.pub
-rw-r--r-- 1 redmine-jbox redmine-jbox 403 Dec 20 22:34 redmine_nicolas_deploy_key_2@redmine_1419111255_425368.pub
Member

n-rodriguez commented Dec 20, 2014

I've done the same and here what I get :

-rw-r--r-- 1 redmine-jbox redmine-jbox 403 Dec 20 22:33 redmine_nicolas_deploy_key_1@redmine_1419111213_815098.pub
-rw-r--r-- 1 redmine-jbox redmine-jbox 403 Dec 20 22:34 redmine_nicolas_deploy_key_2@redmine_1419111255_425368.pub

@n-rodriguez n-rodriguez reopened this Dec 20, 2014

@n-rodriguez

This comment has been minimized.

Show comment
Hide comment
@n-rodriguez

n-rodriguez Dec 20, 2014

Member

On Gitolite side :

repo    blabla
  R                              = DUMMY_REDMINE_KEY gitweb
  config redminegitolite.projectid = blabla
  config redminegitolite.repositoryid = 
  config redminegitolite.repositorykey = KWNEKEXAISXCLTHKARVRKDUHEMGFMVDJOPOVRPSWSKYYWEROOVDNHABSDDHIYHFHSBKYANOPVTYWSBRT
  config http.uploadpack = true
  config multimailhook.enabled = false

repo    child-project/subrepository
  RW+                            = redmine_admin
  R                              = gitweb
  config redminegitolite.projectid = child-project
  config redminegitolite.repositoryid = subrepository
  config redminegitolite.repositorykey = INNURFCXJQKCRYTLIMXADOOAQXODEMWALFYXMIKPNTQCHEXMQNKLFAEMPNGFSUSGGIFHNXMKMSNRTEMWLYVWUDHLFVPWVXOFJX
  config http.uploadpack = true
  config multimailhook.enabled = false

repo    gitolite-admin
  RW+                            = redmine_gitolite_admin_id_rsa

repo    parent-project
  RW+                            = redmine_admin redmine_nicolas redmine_nicolas_deploy_key_1
  R                              = gitweb
  config redminegitolite.projectid = parent-project
  config redminegitolite.repositoryid = 
  config redminegitolite.repositorykey = YSRBBWXIOPXHGFPIEYDLDIROWTKVMYARTHODOLYEJVAXLTYNRBUKQTMTXNGHGXYYCSBHJOYTWIMTRGUNSWKUHFVCJNWWFMWMCWRKPWJAUEOYBRWWVDHLTQOGSWWX
  config http.uploadpack = true
  config multimailhook.enabled = false

repo    parent-project/parentrepo2
  RW+                            = redmine_admin redmine_nicolas
  R                              = gitweb redmine_nicolas_deploy_key_2
  config redminegitolite.projectid = parent-project
  config redminegitolite.repositoryid = parentrepo2
  config redminegitolite.repositorykey = QRQXJNEXMWCFQULOSBWMQEMOKYYJVREBQAIQBXIPXVCXPTYPSPRCJWBSPNWYBSASRPYNBSPEGQXNUHXFTUTLFHEBJBLGNGBKNXOOSLMUKYOMGY
  config http.uploadpack = true
  config multimailhook.enabled = false
Member

n-rodriguez commented Dec 20, 2014

On Gitolite side :

repo    blabla
  R                              = DUMMY_REDMINE_KEY gitweb
  config redminegitolite.projectid = blabla
  config redminegitolite.repositoryid = 
  config redminegitolite.repositorykey = KWNEKEXAISXCLTHKARVRKDUHEMGFMVDJOPOVRPSWSKYYWEROOVDNHABSDDHIYHFHSBKYANOPVTYWSBRT
  config http.uploadpack = true
  config multimailhook.enabled = false

repo    child-project/subrepository
  RW+                            = redmine_admin
  R                              = gitweb
  config redminegitolite.projectid = child-project
  config redminegitolite.repositoryid = subrepository
  config redminegitolite.repositorykey = INNURFCXJQKCRYTLIMXADOOAQXODEMWALFYXMIKPNTQCHEXMQNKLFAEMPNGFSUSGGIFHNXMKMSNRTEMWLYVWUDHLFVPWVXOFJX
  config http.uploadpack = true
  config multimailhook.enabled = false

repo    gitolite-admin
  RW+                            = redmine_gitolite_admin_id_rsa

repo    parent-project
  RW+                            = redmine_admin redmine_nicolas redmine_nicolas_deploy_key_1
  R                              = gitweb
  config redminegitolite.projectid = parent-project
  config redminegitolite.repositoryid = 
  config redminegitolite.repositorykey = YSRBBWXIOPXHGFPIEYDLDIROWTKVMYARTHODOLYEJVAXLTYNRBUKQTMTXNGHGXYYCSBHJOYTWIMTRGUNSWKUHFVCJNWWFMWMCWRKPWJAUEOYBRWWVDHLTQOGSWWX
  config http.uploadpack = true
  config multimailhook.enabled = false

repo    parent-project/parentrepo2
  RW+                            = redmine_admin redmine_nicolas
  R                              = gitweb redmine_nicolas_deploy_key_2
  config redminegitolite.projectid = parent-project
  config redminegitolite.repositoryid = parentrepo2
  config redminegitolite.repositorykey = QRQXJNEXMWCFQULOSBWMQEMOKYYJVREBQAIQBXIPXVCXPTYPSPRCJWBSPNWYBSASRPYNBSPEGQXNUHXFTUTLFHEBJBLGNGBKNXOOSLMUKYOMGY
  config http.uploadpack = true
  config multimailhook.enabled = false
@gituser

This comment has been minimized.

Show comment
Hide comment
@gituser

gituser Dec 20, 2014

hey Nicholas

What I meant the issue is reproduced fully if you migrate from older version of plugin maintained by kubitron.

Can you check?

gituser commented Dec 20, 2014

hey Nicholas

What I meant the issue is reproduced fully if you migrate from older version of plugin maintained by kubitron.

Can you check?

@gituser

This comment has been minimized.

Show comment
Hide comment
@gituser

gituser Dec 20, 2014

I've done the same and here what I get :

Well, first deployment key I've just generated had the same number (11) I had before for some reason..
Next generated development key had 12 so it's fine..

gituser commented Dec 20, 2014

I've done the same and here what I get :

Well, first deployment key I've just generated had the same number (11) I had before for some reason..
Next generated development key had 12 so it's fine..

@gituser

This comment has been minimized.

Show comment
Hide comment
@gituser

gituser Dec 21, 2014

OK, I think I've found what is the issue.

Here is how to properly re-produce:

  1. Create some deployment key (if you didnt had any key before it should be redmine__deploy_key_1
  2. Create another key (should be redmine__deploy_key_2)
  3. Add another key (should be redmine__deploy_key_3)
  4. Now delete 2nd key (redmine__deploy_key_2)
  5. Add a new key and surprise!! it's going to be (redmine__deploy_key_3@....) so there you go it's going to duplicate access as 3rd key.

In short: problem is because you're getting amount of user keys and add of total to the key.. I don't get why do you add @ instead of regular format: redmine_deploy_key_timestamp.pub.

gituser commented Dec 21, 2014

OK, I think I've found what is the issue.

Here is how to properly re-produce:

  1. Create some deployment key (if you didnt had any key before it should be redmine__deploy_key_1
  2. Create another key (should be redmine__deploy_key_2)
  3. Add another key (should be redmine__deploy_key_3)
  4. Now delete 2nd key (redmine__deploy_key_2)
  5. Add a new key and surprise!! it's going to be (redmine__deploy_key_3@....) so there you go it's going to duplicate access as 3rd key.

In short: problem is because you're getting amount of user keys and add of total to the key.. I don't get why do you add @ instead of regular format: redmine_deploy_key_timestamp.pub.

@n-rodriguez

This comment has been minimized.

Show comment
Hide comment
@n-rodriguez

n-rodriguez Dec 21, 2014

Member

Ok now it's clear :)

I don't get why do you add @ instead of regular format: redmine_deploy_key_timestamp.pub.

#22

Member

n-rodriguez commented Dec 21, 2014

Ok now it's clear :)

I don't get why do you add @ instead of regular format: redmine_deploy_key_timestamp.pub.

#22

@gituser

This comment has been minimized.

Show comment
Hide comment
@gituser

gituser Dec 21, 2014

to avoid collisions I highly recommend to add _timestamp as well there in the key name..

I don't really get why would you need to predict username key? What's the point?
If you need to ID specific key you can add field in the redmine and display actual key name (.pub) ?

gituser commented Dec 21, 2014

to avoid collisions I highly recommend to add _timestamp as well there in the key name..

I don't really get why would you need to predict username key? What's the point?
If you need to ID specific key you can add field in the redmine and display actual key name (.pub) ?

@n-rodriguez

This comment has been minimized.

Show comment
Hide comment
@n-rodriguez
Member

n-rodriguez commented Dec 21, 2014

@n-rodriguez

This comment has been minimized.

Show comment
Hide comment
@n-rodriguez

n-rodriguez Dec 21, 2014

Member

to avoid collisions I highly recommend to add _timestamp as well there in the key name..

I think it would be better to improve set_identifier method.

Member

n-rodriguez commented Dec 21, 2014

to avoid collisions I highly recommend to add _timestamp as well there in the key name..

I think it would be better to improve set_identifier method.

@n-rodriguez

This comment has been minimized.

Show comment
Hide comment
@n-rodriguez

n-rodriguez Dec 21, 2014

Member

If a deployment key with redminedeploy_key_<id> already exists just add 1 and retry.

Member

n-rodriguez commented Dec 21, 2014

If a deployment key with redminedeploy_key_<id> already exists just add 1 and retry.

n-rodriguez added a commit that referenced this issue Dec 21, 2014

@n-rodriguez

This comment has been minimized.

Show comment
Hide comment
@n-rodriguez

n-rodriguez Dec 21, 2014

Member

This is fixed in v0.7.9 branch.

Member

n-rodriguez commented Dec 21, 2014

This is fixed in v0.7.9 branch.

@gituser

This comment has been minimized.

Show comment
Hide comment
@gituser

gituser Dec 23, 2014

updated to v0.7.9

will report asap how it's working

EDIT: seems to be working, thanks!

gituser commented Dec 23, 2014

updated to v0.7.9

will report asap how it's working

EDIT: seems to be working, thanks!

@n-rodriguez n-rodriguez added bug and removed question labels Dec 23, 2014

@n-rodriguez

This comment has been minimized.

Show comment
Hide comment
@n-rodriguez

n-rodriguez Dec 23, 2014

Member

EDIT: seems to be working, thanks!

You're welcome :)

Member

n-rodriguez commented Dec 23, 2014

EDIT: seems to be working, thanks!

You're welcome :)

@n-rodriguez n-rodriguez changed the title from [security] development keys access to repositories to [security] deployment keys access to repositories Dec 29, 2014

n-rodriguez added a commit that referenced this issue Jan 5, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment