openpubkey / openpubkey Goto Github PK
View Code? Open in Web Editor NEWReference implementation of OpenPubkey
Home Page: https://eprint.iacr.org/2023/296
License: Apache License 2.0
Reference implementation of OpenPubkey
Home Page: https://eprint.iacr.org/2023/296
License: Apache License 2.0
cd examples/simple
go build
./simple
or mfa
or google
login via google oidc returns
failed to exchange token: issuedAt of token is in the future: \
(iat: 2024-02-22 14:40:31 -0500 EST, now with offset: 2024-02-22 19:40:22 +0000 UTC)
When available the OpenPubkey client should support the ability to use a refresh token to request an refreshed ID Token. This refreshed ID Token will be stored in an unprotected header of the ID Token.
This feature does not automatically refreshing an ID Token. It just provides the ability of an implementor to request a refresh.
As zkLogin was inspired by OpenPubkey to use the nonce in the ID Token to commit to the user's public key upk it is worth examining if OpenPubkey can employ zkLogin zero knowledge proofs to provide anonymous or private PK Tokens. This issue examines the feasibility, benefits and disadvantages of using zkLogin proofs in OpenPubkey.
zkLogin is a protocol for using zero knowledge proofs to add privacy to OIDC.
Given an ID token idt and public key upk it creates a zero knowledge proof, pi, which proves knowledge of the ID Token and commits to certain claims in the ID Token.
Using Google as the example OP the format of a ID Token payload using a zkLogin nonce is:
payload: {
"iss": "https://accounts.google.com",
"aud": "878305696756-6maur39hl2psmk23imilg8af815ih9oi.apps.googleusercontent.com",
"sub": "123456789010",
"email": "[email protected]",
"nonce": 'Base64(Poseidon_BN254(upk, max_epoch, rz)'
...
}
The format of the nonce is very similar to that of OpenPubkey. The main differences are:
The zkLogin proof commits to the user's address which is computed as:
addr = Blake2b_256(flag, iss, addr_seed)
where
addr_seed = Poseidon_BN254(claimkey, idT.[claimkey], idT.aud, Poseidon_BN254(user_salt))
If the claimkey was "email" then it would be
addr_seed = Poseidon_BN254(email, idT.email, idT.aud, Poseidon_BN254(user_salt))
.
The addr serves as a pseudonymous identity with the user_salt
blinding the identifier. If you don't know the user_salt
, you can't determine the email. The user knows the user_salt
. This allows them to selectively reveal the identity claim in the ID Token. zkLogin generally assumes user's want anonymity so they don't reveal the user_salt
and they just use the addr
as their identity.
This allows users to interact in public using pseudonym, but then privately interact using their true name identifiers.
zkLogin assumes the following is public:
zkLogin assumes the following is private:
Below is a proposal for how to structure a OpenPubkey PK Token using the objects in zkLogin. This PK Token does not reveal any identity claims about the user, sub, email are all protected.
payload: {
"iss": "https://accounts.google.com",
"aud": "878305696756-6maur39hl2psmk23imilg8af815ih9oi.apps.googleusercontent.com",
"nonce": 'Base64(Poseidon_BN254(upk-ext, max_epoch, rz)'
}
signatures: [
{"protected": {"typ": "ZWT", "alg": "...", "kid": "1234...", "addr": 'addr_seed'},
"signature": ZKP(idt, salt)`
},
{"protected": {"typ": "ZIC", "upk-ext": alice-pubkey, "alg": "EC256", "rz": crypto.Rand(), max_epoch": exp, },
"signature": <SIGN(alice-signkey, (payload, signatures[1].protected))>
},
]
We now consider the case where a user wishes expose one of their identity claims, email
payload: {
"iss": "https://accounts.google.com",
"aud": "878305696756-6maur39hl2psmk23imilg8af815ih9oi.apps.googleusercontent.com",
"nonce": 'Base64(Poseidon_BN254(upk, max_epoch, rz)'
}
signatures: [
{"protected": {"typ": "ZWT", "alg": "...", "kid": "1234...", "addr": 'addr', "claimKey": "email" "claimValue": "[email protected]", "user_salt": 'user_salt'},
"signature": ZKP(idt, salt)`
},
{"protected": {"typ": "ZIC", "upk": alice-pubkey, "alg": "EC256", "rz": crypto.Rand(), max_epoch": exp, },
"signature": <SIGN(alice-signkey, (payload, signatures[1].protected))>
},
]
Since user_salt is revealed, we can now open the addr_seed and check claimKey and claimValue. We use email in this example but other claims can be used as well.
We currently (as of PR #86) do not handle the case where the same provider (OP) is specified twice in an allowlist with different audiences. That is same providers sharing the same Issuer but having separate audience claims. Instead it will match on the first provider with the correct issuer and then verify using that provider.
The reason we check for matches rather than just verifying is to avoid blindly downloading a JWKS URI based on the issuer (iss) claim of an ID Token. Instead we check that the issuer is correct and then run verification.
The ideal way to solve this problem is to add a function to the (OP) Provider interface called something like Matches(idt IDToken) (bool)
and let each provider determine how decides if an ID Token belongs to it.
As OpenPubkey tokens are JWS, we need to have a way of identifying the signature as being a GQ signature. The alg
will be set to RS256
because the protected headers are fixed, so we will need to add an unprotected header to flag this.
In implementing a ledger for OIDC public key, we've found that according to spec, the kid
is only mandatory if there is more than one key on the JWKS endpoint, and only needs to be unique for each HTTP response on that endpoint. kid
is obviously super important when trying to lookup the public key given an OPK. I was wondering if adding a hash of the OP public key to the OPK header would solve this? Any other ideas?
Currently the OpenIdProvider has the following PublicKey function:
PublicKey(ctx context.Context, headers jws.Headers) (crypto.PublicKey, error)
This works for now but doesn't provide the extensibility and features the project will need in the future. More specifically:
The return value doesn't provide everything we need
crypto.PublicKey
without the full JWK. There are fields in the JWK the caller may wish to inspect. If we ever want to support non-RSA OP keys we will want the alg at minimum.It takes jws.header as the single selection criteria where this header is the protected header or public header of ID Token. This is done to allow the function to look up keys by both kid and jtk. Additionally it allows PublicKey to detect that a protected header is a GQ protected header and that it needs to decode the kid to find the original header and original kid.
The downsides of taking a jws.header:
This one should be a bit simpler than others because we just have to call a URL with a token, both provided as environment variables, passing an audience. We will have to put the nonce in the audience because we don't have access to a full PKCE flow.
Currently we don't check the RSA Public Key is correct when generating a GQ Signature. If you accidentally supply the wrong Public Key you get the hard to diagnose error message "error creating GQ signature: input overflows the modulus."
Benefits:
This behavior was noticed will writing PR #122 but was the bug was not introduced in that PR
95% of the time this will trigger the bug and throw the error: "input overflows the modulus"
for i := 0; i < 100; i++ {
oidcPrivKey1, err := rsa.GenerateKey(rand.Reader, 2048)
require.NoError(t, err)
oidcPrivKey2, err := rsa.GenerateKey(rand.Reader, 2048)
require.NoError(t, err)
require.NotEqual(t, oidcPrivKey1, oidcPrivKey2)
idToken, err := createOIDCToken(oidcPrivKey1, "test")
require.NoError(t, err)
signerVerifier, err := NewSignerVerifier(&oidcPrivKey2.PublicKey, 256)
require.NoError(t, err)
gq, err := signerVerifier.SignJWT(idToken)
require.NoError(t, err)
require.Nil(t, gq)
}
Derived from #11
Lifecycle and protection of secrets: Ensuring that sensitive cryptographic secrets are zerod/deleted when no longer in use and that they do not persist after they are needed. For instance with GQ signatures, we should delete the RSA signature immediately after the GQ signature is constructed. We should carefully consider the lifecycle of our ephemeral keypairs, currently we just write them to disk.
Potential ways to address this:
LockedBuffers
or Enclave
objects to handle sensitive data and use the built in data scrubbing methodsThe GQ package uses math/big
for its implementation which is not safe for cryptographic use:
Note that methods may leak the Int's value through timing side-channels. Because of this and because of the scope and complexity of the implementation, Int is not well-suited to implement cryptographic operations. The standard library avoids exposing non-trivial Int methods to attacker-controlled inputs and the determination of whether a bug in math/big is considered a security vulnerability might depend on the impact on the standard library.
I had a look at what how the crypto/rsa package deals with this and found that there's an internal bigmod
package which is used instead of math/big
. Interestingly, this has only been the case since Go 1.20 as detailed on Filippo Valsorda's blog. This bigmod
package would be the ideal package to use instead of math/big
, but as it's internal we cannot import it. Filippo maintains a re-exported copy of the internal library at https://github.com/FiloSottile/bigmod so that could be the easiest thing to use.
We have a small amount of in repo "paper work" to finish up.
We have not created license headers in each source file as specified https://www.apache.org/licenses/LICENSE-2.0
Copyright 2024 OpenPubkey
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
SPDX-License-Identifier: Apache-2.0
We do not have a NOTICE file. Perhaps we don't want one and I believe we can add one later.
This ticket can be resolved with a PR that addresses the following:
- [ ] Fill out license boilerplate
The Sign()
method generates ephemeral keys for signing attestations shown here:
https://github.com/openpubkey/signed-attestation/blob/f64048341ecd651ed894f094ab70213d20ee6bbb/opk.go#L30-L33
Using GenKeyPair()
in utils
:
Lines 100 to 109 in f662c2c
Depending upon the system on which this is running, the entropy available (and sources it is derived from) may not be very high quality. The quality of the entropy source is important in determining how secure the resulting signatures are by ensuring that we are generating unpredictable private keys.
A planned use-case is running this as a GitHub Action (GHA). GHAs run on Virtual Machines. Typically, VMs are poor sources of entropy and are very deterministic unless effort is made to provision them with a quality entropy source.
There are several mitigating factors for key compromise built into the solution. Although, identifying the entropy source for GHA runners and improving it if needed, is another mitigating factor for preventing key compromise.
It appears that the ubuntu VM images have been updated to include haveged
actions/runner-images#672. Unsure if that is the latest change w.r.t. entropy sources in GHA VMs.
haveged is a daemon — derived from the HAVEGE algorithm — designed to help (the kernel) gather entropy from more sources (than the kernel itself does). It is common to install it on physical hosts to gather entropy faster from their entropy-rich environment. However, it is not recommended to use it in virtual machines, since the very reasons that make them prone to entropy starvation will hinder, if not defeat, HAVEGE (or the quality — randomness — of the entropy it will gather).
A lot of sources point to this writeup about HAVAGE
(the algorithm that haveged
is based on). It discusses how it uses the rdtsc (x86 processor time stamp counter with sub-nanosecond precision) as a source of entropy.
They highlight three possible VM configurations for how rdtsc
is implemented:
For a virtual machine system, there are three choices with regards to this instruction:
- Let the instruction go to the hardware directly.
- Trap the instruction and emulate it.
- Disable the instruction altogether.
I created a GHA action to run the example code to gain some insight into how the GHA VMs might be configured for rdtsc
and good news! It appears that the rdtsc
instruction is configured to go directly to the host hardware. Which is the best possible option of the three configurations.
See the action run here https://github.com/mrjoelkamp/gha-vm-rdtsc-test/actions/runs/6720417160/job/18264356488
Looking for a value between 10 - 100.
result:
average : 34.859
This result determines whether the GHA VMs are configured to use the host's rdtsc
derived from hardware as opposed to being emulated or not available. If it failed (and the entropy was emulated or hardcoded) that could be a major issue.
What it doesn't answer, is whether it is appropriate to use the GHA VM's entropy source (haveged
) for RNG when generating cryptographic key material. We also assume that the GHA VM will continue to provide an environment where we can securely generate key material and perform cryptographic operations in.
In addition to the current Go based KDF and crypto library, I recommend that OpenPubKey support cloud provider KMS solutions for cryptographic functions (AWS, GCP, Azure, Hashicorp, etc.). A lot of these systems are backed by Hardware Security Modules (HSMs) with high entropy RNG.
For example:
AWS KMS key generation is performed on the AWS KMS HSMs. The HSMs implement a hybrid random number generator that uses the NIST SP800-90A Deterministic Random Bit Generator (DRBG) CTR_DRBG using AES-256. It is seeded with a nondeterministic random bit generator with 384-bits of entropy and updated with additional entropy to provide prediction resistance on every call for cryptographic material.
There are many other added benefits to supporting external KMS solutions as well:
If the system only trusts the ephemeral keys to sign data in a short time frame, the compromise of the private ephemeral may be of little significance. Although, depending on how deterministic the RNG is, an attacker could potentially determine the private key ahead of ephemeral key generation with significant statistical probability.
Including a Transparency Log to store signatures that could be monitored by publishers to determine if an actor forged a signature using a compromised key and generated a signature outside of their control. Although, this countermeasure would be reactive as opposed to preventative and requires active monitoring and alerting.
The OPK One Time Signature could also mitigate this issue by providing a hash of the image in the payload that is signed by the Identity Provider. If an image was modified and signed by someone who had compromised a user signing key, the hash comparison would fail for the modified image. This requires that the verification client implement this check.
Currently the OpkClient only supports Google as the OIDC provider:
This should be pluggable so that we can create implementations for other OIDC providers too.
As discussed in Issue 26: Add GitLab CI signing example the approach used to sign software artifacts in github can not be used with gitlab because the gitlab workload can not specify the aud (audience) claim. Instead an aud claim is hardcoded in the workload configuration YAML. In this issue we propose the following design for gitlab and other settings where there is no client specified nonce or audience field.
To generate a PK Token:
The CIC is included as part of the GQ protected header of the ID Token. Thus, when the GQ signature signs the new protected header, it also commits to the CIC. As is currently done with GQ signatures, the original protected header of the ID Token is base64 encoded and included in the protected header signed by the GQ signature.
payload: {
"iss": "gitlab.example.com",
"aud": "OPENPUBKEY-PKTOKEN:1234567890",
...
}
signatures: [
{"protected": {"typ": "JWT", "alg": "GQ256", "kid": Base64(origProtected),
"cic": SHA3_256(upk=alice-pubkey, alg=ES256, rz=crypto.Rand()),
"signature": GQSIGN(RSA-sig, (payload, signatures[0].protected))`
},
{"protected": {"upk": alice-pubkey, "alg": "EC256", "rz": <rz>},
"signature": <SIGN(alice-signkey, (payload, signatures[1].protected))>
},
]
If configured correctly, this is approach offers the same security as the approach used with github. However this approach does introduce a security risk of misconfiguration not present in the github case. If a GQ signature is not used and an attacker learns the ID Token, the attacker can compute the GQ signature from the RSA signature and insert their own public key.
We mitigate the risk of this occurring by:
We argue that this sort of misconfiguration is very unlikely because outside of OpenPubkey, ID Tokens are treated as highly sensitive authentication secrets and within OpenPubkey we prevent this misconfiguration. Someone must configure their gitlab to put "OPENPUBKEY-PKTOKEN-" in the aud claim, not use the OpenPubkey protocol and then must expose the ID token to an attacker. We can not prevent people from writing their own insecure protocols.
We propose a security enhancement that adds a cosigner to remove the gitlab OP as a single point of compromise. It also provides additional protection against replay attacks, however we consider this protection to be unnecessary given the other safeguards above.
The aud claim can be postfixed by a commitment to a public key, cpk
, whose corresponding signing key, csk
, configured in gitlabs secret manager. This signing key can be used by the workload to sign the CIC in addition to the GQ signature.
Since this signing key's public key is committed to in the aud claim in the ID Token, an ID Token even with a aud claim prefixed with "OPENPUBKEY-PKTOKEN-" and the RSA signature is no longer sufficient to attest to the CIC.
payload: {
"iss": "gitlab.example.com",
"aud": "OPENPUBKEY-PKTOKEN-CPK"+SHA3_256(cpk),
...
}
signatures: [
{"protected": {"typ": "JWT", "alg": "GQ256", "kid": Base64(origProtected),
"cic": SHA3_256(upk=alice-pubkey, alg=ES256, rz=crypto.Rand()),
"signature": GQSIGN(RSA-sig, (payload, signatures[0].protected))`
},
{"protected": {"upk": alice-pubkey, "alg": "EC256", "rz": <rz>},
"signature": <SIGN(alice-signkey, (payload, signatures[1].protected))>
},
{"protected": {"cospk": cosigner-pubkey, "alg": "EC256"
"cic": SHA3_256(upk=alice-pubkey, alg=ES256, rz=crypto.Rand()),
},
"signature": <SIGN(cosigner-signkey, (payload, signatures[2].protected))>
},
]
Any OpenPubkey verifier configured to expect this audience claim can check this cosigner signature. This removes the gitlab OP as a single point of compromise. This approach can be adapted to JWKS URIs, but changing the aud claim to use be "OPENPUBKEY-PKTOKEN-CPK"+Cosigner-JWKS-URI
We currently have no automated test code coverage on CosignerVerifier. We recently hit a bug #114 in the CosignerVerifier that could've have been caught by a unittest.
This issue is to track adding unittests for CosignerVerifier which allows us to test our bugfix.
Create a standalone package for signing/verifying with GQ signatures. This can later be used to generate GQ signatures in place of OIDC provider signatures.
I removed pkt.Compact in #110 so it is reasonable to remove the unittest for pkt.Compact. I want to make sure we aren't losing any coverage by removing this unittest.
This is a very early draft to pose the questions of how we think about versioning OpenPubkey.
This issue exists to think discuss if, and in what places ,we might want to version the OpenPubkey. The primary purpose of such versioning would be simplify our lives. We should take a goal that version must be simple and not introduce complex handshakes or versioning negotiation.
We can version the PK Token using a key in the protected header of each signature we want to version. The OP signature and payload can't be versioned because it is OIDC token and not under our control. Do we want one version for the entire PK Token or instead version the CIC Signature and COS Signature separately.
The MFA Cosigner API uses the well-known URI and this provides an excellent point to specify parameters and versions from the cosigner to the client.
OSM and POP Auth versioning
OSM and POP Auth could be versioned at the signature or the API layer.
There are lots of calls to panic
and logrus.Fatalf
which can crash the calling program. We should instead make sure we're returning errors in most of these cases. (There may be good reasons to panic in a truly exceptional situation.)
Given the broad use of GitLab in environments where an OIDC provider as a CA has significant benefits, it would be helpful to have a reference example.
This would add a full end to end integration of OpenPubkey using an OP we spin up ourselves.
I want to have a discussion about what our goals are with the PK Token because it impacts data structures which are always nice to get as right as possible, as soon as possible.
I propose that the PK Token should have the following three properties:
Property 1: The PK Token does not require any outside information in order to verify itself
Property 2: A valid PK Token should have, at a minimum:
Property 3: The PK Token supports any number of cosigners
When using a Self-Issued OP, registration is not required. The Client can proceed without registration as if it had registered with the OP and obtained the following Client Registration Response:
client_id redirect_uri value of the Client.
OpenID Connect Core 1.0 7.2. Self-Issued OpenID Provider Registration
As discussed in #105 Fixes JWS signing bug where we JSON unmarshalling breaks verification, the function pkt.Compact
is intended to produce compact tokens that can pass signature verification. #105 makes pkt.Compact
safe to use by just having it pull the corresponding saved token from the PK Token struct. This makes pkt.Compact
redundant the developer could just pull the saved token themselves in fewer lines of code.
pkt.Compact
, developers should simply use the compact token saved to the PK Token.
Rather than:
cicToken, err := pkt.Compact(pkt.Cic)
do this instead:
cicToken := pkt.CicToken
This issue should be resolved when we create a PR to remove pkt.Compact
from the PK Token and from the examples. It depeneds on #105 being merged first.
The OPK paper discusses an method for the client instance authenticating to the MFA cosigner called "POP Auth". This protocol would make it so the MFA cosigner does not blindly trust the identity presented by the PK Token (which could be presented by anybody), but instead forces the requesting party to prove they have the corresponding signing key.
The reason this hasn't been included is because the details are still being ironed out and we will add this in when the protocol is ready for implementation.
The "zli login --opk" command does not automatically open the browser from Putty in Win10/Chrome.
Returning:
"Login required, opening browser"
Here the specific URL should be printed, to allow this to manually be copied into a browser.
Thanks.
PR #68 adds the OpenPubkey MFA Cosigner functionality. Since we now have a implementation it makes sense to provide a more detailed protocol description and standard.
Reading through #14 I had the following thoughts I wanted record about possible ways to flag that a signature if GQ Signature.
Consider the following two approaches:
We flag that a OP RSA signature has been replaced by a GQ Signature by adding a unsigned header field: "sig_type": "oidc_gq"
Lets call
{
"payload": "payload",
"signatures": [{
"protected": {"alg":"ES256","rz":"5","upk":{"crv":"P-256","kty":"EC","x":"RI","y":"Hk"}},
"header": {
"sig_type": "cic"
},
"signature": "user signature"
}, {
"protected": {"alg":"RS256","kid":"6f7254101f56e41cf35c9926de84a2d552b4c6f1","typ":"JWT"},
"header": {
"sig_type": "oidc_gq"
},
"signature": "GQ signature"
}]
}
Alternatively we could alter the protected header changing the alg
field to GQ256
and then signing the altered protected header and payload with the GQ signature. A verifier would then reconstruct the message the OP signed with RSA by changing the algorithm field back. It would look like:
{
"payload": "payload",
"signatures": [{
"protected": {"alg":"ES256","rz":"5","upk":{"crv":"P-256","kty":"EC","x":"RI","y":"Hk"}},
"header": {
"sig_type": "cic"
},
"signature": "user signature"
}, {
"protected": {"alg":"GQ256","kid":"6","typ":"JWT"},
"signature": "GQ signature"
}]
}
The unsigned header flag approach
Pros:
Cons:
sig_type
header so they will likely attempt to verify the GQ signature as if it was a very large RSA signature resulting in a hard to debug failure.The altered protected header approach
Pros:
alg
is set to algorithm they don't support and fail gracefully producing a meaningful error message: Signature algorithm GQ256
is unsupported.alg
is the standard way of flagging the signing algorithmCons:
We could bypass this con by storing the base64 encoding of the OPs signature in the protected header. That seems a little abusive toward the JWS standard.
Currently, an OpenPubkey token contains the original signature from the OIDC provider. This arguably means that it isn't safe to share the OpenPubkey token, as the token could be used to impersonate the subject to a service that doesn't check the audience claim, or to trick an OpenPubkey client to issue a token for the subject.
We can create a GQ signature to prove that we have the original OIDC provider's signature. This can be verified using the original OIDC signing payload (header || '.' || payload
) and the OIDC provider's public key. This works because GQ signature private keys are equivalent to RSA signatures.
The GQ signature can be published in place of the OIDC provider's signature in an OpenPubkey token.
We have a use case for binding a PK token to a single signature, so it would be useful to allow extra claims to be added in the CIC before it's signed by the OP.
This documentation should consist of:
Perhaps this can be done by modifying the library used to validate JWS tokens, or just by doing JWS validation ourselves.
As discussed in #67, we can further validate our GQ implementation by adding test cases against the specific numerical values in the ISO standard.
This will require some refactoring of the existing code because the ISO tests use SHA-1/PSS as a format mechanism, whereas we use SHAKE-256/PKCSv15.
Changes to the Cosiginer protocol and APIs are likely to present backwards compatibility problems. To address this we should:
Put the supported Cosigner protocol versions in the well-known configuration URI of the cosigner. Using the URL pattern like: https:///.well-known/openpubkey-configuration
{
"issuer":"https://<cosigner>",
"jwks_uri":"https://<cosigner>/.well-known/jwks.json",
"cosigner_versions_supported": ["1", "2", "4"],
}
Then the client can determine if any of the cosigner versions it supports match any of the cosigner versions supported by the cosigner server. If no match is found, the client outputs a meaningful error.
The client species the cosigner version it wants via the URI path it uses to access the API. For instance https://<cosigner>/<cosigner-version>/initAuthURI?...
.
Identified in #48
GQ signing uses the math/big
library for big integer math, specifically the modulus inverting function:
Lines 95 to 104 in 69500f1
Sensitive data (OIDC provider signature) is copied into memory within the big.Int
type as part of the SetBytes()
method. Unfortunately, the bit.Int Bytes()
method copies the underlying data to a new byte slice. So there is no way to wipe the data from this object once it is copied in.
We don't actually check that the nonce is a hash of the CIC when verifying a PK token. The check currently reads the nonce from the PK and then checks that the PK token contains the same nonce, and of course this will always be true. We should instead construct it from the CIC.
We have a function to compute the nonce hash already which is used to get the nonce when signing, but it suffers from the same JSON key order issue mentioned at the end of #15 - I haven't checked but wouldn't be surprised if the nonces we're generating aren't actually hashes of the CIC protected headers. We should instead hash the exact bytes that are going into the JWS. This might mean that we need to do some more manual signing of the JWS rather than leaning on the jws library.
As mentioned by @lgmugnier on the community call yesterday, there's an issue with the Google login example. The login process in the browser seems to work correctly but we get an error on the library side:
❯ go run example.go login
INFO[0000] listening on http://localhost:3000/
INFO[0000] press ctrl+c to stop
Error logging in: error verifying PK Token: failed to verify signature from OIDC provider: error verifying OP signature on PK Token (ID Token invalid): invalid signature (signature verification failed: square/go-jose: error in cryptographic primitive)
The original method to generate a PK Token is OidcAuth
. When adding the Cosigner, I added a second method called Auth
which performs OidcAuth
and then if a cosigner is configured, runs the cosigner protocol. The Auth
method is intended as a replacement for OidcAuth
but OidcAuth
is being kept around for backwards compatibility until we move it out of the docker code.
Auth
OidcAuth
from OpenPubkey codebase by changing it to oidcauth
so it is no longer exposed. This PR should move unittests on OidcAuth
to oidcauth
and add coverage to Auth
.We are currently on zitadel/oidc/v2 which has not been updated in sometime. It appears that if we want updates and patches we should move to zitadel/oidc/v3
We only use Zitadel in two places so the update should be fairly simple:
https://pkg.go.dev/github.com/zitadel/oidc/v2 Last updated Nov 16, 2023
https://pkg.go.dev/github.com/zitadel/oidc/v3 Last updated Apr 11, 2024 (the day this ticket was created)
The Zitadel provides an upgrade guide for upgrading from v2 to v3.
I'd like to propose that we remove memguard from OpenPubkey.
Some background to why we added memguard can be found in the original issues #11 and #47. The PR that added it is #48. I added a comment to that PR with some thoughts which I can expand on here.
I think there are three issues with the implementation:
Memguard does two things for us:
I think the wiping byte slices is a reasonable idea, but the LockedBuffer is more problematic. If we need to pass the underlying bytes from a LockedBuffer to a library function (in the stdlib or a dependency) and that function copies the bytes, we lose the protection that the LockedBuffer is giving us. This kind of thing is very difficult to keep track of - any upgrade to a dependency or the stdlib could introduce some copying without us noticing unless we're auditing the code. For the stdlib this is even more difficult because the version of Go that an application using OpenPubkey is built with determines the version of the stdlib.
Some places where this happens are documented in #51, #52, and #53. This may not be exhaustive!
I think at the end of the day, Go isn't really suited to this sort of manual memory management.
The way memguard needs to be used means that the OIDC client interface needs to know about memguard, which is slightly cumbersome for implementers.
This isn't the end of the world, and I do think that this added complexity of the implementation could be worth it if the result was totally bulletproof, but as it doesn't fully prevent secrets being swapped etc, I don't think the juice is worth the squeeze.
I've also ran into issues debugging the code with VS Code which I think is down to awnumar/memguard#150.
This would be a breaking change as memguard forms part of the public-facing interface as noted above. We should make a decision on whether to do this before we release v1.0.
I wanted to write up an issue on this because while we in OpenPubkey have discussed the nature of the relationship between OP (OpenID Providers) and PK Tokens there isn't much written down.
One can not meaningfully ask if a PK Token is valid by itself. The question validity must always been understood in reference to both a OpenID Provider and an OIDC Client.
Some examples:
For things like zkLogin we can use the architectural existence of providers clients (OPs) provides an excellent point to transform SNARK based ID Tokens into PK Tokens. The makes expiration of the PK Token depend on the provider client implementation. #101
Beyond just validity, using a PK Token depends on the OP.
The following pattern fits this relationship well:
googlePkt, err := GoogleOP.Verify(pkt)
if googlePkt.email == "[email protected]" {
...
}
Since we know that Google PK Tokens always have email addresses, if we cast PK Tokens to the subtypes of PK Tokens based on the OP we can let have compile time type checking of PK Token claims.
As noted in #3 our GQ signature implementation uses golang's bigint library which is known to be leak information via a timing side-channel. Before going 1.0 with this code base we need perform a review and remediation of cryptography used in this project.
At minimum this should consist of:
Suggestions to grow this list are welcome. When we get closer to 1.0, many of the items on list will be broken out into separate tickets.
Identified in #48
The pktoken
package in pktoken.go
uses the "github.com/lestrrat-go/jwx/v2/jws"
package as introduced in #45.
The OIDC provider signature is stored in the PKToken
struct in the Op
field as a jws.Signature
type:
Lines 28 to 35 in 463f605
The Signature
type stores the byte slice containing the signature data and is not directly exposed making it not possible to implement LockedBuffer
at this level. But we can wipe the value from memory using the memguard.WipeBytes()
method like so:
Lines 88 to 90 in 463f605
Although, this won't provide the same data safety features we would get if implemented as a LockedBuffer
. We could gain a bit more control over the memory management by reverting back to []byte
for PK token signatures (and refactoring them to LockedBuffer
) but a discussion around that should occur before proceeding.
DOI (Docker Official Images) using OpenPubkey extends OpenPubkey with the notion of single use signatures. This provides the security property that even if the signing key was compromised it can not be used or abused to sign another image.
To accomplish this Docker exploits the flexibility of the CIC and specifies an extra claim in CIC which commits to the hash of the data to be signed using the claim 'att'.
func (sv *opkSignerVerifier) Sign(ctx context.Context, data []byte) ([]byte, error) {
hash := s256(data) // SHA-256
hashHex := hex.EncodeToString(hash)
tokSigner, err := pktoken.NewSigner("", "ES256", true, map[string]any{"att": hashHex})"
https://github.com/openpubkey/signed-attestation/blob/main/opk.go#L24
This results in CIC
which is {'rz': crypto.random(), 'upk': <publickey>, 'alg': 'EC256', 'sig': SHA-256(dokcerImage)}
This single use signature feature is generic. We should consider if we want to support this as part of OpenPubkey itself.
Identified in #48
The crypto
package is used for ephemeral key generation and cic
signing:
Lines 100 to 109 in cdf1ff6
Lines 61 to 65 in a11ca00
The private key is generated and stored in crypto.PrivateKey
which uses math/big
big.Int
to store the private key data. Similar to #51 the bit.Int Bytes()
method copies the underlying data to a new byte slice. So there is no way to wipe the data from this object once it is copied in.
RFC-7515: JWS (JSON Web Signatures) specifies two serialization formats for JWS: JSON and Compact.
Two closely related serializations for JWSs are defined. The JWS
Compact Serialization is a compact, URL-safe representation intended
for space-constrained environments such as HTTP Authorization headers
and URI query parameters. The JWS JSON Serialization represents JWSs
as JSON objects and enables multiple signatures and/or MACs to be
applied to the same content.
RFC-7515: Section 7.1 defines the Compact Serialization format as:
7.1 JWS Compact Serialization
The JWS Compact Serialization represents digitally signed or MACed
content as a compact, URL-safe string. This string is:
BASE64URL(UTF8(JWS Protected Header)) || '.' ||
BASE64URL(JWS Payload) || '.' ||
BASE64URL(JWS Signature)
Only one signature/MAC is supported by the JWS Compact Serialization
and it provides no syntax to represent a JWS Unprotected Header
value.
Note that this means the JWS Compact Serialization only supports one signature. This prevents us from using it as-is for PK Tokens as our PK Tokens are JWS with more than one signature.
While the JWS JSON Serialization supports multiple signatures and thus can easily support our PK Tokens the RFC says: This representation is neither optimized for compactness nor URL-safe.
It would be useful for OpenPubkey to have a compact, URL-safe serialization. I am proposing the following format for JWS Compact Serialization format.
(JWS Protected Header1,Signature1) through (JWS Protected HeaderN,SignatureN)
BASE64URL(JWS Payload) || ':' ||
BASE64URL(UTF8(JWS Protected Header1)) || ':' ||
BASE64URL(JWS Signature1) || ':' ||
BASE64URL(UTF8(JWS Protected Header2)) || ':' ||
BASE64URL(JWS Signature2) || ':' ||
...
BASE64URL(UTF8(JWS Protected HeaderN)) || ':' ||
BASE64URL(JWS SignatureN)
in golang
func FromCompact(pktCom []byte) (*PKToken, error) {
splitCom := bytes.Split(pktCom, []byte(":"))
if len(splitCom) == 5 {
return &PKToken{
Payload: splitCom[0],
OpPH: splitCom[1],
OpSig: splitCom[2],
CicPH: splitCom[3],
CicSig: splitCom[4],
CosPH: nil,
CosSig: nil,
}, nil
} else if len(splitCom) == 7 {
return &PKToken{
Payload: splitCom[0],
OpPH: splitCom[1],
OpSig: splitCom[2],
CicPH: splitCom[3],
CicSig: splitCom[4],
CosPH: splitCom[5],
CosSig: splitCom[6],
}, nil
} else {
return nil, fmt.Errorf("A valid PK Token should have exactly two or three (protected header, signature pairs), but has %d signatures", len(splitCom))
}
}
func (p *PKToken) ToCompact() []byte {
if p.Payload == nil {
panic(fmt.Errorf("Payload can not be nil"))
}
var buf bytes.Buffer
buf.WriteString(string(p.Payload))
buf.WriteByte(':')
buf.WriteString(string(p.OpPH))
buf.WriteByte(':')
buf.WriteString(string(p.OpSig))
buf.WriteByte(':')
buf.WriteString(string(p.CicPH))
buf.WriteByte(':')
buf.WriteString(string(p.CicSig))
if p.CosPH != nil {
buf.WriteByte(':')
buf.WriteString(string(p.CosPH))
buf.WriteByte(':')
buf.WriteString(string(p.CosSig))
}
pktCom := buf.Bytes()
return pktCom
}
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.