Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Different approach to authentication #29

Open
wants to merge 6 commits into from

1 participant

Valentin
Valentin

Hi there,
I've been using Scrup for a few weeks now and have fallen in love with it, but the authentication-thing has also been bugging me.

I understand that transmitting a secret key together with an upload request over an insecure connection is not "secure" either (as you mentioned in the PR of 16a2da6), but there are a few points to this change:

  • Any authentication is better than none at all
  • SSL solves this problem
  • Although possibly insecure over-the-cable, it prevents malicious users from uploading anything to your server
  • The "default" can still be the simple method that does not require a key

So, I took the freedom of forking your code and came up with this:
New 'Secret' input field
Warning dialog on insecure connections

I've also thought about enforcing the use of HTTPS if the secret is set (as you suggested), but I came to the conclusion that this may be limiting pro-users.

I tried to keep it simple, please let me know what you think! I'd really like to see such a feature in Scrup, but probably don't have the experience to properly maintain a fork for long. :o

Also, please be gentle with my Objective-C, it's not exactly my every-other-day language. :-)

v4lli added some commits
Valentin v4lli Update XCode Project to Mac OS 10.7 and XCode 4.3.2
Also remove some obsolete build parameters (suggested by XCode)
8eaeb40
Valentin v4lli Send base64 encoded security token in the HTTP-Header
This adds a new NSTextfield for a "secret key" which is sent with the
image's upload request.
OpenSSL is used to base64-encode the token, as a HTTP Header needs to be
7Bit ASCII clean, as per RFC1945.
2ef2929
Valentin v4lli Make recv.php check for the secret
Use the $_SERVER array to get to the HTTP-Request-Header data.
016353b
Valentin v4lli Save the secret to the dictionary so it doesn't get lost on restarts 682a0f9
Valentin v4lli Make it possible to return the http-link over HTTPS
This may be usefull to people with a self-signed SSL certificate, which
they themselves have marked as valid, but other people receiving the link might
not.
4ebc064
Valentin v4lli Warn user about the security issue when using HTTP + Auth
The user may choose to re-edit the URL or ignore the error, for ever
(saved to NSUserDefaults).
837d0db
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 10, 2012
  1. Valentin

    Update XCode Project to Mac OS 10.7 and XCode 4.3.2

    v4lli authored
    Also remove some obsolete build parameters (suggested by XCode)
  2. Valentin

    Send base64 encoded security token in the HTTP-Header

    v4lli authored
    This adds a new NSTextfield for a "secret key" which is sent with the
    image's upload request.
    OpenSSL is used to base64-encode the token, as a HTTP Header needs to be
    7Bit ASCII clean, as per RFC1945.
  3. Valentin

    Make recv.php check for the secret

    v4lli authored
    Use the $_SERVER array to get to the HTTP-Request-Header data.
  4. Valentin
Commits on Apr 11, 2012
  1. Valentin

    Make it possible to return the http-link over HTTPS

    v4lli authored
    This may be usefull to people with a self-signed SSL certificate, which
    they themselves have marked as valid, but other people receiving the link might
    not.
  2. Valentin

    Warn user about the security issue when using HTTP + Auth

    v4lli authored
    The user may choose to re-edit the URL or ignore the error, for ever
    (saved to NSUserDefaults).
This page is out of date. Refresh to see the latest.
9 recv.php
View
@@ -4,7 +4,9 @@
# Install by putting this file on your web server and give the web server
# user write permissions to the directory in which you put this script.
#
+$SECRET = ""; # Set to "" if you don't want authentication
$MAXLENGTH = 4096000; # 4 MB
+$HTTP_LINK = true; # Set to true if you want all image links to point to the unsecure version
function rsperr($msg='', $st='400 Bad Request') {
header('HTTP/1.1 '.$st);
exit($msg);
@@ -19,9 +21,14 @@ function pathfromid($id, $suffix='') {
$suffix = strrchr($_GET['name'], '.');
$path = pathfromid($id, $suffix);
$abspath = dirname(realpath(__FILE__)).'/'.$path;
-$url = (isset($_SERVER['HTTPS']) ? 'https://' : 'http://')
+$url = (isset($_SERVER['HTTPS'] && $HTTP_LINK != true) ? 'https://' : 'http://')
. $_SERVER['SERVER_NAME'] . dirname($_SERVER['SCRIPT_NAME']) . '/' . $path;
+// Check if the secret is correct
+if(!empty($SECRET) && $_SERVER['HTTP_X_SCRUP_AUTH'] != base64_encode($SECRET)) {
+ rsperr("Bad authentication credentials!", "401 Unauthorized");
+}
+
# make dir if needed
$dirpath = dirname($abspath);
if (!file_exists($dirpath) && @mkdir($dirpath, 0775) === false)
2,173 resources/MainMenu.xib
View
835 additions, 1,338 deletions not shown
31 scrup.xcodeproj/project.pbxproj
View
@@ -3,7 +3,7 @@
archiveVersion = 1;
classes = {
};
- objectVersion = 45;
+ objectVersion = 46;
objects = {
/* Begin PBXAggregateTarget section */
@@ -554,8 +554,11 @@
/* Begin PBXProject section */
29B97313FDCFA39411CA2CEA /* Project object */ = {
isa = PBXProject;
+ attributes = {
+ LastUpgradeCheck = 0430;
+ };
buildConfigurationList = C01FCF4E08A954540054247B /* Build configuration list for PBXProject "scrup" */;
- compatibilityVersion = "Xcode 3.1";
+ compatibilityVersion = "Xcode 3.2";
developmentRegion = English;
hasScannedForEncodings = 1;
knownRegions = (
@@ -774,7 +777,6 @@
buildSettings = {
COPY_PHASE_STRIP = YES;
DEBUG_INFORMATION_FORMAT = "dwarf-with-dsym";
- GCC_ENABLE_FIX_AND_CONTINUE = NO;
PRODUCT_NAME = Distribution;
ZERO_LINK = NO;
};
@@ -787,14 +789,12 @@
ARCHS = "$(NATIVE_ARCH_ACTUAL)";
COPY_PHASE_STRIP = NO;
GCC_DYNAMIC_NO_PIC = NO;
- GCC_ENABLE_FIX_AND_CONTINUE = YES;
GCC_ENABLE_OBJC_GC = supported;
GCC_MODEL_TUNING = G5;
GCC_OPTIMIZATION_LEVEL = 0;
INSTALL_PATH = /usr/local/lib;
- PREBINDING = NO;
PRODUCT_NAME = pngcrush;
- SDKROOT = macosx10.6;
+ SDKROOT = macosx;
VALID_ARCHS = "i386 ppc ppc64 ppc7400 ppc970 x86_64";
};
name = Debug;
@@ -810,7 +810,6 @@
GCC_DEBUGGING_SYMBOLS = used;
GCC_DYNAMIC_NO_PIC = YES;
GCC_ENABLE_CPP_EXCEPTIONS = NO;
- GCC_ENABLE_FIX_AND_CONTINUE = NO;
GCC_ENABLE_OBJC_GC = supported;
GCC_ENABLE_SSE3_EXTENSIONS = YES;
GCC_ENABLE_SSE41_EXTENSIONS = YES;
@@ -820,9 +819,8 @@
GCC_OPTIMIZATION_LEVEL = 3;
GCC_UNROLL_LOOPS = YES;
INSTALL_PATH = /usr/local/lib;
- PREBINDING = NO;
PRODUCT_NAME = pngcrush;
- SDKROOT = macosx10.6;
+ SDKROOT = macosx;
VALID_ARCHS = "i386 ppc ppc64 ppc7400 ppc970 x86_64";
ZERO_LINK = NO;
};
@@ -839,7 +837,6 @@
"\"$(SRCROOT)/frameworks\"",
);
GCC_DYNAMIC_NO_PIC = NO;
- GCC_ENABLE_FIX_AND_CONTINUE = YES;
GCC_ENABLE_OBJC_GC = required;
GCC_MODEL_TUNING = G5;
GCC_OPTIMIZATION_LEVEL = 0;
@@ -852,8 +849,9 @@
GCC_WARN_UNUSED_VALUE = YES;
INFOPLIST_FILE = resources/Info.plist;
INSTALL_PATH = "$(HOME)/Applications";
+ OTHER_LDFLAGS = "-lcrypto";
PRODUCT_NAME = Scrup;
- SDKROOT = macosx10.6;
+ SDKROOT = macosx;
};
name = Debug;
};
@@ -880,8 +878,9 @@
GCC_WARN_UNUSED_VALUE = YES;
INFOPLIST_FILE = resources/Info.plist;
INSTALL_PATH = "$(HOME)/Applications";
+ OTHER_LDFLAGS = "-lcrypto";
PRODUCT_NAME = Scrup;
- SDKROOT = macosx10.6;
+ SDKROOT = macosx;
};
name = Release;
};
@@ -894,8 +893,8 @@
GCC_WARN_ABOUT_RETURN_TYPE = YES;
GCC_WARN_UNUSED_VARIABLE = YES;
ONLY_ACTIVE_ARCH = YES;
- PREBINDING = NO;
- SDKROOT = macosx10.5;
+ OTHER_LDFLAGS = "";
+ SDKROOT = macosx;
};
name = Debug;
};
@@ -906,8 +905,8 @@
GCC_C_LANGUAGE_STANDARD = c99;
GCC_WARN_ABOUT_RETURN_TYPE = YES;
GCC_WARN_UNUSED_VARIABLE = YES;
- PREBINDING = NO;
- SDKROOT = macosx10.5;
+ OTHER_LDFLAGS = "";
+ SDKROOT = macosx;
};
name = Release;
};
1  src/DPAppDelegate.h
View
@@ -15,6 +15,7 @@
IBOutlet NSView *advancedSettingsView;
IBOutlet SUUpdater *updater;
IBOutlet NSTextField *receiverURL;
+ IBOutlet NSTextField *receiverSecret;
IBOutlet NSMenuItem *pauseMenuItem;
BOOL openAtLogin,
showInDock,
75 src/DPAppDelegate.m
View
@@ -10,6 +10,11 @@
#include <sys/stat.h>
#include <unistd.h>
+/* For base64 encoding */
+#include <openssl/bio.h>
+#include <openssl/evp.h>
+
+
#define SCREENSHOT_LOG_LIMIT 10 /* todo: make configurable */
/*@interface NSStatusBar (Unofficial)
@@ -153,6 +158,12 @@ - (void)awakeFromNib {
[self performSelectorInBackground:@selector(debugPerpetualStateCheck) withObject:nil];
#endif
+ /* In order to check for http/https if the secret is set.
+ XXX The user is still not warned if the preferences window is closed without "ending"
+ the input session (because then controlTextDidEndEditing doesn't get called, for some reason */
+ [receiverSecret setDelegate:self];
+ [receiverURL setDelegate:self];
+
// XXX
/*[self displayPreprocessingUIForScreenshotAtPath:@""
meta:[NSDictionary dictionaryWithObjectsAndKeys:[NSDate date], @"du", nil]
@@ -161,6 +172,47 @@ - (void)awakeFromNib {
}];*/
}
+-(void) controlTextDidEndEditing:(NSNotification *)aNotification {
+ if([aNotification object] == receiverSecret || [aNotification object] == receiverURL) {
+ /* If we're losing focus because sslAlertEnd switched to receiverURL, don't display another sheet.
+ Also check user preference. */
+ if([mainWindow attachedSheet] != NULL
+ || [defaults boolForKey:@"sslDontAskAgain"]) {
+ return;
+ }
+
+ /* If a secret is set, check if receiverURL contains 'https://' and alert if it doesn't */
+ if(!([[receiverSecret stringValue] isEqualToString:@""] || [[receiverURL stringValue] isEqualToString:@""])) {
+ NSRange matchHTTPS = [[receiverURL stringValue] rangeOfString:@"https://"];
+ if(matchHTTPS.length <= 0) {
+ NSAlert *sslAlert = [[[NSAlert alloc] init] autorelease];
+ [sslAlert setMessageText:@"Security Tip"];
+ [sslAlert setInformativeText:@"Great, you set a secret key! But you should really use a secure connection (SSL, \"https://\") in this case. Otherwise anyone eavesdropping on your connection can easily filter out the key."];
+ [sslAlert addButtonWithTitle:@"Re-Edit Receiver URL ..."];
+ [sslAlert addButtonWithTitle:@"No! Don't ever ask again."];
+ [sslAlert beginSheetModalForWindow:mainWindow modalDelegate:self didEndSelector:@selector(sslAlertEnd:returnCode:contextInfo:) contextInfo:nil];
+ }
+ }
+ }
+}
+
+-(void)sslAlertEnd:(NSWindow *)sheet returnCode:(NSInteger)returnCode contextInfo:(void *)contextInfo {
+ switch(returnCode) {
+ case NSAlertFirstButtonReturn:
+ /* Select the URL field's 'http://' portion */
+ [receiverURL becomeFirstResponder];
+ NSText* textEditor = [mainWindow fieldEditor:YES forObject:receiverURL];
+ NSRange range = {0, 7};
+ [textEditor setSelectedRange:range];
+
+ break;
+ case NSAlertSecondButtonReturn:
+ [defaults setBool:YES forKey:@"sslDontAskAgain"];
+ break;
+ }
+ return;
+}
+
- (NSRect)menuItemFrame {
// ugly, ugly hack... damn you Apple.
@@ -491,6 +543,26 @@ -(void)processScreenshotAtPath:(NSString *)path modifiedAtDate:(NSDate *)dateMod
return;
}
+ // Check for a secret and base64 encode it
+ NSString *secret = [receiverSecret stringValue];
+ NSString *encodedSecret = NULL;
+ if([secret length] > 0) {
+ BIO *context = BIO_new(BIO_s_mem());
+
+ BIO *command = BIO_new(BIO_f_base64());
+ context = BIO_push(command, context);
+
+ // Encode all the data
+ BIO_write(context, [[secret dataUsingEncoding:NSUTF8StringEncoding] bytes], [[secret dataUsingEncoding:NSUTF8StringEncoding] length]);
+ BIO_flush(context);
+
+ char *outputBuffer;
+ long outputLength = BIO_get_mem_data(context, &outputBuffer);
+ encodedSecret = [NSString stringWithCString:outputBuffer length:outputLength];
+ BIO_free_all(context);
+ }
+
+
// Register
NSMutableDictionary *rec = [uploadedScreenshots objectForKey:fn];
if (rec) {
@@ -505,6 +577,9 @@ -(void)processScreenshotAtPath:(NSString *)path modifiedAtDate:(NSDate *)dateMod
void (^continue_block)(NSString *) = ^(NSString *actualPath){
HTTPPOSTOperation *postOp = [HTTPPOSTOperation alloc];
[postOp initWithPath:actualPath URL:url delegate:self];
+ if(encodedSecret != NULL)
+ [postOp.request setValue:[encodedSecret stringByTrimmingCharactersInSet: [NSCharacterSet whitespaceAndNewlineCharacterSet]] forHTTPHeaderField:@"X-Scrup-Auth"];
+
nCurrOps++;
[self updateMenuItem:self];
[statusItem setImage:iconSending];
Something went wrong with that request. Please try again.