The most important thing when go-fuzzing

The most important thing to know, when you are using go-fuzz, is that the cover metric should be increasing.

I didn’t know that and I wasted one 12 hour run of fuzzing because my fuzzing function was misbehaving in a way that made it return the same useless error for every input no matter what. That meant that no matter what go-fuzz mutated in the input, it could not find a way to explore more code, and could not find any interesting bugs. It was trying to tell me this by not incrementing the cover metric it was reporting.

Do not do like I did. Watch that cover is going up before leaving your go-fuzz to go spend hours and hours wasting time.

Learning Swift, sans Xcode

Say you are learning Swift. And like a good fanboi, the first thing you do is update to the latest and greatest because that’s like what you do when you are a nerd.

But you live in Osh, Kyrgyzstan. You have bitchin’ FTTH from Unilink, but access outside of Kyrgyzstan is still limited by the great firewall that Putin has put up in Moscow or whatever. I don’t know, but it’s slow as hell.

So you want to learn Swift, but Xcode is out of commission because it is upgrading. Well, sort of out of commission. It gives an “I’m upgrading” message, but it is still there in /Applications/Xcode.app.

So you can use this script to call Swift as an interpreter, and then you can learn Swift in Emacs, where you should be programming anyway, YOU FOOL.

#!/bin/sh

xc="/Applications/Xcode.app/Contents/"

$xc/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swift \
 -frontend \
 -interpret $1 \
 -target x86_64-apple-darwin15.2.0 \
 -enable-objc-interop \
 -sdk $xc/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk \
 -module-name `basename $1 .m`
rc=$?

echo
echo "Exit code: $rc"

HTTP/2: Thanks Cloudflare and Go!

Look what happened today:

2015/12/04 11:38:07 fetching https://nella.org
2015/12/04 11:38:08 {200 OK 200 HTTP/2.0 2 0 map[Server:[cloudflare-nginx] Date:[Fri, 04 Dec 2015 05:38:08 GMT] Content-Type:[text/html] Set-Cookie:[__cfduid=d3a3ea49ee46eb6a6803e2eb7f597e26e1449207488; expires=Sat, 03-Dec-16 05:38:08 GMT; path=/; domain=.nella.org; HttpOnly] Vary:[Accept-Encoding] Cf-Ray:[24f529d18893372c-ARN]] 0xc8203bbf60 -1 [] false map[] 0xc8200be000 0xc8206cc420}

Thank you Go 1.6 and Cloudflare. You guys are bringing my website into the bright future of 2016 with no help at all from me. 🙂

Industrial-scale power storage and waste heat

There will, eventually, be a giant wind farm above my house. I say eventually because though Switzerland is not immune from NIMBYism, our court system deals efficiently enough with oppositions so that if something is allowed by law (zoning laws, eco-protection laws, etc) then it does go through. The opposition (and there’s always opposition) does a few court challenges, it goes up a couple layers, sometimes to the supreme court, and the court rather quickly says, “It’s legal, shut up. If you don’t like it, change the laws, don’t come begging us to do so.”

The opposition have two complaints. They choose which one to talk about depending on the context. If they are going for “shock and awe” they use Photoshopped pictures to show how “ugly” the windmills will be. I’m suspicious of their Photoshopping, because though I think the relative sizes are correct, I don’t think the visibility (i.e. brightness of the windmills themselves and the reduction in visibility due to natural haze) in their pictures is correct. Whatever, it’s true, they are giant industrial installations in areas previously used only for grazing and milking cows. But we should recall that raising and milking cows it itself a giant industrial operation with a twice-daily milk run with a diesel-powered truck through this scenic wonderland. Some of the milking barns even run off of polluting diesel generators…

If you tell them, “I don’t mind the windmills, they look like progress to me” (and I have!), then the opposition falls back onto their second line of defense. They say, “windmills do not produce energy when it is needed, so they can’t replace nuclear”.

So, first, that’s a straw-man. No one is talking about nuclear here; if we were we wouldn’t agree anyway because I’m pro-nuclear. I consider nuclear power to be green energy (and the founder of Greenpeace does as well). What I’m interested in is eliminating fossil fuels from electrical generation, and from transport use.

The Tesla battery technology for utilities is the missing part of the equation. They shift energy from the peak generation time to the time when the energy is needed, making it possible for windmills and solar to contribute to the baseline load that existing dirty electric plants provide. But they have so far been tested in giant, ugly, industrial installations, which even I would not like to see here in my backyard.

So that got me thinking about how the Mollendruz windmills could be hooked up to batteries.

Batteries heat up when they are charged, and part of what’s special about Tesla’s innovations is to integrate cooling and fire protection into the heart of their batter packs. So a utility-scale battery installation will create utility-scale waste heat. In the current utility scale batteries, this is appears to be dumped into the atmosphere. But remote heating is a mature and well respected technology in Switzerland. Wouldn’t it be interesting to put that waste heat to use heating our schools and government buildings?

I don’t know how near batteries need to be to windmills to be efficient. Windmills generate alternating current, because they are a rotative power source. And AC travels at lower loss than DC. So it seems that putting the batteries where the heat is needed would be ok.

But the batteries are still ugly. What can we do about that? If you are harvesting the heat from them, and they are already engineered to be installed into moving cars and houses, the utility scale batteries can probably be installed indoors. In Switzerland, when we need to put things indoors and we want the land to remain pretty, we put them underground. Near the Col du Mollendruz there’s an old military fort called Petra Felix. I wonder if there’s enough room inside of it to hold the batteries?

Hacking cars and fixing them

A few years ago, I read an academic paper on how to hack cars. Today news came out that what was previously demonstrated via direct access is also possible over the air.

I thought it would be fun to look at the firmware update file that fixes this, to see what format it is in, what’s in it, etc. To get an update for 2014 Jeep Cherokees, you need a VIN. It turns out a used car sales website posted the VINs of their inventory on their website, so I found one: 1C4PJMDB6EW255433

Then you put it into the UConnect website, which is a typical late 2000’s travesty of over-engineering. It wants you to use some plugin from Akami to download the file, but in small print tells you that you can also click on this link. But of course, there’s javascript insanity to prevent you from finding out what the link is. It is delivered via TLS, which is interesting. It is a 456 meg zip file. It also has a user-specific token on the end of it, and without that you get a 404 when you try to fetch it.

The zip file has an ISO inside of it:

$ unzip -l MY13_MY14_RA4_15_26_1.zip 
Archive:  MY13_MY14_RA4_15_26_1.zip
  Length     Date   Time    Name
 --------    ----   ----    ----
583661568  06-23-15 14:48   swdl.iso
 --------                   -------
583661568                   1 file

The ISO file is slightly bigger than the zip file, at 583 megs:

$ ls -l swdl.iso 
-rw-r--r--  1 jra  staff  583661568 Jun 23 14:48 swdl.iso

Inside the ISO file is:

dr-xr-xr-x  2 jra  staff  2048 Jun 23 16:47 bin
dr-xr-xr-x  2 jra  staff  2048 Jun 23 16:47 etc
dr-xr-xr-x  2 jra  staff  2048 Jun 23 16:47 lib
-r-xr-xr-x  1 jra  staff  1716 Jun 23 16:47 manifest
dr-xr-xr-x  4 jra  staff  2048 Jun 23 16:47 usr

And that manifest file? It is Lua, which is apparently read into the updater via execution.

So right. The updater itself apparently gives an attacker execute privs in the address space of the Lua interpreter via an unsigned file.

Jeeze, Chrysler, that’s like Game Set and Match, and I haven’t even looked into bin/ yet. WTF?

Update after reading some more…

Well something interesting happens in ioschk.lua, where the second block of 64 bytes from the ISO is read and then fed to “openssl rsautl”, using a public key that is on the device. But ioschk.lua is loaded from the ISO itself, and is called by install.sh, from the ISO. So it seems like if you want to make your own ISO, you need to remember to make install.sh’s call to isochk.lua a no-op.

Other interesting things I found while trolling around… they have the Helvetica Neue font, and right next to it a license file saying, “for evaluation only”. Jeeze, sure hope that Harman have paid up, or else they might have a bill in the mail.

There’s a file called cisco.sh which does the necessary to put the device on the Ethernet if a Linksys USB300M adapter is plugged in. It has some checks in it for an internal development mode, but those would be easy to bypass if you can in fact edit the ISO image at will.

So, all in all, it would be fun to play if I had a Jeep. But I’m still planning on getting a Tesla.

Doing it the hard way

In my last post I offered to point out some things in Golang Challenge #2 submissions that struck me as “worthy of receiving a (polite) rebuke in code review”, otherwise known as WTFs.

This is opt-in abuse. I don’t mind abusing my colleagues, when I know I can take them out for lunch later and buy them a beer. Hassling random Golang Challenge entrants is not my style. But some have decided they are up for it, even if I’m remote and can’t buy them a beer afterwards.

io.Toys

First up, is Jason Clark. Jason’s entry is quite good, from an idiomatic Go point of view. He fell prey to the “under-testing” problem I mentioned, in that his entry does not handle message framing, or what happens if the buffer passed to Read is smaller than the incoming message. But what I’m really looking for for the purposes of this post are the WTF’s. And seeing io.TeeReader tipped me off that something interesting was about to happen:

// Write encrypts the contents of p and writes it to the underlying io.Writer.
func (w *SecureWriter) Write(p []byte) (int, error) {
	// Each message starts with a generated nonce. Only write the nonce once.
	if w.nonce == nil {
		var nonce [24]byte
		if _, err := io.ReadFull(io.TeeReader(rand.Reader, w.w), nonce[:]); err != nil {
			return 0, ErrNonceWrite
		}
		w.nonce = &nonce
	}

	// encrypted and send message
	_, err := w.w.Write(box.SealAfterPrecomputation(nil, p, w.nonce, w.key))
	if err != nil {
		return 0, err
	}
	return len(p), nil
}

So, the first problem here is that this is just wrong. The docs for NaCl are clear that security is compromised if you use the same nonce for two different messages. But second, the use of io.TeeReader here is overly clever. I didn’t even know what io.TeeReader did, but the docs are clear enough: “TeeReader returns a Reader that writes to w what it reads from r”. So what that line does is get the nonce from cyrpto.Rand, and send the nonce down the unencrypted channel (correct!) while also sending it into nonce, to be saved for later as w.nonce (not correct!). To reason about that line as a reader you need to keep two unrelated processes in mind at once (saving the nonce for later, and sending it down now), and then ask yourself if the error handling correct in both of those cases.

So, if ErrNonceWrite is returned, was a nonce sent down the connection anyway? What was it?

The simpler way of doing this is in two steps. First read into a local called nonce, and check the errors on reading. Once you are sure you have a good nonce, send it down with w.Write(). Checking errors correctly here is critical, because if you don’t manage to get a new nonce, you must not send the message, since you might be sending it with a nonce of all zeros. The second time you’ve done this, you’ve compromised your security.

There’s nothing wrong with using the nifty toys in the io package. And in fact, later in his submission, Jason uses io.MultiWriter in an interesting way. I can’t say that I’m personally 100% confident in the technique of using io.Copy to implement an echo server. I understand the theory, but it seems to me you become dependent on the right magic happening inside of io.Copy, instead of making sure that the logic of your program is front and center, where your reader can see it.

Always remember, code is for reading. When you work on a big system, with lots of coworkers, they will read and re-read your code for months and years after you’ve written it. Code is written once and read a thousand times. Make sure what’s there to read explains what you are doing and what you are depending on.

And now for a random global problem…

Trevor Arjeski got in touch and asked if he had any WTFs. By chance, the first file I clicked on was secure_io.go, and within the first few lines I knew, “we have a winner!”

In a crypto system, the quality of your random numbers matters. This has been known by professional cryptographers for decades, and to the rest of us since 1996. So when I saw that Trevor was importing “math/rand” instead of “crypto/rand”, I knew that he had a big problem coming.

Worse, I read down a few lines and saw the global “seeded”. The problem with this global is that access to it is not protected from a data race. There are three options for how to fix this:

  • Nuke it: This problem calls for using the “crypto/rand” package, which does not need seeding by the Go programmer.
  • Move it to init: Lazy initialization, controlled by a global flag can sometimes be more simply moved to explicit initialization, before main starts running. Go guarantees that init functions are run in a single-threaded context (the spec says: “Package initialization—variable initialization and the invocation of init functions—happens in a single goroutine, sequentially, one package at a time.”)
  • Use sync.Once to arrange for something to be lazily initialized once and only once.

I’ve said before that good testing matters. But to catch this data race, Trevor should have written a test that concurrently created multiple SecureWriters and written into them. Then he would have had to run “go test -race”. It is wonderful that Go has the race detector, but it is better to never write data races to begin with. And a good way to not write them is to not allocate any shared data.

Get in the habit of asking hard questions about every global you see: when it is initialized? Are there writes to it? How are they sequenced with respect to reads?

Want some practice? Move on to lines 14 and 15… 🙂

Addicted to encoding/binary?

Kristoffer Berdal got in touch and asked me to take a look. Kristoffer made a choice for readability that would eventually be a performance problem, if he wanted to benchmark and tune his implementation.

In his Writer we see a nice symmetrical list of calls to binary.Write. Reading through it, it makes the exact format of the protocol completely clear. So score points for self-documenting code!

But there’s a cost: check out the signature of binary.Write: func Write(w io.Writer, order ByteOrder, data interface{}) error

That means that each time we call into binary.Write, we’ll be doing a type conversion from the fundamental type of the third argument into an interface{}. This may involve an allocation, and once control gets inside of binary.Write, it will end up in the “fall back onto reflection” code path, because the nonce and (usually) the message are not less than 8 bytes long. Reflection is powerful and magical, and a strong point of Go. But it is not fast, when the alternative is not using reflection.

A better choice for writing the nonce and the message, which are simple []byte types, would have been to use the Write method of the underlying io.Writer. (Also, this saves the reader of puzzling over what the meaning of the little endian representation of a [24]byte is…)

One thing that’s interesting about Go is that it can be highly friendly and easy like Python, or highly precise and speedy like C. Choosing when to do one or the other (or finding a way to do both) is the trick that requires experience. It is always important to first get it right, then get it clear, then measure, and only if measurement says to do so, make it fast. In a case like this, Kristoffer has written it right, and clearly. But by turning to a function that uses interface{} where there was another option, he left some performance on the table. Worse, he might have left behind a trap that a colleague will need to deal with later when the prototype goes to production and starts handling gigbits of data per second.

Thanks

Thanks to those who gave me permission to pick on them. I hope that they’ve enjoyed the 15 minutes of fame, and that we’ve all learned something useful.

Golang Challenge 2 comments

I’ve just finished evaluating 40 of the 105 entries to the Golang Challenge #2. The organizer, Satish, asked me to write up my thoughts.

The main similarity I noticed in the entries was not enough testing. The vast majority of the entries used the tests provided in with the challenge unmodified. Taking the given tests without thinking critically about them lead people to make a number of critical mistakes, over and over again. The majority of the entries I graded passed the tests, but would not have stood up to production use. Of the 40 I graded, only 2 or 3 would have received a “ship it” from me in my day job. Those were (not by chance) the ones with extra tests beyond the ones provided in the challenge.

A key part of growing into a mature programmer is to understand that the problem as it arrives in your “todo basket” is rarely correctly specified. This challenge is no exception. The challenge asked participants to “make the given tests pass”, but didn’t mention if you could or should write extra tests. The bottom line is that you can always write extra tests, and by doing so, you will discover where the spec is weak, or wrong. You will also decide for yourself where the “edge” of the task is; thinking about tests you can and should write will help fix the size of the job at hand.

What I’m describing here is test-driven development. Go’s tools make it easy to stay on the golden path of TDD, you just need to decide that you believe in its benefits, and then teach yourself the discipline to do it. In TDD, the tests push the implementation away from the hacky thing that seems to work towards the robust thing that actually does work, that lives alongside of proof that it works (the tests) and that will keep working even as the world changes around it (because you’ll fix the code when the tests start failing).

Two deficiencies in the given tests were that they did not test legitimate network behaviors like short reads, and they gave the impression that the protocol should be one-shot (i.e. set up a TCP connection, exchange public keys, echo one message and close). Given that the problem statement asked for an io.Reader and and io.Writer that encrypted data, it stands to reason that a good solution would be able to handle more than one message, since io.Reader/io.Writer are not one-shot interfaces.

A common problem in TCP servers is short reads. Just because the sender put 1000 bytes into the TCP stream via one write system call does not mean you are guaranteed to receive those same 1000 bytes in a matching read call. To get that behavior, you’d need a reliable transaction protocol like SCTP. With TCP, all you are guaranteed is that the same bytes will arrive at the far end, in the same order.

To see where a short read can come from, we need to look at it one packet at a time. Imagine what happens if the sender gives the kernel 2000 bytes, then sleeps on a the read of a reply. If the MTU on the link is 1500 bytes, the TCP implementation might send out one packet with about 1500 bytes in it, and a second with 500 (the “about” is hand waving for the TCP/IP overhead we’re ignoring here). Now imagine that the second packet is lost, and the replacement is lost as well. On the third try it gets through. On the receiving end, what we see is 1000 bytes which arrive, followed by a pause and 500 more. If the pause is long enough, the kernel will decide to stop making the application wait around for the first 1000 bytes and pass them up. The reader, blocked in the recv() system call, will get unblocked and receive the 1000 bytes. The next time it blocks on recv(), it will get the enxt 500 bytes. This is one of many explanations for how you can get a short read. There’s no guarantees, and code that expects there to be is wrong and will fail in real life (but not in your fast, non-lossy, testing environment; in particular, if you test via localhost, you’re very unlikely to ever see short reads).

But short reads could some from someplace much simpler… from the tests. Consider how your challenge entry would have behaved if it was reading from an io.Reader that intentionally returned only one byte at a time.

Given the reality of short reads, it is up to the application to handle framing the bytes into messages. The majority of entries did not take this into consideration. The solution is that each message needs to be sent with it’s length. And when SecureReader reads from the underlying io.Reader, it needs to use something like ioutil.ReadAll to make certain all the bytes arrive, no matter how many calls to the underlying io.Reader it takes to get them all. But then that opens up the question of what to do when the connection hangs? Do you timeout? How can you cause ioutil.ReadAll to return early?

In Go, different pieces of the language and the standard library can be composed to make something better than any of them alone. A great way to read frames from the network is to prepend the frame with it’s length. You read the length using encoding/binary.Read, make an io.LimitedReader which is limited to the length, then give that LimitedReader to ioutil.ReadAll. You can handle timeouts by checking that if the underlying io.Reader satisfies an interface with SetReadDeadline in it (as the various connection types in package net do), and then using it to set a deadline. After the deadline passes, the underlying io.Reader will return a timeout error, which the LimitedReader will return to ioutil.ReadAll, which will cause it to return early, telling you that the read timed out.

The other thing I noticed is that even in a language designed for simplicity, there are a myriad of ways to do things too difficultly in Go. In other words, go is not WTF-proof. *

The only solution to this problem is that you need to read lots and lots of code (way, way more than your write!). Read the highest quality code you can find. Code from the Go core team is a very good choice (the standard library itself and side projects by Andrew, Russ, and Brad). You can also read the reviews of potential new code for the Go core (read the golang-dev mailing list to find out where the reviews are). By watching other people get their screw ups corrected, you can avoid making them in the first place! It is much easier on the ego too, to watch other people get corrected for a mistake you would have made. 🙂

* In order to not single people out here in public I’m not going to talk about the specific WTF’s I saw. But if you’d be willing to have your code publicly mocked by me, drop me a line with a pointer to your challenge solution. If there’s a WTF in there, I’ll post about it and you’ll be infamous, and we can all have a good laugh… 🙂