Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
47 changes: 36 additions & 11 deletions secp-ffm/src/main/java/org/bitcoinj/secp/ffm/Secp256k1Foreign.java
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public SecpPrivKey ecPrivKeyCreate() {
do {
privKeySeg = fill_random(ta, 32);
} while (secp256k1_h.secp256k1_ec_seckey_verify(ctx, privKeySeg) != 1);
SecpPrivKey privKey = SecpPrivKey.of(privKeySeg.toArray(JAVA_BYTE));
SecpPrivKey privKey = new SecpPrivKeySegment(privKeySeg);
privKeySeg.fill((byte) 0x00);
return privKey;
}
Expand All @@ -151,9 +151,9 @@ public SecpPrivKey ecPrivKeyCreate() {
public SecpPubKey ecPubKeyCreate(SecpPrivKey privkey) {
try (Arena ta = Arena.ofConfined()) {
// Should we verify the key here for safety? (Probably)
MemorySegment privkeySegment = ta.allocateFrom(JAVA_BYTE, privkey.getEncoded());
MemorySegment privkeySegment = privKeySeg(ta, privkey);
MemorySegment pubKey = ecPubKeyCreate(ta, privkeySegment);
privkeySegment.fill((byte) 0x00);
zeroIfTemp(privkeySegment);
// Return serialized pubkey
return toSecpPubKey(ta, pubKey);
}
Expand Down Expand Up @@ -205,9 +205,9 @@ public SecpKeyPair ecKeyPairCreate() {
public SecpKeyPair ecKeyPairCreate(SecpPrivKey privKey) {
try (Arena ta = Arena.ofConfined()) {
MemorySegment keyPairSeg = secp256k1_keypair.allocate(ta);
MemorySegment privKeySeg = ta.allocateFrom(JAVA_BYTE, privKey.getEncoded());
MemorySegment privKeySeg = privKeySeg(ta, privKey);
int return_val = secp256k1_h.secp256k1_keypair_create(ctx, keyPairSeg, privKeySeg);
privKeySeg.fill((byte) 0x00);
zeroIfTemp(privKeySeg);
assert(return_val == 1);
// TODO: Parse keyPairSeg into standard SecpKeyPairImpl
SecpKeyPair keyPair = toKeyPair(ta, keyPairSeg);
Expand Down Expand Up @@ -346,9 +346,9 @@ public SecpResult<EcdsaSignature> ecdsaSign(byte[] msg_hash_data, SecpPrivKey pr
MemorySegment msg_hash = ta.allocateFrom(JAVA_BYTE, msg_hash_data);
MemorySegment sig = secp256k1_ecdsa_signature.allocate(ta); // internal signature format
MemorySegment serSigSeg = secp256k1_ecdsa_signature.allocate(ta); // serialized signature format
MemorySegment privKeySeg = ta.allocateFrom(JAVA_BYTE, privKey.getEncoded());
MemorySegment privKeySeg = privKeySeg(ta, privKey);
int return_val = secp256k1_h.secp256k1_ecdsa_sign(ctx, sig, msg_hash, privKeySeg, NULL, NULL);
privKeySeg.fill((byte) 0x00);
zeroIfTemp(privKeySeg);
secp256k1_h.secp256k1_ecdsa_signature_serialize_compact(ctx, serSigSeg, sig);
return SecpResult.checked(return_val, () -> new EcdsaSignatureImpl(serSigSeg.toArray(JAVA_BYTE)));
}
Expand All @@ -363,7 +363,7 @@ public SecpResult<EcdsaSignature> ecdsaSignLowR(byte[] msg_hash_data, SecpPrivKe
checkArg(msg_hash_data.length == 32, "Message must be 32-byte (hash)");
try (Arena ta = Arena.ofConfined()) {
MemorySegment msg_hash = ta.allocateFrom(JAVA_BYTE, msg_hash_data);
MemorySegment privKeySeg = ta.allocateFrom(JAVA_BYTE, privKey.getEncoded());
MemorySegment privKeySeg = privKeySeg(ta, privKey);
MemorySegment sig = secp256k1_ecdsa_signature.allocate(ta); // internal signature format
MemorySegment serSigSeg = secp256k1_ecdsa_signature.allocate(ta); // serialized signature format
MemorySegment nonce = null;
Expand All @@ -383,7 +383,7 @@ public SecpResult<EcdsaSignature> ecdsaSignLowR(byte[] msg_hash_data, SecpPrivKe
count++;
secp256k1_h.secp256k1_ecdsa_signature_serialize_compact(ctx, serSigSeg, sig);
} while (return_val == OK && !hasLowR(serSigSeg)); // Retry until we get an error or low-R
privKeySeg.fill((byte) 0x00);
zeroIfTemp(privKeySeg);
return SecpResult.checked(return_val, () -> new EcdsaSignatureImpl(serSigSeg.toArray(JAVA_BYTE)));
}
}
Expand Down Expand Up @@ -479,6 +479,22 @@ private SchnorrSignature schnorrSigSign32(SegmentAllocator alloc, byte[] message
return new SchnorrSignatureImpl(sig.toArray(JAVA_BYTE));
}


/// Get a segment with a private key in it. If `privKey` is an [SecpPrivKeySegment] we return
/// the read-only segment provided by [SecpPrivKeySegment#segment()], otherwise we allocate
/// a temporary segment, that should be zero filled after use by [Secp256k1Foreign#zeroIfTemp(MemorySegment)]
/// @param alloc allocator (arena) to use if we need to allocate a temporary segment
/// @param privKey a private key
/// @return private key segment (writable if temporary copy)
private MemorySegment privKeySeg(SegmentAllocator alloc, SecpPrivKey privKey) {
if (privKey instanceof SecpPrivKeySegment privKeyFfm) {
return privKeyFfm.segment();
} else {
// TODO: zero temporary array copy
return alloc.allocateFrom(JAVA_BYTE, privKey.getEncoded());
}
}

/// Create a `secp256k1_keypair` segment from a [SecpPrivKey]
/// @param alloc allocator to create segments with
/// @param privKey private key
Expand Down Expand Up @@ -529,10 +545,10 @@ public SecpResult<EcdhSharedSecret> ecdh(SecpPubKey pubKey, SecpPrivKey privKey)
SecpResult<MemorySegment> parsedPubKey = pubKeyParse(ta, pubKey);
if (parsedPubKey instanceof SecpResult.Err<MemorySegment> err) return SecpResult.err(err.code());
MemorySegment pubKeySeg = parsedPubKey.get(); // Get pubkey in 64-byte internal format
MemorySegment privKeySeg = ta.allocateFrom(JAVA_BYTE, privKey.getEncoded());
MemorySegment privKeySeg = privKeySeg(ta, privKey);
MemorySegment output = ta.allocate(32);
int success = secp256k1_h.secp256k1_ecdh(ctx, output, pubKeySeg, privKeySeg, NULL, NULL);
privKeySeg.fill((byte) 0x00);
zeroIfTemp(privKeySeg);
return SecpResult.checked(success, () -> new EcdhSharedSecretImpl(output.toArray(JAVA_BYTE)));
}
}
Expand All @@ -542,6 +558,15 @@ public String toString() {
return "Secp256k1/" + ProviderId.LIBSECP256K1_FFM;
}

/// Fill temporary segments with zeros. In this context, non-temporary segments are read-only

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this comment can be a little more clear. Perhaps it should be stated that generally non-temporary segments are from the Secp*Segment objects and temporary segments are all others.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Maybe the method should be named zeroIfWritable? It's a low-level helper method and it doesn't really know what is temporary or not -- it just zeros MemorySegments that are writable. We could then document how it used elsewhere.

/// so they shouldn't (and can't) be erased.
/// @param segment a (private key) that is writable if temporary
private void zeroIfTemp(MemorySegment segment) {
if (!segment.isReadOnly()) {
segment.fill((byte) 0x00);
}
}

/// @param allocator allocator to create segment with
/// @param size size in bytes of random data
/// @return A newly-allocated memory segment full of random data
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* Copyright 2023-2026 secp256k1-jdk Developers.
*
* 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.
*/
package org.bitcoinj.secp.ffm;

import org.bitcoinj.secp.SecpPrivKey;
import org.bitcoinj.secp.internal.ByteArray;

import java.io.IOException;
import java.io.NotSerializableException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serial;
import java.lang.foreign.Arena;
import java.lang.foreign.MemorySegment;
import java.math.BigInteger;
import java.util.Arrays;

import static java.lang.foreign.ValueLayout.JAVA_BYTE;

/// Native implementation of SecpPrivKey
class SecpPrivKeySegment implements SecpPrivKey {
private static final int KEY_LENGTH = 32;

private volatile boolean destroyed = false;
private final MemorySegment segment;

SecpPrivKeySegment(MemorySegment privKeySeg) {
var segment = Arena.ofAuto().allocate(KEY_LENGTH);
MemorySegment.copy(privKeySeg, 0, segment, 0, KEY_LENGTH);
this.segment = segment;
}

SecpPrivKeySegment(byte[] privKeyBytes) {
this(MemorySegment.ofArray(privKeyBytes));
}

@Override
public byte[] getEncoded() {
if (destroyed) throwKeyDestroyed();
return segment.toArray(JAVA_BYTE);
}

@Override
public BigInteger getS() {
if (destroyed) throwKeyDestroyed();
byte[] bytes = getEncoded();
try {
return ByteArray.toInteger(bytes);
} finally {
Arrays.fill(bytes, (byte) 0);
}
}

@Override
public void destroy() {
// TODO: Make sure the zeroing is not optimized out by the compiler or JIT

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Here is the C equivalent.

I believe this may work:

private static final VarHandle BYTE_VH = ValueLayout.JAVA_BYTE.varHandle();
public void destroy() {
    if (destroyed) return;
    for (long i = 0, n = segment.byteSize(); i < n; i++) {
        BYTE_VH.setVolatile(segment, i, (byte) 0);
    }
    destroyed = true;
}

This basically allows for volatile writes to off-heap memory, which shouldn't be optimized away.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's in interesting idea. I think we should find out whether the JDK/FFM can optimize out a call to fill() and if we need to do what you are suggesting, we'll do it.

if (!destroyed) {
segment.fill((byte) 0);
destroyed = true;
}
}

@Override
public boolean isDestroyed() {
return destroyed;
}

MemorySegment segment() {
if (destroyed) throwKeyDestroyed();
return segment.asReadOnly();
}

private void throwKeyDestroyed() {
throw new IllegalStateException("Private Key has been destroyed");
}

@Serial
private void writeObject(ObjectOutputStream out) throws IOException {
throw new NotSerializableException("Serialization of private keys is prohibited.");
}

@Serial
private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
throw new NotSerializableException("Deserialization of private keys is prohibited.");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* Copyright 2023-2026 secp256k1-jdk Developers.
*
* 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.
*/
package org.bitcoinj.secp.ffm;

import org.bitcoinj.secp.Secp256k1;
import org.bitcoinj.secp.SecpPrivKey;
import org.bitcoinj.secp.SecpPubKey;
import org.bitcoinj.secp.internal.SecpScalarImpl;
import org.junit.jupiter.api.Test;

import java.math.BigInteger;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

/// Unit tests of [SecpPrivKeySegment]
public class SecpPrivKeySegmentTest {

@Test
void constructOne() {
var oneInt = BigInteger.ONE;
var oneBytes = SecpScalarImpl.integerTo32Bytes(oneInt);

var onePrivKey = new SecpPrivKeySegment(oneBytes);
assertEquals(oneInt, onePrivKey.getS());

var onePrivKeyClone = new SecpPrivKeySegment(onePrivKey.segment());
assertEquals(oneInt, onePrivKeyClone.getS());
}

@Test
void constructAndDestroy() {
try (Secp256k1Foreign secp = new Secp256k1Foreign()) {
SecpPrivKey privKey = secp.ecPrivKeyCreate();
assertFalse(privKey.isDestroyed());
privKey.destroy();
assertTrue(privKey.isDestroyed());
}
}

@Test
void multiplyTestOne() {
try (Secp256k1Foreign secp = new Secp256k1Foreign()) {
SecpPrivKey onePrivKey = new SecpPrivKeySegment(SecpScalarImpl.integerTo32Bytes(BigInteger.ONE));
SecpPubKey pubKey = secp.ecPubKeyCreate(onePrivKey);
SecpPubKey check = secp.ecPubKeyTweakMul(Secp256k1.G, onePrivKey.getS());
assertEquals(pubKey.x(), check.x());
assertEquals(pubKey.y(), check.y());
}
}

@Test
void multiplyTest() {
try (Secp256k1Foreign secp = new Secp256k1Foreign()) {
SecpPrivKey privKey = secp.ecPrivKeyCreate();
SecpPubKey pubKey = secp.ecPubKeyCreate(privKey);
SecpPubKey check = secp.ecPubKeyTweakMul(Secp256k1.G, privKey.getS());
assertEquals(pubKey.x(), check.x());
assertEquals(pubKey.y(), check.y());
}
}

}