Apple의 SSL 긴급 update 로 알려진 goto 를 사용하는 나쁜 예.

 불과 몇일 전 Apple 은 긴급히 iOS 7.0.5 에서 7.0.6 은 물론, iOS 6.1.5 와 6.1.4 를 사용하던 기기들 에 대해 SSL 버그를 수정한 update 를 수행 하였습니다. 이는 iPhone3Gs 부터 iPhone5s 까지 모든 기기에 해당 하는 update 였습니다만, 이 문제는 다름아닌 goto 를 사용하는 나쁜 습관을 가진 프로그래머로 부터 발생된 문제라 할 수 있겠습니다.

 Apple 쪽에서 사용되었던 SSL code 를 보면 다음과 같습니다. (공개된 부분에 한해)

static OSStatus
SSLVerifySignedServerKeyExchange(SSLContext *ctx, bool isRsa, SSLBuffer signedParams, 
                                 uint8_t *signature, UInt16 signatureLen )
{
	OSStatus		err;
	
	....
	
	if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
		goto fail;
	if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
		goto fail;
		goto fail;
	if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
		goto fail;
	
	....
	
fail:
	SSLFreeBuffer(&signedHashes);
	SSLFreeBuffer(&hashCtx);
	
	return err;
}

 C나 C++ 을 쓰면서 가장 피해야 하는 부분이 "있으면서도 쓰지 말아야 할 goto" 의 사용이라 하겠습니다.

위 코드를 보면 아마 Version control 을 위해 merge 를 하면서 발생한 문제인지 모르겠습니다만, 같은 문장 2개에 goto fail; 이 무려 2개가 겹쳐 있습니다. 같은 조건문은 여러번 쓰여도 무시될 수 있는 버그다 할 수 있겠지만 조건문 다음의 goto fail; 이 두번있게 된다면 무조건 저기에서 fail 로 처리 된다는 말이 되어 버립니다.

 제대로 개념이 서 있는 개발자 였다면 위 코드중 goto fail 대신에 반복적이고 귀찮더라도 SSLFreeBuffer() 를 해 주고 return err 로 조건 문 다음에서 벗어나는 코드를 사용했어야 합니다.

 나름 최적화의 대표주자로 생각했던 Apple 의 이런 작지만 너무 큰 실수에 대해서는 앞으로 벌어져서도 안될 것이며, 이런 식의 엄청난 재앙을 불러 오는 형태로 C/C++ 프로그래밍을 하는 일은 없어야 겠습니다.

 하나 예를 들어 보면 다음 코드가 가진 문제점을 찾아 보도록 하겠습니다.

int checkCypherKey( const char* ref )
{
	int   retID = 0;

	if ( ref == NULL )
		goto fail;
		
	char* tmpKey = new char[ CYPHER_KEY_MAX_LEN ];
	
	CypherTool.GetDefaultKey( tmpKey );
	
	if ( strcmp( tmpKey, ref ) == 0 )
	{
		retID = 1;
		goto success;
	}
	
	
fail:
success:
	delete[] tmpKey;
	
	return retID;
}

 여러 회사를 다니다 보면 위와 비슷한 형태의 코드를 볼 일이 있습니다. C++ 로 뭔가를 만들었지만 내부에 goto 를 남용하고 있으며, ref 인자를 검사하는 부분에서 goto 를 사용함으로서 잡혀 있지 않은 tmpKey 를 delete 하려 하므로서 오류를 발생 시킵니다.

 만약 제가 프로그래밍 하는 기준이라면 위의 코드는 아래처럼 최소한의 안전성을 가진 형태로 변경 되어야 합니다.

int checkCypherKey( const char* ref )
{
	if ( ref == NULL )
		return 0;
		
	char* tmpKey = new char[ CYPHER_KEY_MAX_LEN ];
	if ( tmpKey != NULL )
	{
		CypherTool.GetDefaultKey( tmpKey );
	
		if ( strcmp( tmpKey, ref ) == 0 )
		{
			delete[] tmpKey;
			return 1;
		}
	}
	
	return 0;
}

 첫번째 코드와 그리 다르지 않은 형태 이지만 변경된 형태는 new[] 를 하였을 경우 메모리 할당이 안되었을 경우의 처리와, 정확히 메모리를 해제 하는 시점, 그리고 결과를 return 하는 시점이 보입니다. 또한 이런 코드에 goto 를 쓸 이유가전혀 없습니다. (차라리 정 써야 겠으면 continue 를 쓰는 것이 차라리 낫겠습니다)

 개발자가 조금 더 피곤하고, 좀 더 많이 생각 할 수록, 그 프로그램과 시스템의 안정성은 높아 지는 것이라 믿습니다. 만약 개발자가 지루한 개발에 지쳐 Apple 이 한 실수와 같은것을 반복 한다면 작은것 하나가 점점 더 큰 오류의 근본이 될 것입니다.

 Apple 의 SSL 에 대한 실수를 번복하는 개발자가 없었으면 좋겠습니다.