Skip to content
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

Oc_credentials security? #17439

Closed
tanguy-opendsi opened this issue Oct 7, 2019 · 13 comments
Closed

Oc_credentials security? #17439

tanguy-opendsi opened this issue Oct 7, 2019 · 13 comments
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement

Comments

@tanguy-opendsi
Copy link

Hi,
Can i get informations about the algorithm used to hash password inside oc_credentials.

I think this is synchronous hash because nextcloud need it with external storage but i’m not sure ?

Best regards

@tanguy-opendsi tanguy-opendsi added 0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement labels Oct 7, 2019
@kesselb
Copy link
Contributor

kesselb commented Oct 7, 2019

/**
* Store a set of credentials
*
* @param string|null $userId Null for system-wide credentials
* @param string $identifier
* @param mixed $credentials
*/
public function store($userId, $identifier, $credentials) {
$value = $this->crypto->encrypt(json_encode($credentials));
$this->dbConnection->setValues(self::DB_TABLE, [
'user' => $userId,
'identifier' => $identifier,
], [
'credentials' => $value,
]);
}

/**
* Encrypts a value and adds an HMAC (Encrypt-Then-MAC)
* @param string $plaintext
* @param string $password Password to encrypt, if not specified the secret from config.php will be taken
* @return string Authenticated ciphertext
*/
public function encrypt(string $plaintext, string $password = ''): string {
if($password === '') {
$password = $this->config->getSystemValue('secret');
}
$this->cipher->setPassword($password);
$iv = $this->random->generate($this->ivLength);
$this->cipher->setIV($iv);
$ciphertext = bin2hex($this->cipher->encrypt($plaintext));
$hmac = bin2hex($this->calculateHMAC($ciphertext.$iv, $password));
return $ciphertext.'|'.$iv.'|'.$hmac;
}

synchronous hash

I would say yes.

Can i get informations about the algorith

Sure. Everything is open. Please use https://help.nextcloud.com for questions.

@kesselb kesselb closed this as completed Oct 7, 2019
@Schmuuu
Copy link

Schmuuu commented Oct 7, 2019

Hi @kesselb

Just to let you know, I told him to better ask the developers regarding these questions. We in the Forum don't have much of the insights. Especially when it comes to questions why certain hash functions have been chosen, the broader community is missing the information I think.

@kesselb
Copy link
Contributor

kesselb commented Oct 7, 2019

told him to better ask the developers regarding these questions

Seems valid. Do you think the answer is sufficient? We can still ping some of the paid engineers. But the question is not specific.

@Schmuuu
Copy link

Schmuuu commented Oct 8, 2019

Thanks :)

Well, I'm not sure; @tanguy-opendsi does this answer your question?
The comment seems to be descriptive: "Encrypts a value and adds an HMAC (Encrypt-Then-MAC)"

And the DB table oc_credentials seems to save a value consisting of
ciphertext|iv|hmac

where "iv" seems to be a random number based on the length of an string. I don't understand what iv could mean or what ivLength is about. But maybe you understand that.
Please let us know what your questions was aiming at and where further clarification is required.

@tanguy-opendsi
Copy link
Author

@Schmuuu Thx for your reply yes i'm understanding the HMAC but same like you not the iv
for the iv i did not understand too :)

@kesselb
Copy link
Contributor

kesselb commented Oct 8, 2019

cc @nextcloud/security 🏓

@LukasReschke
Copy link
Member

IV = Initialization Vector, which is required as this is using AES in CBC mode.

https://en.wikipedia.org/wiki/Initialization_vector has some more details.

@tanguy-opendsi
Copy link
Author

@LukasReschke that mean HMAC use AES?

@LukasReschke
Copy link
Member

The HMAC is there to provide integrity. AES CBC alone doesn’t provide that.

The answer at https://security.stackexchange.com/questions/63132/when-to-use-hmac-alongside-aes is describing this quite well.

@LukasReschke
Copy link
Member

I guess it would be good if you could rephrase your original question so that we can give a better answer :-)

What are your concerns? What do you want to protect against?

@tanguy-opendsi
Copy link
Author

@LukasReschke
Ok :)
My question is :
How work the mechanism when you use external storage?
Nextcloud have to store and decrypt the password to use external storage properly ?
If yes, what is the strength of your code (Using salt, what algorithm ect..)
Can you describe the mechanism ?

@olivluca
Copy link

Isn't it futile to encrypt the password, considered that if an attacker compromises the system he can easily obtain the secret?
I'd much prefer for the password not to be stored if there is no need for them, yet they are even if I have no external storage configured with stored credentials.

@kesselb
Copy link
Contributor

kesselb commented May 19, 2020

@olivluca #21037

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement
Projects
None yet
Development

No branches or pull requests

5 participants