Add nitf #294

riverloopsec · 2020-06-08T04:17:56Z

This PR adds the NITF (National Image Transition Format) format specification.

Please see attached ZIP that has a selected set of NITF images collected from around the internet and carefully selected to have good coverage for testing purposes -
corpora.zip. In addition, https://github.com/codice/imaging-nitf/tree/master/shared-test-resources/src/main/resources is another repository of these files.

We have conducted extensive analysis of the security of this format using this as one of our methods for exploring it.

Credit to @TACIXAT for implementation.

KOLANICH · 2020-06-10T20:30:51Z

image/nitf.ksy

+  ks-version: 0.9
+  encoding: UTF-8
+  endian: be
+  license: CC-BY-SA-4.0


Not usually used for software.

Updated to MIT.

KOLANICH · 2020-06-10T20:32:26Z

image/nitf.ksy

+        type: encrypt
+      - id: fbkgc
+        size: 3
+      - id: oname


I guess names should be more meaningful. For original names there is -orig-id. Could you also add doc to each?

Updated these for many of the items. Full descriptions in the source doc linked are long and prose, so included some shortened ones where I felt the field could use more context.

KOLANICH · 2020-06-10T20:34:45Z

image/nitf.ksy

+  clasnfo:
+    instances:
+      total_size: 
+        value: (1 + 2 + 11 + 2 + 20 + 2 + 8 + 4 + 1 + 8 + 43 + 1 + 40 + 1 + 8 + 15)


sizeof<...>

@riverloopsec @rmspeers As @KOLANICH points out, there's really no need for such a lengthy and error-prone summation. Exactly this problem is addressed by the sizeof operator (make sure you're using the latest unstable compiler version available from https://kaitai.io/#download, or you can use https://ide.kaitai.io/devel/, which has constantly the latest development version out of the box). But I noticed you already indicated ks-version: 0.9 in the top-level meta section, so this shouldn't be a problem.

I saw that you're using it somewhere like reclasnfo.total_size. You can replace this usage with either

type-based sizeof: sizeof<clasnfo>

value-based sizeof: reclasnfo._sizeof

Choose the one that suits you best. In both cases, you can delete this manually created attribute total_size, the size of the whole clasnfo type will be calculated automatically.

rmspeers · 2020-07-03T15:46:10Z

@KOLANICH can you look at this we ran into when updating? This is produced by KSV, no errors on KSC.

Compilation OK
... processing image/nitf.ksy 0
...... loading nitf.rb
Classes loaded OK, main class = Nitf
Traceback (most recent call last):
	8: from /usr/local/bin/ksv:23:in `<main>'
	7: from /usr/local/bin/ksv:23:in `load'
	6: from /Library/Ruby/Gems/2.6.0/gems/kaitai-struct-visualizer-0.7/bin/ksv:53:in `<top (required)>'
	5: from /Library/Ruby/Gems/2.6.0/gems/kaitai-struct-visualizer-0.7/lib/kaitai/struct/visualizer/visualizer.rb:13:in `run'
	4: from /Library/Ruby/Gems/2.6.0/gems/kaitai-struct-visualizer-0.7/lib/kaitai/struct/visualizer/parser.rb:32:in `load'
	3: from /var/folders/11/nn476p9172xb4__85r_ncw6h0000gn/T/d20200703-41609-v4w6iu/nitf.rb:28:in `_read'
	2: from /var/folders/11/nn476p9172xb4__85r_ncw6h0000gn/T/d20200703-41609-v4w6iu/nitf.rb:28:in `times'
	1: from /var/folders/11/nn476p9172xb4__85r_ncw6h0000gn/T/d20200703-41609-v4w6iu/nitf.rb:31:in `block in _read'
/var/folders/11/nn476p9172xb4__85r_ncw6h0000gn/T/d20200703-41609-v4w6iu/nitf.rb:229:in `_read': undefined method `total_size' for nil:NilClass (NoMethodError)

Note that I tried to switch ks-version to 0.8, which actually may be what broke this - rather than any changes to the format. Does that make sense?

KOLANICH · 2020-07-03T17:41:03Z

image/nitf.ksy

+    seq:
+      - id: des_base
+        type: data_sub_header_base
+      - id: desoflw


should these 2 fields be separated in a struct?

please don't merge words in variable names.

desoflw is one from the original data, I'll go move these to orig-id and put instead other ids.

KOLANICH · 2020-07-03T17:44:15Z

image/nitf.ksy

+        type: tre_header
+  band_info:
+    seq:
+      - id: n_band_representation


should n_band_ be ommitted?

I had it in there per the doc naming (pg 97 of reference), but have removed.

KOLANICH · 2020-07-03T17:46:23Z

image/nitf.ksy

+      - id: image_date_time
+        -orig-id: idatim
+        type: date_time
+        doc: 'UTC time of image acquisition in the format CCYYMMDDhhmmss: CC century, YY last two digits of the year, MM month, DD day, hh hour, mm minute, ss second'


The format of the time string docs should be moved into the struct representing time.

rmspeers · 2020-07-03T17:58:57Z

FYI for a basic single image test see attached rgb16.ntf:

rgb16.ntf.zip

generalmimon · 2020-07-03T19:16:18Z

image/nitf.ksy

+      - id: image_data_mask
+        type: image_data_mask
+        if: has_mask
+      - id: image_data_field
+        size: _parent.header.linfo[idx].li.to_i - image_data_mask.total_size


Parsing of the file rgb16.ntf you provided in #294 (comment) fails due to image_data_mask being null (or more specifically, undefined in JavaScript - tested on https://ide.kaitai.io/devel/), because you're unconditionally reading its total_size attribute, whether it is null or not. Shouldn't it have if: has_mask as well?

… operator.

rmspeers · 2020-07-04T14:37:15Z

@generalmimon / @KOLANICH - let me know if you see any other issues to resolve still.

image/nitf.ksy

Apply suggestions from PR. Co-authored-by: Petr Pučil <[email protected]>

generalmimon · 2020-07-04T17:02:14Z

image/nitf.ksy

+      - id: sctlh
+        type: str
+        size: 2
+      - id: srel
+        type: str
+        size: 20
+      - id: sdctp
+        type: str
+        size: 2
+      - id: sdcdt
+        type: str
+        size: 8
+      - id: sdcxm


I see that you improved a lot of field namings in 77c94a2, but I still feel that these (and similar ones) deserve a more meaningful id.

Anything else we need to do or is this good to merge?

generalmimon · 2020-07-04T17:03:49Z

image/nitf.ksy

+doc: |
+  The NITF (National Image Transition Format) format is a file format developed by the U.S. Government for
+  storing imagery, e.g. from satellites.
+
+  According to the [foreword of the specification](https://gwg.nga.mil/ntb/baseline/docs/2500c/2500C.pdf):
+  > The National Imagery Transmission Format Standard (NITFS) is the suite of standards for formatting digital
+  > imagery and imagery-related products and exchanging them among members of the Intelligence Community (IC) as
+  > defined by the Executive Order 12333, and other United States Government departments and agencies."


@rmspeers A lot better! 👍

generalmimon

Overall looks good to me, you did a nice job! Just one small thing before we merge it (feel free to just apply the suggestion).

image/nitf.ksy

Co-authored-by: Petr Pučil <[email protected]>

generalmimon

@riverloopsec @rmspeers Thanks for your huge contribution, merging in!

TACIXAT and others added 2 commits June 7, 2020 09:13

Add NITF.

7371f16

Implemented license on NITF

3d4e22f

KOLANICH reviewed Jun 10, 2020

View reviewed changes

Updated field namings, added some doc strings.

77c94a2

KOLANICH reviewed Jul 3, 2020

View reviewed changes

PR comments

830cfdf

generalmimon reviewed Jul 3, 2020

View reviewed changes

Added if has_mask on image_data_field (to confirm); change to _sizeof…

a1f7626

… operator.

generalmimon reviewed Jul 4, 2020

View reviewed changes

image/nitf.ksy Outdated Show resolved Hide resolved

generalmimon reviewed Jul 4, 2020

View reviewed changes

image/nitf.ksy Outdated Show resolved Hide resolved

generalmimon reviewed Jul 4, 2020

View reviewed changes

image/nitf.ksy Outdated Show resolved Hide resolved

generalmimon reviewed Jul 4, 2020

View reviewed changes

image/nitf.ksy Outdated Show resolved Hide resolved

generalmimon reviewed Jul 4, 2020

View reviewed changes

image/nitf.ksy Outdated Show resolved Hide resolved

generalmimon reviewed Jul 4, 2020

View reviewed changes

image/nitf.ksy Outdated Show resolved Hide resolved

generalmimon reviewed Jul 4, 2020

View reviewed changes

image/nitf.ksy Outdated Show resolved Hide resolved

Ryan Speers and others added 2 commits July 4, 2020 12:33

Apply suggestions from code review

fb0333d

Apply suggestions from PR. Co-authored-by: Petr Pučil <[email protected]>

Added more description.

7005026

generalmimon reviewed Jul 4, 2020

View reviewed changes

Update NITF with more descriptive IDs than original from spec.

62bb489

generalmimon requested changes Jul 6, 2020

View reviewed changes

image/nitf.ksy Outdated Show resolved Hide resolved

Update image/nitf.ksy

470d56b

Co-authored-by: Petr Pučil <[email protected]>

generalmimon approved these changes Jul 6, 2020

View reviewed changes

generalmimon merged commit b86f71e into kaitai-io:master Jul 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add nitf #294

Add nitf #294

riverloopsec commented Jun 8, 2020 •

edited

Loading

KOLANICH Jun 10, 2020

rmspeers Jul 3, 2020

KOLANICH Jun 10, 2020 •

edited

Loading

rmspeers Jul 3, 2020

KOLANICH Jun 10, 2020

generalmimon Jul 3, 2020 •

edited

Loading

rmspeers commented Jul 3, 2020 •

edited

Loading

KOLANICH Jul 3, 2020

rmspeers Jul 3, 2020

KOLANICH Jul 3, 2020

rmspeers Jul 3, 2020

KOLANICH Jul 3, 2020

rmspeers Jul 3, 2020

rmspeers commented Jul 3, 2020

generalmimon Jul 3, 2020 •

edited

Loading

rmspeers commented Jul 4, 2020

generalmimon Jul 4, 2020

rmspeers Jul 5, 2020

rmspeers Jul 6, 2020

generalmimon Jul 4, 2020

generalmimon left a comment

generalmimon left a comment

Add nitf #294

Add nitf #294

Conversation

riverloopsec commented Jun 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KOLANICH Jun 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

generalmimon Jul 3, 2020 • edited Loading

Choose a reason for hiding this comment

rmspeers commented Jul 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmspeers commented Jul 3, 2020

generalmimon Jul 3, 2020 • edited Loading

Choose a reason for hiding this comment

rmspeers commented Jul 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

generalmimon left a comment

Choose a reason for hiding this comment

generalmimon left a comment

Choose a reason for hiding this comment

riverloopsec commented Jun 8, 2020 •

edited

Loading

KOLANICH Jun 10, 2020 •

edited

Loading

generalmimon Jul 3, 2020 •

edited

Loading

rmspeers commented Jul 3, 2020 •

edited

Loading

generalmimon Jul 3, 2020 •

edited

Loading