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

Signing broke by #8507 and arduino-builder update #8659

Closed
earlephilhower opened this issue Aug 21, 2022 · 5 comments
Closed

Signing broke by #8507 and arduino-builder update #8659

earlephilhower opened this issue Aug 21, 2022 · 5 comments

Comments

@earlephilhower
Copy link
Collaborator

See discussion in earlephilhower/arduino-pico#783

arduino-builder no longer includes {build.path} in the includes, so the generated Updater_Signing.h is never used.

#8507 adjusts _size here but it should be binSize:

int binSize = _size;
if (expectedSigLen > 0) {
_size -= (sigLen + sizeof(uint32_t) /* The siglen word */);
}

That means the signature will be calculated over the entire blob, including the signature itself, causing failure.

See the prior line:

int binSize = _size - sigLen - sizeof(uint32_t) /* The siglen word */;

@mcspr
Copy link
Collaborator

mcspr commented Aug 21, 2022

#8545 ?

@earlephilhower
Copy link
Collaborator Author

earlephilhower commented Aug 21, 2022

-edited-

Lots of stuff happening in that PR other than the incorrect signature calc. I'll need some time to grok exactly what's going on in it.

FWIW undoing the 1-liner change above seems to be enough to unbreak checks in the common case. -edit- I see one of the changes fixes this problem, great

The platform.txt changes are also needed or the generated signing file will be ignored, it seems (with current Arduino cores....earlier revs worked but operated differently.)

@mcspr
Copy link
Collaborator

mcspr commented Aug 21, 2022

No rush, PR stayed there a long time anyway :>
The idea is to also update the structure on updater touching on these vars; one line change fixes specific stuff, but not the underlying issue of type handling.

(also I don't really remember what I did there, but I do remember that I definitely tested it)

@mcspr
Copy link
Collaborator

mcspr commented Aug 21, 2022

The platform.txt changes are also needed or the generated signing file will be ignored, it seems (with current Arduino cores....earlier revs worked but operated differently.)

I think it is already handled? Unless I misunderstood the issue with the builder / arduino-cli in IDE.

compiler.cpreprocessor.flags=-D__ets__ -DICACHE_FLASH -U__STRICT_ANSI__ -D_GNU_SOURCE -DESP8266 {build.opt.flags} "-I{compiler.sdk.path}/include" "-I{compiler.sdk.path}/{build.lwip_include}" "-I{compiler.libc.path}/include" "-I{build.path}/core"

recipe.cpp.o.pattern="{compiler.path}{compiler.cpp.cmd}" {compiler.cpreprocessor.flags} {compiler.cpp.flags} -D{build.sdk}=1 -DF_CPU={build.f_cpu} {build.lwip_flags} {build.debug_port} {build.debug_level} -DARDUINO={runtime.ide.version} -DARDUINO_{build.board} -DARDUINO_ARCH_{build.arch} -DARDUINO_BOARD="{build.board}" -DARDUINO_BOARD_ID="{_id}" {build.led} {build.flash_flags} {compiler.cpp.extra_flags} {build.extra_flags} {includes} "{source_file}" -o "{object_file}"

BTW Is the issue with the orig java IDE or the new one? (or, if that matters at all)

@earlephilhower
Copy link
Collaborator Author

Yeah, that's the same thing we ended up on. So, no worries here then once #8545 gets in! Will close this, too.

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

No branches or pull requests

2 participants