Add Brainpool EC-curves support #309
Conversation
|
@spilikin is there an RFC or other standardization body that defines the names and parameters for these curves? |
|
The main issue I see with this PR is that it seem the names for these curve have been made up without registration at IANA, nor any other standard document sanctioning them. As such they are arbitrary and there is no guarantee a future standardization (if any) will use the same name. I really do not like when people sidestep and stomp on standards that clearly define standardization procedures for adding new stuff. If there is no such document yet, is there any chance to get the German DHA to at least try to write a draft and attempt to register those names? |
|
jwks_uri: access_token: We also use the JWE as Part of Challenge-Response with the server ENC-key: |
Still waiting for an answer |
Still waiting for an answer |
It's germane Federal Office of IT-Security (the didnt do the JOSE Part yet). |
Does it means the answers to my 2 questions are No and No ? |
|
No and No. |
|
Ok then, please to each algorithm description string add the following:" (unregistered, custom-defined in breach of IETF rules by <official name of agency overseeing the format>)" |
|
Btw, I expect you to rebase and squash all commits, add a descriptive commit message and add a signed-off-by line before I merge this PR. Thanks. |
|
Look at commit 34b6525 for an example of well formatted commit message |
|
Please make clear in the commit message that these names are not defined in any standard yet and are defined by "whatever govt agency". |
I am on it. Thanx for your patience. |
spilikin
force-pushed
the
master
branch
2 times, most recently
from
36e39ba
to
4b1076f
Compare
|
Hi Simo, the new commit is ready for your review. Cheers |
jwcrypto/jwa.py
Outdated
| class _BP256R1(_RawEC, JWAAlgorithm): | ||
|
|
||
| name = "BP256R1" | ||
| description = "ECDSA using Brainpool256R1 curve and SHA-256" |
There was a problem hiding this comment.
You should add the "unregistered" description bit here too (and in the other 2 algorithms).
|
I think everything else looks good, thanks. |
This commit adds the support of Brainpool curves to jwcrypto. The Brainpool curves defined in RFC 5639 are mandatory for use in german e-health systems as defined by the Federal Office of Information Security (BSI) and National Digital Health Agency (gematik GmbH). In order to use the public E-Health APIs clients are required to: * Load and use the Brainpool keys using JWK * Sign and verify the signatures using the Brainpool elliptic curves using JWS * Encrypt and decrypt the data using the Brainpool elliptic curves and AES using JWE At the time of this commit there is no official standardization of these algorithms for JOSE/JWK/JWS/JWE. The use of these algorithms is specified solely by the gematik GmbH – National Digital Health Agency - for use in german e-health applications. Signed-off-by: Sergej Suskov <git@spilikin.dev>
|
Done. |
|
merged but you should have really considered naming your curves and algorithms with the "gematik_" prefix to make sure no conflict will arise in future if someone else decide they want to use Brainpool ECC. Squatting on namespaces w/o even attempting an informational RFC (which is really easy to do and takes an afternoon) is quite bad form. |
Hi,
this pull request adds the Brainpool curves support to jwcrypto. Since cryptography already supports the Brainpool curves,
it's more or less just a boilerplate code. I tried to add the test cases in your style too.
In Germany, especially in Healthcare we are obliged to use the Brainpool elliptic curves, additionally to NIST P-xxx.
https://www.bsi.bund.de/SharedDocs/Downloads/DE/BSI/Publikationen/TechnischeRichtlinien/TR03116/BSI-TR-03116-5.pdf?__blob=publicationFile&v=4
https://fachportal.gematik.de/fachportal-import/files/gemSpec_IDP_Dienst_V1.4.0.pdf