Skip to content

Comments

Add Cross-Platform Cryptography document#1471

Merged
qmuntal merged 19 commits intomicrosoft/mainfrom
dev/qmuntal/cpdoc
Jan 15, 2025
Merged

Add Cross-Platform Cryptography document#1471
qmuntal merged 19 commits intomicrosoft/mainfrom
dev/qmuntal/cpdoc

Conversation

@qmuntal
Copy link
Member

@qmuntal qmuntal commented Jan 9, 2025

This document is a port of Cross-Platform Cryptography in .NET.

This guide aims to replace our FIPS User Guide by being more user-friendly and easier to maintain. It is also a nice visual summary of what each backend supports and what needs more work.

Preview: https://github.com/microsoft/go/blob/dev/qmuntal/cpdoc/eng/doc/CrossPlatformCryptography.md

For #1377.

@qmuntal qmuntal requested a review from a team as a code owner January 9, 2025 13:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more


Multi-prime RSA keys are not supported.

The RSA key size is subject to the limitations of the underlying cryptographic library. For example, on Windows and when using SCOSSL, the key size should be multiple of 8.
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth being more exact? Are there more potential situations than this one example?

Suggested change
The RSA key size is subject to the limitations of the underlying cryptographic library. For example, on Windows and when using SCOSSL, the key size should be multiple of 8.
The RSA key size is subject to the limitations of the underlying cryptographic library:
* On Windows or when using SCOSSL, the key size must be a multiple of 8 (in bits).

But now I wonder if this was intended to be saying something about algorithms implemented by SCOSSL vs. by the built-in OpenSSL provider...

Copy link
Member Author

Choose a reason for hiding this comment

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

I intentionally didn't want to enumerate a possible limitations, as can vary in time and are normally not part of the public documentation of the crypto libraries.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, makes sense. I think the doc probably should state that variability pretty clearly, because otherwise it might be a bit confusing why more detail isn't provided. Also, it sounds like the user is meant to get a hint that they should avoid depending on exact details here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've rephrased the paragraph to be less confusing.

@qmuntal qmuntal requested a review from dagood January 14, 2025 07:47
Copy link
Member

@dagood dagood left a comment

Choose a reason for hiding this comment

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

Nice!

@qmuntal qmuntal requested a review from Copilot January 15, 2025 08:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Tip: Turn on automatic Copilot reviews for this repository to get quick feedback on every pull request. Learn more

@qmuntal qmuntal enabled auto-merge (squash) January 15, 2025 08:48
@qmuntal qmuntal merged commit 800df7f into microsoft/main Jan 15, 2025
31 checks passed
@dagood dagood deleted the dev/qmuntal/cpdoc branch January 15, 2025 18:50
Sign up for free to 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.

3 participants