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

aes: Add doc.odin with an AES decryption example #3932

Closed
wants to merge 3 commits into from

Conversation

sjakk
Copy link

@sjakk sjakk commented Jul 16, 2024

No description provided.

core/crypto/aes/doc.odin Outdated Show resolved Hide resolved
core/crypto/aes/doc.odin Outdated Show resolved Hide resolved
srcdata := make([]byte, len(data))

// Decrypt data in 16-byte blocks
for i in 0 ..< len(data) / aes.BLOCK_SIZE {
Copy link
Member

Choose a reason for hiding this comment

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

I would rename srcdata to plaintext and rewrite the loop like this:

for len(data) > 0 {
   aes.decrypt_ecb(&ctx, plaintext[:aes.BLOCK_SIZE], data[:aes.BLOCK_SIZE])
   data = data[aes.BLOCK_SIZE:]
   plaintext = plaintext[aes.BLOCK_SIZE:]
}

Copy link
Author

Choose a reason for hiding this comment

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

For some reason, when I rewrite the for loop this way, it gives me no output. I'm still studying how to write more idiomatic Odin code, and I will rewrite it properly soon.

Copy link
Member

Choose a reason for hiding this comment

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

I didn't have an encrypted file to test with so I just wrote it here.

And I think I know why. :-) We're advancing the output slice here, which means it's 0 bytes at the end.
Will need to keep a backup of the original slice, too.

_plaintext := plaintext // Preserve original slice view of the decrypted data
for len(data) > 0 {
   aes.decrypt_ecb(&ctx, _plaintext[:aes.BLOCK_SIZE], data[:aes.BLOCK_SIZE])
   data = data[aes.BLOCK_SIZE:]
   _plaintext = _plaintext[aes.BLOCK_SIZE:] // Advance second view of the same destination memory
}

Copy link
Member

Choose a reason for hiding this comment

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

package foo
import "core:fmt"
import "core:crypto/aes"
import "core:os"
import "core:bytes"

BLOCK_SIZE :: aes.BLOCK_SIZE

main :: proc() {
	data, ok := os.read_entire_file("data.bin")
	if !ok {
		fmt.eprintln("Error Reading File")
		return
	}

	if len(data) % aes.BLOCK_SIZE != 0 {
		fmt.eprintln("Error: Data length is not a multiple of AES block size")
		return
	}

	key := transmute([]u8)string("aeskey")
	ctx: aes.Context_ECB
	aes.init_ecb(&ctx, key)
	// Allocate space for decrypted data
	plaintext := make([]byte, len(data))

	plain := plaintext // Preserve original slice view of the decrypted data
	for len(data) > 0 {
		aes.decrypt_ecb(&ctx, plain[:BLOCK_SIZE], data[:BLOCK_SIZE])
		data  = data[BLOCK_SIZE:]  // Advance encrypted data
		plain = plain[BLOCK_SIZE:] // Advance second view of the same destination memory
	}

	// Output decrypted data
	fmt.println(plaintext)
	os.write_entire_file("rcdecrypted.bin", plaintext)
}

Copy link
Member

Choose a reason for hiding this comment

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

Does this work?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, worked like a charm

@Kelimion
Copy link
Member

@Yawning, thoughts?

@Yawning
Copy link
Contributor

Yawning commented Jul 16, 2024

@Yawning, thoughts?

  • Examples should be self-contained (eg: Demonstrate encrypting/decrypting a static buffer) and functional (This is not, the key will cause a panic). See core/crypto/hash.doc for an example.
  • Examples shouldn't be a "this is why we can't have nice things" (There is a warning as part of the Context_ECB definition for a reason).

So tentatively NACK? The one mode that people should use is GCM, and at some point (Real Soon Now) I want to add core:crypto/aead in the spirit of core:crypto/hash and discourage importing the aes package directly...

@Yawning
Copy link
Contributor

Yawning commented Jul 16, 2024

Just so my intentions are extra clear, similar to how the documentation for "how to use the hash functions" lives in core/crypto/hash/doc.odin (and not the individual hash function implementation packages), once core/crypto/aead exists, that is where the documentation will live (as opposed to inside the aes and chacha20poly1305 packages).

So this is (IMO) kind of premature.

@sjakk
Copy link
Author

sjakk commented Jul 16, 2024

Just so my intentions are extra clear, similar to how the documentation for "how to use the hash functions" lives in core/crypto/hash/doc.odin (and not the individual hash function implementation packages), once core/crypto/aead exists, that is where the documentation will live (as opposed to inside the aes and chacha20poly1305 packages).

So this is (IMO) kind of premature.

Thank you for the feedback. I understand your point about the organization of documentation, and it makes sense to have the documentation for "how to use the hash functions" located in core/crypto/hash/doc.odin rather than in the individual hash function implementation packages. Once core/crypto/aead exists, it is logical for the documentation to reside there instead of inside the aes and chacha20poly1305 packages.

I created this example because I was participating in a CTF that required it, and I wanted to complete the entire CTF using Odin. During this process, I felt the need for some helper functions to accomplish the task, which is why I developed them. My focus was not on security but on practicality and ease of use for the specific CTF challenge. I appreciate your guidance and will keep it in mind for future developments.

@Yawning
Copy link
Contributor

Yawning commented Jul 16, 2024

Just so my intentions are extra clear, similar to how the documentation for "how to use the hash functions" lives in core/crypto/hash/doc.odin (and not the individual hash function implementation packages), once core/crypto/aead exists, that is where the documentation will live (as opposed to inside the aes and chacha20poly1305 packages).
So this is (IMO) kind of premature.

I created this example because I was participating in a CTF that required it, and I wanted to complete the entire CTF using Odin. During this process, I felt the need for some helper functions to accomplish the task, which is why I developed them. My focus was not on security but on practicality and ease of use for the specific CTF challenge. I appreciate your guidance and will keep it in mind for future developments.

No worries. I understand the need for "someone used AES-ECB to encrypt something, how do I decrypt it", and apologies if the API was cumbersome. When it comes to something like documentation for a library, I lean towards "examples should demonstrate best practices".

@Kelimion
Copy link
Member

I'll close this now. Please don't be discouraged, @sjakk. The impulse to add documentation was a good one.

@Kelimion Kelimion closed this Jul 16, 2024
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

Successfully merging this pull request may close these issues.

None yet

3 participants