Securing access to PHP services from Javascript Ajax calls

Question

I am creating a generic PHP handler, or "dispatcher", for Ajax calls made from Javascript running on the page served by PHP. The handler will take posted data and, depending on a value, decide how to route the call further. WordPress is being used, but my question is not specific to WordPress (if it was I would have asked on the WordPress sub-site).

The PHP Ajax handler should be secured, not serving calls from anywhere other than the page that the visitor is viewing in the browser (assuming that Apache is not configured to restrict access to the handler). I have designed a security structure with this aim in mind, and would like to know if my approach is good or could be improved. Are there any weaknesses that could be exploited?

The design is quite simple. When the page is constructed for serving, a session cookie is created, and its value is an encrypted token, derived from the client's IP, and a random salt. The cookie's value, the encrypted token, is also used as the name of a temporary db record for 60 minutes, and the record's value is the salt that was used to generate the token.

$ajaxSalt = bin2hex(openssl_random_pseudo_bytes(30)); // Create random salt
$cookieAndTrans = md5(crypt($_SERVER['REMOTE_ADDR'], $ajaxSalt)); // We use cookie value as token
setCookie('qnrwp_ajax_cookie', $cookieAndTrans); // Set session cookie, for JS Ajax caller to echo back to us
set_transient('qnrwp_ajax_temp_salt_'.$cookieAndTrans, $ajaxSalt, 60 * MINUTE_IN_SECONDS); // Save salt for 60 mins

When the page is served and the client makes an Ajax request, the Javascript code will, as part of the data payload, send the value of the cookie to the handler.

Receiving the call, the handler will check that the transmitted cookie value matches the cookie value that was set initially. It then uses this value as the name of the temporary db record to call up (if 60 minutes haven't passed), and uses the value of the record, the salt, to confirm that the IP matches.

// ----------------------- Security check
if ($_POST['qnrwp_ajax_cookie'] !== $_COOKIE['qnrwp_ajax_cookie']) wp_die();
$ajaxTrans = get_transient('qnrwp_ajax_temp_salt_'.$_POST['qnrwp_ajax_cookie']);
if (!$ajaxTrans) wp_die();
if ($_POST['qnrwp_ajax_cookie'] !== md5(crypt($_SERVER['REMOTE_ADDR'], $ajaxTrans))) wp_die();

A couple of things to clarify to avoid confusion: md5() is primarily used as replacement for bin2hex(), its weak crypto security just an added bonus, but not being relied upon - crypt() is used for good encryption. crypt() is able to generate a salt, but I prefer creating my own as I'll store it in the temporary record (set/get_transient() in the code). The salt is quite long, helping avoid clashes between different clients accessing the site at the same time. A clash is still possible, but I think unlikely, and even if it happened, it would not be catastrophic - if the clashing clients share their IP, one of them may find up to 120 minutes available after page load rather than 60, no big deal.

I believe my question is at quite an advanced level and would prefer if those with good knowledge of the subject write answers rather than try to engage in discussion in the comments. That said, if anything is unclear, feel free to ask in the comments. If you see weaknesses in the code, please try to provide concrete suggestions for improvement, rather than merely pointing out the loopholes. The most constructive answer will be accepted. I will wait a few days before accepting.

Please note I'm not open to suggestions of third-party tools or WordPress plugins - this is a case of "rolling my own".

Answer 1

I consider myself a security nazi, and you're asking security-related questions, so I'm going to do something I don't normally do and be pretty blunt here. Please don't take it personally.

You've completely missed the mark on security. Not to say that your solution is "insecure" (you haven't given us enough information to really decide that), but simply because, from a security perspective, most of what you are doing just doesn't make sense. Your code more-or-less looks like you threw together as many security-related functions as you could and called it secure. Granted, these things aren't making things less-secure, but it also isn't making you more-secure. I'm a firm believer that code that doesn't actually do anything should be thrown away. That is the case with most of your code here.

I think the problem you have is that you've missed out on the big picture: what is your code actually trying to accomplish? The answer, as near as I can tell, is to make sure that only the original viewer of a page can access the API calls related to that page. In other words, you are trying to identify users via a secret. In that context, here's the thing:

1. Don't use the remote address. The client's IP address can change for any number of reasons, completely breaking your system. This can happen as easily as someone opening your website on their phone with their home WIFI connection, and then stepping outside and switching to internet via their mobile carrier. When that happens their IP changes and suddenly your javascript refuses all subsequent requests. A system that breaks when the user moves 50 feet is definitely a problem. Moreover, this is not a perfect security step anyway. It is quite common for multiple users to share the same IP address, sometimes in contexts in which session stealing is most relevant (i.e. public wifi). This means that identifying users by IP Address causes UI problems, and is also not secure. Lose/Lose.
2. bin2hex is not the best choice. If you want to encode random bytes as a string (which is perfectly normal: strings, especially ASCII, move back and forth from server-to-client easily), then don't use bin2hex. For a given number of bytes, a hexadecimal string will be longer than an equivalent ASCII string. Instead store your random string as an ASCII string using only numbers and letters. You'll get shorter strings with the same amount of entropy. Granted, this is really just a nitpick: this won't impact your security either way. In essence, you are trying to generate a random string, and are doing that by taking random bytes and converting it to hex. It's better to just generate a random string, IMO.
3. Neither crypt nor md5 are doing anything for you at all. Again, focus on the big-picture goal here. You are (as near as I can tell) trying to generate a random string to act as a "secret" to later identify users. Encrypting things doesn't make them more random, and neither does hashing. It's especially pointless to hash an encrypted string. Encryption preserves data, while hashing destroys it. The fact that you would follow up one with the other suggests that you don't really understand what you are doing.
4. Checking for the value both in the cookie and post data doesn't make your app more secure: it just gives you more code to maintain on both the front-end and back-end. Moreover, it violates Kerckhoff's principle, which states that the only thing that should be a secret in any secure system is the actual secret (in this case, your key). You're effectively trying to rely on an extra security step to keep out attackers, but that security step is already public (because it is present in your front-end javascript code). As a result, it is pointless. Just let the value come up in the cookie that you already set.

So what should you do? I would suggest one of two things:

1. Best bet: authenticate users in this API call in exactly the same way that you authenticated them for the initial page load. The only reason you can't do that is if:
2. Your system is anonymous and people are not authenticated at all. In this case though, you don't need to do anything fancy at all: just use PHP's built in session system. Its entire purpose is to securely identify users from one page-view to another, and everything you have here is basically a poor-man's session system. You don't have to worry about errors in implementation if you use one that has been around and vetted for many years and by countless websites. Don't reinvent the wheel, especially when it comes to security.

Also, standard things apply:

1. Make sure to flag your cookies as HTTP-only
2. Make sure to configure your cookies to only be transmitted over HTTPS.
3. This won't protect against session stealing, but then again neither will your solution. That topic is a completely different discussion.

Forgot to say

If you don't want to use PHP sessions, then things can still be substantially simplified. To reiterate the big picture goal, all you really want is a unique (and unguessable) key that lets you identify visitors from page-view to page-view. You don't need crypt, md5, or salting to make that happen. All you need is, literally, a long random string. A 32 character string made from just numbers and letters (upper+lower case) gives you almost 200 bits of entropy, and is very easy to generate. As long as your random string generator has no weaknesses, no one is going to guess such a string. This becomes your session ID. Store it in a cookie, and get it out of the cookie when it comes back up. No need to encrypt or hash it: those don't increase your security at all. So in the end your code basically just looks like:

$sessionId = randomString(32);
setCookie('qnrwp_ajax_cookie', $sessionId);
set_transient('qnrwp_ajax_temp_salt_'.$sessionId, 60 * MINUTE_IN_SECONDS);

Then:

$ajaxTrans = get_transient('qnrwp_ajax_temp_salt_'.$_COOKIE['qnrwp_ajax_cookie']);
if (!$ajaxTrans) wp_die();
// Also make sure and check session expiration

Also, you don't want to just die. You should return some sort of API error that your front-end AJAX call can turn into a more user friendly "Your session has expired" error message. Don't forget to check the session expiration.