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.

Sign up for GitHub

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

Jump to bottom

Allow loading EC, ED25519, ED448 public keys from cryptography #1310

Merged
merged 1 commit into from
Jun 22, 2024
Merged

Allow loading EC, ED25519, ED448 public keys from cryptography #1310

merged 1 commit into from
Jun 22, 2024

Conversation

jlaine
Copy link
Contributor

@jlaine jlaine commented Jun 21, 2024

Our code is quite capable of supporting these public key types, so allow users to do so.

We also beef up the test suite to test all these key types, along with DSA keys which were not explicitly tested.

@jlaine
Copy link
Contributor Author

jlaine commented Jun 21, 2024

This also fixes our type annotations, which did not mention we support from_cryptography on elliptic curve private keys.

@jlaine
Copy link
Contributor Author

jlaine commented Jun 21, 2024

I now notice this seems to overlap with #636

alex
alex reviewed Jun 21, 2024
View reviewed changes
Copy link
Member

@alex alex left a comment

Choose a reason for hiding this comment

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

@reaperhulk do you remember if there's a reason we did rsa/dsa only originally?

src/OpenSSL/crypto.py Outdated Show resolved Hide resolved
@jlaine
Allow loading EC, ED25519, ED448 public keys from cryptography
6a0b519 
Our code is quite capable of supporting these public key types, so allow
users to do so.

We also beef up the test suite to test all these key types, along with
DSA keys which were not explicitly tested.
alex
alex approved these changes Jun 21, 2024
View reviewed changes
Copy link
Member

@alex alex left a comment

Choose a reason for hiding this comment

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

LGTM, pending @reaperhulk confirming that there wasn't a good reason we limited this.

@jlaine
Copy link
Contributor Author

jlaine commented Jun 21, 2024 •
edited
Loading

For context I was trying to eliminate mypy errors in aiortc, as it uses PKey.from_cryptography on elliptic curve private keys. The code worked fine, but the definition of _Key did not include ec.EllipticCurvePrivateKey.

I then noticed how asymmetrical (heh) the behaviour was: for DSA and RSA we supported both private keys and public keys, for ec / ed25519 / ed448 only private keys. That's why this PR ended up bringing everything into line.

@reaperhulk
Copy link
Member

The PKey object has a type() method that returns the key type. This will work, but we only expose RSA/DSA constants to check the return value. We probably need to expose the additional constants from cryptography (bleh) or hard code them in pyOpenSSL.

@alex
Copy link
Member

alex commented Jun 22, 2024

I think it's true that you can't use type() effectively without that, but strictly speaking that problem already existed -- if you had a cert and got a key out of it for example. Therefore I don't think it's a blocker for this PR.

@alex alex merged commit 9a21055 into pyca:main Jun 22, 2024
35 checks passed
@jlaine jlaine deleted the public-key-roundtrip branch June 24, 2024 08:59
@jlaine
Copy link
Contributor Author

jlaine commented Jun 24, 2024 •
edited
Loading

Thanks @alex ! Should I submit a follow up PR that mentions both my changes in the changelog?

Sorry for not doing that inline in the PRs!

Is 24.2.0 the next version?

@alex
Copy link
Member

alex commented Jun 24, 2024

Ooops, yes, changelog would be good.

jlaine added a commit to jlaine/pyopenssl that referenced this pull request Jun 24, 2024
@jlaine
Update changelog for PR pyca#1308 and pyca#1310
a9f833c
reaperhulk pushed a commit that referenced this pull request Jun 24, 2024
@jlaine
Update changelog for PR #1308 and #1310 (#1311)
caa1ab3
netbsd-srcmastr referenced this pull request in NetBSD/pkgsrc Jul 21, 2024
@0-wiz-0
py-OpenSSL: update to 24.2.1.
df0df6f 
24.2.1 (2024-07-20)
-------------------

Changes:
^^^^^^^^

- Fixed changelog to remove sphinx specific restructured text strings.


24.2.0 (2024-07-20)
-------------------

Deprecations:
^^^^^^^^^^^^^

- Deprecated ``OpenSSL.crypto.X509Req``, ``OpenSSL.crypto.load_certificate_request``, ``OpenSSL.crypto.dump_certificate_request``. Instead, ``cryptography.x509.CertificateSigningRequest``, ``cryptography.x509.CertificateSigningRequestBuilder``, ``cryptography.x509.load_der_x509_csr``, or ``cryptography.x509.load_pem_x509_csr`` should be used.

Changes:
^^^^^^^^

- Added type hints for the ``SSL`` module.
  `#1308 <https://github.com/pyca/pyopenssl/pull/1308>`_.
- Changed ``OpenSSL.crypto.PKey.from_cryptography_key`` to accept public and private EC, ED25519, ED448 keys.
  `#1310 <https://github.com/pyca/pyopenssl/pull/1310>`_.
devonh pushed a commit to element-hq/synapse that referenced this pull request Jul 22, 2024
@dependabot
Bump pyopenssl from 24.1.0 to 24.2.1 (#17465)
e3a0681 
Bumps [pyopenssl](https://github.com/pyca/pyopenssl) from 24.1.0 to
24.2.1.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/pyca/pyopenssl/blob/main/CHANGELOG.rst">pyopenssl's
changelog</a>.</em></p>
<blockquote>
<h2>24.2.1 (2024-07-20)</h2>
<p>Backward-incompatible changes:
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^</p>
<p>Deprecations:
^^^^^^^^^^^^^</p>
<p>Changes:
^^^^^^^^</p>
<ul>
<li>Fixed changelog to remove sphinx specific restructured text
strings.</li>
</ul>
<h2>24.2.0 (2024-07-20)</h2>
<p>Backward-incompatible changes:
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^</p>
<p>Deprecations:
^^^^^^^^^^^^^</p>
<ul>
<li>Deprecated <code>OpenSSL.crypto.X509Req</code>,
<code>OpenSSL.crypto.load_certificate_request</code>,
<code>OpenSSL.crypto.dump_certificate_request</code>. Instead,
<code>cryptography.x509.CertificateSigningRequest</code>,
<code>cryptography.x509.CertificateSigningRequestBuilder</code>,
<code>cryptography.x509.load_der_x509_csr</code>, or
<code>cryptography.x509.load_pem_x509_csr</code> should be used.</li>
</ul>
<p>Changes:
^^^^^^^^</p>
<ul>
<li>Added type hints for the <code>SSL</code> module.
<code>[#1308](pyca/pyopenssl#1308)
&lt;https://github.com/pyca/pyopenssl/pull/1308&gt;</code>_.</li>
<li>Changed <code>OpenSSL.crypto.PKey.from_cryptography_key</code> to
accept public and private EC, ED25519, ED448 keys.
<code>[#1310](pyca/pyopenssl#1310)
&lt;https://github.com/pyca/pyopenssl/pull/1310&gt;</code>_.</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/pyca/pyopenssl/commit/8dd94578656d47f854c087783b72431aaeb73d8e"><code>8dd9457</code></a>
24.2.1 (<a
href="https://redirect.github.com/pyca/pyopenssl/issues/1320">#1320</a>)</li>
<li><a
href="https://github.com/pyca/pyopenssl/commit/19f093e0c3db5d7354df0a5256344d9ebc5d931f"><code>19f093e</code></a>
make changelog vanilla rst (<a
href="https://redirect.github.com/pyca/pyopenssl/issues/1319">#1319</a>)</li>
<li><a
href="https://github.com/pyca/pyopenssl/commit/e265b2867bbc4f45a14b31affe4a0be710d2b293"><code>e265b28</code></a>
Prepare for 24.2.0 release (<a
href="https://redirect.github.com/pyca/pyopenssl/issues/1318">#1318</a>)</li>
<li><a
href="https://github.com/pyca/pyopenssl/commit/6943ee524e511138dd78fde6000e6f8e2bdd4b0c"><code>6943ee5</code></a>
Deprecate CSR support in pyOpenSSL (<a
href="https://redirect.github.com/pyca/pyopenssl/issues/1316">#1316</a>)</li>
<li><a
href="https://github.com/pyca/pyopenssl/commit/01b9b5637318bde2490e3f8bb243723546dbcc62"><code>01b9b56</code></a>
Add more type definitions for <code>SSL</code> module, check with mypy
(<a
href="https://redirect.github.com/pyca/pyopenssl/issues/1313">#1313</a>)</li>
<li><a
href="https://github.com/pyca/pyopenssl/commit/cdcb48baf7d5dd0f040f244934c2967b04b9f4bc"><code>cdcb48b</code></a>
Prune redundant <code>:rtype:</code> from SSL module (<a
href="https://redirect.github.com/pyca/pyopenssl/issues/1315">#1315</a>)</li>
<li><a
href="https://github.com/pyca/pyopenssl/commit/b86914d37fc6c0e138e2a0f733dd65dec2efc96c"><code>b86914d</code></a>
Fix <code>ruff</code> invocation (<a
href="https://redirect.github.com/pyca/pyopenssl/issues/1314">#1314</a>)</li>
<li><a
href="https://github.com/pyca/pyopenssl/commit/caa1ab3ac5c5096b20b69fac2e42cb67c6f716d1"><code>caa1ab3</code></a>
Update changelog for PR <a
href="https://redirect.github.com/pyca/pyopenssl/issues/1308">#1308</a>
and <a
href="https://redirect.github.com/pyca/pyopenssl/issues/1310">#1310</a>
(<a
href="https://redirect.github.com/pyca/pyopenssl/issues/1311">#1311</a>)</li>
<li><a
href="https://github.com/pyca/pyopenssl/commit/9a2105501f601798bb2fb1928cee5ed8b21e32e4"><code>9a21055</code></a>
Allow loading EC, ED25519, ED448 public keys from cryptography (<a
href="https://redirect.github.com/pyca/pyopenssl/issues/1310">#1310</a>)</li>
<li><a
href="https://github.com/pyca/pyopenssl/commit/9eaa1073621dd46b3e721183efc318c95aba0d74"><code>9eaa107</code></a>
Add type annotations for the <code>SSL</code> module (<a
href="https://redirect.github.com/pyca/pyopenssl/issues/1308">#1308</a>)</li>
<li>Additional commits viewable in <a
href="https://github.com/pyca/pyopenssl/compare/24.1.0...24.2.1">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=pyopenssl&package-manager=pip&previous-version=24.1.0&new-version=24.2.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alex alex alex approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jlaine @reaperhulk @alex