Skip to content

Security audit fixes: harden key derivation, demote sensitive logs, version-gate protocol changes #908

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

Closed
DavidGershony wants to merge 1 commit into from
Closed

Security audit fixes: harden key derivation, demote sensitive logs, version-gate protocol changes #908

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/shared/Angor.Shared.Tests/DataBuilders/InvestmentOperations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Text;
using System.Text;
using Angor.Shared;
using Angor.Shared.Models;
using Blockcore.NBitcoin.DataEncoders;
Expand Down Expand Up @@ -88,18 +88,18 @@ public Transaction SignInvestmentTransaction(Network network,string changeAddres
//var nbitcoinNetwork = NetworkMapper.Map(network);
//var trx = NBitcoin.Transaction.Parse(transaction.ToHex(), nbitcoinNetwork);

var coins = _walletOperations.GetUnspentOutputsForTransaction(walletWords, utxoDataWithPaths);
var signingCoins = _walletOperations.GetUnspentOutputsForTransaction(walletWords, utxoDataWithPaths);

// var fees = _walletOperations.GetFeeEstimationAsync().GetAwaiter().GetResult();
// var fee = fees.First(f => f.Confirmations == 1);


//var incoins = coins.coins.Select(c => new NBitcoin.Coin(OutPoint.Parse(c.Outpoint.ToString()), new NBitcoin.TxOut(NBitcoin.Money.Satoshis(c.Amount.Satoshi), new NBitcoin.Script(c.ScriptPubKey.ToBytes()))));
//var inKeys = coins.keys.Select(k => new Key(k.ToBytes())).ToArray();
//var incoins = signingCoins.Select(sc => new NBitcoin.Coin(OutPoint.Parse(sc.Coin.Outpoint.ToString()), new NBitcoin.TxOut(NBitcoin.Money.Satoshis(sc.Coin.Amount.Satoshi), new NBitcoin.Script(sc.Coin.ScriptPubKey.ToBytes()))));
//var inKeys = signingCoins.Select(sc => new Key(sc.Key.ToBytes())).ToArray();

var builder = new TransactionBuilder(network) // nbitcoinNetwork.CreateTransactionBuilder()
.AddCoins(coins.coins)
.AddKeys(coins.keys.ToArray())
.AddCoins(signingCoins.Select(sc => sc.Coin).ToList())
.AddKeys(signingCoins.Select(sc => sc.Key).ToArray())
.SetChange(BitcoinAddress.Create(changeAddress, network))
.ContinueToBuild(transaction)
.SendEstimatedFees(new FeeRate(Money.Satoshis(feeRate.FeeRate)))
Expand Down
2 changes: 1 addition & 1 deletion src/shared/Angor.Shared.Tests/InvestmentOperationsTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public InvestmentOperationsTest()

var coins = keys.Select(key => new Coin(fakeInputTrx, fakeTxout)).ToList();

return (coins, keys);
return coins.Zip(keys, (c, k) => new SigningCoin(c, k)).ToList();
});

_networkConfiguration = new Mock<INetworkConfiguration>();
Expand Down
2 changes: 1 addition & 1 deletion src/shared/Angor.Shared.Tests/Protocol/FounderTransactionActionTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public FounderTransactionActionTest()

var coins = keys.Select(key => new Blockcore.NBitcoin.Coin(fakeInputTrx, fakeTxout)).ToList();

return (coins, keys);
return coins.Zip(keys, (c, k) => new SigningCoin(c, k)).ToList();
});


Expand Down
2 changes: 1 addition & 1 deletion src/shared/Angor.Shared.Tests/Protocol/InvestmentIntegrationsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public InvestmentIntegrationsTests()

var coins = keys.Select(key => new Coin(fakeInputTrx, fakeTxout)).ToList();

return (coins, keys);
return coins.Zip(keys, (c, k) => new SigningCoin(c, k)).ToList();
});

_networkConfiguration = new Mock<INetworkConfiguration>();
Expand Down
11 changes: 5 additions & 6 deletions src/shared/Angor.Shared.Tests/WalletOperationsTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,13 +365,12 @@ public void GetUnspentOutputsForTransaction_ReturnsCorrectOutputs()
var walletOperations = new WalletOperations(null, mockHdOperations.Object, null, null);

// Act
var (coins, keys) = walletOperations.GetUnspentOutputsForTransaction(walletWords, utxos);
var signingCoins = walletOperations.GetUnspentOutputsForTransaction(walletWords, utxos);

// Assert
Assert.Single(coins);
Assert.Single(keys);
Assert.Equal((uint)0, coins[0].Outpoint.N);
Assert.Equal(1500000, coins[0].Amount.Satoshi);
Assert.Equal(expectedExtKey.Derive(new KeyPath("m/0/0")).PrivateKey, keys[0]);
Assert.Single(signingCoins);
Assert.Equal((uint)0, signingCoins[0].Coin.Outpoint.N);
Assert.Equal(1500000, signingCoins[0].Coin.Amount.Satoshi);
Assert.Equal(expectedExtKey.Derive(new KeyPath("m/0/0")).PrivateKey, signingCoins[0].Key);
}
}
188 changes: 144 additions & 44 deletions src/shared/Angor.Shared/DerivationOperations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,28 +81,51 @@ public FounderKeys GetProjectKey(FounderKeyCollection founderKeyCollection, int

public string DeriveLeadInvestorSecretHash(WalletWords walletWords, string founderKey)
{
ExtKey extendedKey = GetExtendedKey(walletWords);
return DeriveLeadInvestorSecretHash(walletWords, founderKey, projectVersion: 1);
}

var upi = this.DeriveUniqueProjectIdentifier(founderKey);
public string DeriveLeadInvestorSecretHash(WalletWords walletWords, string founderKey, int projectVersion)
{
ExtKey extendedKey = GetExtendedKey(walletWords);

var path = $"m/5'/0'/{upi}'/2'";
var subPath = BuildProjectSubPath(founderKey, projectVersion);
var path = $"m/5'/0'/{subPath}/2'";

ExtPubKey extPubKey = _hdOperations.GetExtendedPublicKey(extendedKey.PrivateKey, extendedKey.ChainCode, path);

var derivedSecret = extendedKey.Derive(new KeyPath(path));

var hash = Hashes.Hash256(derivedSecret.ToBytes()).ToString();
if (projectVersion >= 3)
{
// V3+: Hash only the public key (deterministic, doesn't expose private key bytes)
var pubKeyBytes = extPubKey.PubKey.ToBytes();
var hash = Hashes.Hash256(pubKeyBytes).ToString();
return hash;
}

return hash;
// V1/V2 legacy: hash full ExtKey serialization (backward compat)
var derivedSecret = extendedKey.Derive(new KeyPath(path));
var secretBytes = derivedSecret.ToBytes();
try
{
var hash = Hashes.Hash256(secretBytes).ToString();
return hash;
}
finally
{
System.Security.Cryptography.CryptographicOperations.ZeroMemory(secretBytes);
}
}

public string DeriveInvestorKey(WalletWords walletWords, string founderKey)
{
ExtKey extendedKey = GetExtendedKey(walletWords);
return DeriveInvestorKey(walletWords, founderKey, projectVersion: 1);
}

var upi = this.DeriveUniqueProjectIdentifier(founderKey);
public string DeriveInvestorKey(WalletWords walletWords, string founderKey, int projectVersion)
{
ExtKey extendedKey = GetExtendedKey(walletWords);

var path = $"m/5'/0'/{upi}'/3'";
var subPath = BuildProjectSubPath(founderKey, projectVersion);
var path = $"m/5'/0'/{subPath}/3'";

ExtPubKey extPubKey = _hdOperations.GetExtendedPublicKey(extendedKey.PrivateKey, extendedKey.ChainCode, path);

Expand All @@ -111,11 +134,15 @@ public string DeriveInvestorKey(WalletWords walletWords, string founderKey)

public AngorKey DeriveInvestorPrivateKey(WalletWords walletWords, string founderKey)
{
ExtKey extendedKey = GetExtendedKey(walletWords);
return DeriveInvestorPrivateKey(walletWords, founderKey, projectVersion: 1);
}

var upi = this.DeriveUniqueProjectIdentifier(founderKey);
public AngorKey DeriveInvestorPrivateKey(WalletWords walletWords, string founderKey, int projectVersion)
{
ExtKey extendedKey = GetExtendedKey(walletWords);

var path = $"m/5'/0'/{upi}'/3'";
var subPath = BuildProjectSubPath(founderKey, projectVersion);
var path = $"m/5'/0'/{subPath}/3'";

ExtPubKey extPubKey = _hdOperations.GetExtendedPublicKey(extendedKey.PrivateKey, extendedKey.ChainCode, path);

Expand All @@ -137,11 +164,15 @@ public string DeriveFounderKey(WalletWords walletWords, int index)

public string DeriveNostrPubKey(WalletWords walletWords, string founderKey)
{
ExtKey extendedKey = GetExtendedKey(walletWords);
return DeriveNostrPubKey(walletWords, founderKey, projectVersion: 1);
}

var upi = this.DeriveUniqueProjectIdentifier(founderKey);
public string DeriveNostrPubKey(WalletWords walletWords, string founderKey, int projectVersion)
{
ExtKey extendedKey = GetExtendedKey(walletWords);

var path = $"m/44'/1237'/{upi}'/0/0";
var subPath = BuildProjectSubPath(founderKey, projectVersion);
var path = $"m/44'/1237'/{subPath}/0/0";

ExtKey extKey = extendedKey.Derive(new KeyPath(path));

Expand All @@ -150,11 +181,15 @@ public string DeriveNostrPubKey(WalletWords walletWords, string founderKey)

public string DeriveFounderRecoveryKey(WalletWords walletWords, string founderKey)
{
ExtKey extendedKey = GetExtendedKey(walletWords);
return DeriveFounderRecoveryKey(walletWords, founderKey, projectVersion: 1);
}

var upi = this.DeriveUniqueProjectIdentifier(founderKey);
public string DeriveFounderRecoveryKey(WalletWords walletWords, string founderKey, int projectVersion)
{
ExtKey extendedKey = GetExtendedKey(walletWords);

var path = $"m/5'/0'/{upi}'/1'";
var subPath = BuildProjectSubPath(founderKey, projectVersion);
var path = $"m/5'/0'/{subPath}/1'";

ExtPubKey extPubKey = _hdOperations.GetExtendedPublicKey(extendedKey.PrivateKey, extendedKey.ChainCode, path);

Expand All @@ -174,11 +209,15 @@ public AngorKey DeriveFounderPrivateKey(WalletWords walletWords, int index)

public AngorKey DeriveFounderRecoveryPrivateKey(WalletWords walletWords, string founderKey)
{
ExtKey extendedKey = GetExtendedKey(walletWords);
return DeriveFounderRecoveryPrivateKey(walletWords, founderKey, projectVersion: 1);
}

var upi = this.DeriveUniqueProjectIdentifier(founderKey);
public AngorKey DeriveFounderRecoveryPrivateKey(WalletWords walletWords, string founderKey, int projectVersion)
{
ExtKey extendedKey = GetExtendedKey(walletWords);

var path = $"m/5'/0'/{upi}'/1'";
var subPath = BuildProjectSubPath(founderKey, projectVersion);
var path = $"m/5'/0'/{subPath}/1'";

ExtKey extKey = extendedKey.Derive(new KeyPath(path));

Expand All @@ -187,24 +226,32 @@ public AngorKey DeriveFounderRecoveryPrivateKey(WalletWords walletWords, string

public AngorKey DeriveProjectNostrPrivateKey(WalletWords walletWords, string founderKey)
{
ExtKey extendedKey = GetExtendedKey(walletWords);
return DeriveProjectNostrPrivateKey(walletWords, founderKey, projectVersion: 1);
}

var upi = this.DeriveUniqueProjectIdentifier(founderKey);
public AngorKey DeriveProjectNostrPrivateKey(WalletWords walletWords, string founderKey, int projectVersion)
{
ExtKey extendedKey = GetExtendedKey(walletWords);

var path = $"m/44'/1237'/{upi}'/0/0";
var subPath = BuildProjectSubPath(founderKey, projectVersion);
var path = $"m/44'/1237'/{subPath}/0/0";

ExtKey extKey = extendedKey.Derive(new KeyPath(path));

return AngorKey.From(extKey.PrivateKey);
}

public async Task<AngorKey> DeriveProjectNostrPrivateKeyAsync(WalletWords walletWords, string founderKey)
{
return await DeriveProjectNostrPrivateKeyAsync(walletWords, founderKey, projectVersion: 1);
}

public async Task<AngorKey> DeriveProjectNostrPrivateKeyAsync(WalletWords walletWords, string founderKey, int projectVersion)
{
ExtKey extendedKey = GetExtendedKey(walletWords);

var upi = this.DeriveUniqueProjectIdentifier(founderKey);

var path = $"m/44'/1237'/{upi}'/0/0";
var subPath = BuildProjectSubPath(founderKey, projectVersion);
var path = $"m/44'/1237'/{subPath}/0/0";

var extKey = await Task.Run(() => extendedKey.Derive(new KeyPath(path)));

Expand All @@ -222,14 +269,48 @@ public uint DeriveUniqueProjectIdentifier(string founderKey)

var upi = (uint)(hashOfid.GetLow64() & int.MaxValue);

_logger.LogInformation($"Unique Project Identifier - founderKey = {founderKey}, hashOfFounderKey = {hashOfid}, hashOfFounderKeyCastToInt = {upi}");
_logger.LogDebug("UPI derived: {Upi} for founderKey={FounderKey}", upi, founderKey);

if (upi >= 2_147_483_648)
throw new Exception();

return upi;
}

/// <summary>
/// Derives a two-level project sub-path with 62 bits of entropy (two 31-bit indices).
/// Used for Version 3+ projects to reduce collision probability from ~46k to ~2 billion projects.
/// </summary>
public (uint Hi, uint Lo) DeriveProjectIndicesV2(string founderKey)
{
var key = new PubKey(founderKey);
var hashOfid = Hashes.Hash256(key.ToBytes());
var low64 = hashOfid.GetLow64();

// Split 62 bits into two 31-bit indices (each valid for BIP-32 hardened derivation)
var hi = (uint)(low64 & int.MaxValue); // bits 0-30
var lo = (uint)((low64 >> 31) & int.MaxValue); // bits 31-61

return (hi, lo);
}

/// <summary>
/// Returns the project derivation sub-path component based on the project version.
/// V1/V2: single level "{upi}'" (31-bit entropy)
/// V3+: two levels "{hi}'/{lo}'" (62-bit entropy)
/// </summary>
private string BuildProjectSubPath(string founderKey, int projectVersion = 1)
{
if (projectVersion >= 3)
{
var (hi, lo) = DeriveProjectIndicesV2(founderKey);
return $"{hi}'/{lo}'";
}

var upi = DeriveUniqueProjectIdentifier(founderKey);
return $"{upi}'";
}

public string DeriveNostrStoragePubKeyHex(WalletWords walletWords)
{
var key = DeriveNostrStorageKey(walletWords);
Expand Down Expand Up @@ -283,33 +364,52 @@ public string DeriveNostrStoragePassword(WalletWords walletWords)
{
var key = DeriveNostrStorageKey(walletWords);

var privateKeyBytes = key.ToBytes();

var hashedKey = Hashes.Hash256(new Span<byte>(privateKeyBytes));

// the hex of the hash of the private key is the password
var hex = Encoders.Hex.EncodeData(hashedKey.ToArray()).Replace("-", "").ToLower();
var privateKeyBytes = key.ToBytes();
try
{
var hashedKey = Hashes.Hash256(new Span<byte>(privateKeyBytes));

return hex;
// Hex-encoded double-SHA256 of the private key serves as the Nostr storage password
return Encoders.Hex.EncodeData(hashedKey.ToArray());
}
finally
{
System.Security.Cryptography.CryptographicOperations.ZeroMemory(privateKeyBytes);
}
}

public string DeriveAngorKey(string angorRootKey, string founderKey)
{
return DeriveAngorKey(angorRootKey, founderKey, projectVersion: 1);
}

public string DeriveAngorKey(string angorRootKey, string founderKey, int projectVersion)
{
Network network = _networkConfiguration.GetNetwork();

var extKey = new BitcoinExtPubKey(angorRootKey, network).ExtPubKey;

var upi = this.DeriveUniqueProjectIdentifier(founderKey);
if (projectVersion >= 3)
{
var (hi, lo) = DeriveProjectIndicesV2(founderKey);
var angorKey = extKey.Derive(hi).Derive(lo).PubKey;

var angorKey = extKey.Derive(upi).PubKey;

var encoder = new Bech32Encoder("angor");
var encoder = new Bech32Encoder("angor");
var address = encoder.Encode(0, angorKey.WitHash.ToBytes());

_logger.LogDebug("DeriveAngorKey V2 - founderKey={FounderKey}, hi={Hi}, lo={Lo}, address={Address}", founderKey, hi, lo, address);
return address;
}

var address = encoder.Encode(0, angorKey.WitHash.ToBytes());
var upi = this.DeriveUniqueProjectIdentifier(founderKey);
var legacyAngorKey = extKey.Derive(upi).PubKey;

var legacyEncoder = new Bech32Encoder("angor");
var legacyAddress = legacyEncoder.Encode(0, legacyAngorKey.WitHash.ToBytes());

_logger.LogInformation($"DeriveAngorKey - angorRootKey = {angorRootKey}, founderKey = {founderKey}, upi = {upi}, angorKey = {angorKey}, angorKeyWitHash = {angorKey.WitHash}, address = {address}");
_logger.LogDebug("DeriveAngorKey - founderKey={FounderKey}, upi={Upi}, address={Address}", founderKey, upi, legacyAddress);

return address;
return legacyAddress;
}

public Script AngorKeyToScript(string angorKey)
Expand All @@ -335,7 +435,7 @@ public string ConvertAngorKeyToBitcoinAddress(string projectId)
var networkEncoder = network.Bech32Encoders[(int)Bech32Type.WITNESS_PUBKEY_ADDRESS];
var bitcoinAddress = networkEncoder.Encode(witnessVersion, data);

_logger.LogInformation($"ConvertAngorKeyToBitcoinAddress - projectId = {projectId}, witnessVersion = {witnessVersion}, bitcoinAddress = {bitcoinAddress}");
_logger.LogDebug("ConvertAngorKeyToBitcoinAddress - projectId={ProjectId}, address={Address}", projectId, bitcoinAddress);

return bitcoinAddress;
}
Expand Down
1 change: 1 addition & 0 deletions src/shared/Angor.Shared/IDerivationOperations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public interface IDerivationOperations
string DeriveFounderKey(WalletWords walletWords, int index);
string DeriveFounderRecoveryKey(WalletWords walletWords, string founderKey);
uint DeriveUniqueProjectIdentifier(string founderKey);
(uint Hi, uint Lo) DeriveProjectIndicesV2(string founderKey);
string DeriveAngorKey(string angorRootKey, string founderKey);
Script AngorKeyToScript(string angorKey);
string DeriveInvestorKey(WalletWords walletWords, string founderKey);
Expand Down
Loading
Loading