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.

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

Update auth header token for CA #813

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

denyeart
Copy link
Contributor

@denyeart denyeart commented Jan 29, 2025

When interacting with Fabric CA REST services,
utilize the updated auth header token format
that includes the URL path and method.

The old format has been deprecated for a long time (since Fabric CA v1.4.0) since the new format is more secure. New versions of Fabric CA (v1.5.14 and later) will not support the old format by default (although they can still support the old format by setting FABRIC_CA_SERVER_COMPATIBILITY_MODE_V1_3=true).

For reference see similar change in legacy Node SDK that never got propagated to the console Fabric CA client:
hyperledger/fabric-sdk-node@602f557

And the original change in Fabric CA that is driving this change:
hyperledger/fabric-ca@99517e9

Note that the new format became effective in Fabric CA v1.4.0 (2019), therefore console will require Fabric CA v1.4.0 or later, which should not be a problem at this stage.

@denyeart denyeart requested a review from a team as a code owner January 29, 2025 06:22
When interacting with Fabric CA REST services,
utilize the updated auth header token format
that includes the URL path and method.

The old format has been deprecated for a long time
since the new format is more secure.

For reference see similar change in legacy Node SDK:
hyperledger/fabric-sdk-node@602f557

Signed-off-by: David Enyeart <[email protected]>
// get the auth token (csr)
const auth_token = t.fabric_utils.generateAuthToken(reenrollCSR, t.signingIdentity);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assume that passing t.signingIdentity was a leftover from an old implementation, so I've removed it. The current generateAuthToken() implementation already accesses t.signingIdentity internally.

Copy link
Contributor Author

@denyeart denyeart left a comment

Choose a reason for hiding this comment

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

Any advice on a unit test that may prove this out? I didn't see any existing related unit tests...

Copy link
Contributor

@dshuffma-ibm dshuffma-ibm left a comment

Choose a reason for hiding this comment

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

+1 looks good to me.

if you could though just comment on this PR the minimum version of a CA the console needs.

note that i'm not able to run this project anymore, so i didn't run anything.. though we do have tests, even on this file.
https://github.com/hyperledger-labs/fabric-operations-console/blob/main/packages/athena/test/test-suites/lib/ca_lib.test.js

i think you can run these tests with npm run test from the ./package/athena directory. there's also a curiously named script of npm run test:local that might be the one to do...

let sig = t.signing_lib.sign(bodyAndCert, { hashFunction: t.cryptoSuite.hash.bind(t.cryptoSuite), signingIdentity: t.signingIdentity });
if (path && method) {
const s = Buffer.from(path).toString('base64');
signString = method + '.' + s + '.' + signString;
Copy link
Contributor

Choose a reason for hiding this comment

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

this line is the real/main change right? the "new method" is to sign over a string that has the method and path. make sense!

so seems like once we merge this there will be some ancient CA versions that will no longer work with the latest console. CA's that are too old for the new way... for the sake of traceability could you jot down in this PR what min CA version the console needs now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is the main change, it is consistent with how all the other Fabric CA clients were updated back in 2019.

I've updated the Description with the Fabric CA versions.

@denyeart denyeart merged commit e8e84b0 into hyperledger-labs:main Feb 3, 2025
18 checks passed
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.

2 participants