Skip to content

Commit 120dc57

Browse files
committed
Hashing sensitive info
Signed-off-by: Effi-S <effi.szt@gmail.com>
1 parent d5fa476 commit 120dc57

9 files changed

Lines changed: 136 additions & 8 deletions

File tree

token/services/identity/idemix/crypto/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ func NewFabricCAIdemixConfig(issuerPublicKey []byte, dir string) (*Config, error
173173
func NewConfigFromRaw(issuerPublicKey []byte, configRaw []byte) (*Config, error) {
174174
config := &config.IdemixConfig{}
175175
if err := proto.Unmarshal(configRaw, config); err != nil {
176-
return nil, errors.Wrapf(err, "failed to unmarshal idemix config at [%s]", string(configRaw))
176+
return nil, errors.Wrapf(err, "failed to unmarshal idemix config of PK [%s]", string(issuerPublicKey))
177177
}
178178
// match public keys
179179
if !bytes.Equal(issuerPublicKey, config.Ipk) {

token/services/identity/idemix/crypto/deserializer.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
bccsp "github.com/IBM/idemix/bccsp/types"
1313
"github.com/LFDT-Panurus/panurus/token/services/identity/idemix/schema"
14+
"github.com/LFDT-Panurus/panurus/token/services/utils"
1415
"github.com/hyperledger-labs/fabric-smart-client/pkg/utils/errors"
1516
"github.com/hyperledger-labs/fabric-smart-client/pkg/utils/proto"
1617
)
@@ -112,7 +113,7 @@ func (d *Deserializer) DeserializeAgainstNymEID(identity []byte, nymEID []byte)
112113
func (d *Deserializer) DeserializeAuditInfo(_ context.Context, raw []byte) (*AuditInfo, error) {
113114
ai, err := DeserializeAuditInfo(raw)
114115
if err != nil {
115-
return nil, errors.Wrapf(err, "failed deserializing audit info [%s]", string(raw))
116+
return nil, errors.Wrapf(err, "failed deserializing audit info [%s]", utils.Hashable(raw))
116117
}
117118
ai.Csp = d.Csp
118119
ai.IssuerPublicKey = d.IssuerPublicKey

token/services/identity/idemix/deserializer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ type AuditInfoDeserializer struct{}
181181
func (c *AuditInfoDeserializer) DeserializeAuditInfo(ctx context.Context, identity driver.Identity, raw []byte) (driver2.AuditInfo, error) {
182182
ai, err := crypto2.DeserializeAuditInfo(raw)
183183
if err != nil {
184-
return nil, errors.Wrapf(err, "failed deserializing audit info [%s]", string(raw))
184+
return nil, errors.Wrapf(err, "failed deserializing audit info [%s]", utils.Hashable(raw))
185185
}
186186

187187
return ai, nil

token/services/identity/idemixnym/deserializer.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
driver2 "github.com/LFDT-Panurus/panurus/token/services/identity/driver"
1515
"github.com/LFDT-Panurus/panurus/token/services/identity/idemix"
1616
"github.com/LFDT-Panurus/panurus/token/services/identity/idemixnym/nym"
17+
"github.com/LFDT-Panurus/panurus/token/services/utils"
1718
"github.com/hyperledger-labs/fabric-smart-client/pkg/utils/errors"
1819
)
1920

@@ -103,7 +104,7 @@ type AuditInfoDeserializer struct{}
103104
func (c *AuditInfoDeserializer) DeserializeAuditInfo(ctx context.Context, identity driver.Identity, raw []byte) (driver2.AuditInfo, error) {
104105
ai, err := nym.DeserializeAuditInfo(raw)
105106
if err != nil {
106-
return nil, errors.Wrapf(err, "failed deserializing audit info [%s]", string(raw))
107+
return nil, errors.Wrapf(err, "failed deserializing audit info [%s]", utils.Hashable(raw))
107108
}
108109

109110
return ai, nil

token/services/identity/interop/htlc/deserializer.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/LFDT-Panurus/panurus/token/services/identity"
1515
idriver "github.com/LFDT-Panurus/panurus/token/services/identity/driver"
1616
"github.com/LFDT-Panurus/panurus/token/services/interop/htlc"
17+
"github.com/LFDT-Panurus/panurus/token/services/utils"
1718
"github.com/hyperledger-labs/fabric-smart-client/pkg/utils/errors"
1819
)
1920

@@ -182,14 +183,14 @@ func (a *AuditDeserializer) DeserializeAuditInfo(ctx context.Context, identity d
182183
si := &ScriptInfo{}
183184
err = json.Unmarshal(raw, si)
184185
if err != nil || (len(si.Sender) == 0 && len(si.Recipient) == 0) {
185-
return nil, errors.Errorf("invalid audit info, failed unmarshal [%s][%d][%d]", string(raw), len(si.Sender), len(si.Recipient))
186+
return nil, errors.Errorf("invalid audit info, failed unmarshal [%s][%d][%d]", utils.Hashable(raw), len(si.Sender), len(si.Recipient))
186187
}
187188
if len(si.Recipient) == 0 {
188189
return nil, errors.Errorf("no recipient defined")
189190
}
190191
ai, err := a.AuditInfoDeserializer.DeserializeAuditInfo(ctx, script.Recipient, si.Recipient)
191192
if err != nil {
192-
return nil, errors.Wrapf(err, "failed unmarshalling audit info [%s]", raw)
193+
return nil, errors.Wrapf(err, "failed unmarshalling audit info [%s]", utils.Hashable(raw))
193194
}
194195

195196
return ai, nil

token/services/identity/interop/htlc/validator.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313

1414
"github.com/LFDT-Panurus/panurus/token/services/identity"
1515
"github.com/LFDT-Panurus/panurus/token/services/interop/htlc"
16+
"github.com/LFDT-Panurus/panurus/token/services/utils"
1617
"github.com/hyperledger-labs/fabric-smart-client/pkg/utils/errors"
1718
)
1819

@@ -87,7 +88,7 @@ func MetadataClaimKeyCheck(action Action, script *htlc.Script, op OperationType,
8788
// Unmarshal signature to ClaimSignature
8889
claim := &htlc.ClaimSignature{}
8990
if err := json.Unmarshal(sig, claim); err != nil {
90-
return "", errors.Wrapf(err, "failed unmarshalling claim signature [%s]", string(sig))
91+
return "", errors.Wrapf(err, "failed unmarshalling claim signature [%s]", utils.Hashable(sig))
9192
}
9293
// Check that it is well-formed
9394
if len(claim.Preimage) == 0 || len(claim.RecipientSignature) == 0 {

token/services/identity/x509/crypto/config.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ SPDX-License-Identifier: Apache-2.0
77
package crypto
88

99
import (
10+
"fmt"
11+
1012
"github.com/LFDT-Panurus/panurus/token/services/identity/x509/crypto/pkcs11"
1113
"github.com/LFDT-Panurus/panurus/token/services/identity/x509/crypto/protos-go/v1/config"
1214
"github.com/go-viper/mapstructure/v2"
@@ -75,6 +77,20 @@ type KeyIDMapping struct {
7577
ID string `yaml:"ID,omitempty"`
7678
}
7779

80+
// String renders the PKCS11 configuration for logging with the PIN redacted, so
81+
// the secret can never reach a log line or error string, even when a *PKCS11 is
82+
// interpolated directly or as a field of a *BCCSP. It is nil-safe.
83+
func (p *PKCS11) String() string {
84+
if p == nil {
85+
return "<nil>"
86+
}
87+
88+
return fmt.Sprintf(
89+
"{Security:%d Hash:%s Library:%s Label:%s Pin:[REDACTED] SoftwareVerify:%t Immutable:%t AltID:%s KeyIDs:%v SessionCacheSize:%d}",
90+
p.Security, p.Hash, p.Library, p.Label, p.SoftwareVerify, p.Immutable, p.AltID, p.KeyIDs, p.SessionCacheSize,
91+
)
92+
}
93+
7894
// ToBCCSPOpts converts the passed opts to `config.BCCSP`
7995
func ToBCCSPOpts(boxed any) (*BCCSP, error) {
8096
opts := &Opts{}

token/services/identity/x509/crypto/pkcs11/pkcs11.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,11 @@ type (
3232
func NewProvider(opts PKCS11Opts, ks bccsp.KeyStore, mapper func(ski []byte) []byte) (*pkcs11.Provider, error) {
3333
csp, err := pkcs11.New(opts, ks, pkcs11.WithKeyMapper(mapper))
3434
if err != nil {
35-
return nil, errors.WithMessagef(err, "Failed initializing PKCS11 library with config [%+v]", opts)
35+
// redact the PIN on the copy so the secret never reaches the error string.
36+
safeOpts := opts
37+
safeOpts.Pin = "[REDACTED]"
38+
39+
return nil, errors.WithMessagef(err, "Failed initializing PKCS11 library with config [%+v]", safeOpts)
3640
}
3741
return csp, nil
3842
}
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/*
2+
Copyright IBM Corp. All Rights Reserved.
3+
4+
SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
package crypto
8+
9+
import (
10+
"fmt"
11+
"strings"
12+
"testing"
13+
14+
"github.com/hyperledger/fabric-lib-go/bccsp"
15+
"github.com/stretchr/testify/assert"
16+
"github.com/stretchr/testify/require"
17+
)
18+
19+
const testPIN = "s3cr3t-pin-9999"
20+
21+
// PKCS11.String must redact the PIN and never emit it verbatim, whether the
22+
// *PKCS11 is logged directly or as a field of a *BCCSP.
23+
func TestPKCS11_String_RedactsPin(t *testing.T) {
24+
p := &PKCS11{Library: "/usr/lib/libpkcs11.so", Label: "tok", Pin: testPIN}
25+
26+
s := p.String()
27+
assert.Contains(t, s, "[REDACTED]")
28+
assert.NotContains(t, s, testPIN)
29+
// Non-secret fields remain visible for debugging.
30+
assert.Contains(t, s, "/usr/lib/libpkcs11.so")
31+
assert.Contains(t, s, "tok")
32+
33+
// %v / %s go through the Stringer as well.
34+
assert.NotContains(t, fmt.Sprintf("%v", p), testPIN)
35+
assert.NotContains(t, fmt.Sprintf("%s", p), testPIN)
36+
}
37+
38+
// A nil *PKCS11 must format without panicking (SW-only configs have a nil PKCS11).
39+
func TestPKCS11_String_Nil(t *testing.T) {
40+
var p *PKCS11
41+
assert.NotPanics(t, func() { _ = p.String() })
42+
assert.Equal(t, "<nil>", p.String())
43+
assert.NotPanics(t, func() { _ = fmt.Sprintf("%v", p) })
44+
}
45+
46+
// Formatting a *BCCSP (as setup.go and kmp.go do) must redact the nested PKCS11 PIN,
47+
// and must not panic when the PKCS11 field is nil.
48+
func TestBCCSP_Format_RedactsPin(t *testing.T) {
49+
t.Run("WithPKCS11", func(t *testing.T) {
50+
cfg := &BCCSP{Default: "PKCS11", PKCS11: &PKCS11{Label: "tok", Pin: testPIN}}
51+
out := fmt.Sprintf("%v", cfg)
52+
assert.Contains(t, out, "[REDACTED]")
53+
assert.NotContains(t, out, testPIN)
54+
})
55+
t.Run("NilPKCS11", func(t *testing.T) {
56+
cfg := &BCCSP{Default: "SW", SW: &SoftwareProvider{Hash: "SHA2", Security: 256}}
57+
assert.NotPanics(t, func() { _ = fmt.Sprintf("%v", cfg) })
58+
})
59+
t.Run("NilBCCSP", func(t *testing.T) {
60+
var cfg *BCCSP
61+
assert.NotPanics(t, func() { _ = fmt.Sprintf("%v", cfg) })
62+
})
63+
}
64+
65+
// getIdentityFactory must never mutate the caller's live config and must never panic,
66+
// across the nil, SW-only, and error paths. Regression test for the pointer-aliasing
67+
// redaction bug (Issue #2069) that overwrote the real PIN and dereferenced nil.
68+
func TestGetIdentityFactory_NoMutationNoPanic(t *testing.T) {
69+
conf := &Config{CryptoConfig: &CryptoConfig{SignatureHashFamily: bccsp.SHA2}}
70+
71+
t.Run("NilBccspConfig", func(t *testing.T) {
72+
// Default SW provider; nil bccspConfig is a supported, common path.
73+
assert.NotPanics(t, func() {
74+
_, err := getIdentityFactory(conf, nil, nil)
75+
require.NoError(t, err)
76+
})
77+
})
78+
79+
t.Run("SWOnlyConfigLeavesPinUntouched", func(t *testing.T) {
80+
bccspConfig := &BCCSP{
81+
Default: "SW",
82+
SW: &SoftwareProvider{Hash: "SHA2", Security: 256},
83+
// A PIN may be present even on the SW path; it must survive verbatim.
84+
PKCS11: &PKCS11{Label: "tok", Pin: testPIN},
85+
}
86+
87+
_, err := getIdentityFactory(conf, bccspConfig, nil)
88+
require.NoError(t, err)
89+
assert.Equal(t, testPIN, bccspConfig.PKCS11.Pin, "the caller's live PIN must not be mutated")
90+
})
91+
92+
t.Run("ErrorPathRedactsWithoutMutating", func(t *testing.T) {
93+
bccspConfig := &BCCSP{
94+
Default: "does-not-exist", // forces GetBCCSPFromConf to return an error
95+
PKCS11: &PKCS11{Label: "tok", Pin: testPIN},
96+
}
97+
98+
_, err := getIdentityFactory(conf, bccspConfig, nil)
99+
require.Error(t, err)
100+
assert.True(t, strings.Contains(err.Error(), "[REDACTED]"), "error must show redaction marker")
101+
assert.False(t, strings.Contains(err.Error(), testPIN), "error must not leak the PIN")
102+
assert.Equal(t, testPIN, bccspConfig.PKCS11.Pin, "the caller's live PIN must not be mutated")
103+
})
104+
}

0 commit comments

Comments
 (0)