Add OAEP Encryption & Decryption - #18

Closed
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep
Closed

Add OAEP Encryption & Decryption#18
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep

Conversation

@lucdew

Copy link
Copy Markdown
Contributor

Add OAEP encryption and decryption to RSA.
Go's crypto module was a source of inspiration.

Since I am a Rustlang newbie, it probably cannot be merged as it is.

Here are important changes:

  1. An oaep module has been added and made public (see 3 for the reason)
  2. Hashes enum now has a digest function for most of enum's elements. Thus the import of sha-1,sha2 and sha3 crates
  3. Keys now supports oaep encrypt/decrypt but only with default options (options cannot be changed) which are a sha1 digest and empty label
  4. OAEP encode/decode options are the hash function (well Hashes instance) and a label. The hash function for the label and OAEP mask generation function cannot be chosen independently

Comment threadsrc/lib.rs Outdated
extern crate digest;
extern crate sha1;
extern crate sha2;
extern crate sha3;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shouldn't be need for this with the 2018 edition.

Comment threadsrc/oaep.rs
mgf1_xor(seed, &h, db);

{
let mut m = BigUint::from_bytes_be(&em);

@phayesphayesApr 3, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you comment on if there's a side-channel vulnerability here? Is BigUint::from_bytes_be constant-time? If it's not, could it leak sensitive info via timing side-channel?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty certain from_bytes_be is not constant time, but I think we do this in other places as well, so likely needs some more analysis as to what can leak, and how to fix it.

Comment threadCargo.toml
byteorder = "1.3.1"
failure = "0.1.5"
subtle = "2.0.0"
sha-1 = "0.8.1"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really like to avoid these dependencies, especially as I want to change signing to be abstract over Digest. See RustCrypto/signatures#7 for some details on the plans there

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I agree that depending on every hashing crates is not a good idea. The digest implementation should be passed. I tried that in the first place but failed, fighting the Rust compiler due to my limited Rust experience, I don't remember the exact reason...
I'll see what I can do (see my other comment on my commitment)

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you for implementing this! I don't know when I will find time for a more detailed review, but left some immediate comments

@lucdew

Copy link
Copy Markdown
ContributorAuthor

Anyway I am not sure I will have time to work on it anymore. so if you don't have any news from me within a week feel free to close/discard the pull request.

@lucdew
lucdewforce-pushed the oaep branch 2 times, most recently from 5842b75 to 9f1464cCompareApril 4, 2019 12:01
@lucdew

Copy link
Copy Markdown
ContributorAuthor

I forced pushed since I leaked some email addresses...

Also for the last commit, I :

  • removed the sha-1,sha2,sha3 dependencies (moved them to dev dependencies)

  • updated oaep to take a DynDigest borrow as argument

  • reverted changes in key and hash.
    key encrypt/decrypt methods signatures need to updated to support OAEP

Therefore in the current state it can only be used through the oaep module directly. Maybe it could be used as a starting point for OAEP support. Anyway, I'll stop working on it. I'll use my fork meanwhile.

I was interested to have a Rust only RSA implementation with OAEP to build a A WASM POC (I know WebCryptoAPI has RSA OAEP support).

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you, I will try and take this to push the rest of the integration through as soon as I find some time.

@str4dstr4d mentioned this pull request Oct 13, 2019
@str4d

Copy link
Copy Markdown
Contributor

Is there anything I could help with to move this over the line?

@dignifiedquire

Copy link
Copy Markdown
Member

@str4d figuring out how to move the current api to sth that supports this and #26 is the main blocker

@str4dstr4d mentioned this pull request Jan 2, 2020
@mehcode

Copy link
Copy Markdown

As a data point, I've copied this PR into SQLx and it works great.

I know we're waiting on the API refactor / redesign but I just wanted to give some context of "this works great in the field".

@TimNN

Copy link
Copy Markdown

Thanks for this PR, it's working great so far in my local code!

One annoyance: Using decrypt without a random number generator cumbersone:

  1. I have to add an explicit dependency on rand to my project.
  2. I have to create a let dummy: Option<&mut rand::rngs::StdRng> = None; for the first argument of decrypt, because Rust currently doesn't allow explicitly specifying type arguments if impl Trait is present in argument position.

@dignifiedquire

Copy link
Copy Markdown
Member

I have integrated this into the existing api in #43

@dignifiedquire

Copy link
Copy Markdown
Member

closing in favor of #43

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@lucdew@dignifiedquire@str4d@mehcode@TimNN@phayes
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Add OAEP Encryption & Decryption - #18

Closed
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep
Closed

Add OAEP Encryption & Decryption#18
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep

Conversation

@lucdew

Copy link
Copy Markdown
Contributor

Add OAEP encryption and decryption to RSA.
Go's crypto module was a source of inspiration.

Since I am a Rustlang newbie, it probably cannot be merged as it is.

Here are important changes:

  1. An oaep module has been added and made public (see 3 for the reason)
  2. Hashes enum now has a digest function for most of enum's elements. Thus the import of sha-1,sha2 and sha3 crates
  3. Keys now supports oaep encrypt/decrypt but only with default options (options cannot be changed) which are a sha1 digest and empty label
  4. OAEP encode/decode options are the hash function (well Hashes instance) and a label. The hash function for the label and OAEP mask generation function cannot be chosen independently

Comment threadsrc/lib.rs Outdated
extern crate digest;
extern crate sha1;
extern crate sha2;
extern crate sha3;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shouldn't be need for this with the 2018 edition.

Comment threadsrc/oaep.rs
mgf1_xor(seed, &h, db);

{
let mut m = BigUint::from_bytes_be(&em);

@phayesphayesApr 3, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you comment on if there's a side-channel vulnerability here? Is BigUint::from_bytes_be constant-time? If it's not, could it leak sensitive info via timing side-channel?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty certain from_bytes_be is not constant time, but I think we do this in other places as well, so likely needs some more analysis as to what can leak, and how to fix it.

Comment threadCargo.toml
byteorder = "1.3.1"
failure = "0.1.5"
subtle = "2.0.0"
sha-1 = "0.8.1"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really like to avoid these dependencies, especially as I want to change signing to be abstract over Digest. See RustCrypto/signatures#7 for some details on the plans there

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I agree that depending on every hashing crates is not a good idea. The digest implementation should be passed. I tried that in the first place but failed, fighting the Rust compiler due to my limited Rust experience, I don't remember the exact reason...
I'll see what I can do (see my other comment on my commitment)

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you for implementing this! I don't know when I will find time for a more detailed review, but left some immediate comments

@lucdew

Copy link
Copy Markdown
ContributorAuthor

Anyway I am not sure I will have time to work on it anymore. so if you don't have any news from me within a week feel free to close/discard the pull request.

@lucdew
lucdewforce-pushed the oaep branch 2 times, most recently from 5842b75 to 9f1464cCompareApril 4, 2019 12:01
@lucdew

Copy link
Copy Markdown
ContributorAuthor

I forced pushed since I leaked some email addresses...

Also for the last commit, I :

  • removed the sha-1,sha2,sha3 dependencies (moved them to dev dependencies)

  • updated oaep to take a DynDigest borrow as argument

  • reverted changes in key and hash.
    key encrypt/decrypt methods signatures need to updated to support OAEP

Therefore in the current state it can only be used through the oaep module directly. Maybe it could be used as a starting point for OAEP support. Anyway, I'll stop working on it. I'll use my fork meanwhile.

I was interested to have a Rust only RSA implementation with OAEP to build a A WASM POC (I know WebCryptoAPI has RSA OAEP support).

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you, I will try and take this to push the rest of the integration through as soon as I find some time.

@str4dstr4d mentioned this pull request Oct 13, 2019
@str4d

Copy link
Copy Markdown
Contributor

Is there anything I could help with to move this over the line?

@dignifiedquire

Copy link
Copy Markdown
Member

@str4d figuring out how to move the current api to sth that supports this and #26 is the main blocker

@str4dstr4d mentioned this pull request Jan 2, 2020
@mehcode

Copy link
Copy Markdown

As a data point, I've copied this PR into SQLx and it works great.

I know we're waiting on the API refactor / redesign but I just wanted to give some context of "this works great in the field".

@TimNN

Copy link
Copy Markdown

Thanks for this PR, it's working great so far in my local code!

One annoyance: Using decrypt without a random number generator cumbersone:

  1. I have to add an explicit dependency on rand to my project.
  2. I have to create a let dummy: Option<&mut rand::rngs::StdRng> = None; for the first argument of decrypt, because Rust currently doesn't allow explicitly specifying type arguments if impl Trait is present in argument position.

@dignifiedquire

Copy link
Copy Markdown
Member

I have integrated this into the existing api in #43

@dignifiedquire

Copy link
Copy Markdown
Member

closing in favor of #43

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@lucdew@dignifiedquire@str4d@mehcode@TimNN@phayes
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add OAEP Encryption & Decryption - #18

Closed
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep
Closed

Add OAEP Encryption & Decryption#18
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep

Conversation

@lucdew

Copy link
Copy Markdown
Contributor

Add OAEP encryption and decryption to RSA.
Go's crypto module was a source of inspiration.

Since I am a Rustlang newbie, it probably cannot be merged as it is.

Here are important changes:

  1. An oaep module has been added and made public (see 3 for the reason)
  2. Hashes enum now has a digest function for most of enum's elements. Thus the import of sha-1,sha2 and sha3 crates
  3. Keys now supports oaep encrypt/decrypt but only with default options (options cannot be changed) which are a sha1 digest and empty label
  4. OAEP encode/decode options are the hash function (well Hashes instance) and a label. The hash function for the label and OAEP mask generation function cannot be chosen independently

Comment threadsrc/lib.rs Outdated
extern crate digest;
extern crate sha1;
extern crate sha2;
extern crate sha3;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shouldn't be need for this with the 2018 edition.

Comment threadsrc/oaep.rs
mgf1_xor(seed, &h, db);

{
let mut m = BigUint::from_bytes_be(&em);

@phayesphayesApr 3, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you comment on if there's a side-channel vulnerability here? Is BigUint::from_bytes_be constant-time? If it's not, could it leak sensitive info via timing side-channel?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty certain from_bytes_be is not constant time, but I think we do this in other places as well, so likely needs some more analysis as to what can leak, and how to fix it.

Comment threadCargo.toml
byteorder = "1.3.1"
failure = "0.1.5"
subtle = "2.0.0"
sha-1 = "0.8.1"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really like to avoid these dependencies, especially as I want to change signing to be abstract over Digest. See RustCrypto/signatures#7 for some details on the plans there

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I agree that depending on every hashing crates is not a good idea. The digest implementation should be passed. I tried that in the first place but failed, fighting the Rust compiler due to my limited Rust experience, I don't remember the exact reason...
I'll see what I can do (see my other comment on my commitment)

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you for implementing this! I don't know when I will find time for a more detailed review, but left some immediate comments

@lucdew

Copy link
Copy Markdown
ContributorAuthor

Anyway I am not sure I will have time to work on it anymore. so if you don't have any news from me within a week feel free to close/discard the pull request.

@lucdew
lucdewforce-pushed the oaep branch 2 times, most recently from 5842b75 to 9f1464cCompareApril 4, 2019 12:01
@lucdew

Copy link
Copy Markdown
ContributorAuthor

I forced pushed since I leaked some email addresses...

Also for the last commit, I :

  • removed the sha-1,sha2,sha3 dependencies (moved them to dev dependencies)

  • updated oaep to take a DynDigest borrow as argument

  • reverted changes in key and hash.
    key encrypt/decrypt methods signatures need to updated to support OAEP

Therefore in the current state it can only be used through the oaep module directly. Maybe it could be used as a starting point for OAEP support. Anyway, I'll stop working on it. I'll use my fork meanwhile.

I was interested to have a Rust only RSA implementation with OAEP to build a A WASM POC (I know WebCryptoAPI has RSA OAEP support).

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you, I will try and take this to push the rest of the integration through as soon as I find some time.

@str4dstr4d mentioned this pull request Oct 13, 2019
@str4d

Copy link
Copy Markdown
Contributor

Is there anything I could help with to move this over the line?

@dignifiedquire

Copy link
Copy Markdown
Member

@str4d figuring out how to move the current api to sth that supports this and #26 is the main blocker

@str4dstr4d mentioned this pull request Jan 2, 2020
@mehcode

Copy link
Copy Markdown

As a data point, I've copied this PR into SQLx and it works great.

I know we're waiting on the API refactor / redesign but I just wanted to give some context of "this works great in the field".

@TimNN

Copy link
Copy Markdown

Thanks for this PR, it's working great so far in my local code!

One annoyance: Using decrypt without a random number generator cumbersone:

  1. I have to add an explicit dependency on rand to my project.
  2. I have to create a let dummy: Option<&mut rand::rngs::StdRng> = None; for the first argument of decrypt, because Rust currently doesn't allow explicitly specifying type arguments if impl Trait is present in argument position.

@dignifiedquire

Copy link
Copy Markdown
Member

I have integrated this into the existing api in #43

@dignifiedquire

Copy link
Copy Markdown
Member

closing in favor of #43

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@lucdew@dignifiedquire@str4d@mehcode@TimNN@phayes
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add OAEP Encryption & Decryption - #18

Closed
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep
Closed

Add OAEP Encryption & Decryption#18
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep

Conversation

@lucdew

Copy link
Copy Markdown
Contributor

Add OAEP encryption and decryption to RSA.
Go's crypto module was a source of inspiration.

Since I am a Rustlang newbie, it probably cannot be merged as it is.

Here are important changes:

  1. An oaep module has been added and made public (see 3 for the reason)
  2. Hashes enum now has a digest function for most of enum's elements. Thus the import of sha-1,sha2 and sha3 crates
  3. Keys now supports oaep encrypt/decrypt but only with default options (options cannot be changed) which are a sha1 digest and empty label
  4. OAEP encode/decode options are the hash function (well Hashes instance) and a label. The hash function for the label and OAEP mask generation function cannot be chosen independently

Comment threadsrc/lib.rs Outdated
extern crate digest;
extern crate sha1;
extern crate sha2;
extern crate sha3;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shouldn't be need for this with the 2018 edition.

Comment threadsrc/oaep.rs
mgf1_xor(seed, &h, db);

{
let mut m = BigUint::from_bytes_be(&em);

@phayesphayesApr 3, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you comment on if there's a side-channel vulnerability here? Is BigUint::from_bytes_be constant-time? If it's not, could it leak sensitive info via timing side-channel?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty certain from_bytes_be is not constant time, but I think we do this in other places as well, so likely needs some more analysis as to what can leak, and how to fix it.

Comment threadCargo.toml
byteorder = "1.3.1"
failure = "0.1.5"
subtle = "2.0.0"
sha-1 = "0.8.1"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really like to avoid these dependencies, especially as I want to change signing to be abstract over Digest. See RustCrypto/signatures#7 for some details on the plans there

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I agree that depending on every hashing crates is not a good idea. The digest implementation should be passed. I tried that in the first place but failed, fighting the Rust compiler due to my limited Rust experience, I don't remember the exact reason...
I'll see what I can do (see my other comment on my commitment)

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you for implementing this! I don't know when I will find time for a more detailed review, but left some immediate comments

@lucdew

Copy link
Copy Markdown
ContributorAuthor

Anyway I am not sure I will have time to work on it anymore. so if you don't have any news from me within a week feel free to close/discard the pull request.

@lucdew
lucdewforce-pushed the oaep branch 2 times, most recently from 5842b75 to 9f1464cCompareApril 4, 2019 12:01
@lucdew

Copy link
Copy Markdown
ContributorAuthor

I forced pushed since I leaked some email addresses...

Also for the last commit, I :

  • removed the sha-1,sha2,sha3 dependencies (moved them to dev dependencies)

  • updated oaep to take a DynDigest borrow as argument

  • reverted changes in key and hash.
    key encrypt/decrypt methods signatures need to updated to support OAEP

Therefore in the current state it can only be used through the oaep module directly. Maybe it could be used as a starting point for OAEP support. Anyway, I'll stop working on it. I'll use my fork meanwhile.

I was interested to have a Rust only RSA implementation with OAEP to build a A WASM POC (I know WebCryptoAPI has RSA OAEP support).

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you, I will try and take this to push the rest of the integration through as soon as I find some time.

@str4dstr4d mentioned this pull request Oct 13, 2019
@str4d

Copy link
Copy Markdown
Contributor

Is there anything I could help with to move this over the line?

@dignifiedquire

Copy link
Copy Markdown
Member

@str4d figuring out how to move the current api to sth that supports this and #26 is the main blocker

@str4dstr4d mentioned this pull request Jan 2, 2020
@mehcode

Copy link
Copy Markdown

As a data point, I've copied this PR into SQLx and it works great.

I know we're waiting on the API refactor / redesign but I just wanted to give some context of "this works great in the field".

@TimNN

Copy link
Copy Markdown

Thanks for this PR, it's working great so far in my local code!

One annoyance: Using decrypt without a random number generator cumbersone:

  1. I have to add an explicit dependency on rand to my project.
  2. I have to create a let dummy: Option<&mut rand::rngs::StdRng> = None; for the first argument of decrypt, because Rust currently doesn't allow explicitly specifying type arguments if impl Trait is present in argument position.

@dignifiedquire

Copy link
Copy Markdown
Member

I have integrated this into the existing api in #43

@dignifiedquire

Copy link
Copy Markdown
Member

closing in favor of #43

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@lucdew@dignifiedquire@str4d@mehcode@TimNN@phayes
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Add OAEP Encryption & Decryption - #18

Closed
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep
Closed

Add OAEP Encryption & Decryption#18
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep

Conversation

@lucdew

Copy link
Copy Markdown
Contributor

Add OAEP encryption and decryption to RSA.
Go's crypto module was a source of inspiration.

Since I am a Rustlang newbie, it probably cannot be merged as it is.

Here are important changes:

  1. An oaep module has been added and made public (see 3 for the reason)
  2. Hashes enum now has a digest function for most of enum's elements. Thus the import of sha-1,sha2 and sha3 crates
  3. Keys now supports oaep encrypt/decrypt but only with default options (options cannot be changed) which are a sha1 digest and empty label
  4. OAEP encode/decode options are the hash function (well Hashes instance) and a label. The hash function for the label and OAEP mask generation function cannot be chosen independently

Comment threadsrc/lib.rs Outdated
extern crate digest;
extern crate sha1;
extern crate sha2;
extern crate sha3;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shouldn't be need for this with the 2018 edition.

Comment threadsrc/oaep.rs
mgf1_xor(seed, &h, db);

{
let mut m = BigUint::from_bytes_be(&em);

@phayesphayesApr 3, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you comment on if there's a side-channel vulnerability here? Is BigUint::from_bytes_be constant-time? If it's not, could it leak sensitive info via timing side-channel?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty certain from_bytes_be is not constant time, but I think we do this in other places as well, so likely needs some more analysis as to what can leak, and how to fix it.

Comment threadCargo.toml
byteorder = "1.3.1"
failure = "0.1.5"
subtle = "2.0.0"
sha-1 = "0.8.1"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really like to avoid these dependencies, especially as I want to change signing to be abstract over Digest. See RustCrypto/signatures#7 for some details on the plans there

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I agree that depending on every hashing crates is not a good idea. The digest implementation should be passed. I tried that in the first place but failed, fighting the Rust compiler due to my limited Rust experience, I don't remember the exact reason...
I'll see what I can do (see my other comment on my commitment)

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you for implementing this! I don't know when I will find time for a more detailed review, but left some immediate comments

@lucdew

Copy link
Copy Markdown
ContributorAuthor

Anyway I am not sure I will have time to work on it anymore. so if you don't have any news from me within a week feel free to close/discard the pull request.

@lucdew
lucdewforce-pushed the oaep branch 2 times, most recently from 5842b75 to 9f1464cCompareApril 4, 2019 12:01
@lucdew

Copy link
Copy Markdown
ContributorAuthor

I forced pushed since I leaked some email addresses...

Also for the last commit, I :

  • removed the sha-1,sha2,sha3 dependencies (moved them to dev dependencies)

  • updated oaep to take a DynDigest borrow as argument

  • reverted changes in key and hash.
    key encrypt/decrypt methods signatures need to updated to support OAEP

Therefore in the current state it can only be used through the oaep module directly. Maybe it could be used as a starting point for OAEP support. Anyway, I'll stop working on it. I'll use my fork meanwhile.

I was interested to have a Rust only RSA implementation with OAEP to build a A WASM POC (I know WebCryptoAPI has RSA OAEP support).

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you, I will try and take this to push the rest of the integration through as soon as I find some time.

@str4dstr4d mentioned this pull request Oct 13, 2019
@str4d

Copy link
Copy Markdown
Contributor

Is there anything I could help with to move this over the line?

@dignifiedquire

Copy link
Copy Markdown
Member

@str4d figuring out how to move the current api to sth that supports this and #26 is the main blocker

@str4dstr4d mentioned this pull request Jan 2, 2020
@mehcode

Copy link
Copy Markdown

As a data point, I've copied this PR into SQLx and it works great.

I know we're waiting on the API refactor / redesign but I just wanted to give some context of "this works great in the field".

@TimNN

Copy link
Copy Markdown

Thanks for this PR, it's working great so far in my local code!

One annoyance: Using decrypt without a random number generator cumbersone:

  1. I have to add an explicit dependency on rand to my project.
  2. I have to create a let dummy: Option<&mut rand::rngs::StdRng> = None; for the first argument of decrypt, because Rust currently doesn't allow explicitly specifying type arguments if impl Trait is present in argument position.

@dignifiedquire

Copy link
Copy Markdown
Member

I have integrated this into the existing api in #43

@dignifiedquire

Copy link
Copy Markdown
Member

closing in favor of #43

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@lucdew@dignifiedquire@str4d@mehcode@TimNN@phayes
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add OAEP Encryption & Decryption - #18

Closed
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep
Closed

Add OAEP Encryption & Decryption#18
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep

Conversation

@lucdew

Copy link
Copy Markdown
Contributor

Add OAEP encryption and decryption to RSA.
Go's crypto module was a source of inspiration.

Since I am a Rustlang newbie, it probably cannot be merged as it is.

Here are important changes:

  1. An oaep module has been added and made public (see 3 for the reason)
  2. Hashes enum now has a digest function for most of enum's elements. Thus the import of sha-1,sha2 and sha3 crates
  3. Keys now supports oaep encrypt/decrypt but only with default options (options cannot be changed) which are a sha1 digest and empty label
  4. OAEP encode/decode options are the hash function (well Hashes instance) and a label. The hash function for the label and OAEP mask generation function cannot be chosen independently

Comment threadsrc/lib.rs Outdated
extern crate digest;
extern crate sha1;
extern crate sha2;
extern crate sha3;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shouldn't be need for this with the 2018 edition.

Comment threadsrc/oaep.rs
mgf1_xor(seed, &h, db);

{
let mut m = BigUint::from_bytes_be(&em);

@phayesphayesApr 3, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you comment on if there's a side-channel vulnerability here? Is BigUint::from_bytes_be constant-time? If it's not, could it leak sensitive info via timing side-channel?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty certain from_bytes_be is not constant time, but I think we do this in other places as well, so likely needs some more analysis as to what can leak, and how to fix it.

Comment threadCargo.toml
byteorder = "1.3.1"
failure = "0.1.5"
subtle = "2.0.0"
sha-1 = "0.8.1"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really like to avoid these dependencies, especially as I want to change signing to be abstract over Digest. See RustCrypto/signatures#7 for some details on the plans there

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I agree that depending on every hashing crates is not a good idea. The digest implementation should be passed. I tried that in the first place but failed, fighting the Rust compiler due to my limited Rust experience, I don't remember the exact reason...
I'll see what I can do (see my other comment on my commitment)

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you for implementing this! I don't know when I will find time for a more detailed review, but left some immediate comments

@lucdew

Copy link
Copy Markdown
ContributorAuthor

Anyway I am not sure I will have time to work on it anymore. so if you don't have any news from me within a week feel free to close/discard the pull request.

@lucdew
lucdewforce-pushed the oaep branch 2 times, most recently from 5842b75 to 9f1464cCompareApril 4, 2019 12:01
@lucdew

Copy link
Copy Markdown
ContributorAuthor

I forced pushed since I leaked some email addresses...

Also for the last commit, I :

  • removed the sha-1,sha2,sha3 dependencies (moved them to dev dependencies)

  • updated oaep to take a DynDigest borrow as argument

  • reverted changes in key and hash.
    key encrypt/decrypt methods signatures need to updated to support OAEP

Therefore in the current state it can only be used through the oaep module directly. Maybe it could be used as a starting point for OAEP support. Anyway, I'll stop working on it. I'll use my fork meanwhile.

I was interested to have a Rust only RSA implementation with OAEP to build a A WASM POC (I know WebCryptoAPI has RSA OAEP support).

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you, I will try and take this to push the rest of the integration through as soon as I find some time.

@str4dstr4d mentioned this pull request Oct 13, 2019
@str4d

Copy link
Copy Markdown
Contributor

Is there anything I could help with to move this over the line?

@dignifiedquire

Copy link
Copy Markdown
Member

@str4d figuring out how to move the current api to sth that supports this and #26 is the main blocker

@str4dstr4d mentioned this pull request Jan 2, 2020
@mehcode

Copy link
Copy Markdown

As a data point, I've copied this PR into SQLx and it works great.

I know we're waiting on the API refactor / redesign but I just wanted to give some context of "this works great in the field".

@TimNN

Copy link
Copy Markdown

Thanks for this PR, it's working great so far in my local code!

One annoyance: Using decrypt without a random number generator cumbersone:

  1. I have to add an explicit dependency on rand to my project.
  2. I have to create a let dummy: Option<&mut rand::rngs::StdRng> = None; for the first argument of decrypt, because Rust currently doesn't allow explicitly specifying type arguments if impl Trait is present in argument position.

@dignifiedquire

Copy link
Copy Markdown
Member

I have integrated this into the existing api in #43

@dignifiedquire

Copy link
Copy Markdown
Member

closing in favor of #43

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@lucdew@dignifiedquire@str4d@mehcode@TimNN@phayes
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add OAEP Encryption & Decryption - #18

Closed
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep
Closed

Add OAEP Encryption & Decryption#18
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep

Conversation

@lucdew

Copy link
Copy Markdown
Contributor

Add OAEP encryption and decryption to RSA.
Go's crypto module was a source of inspiration.

Since I am a Rustlang newbie, it probably cannot be merged as it is.

Here are important changes:

  1. An oaep module has been added and made public (see 3 for the reason)
  2. Hashes enum now has a digest function for most of enum's elements. Thus the import of sha-1,sha2 and sha3 crates
  3. Keys now supports oaep encrypt/decrypt but only with default options (options cannot be changed) which are a sha1 digest and empty label
  4. OAEP encode/decode options are the hash function (well Hashes instance) and a label. The hash function for the label and OAEP mask generation function cannot be chosen independently

Comment threadsrc/lib.rs Outdated
extern crate digest;
extern crate sha1;
extern crate sha2;
extern crate sha3;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shouldn't be need for this with the 2018 edition.

Comment threadsrc/oaep.rs
mgf1_xor(seed, &h, db);

{
let mut m = BigUint::from_bytes_be(&em);

@phayesphayesApr 3, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you comment on if there's a side-channel vulnerability here? Is BigUint::from_bytes_be constant-time? If it's not, could it leak sensitive info via timing side-channel?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty certain from_bytes_be is not constant time, but I think we do this in other places as well, so likely needs some more analysis as to what can leak, and how to fix it.

Comment threadCargo.toml
byteorder = "1.3.1"
failure = "0.1.5"
subtle = "2.0.0"
sha-1 = "0.8.1"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really like to avoid these dependencies, especially as I want to change signing to be abstract over Digest. See RustCrypto/signatures#7 for some details on the plans there

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I agree that depending on every hashing crates is not a good idea. The digest implementation should be passed. I tried that in the first place but failed, fighting the Rust compiler due to my limited Rust experience, I don't remember the exact reason...
I'll see what I can do (see my other comment on my commitment)

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you for implementing this! I don't know when I will find time for a more detailed review, but left some immediate comments

@lucdew

Copy link
Copy Markdown
ContributorAuthor

Anyway I am not sure I will have time to work on it anymore. so if you don't have any news from me within a week feel free to close/discard the pull request.

@lucdew
lucdewforce-pushed the oaep branch 2 times, most recently from 5842b75 to 9f1464cCompareApril 4, 2019 12:01
@lucdew

Copy link
Copy Markdown
ContributorAuthor

I forced pushed since I leaked some email addresses...

Also for the last commit, I :

  • removed the sha-1,sha2,sha3 dependencies (moved them to dev dependencies)

  • updated oaep to take a DynDigest borrow as argument

  • reverted changes in key and hash.
    key encrypt/decrypt methods signatures need to updated to support OAEP

Therefore in the current state it can only be used through the oaep module directly. Maybe it could be used as a starting point for OAEP support. Anyway, I'll stop working on it. I'll use my fork meanwhile.

I was interested to have a Rust only RSA implementation with OAEP to build a A WASM POC (I know WebCryptoAPI has RSA OAEP support).

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you, I will try and take this to push the rest of the integration through as soon as I find some time.

@str4dstr4d mentioned this pull request Oct 13, 2019
@str4d

Copy link
Copy Markdown
Contributor

Is there anything I could help with to move this over the line?

@dignifiedquire

Copy link
Copy Markdown
Member

@str4d figuring out how to move the current api to sth that supports this and #26 is the main blocker

@str4dstr4d mentioned this pull request Jan 2, 2020
@mehcode

Copy link
Copy Markdown

As a data point, I've copied this PR into SQLx and it works great.

I know we're waiting on the API refactor / redesign but I just wanted to give some context of "this works great in the field".

@TimNN

Copy link
Copy Markdown

Thanks for this PR, it's working great so far in my local code!

One annoyance: Using decrypt without a random number generator cumbersone:

  1. I have to add an explicit dependency on rand to my project.
  2. I have to create a let dummy: Option<&mut rand::rngs::StdRng> = None; for the first argument of decrypt, because Rust currently doesn't allow explicitly specifying type arguments if impl Trait is present in argument position.

@dignifiedquire

Copy link
Copy Markdown
Member

I have integrated this into the existing api in #43

@dignifiedquire

Copy link
Copy Markdown
Member

closing in favor of #43

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@lucdew@dignifiedquire@str4d@mehcode@TimNN@phayes
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Add OAEP Encryption & Decryption - #18

Closed
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep
Closed

Add OAEP Encryption & Decryption#18
lucdew wants to merge 9 commits into
RustCrypto:masterfrom
lucdew:oaep

Conversation

@lucdew

Copy link
Copy Markdown
Contributor

Add OAEP encryption and decryption to RSA.
Go's crypto module was a source of inspiration.

Since I am a Rustlang newbie, it probably cannot be merged as it is.

Here are important changes:

  1. An oaep module has been added and made public (see 3 for the reason)
  2. Hashes enum now has a digest function for most of enum's elements. Thus the import of sha-1,sha2 and sha3 crates
  3. Keys now supports oaep encrypt/decrypt but only with default options (options cannot be changed) which are a sha1 digest and empty label
  4. OAEP encode/decode options are the hash function (well Hashes instance) and a label. The hash function for the label and OAEP mask generation function cannot be chosen independently

Comment threadsrc/lib.rs Outdated
extern crate digest;
extern crate sha1;
extern crate sha2;
extern crate sha3;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shouldn't be need for this with the 2018 edition.

Comment threadsrc/oaep.rs
mgf1_xor(seed, &h, db);

{
let mut m = BigUint::from_bytes_be(&em);

@phayesphayesApr 3, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you comment on if there's a side-channel vulnerability here? Is BigUint::from_bytes_be constant-time? If it's not, could it leak sensitive info via timing side-channel?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty certain from_bytes_be is not constant time, but I think we do this in other places as well, so likely needs some more analysis as to what can leak, and how to fix it.

Comment threadCargo.toml
byteorder = "1.3.1"
failure = "0.1.5"
subtle = "2.0.0"
sha-1 = "0.8.1"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really like to avoid these dependencies, especially as I want to change signing to be abstract over Digest. See RustCrypto/signatures#7 for some details on the plans there

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I agree that depending on every hashing crates is not a good idea. The digest implementation should be passed. I tried that in the first place but failed, fighting the Rust compiler due to my limited Rust experience, I don't remember the exact reason...
I'll see what I can do (see my other comment on my commitment)

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you for implementing this! I don't know when I will find time for a more detailed review, but left some immediate comments

@lucdew

Copy link
Copy Markdown
ContributorAuthor

Anyway I am not sure I will have time to work on it anymore. so if you don't have any news from me within a week feel free to close/discard the pull request.

@lucdew
lucdewforce-pushed the oaep branch 2 times, most recently from 5842b75 to 9f1464cCompareApril 4, 2019 12:01
@lucdew

Copy link
Copy Markdown
ContributorAuthor

I forced pushed since I leaked some email addresses...

Also for the last commit, I :

  • removed the sha-1,sha2,sha3 dependencies (moved them to dev dependencies)

  • updated oaep to take a DynDigest borrow as argument

  • reverted changes in key and hash.
    key encrypt/decrypt methods signatures need to updated to support OAEP

Therefore in the current state it can only be used through the oaep module directly. Maybe it could be used as a starting point for OAEP support. Anyway, I'll stop working on it. I'll use my fork meanwhile.

I was interested to have a Rust only RSA implementation with OAEP to build a A WASM POC (I know WebCryptoAPI has RSA OAEP support).

@dignifiedquire

Copy link
Copy Markdown
Member

Thank you, I will try and take this to push the rest of the integration through as soon as I find some time.

@str4dstr4d mentioned this pull request Oct 13, 2019
@str4d

Copy link
Copy Markdown
Contributor

Is there anything I could help with to move this over the line?

@dignifiedquire

Copy link
Copy Markdown
Member

@str4d figuring out how to move the current api to sth that supports this and #26 is the main blocker

@str4dstr4d mentioned this pull request Jan 2, 2020
@mehcode

Copy link
Copy Markdown

As a data point, I've copied this PR into SQLx and it works great.

I know we're waiting on the API refactor / redesign but I just wanted to give some context of "this works great in the field".

@TimNN

Copy link
Copy Markdown

Thanks for this PR, it's working great so far in my local code!

One annoyance: Using decrypt without a random number generator cumbersone:

  1. I have to add an explicit dependency on rand to my project.
  2. I have to create a let dummy: Option<&mut rand::rngs::StdRng> = None; for the first argument of decrypt, because Rust currently doesn't allow explicitly specifying type arguments if impl Trait is present in argument position.

@dignifiedquire

Copy link
Copy Markdown
Member

I have integrated this into the existing api in #43

@dignifiedquire

Copy link
Copy Markdown
Member

closing in favor of #43

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@lucdew@dignifiedquire@str4d@mehcode@TimNN@phayes