Improve security of generated SIDs

[Shjpr9]

Currently, the output of RandomUtil.randomShortId() is not guaranteed to be cryptographically secure, which makes it unsuitable for security-sensitive applications. I suggest that we use the ~Openssl library to generate random IDs, using the command "openssl rand -hex 8"~, as it provides a higher level of security and ensures that the ID is truly random. By making this change, we can ensure that our application is more robust and suitable for use in security-sensitive environments.

Resources: Res1 Res2 Res3

This action can also be done by this piece of code: ~const { execSync } = require('child_process');~ ~const SID = execSync('openssl rand -hex 8').toString().trim();~

--> EDIT

Thank you for your time and I look forward to your response.

[Shjpr9]

Idk if it's necessary or not, but using an external tool like openssl is not a good idea also, JS built-in crypto module will do the same as openssl rand can use "window.crypto.getRandomValues" and then turn it to hex

Yes, you are right. Using an external tool is not a good idea. On the other hand, crypto.getRandomValues() is only available in modern browsers, so it may cause problem on some devices. Overall, window.crypto.getRandomValues() is better suited for cryptographic use where secure random values are required.

Here's what I found

const randomBytes = new Uint8Array(8); 
window.crypto.getRandomValues(randomBytes);
const hexString = Array.from(randomBytes)
  .map(byte => byte.toString(16).padStart(2, '0'))
  .join('');
console.log(hexString);

[sudospaes]

Idk if it's necessary or not, but using an external tool like openssl is not a good idea also, JS built-in crypto module will do the same as openssl rand can use "window.crypto.getRandomValues" and then turn it to hex

Yes, you are right. Using an external tool is not a good idea. On the other hand, crypto.getRandomValues() is only available in modern browsers, so it may cause problem on some devices. Overall, window.crypto.getRandomValues() is better suited for cryptographic use where secure random values are required.

Here's what I found

const randomBytes = new Uint8Array(8); 
window.crypto.getRandomValues(randomBytes);
const hexString = Array.from(randomBytes)
  .map(byte => byte.toString(16).padStart(2, '0'))
  .join('');
console.log(hexString);

This can generate a really random shortId. Interesting to lets do try it :)

[Shjpr9]

Yes, you are right. Using an external tool is not a good idea. On the other hand, crypto.getRandomValues() is only available in modern browsers, so it may cause problem on some devices. Overall, window.crypto.getRandomValues() is better suited for cryptographic use where secure random values are required. Here's what I found

const randomBytes = new Uint8Array(8); 
window.crypto.getRandomValues(randomBytes);
const hexString = Array.from(randomBytes)
  .map(byte => byte.toString(16).padStart(2, '0'))
  .join('');
console.log(hexString);

This can generate a really random shortId. Interesting to lets do try it :)

One more step is to assign a function...

function generateRandomHexString(length) {
  const randomBytes = new Uint8Array(length / 2);
  window.crypto.getRandomValues(randomBytes);
  const hexString = Array.from(randomBytes)
    .map(byte => byte.toString(16).padStart(2, '0'))
    .join('');
  return hexString;
}

console.log(generateRandomHexString(16));

Output: bca8a5d0e747799d

[sudospaes]

Yes, you are right. Using an external tool is not a good idea. On the other hand, crypto.getRandomValues() is only available in modern browsers, so it may cause problem on some devices. Overall, window.crypto.getRandomValues() is better suited for cryptographic use where secure random values are required. Here's what I found

const randomBytes = new Uint8Array(8); 
window.crypto.getRandomValues(randomBytes);
const hexString = Array.from(randomBytes)
  .map(byte => byte.toString(16).padStart(2, '0'))
  .join('');
console.log(hexString);

This can generate a really random shortId. Interesting to lets do try it :)

One more step is to assign a function...

function generateRandomHexString(length) {
  const randomBytes = new Uint8Array(length / 2);
  window.crypto.getRandomValues(randomBytes);
  const hexString = Array.from(randomBytes)
    .map(byte => byte.toString(16).padStart(2, '0'))
    .join('');
  return hexString;
}

console.log(generateRandomHexString(16));

Output: bca8a5d0e747799d

Yeah i tested now

i think this better code for shortId Generator

static randomShortId() {
        const randomBytes = new Uint8Array(8); 
        crypto.getRandomValues(randomBytes);
        const shortId = Array.from(randomBytes).map(byte => byte.toString(16).padStart(2, '0')).join('');
        return shortId;
}

[Shjpr9]

Yeah i tested now

i think this better code for shortId Generator

static randomShortId() {
        const randomBytes = new Uint8Array(8); 
        crypto.getRandomValues(randomBytes);
        const shortId = Array.from(randomBytes).map(byte => byte.toString(16).padStart(2, '0')).join('');
        return shortId;
}

add "length" as an input and use it as I mentioned for the function. We will have more control over generation of SIDs

[sudospaes]

Yeah i tested now i think this better code for shortId Generator

static randomShortId() {
        const randomBytes = new Uint8Array(8); 
        crypto.getRandomValues(randomBytes);
        const shortId = Array.from(randomBytes).map(byte => byte.toString(16).padStart(2, '0')).join('');
        return shortId;
}

add "length" as an input and use it as I mentioned for the function. We will have more control over generation of SIDs

Sure, also i think using crypto.randomUUID() for uuid is better Are you agree ?

[sudospaes]

Sure, also i think using crypto.randomUUID() for uuid is better Are you agree ?

Of course. However, the compatibility issue is still a problem!

It doesn't seem to cause a problem, usually people use modern browsers

[Shjpr9]

It doesn't seem to cause a problem, usually people use modern browsers

You're right. The compatible versions have released on 2012-2013. So there won't be a problem.

[sudospaes]

I did rewrite this functions with crypto.getRandomValues() is it okey ?

static randomShortId() {
        const randomBytes = new Uint8Array(8);
        crypto.getRandomValues(randomBytes);
        const shortId = Array.from(randomBytes).map(byte => byte.toString(16).padStart(2, '0')).join('');
        return shortId;
}

static randomUUID() {
        return ('10000000-1000-4000-8000-100000000000').replace(/[018]/g, c => (
            c ^ (crypto.getRandomValues(new Uint8Array(1))[0] & (15 >> (c / 4)))).toString(16)
        );
}

I wasn't too sure about the length because it seems that creating a hexadecimal 16 characters string is appropriate

[Shjpr9]

I did rewrite this functions with crypto.getRandomValues() is it okey ? I wasn't too sure about the length because it seems that creating a hexadecimal 16 characters string is appropriate

Here's the function:

static generateRandomShortId(length) {
  const randomBytes = new Uint8Array(length / 2);
  crypto.getRandomValues(randomBytes);
  const hexString = Array.from(randomBytes).map(byte => byte.toString(16).padStart(2, '0')).join('');
  return hexString;
}

Also randomUUID is a predefined method in crypto. You just need to do const UUID = crypto.randomUUID();

[sudospaes]

I did rewrite this functions with crypto.getRandomValues() is it okey ? I wasn't too sure about the length because it seems that creating a hexadecimal 16 characters string is appropriate

Here's the function:

static generateRandomShortId(length) {
  const randomBytes = new Uint8Array(length / 2);
  crypto.getRandomValues(randomBytes);
  const hexString = Array.from(randomBytes).map(byte => byte.toString(16).padStart(2, '0')).join('');
  return hexString;
}

Also randomUUID is a predefined method in crypto. You just need to do const UUID = crypto.randomUUID();

We cannot use crypto.randomUUID(); because only runs when your domain is localhost, or when the request is via HTTPS. Otherwise, the browser will encounter "crypto.randomUUID is not a function" error so the user has to activate SSL for the panel to create inbound.

In the case of length, it is used when we allow the user to specify the length of the string for the SID.

[alireza0]

@Shjpr9 Thank you for this idea. But in my point of view, it looks like a conspiracy illusion.

1. ShortID is not the main parameter for cryptography process. It is just a random number for separating users. ref
2. Security-sensitive applications may never involve in what is exactly hapenning between v2ray's client and server. Secure your server instead!
3. Even openssl solution is more secure than OSRND, it is not sane to use it every where.
4. RandomUtil.randomShortId() will run in panel's frontend running by admin's browser! It is not about security conserns for servers which was the main reason of using openssl random functions.
5. I don't understand the relation between anti-sensorship VPN and Security-sensitive environments. AFAK they have only limited local accesses ( and with a lot of security protections, a narrow line to internet with proxies).
6. Your references are not declare why and where should we use this openssl functions.

P.S. Don't forget this fact: Secure system is the powered off system! ( this sentence is not conclusive )

[Shjpr9]

@alireza0 You are right. However, even if we do not consider its security though, these numbers do not look like a short Id.

Why don't we use a function that is made for this purpose and has more accuracy? Anyway, improvement is improvement.

For this problem "This code will create only one shortId", I'm sure there are many ways to generate more with one request. Ultimately, the final decision rests with you. I have simply presented an idea for your consideration.

By the way, the last change I made was this one and we won't use openssl anymore.

[alireza0]

@Shjpr9 Thank you for your kind explanation. Please read my answers with happy face :) We are all trying to use better solutions which help users.

these numbers do not look like a short Id.

I don't know what do you expect from short-id. But this generated text contains what we need based on official documentation. Security encryption needs only private/public keys.

Why don't we use a function that is made for this purpose and has more accuracy? Anyway, improvement is improvement.

I don't think that crypto is made for this purpose! Also, accuracy has no meaning for a random ID. I can't name it improvement when it is only using an irrelevant method with same result but with more resource usages. But in the suggested codes, refactoring of randomUUID() was exactly an improvement regarding accuracy and security.

Let me explain it by this fact that if someone could guess the next short-id, it is not possible to use another account! But regardig UUID, it is exaclty about another account. Therefore, secure short-id here will add no values.

For this problem "This code will create only one shortId", I'm sure there are many ways to generate more with one request.

Yes it is. I am just asked it to be done in your side.

Adding another dependancy like using openssl is not a good idea till we have no choice.

[Shjpr9]

@alireza0 Your insight is appreciated. Thank you very much for your time and effort.