diff --git a/Changes b/Changes index c9cc4b5..835357d 100644 --- a/Changes +++ b/Changes @@ -1,6 +1,9 @@ Revision history for App-opan - Fix upload argument name in docs + - Fix upload authentication vulnerability + only checked password, not username + This required to change the OPAN_AUTH_TOKENS, see the docs for details 0.003002 - 2019-05-09 - Document OPAN_MIRROR diff --git a/script/opan b/script/opan index 12c5b3c..300eeb7 100644 --- a/script/opan +++ b/script/opan @@ -10,7 +10,7 @@ use Dist::Metadata; use File::Open qw(fopen); use List::UtilsBy qw(sort_by); use IPC::System::Simple qw(capture); -use Mojo::Util qw(monkey_patch); +use Mojo::Util qw(monkey_patch b64_encode); use Mojo::File qw(path); use Import::Into; @@ -274,7 +274,8 @@ Mojo::IOLoop->recurring(600 => sub { do_pull(app); }) if $ENV{OPAN_RECURRING_PUL post "/upload" => sub { my $c = shift; - unless ($TOKENS{$c->req->url->to_abs->password || ""}) { + my $b64credentials = b64_encode($c->req->url->to_abs->userinfo || '', ''); + unless ($TOKENS{$b64credentials}) { $c->res->headers->www_authenticate("Basic realm=opan"); return $c->render(status => 401, text => "Token missing or not found\n"); } @@ -623,8 +624,10 @@ specified you wanted that). To enable the /upload endpoint, set the ENV var OPAN_AUTH_TOKENS to a colon separated list of accepted tokens for uploads. This will allow a post with a -'dist' upload argument, checking http basic auth password against the provided -auth tokens. +'dist' upload argument, checking http basic auth credentials against the +provided auth tokens. +Each token is a base64 encoded username:password as sent in the Authorization: +Basic http header. =head2 recurring pull diff --git a/t/upload.t b/t/upload.t index c507769..e2148ac 100644 --- a/t/upload.t +++ b/t/upload.t @@ -7,7 +7,9 @@ use Mojo::File qw/tempdir path/; my $tempdir = tempdir(CLEANUP => 1); -BEGIN { $ENV{OPAN_AUTH_TOKENS} = 'abc:bcd'; } +# user1:pass +# user2:foobar +BEGIN { $ENV{OPAN_AUTH_TOKENS} = 'dXNlcjE6cGFzcw==:dXNlcjI6Zm9vYmFy'; } require "$FindBin::Bin/../script/opan"; @@ -16,16 +18,16 @@ my $t = Test::Mojo->new; $t->app->start('init'); $t->post_ok('/upload')->status_is('401'); -$t->post_ok('/upload', {Authorization => "Basic OmFiYw=="})->status_is('400'); +$t->post_ok('/upload', {Authorization => "Basic dXNlcjE6cGFzcw=="})->status_is('400'); my $upload = {dist => {content => 'HELLO', filename => 'world.tgz'}}; -$t->post_ok('/upload', {Authorization => "Basic OmFiYw=="}, form => $upload)->status_is('200'); +$t->post_ok('/upload', {Authorization => "Basic dXNlcjE6cGFzcw=="}, form => $upload)->status_is('200'); my $f=$tempdir->child('test')->spurt('TEST'); my $url=$t->ua->server->url->path('upload')->to_abs.''; if (eval { require CPAN::Uploader; 1 }) { Mojo::IOLoop->subprocess(sub { my $sub = shift; - return CPAN::Uploader->upload_file($f.'', {upload_uri => $url, user => 'foo', password => 'abc', debug => 1 }); + return CPAN::Uploader->upload_file($f.'', {upload_uri => $url, user => 'user1', password => 'pass', debug => 1 }); }, sub { my ($sub,$ret) = @_; ok(!$ret, 'no return is good return');