Bindings for PHP

afk11 commented at 4:56 pm on May 24, 2015: contributor

I’ve been working on an extension for PHP which wraps libsecp256k1, and think it’s just about complete. I noticed the PR’s for java bindings, and I wondered if it might be worth moving the code here?

I’m not sure if anyone else is developing these, but mainly asking since I’m experimenting with packaging the extension as .deb package, but I need to ensure that libsecp256k1 is installed on the system as well.

I know it’s quite early for a secp256k1-dev package in apt, but since I’m now considering gitian builds to produce a .deb for this extension, the same could be done for the library itself. If we keep the sources here we can build each binding/release in the one gitian build.

The bindings are here: https://github.com/Bit-Wasp/secp256k1-php, and there are tests for pretty much everything besides the privkey_import/export functions(). Yet to include the edge cases (r/s = 0, etc), but it’s standing up pretty well. It is my first foray into C in a while, so I’d appreciate all the feedback I can get!

Anyway, interested to hear what people think, I can follow up with a PR if people are happy enough!

ghost commented at 6:07 pm on May 24, 2015: none

hey @afk11 , I noticed your bindings before I got started on the java bindings, and had a discussion on IRC w/ @gmaxwell about this awhile back as well; it looks like upstream may not be 100% ready to start taking bindings because of the evolving API (eg. adding contexts just a few mo. ago) and I only figured that out after I was halfway through the implementation process.

I originally started the work to try to close out #64, and since that has (basically) been deprecated now, I was going to ask (somewhere, here) if it made more sense to just take that PR and maintain them in a fork until the API is more mature… possibly w/ a link in the Readme to direct new users, if that makes sense, too.

DavidEGrayson commented at 6:23 pm on May 24, 2015: none

Hello, I wrote some Ruby bindings for the secp256k1. In case it’s useful to you, the main features I wanted to achieve for that project were:

Provides access to all features of the secp256k1 library.
Does not add any new features.
Avoids making arbitrary decisions when possible.
Provides a safe interface: there should be no way for a user of the wrapper to cause undefined behavior or memory leaks. (One exception is when multiple threads are used in an unsafe way.)
Exception messages produced by this wrapper are less verbose than they could be to help avoid leaking secret information.

I noticed your PHP binding is creating some kind of global context instead of letting the user create their own. That only seems like a good idea to me if that global context can somehow be saved so it doesn’t have to be recreated for every HTTP request that uses it. Is that the case?

I don’t see any code of yours for checking the lengths of input strings and things like that. I suspect that, like Ruby users, PHP users are accustomed to using libraries that don’t produce segmentation faults when invalid input is passed in, so I would recommend validating the user’s inputs to prevent things like that.

gmaxwell commented at 10:07 pm on May 24, 2015: contributor

Very cool. I’m happy to see that your bindings have tests. Please be clear in your readme that secp256k1 is unreleased and experimental right now; it shouldn’t generally be used or depended on.

With respect to input validation the principle to assure is that the behavior is as unconditionally safe as possible. I’ve seen things like JS crypto libraries where if the object passed in was an array rather than a string they’d silently sign zero or something like that. Ideally it should be hard to use incorrectly, but if its used incorrectly it should fail in the safest way possible.

afk11 commented at 12:37 pm on May 25, 2015: contributor

@faizkhan00 - yeah, I can appreciate that. I had been thinking of roughly the same - package a versioned release secp256k1-dev-something after each break, and let people fall behind in releases if they dare (exposing people to updates every week or two might get old fast, but know personally I’d end up compiling against the latest for myself). @DavidEGrayson Thanks, good advise to stick to. I’ll have a look at your code today.

Input validation, I can do today. I’ll have a look at copying contexts somehow, though not sure how I’d approach that. To get them into userland I’m thinking I’ll have to wrap them in a resource. I’ll have a look over libsecp256k1 to see if I can dump that memory or something, and initialize contexts from this. Might come back to you for more details here!

I guess I should detail any other places where the bindings deviate from libsecp256k1:

A global sign and verify context is created.
Userland functions don’t include the context param, the extension takes care of that.
In fact, none of the context_* functions have been exposed yet. I ought to get working on that.
I haven’t exposed secp256k1_nonce_function_t whatsoever - the bindings don’t offer that argument to userland, and passes null forcing deterministic signatures. @gmaxwell - OK, will do! Thanks for that warning, I’ll start thinking defensively.

https://github.com/Bit-Wasp/bitcoin-php/tree/master/src/Crypto/EcAdapter

I ultimately started writing this so I’d have an alternative to pure PHP ECDSA in something else. In that library, the interface has typehinted args, takes care of serializing (why I haven’t had problems passing few characters yet) and throws exceptions if the secp256k1_* returns any of the documented error codes, so it’s usage is quite safe there. (See Secp256k1.php in the link above)

But I suppose I have ignored some things for the sake of getting it to work. I’ll spend some time on the points today and see what I can turn up!

afk11 commented at 12:44 pm on May 25, 2015: contributor

Pinging @rubensayshi who works on this also

afk11 commented at 12:52 pm on May 25, 2015: contributor

@DavidEGrayson Just checking which you mean, if the extension can save the contexts state and use it for anyone next time, or allow the user to save it?

In general, what state is kept in the contexts besides precomputed EC points? I’m wondering if guaranteeing a new one each time has any merit, or if it matters at all (aside from computation time)

afk11 cross-referenced this on May 26, 2015 from issue Add stricter arg checking by afk11

sipa commented at 0:40 am on August 28, 2015: contributor

Just to let you know that we’re working towards a stable API and tagged release soon. I’m also fine with having other language bindings included in the repository, assuming they have a minimally covering travis test integrated (and don’t do inadvisable things of course, but I’ll gladly help towards that goal).

DavidEGrayson commented at 1:57 am on August 28, 2015: none

Cool, I’m looking forward to having tagged releases.

afk11 commented at 10:41 pm on August 30, 2015: contributor

That’s great news indeed! I’ll have to tackle the Schnorr/EcDH modules soon, but otherwise mostly up to date.

fanatid cross-referenced this on Mar 6, 2016 from issue [readme] Add section Bindings by fanatid

ysangkok commented at 9:54 pm on September 13, 2019: none

I think this should be closed since bindings for all the languages can’t be put in here.

afk11 commented at 10:03 pm on September 13, 2019: contributor

It can probably be closed, I haven’t followed up on it.

But, there are some arguments for a mono-repo.. I currently tell people to use commit X from secp256k1 with version Y of my bindings. I don’t know when my code will by broken by libsecp. If the bindings were here, they could be released in lockstep.

Users of the bindings probably aren’t going to bother testing newer libsecp commits until things break again.. even though they should.

I just really want semver releases :)

tldr: please close after fixing https://github.com/bitcoin-core/secp256k1/issues/286

afk11 cross-referenced this on Jan 6, 2020 from issue Create a release by shazow

elichai commented at 1:31 pm on January 7, 2020: contributor

FWIW in the rust bindings we’re just vendoring the lib and statically linking to it https://github.com/rust-bitcoin/rust-secp256k1

afk11 commented at 1:40 pm on January 7, 2020: contributor

Hi @elichai - This issue is re adding the PHP extension for libsecp256k1 to this repo, not how the PHP extension interfaces with libsecp256k1. But I think your comments would be more visible + valuable to this thread about tagged versions for libsecp? #286 Up to you :)

To me it feels wrong to build and ship libsecp in my app:

If it has a patch release, everyone has to (1) discover this, and (2) recompile + redistribute their projects. Whereas with shared libraries mean once the bug is discovered in libsecp, and they redistribute, everyone gets the fix.

I also think the community would do well to have a gitian build process for libsecp like Bitcoin Core.. The hard work only really needs to be done once.

apoelstra commented at 9:02 pm on January 24, 2020: contributor

Things are somewhat simpler because libsecp has never distributed anything, let alone redistributed :). If there is a serious bug found in this library unfortunately it will be up to the maintainers of the various bindings to notice it (though helpfully, such a thing would probably get media attention because of its effect on Bitcoin) and update their static versions of their library.

+1 to having gitian builds though.

joemphilips cross-referenced this on Feb 28, 2020 from issue TODO: Make integration of Secp256k1.Native more secure. by joemphilips

real-or-random cross-referenced this on Mar 16, 2020 from issue Build on PHP7 fails: Cannot find config.m4 by typoworx-de

real-or-random commented at 10:43 am on January 5, 2023: contributor

tldr: please close after fixing #286

real-or-random closed this on Jan 5, 2023

Bindings for PHP #255